Add two-axis ratchet for agent-config PUT; document open content-axis ruling (#7470)

[bolichen97]

Addresses #7470 (agent-config PUT: a stale snapshot can revert a live app bridge to a superseded definition).

The issue has two clearly separable halves, with different readiness levels — the maintainer comment calls this out explicitly. This PR lands the half that needs no ruling and surfaces the half that does.

What landed (ruling-free)

Region-level two-axis existence ratchet. The issue's "A gate, either way" section asks for a test asserting that every key of an app-owned mcpServers region (keys containing :, excluding host-owned names) has its existence decided by on-disk state on both the present-drop axis (#7089) and the absent-preserve axis (#6975), so adding a new owned region without a present-axis rule fails the suite instead of shipping. That test genuinely did not exist. It now does: test_app_owned_region_existence_is_decided_on_disk_on_both_axes.

• It exercises the real PUT commit path via the _put harness (no mocking of the drop / merge / ownership decision).
• The region-level assertions derive their name-sets from the fixture's actual PUT-input dicts, so a future added owned region is swept automatically rather than needing the loop edited.

What is documented, not implemented (needs a maintainer ruling)

The content axis — where an <app>:<server> name is on disk and in the submission, the submitted row still wins, so a stale editor snapshot can revert a bridge the platform already corrected (e.g. a backend.port: "auto" app rewritten to its live port by _resolve_live_mcp_url) — reverses the editor-snapshot-wins contract kept deliberately in #5899 and re-affirmed for #6664. Per the issue and the maintainer comment, whether it may be reversed at all, and if so which candidate shape (on-disk row substitution vs. optimistic-concurrency/etag) closes it, needs a maintainer ruling. #7089 shipped only the deletion half for exactly this reason.

No behavioral source change was made. The handler docstring, the spec decision table, and test_app_owned_entry_present_in_the_snapshot_is_updated already correctly reflect the open ruling. A decision brief assembling everything the decider needs (both candidate shapes with costs, the "breaks EVERY kiro session" blast radius, the self-heal mitigation, and verified current file:line anchors) is added at .agents/tasks/task-agent-config-content-axis-ratchet/CONTENT-AXIS-RULING.md.

Testing

• pytest -q test/test_agent_config_merge_on_write.py → 47 passed (baseline 46, +1 new).
• Related sweep across test_agent_config_merge_on_write.py, test_api_agent_config_put_succeeds.py, test_agent_config_owner_gate_invariant.py → 67 passed.
• Test-quality confirmed: temporarily neutering the present-axis del submitted[name] drop in _drop_unbacked_app_entries makes the new test FAIL on its present-axis assertion; restoring returns to green with git diff src/ empty.
• git diff src/ is empty — the committed diff is exactly two files (the new test and the decision brief).

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 0161a3c3b1cf21e8c85d7626b7447144094e4518 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 0161a3c

Verdict parsed from the review's SHA-scoped output markers for commit 0161a3c3b1cf21e8c85d7626b7447144094e4518.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 0161a3c3b1cf21e8c85d7626b7447144094e4518: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 0161a3c3b1cf21e8c85d7626b7447144094e4518 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound ratchet test, but the "ruling brief" invents an unindexed top-level .agents/ convention, and the ratchet only guards names a future author volunteers.

Watch

• The brief lands in a brand-new top-level .agents/tasks/ dir (absent from the base tree), duplicating the ruling already recorded in app-kit-platform.md's "Not closed here" paragraph, and pins ephemeral branch SHAs (d0fc4f9e5, 6581a04ee) and file:line anchors that rot on the next edit. Repo convention routes exactly this artifact to docs/task-specs/YYYY/MM/<task-id>/ (an indexed archive, covered by docs-lint); a one-off dotdir escapes both, and the next task will copy the pattern.
• The ratchet's set-comprehension clauses range only over this fixture's two dicts — its own comment concedes a genuinely new owned region is covered only if a future author "add[s] that region's names" here. That manual opt-in is precisely the review blind spot the test narrates (fix(dashboard): merge-on-write for app-owned entries in agent-config PUT #6975 never asking about the mirror axis), so the "complete because it IS complete" claim overstates what the gate enforces.

Suggestions

• Fold the brief's durable content (the ruling question, the two candidate shapes) into the existing spec paragraph or docs/task-specs/, and drop the verification transcript/SHA ephemera; the GitHub issue already carries the process record.

[DESIGN-REVIEWED] 0161a3c

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 0161a3c3b1cf21e8c85d7626b7447144094e4518 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0161a3c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 0161a3c3b1cf21e8c85d7626b7447144094e4518: <one-sentence reason>

[bolichen97]

Closing — already landed on main

Verified relationship: already on main``

I decomposed #7970 into five parts before reading the claim's conclusion. (1) Source/behaviour: none — 2 files, +255/-0, and both rules it asserts are already on main: _drop_unbacked_app_entries at src/kiro_crew/dashboard/handlers/agents.py:244 (f7d6701 = merged #7465) and _merge_unowned_servers at :331 (96decec = merged #6975); I confirmed both commits in the landed-commit index for main and as ancestors of the PR's own merge-base 6581a04, so this is not a stacked artifact (3 commits, all its own, all tagged #7470). (2) The new test test_app_owned_region_existence_is_decided_on_disk_on_both_axes is the same test as main's test_both_directions_decide_from_one_baseline_in_a_single_put (test/test_agent_config_merge_on_write.py:1263): identical on_disk map {"demo:notes": {"command": "notes-mcp"}, "plainuser": {"url": "u"}}, identical apps={"demo": ("notes",)}, identical enabled=True harness default, and the same three assertions. Only the unbacked submitted name differs (gone:tool there, demo:custom here), and that is not a second code path: _drop_unbacked_app_entries decides purely on ":" in name and name not in existing and name not in host, its docstring states "THE DECLARED-NAME CENSUS IS DELIBERATELY NOT CONSULTED", the _put harness docstring names demo:custom as its own example of a non-owned name, and main already uses that exact name in test_client_entry_under_an_installed_apps_namespace_is_deleted (:710). (3) The one novel element, the region-level loop, derives on_disk_names/submitted_names from the same two literal fixture dicts the explicit assertions already name, so resurrected evaluates to exactly assertion 1 and dropped to exactly assertion 2 — zero added coverage, as the test's own comment concedes ("it does not conjure an unrelated owned region out of nothing"). (4) The brief's core claim is on main twice verbatim: docs/system-specs/modules/app-kit-platform.md:288 "Not closed here: ... it self-heals on the next gateway start, so it is left to a separate ruling", plus agents.py:287 "WHAT THIS DELIBERATELY DOES NOT DO: it never rewrites a submitted value ... Reversing it needs a maintainer ruling and is tracked separately"; the decision-table row (:265), the blast-radius sentence (agents.py:285) and the pinning test (:1034) are all there too. (5) Only the brief's two candidate shapes with costs are not on main — and that is doc-only prose at .agents/tasks/…, a path that has never existed on main (its creating commit 6f79ed0 lives only on refs/remotes/pr/7970), while AGENTS.md routes task specs to docs/task-specs/YYYY/MM/${task-id}/ and forbids creating extra markdown files; the file even embeds its own branch name and commit SHAs d0fc4f9e5/6581a04ee. That is a harvestable remainder, not a capability main structurally cannot deliver.

This was adjudicated twice, independently; the second reviewer reached the same ruling (already on main``). Their strongest corroborating fact:

origin/main:test/test_agent_config_merge_on_write.py:1263 test_both_directions_decide_from_one_baseline_in_a_single_put already does one PUT off the identical baseline {"demo:notes": {"command": "notes-mcp"}, "plainuser": {"url": "u"}} with apps={"demo": ("notes",)} and the same three assertions the PR's test_app_owned_region_existence_is_decided_on_disk_on_both_axes makes; the absent axis is separately pinned at :380 test_app_owned_entry_absent_from_the_snapshot_is_preserved and :735 test_a_declared_app_server_is_still_preserved. || origin/main:src/kiro_crew/dashboard/handlers/agents.py:320-325 — the drop predicate is name for name in submitted if ":" in name and name not in existing and name not in host, and its docstring (:287-292, :299-306) says the declared-name census is deliberately not consulted and that reversing the content axis "needs a maintainer ruling and is tracked separately", so demo:custom and gone:tool take one branch and the brief's headline point is already in-tree. || git ls-tree origin/main -- .agents is empty and git grep -l CONTENT-AXIS-RULING origin/main exits 1, while f7d6701a1 (#7465) and 96decec0d (#6975) are both --is-ancestor origin/main = YES; #7470 is an open ISSUE (the issue/PR reference check), so it carries no code.

Evidence

test/test_agent_config_merge_on_write.py:1263 on origin/main — test_both_directions_decide_from_one_baseline_in_a_single_put already does one PUT off on_disk={"mcpServers": {"demo:notes": {"command": "notes-mcp"}, "plainuser": {"url": "u"}}} with apps={"demo": ("notes",)} and asserts the unbacked namespaced submitted name is absent, written["mcpServers"]["demo:notes"] == {"command": "notes-mcp"}, and plainuser untouched — the identical three assertions PR #7970 re-adds with demo:custom in place of gone:tool. src/kiro_crew/dashboard/handlers/agents.py:287 on origin/main — "WHAT THIS DELIBERATELY DOES NOT DO: it never rewrites a submitted value ... Reversing it needs a maintainer ruling and is tracked separately", mirrored at docs/system-specs/modules/app-kit-platform.md:288 ("Not closed here: ... left to a separate ruling"), so the open content-axis ruling the brief documents is already recorded on main in both the code and the spec. _drop_unbacked_app_entries filters on name for name in submitted if ":" in name and name not in existing and name not in host with no census lookup, so demo:custom (installed app, undeclared server) and gone:tool (uninstalled app) traverse the same branch to the same written map — the PR's test cannot fail where main's passes.

Why this one and not the other

Yes. The survivor is main as it stands, via merged #7465 (f7d6701), which landed both the _drop_unbacked_app_entries source rule and the equivalent single-PUT both-axes test; #6975 (96decec) is correctly only partial, since it shipped the absent axis alone and its suite therefore does not carry the present-axis assertion. #7970 could never be the better survivor: it changes no source and its test only passes because #7465's rule is already merged. The claim's framing is accurate on both counterparts.

Carry this over first

This closure is about redundancy, and these items are the exception: they are not on main and not in the surviving PR, so they need a home before the topic is finished. Please don't let them go with the branch.

One item, doc-only: the brief's candidate-shape analysis for issue #7470, which is the PR's only content not mirrored on main. Paste it as a comment on issue #7470 (no doc on main references #7470 at all) before closing — specifically Shape 1 (substitute the on-disk row inside _drop_unbacked_app_entries, ~6 lines plus flipping the assertion in test_app_owned_entry_present_in_the_snapshot_is_updated at test/test_agent_config_merge_on_write.py:1034) and Shape 2 (etag/If-Match optimistic concurrency) together with the three landing-path facts I independently verified on main: no etag/If-Match/version field anywhere in src/kiro_crew/dashboard/handlers/agents.py, saveAgentConfig: (config: object) => put('/api/agent/config', { config }) at website/src/api/client.ts:2774, and useEffect(() => { if (loadedCfg && !cfg) setCfg(loadedCfg) }, [loadedCfg, cfg]) at website/src/pages/overview/AgentCfgTab.tsx:18, which fills the edit buffer once so a stale-PUT rejection has nothing to land on. Nothing from the test file needs carrying over.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. Findings that implied a closure were re-adjudicated independently, including an adversarial pass whose only job was to refute them; the reasoning above is what survived. If it is wrong, reopening costs nothing — please say so, and treat the reasoning rather than the outcome as the thing to correct.}