fix(ops-mission-control): never publish the incident index or app config over a failed read

[chenmingwei23]

Two reads in ops_mission_control collapse every failure to an empty document and then feed a
whole-file rewrite, so one transient read error makes the app believe a store is empty and publish
that emptiness back over it.

1. What is the problem?

store._read_index_unlocked and providers.read_config both return {} on any failure. As DISPLAY
reads that is correct -- the board must render on an index it could not load. But they are also the BASE
of a read-modify-write that rewrites the whole file, where empty means "delete every incident".

The index is not a view, it is the CLAIM ledger: claim is a compare-and-set against those rows.
Emptied, every signal reads as unowned, so the next heartbeat re-claims alarms already being worked and
opens a duplicate investigation of each.

An emptied config does not error, which is what makes it quiet: provider_enabled defaults to False,
so polling stops for every provider the operator switched on while their credentials stay in the
keystone store and Settings still shows each one as configured.

2. Why this issue matters to the user

The failure is silent, durable, and looks like good news: a clean board and no firing signals is the
same picture as a quiet night. Nothing errors, nothing is logged, and the in-flight incidents are gone
from disk. In act mode, duplicate investigations mean duplicate real writes to PagerDuty or Datadog.

3. How our fix solves it

Each module gains a private *_for_update reader used only as the base of a mutation, in which a
missing file still reads as empty (nothing has been written yet, so empty is the truth), and every
other way of failing to read it propagates so the mutation is abandoned:

• unreadable -- EACCES, EIO, a scanner holding the handle on Windows
• unparseable -- malformed JSON
• anything the load would silently rewrite -- checked structurally, not by enumerating shapes
  That third rule is the one worth reviewing. Three review rounds each found the same loss one layer
  deeper: the document root, then a row, then a nested field ("signal": [] coercing to an empty
  Signal). Every fix that enumerated a shape was beaten by a deeper example. So the index reader now
  deserializes, re-serializes, and refuses if anything on disk did not survive -- comparing parsed
  structures so key order cannot cause a false refusal. It made the reader SMALLER: the per-row and
  per-field isinstance guards are deleted, because dropping a row or blanking a field is exactly what the
  round trip detects.

The guarantee is "never publish over a read that failed", and a read that silently rewrote what it
could not understand is a read that failed.

Corruption propagating is a deliberate divergence from four merged siblings (library.py:95,
shares.py:60, secrets.py:231, policy_store.py:147), which read an unparseable document as empty.
Their rationale is real -- such a document carries nothing to merge into. The counter is stronger:
"cannot merge into" is not "safe to destroy." A truncated file still holds most of its records.
Tracked in #7805, secrets.py first, where the discarded bytes are provider credentials that exist
nowhere else. A fifth reader shares the SHAPE (ledger_index._read_cursor) and is exempt on the merits:
its write is a set UNION, so an empty read only adds ids and can never drop one.

The display reads stay lenient -- failing a render would turn a recoverable file into an unusable app
-- and now LOG every degradation other than an absent file, structural ones included.

Caller policy differs by what has already happened. claim raises on everything -- a
compare-and-set has no safe degraded answer. dispatch.run_cycle's maintenance passes and
slot_watch.reconcile tolerate OSError (transient) but refuse corruption (persistent).
routes._schedule_verification and the post-claim ledger annotation REPORT instead: their irreversible
step has already happened, so raising would call a completed action failed and invite a retry that writes
twice. Both log and audit the degradation.

json.JSONDecodeError subclasses ValueError, so an existing except ValueError silently claims
corruption and reports it as a validation error. All ten such arms were audited: one was reachable and
wrong (_handle_propose answered 400 invalid_proposal on a corrupt store), three already carried the
clause, six cannot reach a strict reader. One shared _store_read_refusal maps three conditions at five
sites: 503 *_unreadable (retry works), 500 *_corrupt (repair the file), and 409 *_version_skew -- a
file written by a NEWER build after a rollback, refused so the write cannot strip the newer field.

4. What tests we did

Is this change workable? The mechanism, not a count. Revert one line and a named test reddens:

Revert	Test that reddens	What the redness means to the operator
_read_index_for_update's except FileNotFoundError: back to return {}	test_a_read_that_failed_never_truncates_the_index	Two claimed incidents vanish from disk; the board comes back clean while both investigations are still in flight. Fails on "a failed read was published back over the index", a byte comparison against a snapshot.
_coerce_index's round-trip equivalence refusal	test_a_malformed_nested_field_is_not_silently_replaced (3 subtests) and test_a_skipped_entry_is_not_silently_deleted_by_the_next_mutation	An incident whose signal is malformed comes back with the signal silently BLANKED and the original gone from disk -- source, native id, title, labels. Disabling the one rule reds all four, which is the evidence it covers the row and nested-field layers together rather than by enumeration.
merge_provider_config's per-slot refusal	test_a_providers_key_of_the_wrong_shape_refuses_the_merge	Every provider slot is wiped because one key was a list instead of an object.
Move _handle_propose's corruption arm AFTER its ValueError arm	test_a_corrupt_index_is_not_reported_as_an_invalid_proposal	A corrupt store is reported as bad INPUT (400), so the operator re-types the form while the real fault sits on disk. Fails with 400 == 400.
Drop the audit entry in _schedule_verification	test_a_corrupt_index_is_audited_rather_than_raising_after_the_action_ran	An executed production action is left with no recheck scheduled and nothing anywhere says so.

Each was run in that reverted state and observed red, then restored. Twenty-eight probes in total; two
of them caught a test passing for the wrong reason, both from asserting something weaker than the
claim being made.

45 new tests. App suite 986 passed, 44 skipped, 266 subtests (from 921 on main). Run with
-o addopts="" to avoid the baked-in xdist parallelism. All seven repo gates green, including
mypy src/kiro_crew/ (4 errors, all pre-existing).

5. Any other suggestions on the work

1. Two commits, deliberately. The black-baseline prune is separate because AGENTS.md says
   formatting a baselined file "belongs in its own commit". MAX_COMMITS is 2.
2. Follow-ups filed, not promised in prose: Four merged update readers replace a corrupt file instead of refusing; secrets.py first #7805 (the merged siblings, secrets.py first,
   plus the exception-type decision), Three remaining lenient-read-feeding-whole-file-rewrite sites, and a ratchet to close the class #7789 (remaining sites), ops-mission-control: store-write failures answer a bare untyped 500, not the app's coded error #7790 (handlers answering a bare
   untyped 500 for other reasons).

Pattern harvest

A lenient read is safe as a DISPLAY read and unsafe as a MUTATION BASE, and the same function is often
both. The discriminator is not the read -- most lenient reads in src/ are legitimate -- it is whether
a whole-file WRITE derives from it. The second lesson cost three review rounds: a guard that enumerates
bad SHAPES is defeated by a deeper example every time, so the check has to be about CONTENT survival.

Rule candidate: for any read -> mutate -> write_all, the read must tell "absent" from "unreadable",
and must refuse when deserializing would discard content the file already held.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound root-cause fix; the round-trip refusal rule quietly makes serializer idempotence and schema stability load-bearing availability constraints humans should ratify.

Watch

• The equivalence rule "outlaws migrate-on-write" (its own docstring): the next field rename/retire makes every old record read as version skew and refuses all mutations until a carve-out ships. Documented at the bite point, but it converts a routine schema change into a coordinated two-step — decide now that this cost is accepted.
• Strictness trades silent truncation for a store-wide mutation halt: one corrupt row blocks claim for every unrelated firing signal, and the only remedy shipped is hand-repairing JSON ("error": "…must be repaired"). The board still renders, so the halt is visible only in logs and per-action 500s. Right trade, but the operational story is "app goes deaf during corruption" with no recovery affordance.
• Correct routing hangs on catch-clause ordering (JSONDecodeError before ValueError) — audited by hand at ten arms, pinned by tests at the fixed sites, but nothing gates the next except ValueError someone adds. Four merged update readers replace a corrupt file instead of refusing; secrets.py first #7805's exception-type decision (a non-ValueError type would delete this trap class) is the real fix; don't let it slip.

[DESIGN-REVIEWED] 6d0bcd4

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 6d0bcd4960dd648ec21cc1ec595a0a2760ec38c7 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/apps/builtins/ops_mission_control/backend/routes.py:1029 -- "except json.JSONDecodeError" classifies UnknownFieldError after rollback as corruption in the audit log -> Fix: catch UnknownFieldError first and record version-skew wording.
[GPT-REVIEWED] 6d0bcd4

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 6d0bcd4960dd648ec21cc1ec595a0a2760ec38c7 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. The change decomposes cleanly, every item traces to the named data-loss cause or to an AGENTS.md-mandated invariant (coded non-2xx bodies), the sibling count is declared and tracked, and the one piece of new surface (UnknownFieldError) has a counted consumer. Final review:

First-Principles-Verdict: PASS

One cause — a lenient read used as the base of a whole-file rewrite — fixed at the reader contract, with every rider declared and derived from it.

What this change ships

Intent: stop one transient read failure from silently deleting every incident and every provider setting on disk — a FIX.

1. Index mutations refuse a failed/corrupt read instead of truncating the store — justified (the fix)
2. Config mutations get the same refusal — justified (same cause, second store)
3. Shape guards replaced by round-trip content-survival check — justified, cause-level, net-smaller
4. Key/incident-id mismatch refused; expiry writes back under the read key — justified (same cause at write time)
5. Newer-build fields refuse as 409 version-skew, not corruption — justified; 1 counted consumer (routes.py:212)
6. Mutating routes answer coded 503/500/409 via one shared mapper — mandated (AGENTS.md code-field invariant)
7. Corruption no longer misreported as 400/409 operator error (propose, transition) — justified (symptom of JSONDecodeError ⊂ ValueError, audited at all ten arms)
8. Heartbeat/reconcile/hygiene tolerate the new refusals instead of aborting the cycle — justified (fix must not trade data loss for outage)
9. Post-action failures audited rather than raised (verification, ledger annotation) — justified (retry would double a real write)
10. Display reads now log degradations they previously swallowed — declared, harm named (silent failure reads as health)

The four unfixed siblings of the root cause (library.py, shares.py, secrets.py, policy_store.py — verified: each still reads unparseable as empty) are counted in the description and tracked in #7805 with the credentials store first; accepted-and-deferred, not a gap. The black-baseline prune rides along but follows the documented own-commit rule. No undeclared items survived the sweep; no existing mechanism does this job (the *_for_update split is this repo's established idiom, hand-rolled per store).

[FIRST-PRINCIPLES-REVIEWED] 6d0bcd4

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 6d0bcd4960dd648ec21cc1ec595a0a2760ec38c7 — this comment is updated in place on each push.

Review details

Both candidates fail the falsification bar.

Candidate 1 (strict round-trip wedge on a non-string label): I confirmed Signal.create stores labels via dict(labels or {}) with no coercion while Signal.from_dict does {str(k): str(v)}, so a non-string label value would indeed fail _lost and wedge every mutation. But I opened every shipped label producer — cloudwatch.py (name/namespace/metric/region/state all str(...)), pagerduty.py (str(...)), datadog.py, github_issues.py, and webhook._normalize_labels (out[str(key)[:100]] = str(value)[:200], covering the one external/untrusted ingress) — and every value reaching Signal.create is stringified. The wedge requires a companion provider violating the dict[str, str] contract or a hand-edit. That is an "if a caller were to" input, not one that occurs in practice, and test_the_serializer_round_trips_every_shape_the_app_writes pins idempotence on shipped shapes. Fails (a).

Candidate 2 (hygiene route answers an uncoded 500 on corruption): the except OSError-only clause and the escaping CorruptDocumentError are real, but they are deliberate — the in-diff comment states corruption "is deliberately NOT caught: that never self-heals, so it must stop the cron loudly," and test_a_corrupt_index_still_stops_the_hygiene_pass pins exactly that escape. It is consistent with run_cycle, verify_pending_actions, and reconcile, which all re-raise corruption while tolerating transient OSError. This is intended behavior, not a defect; the candidate's own consequence is "an inconsistency with the coded-body convention rather than a data-safety bug." Fails (c).

No grounded self-originated finding survived falsification.

No findings.

[OPUS-REVIEWED] 6d0bcd4

Verdict parsed from the review's SHA-scoped output markers for commit 6d0bcd4960dd648ec21cc1ec595a0a2760ec38c7.

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

[chenmingwei23]

Round 2 dispositions -- #7794

Pushed ad6808ffe. Design Review found a real hole in my own guard, and it also correctly
diagnosed why my tests could not have caught it.

Design Review -- CONCERNS, the guard is bypassed on any busy install

Real, verified, and the most useful finding on either PR. Confirmed at
dispatch.py:657: the pre-filter is index = await asyncio.to_thread(store.read_index) -- the
LENIENT read. So on an unreadable index it returns {}, owned is empty, every firing signal
becomes a candidate, and store.claim raises at the loop body before webhook.ack (726), my
guarded sweep (741), slack_out.publish_all (763), _notify_cycle_changes (770) and the cycle's
SEL entry. The guard I added for the maintenance passes was unreachable on exactly the installs
where it mattered -- anything actually firing.

And the transient case is worse than the persistent one, as you said: a signal claimed earlier in
the same loop is durably on disk, so escaping meant an in-flight investigation that is never
mirrored to Slack, never notified and never audited.

Fixed as suggested -- the claim loop catches OSError, logs, and breaks. break rather than
continue because whatever stopped this claim stops every remaining one identically. claim
itself still raises, so the asymmetry I defended is intact: a compare-and-set has no safe degraded
answer, since None already means "another instance owns this signal".

Your critique of my tests was the part I most needed. TestAMaintenancePassCannotCostTheCycle
stubbed store.expire_stale_proposals / store.sweep_stale directly, which proves the guards work
but not that they are reachable -- and they were not. Two end-to-end tests now drive run_cycle
with a firing candidate and a genuinely unreadable index instead of stubbing the store: one for the
persistent case (the cycle must reach its end and report rather than escaping) and one for the
transient case (a claim already made must survive in the result). The second arms the fault the
moment the index actually holds an incident rather than after a fixed number of reads, so it stays
anchored to "there is now something to lose" instead of to a read count that varies with the
maintenance passes. Mutation-verified: removing the claim-loop guard reds both with
PermissionError escaping.

prune_closed aborting hygiene before the push -- acknowledged, and your framing is right. My
deferral rationale ("main already 500s there") was about the write, not this newly likely read, and
that distinction is fair. The remedy lives in routes.py, which this PR deliberately does not touch
so the two diffs stay disjoint and cannot conflict on that file -- #7788 adds the coded-error helper
there. Carried into #7790 with the ordering consequence stated, rather than left as a status-code
note.

Suggestion: log the silent display degradations -- taken. Both display readers now split the
except: an absent file stays silent (that is a fresh install), and any other failure logs a warning
naming the consequence -- "the board will render empty", and for the config "every provider will read
as unconfigured and polling will stop". That closes the half of the harm I named myself and had left
unaddressed: the config failure looks exactly like health, and nothing else would prompt an operator
to look.

GPT 5.6 -- no blocking; FINDING on the guard comments

Correct, and fixed. My comments said "the index refused to be READ", but except OSError also
catches the sidecar lock and the atomic write, both of which could always raise -- so the comment
misdiagnosed a whole class of failure as unreadability. Both handlers now describe generic
index-maintenance I/O failures, note that the strict read only made the case common rather than
possible, and state why the durable state is intact either way (a failed read abandons before
writing; a failed write leaves the previous document). The log messages changed to match.

First Principles -- PASS

ledger_index._read_cursor is the one remaining instance, and its self-healing direction is why it
is filed rather than fixed: #7789 carries it alongside aws_control/backend/backup.py, which is the
one with durable harm.

Opus 4.8 -- no findings

Noting one thing for the record: Opus considered the claim-loop candidate and dropped it as "a comment
overstating scope, not a reachable wrong outcome". Design's version was stronger and correct, because
it named the observable loss -- the mirror, notification and audit for incidents already claimed this
cycle. The two lanes disagreed and the more specific one was right.

---

App suite 936 passed, 44 skipped, 257 subtests (+15 versus main). isort/flake8 clean on all 6
files. Five mutation probes, each reverting to red.

[chenmingwei23]

Diagnosis of the one edited test, and a property it led me to

Pushed 8da800751. This addresses a fair challenge: a locking-atomicity test that has to be edited
to accommodate a fix is usually a sign the fix changed the lock's shape. So here is the check rather
than the assurance.

The lock's shape is unchanged, and the diff is the evidence. Across all six mutations in
store.py, no with _IndexLock(): line is added, removed, or moved. The only change inside any lock
block is the reader's NAME:

-        index = _read_index_unlocked()
+        index = _read_index_for_update()

The read stays inside the lock, the write stays inside the lock, the span is identical, and
_read_index_for_update contains no lock primitives of its own -- it is a read_text plus
json.loads and a shared coercion helper. TestProposalExpiryIsAtomic::test_the_whole_sweep_is_locked
guards that the sweep runs under ONE lock and does not re-enter it through update_fields; all three
of its other assertions still pass unmodified. What broke was the single assertIn that names the
reader by string literal.

I repointed that literal and added assertNotIn("_read_index_unlocked()"), so the guard now
rejects strictly more than it did: the relocking path as before, plus any future edit that puts the
lenient reader back under the lock. Verified by reverting only the sweep's reader, which reds it.

But the challenge did surface something real, and it is the better find. The strict read makes
"raise while holding _IndexLock" reachable for the first time. _read_index_unlocked swallowed
every failure, so before this change no mutation could leave that block by exception -- and nothing
had to be exception-safe about the release, because nothing could test it.

That matters more than an ordinary regression would: flock is per-descriptor, so a leaked lock
deadlocks the same process against itself. Every later mutation would block on a lock nobody holds
and the app would WEDGE rather than error -- a worse outcome than the data loss this PR fixes, and
silent.

_IndexLock.__exit__ is already correct (with guarantees it runs; release_lock in a try,
os.close and the fd reset in the finally). test_a_raising_read_still_releases_the_index_lock
now pins it behaviourally: after a mutation raises from the strict read, the very next mutation must
complete. Mutation-verified the hard way -- making __exit__ skip its release when an exception is
in flight hangs the test (rc=124 under a 30s cap, a --timeout=120 failure in CI), so it detects
the deadlock rather than merely passing.

One process note, for the record: I reported this test as "added and passing, mutation probe not
completed" in an earlier update, because tool access dropped mid-probe and I would not claim a
verification I had not finished. The probe is now done and is the rc=124 result above.

[chenmingwei23]

Round 3 -- lint fixed; the new BLOCKING finding is a false positive on its premise

Pushed 600962275. The previous finding (store.py:395, strict transition reads escaping nonfatal
reconciliation) is resolved and no longer appears -- thank you, it was real and I fixed it forward
rather than by the suggested revert, since reverting would have restored the truncation bug.

Backend Lint & Type Check (3.12) -- mine, fixed

Not black, as I first assumed: the failing step was mypy. Seven union-attr errors in
test_store_and_gate.py, all the same shape -- store.claim returns Incident | None and my new
tests used .incident_id without narrowing. Fixed with the assert ... is not None this file already
uses elsewhere.

Worth recording why I missed it: I had been running mypy <changed files> rather than CI's
mypy src/kiro_crew/, and the scoped form did not report them. The four errors that remain locally
(transcribe.py, providers/cloudwatch.py) are absent in CI -- #7788 has the identical four and its
lint job is green -- so they are local stub-version noise, not this diff.

The black gate then failed the other way: formatting test_store_and_gate.py graduated it off the
baseline, which the gate requires be pruned. Done -- baseline is 1208 entries, one fewer.

The corrupt-JSON finding -- disputing the premise, not the principle

providers/__init__.py:131 (also store.py:231) -- corrupt JSON still permits destructive
whole-file rewrites. Fix: catch only FileNotFoundError; propagate JSONDecodeError.

The mechanism named is "transiently malformed JSON", and that is the part I do not think holds for
these two files. Transient malformation means a torn read -- a reader observing a half-written
document. That requires a writer that truncates in place. Both of these files have exactly one product
writer each, and both go through atomic_write:

• index: store.py:238 -- atomic_write(index_path(), ...), and it is the ONLY product write to that
  path (grep index_path() finds no other writer)
• config: providers/__init__.py:219 -- write_config -> atomic_write, likewise the only one

atomic_write is mkstemp + os.replace. A reader therefore sees either the complete previous
document or the complete next one, never a partial one. There is no window in which valid content
parses as invalid.

This is exactly the property that distinguishes these files from config.json, and it is why
config/loader.read_config_for_update is right to raise on JSONDecodeError while these are not.
That function's own docstring gives the reason: "The read fails for mundane reasons -- most commonly a
torn read: several config writers still truncate-then-write, so a concurrent reader can observe a
half-written file." config.json has writers that are not atomic. These two do not.

So on these files "corrupt" cannot mean "transiently corrupt". It means external tampering, disk-level
corruption, a hand edit, or a stale incompatible format -- none of which is recoverable by retrying,
and none of which the next read would resolve.

And the remedy has a concrete cost in the other direction. Propagating JSONDecodeError from the
for-update readers turns a self-healing state into a permanent wedge. A corrupt index would make all
six mutations refuse indefinitely: no claims, no transitions, no sweep, no prune, no proposal
decisions, until a human hand-edits the file. On an incident-response board that is a worse outcome
than repairing, and it is the surface where wedging costs most -- the app stops responding to pages.
The same applies to config: every settings save refused.

Three further reasons I am not making this change in this PR:

1. It would diverge from the merged precedent. fix(aws-control): never publish the share or library ledger over a failed read #7620 (8064a9bb5) applied this exact idiom to
   aws_control's library.py and shares.py two days ago with corruption reading as empty. Changing
   it here creates two different answers to one pattern in sibling apps, which is worse than either
   answer consistently applied.
2. It changes behaviour that predates this PR. Corrupt-repairs-on-write is main's behaviour for
   both stores, pinned here by test_a_corrupt_index_still_repairs_on_write and
   test_a_corrupt_config_still_repairs_on_write -- negative controls added precisely so this PR could
   not be read as licence to start failing on corruption.
3. It is a scope decision, not a defect fix. "Should a corrupt store be repairable or refuse?" is a
   design question worth answering deliberately across all six sites of this pattern at once, not as a
   rider on a transient-EACCES fix.

I have not used /ai-review override: this needs a repository writer's judgement, and I would rather
put the evidence on the record and let a human decide than clear my own blocker. If the call goes the
other way -- that corrupt should refuse everywhere -- the right shape is a follow-up covering all six
sites plus #7620's two, with the wedge risk handled (probably a repair path the operator can trigger
explicitly), and I will take that on.

[chenmingwei23]

Design's sidecar suggestion is the right answer, and it holes my own rebuttal

Accepting the finding and deferring the change -- reasoning below, because the deferral is a scope
decision rather than a disagreement.

A corrupt (JSONDecodeError) index still reads as empty for-update, so the next mutation publishes
over it -- a truncated-but-mostly-intact ledger is salvageable by hand; renaming the unparseable file
to a .corrupt sidecar before the first overwrite would close the residual loss path this PR's title
names, at one line of cost.

This settles a standoff that had been running for two rounds. GPT filed the same underlying concern as
BLOCKING and prescribed propagating JSONDecodeError from the for-update readers. I rebutted that,
and I still think the rebuttal was right about the remedy: propagating turns a self-healing state into
a permanent wedge, because a corrupt index would make all six mutations refuse until a human hand-edits
the file, and on an incident board that means no claims, no transitions and no sweep while pages keep
firing.

But my rebuttal also asserted that corruption has "nothing left to lose by replacing it," and that
part was wrong. A truncated document is not an empty one. If the tail of the index is lost but two
hundred incidents are still readable in the prefix, those bytes are worth something to a human with a
text editor, and overwriting throws them away. Design is right and I was wrong on that specific claim;
the PR description says the same thing in the same words and inherits the same error.

What makes the sidecar better than either previous position is that it separates the two things GPT's
remedy had fused. Repair-on-write is about availability -- the app must keep working on a store it
cannot parse. Preserving the bytes is about recoverability -- the operator must keep their chance to
salvage. Propagating buys recoverability by sacrificing availability. A rename buys both: the mutation
proceeds exactly as it does today, and the unparseable bytes survive at <name>.corrupt instead of
being overwritten. It also does not diverge from #7620's behaviour, since corruption still reads as
empty and the write still lands -- it only stops the discard.

Deferring it, and this is a scope call, not a judgement on the finding. This PR is being closed out
under an explicit instruction to finish it and open no further work in this app. The suggestion is
advisory on a PASS lane, so it does not gate readiness, and it is a behaviour change to two product
files rather than a fix for a red. Doing it properly also means more than the one line: the rename wants
to be atomic, to not clobber an existing sidecar from a previous incident, to be bounded so a repeatedly
corrupt file cannot fill the disk with sidecars, and to apply to all the sites of this pattern rather
than the two here.

Recorded in #7789 alongside the three remaining sites of this class, as the preferred resolution to the
corrupt-JSON question rather than GPT's propagate-and-wedge. That issue already proposes an AST ratchet
for the class; the sidecar belongs with it, so the answer lands once instead of twice.

[chenmingwei23]

Two First Principles items, dispositioned without a push -- and the head is now frozen

9e29e4c75 is the frozen head. No code change in this round: I audited for the remaining
ValueError sites and there are none, so there is nothing left to push. Everything from here is
comments.

Why the head is frozen, since this is my error to own

I pushed three heads in about twenty-five minutes. Each restarts twenty-seven checks and re-rolls five
non-deterministic lanes, so the slowest lane never reaches the head I am standing on -- FP was still
judging de320942f while I was already on 9e29e4c75. The exit condition for this PR requires every
lane to have judged the CURRENT head, so an advisory-fix loop at that cadence has no fixed point. That
is a method problem I created, not the lanes being slow.

The ValueError audit is complete, and nothing remains

Extending the earlier ten-arm audit past routes.py and dispatch.py to every ValueError-catching
arm in the app: none of the rest can see a strict reader.

• store.py x6 -- an index-key int(), two datetime.strptime, a log-file write, a log-file read,
  and _coerce_index's own from_dict guard (which re-raises in strict mode).
• slot_watch.py:143 -- sits AFTER the corruption clause at 132, so ordering is already right.
• ledger.py, ledger_sync.py, models.py, providers/http.py, schedule_file.py -- number and
  date parsing on their own data.
• dispatch.py:331's store.update_fields is unguarded, and deliberately: corruption propagates out
  of run_cycle rather than being claimed by any arm. Loud, which is the intended answer.
• ledger_sync.set_settings and notify_out.set_settings reach set_top_level unguarded, but both
  are called through _settings_write_or_refuse (verified at routes.py:1976 and :1991), which
  carries the corruption arm.

CorruptDocumentError: keeping it, and the reason is sharper than my last one

FP's evidence is correct and I am not disputing a word of it: zero production catchers name the type,
every catcher matches the base class, so behaviour is byte-identical. On that evidence alone the class
looks like it exists to be grepped for.

The observation is the same fact the design depends on, pointing the other way. The subclass is
SAFE precisely because every existing catcher already matches the base -- that is what lets a refusal
route through callers nobody edited, and a type with no dedicated catcher is the intended end state
rather than an oversight. A fresh type outside that hierarchy would be caught by none of them, which
is how this class of data loss gets reintroduced silently.

And there is a concrete cost to the remedy that decides it. json.JSONDecodeError.__init__ requires
(msg, doc, pos), but a wrong-SHAPED document has no parse position -- the raises already pass a
synthesised str(raw)[:120], 0. Raising the base class directly at the five shape sites means that
same fiction five times over, each one unexplained at the call site. One named type that carries the
fake coordinates in a single place, with a docstring saying why it subclasses deliberately, is the
smaller honest version. That is FP's own criterion, applied to its own remedy.

Design Review raised the opposite concern on the same type -- that subclassing ValueError makes the
arm-ordering invariant permanent and ungated for the four modules #7805 will write. That argument I
accept, and it is recorded on #7805 as the decision to settle before those readers exist, because it
is worth paying once across five modules rather than retrofitting later.

backup.py is not mine -- correcting my own record

FP is right that aws_control/backend/backup.py:_locked_state_update is the same pattern, and right
that it was named by neither #7789 nor #7805.

It is being fixed in #7618, which owns that file and is editing those exact lines now. I should not
have added it to #7789 last round -- that was me filing someone else's in-flight work as a deferral,
which would have produced a duplicate. Corrected on #7789.

Body tightened

FP noted the description truncates at 8000 bytes and it could not confirm from the visible part
whether backup.py was named -- a fair hit, since a body a reviewer cannot finish is a body judged on
guesses. Rewritten from 26,417 bytes to 7,983, under the limit in full. What survived is the guarantee
statement and the mutation table (now an actual table, with a column for what each redness means to
the operator). What went is the round-by-round history, which belongs in these comments.

[chenmingwei23]

Design's second Watch item is a real coupling I chose, and I am escalating rather than patching it

All four lanes have now judged the frozen head 9e29e4c75: GPT no blocking, Opus "No findings", First
Principles PASS, Design CONCERNS with two advisory items. Head stays frozen -- no push for either.

The first item (the exception hierarchy) was already dispositioned; Design added a third option I had not
recorded -- a mechanical ratchet instead of a type change -- now on #7805, along with the fair observation
that this PR's own pattern-harvest rule ("retire the class, not the instance") argues against per-site
audits as the mitigation.

The second item is new, and Design is right that it wants a human:

Corruption now hard-stops _handle_ledger_hygiene before the ledger push, so a never-self-healing
fault also indefinitely stops the sync other instances wait on. Deliberate and argued in the diff, but
it couples "loud" to "blocking maintenance unrelated to the index" -- worth a human eye.

The coupling is real and I did choose it. The hygiene cron runs pull -> hygiene -> index ->
prune_closed -> push. My guard there catches OSError precisely so a transient fault cannot cost the
push, and deliberately does NOT catch corruption, on the reasoning that a corrupt index must not be
skipped quietly on every future run. The consequence I did not weigh: prune_closed reads the INCIDENT
index, while the push publishes the LEDGER. A corrupt index therefore blocks ledger convergence for the
whole team, indefinitely, over a fault in a file the ledger sync does not touch.

That is worse than it sounds in one specific way. The argument I made for refusing corruption everywhere
else is that refusing is the LOUD option and silence is the harm. Here refusing is loud AND it blocks
unrelated work that other instances are waiting on -- so "loud" stopped being free, and I kept applying
the rule as though it still were.

There is a clean fix and I am not making it under the freeze. The order is
pull -> hygiene -> index -> prune -> push, and the ordering that is actually load-bearing is the
LEDGER one, argued in TestLedgerHygieneWiring: dedupe before the merge, index before pruning rows,
push after hygiene so instances converge instead of each re-deriving the same dedupe. prune_closed has
no ordering relationship with any of that -- it retires closed INCIDENTS. Moving it after the push
decouples the two completely: a corrupt index then stops the prune loudly, exactly as now, while the
ledger still converges.

I am not doing it in this PR for two reasons. It is a behaviour change to the cron's step order rather
than a fix to anything this PR broke -- on main today prune_closed cannot raise at all, so the
coupling is new but the ordering is not. And this head is frozen deliberately: I pushed three heads in
twenty-five minutes and starved the slowest lane, which is why all four verdicts only landed together
once I stopped.

So it goes to Raymond as a decision, which is what Design asked for. Three ways forward, in my order of
preference:

1. Reorder to pull -> hygiene -> index -> push -> prune in a follow-up. Removes the coupling
   entirely, keeps corruption loud, touches no failure policy. My recommendation.
2. Leave it. Defensible: a corrupt index is a two-minute fix for whoever reads the 500, and blocking
   the ledger push until someone looks is arguably the correct forcing function for a fault nothing
   self-heals.
3. Report-and-continue at that site, the way _schedule_verification does -- audit the corruption
   and let the push run. I like this least: it puts the only signal in the audit log for a fault that
   needs a person, on a cron nobody watches.

Filed as #7790's sibling rather than folded in here, and flagged in the summary so it is a decision
rather than a discovery.

[chenmingwei23]

GPT's blocking finding is real: a third shape door, one level below the two I closed

Pushed dca487db1, and this is the ONE push I said the freeze allowed for a blocking finding. New frozen
head.

Worth stating up front how this verdict arrived, because it matters for reading it: GPT reported no
blocking findings on this exact SHA one cycle ago. I then edited the PR body, which re-triggers the
codex lane on the same commit, and the re-roll produced these two findings with zero code change between
them. That is the known non-determinism -- and it is also why "a green lane is a sample, not proof of
absence" is the right way to read all of these. I judged the finding on its merits rather than on the
flip, and it is correct.

The blocking one: nested fields are coerced, not rejected

Incident.from_dict does not raise on a malformed nested field. It substitutes:

• signal -> Signal.from_dict({}), an EMPTY signal
• ledger_matches -> []
• proposed_action -> None

So a row whose "signal" is [] loads clean through the strict reader, and the next mutation writes the
empty substitute back -- permanently discarding the source, native id, title and labels that were on
disk. No parse failure. No raise. Nothing logged. Exactly the loss this PR exists to stop, one level
below the root and row doors I had just closed.

The part I should own: I had already found the enabling fact and drew the wrong conclusion from it.
Two rounds ago I wrote that Incident.from_dict "coerces every field through str() and type guards, so
it essentially never raises" -- and used that to argue one branch of GPT's earlier finding was near-dead
code. The correct reading of the same sentence is that its TOLERANCE is a data-loss door. I looked
directly at the mechanism and described it as a reason to relax rather than a reason to check.

Strict mode now rejects a nested field that is PRESENT with the wrong type. Absent stays legal, because
these keys are missing on records written before the fields existed, and a first-write must not start
refusing old rows.

The non-blocking one, which was also a false claim in my description

GPT is right that structural corruption reached return {} and a bare continue in the DISPLAY path
with no logging, while the description promises the display reads "LOG when they degrade for any reason
other than an absent file". The promise covered the read failures and not the shape ones. Both now log at
warning, and the board still renders -- that asymmetry is unchanged and is the point.

Verification

Twenty-five mutation probes. The two new ones:

• Disabling the nested-field rejection reds test_a_malformed_nested_field_is_not_silently_replaced
  across all three subtests (signal, ledger_matches, proposed_action), each asserting the file is
  byte-identical afterwards.
• Demoting the root-degradation log to debug reds
  test_the_display_read_logs_every_structural_degradation.

A third test pins that an ABSENT nested field is still a normal record, so the new refusal cannot creep
into the ordinary path.

App suite 984 passed, 44 skipped, 265 subtests. All seven gates green -- the black gate caught the new
test file as an offender and it is formatted. Two commits, 12 files, zero website/ files.

Where this leaves the head

Frozen again at dca487db1. I am not editing the body from here: a body edit re-rolls this lane on an
unchanged commit, which is what produced this round. The mutation table in the description does not yet
carry a row for the nested door -- I would rather say that plainly here and leave the description alone
than re-roll five lanes to add a table row. If a reviewer wants it in the body, that is a cheap edit once
the lanes have settled.

[chenmingwei23]

The guard was enumerating shapes when the invariant is about content -- replaced, and the reader got smaller

Pushed 358f49580. This closes GPT's blocking finding, and it does not close it the way GPT asked.

The diagnosis I should have made two rounds ago

Three review rounds found the same class of loss, one layer deeper each time:

1. the index root normalized away (isinstance(raw, dict) -> {})
2. a row normalized away (index entry is not an object)
3. a nested field normalized away ("signal": [] -> an empty Signal inside Incident.from_dict)

I fixed each one where it was found. All three fixes were the same mistake: they enumerated shapes
while the invariant is about content. My own docstring stated the right rule -- refuse when
normalization would discard document content -- and the implementation asked "is this a dict? is that a
dict?", which any deeper example defeats. Round four was already written.

GPT's remedy ("reject malformed nested structured fields before deserialization") is enumeration one
level along, so taking it would have bought exactly one more round. I did take it in the previous push,
which was the wrong call.

The structural rule

_coerce_index in strict mode now deserializes, re-serializes, and refuses if anything that was on
disk did not survive. Details that matter:

• Parsed structures, not text. Key order and formatting cannot produce a false refusal.
• Added keys are ignored. A record written before a field existed legitimately lacks it, and
  from_dict filling in an honest empty default is not a loss. Only a value that WAS on disk and came
  back changed or missing counts.
• One rule covers the root, the rows, the nested fields, and every layer beneath them.

The reader is smaller than it was a round ago, which is the honest signal that this is the rule
rather than another patch. The per-row isinstance raise and the whole nested-field enumeration block
are deleted -- dropping a row or blanking a field is exactly what the round trip detects. Net
55 insertions, 52 deletions on store.py, most of it the docstring explaining why.

Verification, and the evidence that it generalizes

The proof is not that the new tests pass; it is that the OLD ones still do. Every test written to pin
the three enumerated guards passes unchanged against the single structural rule -- I deleted the guards,
not their tests.

Then the probe: disabling the one equivalence check reds four things at once --
test_a_skipped_entry_is_not_silently_deleted_by_the_next_mutation plus all three subtests of
test_a_malformed_nested_field_is_not_silently_replaced (signal, ledger_matches,
proposed_action). One rule, three layers, one probe. (The non-object-root test keeps passing, because
that check is separate and deliberately kept: you cannot iterate a document that is not one.)

App suite 984 passed, 44 skipped, 265 subtests. Twenty-six probes. All seven gates green.

Body

The mutation table row now names the equivalence refusal and what its redness means: an incident whose
signal is malformed comes back with the signal silently BLANKED and the original gone from disk.

I also deleted the sentences describing the three shape guards, because those guards no longer exist and
a body describing them would be a phantom claim -- the exact finding this repo's reviewers have made
against me twice already in this PR. Section 3 now describes the content rule, and the pattern harvest
carries the lesson that cost three rounds: a guard enumerating bad shapes loses to a deeper example
every time.

Body is 7,991 bytes, under the 8,000 truncation point, so a reviewer can still read all of it.

GPT's advisory about the display paths degrading without logging landed in the previous push and is
still in: both the non-object root and the skipped row log at warning, and
test_the_display_read_logs_every_structural_degradation pins it.

Frozen here.

[chenmingwei23]

Design found a real bug in the structural rule I just landed: it read a NEWER file as corrupt

Pushed 777a72ce6. First Principles is PASS on 358f49580; Design CONCERNS with one new item that was
a genuine defect, and one already-dispositioned item.

The defect

Incident.to_dict is asdict() over the fields THIS build knows. So a row written by a newer instance
carries a key that vanishes on the round trip -- and my equivalence rule read that as content loss and
raised CorruptDocumentError. Consequences, all of which Design named correctly:

• Every mutation refuses: claim, transition, sweep, decide.
• run_cycle deliberately lets corruption abort the whole cycle, so the older instance stops working
  entirely rather than degrading.
• The operator is told the file is "unreadable and must be repaired" -- a 500 in the
  never-self-heals taxonomy -- when the file is fine and the reader is behind. The remedy is an
  upgrade, and nothing in the message says so.
• This is not a hypothetical topology. Ledger sync explicitly supports two instances at different
  versions sharing one repository, so a mid-rollout fleet or a single downgraded instance hits it.

My round-trip carve-out ignored keys the trip ADDED -- old record, new code -- and had no counterpart
for the reverse. I built the rule around one direction of schema skew and did not ask about the other.

The fix keeps the refusal and corrects the taxonomy

Design's framing is what I implemented, because it is right: refusing to strip the unknown field is
correct -- writing would destroy a newer instance's data, which is the same loss every other refusal
here prevents -- so what needed fixing was the classification, not the strictness.

The equivalence check now separates two questions:

• _lost -- did a value this build UNDERSTANDS come back changed or shortened? Compares only keys
  present in both. That is corruption: CorruptDocumentError, 500, repair the file.
• _unknown -- does the document hold a field this build would silently DROP on write? That is version
  skew: UnknownFieldError, 409, "written by a newer version of this app; upgrade this instance".

UnknownFieldError subclasses CorruptDocumentError, so every caller that already refuses corruption
refuses this too with no change -- the distinction only has to exist where an operator READS it, which
is the route layer's code and status. 409 rather than 500 because nothing is broken and no retry will
help; a human has to move the instance forward.

Verification

test_a_field_from_a_newer_build_refuses_without_calling_the_file_corrupt writes a real claimed
incident, adds "a_field_from_the_future": {"nested": True}, and asserts three things: the mutation
raises UnknownFieldError, the file is byte-identical afterwards (the newer field was NOT stripped), and
UnknownFieldError is still a CorruptDocumentError so no caller can accidentally tolerate it.

Mutation-verified: folding _unknown back into the corruption branch reds it. Twenty-seven probes total.

App suite 985 passed, 44 skipped, 265 subtests. All seven gates green.

The other item

The CorruptDocumentError -> JSONDecodeError -> ValueError subclassing making arm-ordering a permanent
unenforced contract -- Design says "fine here; risky once #7805 replicates the idiom across four more
modules", which matches where I put it. #7805 carries all three options, including the AST ratchet Design
suggested as an alternative to changing the hierarchy. No action on this head.

Frozen at 777a72ce6.

[chenmingwei23]

Conceding a false justification: I cited a topology this app explicitly forbids

Pushed 78be7eb0c. Both lanes CONCERNS on 777a72ce6, no blocks, and both items are about the same
thing I added last push.

First Principles caught me justifying UnknownFieldError with a topology that does not exist

I wrote that the version-skew case was reachable because "ledger sync explicitly supports two instances
at different versions sharing one repository". That is false for this file, and the module I cited says so
directly (ledger_sync.py:18):

Only the ledger. NOT the dispatch index. The index is last-writer-wins on a shared key, so syncing
it would silently let two instances believe they each own an incident. Cross-instance claim arbitration
is a separate contract that has to be designed, not a file copy.

So the index is never shared between instances, by explicit design. I reached for the nearest plausible
justification and did not read the module I was citing.

The mechanism survives; my reason for it did not. The reachable path is a version ROLLBACK on a single
machine: a bad release is rolled back, the index on disk was written by the newer build, and the older
build now reads it. One instance, one file, no sharing. That is ordinary enough to be worth handling, and
Design Review's original finding stands on it -- without the carve-out, a rolled-back instance refuses
every mutation forever while telling the operator to repair a healthy file.

Corrected in three places rather than one, because the wrong claim had propagated: the
UnknownFieldError docstring, _unknown's docstring, and the test's docstring. Each now names rollback
and states that ledger sync deliberately does NOT cover the index, with the quote, so the next reader
cannot re-derive my mistake from the code.

The 409 surface was also, as FP put it, riding along undeclared. Now declared in the description.

Design: the round trip makes serializer idempotence load-bearing for AVAILABILITY

Accurate, and worth having on the record as a property rather than a surprise. Because a mutation refuses
when to_dict(from_dict(x)) fails to preserve what x held, a field that does not survive its own round
trip would make every claim, transition, sweep and decide refuse -- a correctness concern became an
availability one.

I had claimed in the new docstring that "the suite exercises it on every incident shape the app writes".
That was aspirational when I wrote it, so I made it true:
test_the_serializer_round_trips_every_shape_the_app_writes drives a real incident through claim,
propose_action, update_fields (diagnosis, ledger matches, verification fields) and transition, then
asserts from_dict(row).to_dict() == row for every row on disk.

Its first probe failed to red, and that mattered. I added a new defaulted dataclass field, expecting the
round trip to break -- it stayed green, because the file is written by the same build and therefore already
contains the field. The probe was wrong, not the test. The real failure shape is from_dict DROPPING a
populated field, so I made from_dict ignore diagnosis, and the test failed with "a field the app
writes does not survive its own round trip". That is the third time in this PR a probe caught a test
proving less than it claimed, and the second time the fault was in my probe rather than the test.

App suite 986 passed, 44 skipped, 266 subtests. Twenty-eight probes. All seven gates green. Frozen at
78be7eb0c.

[chenmingwei23]

Design's migration point is right and sharper than stated; FP's is a stale-body read with one live sub-point

Pushed 865dd7697. Both lanes CONCERNS on 78be7eb0c, no blocks.

Design: the round trip outlaws migrate-on-write

Correct, and the consequence is worse than "outlaws" -- it MISREPORTS. Renaming or retiring a field means
an old record carries a key the new to_dict does not emit, which at the point of detection is
indistinguishable from a field written by a newer build. Both are "on disk, absent from the round trip".
So a migration would have hit UnknownFieldError and been told the file "was written by a newer version
of this app" -- pointing the reader in exactly the wrong direction.

Two changes, no new machinery:

1. The error no longer claims a direction it cannot know. It now says the document holds a field this
   build does not serialize and that writing would drop it, then suggests checking whether the instance
   needs upgrading. True in both directions; the previous wording was only true in one.
2. The constraint is declared where the next author will hit it -- _unknown's docstring states that
   a schema change must extend this check with an explicit carve-out naming retired keys, added in the
   same change as the rename, rather than expecting migrate-on-write to work through the normal path.

I am not adding the carve-out mechanism now. There is no migration to serve, and a speculative allowlist
would be untested machinery guarding a case that does not exist -- the same "speculative generality" this
PR has otherwise avoided. Declaring the constraint plus a message that does not lie is the honest version.

First Principles: declared after their capture, and the sub-point answered on evidence

The "undeclared rider" half was true when the lane captured the body and is not true now -- reviewer bots
read the body as captured at review launch, and I added the 409 to section 3 after 78be7eb0c was pushed.
The current body names it, the status, the rollback scenario, and why it is not classified as corruption.
Nothing to fix; worth stating so it is not re-derived.

The live sub-point is the measurement: "0 hits for version_skew in website/src". True, and I
checked whether that distinguishes my addition from the existing ones -- it does not. There are zero hits
in website/src for EVERY code in this family, including #7788's merged policy_store_unwritable and
secret_store_unwritable. The reason is in api.ts:1083: if (body?.error) detail = body.error. The
dashboard surfaces the error TEXT and ignores code entirely. So code serves programmatic callers and
the audit trail, and the operator-visible channel is error, which this 409 populates with the actionable
sentence. Uniform with all five siblings rather than a gap in the new one.

App suite 986 passed, 44 skipped, 266 subtests. Twenty-eight probes. All seven gates green. Frozen at
865dd7697.

[chenmingwei23]

GPT is clear on 865dd7697; Design's remaining item is the ratchet decision, recorded on #7805

GPT 5.6 reports no blocking findings on the current head, which closes the nested-field block from
9e29e4c75. Design Review stays CONCERNS -- advisory -- and its item is not a defect in the change:

it installs two convention-only invariants -- exception-arm ordering and serializer idempotence -- that
future code can silently break

Both are accurate, both were introduced deliberately, and both are asking for the same thing: mechanical
enforcement instead of per-site vigilance. That is the decision already open on #7805, so I have added the
second invariant there rather than acting on it here, along with the concrete cheap form it could take --
enumerate dataclasses.fields(Incident) instead of the flows, so a new field that does not round-trip reds
automatically.

Worth being explicit about the gap in my own test, since Design is right that it exists.
test_the_serializer_round_trips_every_shape_the_app_writes covers the fields the flows it drives POPULATE.
A field added to the dataclass and not touched by claim/propose_action/update_fields/transition
would not be exercised, so that test can pass while the invariant is broken. It is a real check, not a
complete one.

Why this is not another push. GPT just went clean on this head after blocking two heads ago, and it is
non-deterministic -- a body or code change re-rolls it on the same commit, which is exactly how the
previous block appeared with zero code change between verdicts. Re-rolling a green that took several rounds
to earn, in order to add an advisory ratchet that belongs to a follow-up covering five modules, is a bad
trade. Head stays frozen at 865dd7697.

Standing state: no blocking findings from any lane. Design CONCERNS (this item), First Principles and Opus
re-running on this head, 24 checks still in flight, zero failures.

[chenmingwei23]

Opus found a real uncoded 500 on a committed claim; its remedy would have misreported it

Pushed f6e556073. All four lanes had judged 865dd7697: GPT no blocking, Opus no blocking with one
advisory, Design and First Principles CONCERNS. The advisory was a genuine gap in code I wrote, so it got
the push; the two CONCERNS are standing items already routed to #7805.

The gap

_handle_claim wrapped only store.claim in the coded translation. attach_ledger_matches, three lines
later, writes through store.update_fields -> transition -> _read_index_for_update() -- which this
change gave two new ways to raise -- and was unguarded. So a transient index read failure there answered
aiohttp's bare uncoded 500, which is exactly the thing this PR spent five rounds removing from its
siblings. Opus's identification is precise, including the reachable trigger (the Windows
scanner-holds-the-handle case this PR's own docstrings cite).

Why I did not apply the suggested fix

The suggested fix was to extend the same except -> _store_read_refusal over the post-claim work. That
would answer 503 -- and by the time attach_ledger_matches runs, the claim is already durably written
and the webhook is already acked. So a 503 tells the operator their claim failed when it succeeded; the
retry answers 409 signal_already_claimed; and the audit entry for a real claim is never written, because
the return jumps over it.

This is the same shape as GPT's remedy at _schedule_verification two heads ago, and it is wrong for the
same reason: once the irreversible step has happened, refusing misreports it. The rule this PR already
established is "a later fault cannot cost an earlier step" -- applied to dispatch.run_cycle's claim loop
and to prune_closed before the ledger push.

So the annotation degrades instead. The ledger match is the deferrable half -- the dispatch cycle
re-derives matches on its next pass -- so the route returns 200 with the claim and empty matches, logs the
exception, and records the degradation in the SAME audit entry rather than adding a second one. That last
detail was worth getting right: my first attempt audited twice on the degraded path, which would have left
two incident_claim rows for one claim.

Mutation-verified both ways. Removing the guard reds the new test; applying the remedy Opus suggested
also reds it, on the assertion "a committed claim was reported as failed" -- which is the clearest way to
show why the finding was right and the fix was not.

The two CONCERNS

Design: the change installs two convention-only invariants -- exception-arm ordering and serializer
idempotence -- that future code can silently break. Accurate, both deliberate, both asking for mechanical
enforcement. Recorded on #7805 with the concrete cheap form for each, including the field-enumerating
version of the idempotence check that would catch a new dataclass field automatically.

First Principles: corruption-refusal is a point patch beside four counted lenient siblings (that is the
declared divergence, tracked in #7805 with secrets.py named first), and the version-skew machinery serves
a trigger unreachable in this build. The second is fair and I am keeping the machinery, stating the trade
rather than hiding it: no field today is absent from to_dict, so UnknownFieldError can only fire on a
hand-edited file or after a future schema change. But the moment a schema change lands, the alternative is
silently stripping a field -- and Design asked for exactly this distinction so a rollback is not reported as
"repair the file". Two lanes want opposite things here; I have taken Design's side because the cost of
being wrong is a misleading error message versus destroyed data.

App suite 987 passed, 44 skipped, 266 subtests. Twenty-nine probes. All seven gates green. Frozen at
f6e556073.

[chenmingwei23]

First Principles is right that the count is five; four of them can lose data

No push this cycle -- both lanes are CONCERNS with standing items, and the one checkable new claim is a
counting question I got wrong in the description. Head stays frozen at f6e556073.

The fifth sibling

I have said "four merged siblings" throughout. Enumerating every *_for_update-shaped reader in the tree
rather than the four I remembered:

Reader	Corruption	Verdict
aws_control/library.py:95	reads as empty	data loss -- tracked in #7805
aws_control/shares.py:60	reads as empty	data loss -- tracked
ops_mission_control/policy_store.py:147	reads as empty	data loss -- tracked
ops_mission_control/secrets.py for-update arm	reads as empty	data loss -- tracked, named FIRST
ops_mission_control/ledger_index.py:83 _read_cursor	reads as empty	exempt, on the merits
mcp_quarantine.py:322	raises OSError	already strict
config/loader.py:661	raises ConfigReadError	already strict, on shape too

So five readers share the shape and four share the harm. The description now says both numbers instead of
the one I had been repeating.

Why the fifth is exempt, structurally rather than by argument. _read_cursor degrades to an empty set
on every fault, and its caller at ledger_index.py:172 writes _write_cursor(cursor | newly) -- a set
UNION. An empty read therefore only ever ADDS ids; it can never remove one. The cost of the degradation is
re-importing entries that were already imported, which is idempotent work, and its docstring says exactly
that ("must degrade to 're-check everything' rather than 'assume nothing needs importing'"). It is the one
place in this family where the lenient read is correct, and it is correct because of the union, not because
someone judged the risk acceptable.

One thing I should have found earlier. config/loader.read_config_for_update is the strictest
precedent in the tree and it already handles the SHAPE case -- "an unreadable or non-object file raises
ConfigReadError". That is the door it took me three review rounds to close here. I cited that function as
precedent in the original PR body and evidently did not read far enough into it.

The standing items

Design: two convention-only invariants (exception-arm ordering, serializer idempotence) that future code
can silently break. Both accurate, both deliberate, both recorded on #7805 with the concrete enforcement
form for each. Design's own earlier framing -- "fine here; risky once #7805 replicates" -- is where I have
left it.

First Principles: the version-skew apparatus has one consumer and a trigger unreachable in this build.
Both true. Keeping it, with the trade stated: the refusal is load-bearing the moment a schema change lands,
and the distinct 409 exists so a rollback is not reported as "repair the file". Two lanes want opposite
things on this one and I have taken Design's side, because being wrong that way costs a misleading message
rather than destroyed data.

Zero failures on this head; 24 checks still in flight.

[chenmingwei23]

All four lanes clean; GPT's advisory was a real hole and wider than it stated

Pushed 33f06bfbc. On f6e556073: GPT no blocking, Opus "No findings", Design and First Principles
CONCERNS with standing items. No blocking findings from any lane. GPT left two advisories, and the first
one earned the push because it is this change's own failure mode reached through a door I did not think of.

Invalid UTF-8 bypassed every corruption clause

read_text(encoding="utf-8") raises UnicodeDecodeError on invalid bytes, so json.loads never runs.
UnicodeDecodeError is a ValueError but NOT a JSONDecodeError -- so it missed every
except json.JSONDecodeError this change added, and at the tolerant callers it landed in
except (KeyError, ValueError, OSError) and was silently swallowed. That is precisely the accidental
tolerance this PR exists to close, reached through a sibling exception type rather than the one I audited
ten arms for.

It was wider than the finding said, and the test is what showed me. GPT pointed at the strict readers.
When I wrote the display-side test, read_index() ALSO raised -- its except (OSError, json.JSONDecodeError)
missed UnicodeDecodeError too. So the display asymmetry was broken as well: a board backed by a
non-UTF-8 file would 500 instead of rendering empty, which is the opposite of this change's whole premise.
Both readers and both display reads now handle it: the mutation path wraps it into CorruptDocumentError
so a corrupt byte stream is ONE condition regardless of which decoder noticed, and the display path
tolerates it and renders empty.

Mutation-verified: leaving UnicodeDecodeError unwrapped at the strict reader reds
test_a_file_that_is_not_utf8_takes_the_corruption_path, and the display test pins that the board still
renders.

The second advisory was another over-claim of mine

GPT: "now LOGS when it does" is false for nested coercions such as "signal": []. Correct -- that is the
third time in this PR a docstring of mine promised more than the code delivered, and all three were the
same habit of writing the intent rather than the behaviour.

Narrowed rather than "fixed", with the reason stated: the display read logs the degradations that explain a
THIN board (unreadable file, unparseable file, non-object root, skipped row) and deliberately does not log a
nested coercion, because detecting one needs the same round trip the strict path does and this function
runs on every board poll and every dispatch cycle. The mutation path catches those, which is where they
cost something. Adding per-render cost for a log line was the wrong trade; claiming it was already there
was the actual error.

Standing items, unchanged

Design: two convention-only invariants (arm ordering, serializer idempotence) -- on #7805 with concrete
enforcement forms. First Principles: the version-skew apparatus has one consumer and a trigger unreachable
in this build -- keeping it, trade stated, because the refusal is load-bearing the moment a schema changes.

App suite 989 passed, 44 skipped, 266 subtests. Thirty-one probes. All seven gates green -- the black gate
caught store.py as a new offender and it is formatted. Frozen at 33f06bfbc.

[chenmingwei23]

Terminal disposition: no blocking findings, and the two remaining items are for a human

Head stays at 33f06bfbc. No push this cycle, and no more pushes for advisories -- that is a
deliberate stop, so the reason is worth writing down.

Why the loop had to be stopped rather than finished

Every push re-rolls five review lanes on a new head, and each lane reliably produces one more advisory on
whatever it now sees. Several of those were real and worth the push -- an uncoded 500 on a committed claim,
UnicodeDecodeError bypassing every corruption clause, the round-trip rule reading a newer file as
corrupt. But the supply does not run out, and the PR's exit condition needs every lane to have judged the
CURRENT head. An advisory-fix loop at that cadence has no fixed point: I was starving the slowest lane
while adding work for it.

Both lanes have now converged on repeating the same two items, and Design's own framing is the tell:

two design commitments deserve human ratification before the idiom spreads

That is a request for a maintainer, not for another commit from me.

The two standing items, and where each is recorded

1. Two convention-only invariants (Design). CorruptDocumentError subclasses ValueError, so
exception-arm ORDER is load-bearing and unenforced; and the round-trip refusal makes serializer idempotence
an availability property. Both accurate, both deliberate, both introduced by this change. Recorded on
#7805 with three concrete options -- move the type out of the ValueError tree, add an AST ratchet keyed on
ValueError arms in modules importing a strict reader, or both -- plus the field-enumerating form of the
idempotence check that would catch a new dataclass field automatically. The argument for settling it there
rather than here is that #7805 writes four more readers, so it is worth paying once across five modules.

2. Two riders on inherited justifications (First Principles). The named exception type no catcher
distinguishes, and the version-skew classification for a scenario unreachable in this build. Both rebutted
in writing rather than actioned: the type carries (msg, doc, pos) coordinates that a wrong-shaped document
does not have, so raising the base class directly would spread that fiction across five sites unexplained;
and the version-skew refusal becomes load-bearing the moment a schema change lands, while the 409 exists so
a rollback is not reported as "repair the file". On that second one the lanes want opposite things -- Design
asked for the distinction, FP asks to drop it -- and I took Design's side because being wrong that way
costs a misleading message rather than destroyed data. That disagreement is exactly the kind a human should
settle, and it is stated plainly rather than resolved by whoever reviewed last.

State

• No BLOCKING findings from any lane. GPT and Opus were both clean on f6e556073; Design and First
  Principles are CONCERNS, which is advisory by this repo's own gate.
• App suite 989 passed, 44 skipped, 266 subtests. Thirty-one mutation probes, each observed red on
  revert. All seven repo gates green.
• Two commits, 12 files, zero website/ files. Description at 7,991 bytes, under the truncation point so
  it can be read in full.
• 24 checks re-running on this head from the last push; zero failures.

Whatever is decided on the two items above, this change does not depend on it: the invariants it installs
are the ones that stop the data loss, and the open question is only how they get enforced for the code that
comes after.

[chenmingwei23]

Reversing my own freeze for one three-line fix, and saying why

Pushed 2a29e6969. All four lanes were clean on 33f06bfbc -- GPT and Opus both no-blocking, Design and
First Principles CONCERNS with their standing items. One cycle ago I said no more pushes for advisories.
I am breaking that for GPT's finding, and the reason is the finding's content rather than a change of heart
about cadence.

What it was

providers.read_config's docstring says "An absent file is silent -- that is a fresh install, not a fault.
Anything else is logged", and then argues why:

because this degradation is the quietest one in the app: every provider stops polling while their
credentials stay in the keystone store, so Settings still shows each one as configured and nothing
reports that the app has gone deaf

The last line of the function was return data if isinstance(data, dict) else {}. A non-object root
parses fine, so no except fires, and that branch answered {} in silence -- in the one function whose
own docstring argues that silence here is the harm. This is the fourth over-claim of mine these lanes have
caught, and the worst of them, because the sentence it contradicts is the sentence making the case.

It is also asymmetric with a fix I already made: the index display read grew exactly this warning one head
earlier and I did not carry it to the config twin.

Why it outweighed the freeze

The freeze exists so the head stops moving and 66 checks can settle. That is worth real money and I broke
it deliberately, not absent-mindedly. What tipped it: this is not a stale comment or a cosmetic log. It is a
silent branch in the degradation path this app is least able to notice, and "the app has gone deaf and
nothing said so" is the exact failure class the whole PR exists to remove. Shipping it while the docstring
claims otherwise would have made the description's own argument false in the code.

Three lines and a test. Mutation-verified: demoting the warning to debug reds
test_the_display_read_logs_a_non_object_root.

Standing items, unchanged and not mine to close

Design wants human ratification of two convention-only invariants (exception-arm ordering, serializer
idempotence) -- both on #7805 with three concrete enforcement options. First Principles wants the
version-skew rider dropped; Design asked for it. That disagreement is stated rather than resolved by
whichever lane spoke last, and it wants a maintainer.

App suite 990 passed, 44 skipped, 266 subtests. Thirty-two probes. All seven gates green. Two commits,
12 files, zero website/ files.

Back to frozen at 2a29e6969. The bar for another push is a BLOCKING finding, not a good one.

[chenmingwei23]

GPT found the one hole the round-trip rule structurally cannot see

Pushed 6d0bcd496. This one is BLOCKING and correct, so it clears the bar I set last round.

Verified before acting, because the claim depended on a write I had not read

GPT: a row whose index KEY disagrees with its own incident_id passes the strict read, and the expiry
sweep then rekeys the record. I checked the write path rather than taking it:

expire_stale_proposals (store.py:1061) iterates index.values() -- which discards the key -- and writes
back with index[inc.incident_id] = replace(...). _write_index_unlocked then persists whatever keys the
dict holds. So a row stored under INV-1 whose id says INV-9 is MOVED on the next expiry sweep: INV-1
is left behind as a duplicate, or a real INV-9 is overwritten.

Why my equivalence rule could not catch it, which is the interesting part. The round trip compares each
row against itself, and a key/id mismatch survives that faithfully -- _lost passes, _unknown passes,
nothing is dropped or changed by the load. The corruption manifests at WRITE time, from the key the writer
CHOOSES. A rule about read fidelity is structurally blind to it. That is a real limit of the structural
approach I argued for two rounds ago, and worth naming rather than papering over: "the load does not rewrite
anything" is necessary but not sufficient, because the write can still put a faithfully-loaded row somewhere
else.

Fixed at both ends

The read now asserts the referential invariant: the key IS the incident_id. This is deliberately its
own check rather than folded into the equivalence rule, because it is not another shape -- it is a
relationship between two parts of the document that both round-trip correctly in isolation.

The write stops rekeying at all: for key, inc in list(index.items()) and index[key] = .... On a
consistent document that is a no-op, so this is belt-and-braces -- but the function that rewrites the whole
index should not be the one relying on the reader to have checked first. Pinned by driving a real expiry and
asserting the key set is unchanged.

Mutation-verified: disabling the identity check reds
test_a_row_whose_id_disagrees_with_its_key_refuses_the_mutation.

State

App suite 992 passed, 44 skipped, 266 subtests. Thirty-three probes. All seven gates green. Two commits,
12 files.

Standing advisories unchanged and still for a maintainer: Design's two convention-only invariants (on #7805
with three enforcement options) and First Principles' request to drop the version-skew rider that Design
asked for.

Frozen at 6d0bcd496.

[chenmingwei23]

GPT clear on 6d0bcd496; its one advisory is accepted and deferred, with the reachability stated

The identity fix closed GPT's block -- no blocking findings from any lane. Design remains CONCERNS with
the standing item it now words as something "humans should ratify", which is where I have left it.

GPT's remaining advisory is correct: _schedule_verification's except json.JSONDecodeError catches
UnknownFieldError too (it subclasses CorruptDocumentError, which subclasses JSONDecodeError), so a
version skew is audited as "index corrupt; action executed with no recheck scheduled". That is the wrong
wording for a file that is fine, and it is an inconsistency I introduced: the HTTP layer distinguishes the
two conditions and the audit layer does not.

Not pushing for it, and the reasoning is the reachability rather than the size. Reaching that line needs
a version rollback AND a schema change that retires a field -- First Principles established two rounds ago
that no field in this build is absent from to_dict, so UnknownFieldError cannot fire today except on a
hand-edited file. The fix is three lines; the head has moved eight times in two hours and each move resets
66 checks and re-rolls five non-deterministic lanes. A stable head is worth more right now than correct
wording on an unreachable audit line, and I set the bar last round at BLOCKING rather than good.

Recorded on #7805, which is where the schema-change work that makes this reachable will happen -- so the
wording gets fixed by the change that first makes it matter, rather than in a PR that cannot exercise it.

State

• No BLOCKING findings from any lane. GPT clear on this head; Opus was clear on the previous one and is
  re-running; Design and First Principles CONCERNS, which this repo's gate treats as advisory.
• App suite 992 passed, 44 skipped, 266 subtests. Thirty-three mutation probes, each observed red on
  revert. All seven repo gates green.
• Two commits, 12 files, zero website/ files. Description 7,991 bytes, under the truncation point.
• 24 checks re-running on this head; zero failures.

Frozen at 6d0bcd496. Next push requires a BLOCKING finding.

[chenmingwei23]

Green and handed off -- needs a human approval, and two decisions that are not mine

All 68 checks on 6d0bcd496: 64 success, 4 skipped, zero failures, zero pending. mergeStateStatus is
BLOCKED solely on REVIEW_REQUIRED; no required context is unmet. I am not merging -- @raymondzhu-kiro makes
that call.

Lanes, all four at this head: GPT 5.6 clear, Opus 4.8 clear ("No findings"), First Principles PASS,
Design CONCERNS (advisory). No BLOCKING finding outstanding.

Two things genuinely want your judgement, not more work from me

1. Design's ratification request. The round-trip refusal makes serializer idempotence and schema
   stability into load-bearing availability constraints: if to_dict(from_dict(x)) ever stops preserving
   x, every mutation refuses. That is deliberate and pinned by
   test_the_serializer_round_trips_every_shape_the_app_writes, but it is a convention a future author can
   break without noticing. Three enforcement options are written up on Four merged update readers replace a corrupt file instead of refusing; secrets.py first #7805.
2. Design and First Principles want opposite things on the version-skew distinction (UnknownFieldError
   -> 409 rather than 500). Design asked for it; FP considers it unreachable in this build and would drop it.
   I kept Design's side, because being wrong that way costs a misleading message while the other direction
   risks a document a newer build wrote being treated as corrupt. Stated rather than resolved by whichever
   lane reviewed last.

One coupling worth a follow-up, flagged rather than fixed here

_handle_ledger_hygiene refuses on a corrupt index before the ledger push that other instances wait on, so
a corrupt index indefinitely blocks their sync. Reordering the prune after the push fixes it; that is a
behaviour change beyond this PR's scope and belongs in its own change. Opus examined this same escape and
called it intended and consistent with run_cycle / verify_pending_actions / reconcile -- so the escape
is right, and only the ORDER is worth revisiting.

Suite 992 passed, 44 skipped, 266 subtests. Thirty-three mutation probes, each observed red on revert. Two
commits, 12 files, +2196/-72, zero website/ files.

[buluoray]

Verdict: 0 blocking, 2 non-blocking. Reviewed at head 6d0bcd4960dd648ec21cc1ec595a0a2760ec38c7. This is a data-loss guard, so I held it to the "a write must never proceed on a failed read" standard and verified the code rather than the prose.

What I verified

Files read from the PR head via the contents API: store.py, providers/__init__.py, dispatch.py, slot_watch.py, models.py; PR body; all bot comment bodies at the current SHA.

(a) Every read failure mode that feeds a whole-file rewrite is accounted for, enumerated against both store._read_index_for_update and providers._read_config_for_update:

• missing file -> except FileNotFoundError: return {} — correct, an absent store is a fresh install, the one benign empty.
• empty / 0-byte file -> json.loads("") raises JSONDecodeError -> re-wrapped CorruptDocumentError -> mutation refuses. Not treated as truncation.
• unreadable (EACCES/EIO/PermissionError) -> OSError is deliberately NOT caught in the for-update readers, so it propagates and the mutation is abandoned. Fail-closed.
• unparseable JSON -> CorruptDocumentError.
• non-UTF-8 bytes -> UnicodeDecodeError re-wrapped to CorruptDocumentError (it is a ValueError but not a JSONDecodeError, so this closes the sibling-type bypass).
• succeeds but returns a default-empty object — the dangerous case: {} is only returned from a genuinely empty/absent file. A non-object root ([], null, string) raises in strict mode; a row/nested field that would not survive to_dict(from_dict(x)) raises via the _lost round-trip; a key/incident_id mismatch raises via the referential check. So an "empty because the read silently degraded" outcome cannot reach the writer.

(b) Fails closed. Every refusal raises inside the _IndexLock / _ConfigLock block before _write_index_unlocked / write_config, so no partial merge is published.

(c) Success path unchanged. Display reads (_read_index_unlocked, read_config) stay lenient and still return {}; only the mutation base swapped to the strict reader, which is a no-op for well-formed shipped data.

(d) Tests pin the refusal, not just absence of exception. The mutation-table tests assert the mutation refuses AND the file is byte-identical afterwards (e.g. test_a_read_that_failed_never_truncates_the_index, test_a_malformed_nested_field_is_not_silently_replaced). Caveat under "what I could not verify".

(e) Concurrent writers. Read-modify-write runs entirely under the pre-existing per-file flock (_IndexLock/_ConfigLock) and commits via atomic_write (mkstemp + os.replace). The PR changes only the reader inside the lock, introducing no new interleaving.

Caller policy is consistent with the fail-closed intent: dispatch.run_cycle and slot_watch.reconcile tolerate transient OSError (retry next cycle) but let CorruptDocumentError (a JSONDecodeError) propagate loudly; the except json.JSONDecodeError: raise arms are correctly ordered before the tolerant except ValueError arms, so corruption is never misfiled as a lost race.

Non-blocking findings

1. docs/system-specs/modules/ops-mission-control.md — spec not updated in this PR. AGENTS.md §Specification management (line 218) asks for a same-commit spec update when documented behavior changes. This PR adds a new documented read-on-mutation contract (*_for_update) and three new HTTP error surfaces (503 *_unreadable, 500 *_corrupt, 409 *_version_skew); the existing spec still references only _read_index_unlocked and does not describe them. Consequence: the module spec drifts from the shipped error contract. Suggestion: add a short section on the strict-read contract and the three codes. Noting the First Principles lane (which checks AGENTS.md-mandated invariants) passed and Docs Lint is green, so a maintainer may already consider this acceptable.

2. store.py (_schedule_verification audit wording) — UnknownFieldError audited as corruption. Already surfaced by GPT 5.6 at the current head and dispositioned to #7805 by the author. except json.JSONDecodeError also catches UnknownFieldError (it subclasses CorruptDocumentError -> JSONDecodeError), so a version-skew after a rollback would be audited as "index corrupt" rather than version skew. Consequence: misleading audit wording only, and unreachable in this build (no field is currently absent from to_dict, so it needs a rollback plus a schema change). Suggestion: fix the wording alongside the schema-change work in #7805, as the author proposed.

What I could not verify

• I did not run the test suite or the mutation probes — the shared checkout is read-only and I made no local build. My confidence in (d) rests on reading the test assertions in the diff, not on executing them; the author reports 33 probes each reverting to red.
• The _lost / _unknown round-trip logic and the _IndexLock exception-safety were verified by reading; I did not exercise the flock behavior under real concurrency.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8248 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8248: CONTINUE_DEVELOPMENT. Recently merged precedent in current main points the opposite way for reads that feed writers; PR 8248 should either follow it or state why the memory markdown store is the exception. Files: src/kiro_crew/apps/builtins/ops_mission_control/backend/store.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.