Skip to content

fix(agent): pair parallel tool calls by internal call id - #266

Merged
gi-dellav merged 1 commit into
gi-dellav:mainfrom
xavierforge:fix/parallel-tool-call-pairing
Aug 27, 2026
Merged

gi-dellav merged 1 commit into
gi-dellav:mainfrom
xavierforge:fix/parallel-tool-call-pairing

Conversation

@xavierforge

Copy link
Copy Markdown
Collaborator

Summary

rig 0.40 streams a parallel tool-call batch as all ToolCall items first, then their ToolResult items, but zerostack tracked the in-flight call in single "most recent call" slots. With more than one call in flight, the first result was recorded under the last call's name and args, and later results under an empty name. This PR keys the pairing on rig's internal_call_id in all four consumers: the headless run_print loop, the spawn_agent event loop, the TUI session recorder, and the ACP bridge.

Motivation

Session JSON tool records are the structured record of what a turn actually did, and any provider that emits parallel tool calls corrupts them today: wrong name and args on the first result, empty name on the rest. The TUI tool-result display and ACP tool-call notifications mispair the same way. Anthropic, OpenAI, and Gemini all emit parallel batches in normal operation, so this is not an edge case.

Design decisions

  • Pairing is keyed by rig's internal_call_id, not the provider-supplied ToolCall.id: Gemini and Ollama set ToolCall.id to the function name, so it is not unique within a batch, while internal_call_id is rig-generated, unique, and carried on both the ToolCall and ToolResult stream items.
  • AgentEvent::ToolCall and AgentEvent::ToolResult gained a call_id field so every consumer pairs by id instead of by arrival order. The TUI keeps its pending calls in one insertion-ordered Vec behind AgentRunState methods, which derives "newest still-pending" for subagent attribution and gives every run-teardown path (done, error, abort, Ctrl-C, mid-turn compaction respawn, context-exhausted stop) a single clear call so a stranded entry cannot leak into the next run.
  • A ToolResult with no matching pending call (reachable in rig via the abandoned-turn path for invalid tool calls) is recorded against a freshly synthesized orphan call instead of being linked to call id 0, which is a real call, the session's first.
  • Subagent call attribution stays a documented best-effort heuristic (newest pending call in the TUI, first-arriving result in headless): the subagent side channel carries no call id today, and threading rig's per-call id through the task tool is a follow-up noted below.

Testing

  • New src/tests/parallel_tool_call_tests.rs (7 tests): a two-call batch through two distinct tools asserts correct name/args/output pairing in run_print's recorded interactions and id-keyed pairing in spawn_agent's event stream, with an ordered-log assertion that the batch shape (both calls before the first result) is what the test exercises; unit tests for the AgentRunState pending-call helpers (pairing in either result order, newest-pending degradation, duplicate-id replacement, clear); an orphan-result test asserting call 0's record stays untouched.
  • cargo test --all-features: 1079 passed, 0 failed. cargo clippy --all-targets --all-features -- -D warnings: clean.
  • Pre-submission review: an independent maintainer-persona review pass ran on this diff; its blocking findings are folded into the commit and the deliberately deferred ones are listed under Notes.

Reviewing

Start with src/event.rs for the event contract, then src/agent/runner.rs (the spawn_agent and run_print loops are the two recording paths), then src/ui/state.rs plus src/ui/event_handler.rs for the TUI pending-call state and teardown coverage, then src/extras/acp/mod.rs. The test file states the intended pairing semantics and is the fastest way to see the behavior change. LoC breakdown: production +232/-57 (runner 70/18, TUI 117/30, ACP 35/8, event 9/0, docs 1/1), tests +375/0.

Notes

  • Follow-up, not in this PR: exact subagent attribution is possible by threading rig's per-call internal_call_id through the task tool via rig's hook stack; this PR keeps the pre-existing best-effort heuristic and documents it accurately on both paths.
  • Follow-up, not in this PR: an ACP bridge-level batch test and an interactive TUI harness run; the TUI pairing state machine is covered by unit tests on the AgentRunState helpers instead.
  • In the headless orphan-result fallback the tool name stays an empty string because rig's ToolResult carries no name field; the recorded pair is self-consistent under its own freshly stamped id.
  • The ACP subagent tool-call notification is now announced with status Completed: it previously defaulted to Pending and nothing ever resolved it. Announce-only is correct until the side channel carries an id to correlate a later update, and the unmatched-result fallback no longer emits a ToolCallUpdate for an id the client was never told about.

@xavierforge
xavierforge force-pushed the fix/parallel-tool-call-pairing branch from 23ef156 to d9f7c82 Compare August 27, 2026 07:12
@gi-dellav
gi-dellav merged commit 58c1d74 into gi-dellav:main Aug 27, 2026
12 checks passed
@xavierforge
xavierforge deleted the fix/parallel-tool-call-pairing branch August 27, 2026 11:17
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.

2 participants