fix(history): pair a frozen save snapshot with its prefix boundary

[bolichen97]

Why no screenshot: backend-only change to the shared transcript-save primitive and the rewind endpoint's boundary transaction; no UI surface is touched.

Scope changed 2026-09-05. This PR was the edit-resend port of #8145's boundary transaction. #8431 is an independent port of the same shape onto the same endpoint and has been picked as the vehicle for #7838, so the endpoint half has been dropped here (chat_regenerate.py, session.md, and the edit-resend tests are reverted to origin/main). What remains is the part #8431 does not touch at all: two defects in the shared _save_slot_to_history, plus the rewind caller that reaches them. Both are reachable on origin/main today, on the merged #8145 rewind, with or without either port.

Problem / Motivation

Two defects in _save_slot_to_history (src/kiro_crew/dashboard/chat_persistence.py), both reachable through the shipped rewind endpoint (chat_rewind.py:233-514, merged in #8145):

1. A frozen window snapshot is written against a live prefix boundary. A boundary transaction freezes msgs_snapshot on the event loop and then awaits the save in a worker thread, where the save read the live slot._disk_older_count. That counter is where the frozen prefix ends. An append at the window cap in that gap trims the front of the window and credits the trimmed rows to the counter — so the file written is frozen_prefix(after the trim) + snapshot(frozen before it), which emits the trimmed rows twice. Demonstrated with the fix removed: a window at its cap plus one arrival landing during the save writes ["u0", "u0", "a0", "edited"].

2. Post-write witnesses are stamped onto a slot the loop may have rebound mid-write. _pending_rewrite, _disk_window_len, _disk_meta_created_at, _disk_meta_observed and _frozen_prefix_cache describe the file this save wrote, but they live on the live slot. The save's routing pin is checked before the write, so the bytes always land on the authorized transcript — but a rebind landing after that pin (a cron injection re-linking the slot) leaves the stamping describing the old file on a slot that now writes a new one: it cleared a _pending_rewrite the new transcript still owed and claimed its unsaved rows as persisted, which a later flush believed.

Why it matters

The first is a silently duplicated transcript — no error, no warning, and the user sees a turn twice in their own history. The second hands a later flush a false "already persisted" reading of a transcript that was never written, which is how unsaved rows get dropped and an owed archive-safe rewrite gets skipped.

What changed (motivation → approach → change)

• _save_slot_to_history takes expected_disk_older_count (keyword, defaults to None = today's behaviour for every other caller). A caller that freezes its own messages snapshot cannot use the existing bounded snapshot retry — the snapshot is already frozen — so it passes the boundary it observed in the same synchronous stretch as the snapshot. Drift refuses the save: False, nothing written, which is the retryable 503 these transactions already contract for. Ignored without an explicit messages, where the bounded retry already takes both halves together.
• The witness stamping is routing-gated. The save re-confirms slot_history_key(slot) against the key it wrote and skips the stamping on a mismatch, logging it. Every witness left at its pre-save value is that witness's conservative reading — the next save re-reads the prefix, re-takes the archive-safe path, and re-observes the file. The ConversationLog cache invalidation is keyed on the file that was written and stays unconditional. Everything the stamp needs (the post-write stat, the carried-forward created_at) is hoisted above the re-check so the guarded region is assignments only — a save runs in a worker thread, and a syscall inside that region was the realistic point at which the loop got to rebind under a half-applied stamp.
• rewind pairs its snapshot with its boundary. pre_await_disk_older_count and pre_await_disk_older_durable_count are captured beside msgs_snapshot with no await between them, the first travels to the save, and the commit re-adopts both — because the commit puts the pre-trim window prefix back on the live window. The durable position base moves with the disk boundary for the same reason and on the same rows: a row counted as having left the window front while it is back inside it either refuses a valid absolute cursor (since < base) or repeats rows. A trim landing after the worker read the boundary cannot be refused (the correct file is already written), so it is corrected at the commit instead.
• _disk_window_len is deliberately left possibly SHORT, and the direction is the argument. The save stamps it absolutely, so a trim landing before the stamp has its decrement erased while one landing after it does not, and the commit cannot tell the two apart without the count the save actually wrote — which is not len(msgs_snapshot) either (a note row authorized elsewhere is filtered out of the write). Over-claiming is the harmful direction: a later trim then credits rows to the frozen prefix the file does not hold, and the next save re-emits window rows — the duplication this PR exists to prevent. Short costs an under-credited prefix, a warning about rows that are in fact on disk, and one whole-file re-read; the foreign-append merge preserves the on-disk window line the memory window has dropped, so no row is lost.

docs/system-specs/modules/history.md documents all four points in the same commit, including the residual below and its remedy, so that knowledge is in the spec rather than only in this thread.

Standing reviewer finding — maintainer decision needed

GPT 5.6 Review has held one finding across rounds 5-8, anchored at the routing gate above (chat_persistence.py, the if slot_history_key(slot) == history_key: region): the route check and the witness assignments are not atomic against the event loop. It is real and it is narrowed as far as this line can be narrowed — the region is now five plain attribute assignments with no yield point, the path.stat() it used to contain having been hoisted out.

What remains needs an atomic multi-field publish, which the reviewer's own remedy names correctly: one routing-keyed record. Concretely, collapsing _pending_rewrite, _disk_window_len, _disk_meta_created_at, _disk_meta_observed and _frozen_prefix_cache into a single _ChatSlot field carrying the history_key it describes, published by one assignment and validated by consumers against their own routing. That would also give the commit the saved window count _disk_window_len is missing above, so both halves have the same fix.

Why it is not in this PR: it is a __slots__ field-shape change with five consumers to migrate (channel_slots.py, chat_persistence.py ×3, chat_handlers.py) plus the delete-won guard, which reads two of those fields independently and fails OPEN when they are unset — so a partial migration weakens a security guard rather than a cache. That is a persistence-core change with its own design and its own tests; it is recorded on #8419 with the consumer list and the rounds 5-8 analysis. Clearing the lane needs either that change landing first, or a maintainer /ai-review override for the residual. Not a call this PR makes.

Follow-up scope (noted, not expanded here)

• save_slot_off_loop does not forward expected_disk_older_count. Its keyword list is explicit, so a boundary transaction routed through it still reads the live counter in the worker and keeps exposure 1. fix(dashboard): make edit-resend a real conversation boundary #8431's edit-resend saves that way. Threading the parameter through is a small follow-up once both land; history.md states the gap.
• Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419: the routing-keyed witness/serialization record above, plus its original item (the SEL record on a post-discard failure).
• The edit-resend port itself is fix(dashboard): make edit-resend a real conversation boundary #8431, which now owns edit_resend shares the rewind context-boundary root cause fixed for rewind in PR #5395 #7838 and session.md's edit-resend section.

Tests

test/test_dashboard_chat_rewind.py (38 pass, against the real _save_slot_to_history / ConversationLog except where a boundary is deliberately failed):

• test_rewind_pairs_the_snapshot_with_the_frozen_prefix_boundary — the wiring: the endpoint hands the save the PRE-await boundary, not the one a worker would have read, and re-adopts both counters at the commit.
• test_rewind_refuses_when_a_cap_trim_moves_the_frozen_prefix — the save's own refusal, for real: window at cap with the rows on disk, an arrival landing between the native clear and the rewrite → 503 rewind_save_failed, no dispatch, transcript byte-identical.
• test_rewind_leaves_the_witnesses_alone_when_a_rebind_wins_the_write — a rebind inside atomic_write → 503 rewind_slot_rebound, _pending_rewrite still owed, window still unpersisted.
• The existing save double in this module adopts the new keyword. No assertion was relaxed and no test was deleted.

Mutation-verified, each caught by exactly one of the two real-save tests: neuter the drift refusal (if False:) → test_rewind_refuses_... fails; make the witness gate unconditional (if True:) → test_rewind_leaves_the_witnesses_alone... fails.

Targeted runs: 38 pass in test_dashboard_chat_rewind.py; 849 pass across test_chat_regenerate_cov80.py, test_forced_save_history_key_pin.py, test_history_composition_contract.py, test_persist_off_loop.py, test_dashboard_chat.py; 2,672 pass + 4 xfailed across all 37 test modules that reference _save_slot_to_history or any of the three witness counters. Local gates clean on the committed range: check_black_formatting, check_subprocess_encoding, isort, flake8 src/kiro_crew test, mypy --platform linux src/kiro_crew (1,294 files), check_brand_name, check_harness_parity, docs-lint, check_changelog_history.

Manual verification

N/A — rewind is exercised end to end through the aiohttp test client against the real persistence layer, on both sides of the stamp; there is no UI change.

Related Issues

Split out of the edit-resend port; #8431 carries the endpoint and closes #7838. Shape source: #8145. Residual hardening: #8419. Context: #7838.

Pattern harvest

Rule candidate: review-prompt
Pattern: "a snapshot handed across an await must travel with every counter that describes where it belongs, not just its own contents — and post-write bookkeeping that lives on a mutable object must be gated on the object still describing what was written." Generalizes beyond this save: any worker-thread write whose bookkeeping lands back on an event-loop-owned object has the same two halves, and the correct end state for both is one routing-keyed record published by a single assignment (#8419).

Checklist

• One commit, Conventional Commits title
• Existing tests pass and new tests added
• Self-review completed
• Documentation updated (docs/system-specs/modules/history.md, same commit)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound, honest fixes to two real races, but both land as opt-in/narrowed patches whose complete fix is deferred to #8419 — humans should ratify that sequencing.

Watch

• The boundary pairing is opt-in per call site: expected_disk_older_count "defaults to None = today's behaviour for every other caller" and "save_slot_off_loop does not forward" it — so the duplication defect this PR fixes remains live for every present and future explicit-snapshot caller (including fix(dashboard): make edit-resend a real conversation boundary #8431's edit-resend saves) unless each one remembers the keyword. The defect class recurs by default rather than by mistake.
• The routing gate is a narrowed race, not a closed one, by the PR's own account ("the route check and the witness assignments are not atomic against the event loop"). The residual and its real fix (one routing-keyed witness record) are parked on Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 with a partial-migration hazard on the fail-open delete-won guard; merging this accepts that residual until Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 lands — a deliberate maintainer call, not a default.

Suggestions

• Make the pairing structurally required instead of remembered: have explicit-messages callers pass one (snapshot, boundary) value — or at least warn when messages arrives without a paired count — so the next boundary transaction can't silently reopen the duplication path.

[DESIGN-REVIEWED] d544f53

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of d544f537222cda5d0d291e8ba6a6f429ed1aa3b3 — 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 claims verified: the 503/rollback machinery and expected_history_key refusal pre-exist (chat_persistence.py:2683, chat_rewind.py:531-548), the new parameter has exactly one consumer, and chat_regenerate.py carries three identical freeze-then-await save sites without the pairing. I have what I need to emit the review.

First-Principles-Verdict: CONCERNS

The boundary pairing lands on the shared save but is wired to one of its four freeze-and-await callers; regenerate's three stay duplicable, and the declared successor keeps that exposure.

What this change ships

Intent: stop a mid-save window trim from duplicating transcript rows, and a mid-save rebind from marking unwritten rows persisted — a FIX.

1. A save given a frozen snapshot now refuses when the prefix boundary drifted — justified, 1 consumer
2. Rewind hands the save its pre-await boundary; drift becomes the existing retryable 503 — justified, the fix
3. Rewind's commit rolls both boundary counters back to their pre-await values — justified, the fix
4. Post-write bookkeeping is skipped, with a warning, when the slot rebound mid-write — justified, mechanism-level at the shared primitive
5. Residual stamping non-atomicity declared, cause (one routing-keyed record) deferred to Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 — declared, out-of-scope cause with counted consumers
6. _disk_window_len deliberately left possibly short after a trim — a recorded non-change, justified
7. Return-False docstring now covers the pre-existing routing refusal — stale-doc correction
8. history.md documents all four points — mandated same-commit spec update

Watch

• Root cause has 3 unfixed siblings: chat_regenerate.py:97, :196, :292 each freeze msgs_snapshot = list(slot.messages) on the loop and call _save_slot_to_history without expected_disk_older_count (grepped the explicit-snapshot call pattern; 3 hits outside rewind). The description defers them to fix(dashboard): make edit-resend a real conversation boundary #8431, but by its own account fix(dashboard): make edit-resend a real conversation boundary #8431 routes through save_slot_off_loop, which does not forward the parameter, so exposure survives both PRs. Note the deferral is not forced: all three sites already discard the save's return, so a drift refusal there degrades to the existing _pending_rewrite/flush retry — the pairing could ride now at three call sites.

[FIRST-PRINCIPLES-REVIEWED] d544f53

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The candidate correctly observes that slot._frozen_prefix_cache is reassigned mid-save at line 2311 (in the collect_foreign=False rewrite path), so the skip-branch comment's "leaves every witness at its pre-save value" is technically imprecise for that one field. But the candidate itself concedes the key point: there is no observable wrong outcome. After a rebind the slot routes to a different transcript file; the next save's fast-path cache check compares (mtime, size, disk_older) against that different file and misses (the cached tuple describes the old file), forcing a safe full re-read. A coincidental match across two distinct files on all three witnesses is not an input that occurs in practice — the candidate could not construct one either.

So (a) has no concrete input and (c) has no observable failure. What remains is a comment-accuracy nitpick on a data-safety path — which fails the survival bar and falls in a category this pipeline does not report. It dies under falsification.

No groundable Step 2 finding follows from the same observation, for the same reason: the stale cache is harmless absent an unrealistic three-way file-identity collision.

No findings.

[OPUS-REVIEWED] d544f53

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

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

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for d544f537222cda5d0d291e8ba6a6f429ed1aa3b3; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

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

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

GPT round 1 (chat_regenerate.py:493, concurrent-arrival erasure) — accepted and fixed at e48f2c538. Verification agreed with the finding and sharpened it: the pre-port implementation truncated inline and only then awaited the save, so a row landing mid-await survived on the live window — the snapshot-replacement commit made that a regression, not an inherited trait. Fix is shape-preserving: capture the live window length in the same synchronous stretch as the reservation (no await between), and have the commit adopt prospective + live[pre_await_len:] — arrivals stay owed to the normal flush, retained-prefix in-place edits already survive via shared dicts. Locked by test_edit_resend_keeps_rows_that_landed_during_the_boundaries plus a mutation check (bare replacement fails it). Note the same erasure exists in chat_rewind.py's commit; added to the #8419 scope rather than forked here.

Design Review (CONCERNS) — per-finding response:

• Duplication vs extraction: agreed on the direction. The extraction is deliberately sequenced AFTER this port and Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419: extracting now would churn the just-litigated fix: isolate edit rewind from discarded context #8145 shape while its two consumers still differ in review-visible ways, and Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 is about to change the shared core again (witness adoption). The regenerate port is the natural first consumer of the extracted core — that ordering is recorded in the PR body's follow-up section.
• "A human should confirm the Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 deferral": correct, and that confirmation is exactly what the required maintainer approval on this PR constitutes — the deferral is documented in the body's review-disposition section with the verified mechanics, so the approver is signing off on it explicitly, not riding a label.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

Design round 2 (e48f2c538) — per-finding response:

• Duplication / extraction: position unchanged from the round-1 response above — extraction is sequenced after this port and Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 settle the shared core's final shape, with the regenerate port as the extracted helper's first consumer. One addition since then: round 1 of the GPT lane already demonstrated the drift risk Design describes (the arrival-erasure fix landed here first and is now queued for rewind via Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419), which strengthens the case for the extraction — and for doing it once, after Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419, rather than while the core is still moving.

• rewind_* code names on edit-resend: deliberate, and the repo's own refusal-code doctrine is the reason (test/test_chat_regenerate_refusal_codes.py): codes are condition-scoped, not endpoint-scoped — "slot is running" shares one slot_running across three endpoints, and the test file's stated failure mode is "a client needs one lookup table per endpoint and the shared vocabulary is a fiction". These four refusals are the same conditions on two endpoints, so minting edit_resend_* twins would create exactly that two-spellings drift; renaming rewind's published codes (merged yesterday, clients may already match on them) is a contract break out of this port's scope. The naming is admittedly historical — a neutral rename (edit_boundary_*) applied to BOTH endpoints in one coordinated change is now recorded in Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419's scope, which already touches every one of these refusal sites.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

GPT round 2 (e48f2c538, 3 findings) — dispositions, new head 41a2a491e:

1. :514 length-based retention drops arrivals at the window cap — accepted and fixed. Verified: at _MAX_SLOT_MESSAGES the append trims the FRONT so window length stays constant and the tail slice reads empty. The commit now counts arrivals by the lifetime-counter delta (slot.total_messages - pre_await_total_messages, captured in the same no-await stretch as the reservation; the prospective copy froze its own counter, so live bumps are all arrivals) and takes that tail — appends land only at the end, the trim eats only the front. Locked by test_edit_resend_keeps_an_arrival_when_the_window_is_at_cap (window pinned at cap, arrival mid-boundary, front-trim observed, arrival survives) plus a mutation check.

2. :573 live-slot save poisons witnesses after a rebind — inherited, tracked in Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 (item 1). Same category as the pre-push review finding dispositioned in the PR body ("Pre-push review disposition"): the reference implementation merged in fix: isolate edit rewind from discarded context #8145 has the identical live-slot save (chat_rewind.py:444), the guard runs once pre-write (chat_persistence.py:2576), stamping is on the live slot (chat_persistence.py:3213-3248), and flush_slot_now clears _dirty on non-raising saves (dashboard_persistence.py:114). Restructuring the save-through-a-frozen-copy mechanism in one endpoint would fork the just-litigated shared shape; Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 fixes both endpoints together, with fix: isolate edit rewind from discarded context #8145's round-9 constraint (post-save witnesses, never pre-save copies) recorded.

3. :635 history-key re-check accepts a closed/replaced slot — verified real, verified inherited, added to Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 (item 5). Confirmed the window: close_slot cancels slot.task (the reservation) but not the request handler, and a detached slot keeps its history key, so the key-only re-check passes in BOTH endpoints — chat_rewind.py has the identical re-check with no slot-identity guard. The fix you sketch (serialize close/replace against the transaction; identity-guard save/commit) is a state-level mechanism spanning close_slot and both endpoints — exactly Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419's scope, now recorded there verbatim.

One PR, one shape: everything endpoint-local and newly-introduced is fixed here (rounds 1–2: arrival erasure, cap-safe retention); everything shape-level and shared with the merged reference is batched in #8419 so the two implementations move together.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

First Principles (round 1) + Design (round 3) on 41a2a491e — consolidated response (both advisory; both restate the two standing themes, now with aligned remedies):

• Extraction sequencing: both lanes now converge on the same plan this PR records — the regenerate port is the trigger ("rule of three" per Design; "before copy refactor: remove dead legacy compatibility shims #3" per FP), and the extracted boundary+commit helper lands WITH that port as its first consumer, parameterized by the pinned divergences. No further dispute to resolve; the sequencing is written in the PR body's follow-up section and Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 cross-references it.
• regenerate reachable today: correct and undisputed — it predates this PR (the issue scoped edit-resend; rewind was fix: isolate edit rewind from discarded context #8145's scope). FP's own verdict frames it accepted-and-deferred; the PR body names it the next mechanical port.
• Deferral risk if Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 stalls: the deferral is deliberately surfaced (PR body disposition section, Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 items 1/5 with verified mechanics) so the required maintainer approval on this PR is an explicit sign-off on it, and Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419 carries the full fix sketch to keep it unstalled. Landing this port does not widen the rewind-side exposure; it brings edit-resend's two user-facing defects (edit_resend shares the rewind context-boundary root cause fixed for rewind in PR #5395 #7838) to fixed while the shared-shape hardening proceeds as one coordinated change.

FP's ships-enumeration (10/10 justified) is appreciated — no contested surface remains in either lane.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#g2-8421]

GPT round 7 (d51533b56, 2 findings) — new head b0ec2996d. Finding 2 is correct and is a defect I introduced; the correction is REVERTED rather than replaced, and the reason is a direction argument worth stating. Finding 1 is the same finding as round 5 and now has a written remedy it cannot get here.

---

2. chat_regenerate.py:597 "commit double-counts a trim that precedes save stamping" — correct, and I verified it before acting. Driving the pre-stamp interleaving end to end (arrival at the cap inside atomic_write, i.e. after the write and before the witness stamping) with round 6's line in place: _disk_window_len lands at 4 against a file whose window region holds 3 rows. The sum-invariant argument was sound about the trim and wrong about the save — the stamp is an absolute assignment, so it erases a pre-stamp decrement, and the commit then adds it back a second time. Over-claiming is the harmful direction: a later trim credits rows to the frozen prefix the file does not hold, and the next save re-emits window rows. That is the duplication this PR exists to prevent, so the round-6 line was a net regression on its own terms.

Reverted, not replaced, because the commit cannot know the right value. Your suggested "commit the exact saved window count" requires the count the save wrote, and the commit has no access to it: a trim can land on either side of the stamp and the two are indistinguishable afterwards, and len(msgs_snapshot) is not that count either (the save filters a note row authorized elsewhere out of the write, so the snapshot can be longer than the file's window region — over-claiming again, by a different route). So _disk_window_len is left possibly SHORT, which round 6 established by measurement is not a loss path: with it short, the flush writes ['u0', 'a0', 'edited', 'workflow result', 'later 0', 'later 1', 'later 2'] — every row, once — because the foreign-append merge preserves the on-disk window line the memory window has dropped. Short costs an under-credited prefix, a warning about rows that are in fact on disk, and one whole-file re-read. Long costs a duplicated transcript. The choice is not close, and it is now written in the code and in history.md rather than left implicit.

The two boundary counters keep their re-adoption unchanged, and the distinction is exactly the one this finding turns on: the save does not stamp _disk_older_count or _disk_older_durable_count, so the pre-await values are the file's truth in every interleaving. The test is now parametrized over both sides of the stamp and asserts the invariant _disk_window_len <= 3 rather than an equality — an equality passes on one interleaving and hides the over-claim on the other, which is precisely how round 6 got through. Mutation-verified: re-add the round-6 line and the before_the_stamp case fails assert 4 <= 3 while after_the_stamp still passes.

---

1. chat_persistence.py:3444 "route check and witness updates are not atomic" — the same finding as round 5, and I am not going to keep re-answering it in a thread. It is real, it is narrowed as far as this line can be narrowed, and its remedy is a change I should not make in this PR.

Round 6 removed the path.stat() from inside the guarded region, so it is now five plain attribute assignments with no yield point — that was the realistic preemption window for a worker thread. What remains needs an atomic multi-field publish, and your remedy names it correctly: one routing-keyed record. Concretely that means collapsing _pending_rewrite, _disk_window_len, _disk_meta_created_at, _disk_meta_observed and _frozen_prefix_cache into a single _ChatSlot field carrying the history_key it describes, published by one assignment and validated by consumers against their own routing. It would also give the commit above the count it is missing, so both of this round's findings have the same fix, which is the strongest argument that the fix is real and that it is not this line.

Why it is not in this PR: it is a __slots__ field-shape change with five consumers to migrate (channel_slots.py:339, chat_persistence.py:937/:1477/the stamp, chat_handlers.py:7890) plus the delete-won guard, which reads two of the fields independently and fails open when they are unset (chat_persistence.py:2992, :3501) — so a partial migration weakens a security guard rather than a cache. Landing that inside a PR whose subject is an endpoint's transaction ordering would put a persistence-core refactor behind an endpoint fix, and this PR is already carrying a duplicate-implementation question with #8431. It is recorded on #8419 with this analysis and the consumer list, and history.md now states the residual and the remedy in the spec so it is not review-thread-only knowledge.

This is where I stop and ask for a maintainer. Seven rounds have produced three real fixes (the original frozen-window/live-boundary duplication, the durable position base, the assignment-only stamping region), two corrections of my own work, and two findings whose stated harm did not survive measurement. The two standing findings both resolve to the witness-record change above. The options are a /ai-review override gpt <sha> for the residual, or landing the witness record first — and either is a maintainer's call, not mine.

Also rebased onto current origin/main (159a9fbee, which bumped main to 0.7.0 and ported the 0.6.0 changelog) — the changelog-history gate flagged my base as pre-dating that section. All gates re-run clean on the new base: flake8, mypy --platform linux (1289 files), black and subprocess-encoding ratchets, brand, harness-parity, docs-lint, scrub-lint, isort, changelog-history (8 shipped sections intact); 1,339 targeted backend tests green across the fourteen files touching this surface.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#g2-8421]

GPT round 8 (b0ec2996d) — one finding left, and it is chat_persistence.py:3444 for the fourth time, now with a concrete scenario. Answering the scenario specifically, because it deserves better than "as previously stated".

"Edit-resend on an unlinked cron slot → cron result binds it during the worker save → old transcript is truncated, then the endpoint returns 503 without committing live state."

That outcome is this transaction's specified contract, not a defect in it, and it is what this PR introduced to REPLACE something strictly worse. The pre-change endpoint is on the base commit of this branch and can be read directly: it did del slot.messages[index:], appended the edited row to the LIVE slot, and then awaited _save_slot_to_history(state, slot, msgs_snapshot) with no expected_history_key and no commit-side re-check. _save_slot_to_history resolves its write target from live routing at write time, so the same cron rebind made the old code write the truncated window onto the newly bound transcript — truncating a conversation the edit was never authorized against, and reporting 200. This PR's two fences turn that into a refusal. session.md states the resulting contract in this same commit: a rebind mid-boundary "returns a 503 and leaves the dashboard slot on the original branch (the live slot is never mutated before the commit), but it cannot restore the discarded native session".

And the truncated turns are not lost. An explicit-snapshot save takes the archive-safe rewrite path, so _archive_dropped_lines archives the dropped window tail before the rebuild (history.md § Rewrite path). The old transcript's file is truncated; its dropped turns are in the archive.

Committing live state anyway would be the actual defect. The slot now routes to the cron transcript; publishing the edited branch onto it would put one conversation's edited history on another conversation's slot. Refusing is the only correct disposition once the write has landed on the authorized transcript, which is why the merged reference implementation answers the same condition the same way — chat_rewind.py returns rewind_slot_rebound 503 from an identical commit-side re-check, shipped in #8145.

The remedy is the same one as rounds 5, 6 and 7, and I have declined it four times for the same reason. "Serialize rebinding with the save, witness update, and live-state commit using shared routing-keyed state" means a rebind gate spanning the event loop and a worker thread. The worker cannot take slot._lock (an asyncio.Lock), so this needs a new routing-keyed serialization primitive plus the routing-keyed witness record, and every rebind site must be migrated onto it: channel_slots.py:339, chat_persistence.py:937/:1477, chat_handlers.py:7890, and the delete-won guard, which reads two of those fields independently and fails open when they are unset (chat_persistence.py:2992, :3501) — so a partial migration weakens a security guard rather than a cache. That is a persistence-core change with its own design, its own tests and its own review; putting it behind an endpoint's transaction-ordering fix is how it gets reviewed badly. It is on #8419 with the consumer list and the analysis from rounds 5-8, and history.md carries the residual and the remedy in the spec so it survives outside this thread.

---

Where this PR stands, stated plainly for whoever picks it up.

Eight lane rounds. Three real defects found and fixed (the original frozen-window / live-boundary duplication — demonstrated writing ['u0', 'u0', 'a0', 'edited'] with the fix removed; the durable position base; the syscall inside the witness-stamping region). Two findings declined with measurement rather than assertion (retained-arrival durability; the stale-_disk_window_len deletion claim, which the foreign-append merge prevents). One of my own corrections reverted after the lane caught it over-claiming. Every fix landed in the SHARED _save_slot_to_history so rewind and edit-resend moved together and the #8145 shape was not forked.

The one standing finding is a residual whose remedy is a persistence-core refactor I am deliberately not making inside this PR. Two maintainer decisions are needed and neither is mine:

1. This lane. Either /ai-review override gpt b0ec2996d09d0690cf7bf50e21b9741b2368cfa7: <reason> for the residual, or land the routing-keyed witness/serialization change (Edit context-boundary shape: restore persistence witnesses on rebind-mid-save and SEL-record post-discard failures (rewind + edit-resend) #8419) first and rebase this on top.
2. fix(dashboard): make edit-resend a real conversation boundary #8431 is an independent port of the same fix onto the same api_chat_slot_edit_resend, also closing edit_resend shares the rewind context-boundary root cause fixed for rewind in PR #5395 #7838, touching the same two source files. Whichever lands second conflicts wholesale. This PR is the superset; fix(dashboard): make edit-resend a real conversation boundary #8431 explicitly scopes the app-isolation gate out. Which supersedes which is not a call this PR makes.

Everything else is green: on head a74dda25b the matrix ran 63 of 64 checks to success — all four Linux 3.12 shards, all four Windows shards, namespace-sandbox, Analyze (python), E2E (stub ACP backend, offline), Coverage Gate, Design Review, SAST (Semgrep), PR Hygiene — with this lane the only red, and b0ec2996d differs from it only by the changes described in rounds 4-8 plus a rebase onto 159a9fbee.

[bolichen97]

Scope reduced — this PR is no longer the edit-resend port.

Duplicate-implementation decision (item 2 of the round-8 note): #8431 is the vehicle for #7838. The endpoint half has been dropped from this PR rather than left to conflict wholesale. Reverted to origin/main: src/kiro_crew/dashboard/chat_regenerate.py, docs/system-specs/modules/session.md, and every edit-resend test in test/test_chat_regenerate_cov80.py. chat_rewind.py's module docstring is back to main's text too, since #8431 now owns edit-resend's contract.

What remains is the part #8431 does not touch at all — 4 files, +377/-43:

• chat_persistence.py — expected_disk_older_count and its drift refusal; the routing-gated witness stamping with the stat() and created_at hoisted above the guard.
• chat_rewind.py — the paired boundary captures, the kwarg on rewind's save, and the commit-side re-adoption of both counters.
• test_dashboard_chat_rewind.py — the wiring pin, plus two real-save tests re-pinned against rewind so the persistence mutations the dropped endpoint tests used to catch stay covered. Verified: if False: on the drift refusal fails test_rewind_refuses_when_a_cap_trim_moves_the_frozen_prefix; if True: on the witness gate fails test_rewind_leaves_the_witnesses_alone_when_a_rebind_wins_the_write. No test was weakened and none was dropped without its mutation being re-pinned.
• history.md — the spec for all of it, including the residual below.

Both defects are reachable on origin/main today, through the shipped #8145 rewind, with or without either port. That is why they are worth landing separately rather than riding whichever endpoint port wins.

One follow-up this exposes: save_slot_off_loop's keyword list is explicit and does not forward expected_disk_older_count, so a boundary transaction routed through it still reads the live counter in the worker. #8431's edit-resend saves that way, so it keeps exposure 1 until the parameter is threaded through. Noted in history.md and in the body.

The GPT 5.6 Review residual travels with this PR, since it is anchored in the one file this keeps. Its disposition is unchanged from rounds 5-8 and is restated in the body: the remedy is one routing-keyed witness record, a __slots__ field-shape change across five consumers plus a delete-won guard that fails OPEN when its two fields are unset, tracked on #8419. Item 1 of the round-8 note still stands: either that lands first, or a maintainer records an override for the residual.

[bolichen97]

GPT round 9 (d544f5372) — one finding, and it is the same residual, now on a diff that no longer contains an endpoint. Recording the one piece of evidence a maintainer needs that the earlier rounds did not state.

The finding verbatim, from the lane's extraction step:

BLOCKING -- src/kiro_crew/dashboard/chat_persistence.py:3444
route check and witness publication are not atomic
`if slot_history_key(slot) == history_key:` … `slot._disk_meta_created_at = _post_write_created_at`
cron rebind after key resolution -> stale check passes -> old identity stamps new route
  -> next flush refuses and clears dirty, losing a follow-up message
Anchor: residual/crash-data-loss-corruption
Fix: publish one routing-keyed witness record atomically and validate its key at consumers.

The race is real and the harm chain is coherent — I traced it rather than re-asserting the earlier disposition. _disk_meta_created_at stamped with the OLD file's identity onto a slot that now routes to a NEW one makes the delete-won guard's identity comparison mismatch on the next save, and that guard's refusal is the one that returns cleanly so the flush loop clears _dirty (chat_persistence.py, "Returning cleanly (no mkdir, no write, no raise) lets the flush loop clear _dirty"). A follow-up message on the new transcript is lost. Nothing about that is overstated.

What is new, and what the fence's residual/ anchor is telling you: this PR is what NARROWED that window, and it cannot be closed further without the change it names. On origin/main the same five witnesses are stamped unconditionally — there is no routing check in that block at all. Verify directly:

git show origin/main:src/kiro_crew/dashboard/chat_persistence.py | sed -n '3357,3396p'

You will see if rewrite: slot._pending_rewrite = False, slot._disk_window_len = len(window), slot._disk_meta_created_at = ..., slot._disk_meta_observed = True and the _frozen_prefix_cache stamp with path.stat() inline, and no slot_history_key anywhere near them. So today a rebind landing anywhere across the whole write — including across the stat() syscall — poisons all five. After this PR it can only land in the gap between one comparison and five attribute stores, with both syscall-bearing computations hoisted above the check. The exposure is strictly smaller in every interleaving and larger in none.

And it cannot be closed here. The only fix is atomic publication of the five as one routing-keyed record, and I checked the two cheaper repairs before concluding that:

• Re-check after the stores and restore the pre-save values — wrong, and specifically wrong on _pending_rewrite. Re-arming it on the NEW transcript sends the next flush down the destructive rewrite path, which rebuilds that file from this slot's memory window and discards any cross-process append. There is no safe forced value for that flag on a transcript this save never read.
• Migrate only _disk_meta_created_at + _disk_meta_observed into one tuple field, which is the pair the named harm chain runs through — that closes this chain but leaves _pending_rewrite and _disk_window_len in the same non-atomic region, so the finding stands anyway, and it half-migrates the delete-won guard, which reads those two fields independently and fails OPEN when they are unset. A half-migrated fail-open guard is worse than none of it.

So the position is unchanged from rounds 5-8, and now with the delta measured: full remedy on #8419 (__slots__ field-shape change, five hydrate sites — channel_slots.py:338, chat_persistence.py:934/:1474/the stamp, chat_handlers.py:8051 — plus the guard reads at :2985/:3511), residual and remedy written into docs/system-specs/modules/history.md so it is not thread-only knowledge.

Override rationale, if a maintainer chooses that path — stated so it can be checked rather than trusted: this change strictly narrows a pre-existing unconditional-stamping race (origin/main has no routing check in that block at all) and introduces no new one; the residual's only remedy is the routing-keyed witness record tracked on #8419, whose partial application would weaken a fail-open security guard. The two verifications are the git show above and the fail-open reads at chat_persistence.py:2992/:3516. I am not posting an override — that is a repository-writer action and the repo says the rationale must be independently verified first.

[bolichen97]

/ai-review override gpt d544f53: residual of a wider main defect this PR narrows -- main stamps all five witnesses with no routing check and stats the file inline (chat_persistence.py:3357-3394), while the delete-won guard's evidence gate at :2992 fails OPEN, so a partial migration is unsafe. The full fix is tracked on open issue #8419.

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for d544f537222cda5d0d291e8ba6a6f429ed1aa3b3.

residual of a wider main defect this PR narrows -- main stamps all five witnesses with no routing check and stats the file inline (chat_persistence.py:3357-3394), while the delete-won guard's evidence gate at :2992 fails OPEN, so a partial migration is unsafe. The full fix is tracked on open issue #8419.

This decision applies only to this commit. A new push requires a new judgment.

[iamwhatever]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: a frozen messages snapshot was written against a _disk_older_count read after a concurrent cap-trim, so trimmed rows landed twice — once in the frozen prefix, once at the head of the snapshot. The save now takes the paired expected_disk_older_count and refuses on drift, and rewind's commit restores the boundary the file was actually written against. Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/modules/history.md.