fix: key delegated approval by toolName with toolCallId fallback#3090
Conversation
Delegated agent approvals were keyed by toolCallId in the forwarded approved_tool_calls metadata. But when the delegated agent re-executes after approval, the LLM generates a new toolCallId that doesn't match the original — causing an infinite approval loop. Fix: key by toolName (stable across re-executions) and carry originalToolCallId for audit. The consumer looks up by toolCallId first, then falls back to toolName for delegated scenarios.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
|
TL;DR — Fixes delegated tool approvals breaking on re-execution by keying the Key changes
Summary ｜ 2 files ｜ 1 commit ｜ base:
In durable workflows, when a delegated agent re-executes after a pause, the AI SDK generates a new
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped fix for a real bug. The root cause analysis is sound — toolCallId is unstable across LLM re-executions, toolName is stable. The fallback chain (toolCallId → toolName) preserves backward compatibility for the direct approval path.
One minor gap (non-blocking): tool-wrapper.ts:156-158 also reads approvedToolCalls but only by toolCallId:
const preApprovedEntry = ctx.durableWorkflowRunId
? ctx.approvedToolCalls?.[toolCallId]
: undefined;After this fix, delegated approvals are keyed by toolName, so isPreApproved will be false for delegated pre-approved tools. This causes writeToolInputStart/writeToolInputDelta/writeToolInputAvailable to fire even though the tool is about to be auto-approved in waitForToolApproval. Not a correctness issue — just a flash of tool-input streaming in the UI for pre-approved delegated tools. Could apply the same fallback:
const preApprovedEntry = ctx.durableWorkflowRunId
? (ctx.approvedToolCalls?.[toolCallId] ?? ctx.approvedToolCalls?.[toolName])
: undefined;Claude Opus ｜ 𝕏
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
🟡 Minor (0) 🟡
No minor issues requiring action.
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
relationTools.ts:452originalToolCallIdfield added but not consumed — consider adding audit logging or documenting the field's purpose
🧹 While You're Here (1) 🧹
🧹 1) scope Missing changeset
Issue: The changeset-bot flagged that this PR has no changeset, but this is a bug fix that changes runtime behavior in agents-api.
Why: Per AGENTS.md, all bug fixes to published packages need changesets for proper versioning and changelog generation.
Fix: Run:
pnpm bump patch --pkg agents-api "Fix infinite approval loop in delegated agent tool approvals by keying by toolName"💡 APPROVE WITH SUGGESTIONS
Summary: This is a well-targeted fix for the infinite approval loop issue in delegated agent scenarios. The approach is sound — toolName is stable across LLM re-executions while toolCallId changes. The fallback pattern (toolCallId ?? toolName) maintains backward compatibility with direct approvals while supporting delegated approvals.
The implementation is minimal and correctly targeted. The only action item is adding the missing changeset before merging.
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
system |
Multiple same-tool calls could receive wrong approval | The workflow design handles one approval at a time — execution suspends until a specific approval is granted, so toolName keying is safe for the delegated single-approval scenario |
tool-approval.ts:52-53 |
Asymmetric delete of both keys | Defensive programming pattern that's actually correct — delete on non-existent key is a no-op in JS, and this ensures cleanup for both direct and delegated approval paths |
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-precision |
3 | 0 | 1 | 0 | 1 | 0 | 2 |
| Total | 3 | 0 | 1 | 0 | 1 | 0 | 2 |
Note: Standards reviewer found no issues. Precision reviewer raised valid structural questions that were validated against the workflow design — the single-approval-at-a-time workflow makes toolName keying safe.
| [agentRunContext.delegatedToolApproval.toolName]: { | ||
| approved: agentRunContext.delegatedToolApproval.approved, | ||
| reason: agentRunContext.delegatedToolApproval.reason, | ||
| originalToolCallId: agentRunContext.delegatedToolApproval.toolCallId, |
There was a problem hiding this comment.
💭 Consider: originalToolCallId not consumed
Issue: The originalToolCallId field is added to the forwarded approval metadata but isn't read by the consumer (tool-approval.ts). The PR description mentions it's "for audit" but there's no audit logging that consumes this field.
Why: Minor dead data concern. However, this could be valuable for future debugging/tracing if logged when the pre-approved decision is applied.
Fix: Either:
- Add a log line in
tool-approval.tswhen a delegated approval is found:logger.debug({ originalToolCallId: preApproved.originalToolCallId }, 'Applied delegated approval') - Or document in a code comment that this field is intentionally included for observability/debugging purposes
| // Look up by toolCallId first, then fall back to toolName (for delegated approvals | ||
| // where the toolCallId changes across re-executions but the toolName stays the same). | ||
| const preApproved = approvedToolCalls[toolCallId] ?? approvedToolCalls[toolName]; | ||
| if (preApproved !== undefined) { | ||
| delete approvedToolCalls[toolCallId]; | ||
| delete approvedToolCalls[toolName]; |
There was a problem hiding this comment.
🟡 Minor: Comment accurately explains the fallback
The comment on lines 48-49 correctly explains the rationale for the fallback lookup. The defensive deletion of both keys (lines 52-53) is appropriate — one will be a no-op depending on the scenario, but it ensures cleanup regardless of which key matched.
Good: This pattern handles both:
- Direct approvals (keyed by
toolCallId) - Delegated approvals (keyed by
toolName)
Preview URLsUse these stable preview aliases for testing this PR:
These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find. Raw Vercel deployment URLs
|
Ito Test Report ✅12 test cases ran. 12 passed. Overall, the unified QA run passed 12/12 executed test cases with 0 failures, found no confirmed production defects, and included one additional execution that did not yield completed code-first outcomes. The most important validated behaviors were robust delegated-approval handling across /run/api/chat and SSE flows (including re-execution and interrupted-stream recovery), correct pending-approval suspend/resume lifecycle, idempotent and race-safe single-consumption of approval decisions, safe handling of stale/forged payloads without unintended tool execution, exact-key matching without same-name/normalization collisions, and fully interactive mobile approval controls at 390x844. ✅ Passed (12)
Commit: Tell us how we did: Give Ito Feedback |
1 participant
Summary
toolCallIdin the forwardedapproved_tool_callsmetadatatoolCallIdthat doesn't match → infinite approval looptoolName(stable across re-executions) and carryoriginalToolCallIdfor audittoolCallIdfirst, then falls back totoolNamefor delegated scenariosTest plan
get_coordinatesapproval kept re-appearing)toolNameso delegated agent finds the pre-approved decision