# A recomputed config delta clears an unchanged access rule list

- **Status**: Done
- **Kind**: Bug
- **Authors**: jp
- **Date**: 2026-09-08

On a continuing conversation, a `--cfg` that changes `access.env` for a tool
also clears that tool's `access.fs` rules.
An empty `fs` list means unrestricted (workspace-confined) access, so the tool
silently gains filesystem reach the user never granted, and the widening
persists for every later turn.

The same happens in reverse, and for any pair of sibling rule lists: whichever
one did *not* change is the one that gets cleared.

## Why

`get_config_delta_from_cli` (`crates/jp_cli/src/cmd/query.rs:1860`) diffs the
conversation's config against the invocation's.
For a rule list that did not change, `rule_delta`
(`crates/jp_config/src/conversation/tool/access.rs:141`) returns an empty append
delta — `MergeableVec::Vec([])`, meaning "nothing to add".

`PartialAccessConfig` does not override `delta_with_unsets`, so it takes the
default at `crates/jp_config/src/delta.rs:37` and reports no cleared paths.
With `unsets` empty, `ConversationStream::add_config_delta`
(`crates/jp_conversation/src/stream.rs:587`) takes its re-diff branch and calls
`config.to_partial().delta(*delta)` — diffing the *delta* as though it were a
snapshot.

On that second pass the empty list is no longer "nothing to add"; it reads as a
complete rule set of zero rules.
`[].starts_with(&[rule])` is false, so `rule_delta` falls through to
`Replace([])`, and the fold resolves the field to empty.

An empty `MergeableVec` cannot distinguish "no change" from "cleared", so the
two passes are entitled to opposite readings of the same value.
No tweak to `rule_delta` alone removes the ambiguity.

## Scope

Not introduced by the rule-ordering fix in #1126.
Substituting the previous `rule_delta` body, an unchanged list also returned
`Vec([])` (`prev.iter().all(|r| next.contains(r))` holds), and the second pass
also emitted `Replace([])`.

The trigger is wider than a reorder: any delta carrying an access block whose
sibling list is unchanged, including a plain `--cfg` append of one env rule.

Only fields that never report an unset are exposed.
A delta carrying at least one cleared path is stored verbatim, so plain lists
going through `delta_opt_vec_at` are already immune.
Access rule lists are reachable precisely because they report nothing.

## Fixes

Two candidates, both defensible.

The root fix is to stop re-diffing a delta that was already computed against the
right state.
`add_config_delta` has the shape of the answer — it stores a delta as given
when `unsets` is non-empty, for exactly this reason — but it infers "already
computed" from a side effect instead of being told.
Carrying that explicitly on `ApplyDelta` fixes the whole class, including field
types added later.

The narrower fix is to give `PartialAccessConfig` a `delta_with_unsets` that
reports the field's path whenever `rule_delta` falls back to `Replace`, threaded
down through the tool and conversation partials the way the other reporting
types already do.
That is the `unsets` mechanism used as designed: a field emitting `Replace` is
saying merging cannot reach the value.
It leaves the second pass in place for any future field that states a strategy
without reporting one.

The root fix is preferable: the map and list conversions in flight put more
strategy-carrying fields on this path.

## Verifying

The delta law suite in `crates/jp_config/src/delta_law_tests.rs` cannot catch
this — it runs one delta-and-merge cycle.
A regression needs the query's calculation and `add_config_delta` together: a
conversation holding `fs` rules, an invocation changing only `env`, and an
assertion that the resolved `fs` rules survive.

## Comments

-----

- **From**: jp
- **Date**: 2026-09-08T10:01:44Z

Fixed by making `ConversationStream::add_config_delta` record what it is given.

## Corrections to the report

**The report describes #1126's branch, not `main`.** `delta_law_tests.rs` and
the `starts_with` body of `rule_delta` only exist there.
The Scope section's both-versions check holds, so the conclusion stands either
way.

**A worse case sits next to the reported one.** Adding a rule drops the rules
already there, which needs no sibling list at all.
A conversation granting `access.fs = [src]` and an invocation granting `[src,
docs]` produce the append-shaped delta `Vec([docs])`; re-read as a complete rule
set that says the tool has exactly one rule, and the fold resolves `fs` to
`["docs"]`.
Confirmed by `appending_an_access_rule_keeps_the_rules_already_there`, which
fails with `["docs"]` against the old body.

**`conversation.labels` was exposed too**, through `labels_delta`: an unchanged
map re-diffs to every key dropped, so the whole map is replaced with an empty
one.
`conversation.attachments` was not — `attachments_delta` has no replace
fallback.

**Four callers passed a raw partial, not a delta** (`config/set.rs:58`,
`summarize.rs:67`, `inquiry.rs:286`, `query.rs:611`), so for them the *first*
pass was already the category error.
`jp config set` on a conversation with label rules cleared them with no `--cfg`
naming labels.

**`ConversationStream::extend` double-diffed as well** (`stream.rs:1608`),
corrupting stream reconstruction — which is how a summary request is built.
Confirmed by `extending_a_stream_reproduces_the_config_of_each_event`.

**Plain lists were immune for a different reason than stated.** Not because a
delta carrying unsets is stored verbatim, but because `Option<Vec<T>>` has a
`None` that means "no change" and is distinct from `Some(vec![])`.
That is the invariant the fix restores.

## What landed

Neither proposed candidate.
The narrow fix cannot work: for the four raw-partial callers,
`delta_with_unsets` is never on the path.
The root fix turned out not to need a flag on `ApplyDelta` either — once
nothing re-diffs, a sparse partial *is* a valid delta ("merge these values"), so
the two inputs stop needing to be told apart.
`add_config_delta` now drops an apply that carries nothing and appends
everything else.

`add_config_reset` no longer duplicates the append: it existed separately only
because `add_config_delta` resolved the stream config, and
`test_add_config_reset_appends_reset_then_nonempty_layers` fails against the old
body, which proves the reason is gone.

Two effects beyond the bug: the stored JSON gets *smaller* (the re-diff wrapped
lists in a `{value, strategy, discard_when_merged}` envelope that is now just
the list), and `extend` loses a full config resolution per event, which was
O(n²) folds over a stream.

## Behaviour change to be aware of

`jp config set` with a value the conversation already resolves to now records
the delta instead of dropping it, pinning it against later workspace changes.
Pinned by `set_in_conversation_records_a_value_already_in_effect`.

## Coverage

Six tests, each watched failing against the old body before being kept:
`appending_an_access_rule_keeps_the_rules_already_there`,
`changing_env_rules_keeps_the_fs_rules`,
`extending_a_stream_reproduces_the_config_of_each_event`,
`a_stored_delta_holds_only_the_fields_that_changed` (exact JSON),
`without_an_unset_a_dropped_argument_appends_instead`, and
`set_in_conversation_records_a_value_already_in_effect`.

`Config Delta` is now in `docs/architecture/ubiquitous-language.md`, with the
delta-versus-snapshot distinction this bug came from.

## Still open

The ambiguity itself survives: `delta_mergeable_vec` and `delta_mergeable_map`
still cannot tell "nothing to add" from "cleared".
Nothing re-diffs a delta any more, so it is unreachable rather than absent.
#1130 and #1131 put every list and map in the config behind those helpers, so
the follow-up — giving the partial collections a state for "no opinion"
distinct from "empty" — is worth a ticket of its own.

-----

- **From**: jp
- **Date**: 2026-09-08T10:11:36Z
- **Re**: #1

One more consequence, raised in review and worth recording: the old body never
converged.

A conversation granting `access.fs = [src]` against an invocation granting
`[src, docs]` stored `Replace([docs])`, so the conversation resolved to
`["docs"]`.
The next identical run then saw `[docs]` against the wanted `[src, docs]` and
produced `Vec([src])`, which re-diffed to `Replace([src])` and dropped `docs`.
The run after that swapped them back.

So every `jp` run on such a conversation appended a config delta and moved the
resolved rule set, indefinitely.
The `{value, strategy, replace}` envelope in the stored delta was what caused
this, not what prevented it.

`a_repeated_invocation_records_no_second_delta` covers it: the second
invocation's diff must be empty and the stream must still hold one delta.
It fails against the old body with a delta re-adding the dropped rule.

Also recorded: `set_in_conversation_twice_records_two_deltas`.
`jp config set` passes values rather than a diff, so a repeat writes a second
delta where the old body dropped it.
Bounded by explicit invocations, each event the size of the `--cfg` values.

A value-equality check in `set.rs` was considered and rejected: comparing a
sparse partial field-wise against a resolved snapshot is the operation whose
ambiguity caused this bug.
The version that would be correct asks whether the conversation already pins the
value, which needs a fold over its own deltas alone.

-----

- **From**: jp
- **Date**: 2026-09-08T10:23:58Z
- **Re**: #2

Regression fixed in the same branch: a `config set` naming a value already in
effect no longer records an event.

The check is not a field-wise comparison of the override against the resolved
config — that is the sparse-versus-snapshot comparison this whole ticket is
about, and it carries the same ambiguity for every collection field.

`config_pipeline::override_to_record` merges the override onto the current
config, resolves both sides, and compares the resolved values:

```rust
let base = current.to_partial();
let mut merged = base.clone();
merged.merge(&(), overrides.clone())?;

let (Ok(before), Ok(after)) = (build(base), build(merged)) else {
    return Ok(Some(overrides));
};

Ok((before != after).then_some(overrides))
```

Resolved configs rather than partials, because a partial also carries the merge
strategy each field arrived with: two partials holding identical values can
still compare unequal, and resolution normalizes that away.
Both sides go through the same transform, so the comparison does not rest on
`to_partial` round-tripping exactly.
An override whose merged result does not resolve is recorded rather than dropped
— its outcome cannot be compared, so it is not known to be a no-op.

The function names no field and knows no merge strategy; every per-field
decision stays with the `#[setting(merge = ...)]` on the field.

Wired into both persisted override call sites: `config/set.rs` and the
editor-provided config in `query.rs`.
The two throwaway-stream callers (`summarize.rs`, `inquiry.rs`) are left alone;
nothing they append is persisted.

Five tests on the helper, covering how a field merges against whether the
override changes it: replacement restated and changed, a strategy-carrying list
restated and extended, and an appending list restated.
That last one records, which is correct — appending without deduplicating is
not idempotent, so asking for the same element twice is a real change. #1130
gives most lists dedup-by-default and adds `ordered_vec_with_strategy` for the
ones where repetition is meaningful, which splits that case deliberately.

`set_in_conversation_skips_a_value_already_in_effect` and
`set_in_conversation_twice_records_one_delta` cover it end to end.
Both were watched failing first.