Repository navigation
refactor: unify duplicated chat-recovery/repair machinery into the shared agents/chat layer (N3) #1642
Description
Problem
The chat-recovery + transcript-repair machinery is duplicated, nearly verbatim, between packages/think/src/think.ts and packages/ai-chat/src/index.ts. Every recovery fix has to be written twice, the two copies drift, and the drift is a real source of bugs and confusion.
This was painful and concrete across the recent recovery stack (#1633, #1634, #1635, #1636, #1638, #1640, #1641): each touched think and ai-chat in lockstep, and the divergence caused issues — e.g. ai-chat's persist gate is streamStillActive-only while think uses _shouldPersistOrphanedPartial (an intentional asymmetry, but one that has to be re-reasoned every time), and ai-chat has no transcript-repair pass at all while think does.
The drift is now actively growing. #1650 made Think's parallel-tool auto-continuation barrier event-driven (#1667, #1671), while ai-chat deliberately stayed on the bounded-wait barrier from #1649/#1651 — so the two now use different mechanisms for the same job. This is the latest concrete example of the cost of duplication and is explicitly deferred here (see the new asymmetry below).
Goal
Hoist the shared recovery/repair machinery into packages/agents/src/chat/ so it is written once and both Think and AIChatAgent consume it. Pure refactor — no behavior change. Design home: design/chat-shared-layer.md.
Duplicated surface to unify (audit, not exhaustive)
- Budget / incident:
_beginChatRecoveryIncident,_handleInternalFiberRecovery,_exhaustChatRecovery,_sweepStaleChatRecoveryIncidents, theChatRecoveryIncidentshape, and the constants (CHAT_RECOVERY_NO_PROGRESS_WINDOW_MS,CHAT_RECOVERY_ALARM_DEBOUNCE_MS,CHAT_RECOVERY_MAX_WINDOW_MS,DEFAULT_CHAT_RECOVERY_MAX_ATTEMPTS,DEFAULT_CHAT_RECOVERY_STABLE_TIMEOUT_MS,CHAT_RECOVERY_PROGRESS_KEY,AGENT_TOOL_STREAM_PROGRESS_BUMP_THROTTLE_MS). - Progress signal:
_chatRecoveryProgressMarker,_bumpChatRecoveryProgress, and the production-time bump sites (_storeChunkDurablyin think /_storeStreamChunkin ai-chat) + the N9_onAgentToolStreamProgressoverride. - Orphan persistence:
_persistOrphanedStream,_shouldPersistOrphanedPartial,_partialHasSettledToolResults,_hasPersistedRecoveredAssistant. - Parallel-tool auto-continuation barrier (NEW — diverged in Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650): the batch-completeness gate (
_hasIncompleteToolBatch, shared signature in both files), the per-result serialization tail (_pendingInteractionPromise/ interaction-apply chain), and the continuation scheduling/coalescing. Think is now event-driven (_fireAutoContinuationWhenStable,_drainInteractionApplies,_scheduleAutoContinuation/_rearmPendingAutoContinuationForBatch, stream-active gate via_streamingAssistant+_onStreamingTurnFinalized, no timer). ai-chat is still bounded-wait (_awaitPendingInteractionBarrierruns inside the queued continuation turn, gated byAUTO_CONTINUATION_PENDING_TOOL_TIMEOUT_MS = 60_000with aconsole.warnfire-through). - Known intentional asymmetries to reconcile (decide: unify or document at the seam):
- ai-chat persist gate is
streamStillActive-only vs think'sstreamStillActive || (streamIsTerminal && !alreadyPersisted). - Transcript repair is think-only (
_repairTranscriptForProvider,_repairToolTranscriptParts,repairInterruptedToolPart,_toolPartHasSettledResult,_normalizeToolInput) — ai-chat has none. Decide whether the shared layer offers it to both or stays think-only by design. - Auto-continuation barrier mechanism (NEW): think event-driven (no timeout, return-and-wait, self-healing across eviction) vs ai-chat bounded-wait (60s timeout + fire-through). The blocker for making ai-chat event-driven is structural: its barrier runs inside the queued continuation turn and can't return-and-wait without occupying the chat-turn queue. Unifying here is the natural place to move ai-chat's batch gate before the turn is queued and drop
AUTO_CONTINUATION_PENDING_TOOL_TIMEOUT_MS, giving both surfaces the same event-driven semantics. Until then ai-chat keeps the two Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650 rough edges (human-in-the-loop fire-through, 60s orphan hold).
- ai-chat persist gate is
Constraints / sequencing
- Do this AFTER the recovery PR stack landed (it has — fix(think,ai-chat): make chat-recovery progress signal compaction-immune (#1628) #1633/fix(think,ai-chat): preserve settled work when a recovery turn is given up (#1631) #1634/feat(think): add repairInterruptedToolPart hook for client-resolved tools (#1631) #1635/feat(think,ai-chat,agents): expose incident identity + enrich onExhausted payload (#1631) #1636/fix(think,ai-chat): wall-clock-keyed-to-progress recovery budget + alarm debounce (#1637) #1638/fix(agents,think,ai-chat): re-attach to still-running sub-agent runs on parent recovery (#1630) #1640/fix(agents,think,ai-chat): count a sub-agent's progress as the orchestrating parent's recovery progress (N9) #1641 are all on
main), to avoid massive rebase conflicts. The three follow-on items the team chose to land first are now also onmain— discussion: route a stream-stall watchdog abort into bounded recovery instead of a terminal error #1626 (watchdog→recovery), Surface a live "recovering…" status to chat clients during durable recovery #1620 (recovering-status), and the durable telemetry work — plus the _scheduleAutoContinuation 50ms timer fires before all parallel client-tool results arrive → MissingToolResultsError #1649/Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650 auto-continuation barrier stack (fix(think,ai-chat): wait for all parallel client-tool results before auto-continuing (#1649) #1651, fix(think): serialize parallel client-tool result/approval applies (#1649 follow-up) #1657, fix(agents,think): recover server-tool turns, terminalize thrown recovery, re-run alarms killed by a script-upgrade supersede #1659, fix(agents,think,ai-chat): enforce object-shaped tool_use.input to stop silent session wedge #1661, fix(agents,think): stabilize chat SDK messenger tests #1665, fix(think): event-driven auto-continuation barrier + stream-active gate (fixes #1649) #1667, fix(think): don't re-arm auto-continuation barrier on RPC stall recovery #1671). So this issue is now unblocked: it is the pre-release refactor checkpoint, and it should fold in the auto-continuation barrier alongside recovery/repair. - Pure refactor: full think + ai-chat suites must stay green with no test changes beyond import paths; behavior identical — except the deliberate ai-chat barrier change (bounded-wait → event-driven), which is a behavior change that needs its own Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650-style tests on the ai-chat side.
- Watch the gotcha: editing
packages/agents/src/chat/*requiresnx run agents:buildbefore think/ai-chat typecheck (they compile against builtdist).
Acceptance
- Recovery/repair logic lives once in
packages/agents/src/chat/;ThinkandAIChatAgentdelegate to it. - The parallel-tool auto-continuation barrier is unified and event-driven for both surfaces;
AUTO_CONTINUATION_PENDING_TOOL_TIMEOUT_MSis removed from ai-chat (closing the residual Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650 rough edges). npm run checkgreen; full suites green; recovery/repair behavior unchanged; the only intended behavior change (ai-chat barrier) is covered by tests; no new public API.
References
- Auto-continuation parallel-tool barrier: make Think event-driven (avoid timeout fire-through for human-in-the-loop tools + orphan keepAlive) — follow-up to #1649 #1650 (event-driven Think barrier — landed; ai-chat parity deferred here), _scheduleAutoContinuation 50ms timer fires before all parallel client-tool results arrive → MissingToolResultsError #1649 (originating parallel-tool barrier), fix(think): event-driven auto-continuation barrier + stream-active gate (fixes #1649) #1667 / fix(think): don't re-arm auto-continuation barrier on RPC stall recovery #1671 (Think barrier PRs), fix(think,ai-chat): wait for all parallel client-tool results before auto-continuing (#1649) #1651 / fix(think): serialize parallel client-tool result/approval applies (#1649 follow-up) #1657 (bounded-wait barrier + serialization).
Activity
fix(think): event-driven auto-continuation barrier + stream-active ga…
threepointone commentedon Jun 25, 2026
this is done