Skip to content

fix(agent): reject duplicate call IDs before executing a tool batch - #133

Draft
LukasParke wants to merge 1 commit into
mainfrom
fix/reject-duplicate-call-ids-128
Draft

LukasParke wants to merge 1 commit into
mainfrom
fix/reject-duplicate-call-ids-128

Conversation

@LukasParke

Copy link
Copy Markdown
Contributor

Fixes #128

This is @chiarorosa's fix. The branch head is their commit dd315f7, unchanged and with their authorship, from the #128 discussion. It is opened from an org branch so CI runs; happy to close in favor of a PR from their fork if they prefer.

What

When one response contains two function_call items with the same call_id, the run now throws before any tool body runs and before any follow-up request is sent. The error names the repeated ID. Previously both calls ran and the follow-up carried two function_call_output items for one ID.

  • The guard (assertUniqueCallIds) runs at all three dispatch sites in model-result.ts: the initial response, later loop turns, and the pending batch on the stopWhen / allowFinalResponse path. On each path it runs before the approval gate.
  • Batches with no auto-executable calls (all manual or caller-executed tools) are skipped. The SDK runs nothing for those, so the caller still gets every call as emitted and owns correlation.
  • Updated existing tests that assumed duplicates execute:
    • approval-gate-regressions.test.ts "gates duplicate identical calls separately..." now uses distinct IDs with identical name and args.
    • doom-loop-integration.test.ts duplicate-ID evasion test now expects the batch to be refused.
  • New tests/unit/duplicate-call-ids.test.ts and a patch changeset.

Policy decision for the maintainer

Reject the whole batch (this PR) vs. drop or de-duplicate the duplicates and run the rest.

  • Rejecting fails closed: nothing runs and the error is loud. The cost is that one malformed model response fails the run (callers retry).
  • Dropping keeps the run going, but the SDK has to guess which of two calls with different arguments is "real", and it silently discards one.

This PR rejects. Please confirm that's the intended behavior before merging.

Known limitation

This only checks IDs within a single response batch. An ID reused in a later turn (the same call_id as a call from an earlier turn) is not detected. Catching that needs a check against the conversation history, so it is a separate follow-up.

Test plan

  • The new duplicate-ID tests and the updated doom-loop test fail with the guard removed (4 failures against main's model-result.ts) and pass with it. The distinct-ID and manual-only controls pass either way.
  • pnpm install --frozen-lockfile, pnpm lint, pnpm typecheck, pnpm test (agent: 1241 passed, mcp: 12 passed).

🤖 Generated with Claude Code

This branch has not been deployed

No deployments
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.

Duplicate call IDs in one Responses batch both execute and produce same-ID outputs (0.11.0)

2 participants