fix(agent): track tool call args per ToolCallID for parallel calls (#33)#34
Conversation
Previously GenerateWithCallbacks stored the most recent tool call's args in a single shared variable, which got clobbered when a provider emitted multiple tool_use blocks in a single step. Every OnToolResult callback then received the args of the last OnToolCall, regardless of which call it was actually resolving — breaking any downstream UI, log, or trace that derived its description from the toolArgs parameter. - Replace the shared currentToolArgs with a map keyed by ToolCallID, guarded by a sync.Mutex in case the streaming layer dispatches callbacks from multiple goroutines. - Delete each entry in OnToolResult so the map cannot accumulate across steps. - Add a regression test driving the streaming wrapper with a fake fantasy.Agent that emits two parallel tool calls before either result, asserting each callback sees its own args. Fixes #33
|
Connected to Huly®: KIT-35 |
|
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 (2)
📝 WalkthroughWalkthroughThis PR fixes a concurrency bug where parallel tool calls emitted in a single step all received the last call's arguments in their result callbacks. The fix replaces a single shared ChangesPer-ToolCallID Argument Tracking for Parallel Tool Calls
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Description
Agent.GenerateWithCallbacksstored the most recent tool call's args in a single sharedcurrentToolArgsvariable, captured by closure across all of its streaming callbacks. When a provider emitted multipletool_useblocks in a single step (Anthropic Claude, OpenAI parallel tool calls, etc.), everyOnToolCalloverwrote that variable before anyOnToolResultfired — so every result-side callback received the args of the last tool call, not its own. Downstream consumers (UI labels, log lines, trace span inputs) inherited the bug verbatim throughToolResultEvent.ParsedArgs.The fix replaces the shared variable with a
map[string]stringkeyed byToolCallID, written inOnToolCalland read+deleted inOnToolResult. Async.Mutexguards the map in case the streaming layer dispatches callbacks from multiple goroutines. Thetc.Inputalready inOnToolCallis forwarded unchanged (its callbacks were never broken — only the result-side lookup was).Before / after for a step with two parallel
load_skillcalls:Fixes #33
Type of Change
Checklist
go vet ./...,golangci-lint runclean)go test -race ./...)Additional Information
internal/agent/agent.go— swap sharedcurrentToolArgsfor a per-ToolCallIDmap with async.Mutex; addsyncimport.internal/agent/agent_parallel_tool_args_test.go— regression test that injects a fakefantasy.Agentemitting two parallelOnToolCalls before eitherOnToolResultand asserts each callback receives its own args.toolArgsparameter delivered toToolResultHandler/OnToolExecutionis now correct for parallel calls; consumers that were already receiving the (incorrect) last-call's args will now receive the right value.masterand pass on this branch.Summary by CodeRabbit