Repository navigation
feat: make V3 ToolRegistry the authoritative tool catalog - #273
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ghfind-review
Bot
added
the
review: high
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe changes make ChangesTool Catalog
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~50 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant AgentRuntime
participant ToolRegistry
participant SecurityPolicy
participant MCPServer
participant RegisteredExecutor
AgentRuntime->>ToolRegistry: Resolve tools and aliases
AgentRuntime->>SecurityPolicy: Check capability and confirmation metadata
SecurityPolicy->>ToolRegistry: Read ToolSpec policy
MCPServer->>ToolRegistry: Read schemas and descriptors
AgentRuntime->>ToolRegistry: Invoke authorized tool
ToolRegistry->>RegisteredExecutor: Execute spec executor
Merge Risk: ⚪ Minimal · up to No confirmed issue from this review prevents merging, subject to the planned maintainer confirmation and normal final checks.
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 15.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 105 functions across 21 files. (1 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
The PR satisfies the coding requirements in directly linked issue #232. R011 uses the workspace-owned ToolRegistry as the authoritative catalog, with compatibility mappings derived from registry spe… |
| Out of Scope Changes check | |
The changes remain within issue #232. Runtime, security, prompt, discovery, MCP, provider-schema, learning, workspace, and documentation changes connect existing consumers to the authoritative registr… |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies the main change: making V3 ToolRegistry the authoritative tool catalog. |
Full details: Docstring Coverage
Explanation
Docstring coverage is 15.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 105 functions across 21 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![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: d3744b0022
ℹ️ 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".
1 participant
Tool identity, capability policy, aliases and MCP schemas previously lived in separate dictionaries, while ToolRegistry was not connected to runtime tools. The active workspace now owns a registry of complete ToolSpec records. Runtime lookup, policy, prompts, discovery, MCP/provider schemas and generated documentation derive from that catalog.
Closes #232. Prerequisite #227 is merged. Only this V3 issue is included.
Requirement coverage
Compatibility and boundaries
The40 adapter tools plus memory/discovery/orchestration yield48 runtime tools. Workspace/session isolation and child orchestration exclusion remain intact. Capability, token expiry/narrowing, configuration/mode restrictions and argument-sensitive approval logic remain owned by security. CLI confirmation surfaces and TUI risk remain distinct to preserve existing gates. Graph refresh retains its read capability but declares internal writes; browser/orchestration tools remain nonparallel. Only the existing pure calculator is marked parallel-safe; no scheduling behavior changes.
AVAILABLE_TOOLS stays a mutable compatibility view: registration obtains metadata, executor replacement retains metadata, removal drops descriptors/aliases without re-binding removed runtime tools, and snapshots retain complete specs. Legacy unknown callables remain dynamic/nonparallel. Registered names and aliases preserve canonical spelling across plain/XML/DSML calls, reject case-insensitive collisions, and immediately update existing action parsing and stream filtering; provider schemas contain canonical names only.
Existing string executors, MCP object/string conversion and shell CommandResult receipt behavior are preserved. Native typed invocation and pipe-protocol removal remain #233; SDK request serialization remains #229; typed result changes remain #235. The registry validates its supported JSON-schema vocabulary and rejects unsupported keywords rather than forwarding them unchecked; it is not a general JSON-schema evaluation engine. No new dependency or persistence schema change.
Validation
0807549c00301635fd9a644de99bee4e837b5299.No paid provider calls were made. Provider-neutral schema generation is tested; live/native provider tool execution is not claimed by this catalog change. Leave this PR unmerged pending maintainer confirmation.
Summary by CodeRabbit