🧭 fix: Handle Mistyped Handoff Tools Locally#161
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 932eac10de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private getRegisteredHandoffNames(): Iterable<string> { | ||
| return this.directToolNames != null && this.directToolNames.size > 0 | ||
| ? this.directToolNames | ||
| : this.toolMap.keys(); |
There was a problem hiding this comment.
getRegisteredHandoffNames() returns only directToolNames whenever that set is non-empty, which drops graph handoff tools that still live in toolMap. In runs with local execution enabled, applyToolExecutionOverrides() commonly populates directToolNames with code tools, so a mistyped lc_transfer_to_* handoff in the normal (non-event-driven) path no longer gets the new exact-name guidance and falls back to a generic not-found error. This regresses the handoff-typo behavior for local-execution configurations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 1c99eae: getRegisteredHandoffNames() now treats directToolNames as an overlay and includes toolMap handoff names as well. I added coverage for a handoff in toolMap with an unrelated direct local tool name present, and verified with NODE_OPTIONS='--experimental-vm-modules' npx jest src/specs/agent-handoffs.test.ts --runInBand, npx tsc --noEmit, npx eslint src/tools/ToolNode.ts src/specs/agent-handoffs.test.ts, and git diff --check.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c99eaeea5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (directToolNames == null || directToolNames.size === 0) { | ||
| return this.executeViaEvent(filteredCalls, config, input, { | ||
| batchIndices: filteredIndices, | ||
| turn, |
There was a problem hiding this comment.
When eventDrivenMode is enabled and directToolNames is empty, this early return dispatches every call to executeViaEvent, so handoff-shaped unknown calls never reach shouldHandleUnknownHandoffLocally. That creates a behavior gap with the Send input branch (which now does local fail-closed handling), and it still lets typoed lc_transfer_to_* calls leak to the host executor in configurations where handoff tools exist only in toolMap (for example schema-only/event-defined handoff names or manually constructed ToolNodes).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c074e9: the event-driven batch path now classifies calls before using the all-host fast path, so unknown handoff-shaped calls stay local even when directToolNames is empty. I added a regression for an event-driven ToolNode with no direct tool names, a real host tool, and a mistyped handoff that exists only in toolMap. Verified with NODE_OPTIONS='--experimental-vm-modules' npx jest src/specs/agent-handoffs.test.ts --runInBand, npx tsc --noEmit, npx eslint src/tools/ToolNode.ts src/specs/agent-handoffs.test.ts, and git diff --check.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
I fixed mistyped graph-managed handoff tool calls so they fail closed inside
ToolNodewith exact-name guidance instead of being silently executed as a guessed handoff or dispatched to the host tool executor.Fixes #60.
runToolerror path so the model receives a normalToolMessageerror tied to its original tool call id.ON_TOOL_EXECUTE.Run.processStreamexecution and a reusabletest:live:handoffsscript.This is empirically better than the contributor patch because the tests now measure the behavioral contract directly: a mistyped handoff is not executed, does not reach the host executor, and still returns actionable exact-name feedback. The live integration test also proves a real Anthropic handoff routes through the graph-managed transfer tool and preserves instructions into the receiving agent. The prior approach added synthetic correction output after resolving the typo, but it introduced several extra passes over the same call list and did not prove the host-dispatch leakage case. This patch keeps the error in the same existing unknown-tool path and verifies the real edge cases with executable assertions.
Change Type
Testing
NODE_OPTIONS='--experimental-vm-modules' npx jest src/specs/agent-handoffs.test.ts --runInBand.NODE_OPTIONS='--experimental-vm-modules' npx jest src/specs/agent-handoffs.live.test.ts --runInBandto verify the live test is skipped by default.NODE_OPTIONS='--experimental-vm-modules' npx jest src/specs/agent-handoffs.live.test.ts src/specs/agent-handoffs.test.ts --runInBand.npm run test:live:handoffswith.envcredentials; the live Anthropic handoff passed withclaude-sonnet-4-6.npx tsc --noEmit.npx eslint src/tools/ToolNode.ts src/specs/agent-handoffs.test.ts.npx eslint src/specs/agent-handoffs.live.test.ts.git diff --check.claude-sonnet-4-6; it returnedhandoff: true,toolNames: ["lc_transfer_to_specialist"], andspecialistReplied: true.Test Configuration:
origin/devatda06b06ada4b6fe3c12757fa2084b445e84b7197v20.19.510.8.2claude-sonnet-4-6Checklist