fix: universal engine-version tool visibility filtering - #2132
Merged
Merged
Conversation
Add EngineCompatibility enum (Both/V1Only/V2Only) to the Tool trait so each tool declares which engine versions it supports. The ToolRegistry gains tool_definitions_for_engine() which filters at the source. This replaces the ad-hoc is_v1_only_tool() / is_v1_auth_tool() string- matching in effect_adapter.rs with a unified, tool-declared mechanism. Changes: - 14 tools marked V1Only (routine_*, create_job, cancel_job, build_software, tool_auth, tool_permission_set, tool_remove, skill_remove) - v2 available_actions() and capability registry use V2Only filtering - v1 dispatcher uses V1Only filtering - Execute-time guard replaced with dynamic engine_compatibility() check - 3 new regression tests in registry.rs Fixes repeated "no lease for action 'routine_create'" and "lease denied: Tool 'tool_permission_set' requires explicit approval" errors when creating routines/missions in engine v2 mode. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace EngineCompatibility parameter with separate EngineVersion enum (V1/V2) for tool_definitions_for_engine() to avoid API footgun where passing Both would confusingly exclude version-specific tools - Improve v2 rejection error message with actionable guidance - Remove legacy is_v1_only_tool()/is_v1_auth_tool() helpers and their tests — production code now uses Tool::engine_compatibility() and the old helpers were out of sync (phantom tools, incomplete set) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Store engine version on ToolRegistry (default V1, set at startup via is_engine_v2_enabled()). All tool visibility surfaces now auto-filter: - tool_definitions() delegates to tool_definitions_for_engine() - all() filters by engine version (affects tool_list, settings UI) - tool_info rejects incompatible tools - tool_definitions_excluding() and tool_definitions_for_domain() filter - Callers simplified back to tool_definitions() — no explicit version This ensures V1Only tools (routine_*, job tools, tool_auth, etc.) are invisible in v2 mode across function schemas, tool_list, tool_info, system prompts, and web API responses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
scope: tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements engine versioning for tools, allowing the ToolRegistry to filter tools based on their compatibility with Engine V1 or V2. It replaces manual filtering logic in the bridge with a centralized engine_compatibility property on the Tool trait and updates various built-in tools accordingly. Review feedback suggests refactoring the ToolRegistry initialization in app.rs to reduce duplication and moving the compatibility check logic into the EngineCompatibility enum to avoid redundant implementations across the codebase.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a trait-based engine compatibility mechanism so tool discovery/visibility automatically filters tools based on whether the app is running engine v1 or v2, eliminating ad-hoc name matching and adding a defense-in-depth runtime guard for v2 execution.
Changes:
- Added
EngineCompatibility(Both/V1Only/V2Only) to theTooltrait andEngineVersionto the registry for centralized filtering. - Updated
ToolRegistryvisibility surfaces (tool_definitions*,all(), domain/excluding variants) to auto-filter by the registry’s configured engine version. - Marked specific tools as
V1Onlyand replaced string-matching v1-only checks in the v2 effect adapter with a trait-based execute-time guard.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/tool.rs |
Introduces EngineCompatibility/EngineVersion and adds Tool::engine_compatibility() with a default of Both. |
src/tools/registry.rs |
Stores engine_version and applies engine-based filtering to tool visibility and tool definitions; adds tests for filtering behavior. |
src/tools/mod.rs |
Re-exports the new enums. |
src/tools/builtin/tool_info.rs |
Rejects tool_info queries for tools incompatible with the active engine version. |
src/tools/builtin/skill_tools.rs |
Marks skill_remove as V1Only. |
src/tools/builtin/routine.rs |
Marks routine tools and event_emit as V1Only. |
src/tools/builtin/job.rs |
Marks create_job and cancel_job as V1Only. |
src/tools/builtin/extension_tools.rs |
Marks select extension/tool management tools as V1Only (e.g., tool_auth, tool_remove, tool_permission_set). |
src/tools/builder/core.rs |
Marks build_software as V1Only. |
src/bridge/router.rs |
Notes that capability registry building uses auto-filtered tool definitions. |
src/bridge/effect_adapter.rs |
Replaces name-matching exclusion with engine_compatibility()-based v1-only rejection at execute time (defense-in-depth). |
src/app.rs |
Sets ToolRegistry engine version at startup based on engine v2 enablement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Consolidate engine visibility logic into EngineCompatibility::is_visible_in() method, removing duplicate is_compatible() and simplifying is_engine_visible() - Filter list() by engine version for universal coverage - Refactor app.rs registry builder to reduce duplication - Add test: tool_info rejects V1Only tools in V2 registry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size: XL
nickpismenkov
approved these changes
Apr 8, 2026
This was referenced Apr 8, 2026
Merged
Merged
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
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.
Summary
EngineCompatibilityenum (Both/V1Only/V2Only) to theTooltrait so each tool declares which engine versions it supportsEngineVersionfield toToolRegistry(default V1, set at startup) — all visibility surfaces auto-filterV1Only: routine_*, create_job, cancel_job, build_software, tool_auth, tool_permission_set, tool_remove, skill_removetool_definitions()(LLM function-calling schema)all()(tool_list output, settings UI, web API)tool_info(individual tool discovery)tool_definitions_excluding()(lightweight routine tool lists)tool_definitions_for_domain()(domain-filtered tool lists)effect_adapter.rsusestool.engine_compatibility()trait checkis_v1_only_tool()/is_v1_auth_tool()string-matching with unified trait-based mechanismtool_definitions()— no explicit engine version passing neededTest plan
registry.rs(engine filtering, auto-filtering, default V1 compat,all()filtering)cargo fmt --checkcleanENGINE_V2=true, calltool_list— should not showroutine_create,tool_auth, etc.ENGINE_V2=true, calltool_info(name: "routine_create")— should return error🤖 Generated with Claude Code