feat(server): run-mirror — engine stream routed by turn-owning session, mid-run switching isolated - #32
Merged
Conversation
Port PR #22's run-mirror semantics to the current architecture so a mid-run session switch never leaks live engine lines into another session's view or persisted record (ticket session-isolation/02). An earlier partial implementation on this branch introduced the per-(cid, engine sid) runChat buffer family in state-bus.js and routed all six stream-write sites in streamAcpPrompt through r.chatArray(). This commit completes the port after review; the buffer family and the always-buffer write routing were kept, the following gaps were fixed: - Live view: with every write landing in the buffer, the wire snapshots still shipped cs.chat only, so the owning session's own view never saw its turn stream. state-bus now exposes runChatViewChat / snapshotViewFields and every snapshot builder (three SSE push sites, /api/state, the SSE first frame, the switch response) re-attaches the buffer for the owning view and scopes the run indicator to the viewed session: a foreign turn renders as idle in the switched-to view, and the owning view keeps its running claim even after a switch's resetContext healed it (projected from the run registry, no cs mutation). - Finalize clobbers: streamAcpPrompt's finalize re-pointed cs.mcodeSessionId and wrote the engine binding + title through whichever record cs pointed at after a mid-run switch, redirecting the switched-to session onto the run's engine conversation. The turn's owning webui record id is now captured at send time and passed through runMcodeAcp; all cs mutations are gated on still viewing the owning session, and the record-side binding/title writes address the owning record by id. - Draft promotion: bindDraftToMcodeSid promoted records through cs and would rename or merge the switched-to session's record when the engine session id arrived after a switch. A pre-bind switch now uses the new bindRecordToMcodeSid helper (promoteDraftToMcodeSid targeted at a record id, no cs access). - Finalize drain: handleSend drains the buffer after the run settles — appending to cs.chat when the user still views the owning session (the ● rewrite then lands on the drained line) or writing the lines to the owning session's persisted record via the new appendChatToSession helper when they switched away; the viewed session's chat and record stay clean either way. Failed and empty turns flush the same way. - createRunChat now replaces the cid's buffer map: beginRun allows one live turn per cid, so a session/load fallback that re-keys the buffer cannot leave a stale entry behind. Tests: new routes/chat-run-mirror.check.mjs drives the real chat.js → runMcodeAcp → sessions.js → state-bus chain plus the real switch route against a fake ACP transport and pins mid-run switch isolation, view merging, record attribution, and first-turn draft promotion under a pre-bind switch. stream-cumulative-render.test.js — whose three failures were caused by the new buffer routing — now reads the routed lines through runChatLinesFor. helpers/_setup.js mirrors the three new sessions.js exports. Regenerated release/public-source.json for the new test file. Verified live on an isolated instance (server 18098 + web 18099, MCODE_WEBUI_DATA_DIR in a temp dir, fake slow multi-tool engine): session A streaming → switch to B mid-run shows no A lines and an idle indicator; switching back shows the record plus the live buffered lines with the running indicator; after completion the full turn persisted to A's record (B stayed at zero lines). Gates: pnpm typecheck (0 errors), webui suites (190 relevant tests ×3 runs green; full suite 1258 pass with only lib/mcode-acp-note.test.js hanging at teardown — reproduced on clean HEAD, environmental), pnpm test:webapp (213 pass), pnpm build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Ports PR #22's run-mirror semantics to the current architecture, closing the verified defect where switching/creating sessions mid-run leaked live streamed lines into the switched-to view and persisted the turn into the wrong draft record.
state-bus.js): per(cid, engine sid)buffers;streamAcpPromptroutes all 6 stream-write sites through an explicit chatTarget (chatArray()— buffer when the viewed session differs from the turn's owner, elsecs.chat).cs.chatwhen still viewing the owner, elseappendChatToSession(owning record); sid/binding/title writes are gated on still-viewing (no post-switch clobbering); failed/empty turns flush too.bindRecordToMcodeSid(record-targeted) — a pre-bind switch can no longer promote/merge the wrong record.Compatibility (regression-checked)
Acceptance (independent agent, REAL engine)
PASS — all 5 ticket criteria live-verified: mid-run switch shows zero leaked lines + idle indicator; switch-back mid-run shows the merged record+live buffer with the running indicator; finalize lands the complete turn in the owner's record only; multi-session cycles and 409s clean. The dev's own live self-check additionally caught and fixed 4 gaps in the inherited code (broken live-view snapshots, finalize clobbers, wrong-record promotion, missing drain). Gates: typecheck 0 · chat-run-mirror 3/3 ×3 · stream-cumulative-render adapted tests pass · test:webapp 213/213 · build ✓ · check:source ✓.
Known decisions/notes: DELETE-mid-run discards buffered content silently ("delete wins" — appendChatToSession returns false for a missing record); a second window (different cid) viewing a running session sees DB-committed lines grow but an idle indicator (cid-keyed run registry — the same-cid semantics this ticket targets are correct). Follow-up ticket 07 (transport-level result.answer accumulation) remains queued.
Full
pnpm verifydeferred to CI (includes main's post-#31 test-teardown fixes on merge).