[codex] Fix v2 tool_info action inventory lookup - #2994
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the agent's tool discovery mechanism by removing hardcoded tool signatures from the system prompt and introducing a dynamic schema retrieval system via tool_info(name="<tool>", detail="schema"). The changes include updates to the codeact_preamble.md, enhancements to the tool_info implementation in both scripting and structured executors to support schema lookups, and improved error handling in the bridge to handle missing action snapshots. I have no feedback to provide as the review comments did not point out actionable issues in the current code changes.
There was a problem hiding this comment.
Pull request overview
Fixes v2 tool_info discovery to resolve against the current per-context action surface (ActionInventory / action snapshots) instead of falling back to the global ToolRegistry, and updates CodeAct prompt content to rely on runtime schema discovery rather than hardcoded mission signatures.
Changes:
- Update
EffectBridgeAdapter’stool_infoexecution path to preferavailable_action_inventory_snapshot, thenavailable_actions_snapshot, and error when neither is present (no ToolRegistry fallback). - Add/adjust tests across bridge + executors to cover schema lookup for engine-native / non-registry actions and prevent registry leakage.
- Remove hardcoded mission tool signatures from the CodeAct preamble and tighten prompt assertions accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/bridge/effect_adapter.rs | Switch tool_info snapshot lookup to ActionInventory-first, add regression tests to prevent ToolRegistry fallback. |
| src/bridge/action_discovery.rs | Add test helpers and new unit tests validating detail="schema" / include_schema behavior and schema overrides. |
| crates/ironclaw_engine/src/executor/structured.rs | Update test stubs/inventory to support tool_info schema resolution; add structured execution regression test. |
| crates/ironclaw_engine/src/executor/scripting.rs | Update scripting test stub + test to validate schema resolution via propagated ActionInventory snapshot. |
| crates/ironclaw_engine/src/executor/prompt.rs | Assert prompt no longer embeds hardcoded mission Python signatures. |
| crates/ironclaw_engine/prompts/codeact_preamble.md | Remove hardcoded mission_* signatures and instruct dynamic schema discovery via tool_info(..., detail="schema"). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
serrrfirat
left a comment
There was a problem hiding this comment.
Reviewed changed files in full and ran targeted tool_info test coverage locally. No blocking findings.
Summary
Fixes v2
tool_infodiscovery so it resolves against the currentActionInventorysurface instead of falling back to the globalToolRegistry. This lets engine-native actions likemission_createreturn schema details even though they are not registered builtin tools, and preventstool_infofrom explaining globally registered tools that are not callable/discoverable in the current v2 context.Also removes the hardcoded
mission_*Python signatures from the CodeAct preamble so compact tools are discovered through the runtime action surface andtool_info(..., detail="schema").Root cause
The model could follow the prompt and call
tool_info(name="mission_create", detail="schema"), but the executed path could still hit registry-backed lookup. Sincemission_createis engine-native and lives inActionInventory, notToolRegistry, that producedNo tool named 'mission_create' is registered.Validation
cargo fmtcargo test tool_info --libcargo test -p ironclaw_engine tool_info --libcargo test -p ironclaw_engine prompt_renders_compact_enabled_tools_once_with_schema_instruction --libgit diff --checkNotes
This PR intentionally keeps
mission_createoutsideToolRegistry; the v2 source of truth is the current action inventory.