Skip to content

feat(think): add repairInterruptedToolPart hook for client-resolved tools (#1631)#1635

Merged
threepointone merged 1 commit into
mainfrom
feat/think-repair-interrupted-tool-part-hook
Jun 1, 2026
Merged

feat(think): add repairInterruptedToolPart hook for client-resolved tools (#1631)#1635
threepointone merged 1 commit into
mainfrom
feat/think-repair-interrupted-tool-part-hook

Conversation

@threepointone

Copy link
Copy Markdown
Contributor

Stack

This is PR 3 of 4 for #1631. Stacked; base is fix/chat-recovery-preserve-settled-work (PR #1634). Retargets to main as the stack merges. Diff shown is only this PR's commit.

  1. fix(think,ai-chat): make chat-recovery progress signal compaction-immune (#1628) #1633 — progress signal compaction-immune (bug: chat-recovery progress marker is count-based, so compaction can cause premature budget exhaustion #1628) · patch
  2. fix(think,ai-chat): preserve settled work when a recovery turn is given up (#1631) #1634 — preserve settled work when a turn is given up · patch
  3. ➡️ repairInterruptedToolPart hook (this PR) · minor
  4. incident identity + onExhausted payload · minor

This is a public-API addition (a new protected override point) — the one most worth a careful API-shape review. @cloudflare/think only (ai-chat has no transcript-repair pass).


Problem (#1631, item 2)

Transcript repair (_repairToolTranscriptParts) flips a tool call with no settled result to an errored tool-result — preserving the record and keeping the provider from 400ing (AI_MissingToolResultsError). That's the right default for server tools.

It's wrong for client-resolved tools like ask_user — a question with no server execute, answered by the user's next message. There, the interrupted call is a question, and the right repair is to preserve it as a text part carrying the prompt, so the model sees a normal Q→A exchange and compaction keeps the question verbatim. Flipping it to output-error turns "the assistant asked X" into "a tool errored" and loses the prose.

Crucially, repair runs and persists before beforeTurn:

// _runInference
const history = await this._repairTranscriptForProvider(this.messages); // repairs + persists + broadcasts
const messages = await convertToModelMessages(truncated, { ... });       // builds model ctx
...
const subclassConfig = (await this.beforeTurn(ctx)) ?? {};               // too late: ctx.messages already built/persisted

So a subclass had no way to shape this for the current turn — any beforeTurn fix-up lands a turn late and races the framework's persisted output-error.

Fix

Extract the per-orphan flip into a protected overridable hook that repair consults during the repair pass (before persist), defaulting to the existing behavior:

/** Repair a single interrupted tool call (no settled result). `input` is already
 *  normalized. Default flips it to an errored tool-result. Override to preserve
 *  client-resolved tools (e.g. ask_user -> a text part carrying the prompt) so the
 *  conversion shapes THIS turn, before repair persists + sends to the model. */
protected repairInterruptedToolPart(
  part: UIMessage["parts"][number]
): UIMessage["parts"][number] {
  return { ...part, state: "output-error", errorText: "The tool call was interrupted before a result was recorded." };
}

_repairToolTranscriptParts now calls this.repairInterruptedToolPart({ ...part, input: normalizedInput }). Default behavior is byte-for-byte unchanged. A subclass returning a text part for ask_user takes effect on the same turn, because it runs inside the repair pass.

Contract (documented on the method): a returned tool part must carry a settled result (so the ignoreIncompleteToolCalls backstop / _incompleteToolCallIds don't drop it); returning a non-tool part (e.g. text) is fine.

Files

  • packages/think/src/think.ts — new protected repairInterruptedToolPart; _repairToolTranscriptParts refactored to call it (default unchanged).
  • packages/think/src/tests/agents/client-tools.tsThinkClientToolsAgent overrides the hook (ask_user → text) to demonstrate the intended usage + a repairToolTranscriptPartsForTest harness.
  • packages/think/src/tests/client-tools.test.ts — tests.
  • .changeset/think-repair-interrupted-tool-part-hook.md (minor · think).

Tests

  • converts an interrupted ask_user into a text part carrying the prompt (the override).
  • falls back to the default errored-result repair for non-ask_user tools (default preserved).
  • leaves a settled ask_user untouched (only interrupted calls are repaired).

The pre-existing repair tests (message-reconciliation.test.ts: orphan → output-error) still pass — the refactor is behavior-preserving for the default, and the demo override only matches tool-ask_user (other agents' orphans are unaffected).

Verification

  • tsc --noEmit clean (think); oxlint clean; oxfmt applied.
  • client-tools + message-reconciliation suites: 58 passed. Full think suite: 460 passed.

API review notes

  • Name/shape up for discussion: repairInterruptedToolPart(part) -> part. Alternatives considered: a beforeTranscriptRepair(messages) pre-hook (rejected — needs extra persist-gating to avoid the repair's early-return swallowing hook-only changes; per-part is cleaner and persists naturally).
  • This only resolves item 2 for think. ai-chat doesn't run this repair pass, so no change there.

Test plan

  • CI green
  • API-shape sign-off on repairInterruptedToolPart
  • Confirm default-path repair unchanged (existing repair tests green)

Made with Cursor

@changeset-bot

changeset-bot Bot commented May 31, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fd8a39d

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

This PR includes changesets to release 1 package
Name Type
@cloudflare/think Minor

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

…ools (#1631)

Transcript repair flips an interrupted tool call (no settled result) to an
errored tool-result — right for server tools, wrong for client-resolved tools
like ask_user, where the call is a question that should be preserved as text so
the model sees normal Q->A conversation. Because repair runs and persists before
beforeTurn, subclasses had no way to shape this for the current turn.

Extract the flip into a protected repairInterruptedToolPart(part) hook (default
unchanged). Overriding it (e.g. ask_user -> text part carrying the prompt) now
takes effect during repair, on the same turn. Test agent demonstrates the
ask_user->text override; tests cover override, default fallback, and that
settled calls are left untouched.

Co-authored-by: Cursor <cursoragent@cursor.com>
@threepointone
threepointone force-pushed the feat/think-repair-interrupted-tool-part-hook branch from 60cf123 to fd8a39d Compare June 1, 2026 14:49
@threepointone

Copy link
Copy Markdown
Contributor Author

Rebased onto main + docs added

Green: think 466, client-tools 50, npm run check clean (91 projects).

@pkg-pr-new

pkg-pr-new Bot commented Jun 1, 2026

Copy link
Copy Markdown

Open in StackBlitz

agents

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

@cloudflare/ai-chat

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

@cloudflare/codemode

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

hono-agents

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

@cloudflare/shell

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

@cloudflare/think

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

@cloudflare/voice

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

@cloudflare/worker-bundler

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

commit: fd8a39d

@threepointone
threepointone merged commit 5995fa8 into main Jun 1, 2026
4 checks passed
@threepointone
threepointone deleted the feat/think-repair-interrupted-tool-part-hook branch June 1, 2026 14:56
threepointone added a commit that referenced this pull request Jun 1, 2026
…sted payload (#1631)

Lets products build a terminal-state policy without re-deriving anything:

- ChatRecoveryContext (onChatRecovery) gains recoveryRootRequestId — the stable
  request id for the whole continuation chain, the right key for per-incident
  budget tracking / fresh-incident detection (no re-deriving from message IDs).
- ChatRecoveryExhaustedContext (onExhausted) gains recoveryRootRequestId,
  terminalMessage (exact user-facing text), partialText/partialParts (what the
  turn produced before it was given up on), and streamId/createdAt — enough to
  render/persist a terminal banner AND emit correlated terminal telemetry
  (msSinceTurnStart, stream correlation) directly.

streamId + createdAt were added after verifying the payload against the actual
consumer (g3's _emitExhaustedRecovery): it reads both from the recovery context
for telemetry, and they already exist on ChatRecoveryContext (the Pick source),
so adding them to the exhausted context is additive and unblocks re-homing the
exhaustion handler onto onExhausted with zero re-derivation (D4).

Shared types in `agents`; wired through think + ai-chat (_exhaustChatRecovery
now receives streamId + createdAt). Test agents capture the exhausted context;
tests assert both contexts (incl. streamId + createdAt) in both packages.

Rebased onto main (dropping the merged #1633/#1634/#1635 commits); adapted the
exhausted-ctx test to N1/#1638's alarm-debounce and gave the think harness an
explicit return shape (the context's MessagePart[] over-instantiates the RPC
stub type).

Co-authored-by: Cursor <cursoragent@cursor.com>
threepointone added a commit that referenced this pull request Jun 1, 2026
…sted payload (#1631) (#1636)

Lets products build a terminal-state policy without re-deriving anything:

- ChatRecoveryContext (onChatRecovery) gains recoveryRootRequestId — the stable
  request id for the whole continuation chain, the right key for per-incident
  budget tracking / fresh-incident detection (no re-deriving from message IDs).
- ChatRecoveryExhaustedContext (onExhausted) gains recoveryRootRequestId,
  terminalMessage (exact user-facing text), partialText/partialParts (what the
  turn produced before it was given up on), and streamId/createdAt — enough to
  render/persist a terminal banner AND emit correlated terminal telemetry
  (msSinceTurnStart, stream correlation) directly.

streamId + createdAt were added after verifying the payload against the actual
consumer (g3's _emitExhaustedRecovery): it reads both from the recovery context
for telemetry, and they already exist on ChatRecoveryContext (the Pick source),
so adding them to the exhausted context is additive and unblocks re-homing the
exhaustion handler onto onExhausted with zero re-derivation (D4).

Shared types in `agents`; wired through think + ai-chat (_exhaustChatRecovery
now receives streamId + createdAt). Test agents capture the exhausted context;
tests assert both contexts (incl. streamId + createdAt) in both packages.

Rebased onto main (dropping the merged #1633/#1634/#1635 commits); adapted the
exhausted-ctx test to N1/#1638's alarm-debounce and gave the think harness an
explicit return shape (the context's MessagePart[] over-instantiates the RPC
stub type).

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

1 participant