fix: isolate edit rewind from discarded context

[chenmingwei23]

Problem / Motivation

Edit + Send visually rewound a dashboard conversation, but the next turn could still receive the discarded suffix through native ACP session resume, generic session-history reconstruction, or queued successors.

Supersedes #5395 (fork PR by @Premshay, who remains the git author of this identical commit). The fork PR converged on the code but could not converge on process: fork review lanes re-review from scratch on every roll and do not honor the recorded /ai-review override marker (target=gpt head=002066e35, actor chenmingwei23), and the marker is voided by every rebase while main moves several times a day. Same-repo lanes honor the override mechanism, which is what this PR restores.

Why it matters

Editing a prompt must create a real conversation boundary: the replacement turn may use only the retained prefix and revised prompt, including after a process restart or in another open client. Without it, discarded context silently re-enters the model's view.

What changed (motivation -> approach -> change)

The rewind request is one-shot and built only from the retained slot prefix. It clears the native conversation identity durably before saving the rewritten history (endpoint-side aflush), rejects the request if that boundary cannot be persisted (503) or was refused (save guards, rebind fence, busy session 409), removes queued successors (snapshot-only, arrivals survive), and notifies other clients via queue_cancel. The slot is reserved before the awaited boundaries so concurrent sends queue instead of racing; a client disconnect mid-save still commits a landed rewrite and dispatches the edited prompt; app-authenticated rewind of a channel-linked slot is refused (404, anti-enumeration). Carries all 12 verified blocking findings fixed across six GPT review rounds plus one First Principles round on #5395.

Accepted residue (operator ruling, recorded via the override marker on #5395): narrow races against the non-atomic rewrite (channel cold-start after discard; slot-close vs in-flight rewrite). Outcomes are recoverable -- the dropped suffix is archived, canonical history is retained per spec -- and strictly smaller than main's rewind, which has no such fences at all. Serializing rebinding/flushing with persistence is deferred to a tracked follow-up (local backlog f-20260903-01: extract the prepare/commit boundary onto _ChatSlot and migrate edit_resend onto it).

Tests

• test/test_dashboard_chat_rewind.py: 34 tests incl. new coverage for save-refusal 503, slot reservation + concurrent-queue survival, rebind refusal, busy-session 409, channel-linked app denial, cancellation-mid-save commit
• test/test_session.py + test/test_context.py + test/test_dashboard_chat.py + test/test_error_code_contract.py: 1174 passed on the rebased tree
• isort, flake8, mypy (1279 files), black baseline gate: clean

Manual verification

The focused endpoint tests cover durable-boundary failure, native-session discard, retained-prefix reconstruction, and queue removal. Manual Edit + Send verification remains appropriate for a live dashboard session.

Why no screenshot: No rendered UI changed; this fixes prompt/session state assembly.

Related Issues

Supersedes #5395. No linked issue: no existing issue matches this bug fix.

Pattern harvest

Rule candidate: review-prompt
Pattern: a destructive edit that removes content in one layer while the data stays reachable through a parallel replay channel (native session resume, history reconstruction, queued successors) -- every replay path must be cleared, durably, before the replacement is dispatched.

Checklist

• At most two commits (one), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (docs/system-specs/modules/session.md)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Dependencies (discard_conversation(skip_if_busy=), aflush, expected_history_key on the save, _start_next_queued_turn) all pre-exist on base, so the PR is self-contained. The discard-before-persist ordering is the correct fail-closed direction for the invariant (persisting first and failing the discard would resurrect the exact leak), and the accepted residue is disclosed in both the description and session.md.

Design-Verdict: CONCERNS

Sound fix for a real replay-leak, but a hand-rolled two-phase commit over _ChatSlot privates now lives inline in an HTTP handler.

Watch

• The prepare/commit protocol is built from copy.copy(slot) plus a field-by-field list of what to re-copy and a comment-maintained list of what to deliberately NOT copy ("Deliberately NOT copied … the persistence witnesses"). Any future _ChatSlot field with mutation semantics is silently shared between the prospective and live slot, so a rejected rewind could leak partial state — the bug class this PR fixes. The author's own follow-up (extract prepare/commit onto _ChatSlot, backlog f-20260903-01) is the fix; it should land before edit_resend or any other caller copies this inline pattern.
• Endpoint-side sessions.aflush() makes rewind's durability depend on flushing the whole session map from a request handler; acceptable now, but it is a second caller-specific durability convention on discard_conversation that the tracked _ChatSlot boundary should absorb rather than proliferate.

[DESIGN-REVIEWED] ac833c8

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of ac833c851818cb846245075bb4af4d98feae95f2 — 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 verification is done. The diff confines itself to the rewind endpoint (every mechanism it calls — discard_conversation(skip_if_busy=), aflush, _save_slot_to_history(expected_history_key=) — already exists on base), the spec, tests, and two mandated baseline files. The one thing that survives the kill-filter is the sibling count: edit-resend in chat_regenerate.py carries the same defect and is still a live UI path.

First-Principles-Verdict: CONCERNS

The rewind path is fixed at cause level, but the same replay defect stays live in edit-resend, which the Mochi chat panel still calls today.

What this change ships

Intent: editing a past prompt must truly discard the suffix from every replay channel — a FIX.

1. The replacement turn can no longer resume the discarded native conversation, even after a restart — justified (the fix).
2. Edit + Send fails with 503 instead of proceeding when the boundary can't be persisted — justified.
3. Edit + Send returns a retryable 409 while a channel reply is in flight, instead of tearing it down — justified.
4. Queued messages from the discarded suffix are cancelled; other clients see the cancellation — justified.
5. A message sent during the edit queues behind it instead of racing it — justified.
6. A client disconnect mid-save still lands the edit and runs the new prompt — justified.
7. An app can no longer rewind a channel-linked slot (404) — rides along; app-isolation boundary, declared.
8. The two existing "not found" errors gain a machine-readable code — rides along; mandated by the error-code ratchet.
9. session.md documents the new boundary — justified (same-commit spec rule).
10. An edit whose slot was rebound mid-save is refused instead of overwriting the injected state — justified.

Watch

• Point patch with counted siblings: api_chat_slot_edit_resend (src/kiro_crew/dashboard/chat_regenerate.py:208) does the same job and "leaves the backing kiro-cli session file with stale forward turns" (src/kiro_crew/dashboard/chat_rewind.py:3-5) — and it has 1 real consumer today, the Mochi panel's edit flow (website/src/apps/mochi/src/renderer/ChatPanel.tsx:1008 via panelBridge.ts:1473). The description defers the migration (backlog f-20260903-01) but frames the deferral as the race follow-up, not as "the reported defect remains user-reachable through this surface."
• Same save-failure class this PR 503s on is swallowed by both siblings: grepped _save_slot_to_history call sites under dashboard/ — regenerate (chat_regenerate.py:88-92) and edit-resend (chat_regenerate.py:277-281) log the failed rewrite and dispatch anyway (2 unfixed siblings); regenerate is not named in the deferred follow-up.

[FIRST-PRINCIPLES-REVIEWED] ac833c8

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @chenmingwei23 overrides the GPT 5.6 finding for ac833c851818cb846245075bb4af4d98feae95f2; 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 ac833c851818cb846245075bb4af4d98feae95f2: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] ac833c8

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

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

[chenmingwei23]

Kiro Crew [operator: chenmingwei23#pr-drive-5395]

Fixed the GPT round-9 blocking finding (chat_rewind.py:393, verified real) in ac833c8.

Finding: after a successful rewind the commit closure copied the prospective slot's PRE-save persistence witnesses back onto the live slot. The save runs on the LIVE slot and stamps the post-rewrite truth (_pending_rewrite cleared, _disk_window_len/disk_meta*/_frozen_prefix_cache matching the truncated file); restoring the stale copies re-armed _pending_rewrite -- so the next flush repeated the destructive rewrite and could discard a cross-process append (workflow/cron) that landed in between -- and moved the monotone _disk_tail_ts floor backwards (state.py note_disk_tail documents that floor as forward-only).

Fix: the commit closure no longer touches the persistence witnesses; the live slot keeps what the save stamped. Both commit call sites (success path and cancellation-after-landed-save) run only after a landed save, so the post-save values are authoritative at each.

Test: test_rewind_commit_keeps_the_post_save_persistence_witnesses -- the mocked save stamps witnesses on the live slot like the real one; asserts the commit leaves _pending_rewrite False and keeps the advanced _disk_tail_ts. Full local gates green (35 rewind tests, 1140 session/context/chat/error-contract, isort/flake8/mypy/black).

Note: this finding is NOT in the operator-accepted residue class (narrow races vs the non-atomic rewrite) -- it was a genuine defect in the round-6 commit-closure refactor, introduced on the superseded #5395 and carried here, hence fixed rather than overridden.

[chenmingwei23]

/ai-review override gpt ac833c8: Operator-accepted residue class: narrow races vs the non-atomic rewind rewrite (channel cold-start after discard; rebind during save; slot-close vs in-flight rewrite). Outcomes recoverable: suffix archived, canonical history retained per spec. Rounds 5-8 on superseded #5395 each landed a variant prescribing feature removal or cross-module serialization, out of scope; serialization deferred to tracked follow-up f-20260903-01.

[github-actions]

Human judgment recorded

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

Operator-accepted residue class: narrow races vs the non-atomic rewind rewrite (channel cold-start after discard; rebind during save; slot-close vs in-flight rewrite). Outcomes recoverable: suffix archived, canonical history retained per spec. Rounds 5-8 on superseded #5395 each landed a variant prescribing feature removal or cross-module serialization, out of scope; serialization deferred to tracked follow-up f-20260903-01.

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