Skip to content

feat(chat): render resumed transcript from ACP replay (flag, off) - #8862

Open
CrysisDeu wants to merge 1 commit into
mainfrom
feat/acp-replay-render-flag
Open

feat(chat): render resumed transcript from ACP replay (flag, off)#8862
CrysisDeu wants to merge 1 commit into
mainfrom
feat/acp-replay-render-flag

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Design rationale: docs/request-for-change/rfc-acp-replay-as-transcript-source.md — why the replay should own what the agent saw and the JSONL should shrink to a sidecar, with phasing P0–P3.

Prototype behind dashboard.replay_from_acp (default off): render a resumed session's transcript from kiro-cli's own session/load replay, with Kiro Crew's JSONL supplying only the rows the agent never saw. The JSONL keeps being written either way — this changes what a resumed session reads, never what it persists.

Today AcpRuntime.load_session issues session/load and the reader loop counts and drops every replayed session/update frame ("counted, not logged"); the dashboard then renders the JSONL it wrote itself. So the transcript is dual-written and the agent's own record is thrown away. With the flag on:

  • AcpRuntime(capture_replay=True) arms a per-sid bucket before the load request goes out; the reader appends that sid's session/update frames instead of dropping them (bounded by _REPLAY_CAPTURE_MAX_FRAMES = 20_000; overflow takes the ordinary counted drop). The bucket is popped on every exit of the load, so a failed/timed-out load never leaves an armed bucket for an id a later session reuses. Frames land on AcpSessionHandle.replay_updates.
  • dashboard/chat_replay.py folds those frames into rows through the same parse_session_update the live path uses (record_metrics=False so replayed calls do not emit near-zero kirocrew.tool.call.duration samples), then overlays JSONL rows: clean user text / mid / sendId / steer / nudge flags from the matching JSONL prompt row (matched by containment inside the replayed assembled prompt, dash-folded), humanized tool title / purpose / kind from the JSONL tool row with the same tool_call_id, and every non-derivable row (notice, error, permission, inject, compaction, steer-user) re-inserted at its turn, anchored on the tool it followed.
  • GET /api/chat/slots/{slot} returns transcript_source (jsonl | acp_replay) and replay_report (rows per source); every row carries meta.source. ChatPage shows a banner when the transcript came from replay. Settings → Chat gets the toggle; dashboard/config GET/PUT carry it.

Flag off (the default) is a no-op: AcpProvider.replay_updates answers None, the handler keeps the JSONL path, the runtime keeps its counted-drop behaviour.

What the probe measured (kiro-cli 2.21.0, engines v1 / v2 / v3)

Full report + scripts live in the operator workspace (research/acp-replay/acp-replay-fidelity.md); the load-bearing findings are also recorded in docs/system-specs/modules/acp-client.md:

question v1 v2 (what Kiro Crew runs) v3
initialize.protocolVersion answered 1 1 1 (rejects our dated string with -32602; we already send the integer there)
session/resume + replayFrom -32601 -32601 -32601
assistant text / thinking text identical to live ✅ / ✅ ✅ / ✅ (115 = 115 chars) ✅ / n/a
tool toolCallId / rawInput / rawOutput / final status
tool title raw tool name (read, shell) same humanized ✅
tool locations, content preview, _meta.kiro.toolName dropped dropped
messageId / timestamp / replay:true on frames none none ✅ every frame
user turn full assembled prompt as user_message_chunk same same
session/load on a sid another process holds refused (Session is active in another process, 0 frames) same same

Real Kiro Crew session (690 JSONL rows, 395 tool calls, 100 nudge cycles, one auto-compaction) replayed in a fresh process: 395/395 tool rows matched by tool_call_id, 176/177 assistant rows matched, 100/100 nudges and 10/11 user rows found wrapped inside the replayed prompt; only [Tool blocked] injects, transient-retry errors and the compaction banner had no replay counterpart. No truncation at the compaction point.

The refused-resident-sid result is why this design captures at the gateway's own resume rather than asking kiro-cli to replay on tab open.

Screenshots — same session, same pod, flag on vs off

Differences between the two renders, in red:

  • ON shows the model's Thought process block on turn 3 — the live path broadcasts thinking but never persists it, so OFF has no reasoning to show after a restart.
  • ON turn 1 renders the tool card with its purpose line and file chip (Read the specified file to quote its first line. · replay-demo-notes.txt) built from the replayed rawInput; OFF shows the JSONL's collapsed "Worked through 1 step".
  • ON carries the "Rebuilt from the agent's own record" banner explaining that this view can show more than the log (e.g. the agent's thinking); OFF has none.
  • Approval cards: the pod ran with --approval reads, so the shell turn raised a permission request; resolved permission rows are not surfaced by _prepare_messages after a restart in either mode, so there is no visible difference to show. Cron-injection cards were not produced in this demo session (no cron ran into it) — the overlay path for them is the generic sidecar branch, covered by test_sidecar_rows_anchor_after_their_tool.
dashboard.replay_from_acp = ON dashboard.replay_from_acp = OFF (default)
replay on replay off

Settings → Chat → Sessions, the new toggle (outlined in red):

settings toggle

Review round 1 — what changed

  • Pagination coherence (GPT F1, Opus advisory): the merge now runs on the WHOLE corpus before limit/before slicing (_merge_replay_corpus), so total/has_more/next_before are computed in the merged index space and every page is cut from the same list; the unbounded path merges only when no rotated-archive head is advertised. Merged rows are cached per slot on (frame count, corpus length, last row) so the detail poll does not re-walk the replay each time (design-review suggestion).
  • Harness parity H14 (GPT F2): replay_updates is declared on LLMProvider with a None default and read directly — no getattr probe.
  • Feature map (GPT F3): new "Transcript from agent replay" row under Sessions and the Settings chat row updated.
  • Agent-SDK boundary gate: the frame folding moved into kiro_crew.agent_sdk.drivers.acp.fold_replay_updates (function-local ACP imports, plain-data result); dashboard/chat_replay.py no longer imports the ACP layer.
  • Design review: the user-request header is now a context.USER_REQUEST_HEADER constant imported by the reader (dash-folded compare) instead of a respelled literal.
  • UX review: banner and toggle copy rewritten in user-outcome language (all 12 locales, {{productName}} interpolation); the Settings toggle is screenshotted above.
  • First principles: dropped replay_covered and unmatched_jsonl_turns (no consumers); transcript_source is emitted only on the replay path, so default installs gain no new field.
  • Frontend catalog parity / DNT / phantom-class findings fixed (all locales carry the keys; bg-card instead of the undeclared bg-surface).

Review round 2 — what changed

  • One index space per session (GPT F1): the merge is refused on BOTH the unbounded and the paginated path whenever the session has a rotated archive (probed via read_rotated_messages_chained, fail-closed to JSONL), so the cursor an initial response hands out is always applied to the same corpus shape.
  • Byte ceiling on capture (GPT F2): _REPLAY_CAPTURE_MAX_BYTES (64 MiB, serialized) alongside the frame cap; on overflow the whole capture is discarded (not truncated) and the handle gets an empty list, so the consumer renders its own transcript. Test added.
  • No turn theft by short prompts (GPT F3): JSONL prompts match the human's slice of the replayed prompt by equality or by a newline-bounded prefix (the writer appends per-turn guidance on its own lines); containment is a fallback only for rows ≥ 40 normalized chars. Tests added for the a / a longer question about a case and the guidance-suffix case.
  • isort order in chat_handlers.py; zh-CN copy reworded to keep ≤ 2 per clause.

Review round 3 — what changed

  • Quick-prompt turns (GPT): a persisted /plain row reaches the agent as its expanded macro, whose first line is the [QUICK PROMPT <token>] header both derive from. quick_prompts.quick_prompt_header() (new public helper on the writer's own module) lets the matcher recognise the row by that header instead of leaving it unmatched and rendering the turn twice. Test added with the real expand_quick_prompt("/plain") output.

Review round 4 — what changed

  • Merge cache witness (GPT): the per-slot cache now keys on a fingerprint that folds in the row fields mutated in place (meta.resolved, meta.done, output length, turn stats, file changes, content length, mid, ts), not just corpus length and last row, so a permission resolved or a tool completed after the last merge misses the cache.
  • Provenance cleared (GPT): both switchSlot/refreshSlot reducers delete slotTranscriptSource[slot] when a detail response carries no transcript_source, so the banner cannot outlive the replay (flag off, provider gone, rotated archive).
  • Error-code contract (Windows backend shard): the replay_from_acp 400 now carries code: invalid_replay_from_acp, returning handlers/files.py to its baseline count. The other Windows failure (test_dashboard_status_snapshot — xdist worker crash) is unrelated to this diff.

Review round 5 — what changed

  • Frontend shards (all four red): the switchSlot/refreshSlot reducers now guard state.slotTranscriptSource (??= on set, existence check on delete) — test fixtures and persisted states predating the field build ChatState without it, and delete undefined[…] threw in ChatPageCoverage.test.tsx.
  • Rewrites (GPT F1): LLMProvider.discard_replay() (no-op default; AcpProvider/AcpSessionProvider drop the handle's frames) is called by rewind, regenerate, edit-and-resend and switch-variant via chat_replay.discard_replay_for_slot, which also drops the merged-rows cache. Independently, the merge itself treats a turn whose JSONL assistant text matches none of the replay's as REWRITTEN and renders that turn from the JSONL, so the old answer cannot come back even if an invalidation is missed. The cache moved into chat_replay.py (merge_replay_transcript_cached, corpus_witness) so the rewrite endpoints can reach it without importing chat_handlers. Tests added for the rewritten turn, the in-place-mutation cache miss, and the invalidation.
  • Frame-count overflow (GPT F2): now discards the whole capture like the byte ceiling does — never a truncated prefix. Test updated.

Review round 6 — what changed

  • Discard placement (GPT): discard_replay_for_slot moved from the top of each rewrite handler to immediately before the transcript mutation (del slot.messages[…] in regenerate / edit-and-resend, the variant content swap, and rewind's _commit_live_state), so a request refused by an authorization or validation gate discards nothing.

Review round 7 — what changed (rebase onto current main)

  • One corpus per cursor chain (GPT F1, chat_handlers.py): the paginated path no longer re-decides replay-vs-JSONL per request from the rotation probe. The FIRST page of a chain (no before) decides, chat_replay.note_cursor_space records it per slot, and every follow-up page (before set) reads it back via cursor_space, so a size rotation landing between two "load earlier" requests cannot flip the corpus under a cursor the client already holds. The paginated read is the full chained corpus, so merging stays valid across a rotation. The unbounded path records its space too (its rotated-archive cursor is JSONL-space). A rewrite forgets the record along with the merge cache; an unknown slot (gateway restart mid-scroll) decides afresh.
  • Frame-cap overflow count (Opus): _replay_capture_frame_capped tracks the sids discarded for the FRAME cap separately from byte-ceiling discards; every later frame for such a sid still increments _replay_capture_overflow, so the discard log reports how far past the cap the replay ran (byte discards keep overflow at 0, as test_byte_ceiling_discards_whole_capture pins). test_capture_cap_discards_whole_capture (overflow == 2) now matches the code.
  • Byte ceiling in bytes (GPT): the cumulative replay-capture size is len(json.dumps(...).encode("utf-8")), not a character count, so CJK-heavy frames are measured against the 64 MiB memory bound correctly.
  • Sidecar anchor (shard 1 red, test_compaction_notice_stays_as_sidecar_after_its_turn): JSONL-only rows are anchored on the last replayed row they followed — tool call OR matched assistant text — instead of tool calls alone, so a compaction banner that followed the answer no longer renders before it. test_sidecar_rows_anchor_after_their_tool updated accordingly (the trailing error row now follows the matched answer, matching the JSONL order).
  • Security posture allowlist (shard 3 red): agent_sdk/drivers/acp.py and dashboard/chat_replay.py added to NON_EGRESS_REDACTION_MODULES with the reason — both scrub as the rows are BUILT; the registered dashboard sinks serve them.
  • Rebase: edit-and-resend's discard now lives inside main's new _commit_live_state (main moved the truncation onto a prospective copy committed after the durable boundaries); locale catalogs re-derived by key-level replay onto main's bytes (+4 keys each, no reordering).

Review round 8 — what changed

  • /clear keeps the replay (GPT F1, chat_runner.py): the EVENT_CLEAR_STATUS handler now calls discard_replay_for_slot before slot.messages.clear(), so the live provider's replay frames, the merge cache and the cursor-space record go with the wiped transcript — the next detail fetch renders the (empty) JSONL instead of rebuilding the cleared conversation from retained frames. Same discipline rewind / regenerate / edit-and-resend already follow.
  • Round-7 shard 4 red (test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists, two ordering tests) is not this PR's: the diff touches no snapshot code, main's own ci.yml failed the same tests at 002081169 and passed them at this PR's base 30946933c.

Review round 9 — what changed

  • Corpus travels IN the cursor (GPT F1, replaces round 7's per-slot record): the per-slot _CURSOR_SPACE map is gone. Every page that hands out next_before while a replay is on offer also returns cursor_space ("acp_replay" | "jsonl"), and the client echoes it back as corpus= with the next before (api.chatSlotDetail, loadOlderMessages, threaded through setPagingCursor / the switch-origin cursor). The server keeps no cross-request state: two clients paginating one slot each carry their own space. A follow-up page that names no corpus (a caller predating the field, or the resume endpoint's JSONL cursor) fails closed to the JSONL corpus; the field is absent from the response whenever no replay is on offer, so default installs gain nothing new.
  • top-level-imports (GPT advisory, agent_sdk/drivers/acp.py): kept function-local on purpose and now says so at the import — the module docstring already documents that every kiro_crew.acp import here is call-time because the module sits on the dashboard boot path and the ACP package pulls in the client and runtime.

Review round 10 — what changed

  • Stale replay cursor is refused, not applied (GPT F1, chat_handlers.py): a follow-up page that echoes corpus=acp_replay while the slot no longer offers a replay (regenerate / rewind / edit / /clear / session reset discarded it between two pages) now answers 409 cursor_stale instead of slicing the differently-indexed JSONL with the replay-space offset. loadOlderMessages recognises the code (isStaleCursorRejection) and re-reads the first page through refreshSlot — a fresh cursor in the current corpus — rather than showing the red retry bar.
  • Default call shape unchanged: loadOlderMessages passes corpus to api.chatSlotDetail only when the backend named one, so default installs send exactly the request they always did (the round-9 shape made two existing vitest suites see a fifth undefined argument).
  • Round-9 Bundle Size Gate red (all chunk 10.25 MB vs 10.24 MB, +5.3 KB) is main-side drift of the eager i18n catalogs: pristine main measures 10450 KB against the 10490 KB ceiling, and the re-measure lands in fix(ci): re-measure the drifted all-chunk bundle ceiling #8935. Not folded in here; this PR rebases once that merges.
  • Round-9 Backend Tests (3.12, 2) red was fetch failed on the runner's tool download step, not a test.

Review round 11 — what changed (rebase onto main after #8935)

  • Replay module off the boot path (GPT B1): no boot-loaded module imports chat_replay any more. chat_handlers._merge_replay_corpus imports the merger only after replay frames are confirmed present, and the five rewrite call sites (rewind, regenerate, edit-and-resend's commit, variant switch, /clear) go through chat_utils.discard_slot_replay, which checks provider.replay_updates first and imports chat_replay.discard_replay_for_slot only when a replay is actually on offer — AUTOSDE no-new-work-on-gateway-boot-path rule 5, gate the import, not just the handler. A default (flag-off) install never loads the module: not at boot, not on a rewrite.
  • Duplicate prompts disambiguated by identity (GPT B2, chat_replay._match_prompt): each pass now collects every JSONL prompt group that matches the replayed prompt text; when there is more than one, the group whose body shares an identity with the replayed turn wins — a tool_call_id the replay carries, or an assistant text the replay has (_turn_identity_score). A replay that omits the first of two identical turns therefore enriches the second turn instead of handing its thinking and tools to the first. Ties (nothing in common) keep the earliest, the previous behaviour. Pinned by test_duplicate_prompts_attach_replay_to_the_turn_that_shares_identity and test_duplicate_prompts_without_identity_keep_the_earliest.
  • Rebase: onto main after fix(ci): re-measure the drifted all-chunk bundle ceiling #8935 raised the all-chunk ceiling; the only conflict was docs/request-for-change/README.md (main rewrote the audit preamble; this PR's RFC row appended to main's table). The round-9 deadKeys red (30 vs baseline 29) was the old base — on the rebased tree the dead set is identical to main's (29).
  • Round-9 shard reds not owned here: test_approval_threading (subagent approval threading), test_snapshot (notification-copy ordering, also red on main's own runs), test_session_pool wedged-shutdown timeout on Windows — none touch replay code; Backend Lint was cancelled by a superseding run, not failed.

Review round 12 — what changed

  • Docs Lint (line citations in prose): the RFC cited eight file.py:NNN locations; docs-lint wants symbol names, which survive the refactor that moves a line. Replaced with the symbols (AcpRuntime.load_session, AcpRuntime._note_dropped_frame, _capture_replay_frame / _take_replay_capture, merge_replay_transcript, the five discard_slot_replay call sites by function). The dangling branch-head SHA in its front matter (audited-at) now names the main base the branch is measured against.

Review round 13 — what changed

  • Replayed thinking is server-backed (GPT F1, chatSlice.ts): a thinking row tagged meta.source === 'acp_replay' comes back on every detail fetch, so it is no longer treated as the client-only live block. isReplayBackedThinking makes isDurableRow count it (paging-cursor arithmetic and coverage), mergePreservedThinking skips preserving it (the page carries it again), and the three kept-head filters keep it as a server row. Live-broadcast reasoning (no source) keeps the existing preserve-and-re-seat path.
  • Cache witness over the actual values (GPT F2, corpus_witness): the witness is now a blake2b digest of every row (role, ts, content, full meta), not lengths and flags, so a same-length in-place refinement of purpose / input / any sidecar field misses the cache. test_cached_merge_misses_on_in_place_mutation extended with a same-length purpose change.
  • Tied prompt candidates are ambiguous (GPT F3, _match_prompt): when the best identity scores tie, the turn is reported _AMBIGUOUS and the merge drops its replay rows, letting the JSONL — which records both turns — stand in order; it no longer picks the earliest. test_duplicate_prompts_with_tied_identity_fall_back_to_jsonl replaces the earliest-wins test.

Review round 14 — what changed

  • Automation-dispatched turns are prompts too (GPT F1, _is_prompt_row): the queue drain persists a cron notification, a recovery re-send, a user-message replay and the synthesis prompt as inject rows (with meta.injectKind naming the dispatch) and a sub-agent completion as a subagent row — never as user. Those rows now open their own JSONL group, so the replayed turn lands on them instead of leaving the prompt in the previous group and rendering it twice with its answer under the wrong prompt. Classification is by persisted provenance (_TURN_STARTING_INJECT_KINDS = cron / recovery / user_replay / synthesis, and role subagent); an inject without one of those kinds — a policy notice, a reconcile note — is still delivered inside a turn and stays sidecar. Pinned by test_cron_and_subagent_dispatches_are_matched_as_their_turns_prompt and test_in_turn_inject_without_dispatch_kind_stays_sidecar.
  • Round-13 shard 4 red was test_snapshot.py notification-copy ordering again (also failing on main's own runs); not owned here.

Review round 15 — rebase onto current main

  • Rebased onto main af7f3e7fe (184 commits). Two conflicts, both unions: acp/_dispatch.py gained main's diff_path_cache parameter beside this PR's record_metrics (both threaded through parse_session_update_tool_call_event), and context.py's request-header line keeps main's reply-format scrubbing block with this PR's USER_REQUEST_HEADER constant in place of the literal. Locale catalogs merged cleanly (+4 keys per catalog, duplicate-key scan clean); the dead-key set equals main's (28). No code change beyond the merge.

Review round 16 — what changed

  • Process-wide retention budget (GPT F1, acp/runtime.py): the per-session 64 MiB ceiling bounded one resume, not N concurrent ones. A module-level _ReplayRetentionLedger (_REPLAY_RETAINED_MAX_BYTES = 256 MiB, shared by every AcpRuntime, lock-guarded) now reserves each accepted frame's bytes per session; a capture that would push the process past the budget is discarded whole — its consumer keeps the JSONL — rather than evicting a session already rendering from its frames. Bytes stay reserved while the handle holds the frames and are released on a failed load, a capture discard (either ceiling or the frame cap), AcpSessionHandle.discard_replay (now the seam discard_replay on the provider goes through, so a rewound/cleared session stops counting), and unregister_session. Pinned by test_process_wide_budget_discards_a_new_capture_and_frees_on_release; the discard log line reports the process-wide total.
  • Scope, documented rather than re-plumbed: capture_replay still reaches every provider the factory builds, but a capture only arms on session/load — a resumed dashboard session; session/new sessions (sub-agents, crons, background prompts) replay nothing and retain nothing. Stated at the factory call site and in docs/system-specs/modules/acp-client.md, which now lists all three bounds and the release paths.

Review round 17 — rebase onto current main

  • Rebased onto main 575a8390e (12 commits); the only conflict was the RFC index (docs/request-for-change/README.md, both rows kept). No code change. Round-16 reds not owned here: Windows shard 4 test_work_ledger (fails on main's own run ef38f4cbe), Windows shard 2 test_external_registry / test_file_change_snapshots (Windows-only; the same Linux shard passed, and this PR's only _dispatch.py delta is the record_metrics flag with an unchanged default); the Lint check-run reported red while its job had completed green.

Review round 18 — what changed

  • Merge cache bounded by bytes (GPT F1, chat_replay._MERGE_CACHE): entries now record their serialized size; the cache holds at most _MERGE_CACHE_MAX_BYTES (64 MiB) in aggregate, evicting oldest-first, and a single merge past _MERGE_CACHE_ENTRY_MAX_BYTES (16 MiB) is served but never cached — re-merging it later is cheaper than pinning that memory for one slot. A slot's stale entry is dropped before its replacement is sized. Pinned by test_merge_cache_is_bounded_by_bytes_and_skips_oversized_entries.
  • Frame-cap comment matches the code (GPT advisory, runtime.py): the _REPLAY_CAPTURE_MAX_FRAMES note now says the whole capture is discarded and the consumer renders its JSONL, which is what the branch does.
  • Backend Lint (check_comment_history.py, new on main): three comments this PR wrote said "no longer" — reworded to present tense. The other two failures the gate reports on this PR — context.py 12 vs baseline 11 and chat_runner.py 109 vs 103 — are main's own drift (the gate scans only touched files, so main's pushes never saw it; measured on pristine fd6684b40 with the gate's own scanner). Fixed in chore(comments): drop the history narration main outgrew its baseline on #9364, not here; this PR rebases once that merges.
  • Round-17 reds not owned here: shard 3 test_security_conductor_skill_contract (Linux + Windows) fails on main's own run 2f9ed9724; Windows shard 4 test_work_ledger is the known flake.

Review round 19 — what changed

  • No cursor arithmetic across corpora (GPT F1, chatSlice.ts): a refresh whose page was cut from a different corpus than the view's cursor (cursor_space on both sides, absent = JSONL — the provider came up between two fetches, or went away) now refetches unbounded instead of keeping a head and subtracting its row count from the new cursor; switchSlot applies the same rule to its cached transcript (corpus read from the cached transcriptSource before it is overwritten) and keeps no head across a corpus change. Both skip the check when the page reaches the start of history, since that page replaces the view whole and no arithmetic crosses anything — so a short session with the flag on pays nothing extra.

Review round 20 — what changed

  • Runtime death releases every replay reservation (GPT F1, AcpRuntime._mark_dead): a dead runtime cannot serve a replay, and a session never destroyed after the death would have kept its bytes reserved in the process-wide ledger for the gateway's lifetime, starving later resumes. _mark_dead now releases every sid the runtime knows — registered sessions and captures still in flight — and clears the capture maps. Pinned by test_runtime_death_releases_every_session_reservation.
  • Stale-cursor recovery is not silent (GPT F2, loadOlderMessages): the refreshSlot refetch is awaited; if it rejects, the load rejects too (rejectWithValue({ slot })), so the red retry bar appears instead of "load earlier" becoming a dead click.
  • Note on the comment-history lines: the head this round builds on (1162ca6ea, pushed by the co-owner) also carries the seven present-tense rewrites in chat_runner.py / context.py and the baseline entry from chore(comments): drop the history narration main outgrew its baseline on #9364, so Backend Lint clears here without waiting. They are byte-identical to chore(comments): drop the history narration main outgrew its baseline on #9364; whichever merges first, the other collapses to no diff on rebase.
  • Round-18 reds not owned here: shard 3 test_security_conductor_skill_contract — fixed on main by fix(test): security-conductor scripts guard checks for stubs, not absence #9362, which the merge ref of this push picks up.

Review round 21 — what changed

  • _mark_dead on a half-constructed runtime (shard 1, own): test_acp_runtime_kill builds an AcpRuntime with __new__ and only the attributes kill() reads, so the round-20 release loop failed on _replay_capture being absent. The death path now reads the capture maps with getattr and clears whichever exist — death can genuinely reach a runtime whose spawn failed before construction finished.

Review round 22 -- what changed

  • Death releases in-flight captures only (GPT F1, AcpRuntime._mark_dead): frames still in the runtime's capture buckets die with the process, so their ledger reservation is returned; frames already handed to a session handle keep serving the dashboard after the death, so their bytes stay counted. They return when the handle drops them: discard_replay (transcript rewrite), unregister_session (subagent destroy), or -- new -- AcpSessionProvider.shutdown(), which calls handle.discard_replay() before runtime.kill(), closing the owned-teardown leak without under-counting memory a handle still holds. Pinned by test_runtime_death_releases_in_flight_captures_only and test_shutdown_discards_the_handles_replay_before_killing.
  • Merge cache is locked (GPT F2, chat_replay._MERGE_CACHE_LOCK): every read, eviction, invalidation and write of _MERGE_CACHE runs under one threading.Lock; the merge itself stays unlocked so slots do not serialize on one CPU. The cache is reached off-loop (asyncio.to_thread), so two slots' detail fetches could previously interleave an unsynchronized pop(next(iter(...))).
  • Round-20 red not owned here: Frontend Tests (2) DrivePage.test.tsx share-dialog focus-return (aws-control, untouched by this PR; passes on main's own run at the same base) -- rerun.

Review round 23 — rebase onto main after #9364

  • Rebased onto main c02cdd67c. chore(comments): drop the history narration main outgrew its baseline on #9364 is merged, so the comment-history rewrites and baseline entry this branch carried collapse to no diff; context.py / chat_runner.py are back to this PR's own lines only (the USER_REQUEST_HEADER constant and the /clear discard). Conflicts: comment-history-baseline.json (took main's), and chat_rewind.py, where main moved the rewind's live rewrite into a _commit_live_state that carries rows that arrived during the boundaries — the replay discard now sits inside that commit, before slot.messages is reassigned, the same placement edit-and-resend already has.
  • Round-21 red not owned here: Backend Lint was cancelled by the superseding push, not failed.

Known limits of the prototype

  • The replay is only available once the gateway has resumed the session (first prompt after a restart creates the provider). A tab opened before that still renders JSONL — the flip to acp_replay happens on the next detail fetch.
  • On v1/v2 the sidecar is anchored by order (tool_call_id, prompt containment), not by a message identity; the v3 engine stamps _meta.kiro.messageId, which would replace the containment match.
  • Rows are tagged meta.source but the per-row UI badge is not in this PR; only the banner is.

Tests

  • test/test_acp_runtime_replay_capture.py — capture helpers, cap/overflow, reader-loop routing (armed sid kept, others dropped, capture-off unchanged), load_session attaches frames and disarms on failure.
  • test/test_chat_replay_merge.py — prompt stripping, turn folding, tool enrichment, sidecar anchoring, dash-folded nudge matching, JSONL turns missing from the replay, steer/compaction handling, redaction.

Static gates run locally: black, flake8, mypy --python-version 3.12 on the touched Python; tsc -b, eslint, npm run i18n:check, gen:settings, generate_config_baseline.py. Test execution is left to CI.

@CrysisDeu
CrysisDeu requested a review from a team September 6, 2026 02:16
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 6, 2026 02:16
@CrysisDeu
CrysisDeu requested a review from patrigao September 6, 2026 02:16
@github-actions github-actions Bot added readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 833fa5b to 71a2506 Compare September 6, 2026 02:26
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

I have what I need for the design assessment.

Design-Verdict: CONCERNS

Sound, measured, flag-off prototype — but its load-bearing prompt-text matcher stays permanent surface unless identity anchors the default engine doesn't emit ever land.

Watch

The merge's correctness rests on string-level knowledge of every wrapper the prompt writer produces (containment + dash-folding + USER_REQUEST_HEADER + quick_prompt_header() + _TURN_STARTING_INJECT_KINDS): a future wrapper added in context.py or the queue drain silently unmatches turns and renders them twice, and no gate couples the writer's spellings to the matcher. P1's messageId fix is gated on the v3 engine, while RFC open question 2 (getting v2 — what Kiro Crew runs — to stamp identity) is unresolved, so the heuristic set is the plan of record indefinitely.
Clears when: RFC §11 Q2 is answered (v2 stamps _meta.kiro.messageId and P1 retires containment), or a test/registry forces every new prompt wrapper to register with chat_replay's matcher.

Suggestions

  • Serving replay-backed transcripts unpaginated (the merged corpus is already fully materialized and ≤16 MiB cached) would delete the cursor_space/corpus=/409-cursor_stale protocol — the single largest piece of client/server surface this prototype adds.
  • Host the PR-body screenshots as GitHub attachments instead of committing 530 KB of PNGs under temp-screenshots/ — deleting the directory later won't remove them from history.

[DESIGN-REVIEWED] 7492782

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

The toggle and banner both read cleanly cold; the banner's two-source counts and clickable-looking tag leave the reader unsure what to trust or touch.

Watch

  • The banner detail (transcript_source_replay_detail: "16 entries come from the agent's own record … and 12 from the Kiro Crew log") made the blind reader ask which record "I should trust when they disagree" — per-source row counts surface an internal split with no action attached. Rare surface (resumed sessions, prototype flag) but persistent once shown. Lead with the user benefit ("includes the agent's thinking and tool detail the log leaves out") and demote the counts to a tooltip.
  • The purple "Rebuilt from the agent's own record" chip (bg-accent/15 text-accent) matches the styling of interactive chips in the same view (file pill, "Thought process"); the reader said "I'd try clicking the tag" — a dead click. Mute it to plain text or give it the detail on click.
  • A stale replay cursor turns "Load earlier" into one silent no-op: isStaleCursorRejectionrefreshSlot re-reads the first page and returns null, so the click yields no older rows and no message. Rare (rewind/edit between pages, flag on), but a dead click at the exact moment the user asked for history — chain the older-page fetch after the refresh or surface a one-line "history changed" status.

[UX-REVIEWED] 7492782

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 74927825dfdcf25962193208a2b57627129bdb1f — 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 complete. Final review:

First-Principles-Verdict: CONCERNS

meta.replay_title is written "for audit" but nothing reads it, and a flag-off resume-failure behavior change ships undeclared.

Not justified as shipped

  • 9. Resume now terminates the session when the served-default check fails (flag-off installs too) — undeclared: not in the PR description's visible text; it changes default-install session/load failure behavior. It is a real fix (the caller falls back to a fresh session at providers/acp.py:785-793, and pre-PR the resumed sid stayed resident, refusing later loads), but it should be named as such.
  • 10. meta.replay_title on enriched tool rows — zero consumers: grepped replay_title → defining site (chat_replay.py:362-366) plus one test assertion; no frontend or backend reader. Speculative surface shipped to clients on every replayed tool row.

What this change ships

Inventory (10 items) — 8 justified

Intent: let a resumed session show the agent's own record (thinking, tool detail) instead of the lossy JSONL reconstruction — an ADDITION (prototype flag, default off). The change has more than 10 items; these are the 10 most noticeable (docs/RFC/spec rows, screenshots, and two small helper exports are workflow- or invariant-mandated riders).

  1. New config key + Settings → Chat toggle dashboard.replay_from_acp, default off — justified
  2. Flag on: a resumed transcript is rebuilt from the agent's replay, showing thinking and tool detail the log omits — justified
  3. "Rebuilt from the agent's own record" banner with per-source row counts — justified
  4. Replay-path responses gain transcript_source / replay_report and meta.source per row (absent on default installs) — justified
  5. Pagination carries its corpus (cursor_space / corpus=) and refuses a stale replay cursor with 409 — justified
  6. Regenerate / variant / edit-resend / rewind / /clear now also discard the retained replay (no unfixed rewrite siblings found: channel_slots and chat_runner:3451 touch only channel windows and transient chunks) — justified
  7. Capture memory ceilings: frame cap, 64 MiB per session, 256 MiB process-wide ledger — justified
  8. replay_updates / discard_replay declared on LLMProvider with safe defaults — justified (H14)
  9. Served-default failure on resume terminates the resumed session, flag-off too — undeclared (real leak fix; say so)
  10. meta.replay_title kept "for audit" on enriched tool rows — zero consumers

Watch

  • The undeclared flag-off change (item 9): the premise ("the caller then falls back to create_session()") is confirmed by reading providers/acp.py:785-793, so the risk is disclosure, not correctness. Clears when: the PR description names the flag-off behavior change on the resume-failure path.

Subtractions

  • Drop rmeta["replay_title"] = rows[idx]["content"] in chat_replay.py:362-366 (and its test assertion) — 0 consumers; re-add it the day something displays it.

[FIRST-PRINCIPLES-REVIEWED] 7492782

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

I've verified the single candidate against the actual caller structure.

Candidate 1 (ledger undercount on re-arm) requires load_session() to be invoked for a resume_sid on a runtime that already holds a live handle for that same sid. Tracing the callers: _start_kiro_runtime_impl (providers/acp.py:823) spawns a fresh AcpRuntime per provider start and resumes each resume_sid exactly once; _load_session_with_retry (providers/acp.py:742) loops sequentially and only re-issues load_session after a failed attempt (which releases in the finally), returning immediately on the first success. No path calls load_session for a sid whose prior handle is still registered on the same owner. The concurrent-resume case the ledger explicitly handles is two runtimes (distinct owners) — precisely why keying includes the owner. Condition (a) — a concrete input that occurs in practice — cannot be re-derived; it rests on a caller pattern that does not exist. Dropped.

No findings.

[OPUS-REVIEWED] 7492782

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

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

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging 74927825dfdcf25962193208a2b57627129bdb1f. 1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

This comment is updated in place on each push.

BLOCKING -- website/src/store/chatSlice.ts:2785 -- Complete replay views lose corpus identity
const corpusChanged = (page.cursorSpace ?? 'jsonl') !== (after.slotCursorSpace ?? 'jsonl')
Complete replay view -> cursor space cleared -> rotation returns a partial JSONL page -> replay and JSONL rows/cursors are incorrectly stitched, losing or mixing history.
Anchor: residual/crash-data-loss-corruption
Fix: When slotCursorSpace is absent, derive the current corpus from slotTranscriptSource[key].source.

[BLOCK-MERGE] 7492782
[GPT-REVIEWED] 7492782

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I have enough to rule. Let me record my analysis.

The adjudicable block is empty (0 findings). One fenced finding, F1.

F1 analysis (chatSlice.ts:2785): The refresh thunk (2681–2796) replaces the transcript with a count-matched page and gates corpus-mismatch on corpusChanged (2785). setPagingCursor clears slotCursorSpace to undefined whenever hasMore is false (1535), so a complete replay view carries no cursor-space. The check then reads undefined ?? 'jsonl', masking a genuinely-replay view as JSONL. If a size rotation later makes a refresh page come back hasMore=true in the JSONL corpus (2788–2793 confirms rotation still returns hasMore with a cursor in the other corpus), corpusChanged evaluates false and the JSONL page is stitched onto the replay view via spansView/overlapsView (2796) — silent history mix/loss, the residual/crash-data-loss-corruption class (unbounded harm).

For a FLAG I need a complete record: every condition at file:line, a named recovery path, and a rarity argument extreme enough that a human would accept the residual risk. Conditions confirmed: cursor-space cleared on complete view (1535), corpus check masks absent space (2785), rotation returns hasMore in other corpus (2788–2793). But (b) I found no next-run self-correction — once the mixed page is stitched it is published and cached, with no path back; and (c) the rarity is not extreme: the flag exists precisely to be enabled and rotation is an ordinary backend event, not a contradictory or writer-impossible timing. The record is incomplete, and the class is the one where a wrong clearance is unrecoverable.

Verdict: UPHOLD-FENCED (when torn, UPHOLD-FENCED).

[ADJUDICATION] 74927825dfdcf25962193208a2b57627129bdb1f total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 74927825dfdcf25962193208a2b57627129bdb1f
[ADJUDICATION-FENCED] 74927825dfdcf25962193208a2b57627129bdb1f fenced=1 flagged=0
UPHOLD-FENCED F1 website/src/store/chatSlice.ts:2785 -- Complete replay views clear slotCursorSpace to undefined (1535), so the ?? 'jsonl' fallback masks a replay view as JSONL and a post-rotation JSONL page (2788-2793) stitches into it silently; no recovery path found and the enabling conditions (flag-on plus ordinary rotation) are not extreme enough to accept the residual unbounded-loss risk.
[GPT-ADJUDICATED-FENCED] 74927825dfdcf25962193208a2b57627129bdb1f

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

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 71a2506 to e111004 Compare September 6, 2026 02:36
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from e111004 to d5710f9 Compare September 6, 2026 03:09
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from d5710f9 to 1d94087 Compare September 6, 2026 03:35
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 1d94087 to 5f22961 Compare September 6, 2026 03:48
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 5135ec6 to 33b44ac Compare September 6, 2026 08:09
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 33b44ac to 09edc92 Compare September 6, 2026 08:26
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 09edc92 to 6354ab6 Compare September 6, 2026 08:28
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

DOC ADDED docs/request-for-change/rfc-acp-replay-as-transcript-source.md @6354ab687

Design rationale RFC for this flag: problem (dual-written transcript, replay counted-and-dropped), the seen-by-the-agent vs. not dividing line, phasing P0 (this PR) → P1 (_meta.kiro.messageId anchor on v3) → P2 (stop writing the body) → P3 (OAuth to live state), alternatives, risks, and the fidelity tables (395/395 tool, 173/177 text, 113/115 prompt). Also registered in docs/request-for-change/README.md; scripts/docs-lint.sh passes. Docs only — no code changed.

Note for the code owner: this was amended onto 09edc922f as a single commit, so the branch head moved to 6354ab687. Fetch and fast-forward the owning worktree onto that head before the next amend, or the lease push will be refused.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

@CrysisDeu — real-device pass of dashboard.replay_from_acp at 5135ec62a on a pod (kiro-cli 2.21.0, KiroCrew 0.7.0 built from this branch), same JSONL corpus rendered with the flag off/on, on the kiro (v2) backend and on acp_backend=kas (v3). Rich session: read + shell tools, a steer during a sleep 12 with interactive approval, /compact, two policy refusals, one spawn_run sub-agent, then a gateway restart + one prompt to trigger session/load. The happy path holds — banner shows, user bubbles are clean, tool titles/purpose are borrowed correctly from the JSONL twin on v2 (meta.replay_title keeps the bare read / learn_add / spawn_run), steer label + compaction notice land in the right place. Three rendering bugs, all reproducible:

  1. Runner-authored prompts are replayed as user turns and duplicate their answers. _PROMPT_ROLES = {user, nudge} excludes inject and steer rows, so the replay turns whose prompt was [Subagent completion event]… / [SYSTEM] Sub-agent synthesis… (v2) or the steer delivered as its own prompt <user_message>…</user_message> (kas) never find a JSONL group in _match_prompt; _overlay_turn takes the prompt_row is None branch and renders the cleaned wire prompt as a grey user bubble (the synthesis one exposes the whole system-prompt text), while the JSONL assistant/inject/tool rows of those turns were already emitted as sidecar of the previous group — so "returned PONG" + Goal/Findings (v2) and the sleep 12 card + answer (kas) each appear twice. Suggest admitting inject (meta.injectKind) and steer rows as prompt-group heads for matching (containment on the [SYSTEM]… / <user_message> wrapper) and keeping their role when they win.
  2. Stray sticky chip over the banner (v2 on): the [Subagent completion event] user row from (1) becomes the pinned-prompt header — "Reply with the single word: PONG ⌄" floats over the replay banner at scroll top.
  3. KAS-internal tool calls leak (v3 on): two 🔧 fetch_cloud_config tool rows appear after the last replayed answer; they were never shown live. Needs a filter (name/kind allowlist, or only keep tool rows with a JSONL twin).

Minor: the approval twin row(s) are absorbed into the replay tool row (harmless today; kas writes two ✅ rows and both vanish), and replayed tool rows carry cls: msg msg-tool where the JSONL history has msg msg-a.

Unrelated but seen on the pod: with acp_backend=kas, spawn_run fails with "gateway isn't reachable (127.0.0.1:5476)" — the MCP config points at the default port instead of the pod's; and a manual /compact on kiro produced a summary claiming "no prior user messages" for an 8-turn session.

Evidence (screens + rows.json per state + 25s GIF) is in ~/.kiro/crew/workspace/mockups/acp-replay-demo/ on the dev desk (README has the full diff table).

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/acp-replay-render-flag branch from 6354ab6 to c4362a7 Compare September 6, 2026 09:26

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tech Lead review: not approving.

The dashboard.replay_from_acp flag verifies genuinely real and default-false (sections.py, loader.py, config-baseline.json all agree), and I found no cross-session transcript exposure in the capture/pop lifecycle. This is not a security hold.

Holding because of your own real-device testing comment (2026-09-06T08:55Z) reporting three reproducible rendering bugs, of which two are still unfixed at the current head:

  • Bug #3: engine-internal fetch_cloud_config tool rows still surface after the last replayed answer on the kas/v3 backend — the only filter is pre-first-prompt, and test_tool_result_without_call_is_kept_visible deliberately pins orphan tool rows as visible, so this needs a real fix, not just a test update.
  • The kas steer-as-its-own-prompt half of bug #1: _is_prompt_row still returns False for steer rows.

Please close bug #3 and the steer half of bug #1 before this lands, even behind the flag — a self-reported user-visible leak of internal tool calls into a transcript shouldn't ship even opt-in.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Do not merge yet

@bolichen97

Copy link
Copy Markdown
Collaborator

@CrysisDeu Thanks for keeping this one alive. I re-audited it today (audited at 5043b35; head has since moved to c8da531, so a few file-level notes below may already be stale).

Good news first: none of this feature has landed on main. I grepped origin/main for every new symbol (replay_from_acp, replay_updates, _capture_replay_frame, merge_replay_transcript, record_metrics) and got zero hits, src/kiro_crew/dashboard/chat_replay.py does not exist there, and src/kiro_crew/context.py still spells the user-request header inline. Nothing needs to be cut for coverage reasons, and the seams you extend are all still present.

What still blocks it is the scope, not the goal:

  1. The two rendering bugs you reported yourself on 2026-09-06 are still open and are what the CHANGES_REQUESTED sits on: KAS-internal fetch_cloud_config tool rows leaking after the last replayed answer (the only filter is pre-first-prompt), and steer rows excluded from _is_prompt_row, so a steer-as-own-prompt turn renders twice.
  2. The cap comment in src/kiro_crew/acp/runtime.py and docs/system-specs/modules/acp-client.md still say frames past the ceiling take the counted drop and the consumer sees a truncated transcript, but the code discards the whole capture.
  3. Please drop the three committed PNGs under temp-screenshots/acp-replay/ and rebase; the branch was 172 commits behind.
  4. Two open PRs touch the same lines and need an order agreed before either lands: feat: render the KAS session recap as a transcript notice #9314 inserts a KAS recap branch into the same demux chain (if your capture branch runs first it swallows the recap frame), and perf(chat): bound paginated transcript reads #8235 replaces the whole-corpus pagination arm your merge depends on with a bounded page reader. feat: opt-in ACP frame recorder (owner-only perms) #9217 also retains ACP frames in the same file, textual conflict only.

Narrowing to the flagged read path plus fixing 1 and 2 would make this reviewable.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
span=60eee5df9fed

self-added: yes
mechanism: _ReplayRetentionLedger reservations keyed by (runtime owner token, sid); _next_replay_owner() monotonic counter per AcpRuntime

  • Concurrent resume attempts bypass the replay memory ceiling (src/kiro_crew/acp/runtime.py:328) — fixed in ba8b47e

The ledger key is now (owner, sid): every AcpRuntime takes a monotonic owner token at construction and passes it on each reserve / release, so a runtime can only pop bytes it reserved itself. A losing concurrent resume's re-arm, capture discard, failed load, unregister_session and _mark_dead all leave the winner's reservation counted, and the process-wide budget never undercounts what is held.
A monotonic counter rather than id(self): a dead runtime's handed-off frames stay reserved after the object is collected, so a recycled id would reopen the same hole.
Pinned by test_concurrent_same_sid_resumes_release_only_their_own_bytes (two runtimes, same sid; loser's re-arm / take / unregister / death leave the winner's total intact). Rulings on same-sid cross-runtime ledger interference are covered by this record wherever the site moves.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
span=4c260ac5c25a

self-added: yes
mechanism: clean_user_text searches the ORIGINAL prompt for both USER_REQUEST_HEADER spellings and slices by the matched spelling's own width

  • Length-changing folding truncates resumed user prompts (src/kiro_crew/dashboard/chat_replay.py:121) — fixed in 2dcf526

clean_user_text no longer derives an index from a folded copy. It runs rfind on the original prompt for each marker spelling (em dash as typed, -- as build_message folds it), keeps the rightmost hit, and slices with that spelling's len(). No index ever crosses between folded and unfolded text, so _MULTIBYTE_TABLE entries that change character counts (, , arrows) ahead of the marker cannot shift the cut.
Pinned by test_clean_user_text_survives_length_changing_punctuation_before_marker (both spellings, // before the marker, user text returned whole). Findings that require a folded-index-into-unfolded-text slice in this helper are covered by this ruling.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
span=60eee5df9fed

self-added: yes
mechanism: load_session wraps ensure_served_default() in terminate-then-raise, mirroring the existing set_mode failure path

  • Post-load failure leaks the replay reservation (src/kiro_crew/acp/runtime.py:3554) — fixed in 72a1bf6

The served-default check main added after session/load (ensure_served_default, landed on main during this PR's rebase) can raise on the wire; the caller then falls back to create_session(). load_session now terminates resume_sid before re-raising, exactly as the adjacent set_mode failure path does: terminate_session unregisters the queue and unregister_session returns the replay frames' process-budget bytes, so a repeated fallback never accumulates reservations.
Every post-session/load failure point in load_session now cleans up the resumed sid before propagating (served-default and set_mode); findings that require a leaked reservation on a post-load raise in this function are covered by this ruling.
Pinned by test_load_session_served_default_failure_releases_resumed_sid (served-default raises after frames are captured; terminate_session called for the sid, queue unregistered, ledger total back to 0).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
span=52a4927c3548

self-added: yes
mechanism: refreshSlot corpus-change retry declines (returns null, view and cursor kept) when the unbounded read still reports hasMore

  • Corpus-change retry can discard loaded scrollback (website/src/store/chatSlice.ts:2786) — fixed in 0a67e7a

The cross-corpus retry is only a whole-history read while nothing is rotated; after a size rotation the handler still answers has_more with a cursor in the other corpus, and a replacing reducer cannot stitch that page to the rows the view holds. refreshSlot now awaits the retry and returns null when it is still hasMore, so the current transcript and cursor stay exactly as they were and the next refresh retries; a retry that reaches the start of history is accepted whole as before.
Any page the replacing reducer receives across a corpus change now reaches the start of history, so no arithmetic crosses corpora; findings that require a partial cross-corpus page being handed to the reducer are covered by this ruling.
Pinned by chatSlice.refreshSlotCorpusChange.test.ts (rotated unbounded retry: view and slotCursorSpace unchanged, two fetches; unrotated retry: whole history accepted).

…lag, default off)

Prototype behind dashboard.replay_from_acp (default false). When on, the
kiro-shared runtime keeps the session/update frames kiro-cli replays during
session/load instead of counting them as drops, hands them to the session
handle, and the slot-detail handler rebuilds the transcript from them through
the same parse_session_update the live path uses, overlaying only the JSONL
rows the agent never saw (notices, approvals, steer/inject provenance,
compaction banners). JSONL is still written either way.

Measured against kiro-cli 2.21.0 (research/acp-replay): every engine speaks
ACP protocolVersion 1 and has no session/resume; replay is authoritative for
assistant text, thinking and tool id/rawInput/rawOutput/status, while v1/v2
collapse tool titles to the raw tool name and stamp no messageId, and a
session/load for a sid another process holds is refused -- so the frames are
captured at the gateway's own resume, never fetched on demand.

- acp/runtime.py: capture_replay + bounded per-load capture bucket
- acp/_dispatch.py: record_metrics opt-out so replayed calls do not emit
  near-zero duration samples
- dashboard/chat_replay.py: merge + sidecar overlay (tool_call_id / prompt
  containment anchors)
- dashboard: transcript_source + replay_report on GET /api/chat/slots/{slot},
  banner in ChatPage, Settings toggle, config flag + baseline
- tests written for the capture path and the merge (CI runs them)
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
span=52a4927c3548

self-added: yes
mechanism: warmSlotCache.fulfilled records/clears slotTranscriptSource per response, the same rule switchSlot/refreshSlot already apply

  • Replay-backed warm caches lose loaded scrollback (website/src/store/chatSlice.ts:5527) — fixed in 7492782

warmSlotCache.fulfilled now writes state.slotTranscriptSource[key] from action.payload.transcriptSource and deletes it when the response carries none -- the per-response provenance rule switchSlot and refreshSlot already follow. A background slot warmed from the replay corpus is therefore classified acp_replay when switchSlot reads the cached corpus, sameCorpus holds, and the kept head is stitched instead of dropped.
Every reducer that consumes a fetchSlotDetail payload now writes the same provenance field, so no cache can carry rows whose corpus is unknown to the stitch; findings that require a slot-detail reducer leaving slotTranscriptSource unset are covered by this ruling.
Pinned in chatSlice.refreshSlotCorpusChange.test.ts (warmSlotCache carries the transcript source: cold warm from the replay corpus records acp_replay; a later warm carrying no source clears it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants