fix(mcp): let an eval cell use a searchable tool that is not active yet - #408
Merged
Conversation
…on demand executeTool() resolved a tool name against the ACTIVE set and, on a miss, consulted the definition registry only to pick between `inactive_tool` and `unknown_tool` before throwing. An eval cell naming a registered-but-inactive tool was therefore stuck, even when the host could trivially activate it. Add registerLazyToolActivator(activator). When a name resolves to a registered but inactive tool, registered activators run before the throw; one returning true means it has actually activated the tool and execution proceeds. Core deliberately does NOT decide eligibility. The definition registry also contains permission-denied tools, MCP list_changed additions held inactive as rug-pull defense, removed-tool tombstones, and capability-gated tools such as look_at and read_video. Ownership stays with the registering extension, so an activator returning false preserves `inactive_tool` byte for byte and `unknown_tool` is untouched. Activators are stashed on the extension and replayed after bindCore(), the same way registerRemovedToolHint already handles factories running before core is bound. Plan: .omo/plans/eval-auto-activate-tools.md
Session 019fa397 ran `tool.mcp_computer_use_click({'x':826,'y':371})` in an eval
cell and got "Tool mcp_computer_use_click is registered but inactive". The
assistant then emitted empty text and stalled, and the user had to abandon eval
for the rest of the session. Moments later tool_search activated those exact
tools and the same work succeeded: search-mode exposure registers the whole
catalog but keeps only directTools active, and only tool_search could promote.
Register a lazy activator whose eligibility is the tier-B searchable catalog and
nothing else, so a cell may reach any tool tool_search could already promote.
Everything else stays inactive: permission-denied tools, list_changed additions
(rug-pull defense), removed-tool tombstones, and capability-gated look_at /
read_video are all absent from that catalog.
Activation goes through the tier-B activate() path rather than the generic
active-set setter, so stubSwap still replaces the registered stub with the full
definition; marking a stub active would have returned stub output while looking
correct in the active set.
Plan: .omo/plans/eval-auto-activate-tools.md
Lazy activation ran for every executeTool caller, so an unrelated extension calling a searchable MCP tool would activate it exactly like an eval cell did. Only code-mode needs this: a cell names tools directly and cannot run tool_search first. Gate it behind ExecuteToolOptions.activateInactiveTool, off by default. The codemode executeTool wrapper is the only caller that sets it, so every other extension keeps the `inactive_tool` contract for the very same tool. Plan: .omo/plans/eval-auto-activate-tools.md
The new _activateLazyTool landed between setActiveToolsByName's docblock and its declaration, leaving the docblock attached to the wrong member.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In session
019fa397-dbbc-7a05-baa2-6a68852ff384anevalcell ran:and got back:
The assistant then emitted empty text and stalled (msg 22). The user had to intervene — "no use eval just pure tool call plz" (msg 25) — and eval was abandoned for the rest of the session.
The tool was registered, just not active. Moments later the model called
tool_search, which activated those exact same tools, and the identical work succeeded. Search-mode MCP exposure registers the whole catalog but keeps onlydirectToolsactive, and onlytool_searchcould promote from it. That asymmetry —tool_searchmay promote, a code-mode cell may not — is the bug.Change
executeTool()resolved a name against the active set and, on a miss, consulted the definition registry only to choose betweeninactive_toolandunknown_toolbefore throwing.Add
registerLazyToolActivator(activator). When a name resolves to a registered-but-inactive tool, registered activators run before the throw; one returningtruemeans it actually activated the tool, and execution proceeds.Core deliberately does not decide eligibility.
builtin/mcpregisters an activator whose eligible set is the tier-B searchable catalog and nothing else.Why not blanket activation
A one-line "activate anything registered" change is unsafe.
_toolDefinitionsalso holds:deny *rules are applied by deactivatingpermission-system/index.ts:96-100,config.ts:71-84list_changedadditionsmcp/notifications.ts:1-9mcp/service.ts:436-439look_at/read_videolook-at/index.ts:37-48None are in the searchable catalog, so all stay inactive. Activation also routes through the tier-B
activate()path rather than the generic active-set setter, sostubSwapstill swaps the registered stub for the full definition — marking a stub active would have returned stub output while the active-set assertion looked correct.An activator returning
falsepreservesinactive_toolbyte for byte;unknown_toolis untouched.execute-tool.test.tsandlook-at-extension.test.tspass unmodified.Evidence
Real
AgentSessionthroughpi.executeTool, same tool and coordinates as the failing session:The capability-gated tool is still refused and never enters the active set.
Gates
test/mcp/+execute-tool+look-at-extension+extensions-runner+permission/: 849 passed (63 files)packages/senpi-codemode: 403 passed, 6 skipped (pre-existing interpreter skips)npx tsgo --noEmit: exit 0npm run check(root): exit 0Both criteria were captured RED first: the policy test failed on the absent module, and the session-wiring test failed with
pi.registerLazyToolActivator is not a function.Review
Reviewed by
momus, which returned REJECT / SAFE-ONLY-IF on the original blanket-activation design. Every blocker was applied: catalog-scoped eligibility, tier-Bactivate()for stub-swap, opt-in registration, and tests covering the refusal paths rather than only the happy path.Plan:
.omo/plans/eval-auto-activate-tools.mdSummary by cubic
Fixes stalled eval cells by lazily activating eligible searchable MCP tools just in time for eval. Activation is opt-in via
activateInactiveTool(enabled only for eval), and gated, tombstoned, and permission‑denied tools remain inactive.Bug Fixes
executeTool()now, whenactivateInactiveToolis true, runs registered lazy activators for registered‑but‑inactive tools and executes on success.setActiveToolsByNamedocblock after introducing_activateLazyTool.New Features
registerLazyToolActivator(activator)to the Extension API andExecuteToolOptions.activateInactiveTool. The codemode wrapper enables it for eval; other callers keepinactive_tool/unknown_toolunchanged.builtin/mcpregisters an activator that promotes only tier‑B searchable tools via itsactivate()path, preserving stub‑swap and catalog rules. Extension loader/runner persist and replay activators acrossbindCore().Written for commit 07ab203. Summary will update on new commits.