fix(ai-chat): simplify turn coordination API#1151
Conversation
…interception - expand waitForPendingInteractionResolution JSDoc to warn about the auto-continuation race: when a tool interaction resolves the SDK queues a continuation turn at the same moment the poll resolves; a second waitForIdle() is required before reading this.messages or calling saveMessages(); add the full three-step recipe in the docstring - expand abortActiveTurn JSDoc to warn that subclasses intercepting CF_AGENT_CHAT_CLEAR and returning early bypass the SDK's built-in epoch increment and abort; they must call abortActiveTurn() explicitly to stop any active stream before performing a scoped delete - expand docs/chat-agents.md turn coordination section: add helper reference table, three-step saveMessages pattern with explanation of why the second waitForIdle is needed, workflow-switch abort example, and CF_AGENT_CHAT_CLEAR interception guide with the mandatory abortActiveTurn call and consequences of omitting it
🦋 Changeset detectedLatest commit: af096ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
hmm we might have to revisit 1142, I'll have a look tomorrow |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
…lution and resetTurnState
Rename waitForPendingInteractionResolution() to waitUntilStable() and make it wait for a fully stable conversation (including queued continuation turns). Demote isChatTurnActive(), waitForIdle(), and abortActiveTurn() to private; expose resetTurnState() as the public way to abort and invalidate queued continuations. Update docs and README to reflect the new helpers and usage guidance, and adjust tests/worker helpers to call the renamed APIs or access internal fields for test-only behavior. Also tighten pending-interaction bookkeeping to avoid leaking rejected tool/apply promises.
Replace the inline timeout Promise in a Promise.race with an explicit timer variable, await the race result, then clearTimeout(timer) before returning to avoid the timeout callback firing after the race completes. Also update a comment to reference waitUntilStable instead of waitForPendingInteractionResolution for clarity.
| const remainingMs = Math.max(0, deadline - Date.now()); | ||
| let timer: ReturnType<typeof setTimeout>; | ||
| const result = await Promise.race([ | ||
| promise, | ||
| new Promise<typeof TIMED_OUT>((resolve) => { | ||
| timer = setTimeout(() => resolve(TIMED_OUT), remainingMs); | ||
| }) | ||
| ]); | ||
| clearTimeout(timer!); | ||
| return result; |
There was a problem hiding this comment.
🟡 Timer leak in _awaitWithDeadline when the input promise rejects
If the promise argument rejects, Promise.race rejects and the await throws, so the clearTimeout(timer!) on line 1076 is never reached. The timer keeps running in the background until it fires after remainingMs. This is called from waitUntilStable at packages/ai-chat/src/index.ts:1017, where a rejected _pendingInteractionPromise (e.g., from a SQLite failure in _findAndUpdateToolPart) triggers catch { continue }, re-entering the loop and potentially creating additional leaked timers. Each leaked timer lives for up to remainingMs before self-cleaning, so the impact is bounded but unnecessary.
| const remainingMs = Math.max(0, deadline - Date.now()); | |
| let timer: ReturnType<typeof setTimeout>; | |
| const result = await Promise.race([ | |
| promise, | |
| new Promise<typeof TIMED_OUT>((resolve) => { | |
| timer = setTimeout(() => resolve(TIMED_OUT), remainingMs); | |
| }) | |
| ]); | |
| clearTimeout(timer!); | |
| return result; | |
| const remainingMs = Math.max(0, deadline - Date.now()); | |
| let timer: ReturnType<typeof setTimeout>; | |
| try { | |
| const result = await Promise.race([ | |
| promise, | |
| new Promise<typeof TIMED_OUT>((resolve) => { | |
| timer = setTimeout(() => resolve(TIMED_OUT), remainingMs); | |
| }) | |
| ]); | |
| return result; | |
| } finally { | |
| clearTimeout(timer!); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Simplifies the turn coordination API surface from 6 protected helpers to 3, and rewrites
waitForPendingInteractionResolution()with event-driven semantics (no more polling).Builds on the turn serialization machinery from #1142. That PR added 6 protected helpers; this PR consolidates them before any release ships.
What changed
1. API surface: 6 protected helpers → 3
Kept (protected):
waitUntilStable()resetTurnState()hasPendingInteraction()Demoted to private:
isChatTurnActive()waitForIdle()waitUntilStable(), which drains the queue as its first stepabortActiveTurn()resetTurnState()is correct in every real scenario (clear, workflow switch)2.
waitForPendingInteractionResolution()→waitUntilStable()Renamed to match the actual semantics ("waits until the conversation is fully stable").
Before: Polled
hasPendingInteraction()on an interval. Could resolve at the exact moment the SDK queued an auto-continuation, requiring the fragilewaitForIdle() → waitForPendingInteraction() → waitForIdle()dance.After: Event-driven via a new
_pendingInteractionPromisefield. Drains_chatTurnQueue, awaits the in-flight tool apply, then loops to catch anyautoContinuecontinuation. One call does everything:3.
resetTurnState()addedEncapsulates the full reset (epoch increment + abort controllers + pending promise cleanup) that the built-in
CF_AGENT_CHAT_CLEARhandler previously did inline. Subclasses intercepting clear can now call one method instead of knowing three internals.4. Unhandled rejection suppression
The
_pendingInteractionPromisecleanup chains now attach.catch(() => {})so rejected tool-result / approval applies do not leak as unhandled rejections.5. Expanded documentation
The
docs/chat-agents.mdturn coordination section now includes:Files changed
packages/ai-chat/src/index.tswaitForPendingInteractionResolution→waitUntilStable; add_pendingInteractionPromise; rewrite with event-driven semantics; addresetTurnState(); suppress unhandled rejectionspackages/ai-chat/src/tests/pending-interaction.test.tswaitUntilStableForTest; add WebSocket tool-result, tool-approval, auto-continuation, active-turn timeout, andresetTurnStatecoveragepackages/ai-chat/src/tests/worker.tswaitUntilStableForTest,resetTurnStateForTest; rewrite demoted wrappers to useas unknown ascasts for private field accessdocs/chat-agents.mdpackages/ai-chat/README.md.changeset/...@cloudflare/ai-chatVerification
npm run checkpasses (sherif, export checks, oxfmt, oxlint, typecheck across 65 projects)packages/ai-chatvitest suite passes (315 tests)Reviewer notes
index.ts(API changes) → tests → docshasPendingInteraction()reads from persisted messages in SQLiteprotectedorprivateas unknown aspattern in test wrappers for private field access matches the existinggetAbortControllerCount()pattern in the same file