Repository navigation
fix(chat): keep restored live turns visibly running - #1900
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Two unresolved moderate findings in use-transcript-view.ts can incorrectly classify empty, disconnected, or restored live-tail views.
No open issues 🎉
What changed in this PR
This pull request fixes restored transcript views that incorrectly present a running session as finished after rejoining canonical live output. It separates reading-range ownership from live-tail presentation while preserving scroll ownership.
Changes:
- Adds a
liveTailboundary for runtime status, elapsed time, planning, approval/question waits, and terminal outcomes. - Adds isolated Electron regression coverage for restoration, live updates, interaction states, session isolation, and completion outcomes.
- Updates source-contract tests and documents the live-tail behavior.
| File | Changes reviewed | Findings |
|---|---|---|
scripts/e2e/transcript-reading-position.tsx |
Includes the live-tail regression fixture. | None reported. |
scripts/e2e/transcript-live-tail.tsx |
Tests restored live status, waits, scrolling, session isolation, and terminal outcomes. | None reported. |
scripts/e2e-transcript-reading-position.mjs |
Runs and validates the live-tail probe. | None reported. |
docs/spec/06-delivery/04-e2e-test-plan.md |
Documents the restored live-tail E2E scenario. | None reported. |
docs/spec/04-ux/09-interaction-patterns.md |
Specifies live-tail presentation and historical-range isolation. | None reported. |
apps/desktop/test/permission-inline.test.mjs |
Updates source-contract assertions for live-tail gating. | None reported. |
apps/desktop/src/hooks/use-transcript-view.ts |
Derives whether the displayed range reaches canonical live output. | Moderate, 1 vote: Require a non-empty live range before comparing tail identities, so empty or disconnected views cannot be marked liveTail. Moderate, 1 vote: Compare non-empty tail message IDs or another canonical identity instead of object references, so restored live tails remain recognized after history reads. |
apps/desktop/src/features/chat/transcript/ChatTranscript.tsx |
Uses live-tail state for runtime status and terminal outcome presentation. | None reported. |
apps/desktop/src/components/SessionPane.tsx |
Gates planning, approval, and question surfaces on the live-tail boundary. | None reported. |
Review effort: Lite
|
Independently confirmed the root cause in the restored-reading/live-status boundary. The candidate passes 85 focused transcript/process/permission tests, Desktop typecheck, and the isolated real SessionPane/Composer Electron journey (restored live status, hidden/revealed panes, historical gaps/search and terminal states). Reversing the four production files to current main makes the same new Electron journey fail; restoring the fix passes. The tail-ID comparison also covers copied message objects, and empty restored ranges do not claim continuity. Current main is being incorporated without rewriting contributor history; proceeding to merge the equivalent tested tree. CI errors are ignored as requested. |
Problem
Fixes #1895.
Returning to a running session can restore a reading range that has already rejoined canonical live output. Treating its bookmark ownership as execution state makes the process header say Finished while Composer still offers Stop and output continues.
Change
Separate reading-position ownership from live-tail presentation. A reading range can show runtime status only when it has no search focus or unread gap and its last non-empty message ID matches the canonical live tail. Compare message IDs across history object copies; two empty reading ranges cannot prove continuity. The ordinary canonical surface still shows the initial wait before its first message arrives.
Use this boundary for process status/elapsed time, planning, approval/question waits and terminal outcomes. Rejoining live output preserves scroll ownership. Production changes touch four renderer files; execution, persistence and provider code remain outside this change.
Validation
Candidate:
d6a2c04dfa13310321a11b334f653d21ee2f454f; base main:850a2679f34b5ae79680beb029b186daa14a906b.transcriptViewMessagesprojection.node scripts/e2e-transcript-reading-position.mjs: three consecutive passes after the boundary fixes. Real SessionPane/Composer, restored live output, quiet elapsed ticking, nonzero scrolling, pane reveal, waits, search/gaps/disjoint ranges, empty canonical versus empty restored ranges, copied identities and terminal states.0c8b038cb42a8121df107fee5bbfa2801420e674: 4,410 passed, 3 skipped, 0 failed out of 4,413 tests. The final candidate merges only upstream Markdown/resource documentation; executable and test source is unchanged by that merge. The focused suite and three Electron runs above were repeated on the final candidate.git diff --checkpassed.CI certificate investigation
The first remote Electron job timed out in the existing provider client-certificate fixture before reaching the transcript steps. That fixture and its dependency graph are unchanged by this PR (1,945 bundled inputs, no changed input); it passed three local runs. No assertion or timeout was relaxed. The updated head is awaiting its new remote checks; the original failed job is not counted as passing.
All Electron fixtures use isolated profiles and no paid model provider.