Fix glitchy fork/brief handoff: seeding lag, phantom duplicate session, false 30s timeout (ADE-122)#858
Conversation
…e session, no false timeout (ADE-122) Three confirmed root causes, one fix each: 1. App lag during fork handoff: appendImportedChatEvents ran synchronously on the runtime event loop and live-published every seeded historical envelope to all subscribers (IPC + sync), re-streaming the entire source chat. It is now async + chunked (event-loop yields between 250-envelope chunks), defers transcript flushes to chunk boundaries (one append syscall per file per chunk instead of a forced sync flush per seeded user message), bulk- updates the history ring once (was O(n^2)), and never live-publishes seeded envelopes — readers load them through the history/transcript APIs. 2. Transient duplicate session: the sidebar handoff placeholder card and the real created session row were both visible from createSession until the handoff RPC settled. SessionListPane now hides a placeholder as soon as a matching real session row is visible (handoffJobLikelyMaterialized: same lane + tool type, started at/after the job began). Placeholder copy is now mode-aware and honest (fork: "Forking chat history...", brief: "Summarizing chat & creating handoff...") instead of fake timed stage hops, the placeholder re-homes to the brief's target lane, and handoffSession guards re-entry while busy. 3. False "3000ms" timeout on brief handoff that later silently succeeded: the chat.handoffSession daemon action fell into the 30s default RPC timeout ("timed out ... (30000ms)" read as 3000 seconds) while the daemon-side handoff (45s-bounded AI brief + session creation + first-message dispatch) kept running to a late "surprise" success. handoffSession and prepareCrossMachineHandoff now carry 120s daemon action timeouts and 150s IPC timeouts on the direct, local-runtime, and remote-runtime paths, and the renderer maps a transport timeout to honest copy ("still finishing in the background") plus delayed session-list refreshes so a late success reconciles as the expected new chat. Regression tests per defect + docs updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…placeholder statuses Dual-review synthesis for the ADE-122 fix: - The transport-timeout detector matched any "timed out after Nms", which would relabel genuine daemon-side failures (e.g. Codex request timeouts surfacing through first-message dispatch) as "still finishing in the background". It now matches only the two transport wrapper shapes (registerIpc invoke timeout, RuntimeRpcClient request timeout). - Removed the now-producerless "creating-chat"/"sending-handoff" placeholder statuses (the fake stage timers were their only writer; the store is in-memory, so no persisted value can carry them). - Added a SessionListPane regression test proving the placeholder hides once the matching real session row is visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…abels Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot review but do not make fixes |
1 participant
Fixes ADE-122 — fork/brief chat handoff was glitchy: the app lagged out during a fork, a duplicate session appeared and then vanished, and a brief handoff reported a timeout and then silently succeeded ~a minute later.
Root causes and fixes
1. App lag during fork handoff —
appendImportedChatEventsseeded the entire source transcript synchronously on the runtime event loop, live-publishing every historical envelope to all IPC/sync subscribers and forcing a sync file flush per seeded user message (×2 transcript files). Now async + chunked (250-envelope chunks withsetImmediateyields), one append per file per chunk, one bulk history-ring update (was O(n²)), and seeded envelopes are no longer live-published — readers load them via the history/transcript APIs (renderer, iOS, and search all back-fill; verified).2. Duplicate session that vanishes — the sidebar handoff placeholder card and the real created session row were both visible from
createSessionuntil the handoff RPC settled; the "vanish" was the placeholder being removed.SessionListPanenow hides a placeholder as soon as a matching real session row is visible (handoffJobLikelyMaterialized: same lane + tool type, started at/after the job began). Placeholder copy is mode-aware and honest (fork: "Forking chat history...", brief: "Summarizing chat & creating handoff...") instead of fake timed stage hops; the placeholder re-homes to the brief's target lane; the handoff callback guards re-entry.3. False "3000" timeout on brief handoff —
chat.handoffSessionfell into the 30s default daemon-RPC timeout ("timed out ... (30000ms)", misread as 3000 seconds) while the daemon-side handoff (45s-bounded AI brief + session creation + first-message dispatch) kept running to a late "surprise" success. Timeout never cancelled the operation. NowhandoffSession/prepareCrossMachineHandoffcarry 120s daemon action timeouts and 150s IPC timeouts on the direct, local-runtime, and remote-runtime paths, and a genuine transport timeout surfaces as honest "still finishing in the background" copy plus delayed session-list refreshes so a late success reconciles as the expected new chat. The detector matches only the two transport wrapper shapes so real daemon-side failures still surface as errors.Tests
Regression test per defect: seeded-history-not-live-published (agentChatService), 120s daemon action timeout (localRuntimeConnectionPool), 150s IPC timeouts across all three paths (ipcTimeouts), placeholder materialization dedupe (handoffLaunchJobs — new colocated suite), placeholder-hides-on-real-row (SessionListPane).
Full
agentChatServicesuite 672/674 — the 2 failures are pre-existing cron tests that fail identically without this branch. Docs updated (chat/README.md,composer-and-ui.md). Parity verified: no iOS/CLI/TUI changes required (no shared types touched).🤖 Generated with Claude Code
Greptile Summary
This PR fixes the glitchy chat handoff flow reported in ADE-122. The main changes are:
Confidence Score: 5/5
Safe to merge with low risk.
The changes are focused on the handoff path and include tests for the lag, duplicate placeholder, and timeout behaviors.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Renderer as AgentChatPane participant IPC as IPC/Runtime timeout layer participant Chat as agentChatService participant Store as Transcript/history storage participant List as SessionListPane Renderer->>Renderer: Create handoff launch job placeholder Renderer->>IPC: handoffSession / prepareCrossMachineHandoff IPC->>IPC: Apply extended handoff timeout IPC->>Chat: Run handoff action Chat->>Store: Seed imported/forked envelopes in chunks Store-->>Chat: Flush transcript chunks and bulk history update Note over Chat,Renderer: Seeded historical envelopes are not live-published Chat-->>Renderer: Return materialized session Renderer->>List: Refresh sessions List->>List: Hide placeholder when matching real session is visible%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Renderer as AgentChatPane participant IPC as IPC/Runtime timeout layer participant Chat as agentChatService participant Store as Transcript/history storage participant List as SessionListPane Renderer->>Renderer: Create handoff launch job placeholder Renderer->>IPC: handoffSession / prepareCrossMachineHandoff IPC->>IPC: Apply extended handoff timeout IPC->>Chat: Run handoff action Chat->>Store: Seed imported/forked envelopes in chunks Store-->>Chat: Flush transcript chunks and bulk history update Note over Chat,Renderer: Seeded historical envelopes are not live-published Chat-->>Renderer: Return materialized session Renderer->>List: Refresh sessions List->>List: Hide placeholder when matching real session is visibleReviews (1): Last reviewed commit: "docs(chat): update composer handoff plac..." | Re-trigger Greptile