Skip to content

fix(agents): progress-keyed agent-tool re-attach so a deploy can't abandon a still-running child (#1630) - #1670

Merged
threepointone merged 5 commits into
mainfrom
fix/agent-tool-reattach-1630
Jun 4, 2026
Merged

threepointone merged 5 commits into
mainfrom
fix/agent-tool-reattach-1630

Conversation

@threepointone

@threepointone threepointone commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A deploy that interrupts an in-flight runAgentTool child used to abandon the still-running child as interrupted and re-run its already-completed work. Parent recovery does re-attach to the child and tail it to its real terminal, but the old re-attach used a flat 120s wall-clock budget that was not reset by the child's forward progress — so a healthy, actively-streaming child whose recovery legitimately ran longer than the budget got sealed interrupted.

Fixes #1630. Builds on #1672 (chat-recovery work budget / shouldKeepRecovering).

The fix

Progress-keyed re-attach budget. agentToolReattachNoProgressTimeoutMs (default 120s) now bounds only time spent with no forward progress (resets on every forwarded chunk). A healthy child that keeps streaming is followed through to terminal; a genuinely silent/hung child seals interrupted after one no-progress window and can never block recovery forever.

Re-arm only on a clean stream-close while advancing. _forwardAgentToolStream returns a discriminated end reason (done | idle | aborted). The re-attach loop re-arms (opens a fresh tail) only when the child's stream closes cleanly while still advancing (a re-evicted-but-progressing child). A full no-progress window seals no-progress immediately — even if the child streamed earlier in that window — so there's no "bonus window" that misreads a stall as recoverable, and at most one pending tail reader is ever alive per re-attach.

Uncapped hard ceiling by default (aligns with #1672). agentToolReattachMaxWindowMs now defaults to Infinity (was 15min). Post-#1672 the child no longer has a wall-clock recovery ceiling, so a finite parent ceiling would tear down a healthy, still-streaming child that simply outran a fixed window under deploy churn — the exact bug #1672 removed, reintroduced at the parent layer. A hung child is already bounded by the no-progress budget; a content-runaway is bounded uniformly (live and recovery) by the child's own maxRecoveryWork / shouldKeepRecovering. Integrators can still set a finite ceiling to opt into a hard wall-clock cap (and the window-exceeded teardown it triggers).

Eager child-side terminalization (both recovery paths). @cloudflare/think and @cloudflare/ai-chat finalize a child facet's own run row as soon as its recovered turn settles — whether recovery took the continue path (_chatRecoveryContinue) or the pre-stream retry path (_chatRecoveryRetry) — so a re-attached parent collects the terminal result immediately instead of waiting out a full no-progress window after the child already finished.

API additions

  • Typed interrupted cause. RunAgentToolResult, the agentTool() AgentToolFailure envelope, the onAgentToolFinish lifecycle result, and the agent-tool-event wire event now carry a machine-readable AgentToolInterruptedReason (no-progress | window-exceeded | not-tailable | inspect-timeout | inspect-failed | recovery-deadline) plus a childStillRunning boolean. These are propagated through the chat reducer into AgentToolRunState so UIs can branch on the cause without parsing error prose. retryable stays coarse (always true for interrupted) — refine via reason / childStillRunning.
  • Configurable budgets. New public AgentStaticOptions: agentToolReattachNoProgressTimeoutMs (default 120000) and agentToolReattachMaxWindowMs (default Infinity — no implicit wall-clock cap).
  • Give-up teardown (ceiling only). A window-exceeded give-up cancels the child (childStillRunning: false) so it stops consuming a fiber/keep-alive. no-progress give-ups stay soft (childStillRunning: true) so a re-issue can still re-attach and repair the child — preserving repair-on-re-issue. In both @cloudflare/think and @cloudflare/ai-chat, cancelAgentToolRun also aborts an in-flight chat-recovery turn (not just the original in-isolate run) — Think sweeps its _submissionAbortControllers, ai-chat its request AbortRegistry — and releases live tails, so a torn-down child stops grinding instead of finishing an orphaned recovered turn.

Tests

  • Child self-finalize on BOTH recovery paths (think + ai-chat): a stranded running child-run row is finalized (completed with a recovered assistant turn, else error) by the reconcile in the _chatRecoveryContinue and _chatRecoveryRetry finally blocks — covering the pre-stream retry path explicitly.
  • Default ceiling is uncapped: asserts an Agent with no override resolves agentToolReattachMaxWindowMs to a non-finite value.
  • Reducer fields: applyAgentToolEvent propagates reason / childStillRunning into run state (soft no-progress and torn-down window-exceeded).
  • cancelAgentToolRun aborts in-flight recovery (think _submissionAbortControllers / ai-chat AbortRegistry) and seals the child aborted.
  • Scripted-adapter re-arm matrix: rearm-then-complete (clean stream-close + progress → re-arm → completed) and idle-after-progress (progress then full idle → seal no-progress after a single tail, no bonus window).
  • Public-knob resolution, window-exceeded teardown (asserts the child row ends aborted), and a not-tailable unit seam.
  • Deterministic e2e repro (reattach-budget.test.ts): a still-progressing child is collected completed (not interrupted) after a real deploy, parentRecoveries: 1.
  • Type-level test pins the public interrupted-cause surface.

Docs

  • docs/agent-tools.md "Interrupted runs and recovery" rewritten to describe re-attach + tail-to-terminal, the progress-keyed budget, the uncapped-by-default ceiling, and the soft (no-progress) vs hard (window-exceeded, torn down) give-up mapping; AgentToolFailure block gains reason / childStillRunning.

Verification

Re-verified on current main (includes #1672):

  • npm run check — green across 90 projects (sherif, exports, oxfmt, oxlint, typecheck)
  • think (full unit suite): 526 passed
  • ai-chat (full unit suite): 550 passed
  • think agent-tools: 26 passed · ai-chat agent-tools: 23 passed · agents chat reducer: 6 passed
  • e2e reattach-budget: passed

Notes for reviewers

@changeset-bot

changeset-bot Bot commented Jun 3, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b6afe93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
agents Patch
@cloudflare/think Patch
@cloudflare/ai-chat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/ai-chat/src/index.ts
@pkg-pr-new

pkg-pr-new Bot commented Jun 3, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@1670

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@1670

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@1670

hono-agents

npm i https://pkg.pr.new/hono-agents@1670

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@1670

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@1670

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@1670

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@1670

commit: b6afe93

…andon a still-running child (#1630)

A deploy that interrupted an in-flight `runAgentTool` child used to abandon
the still-running child as `interrupted` and re-run its already-completed
work. Parent recovery re-attaches to the child and tails it to its real
terminal, but the old re-attach used a flat 120s wall-clock budget that was
NOT reset by the child's forward progress — so a healthy, actively-streaming
child whose recovery legitimately ran longer than the budget got sealed
`interrupted`.

Core fix
- Re-attach budget is now progress-keyed: it bounds time spent with NO forward
  progress (resets on every forwarded chunk) and is hard-bounded by the child's
  own recovery ceiling, so a genuinely hung child still seals and can never
  block recovery forever.
- `_forwardAgentToolStream` returns a discriminated end reason
  (`done | idle | aborted`). The re-attach loop re-arms (opens a fresh tail)
  ONLY on a clean stream-close while the child is still advancing (a
  re-evicted-but-progressing child). A full no-progress window seals
  `no-progress` immediately even if the child streamed earlier in that window —
  no bonus window — which also guarantees at most one pending tail reader per
  re-attach (fixes per-cycle reader accumulation).
- think/ai-chat finalize a child facet's own run row as soon as its recovered
  turn settles, so a re-attached parent collects the terminal result
  immediately instead of waiting out a full no-progress window.

Follow-ups
- Typed interrupted cause: `RunAgentToolResult`, the `agentTool()`
  `AgentToolFailure` envelope, the `onAgentToolFinish` lifecycle result, and
  the `agent-tool-event` wire event now carry a machine-readable
  `AgentToolInterruptedReason` (`no-progress | window-exceeded | not-tailable |
  inspect-timeout | inspect-failed | recovery-deadline`) plus a
  `childStillRunning` boolean, propagated through the chat reducer into
  `AgentToolRunState` for UIs. `retryable` stays coarse (always true for
  interrupted); refine via `reason` / `childStillRunning`.
- Configurable budgets: public `AgentStaticOptions`
  `agentToolReattachNoProgressTimeoutMs` (default 120000) and
  `agentToolReattachMaxWindowMs` (default 900000).
- Give-up teardown (ceiling only): a `window-exceeded` give-up cancels the
  child (`childStillRunning: false`) so it stops consuming a fiber/keep-alive;
  `no-progress` give-ups stay soft (`childStillRunning: true`) to preserve
  repair-on-re-issue. think `cancelAgentToolRun` also aborts an in-flight
  chat-recovery turn and releases live tails.

Tests
- New behavioral/unit coverage: scripted-adapter re-arm matrix
  (rearm-then-complete, idle-after-progress), public-knob resolution,
  window-exceeded teardown, and a not-tailable unit seam (defensive branch is
  unreachable through a real RPC child, so exercised via a plain adapter).
- Deterministic e2e repro (`reattach-budget.test.ts`) verifies a still-
  progressing child is collected `completed` (not `interrupted`) after a deploy.
- Type-level test pins the public interrupted-cause surface.

Verified: agents workers (1296), ai-chat (617), think agent-tools (22),
e2e repro, and full `npm run check` across 91 projects.
…ry (#1630 review follow-ups)

Two review findings against the progress-keyed re-attach PR, both
parity gaps between the continue and retry recovery paths / between
Think and AIChatAgent.

1. Retry-path reconcile (think + ai-chat). The eager child-side
   terminalization was only wired into `_chatRecoveryContinue`'s
   `finally`. A child facet evicted *before* producing any assistant
   content recovers via the pre-stream retry path
   (`_chatRecoveryRetry` → `_retryLastUserTurn` / a fresh user turn),
   which — like `continueLastTurn` — never flows through
   `startAgentToolRun`'s finalizer. Without the reconcile there, the
   child's run row strands `running` with its tailers open, forcing the
   re-attached parent to wait out a full no-progress window before
   collecting an already-settled result — the exact delay the PR set out
   to remove. Mirrored the `_reconcileOwnStaleAgentToolChildRuns()` call
   into both retry `finally` blocks and updated the doc comments to name
   both recovery paths.

2. AIChatAgent teardown now cancels the in-flight recovery turn. Think's
   `cancelAgentToolRun` sweeps `_submissionAbortControllers` to tear down
   an orphaned chat-recovery turn on give-up; AIChatAgent's did not, so a
   `window-exceeded` teardown left the recovered turn grinding (and
   holding a keep-alive) even though the row was already sealed `aborted`
   and tailers closed. AIChatAgent has no submission layer but does have
   a per-request `AbortRegistry` with `abortAllRequests()` — purpose-built
   for single-purpose sub-agent facets — so this was a fixable parity
   gap, not a design limitation. `cancelAgentToolRun` now calls
   `abortAllRequests(reason)`, and `AbortRegistry.destroyAll` /
   `abortAllRequests` gained an optional `reason` (backward-compatible)
   so the abort carries the give-up cause for diagnostics, matching Think.

Verified: npm run check (91 projects), ai-chat 617, think 513, think
agent-tools 22, ai-chat agent-tools 20, and the e2e reattach-budget
repro (child collected `completed`).
…r child self-finalize/cancel (#1630 review)

Review follow-ups for the progress-keyed agent-tool re-attach work.

Behavior:
- Default `agentToolReattachMaxWindowMs` to `Infinity` (was 15min). Post-#1672
  the child no longer has a wall-clock recovery ceiling, so a finite parent
  ceiling would tear down a healthy, still-streaming child that simply outran
  a fixed window under deploy churn — the exact bug #1672 removed, reintroduced
  at the parent layer. A hung child is already bounded by the no-progress
  budget; a content-runaway is bounded uniformly by the child's own
  `maxRecoveryWork` / `shouldKeepRecovering`. Integrators can still set a finite
  ceiling (which keeps the `window-exceeded` teardown).

Docs/comments:
- Rewrite docs/agent-tools.md "Interrupted runs and recovery" to describe
  re-attach + tail-to-terminal, the progress-keyed budget, the uncapped-by-
  default ceiling, and the soft (`no-progress`) vs hard (`window-exceeded`,
  torn down) give-up mapping; add `reason`/`childStillRunning` to the
  `AgentToolFailure` block.
- Fix stale comments/JSDoc that referenced the removed child ceiling and the
  old 15min default; clarify `window-exceeded` only fires with an opt-in cap.

Tests:
- P1: child facet self-finalizes its own run row via the reconcile in BOTH
  `_chatRecoveryContinue` AND `_chatRecoveryRetry` finally blocks (the
  pre-stream retry path the earlier review flagged), in Think and ai-chat.
- P2: assert the default re-attach ceiling resolves uncapped (no finite cap).
- P3: assert `applyAgentToolEvent` propagates `reason`/`childStillRunning`.
- P4: assert `cancelAgentToolRun` aborts an in-flight recovery turn (Think
  `_submissionAbortControllers` / ai-chat `AbortRegistry`) and seals `aborted`.
@threepointone
threepointone force-pushed the fix/agent-tool-reattach-1630 branch from 1555e78 to cb56372 Compare June 4, 2026 11:48
… replay (#1630 follow-up)

The #1630 re-attach work added a typed interrupted cause (`reason` /
`childStillRunning`) on RunAgentToolResult and emitted it on live wire
events. But those fields were never persisted to `cf_agent_tool_runs`, so
a client that reconnected and replayed a stored `interrupted` run saw
`reason`/`childStillRunning` as `undefined` — silently regressing any UI
told (by our docs) to branch on them instead of the `error` prose.

Persist and faithfully reconstruct the cause:

- Schema v9: add additive `interrupted_reason TEXT` + `child_still_running
  INTEGER` columns to `cf_agent_tool_runs` (CREATE TABLE + idempotent
  addColumnIfNotExists migrations; bump CURRENT_SCHEMA_VERSION 8 -> 9).
- _updateAgentToolTerminal writes both columns unconditionally, so
  repairing a soft `interrupted` row to a hard terminal (e.g. a re-attach
  that finally collects `completed`) CLEARS the stale cause instead of
  leaving it dangling. childStillRunning maps undefined->NULL, true->1,
  false->0.
- New shared helper _agentToolInterruptedExtrasFromRow reconstructs the
  typed cause from a row, guarding on status === 'interrupted' and
  treating each column's NULL independently. Wired into every row->result
  / row->event rebuild: _resultFromAgentToolRow, _readAgentToolRun,
  _replayAgentToolRuns (reconnect replay), and _reconcileAgentToolRuns.

Tests: new TestAgentToolReplayAgent fixture drives the real persist path
and a capture-connection replay. agent-tool-replay.test.ts covers
no-progress (childStillRunning true), torn-down window-exceeded
(childStillRunning false), reason-only (the reconcile path: no
childStillRunning), legacy rows (both columns NULL -> bare interrupted
event), and clear-on-repair to completed.

All green: build, npm run check (90 projects typecheck), and the agents /
ai-chat / think agent-tool suites pass.
…on no-progress', not 'skip waiting'

Review follow-up. `agentToolReattachNoProgressTimeoutMs` is typed `number?`
(which includes `Infinity`), and the sibling `agentToolReattachMaxWindowMs`
uses `Infinity` as its 'off / uncapped' default — so a user setting the
no-progress budget to `Infinity` reasonably expects 'never give up on a
silent child'. Instead, the guard in _reattachAgentToolRunToTerminal
short-circuited on `!Number.isFinite(...)`, returning immediately with an
`interrupted`/`no-progress` seal — identical to passing `0`, the exact
opposite of the intended semantic.

- Drop the `!Number.isFinite` clause from the early-return guard so a
  positive (incl. Infinity) budget falls through to the tail loop; only a
  non-positive budget still means 'do not wait'.
- In _forwardAgentToolStream, require Number.isFinite(idleTimeoutMs) for
  idleEnabled so an Infinity budget disables the idle timer entirely (it is
  never armed -> never fires). This also avoids handing Infinity to
  setTimeout (which clamps to ~1ms and fires almost immediately in the
  Workers/Node runtime). A silent-but-alive child is now followed until its
  stream closes or the finite hard ceiling fires.

`0` remains the 'don't wait, collect only an already-terminal child'
sentinel; semantics for finite budgets are unchanged.

Tests: new `infinite-no-progress-ceiling` scenario in the Think scripted
re-attach seam drives a silent, never-closing stream with an Infinity
no-progress budget + a finite ceiling and asserts it tails (tailAttempts
1) and seals `window-exceeded` (never `no-progress`) — pre-fix it
short-circuited with zero tail attempts. Docs + changeset updated.

(Note: the ai-chat _reconcileOwnStaleAgentToolChildRuns 'misses starting
rows' review item is NOT a bug — ai-chat inserts child run rows directly
as 'running' and has no 'starting' status, unlike Think's child-run
table, so `WHERE status = 'running'` is complete. No change made.)

All green: build, npm run check (90 projects), agents/ai-chat/think
agent-tool suites.
@threepointone
threepointone merged commit 5d64940 into main Jun 4, 2026
4 checks passed
@threepointone
threepointone deleted the fix/agent-tool-reattach-1630 branch June 4, 2026 13:12
@github-actions github-actions Bot mentioned this pull request Jun 4, 2026
threepointone added a commit that referenced this pull request Jun 5, 2026
…ets; close think hibernation gap

A turn parked on a pending CLIENT interaction (an `input-available` part for a
client-side tool with no server `execute`, or an `approval-requested` part) is
waiting on the human, not stuck. After a mid-turn Durable Object restart the
in-memory pending-interaction promise is gone, so `waitUntilStable()` repeatedly
times out until the client reconnects and replays the tool-result/approval. The
recovery loop was treating those timeouts as deploy churn and sealing perfectly
healthy turns with `stable_timeout` / `no_progress_timeout` — the "session
interrupted" banner users hit when they simply took a while to answer a prompt.

Core behavior (packages/ai-chat, packages/think — each carries its own copy of
the recovery engine):

- While a client interaction is pending the turn is budget-free:
  `_beginChatRecoveryIncident` suppresses the no-progress window, attempt cap,
  `maxRecoveryWork`, and `shouldKeepRecovering`, and keeps the no-progress clock
  fresh so the turn gets a full window once the human answers.
- `_chatRecoveryContinue` / `_chatRecoveryRetry` PARK instead of rescheduling or
  exhausting: the incident is marked `skipped` with reason
  `awaiting_client_interaction`, the live "recovering…" indicator is cleared,
  and the client's eventual replay resumes the turn via the normal
  auto-continuation. A client that never returns is reclaimed by the incident
  TTL sweep and DO idle eviction.
- think: a `submitMessages`-backed turn additionally has its durable submission
  row COMPLETED at park time. Recovery is that row's sole completion driver
  after a restart, so parking without completing would leave it `running` and
  the next boot's `_recoverSubmissionsOnStart` would sweep it to `error` (a false
  "session recovery error"). A fully-materialized client tool call in the leaf is
  exactly the terminal state a non-interrupted submission reaches, so `completed`
  is correct.
- Server-tool orphans are deliberately excluded (their `execute()` died with the
  isolate, nothing will resolve them) and still recover via transcript repair.
  ai-chat's existing `hasPendingInteraction()` (used by `waitUntilStable`) does
  not distinguish client from server tools, so a narrower client-only predicate
  `hasPendingClientInteraction()` was added there to gate the exemption without
  changing `waitUntilStable` semantics. think's `hasPendingInteraction()` was
  already client-only.

think hibernation-ordering fix:

- The exemption depends on `_lastClientTools`. ai-chat restores them in its
  constructor (available when boot recovery evaluates budgets), but think
  restored them in `onStart()`, which the base `Agent` runs AFTER the
  boot-recovery path (`_handleInternalFiberRecovery` -> `_beginChatRecoveryIncident`).
  On a fresh wake the in-memory cache was empty, so a client-tool
  `input-available` orphan re-detected past the no-progress window was misread as
  "stuck" and wrongly sealed.
- `_beginChatRecoveryIncident` now re-hydrates `_lastClientTools` from the
  durable `think_config` store (its own table, no Session init required, so the
  read is safe this early) before evaluating the budget; guarded so it is
  idempotent with the onStart restore and a no-op on the live-isolate stall path.
  `approval-requested` turns were never affected.

agents: warn when `chatRecovery` is configured in `onStart()`

- On every wake the SDK evaluates recovery budgets (and may seal a turn, firing
  `onExhausted`) BEFORE `onStart()` runs, so a `chatRecovery` produced there is
  read as the built-in defaults for the recovery that matters. The SDK now logs a
  one-time warning when `chatRecovery` is assigned during `onStart()` — for a
  config object or `chatRecovery = true` (disabling with `false` is a benign
  no-op and is not warned). Documented on `ChatRecoveryConfig`.
- Also fixes an unsafe `as` cast in `Agent._rootAlarmOwner` (`binding as unknown
  as DurableObjectNamespace<Agent>`) so classic tsc agrees with tsgo.

Tests:

- ai-chat + think: retry-path park unit tests mirroring the continue-path ones
  (no reschedule, no budget spent, incident parked `awaiting_client_interaction`;
  think also asserts the submission row is `completed` and a follow-up
  `_recoverSubmissionsOnStart` sweep does not resurrect it as `error`).
- think: boot-path hibernation regression test driving two fresh wakes with the
  in-memory client-tool cache cleared and tools only in the durable store —
  fails without the hydration guard (sealed `no_progress_timeout`), passes with
  it. New harness helpers: `seedDurableClientToolsForTest`,
  `clearInMemoryClientToolsForTest`; ai-chat `preScheduleRecoveryRetryForTest`.

e2e hardening:

- Add `packages/think/src/e2e-tests/harden-net.ts` (and inline equivalents in the
  ai-chat / agents chaos suites): best-effort wrap of `Socket.setTypeOfService`
  to swallow the benign write-time `EINVAL` undici throws synchronously when a
  socket is torn down mid-SIGKILL/restart, which otherwise surfaced as an
  unhandled exception and flaked otherwise-green runs. Imported by the 7 think
  e2e suites.
- Rewrite the stale `should interrupt a stale parent agent-tool run` e2e to the
  progress-keyed re-attach semantics (#1630/#1670): the parent re-attaches to the
  still-running child (which self-heals via `continue`) and collects its real
  `completed` terminal instead of an abandoned `interrupted`.

docs:

- chat-agents.md: `onStart` guidance callout, and a "Turns waiting on a human are
  not sealed" subsection (parks with `awaiting_client_interaction`, client-only).
- human-in-the-loop.md: "Surviving restarts while waiting for a human" subsection
  cross-linked to the stream-recovery reference.
- NOTE: both still need a manual port to cloudflare/cloudflare-docs.

experimental/chat-recovery-probe:

- Extend the live-worker harness with `hitl`, `server-orphan`, and `approval`
  synthetic modes; add scenarios a6 (HITL exemption), a7 (server-orphan recovers,
  not exempt), a8 (approval exemption), rapid (onExhausted exactly once), and idem
  (submission idempotency), plus WebSocket priming/reply/approval helpers. a6/a7/a8
  verified locally against real isolate resets.

Changesets: chat-recovery-pending-client-interaction (think, ai-chat patch),
chat-recovery-config-onstart-warning (agents, think, ai-chat patch).
threepointone added a commit that referenced this pull request Jun 5, 2026
…ets; close think hibernation gap (#1684)

* experimental: add chat-recovery-probe harness for #1672

A headless @cloudflare/think harness that validates the durable chat-recovery
assumptions from #1672 against the real production runtime, using a deterministic
synthetic in-DO model (no LLM) so the only interruption is a real deploy or
ctx.abort().

Validated in prod:
- happy-path: a progressing turn completes (submission `completed`).
- A1 invariant (headline): a progressing turn survives >15min of real deploy
  churn with monotonic progress and ZERO seals — no `max_recovery_window_exceeded`
  (which the pre-#1672 build emitted at the ~15min ceiling).

Findings recorded for follow-up (not exercised cleanly by this harness):
- `ctx.abort()` opens fresh retry-incidents (`conversation_changed`) instead of
  incrementing one incident's attempt.
- Under churn, `submitMessages` submission-recovery proliferates incidents and
  can error submissions; the per-attempt guards (maxRecoveryWork /
  shouldKeepRecovering / no-progress) need a single accumulating incident with
  same-message continuation — best covered by #1672's deterministic unit tests.

Experimental; not a product example.

* fix(chat-recovery): exempt human-in-the-loop turns from recovery budgets; close think hibernation gap

A turn parked on a pending CLIENT interaction (an `input-available` part for a
client-side tool with no server `execute`, or an `approval-requested` part) is
waiting on the human, not stuck. After a mid-turn Durable Object restart the
in-memory pending-interaction promise is gone, so `waitUntilStable()` repeatedly
times out until the client reconnects and replays the tool-result/approval. The
recovery loop was treating those timeouts as deploy churn and sealing perfectly
healthy turns with `stable_timeout` / `no_progress_timeout` — the "session
interrupted" banner users hit when they simply took a while to answer a prompt.

Core behavior (packages/ai-chat, packages/think — each carries its own copy of
the recovery engine):

- While a client interaction is pending the turn is budget-free:
  `_beginChatRecoveryIncident` suppresses the no-progress window, attempt cap,
  `maxRecoveryWork`, and `shouldKeepRecovering`, and keeps the no-progress clock
  fresh so the turn gets a full window once the human answers.
- `_chatRecoveryContinue` / `_chatRecoveryRetry` PARK instead of rescheduling or
  exhausting: the incident is marked `skipped` with reason
  `awaiting_client_interaction`, the live "recovering…" indicator is cleared,
  and the client's eventual replay resumes the turn via the normal
  auto-continuation. A client that never returns is reclaimed by the incident
  TTL sweep and DO idle eviction.
- think: a `submitMessages`-backed turn additionally has its durable submission
  row COMPLETED at park time. Recovery is that row's sole completion driver
  after a restart, so parking without completing would leave it `running` and
  the next boot's `_recoverSubmissionsOnStart` would sweep it to `error` (a false
  "session recovery error"). A fully-materialized client tool call in the leaf is
  exactly the terminal state a non-interrupted submission reaches, so `completed`
  is correct.
- Server-tool orphans are deliberately excluded (their `execute()` died with the
  isolate, nothing will resolve them) and still recover via transcript repair.
  ai-chat's existing `hasPendingInteraction()` (used by `waitUntilStable`) does
  not distinguish client from server tools, so a narrower client-only predicate
  `hasPendingClientInteraction()` was added there to gate the exemption without
  changing `waitUntilStable` semantics. think's `hasPendingInteraction()` was
  already client-only.

think hibernation-ordering fix:

- The exemption depends on `_lastClientTools`. ai-chat restores them in its
  constructor (available when boot recovery evaluates budgets), but think
  restored them in `onStart()`, which the base `Agent` runs AFTER the
  boot-recovery path (`_handleInternalFiberRecovery` -> `_beginChatRecoveryIncident`).
  On a fresh wake the in-memory cache was empty, so a client-tool
  `input-available` orphan re-detected past the no-progress window was misread as
  "stuck" and wrongly sealed.
- `_beginChatRecoveryIncident` now re-hydrates `_lastClientTools` from the
  durable `think_config` store (its own table, no Session init required, so the
  read is safe this early) before evaluating the budget; guarded so it is
  idempotent with the onStart restore and a no-op on the live-isolate stall path.
  `approval-requested` turns were never affected.

agents: warn when `chatRecovery` is configured in `onStart()`

- On every wake the SDK evaluates recovery budgets (and may seal a turn, firing
  `onExhausted`) BEFORE `onStart()` runs, so a `chatRecovery` produced there is
  read as the built-in defaults for the recovery that matters. The SDK now logs a
  one-time warning when `chatRecovery` is assigned during `onStart()` — for a
  config object or `chatRecovery = true` (disabling with `false` is a benign
  no-op and is not warned). Documented on `ChatRecoveryConfig`.
- Also fixes an unsafe `as` cast in `Agent._rootAlarmOwner` (`binding as unknown
  as DurableObjectNamespace<Agent>`) so classic tsc agrees with tsgo.

Tests:

- ai-chat + think: retry-path park unit tests mirroring the continue-path ones
  (no reschedule, no budget spent, incident parked `awaiting_client_interaction`;
  think also asserts the submission row is `completed` and a follow-up
  `_recoverSubmissionsOnStart` sweep does not resurrect it as `error`).
- think: boot-path hibernation regression test driving two fresh wakes with the
  in-memory client-tool cache cleared and tools only in the durable store —
  fails without the hydration guard (sealed `no_progress_timeout`), passes with
  it. New harness helpers: `seedDurableClientToolsForTest`,
  `clearInMemoryClientToolsForTest`; ai-chat `preScheduleRecoveryRetryForTest`.

e2e hardening:

- Add `packages/think/src/e2e-tests/harden-net.ts` (and inline equivalents in the
  ai-chat / agents chaos suites): best-effort wrap of `Socket.setTypeOfService`
  to swallow the benign write-time `EINVAL` undici throws synchronously when a
  socket is torn down mid-SIGKILL/restart, which otherwise surfaced as an
  unhandled exception and flaked otherwise-green runs. Imported by the 7 think
  e2e suites.
- Rewrite the stale `should interrupt a stale parent agent-tool run` e2e to the
  progress-keyed re-attach semantics (#1630/#1670): the parent re-attaches to the
  still-running child (which self-heals via `continue`) and collects its real
  `completed` terminal instead of an abandoned `interrupted`.

docs:

- chat-agents.md: `onStart` guidance callout, and a "Turns waiting on a human are
  not sealed" subsection (parks with `awaiting_client_interaction`, client-only).
- human-in-the-loop.md: "Surviving restarts while waiting for a human" subsection
  cross-linked to the stream-recovery reference.
- NOTE: both still need a manual port to cloudflare/cloudflare-docs.

experimental/chat-recovery-probe:

- Extend the live-worker harness with `hitl`, `server-orphan`, and `approval`
  synthetic modes; add scenarios a6 (HITL exemption), a7 (server-orphan recovers,
  not exempt), a8 (approval exemption), rapid (onExhausted exactly once), and idem
  (submission idempotency), plus WebSocket priming/reply/approval helpers. a6/a7/a8
  verified locally against real isolate resets.

Changesets: chat-recovery-pending-client-interaction (think, ai-chat patch),
chat-recovery-config-onstart-warning (agents, think, ai-chat patch).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: sub-agent (agent-tool) runs are abandoned and re-run on parent recovery instead of re-attaching

1 participant