fix: filter unregistered profile tools from active set and improve missing tool error#228
fix: filter unregistered profile tools from active set and improve missing tool error#228LifeJiggy wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: 2d201f1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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: 53daca0daf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 144340ff1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2d4bea8b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eabbdb0aeb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
When setActiveTools() is called before initializeBuiltinTools() (e.g. profile applied before model config lands), every builtin name was silently dropped because �uiltinTools was empty. - Save pending builtin names in pendingBuiltinToolNames instead of warning immediately when �uiltinTools.size === 0 - Reapply pending names inside initializeBuiltinTools() after the builtins map is populated; warn only for tools still genuinely missing - Reset pendingBuiltinToolNames when a subsequent setActiveTools call resolves all names
- Merge pendingBuiltinToolNames on repeated setActiveTools calls (P2) Prevents deferred names from being lost when an intermediate call (e.g. a user-tool-only activation) triggers the setActiveTools path. - Log info when pending names are replayed at builtin init (P2) - Change pendingBuiltinToolNames from private to protected (P2) - Add test for background mode via pending task tools (P2) - Add test for pending name merge across repeated calls (P2)
Replace merge with replace + builtins-not-init guard so that stale pre-init tool selections are not re-enabled when an explicit setActiveTools call replaces the tool set before provider init. Pending names survive intermediate all-available calls but are replaced (not merged) on the next call with missing tools.
When setActiveTools([]) or setActiveTools(['mcp__*']) is called before provider init, nonMcpNames is empty — the call represents an explicit replacement of the tool set. Clear pendingBuiltinToolNames so the next initializeBuiltinTools() does not re-enable tools the caller explicitly removed. User-tool-only calls (nonMcpNames.length > 0) that resolve all their names still preserve pending for a previous builtin deferral.
LifeJiggy
force-pushed
the
fix/tool-websearch-not-found-filter
branch
from
4fadcaa to
f797260
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7972606b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…tinTools calls When initializeBuiltinTools() runs and some pending tools are filtered by model capabilities (e.g. ReadMediaFile without image_in), the pending list was cleared entirely. On a later model swap that provides the capability, nothing re-adds the tool to enabledTools. Instead of clearing pending, retain the still-missing names so a subsequent initializeBuiltinTools() call can re-check and re-enable them when the capability becomes available.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22a417fd38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // When builtins.size === 0, missingTools.length === 0, and | ||
| // nonMcpNames.length > 0, the call resolved all names against | ||
| // user tools without touching builtins. Keep existing pending alive | ||
| // so a previous deferral is not silently dropped. | ||
| this.enabledTools = new Set(availableNames); |
There was a problem hiding this comment.
setActiveTools is a replacement API (callers like Agent.useProfile pass the full profile.tools, and before this change it assigned the active set directly), but this branch keeps old pendingBuiltinToolNames when the new pre-init active set contains only already-registered user tools. If a provider-less agent first defers Bash from one profile and then switches to an active set like ['UserTool'], initializeBuiltinTools() later re-adds Bash even though the latest active set removed it. Clear the pending builtins whenever a replacement call has no missing names, not only for empty/MCP-only calls.
Useful? React with 👍 / 👎.
…set (Codex P2) setActiveTools is a replacement API — each call expresses the complete desired tool set. The previous three-way guard kept old pending names alive when the replacement resolved entirely against user tools (e.g. switching from ['Bash', 'TaskList'] to ['UserTool'] before init). This caused initializeBuiltinTools() to re-enable tools from the stale profile. Simplified the guard: pending is replaced when missingTools.length > 0 (profile requested unavailable tools), and cleared in all other cases (replacement fully resolved, no stale names to carry forward).
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Related Issue
Resolve #127
Problem
When a profile includes tools that require service configuration (e.g.
WebSearchrequiresmoonshotSearch), self-hosted users without that service see confusing "Tool not found" errors at runtime. The unavailable tools also appear in the LLM's tool list, wasting context and causing failed calls.What changed
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.