Skip to content

refactor: unify duplicated chat-recovery/repair machinery into the shared agents/chat layer (N3) #1642

Description

@threepointone

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, the ChatRecoveryIncident shape, 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 (_storeChunkDurably in think / _storeStreamChunk in ai-chat) + the N9 _onAgentToolStreamProgress override.
  • 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 (_awaitPendingInteractionBarrier runs inside the queued continuation turn, gated by AUTO_CONTINUATION_PENDING_TOOL_TIMEOUT_MS = 60_000 with a console.warn fire-through).
  • Known intentional asymmetries to reconcile (decide: unify or document at the seam):
    • ai-chat persist gate is streamStillActive-only vs think's streamStillActive || (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).

Constraints / sequencing

Acceptance

References

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions