Skip to content

Add namespaces to dynamic tools#18413

Draft
pash-openai wants to merge 1 commit intomainfrom
codex/fix-deferred-dynamic-tool-routing
Draft

Add namespaces to dynamic tools#18413
pash-openai wants to merge 1 commit intomainfrom
codex/fix-deferred-dynamic-tool-routing

Conversation

@pash-openai
Copy link
Copy Markdown
Collaborator

@pash-openai pash-openai commented Apr 17, 2026

Dynamic tools can now carry an explicit namespace from the app-server protocol through core registration, tool search, and dispatch. This lets deferred dynamic tools be returned from tool_search as a real namespace containing their callable function, instead of relying on Responses echoing a self-namespaced function name.

The dynamic tool handler still sends the plain tool name back to the app, so app-side dynamic tool handlers keep receiving names like automation_update while core routes the model call through the namespaced registry key.

@pash-openai pash-openai marked this pull request as ready for review April 17, 2026 23:50
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 🎉

ℹ️ 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".

@pash-openai pash-openai changed the title Route self-namespaced dynamic tools Add namespaces to dynamic tools Apr 18, 2026
@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch from 9d8f242 to ad75171 Compare April 18, 2026 01:41
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

if name == "mcp" || name.starts_with("mcp__") {
return Err(format!("dynamic tool name is reserved: {name}"));
}

P1 Badge Validate dynamic tool namespaces against reserved prefixes

validate_dynamic_tools only checks tool.name for reserved mcp prefixes and duplicate conflicts, but never validates tool.namespace. A dynamic tool can therefore use an MCP namespace/name pair and collide with real MCP tools. That key is later registered as a dynamic handler and can overwrite the existing handler, causing tool calls to be routed to the wrong backend.

ℹ️ 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".

Comment thread codex-rs/core/src/tools/handlers/dynamic.rs Outdated
@pash-openai pash-openai marked this pull request as draft April 18, 2026 01:52
defer_loading,
} = tool;
Ok(ToolDefinition {
name: name.clone(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this used for that we don't care about ns?

Comment thread codex-rs/tools/src/tool_registry_plan.rs Outdated
@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch from ad75171 to 7b3d791 Compare April 18, 2026 20:17
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b3d791662

ℹ️ 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".

Comment thread codex-rs/app-server/src/codex_message_processor.rs
Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs
@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch from 7b3d791 to 984d794 Compare April 18, 2026 20:42
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch from 984d794 to d8411aa Compare April 18, 2026 20:47
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 984d7945c7

ℹ️ 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".

Comment thread codex-rs/tools/src/tool_registry_plan.rs Outdated
@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch 5 times, most recently from 9f7fd1f to b619c18 Compare April 18, 2026 22:22
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Review result: changes requested

I re-reviewed the PR state at b619c18 and found one remaining contract bug.

1) P2 — namespace is optional in Rust/JSON Schema but required in generated TS

DynamicToolSpec.namespace is declared as Option<String> in Rust, but it is missing #[ts(optional = nullable)], so the generated TypeScript type requires the field (namespace: string | null) instead of making it optional. That can break existing TS callers that omit namespace. codex-rs/app-server-protocol/src/protocol/v2.rsL607-L617 codex-rs/app-server-protocol/schema/typescript/v2/DynamicToolSpec.tsL1-L6
JSON Schema still treats namespace as optional (required excludes it), confirming the mismatch is real at the API contract boundary. codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.jsonL3867-L3891

Notes on previously flagged items

Checks run

  • git show --stat --oneline b619c1801f961033fbfb3dfb266cde4a3fbce0d0
  • nl -ba codex-rs/app-server-protocol/src/protocol/v2.rs | sed -n '600,640p'
  • nl -ba codex-rs/app-server-protocol/schema/typescript/v2/DynamicToolSpec.ts
  • nl -ba codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json | sed -n '3860,3925p'
  • nl -ba codex-rs/app-server/src/codex_message_processor.rs | sed -n '9190,9245p'
  • nl -ba codex-rs/tools/src/tool_registry_plan.rs | sed -n '552,590p'
  • nl -ba codex-rs/tools/src/tool_registry_plan_tests.rs | sed -n '1411,1465p'

No code changes were made in this pass, so no commit/PR was created.

View task →

@pash-openai pash-openai force-pushed the codex/fix-deferred-dynamic-tool-routing branch from 61927ed to 7a8eb68 Compare April 19, 2026 04:36
@pash-openai
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a8eb680cb

ℹ️ 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".

{
tool_name.display()
} else {
tool_name.name.clone()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve namespaces in code-mode tool names

Return a fully-qualified name for non-MCP namespaced tools. Flattening to tool_name.name makes different tools share the same code-mode identifier (e.g. shell), and later dedup/lookup logic is name-based. This can hide one tool from ALL_TOOLS and route tools.<name>(...) to the wrong handler when names collide across namespaces or with built-ins.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants