fix(ai-client): re-evaluate continuation after chained tool approvals#347
Merged
fix(ai-client): re-evaluate continuation after chained tool approvals#347
Conversation
When a second tool approval arrives while a continuation stream is already in progress, the checkForContinuation call is skipped due to the continuationPending guard. This adds a continuationSkipped flag so that, once the active stream finishes, the client re-checks whether another continuation is needed, ensuring chained approval flows complete correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds handling for chained tool approvals by introducing a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(200,230,250,0.5)
participant ConnectionAdapter
participant ChatClient
participant ApprovalUI
participant ToolRuntime
end
ConnectionAdapter->>+ChatClient: stream chunks (tool call start / approval-requested)
ChatClient->>+ApprovalUI: emit approval request
ApprovalUI-->>-ChatClient: user approves (may occur during active stream)
alt continuationPending true
ChatClient-->>ChatClient: set continuationSkipped = true
Note right of ChatClient: wait for stream to finish
end
ChatClient->>+ToolRuntime: continue / invoke next tool (shouldAutoSend)
ToolRuntime-->>-ChatClient: stream chunks (tool output / run_finished)
ChatClient->>ChatClient: if continuationSkipped -> clear flag and re-run checkForContinuation
ChatClient->>+ToolRuntime: final assistant generation (if any)
ToolRuntime-->>-ChatClient: final assistant text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 3370830
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
jherr
commented
Mar 6, 2026
Contributor
Author
jherr
left a comment
jherr
left a comment
There was a problem hiding this comment.
Fix for mulitool approvals
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai-client/tests/chat-client.test.ts (1)
3-10:⚠️ Potential issue | 🟡 MinorSort the
./test-utilsnamed imports to satisfysort-imports.This specifier list is out of alphabetical order, so ESLint will fail on the new import.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-client/tests/chat-client.test.ts` around lines 3 - 10, The named imports from './test-utils' are not alphabetized; reorder the specifier list so it is sorted alphabetically (e.g., createApprovalToolCallChunks, createCustomEventChunks, createMockConnectionAdapter, createThinkingChunks, createTextChunks, createToolCallChunks) to satisfy the sort-imports ESLint rule and update the single import statement that currently imports those symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/typescript/ai-client/src/chat-client.ts`:
- Around line 649-655: The current logic unconditionally replays a queued
continuation when continuationSkipped is true, which can run even if the awaited
streamResponse() aborted; change the behavior so replay only happens after
streamResponse() reports success. Specifically, have streamResponse() return (or
set) a success boolean/state and check that signal before clearing
continuationSkipped and calling checkForContinuation(); reference the
continuationSkipped flag, the checkForContinuation() method, and the
streamResponse() call so you only replay when streamResponse() succeeded (and
still respect stop() by not replaying on abort/failure).
---
Outside diff comments:
In `@packages/typescript/ai-client/tests/chat-client.test.ts`:
- Around line 3-10: The named imports from './test-utils' are not alphabetized;
reorder the specifier list so it is sorted alphabetically (e.g.,
createApprovalToolCallChunks, createCustomEventChunks,
createMockConnectionAdapter, createThinkingChunks, createTextChunks,
createToolCallChunks) to satisfy the sort-imports ESLint rule and update the
single import statement that currently imports those symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 878b9ce8-96d6-469a-b36d-94a78b079493
📒 Files selected for processing (4)
.changeset/chained-approvals-fix.mdpackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai-client/tests/chat-client.test.tspackages/typescript/ai-client/tests/test-utils.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/architecture/approval-flow-processing.md (1)
176-176: Optional: Add language specifiers to code blocks.The markdownlint tool flags these code blocks for missing language specifiers. Since they contain pseudo-code and execution timelines rather than valid code in a specific language, consider adding
textas the language specifier to silence the warnings:```text Timeline: ... ```This is purely a style preference and doesn't affect the documentation quality.
Also applies to: 191-191, 202-202, 215-215, 226-226, 247-247, 330-330, 396-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture/approval-flow-processing.md` at line 176, Several fenced code blocks in the approval-flow-processing markdown contain pseudo-code/timeline text without a language specifier; update each of those fences (the ones that start with ``` and include the "Timeline:" sections and similar pseudo-code blocks) to use ```text so markdownlint stops flagging them, i.e., replace ``` with ```text for those blocks throughout the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/architecture/approval-flow-processing.md`:
- Line 176: Several fenced code blocks in the approval-flow-processing markdown
contain pseudo-code/timeline text without a language specifier; update each of
those fences (the ones that start with ``` and include the "Timeline:" sections
and similar pseudo-code blocks) to use ```text so markdownlint stops flagging
them, i.e., replace ``` with ```text for those blocks throughout the document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c85f29f-735d-41b6-8945-0c18f523191d
📒 Files selected for processing (1)
docs/architecture/approval-flow-processing.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
continuationSkippedflag that triggers a re-evaluation ofcheckForContinuationonce the active stream finishesTest plan
should continue after second approval arrives during active continuation streampnpm test:libinpackages/typescript/ai-client🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores