dynamic tool calls: add param exposeToContext to optionally hide tool#14501
dynamic tool calls: add param exposeToContext to optionally hide tool#14501
exposeToContext to optionally hide tool#14501Conversation
cconger
force-pushed
the
cconger/hidden-dynamic-tools
branch
from
8a39ba4 to
db304ca
Compare
codex-rs/core/src/codex.rs
Outdated
| let hidden_dynamic_tools = turn_context | ||
| .dynamic_tools | ||
| .iter() | ||
| .filter(|tool| !tool.expose_to_context) |
There was a problem hiding this comment.
we call this defer_loading elsewhere should we go with the same name?
codex-rs/core/src/codex.rs
Outdated
| .filter(|tool| !tool.expose_to_context) | ||
| .map(|tool| tool.name.as_str()) | ||
| .collect::<HashSet<_>>(); | ||
| let tools = if hidden_dynamic_tools.is_empty() { |
There was a problem hiding this comment.
do we need to filter them out or just set defer_loading on them?
There was a problem hiding this comment.
They're not callable as remote tools so I would prefer not to even expose them to responses but maybe I'm missing something here.
| .bind(tool.name.as_str()) | ||
| .bind(tool.description.as_str()) | ||
| .bind(input_schema) | ||
| .bind(tool.expose_to_context) |
There was a problem hiding this comment.
what happens when a model decides to run a stored tool that isn't registered anymore?
There was a problem hiding this comment.
Yeah I think this is an existing bug/problem with dynamic tool calls either way right now. Without a registry with lifecycles we're just trusting that there's a listener for the dynamic tool call for the resumer too.
pakrym-oai
left a comment
pakrym-oai
left a comment
There was a problem hiding this comment.
Consider renaming to defer_loading
cconger
force-pushed
the
cconger/hidden-dynamic-tools
branch
2 times, most recently
from
c54532f to
16f0e7f
Compare
This extends dynamic_tool_calls to allow us to hide a tool from the model context but still use it as part of the general tool calling runtime/harness. (For ex from js_repl/code_mode).
cconger
force-pushed
the
cconger/hidden-dynamic-tools
branch
from
16f0e7f to
f70b13c
Compare
|
@codex review |
![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: f70b13cac9
ℹ️ 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".
| router | ||
| .model_visible_specs() | ||
| .into_iter() | ||
| .filter(|spec| !deferred_dynamic_tools.contains(spec.name())) |
There was a problem hiding this comment.
build_prompt removes every model-visible spec whose name matches a deferred dynamic tool. If a deferred dynamic tool shares a name with a built-in/MCP tool, this line drops the unrelated tool from the prompt as well. That can silently disable expected tool access for the model, exceeding the intended “hide this dynamic tool” behavior.
Useful? React with 👍 / 👎.
2 participants
This extends dynamic_tool_calls to allow us to hide a tool from the model context but still use it as part of the general tool calling runtime (for ex from js_repl/code_mode)