🧷 fix: Dedupe Agent Tool Bindings#179
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea820edc3
ℹ️ 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 (seenNames.has(name)) { | ||
| duplicateNames.add(name); | ||
| continue; |
There was a problem hiding this comment.
Keep dedupe precedence aligned with runtime tool resolution
This dedupe path keeps the first tool for a duplicate name, but runtime execution still resolves duplicates by Map overwrite semantics (later entries win) in the tool-node initialization path (Graph.initializeTools builds maps from arrays, and ToolNode executes via toolMap.get(call.name)). That means when duplicate names exist, the model is bound with one tool schema while execution can run a different implementation, causing wrong routing or argument/schema mismatches at call time. Please make binding dedupe use the same precedence as runtime resolution (or dedupe both binding list and execution map from one shared source).
Useful? React with 👍 / 👎.
Summary
I added a generic duplicate-name guard in
AgentContextso duplicate tool bindings are deduped before reaching the model, with logging as a safety net. Related to danny-avila/LibreChat#13162.web_searchbinding where the schema-only LibreChat tool wins over the appended native instance.Change Type
Testing
git diff --check HEAD~1..HEADnpx --no-install jest src/agents/__tests__/AgentContext.test.ts --runInBandbecause this worktree is missingts-jest.Test Configuration:
/Users/danny/.codex/worktrees/0ae6/agentsChecklist