[codex] Add model tool mode selector#25031
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c264fbbd2
ℹ️ 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_mode: model_info | ||
| .tool_mode | ||
| .unwrap_or_else(|| tool_mode_from_features(features)), |
There was a problem hiding this comment.
Keep metadata-driven code mode behind a context cap
When the /models payload sets tool_mode to code_mode or code_mode_only, this overrides the local CodeMode feature flags, so model metadata can now expose the codex freeform tool even in sessions where code mode was not locally enabled. In code_mode_only, build_exec_tool_description expands every visible nested tool, including MCP/extension/dynamic tool descriptions, into one model-visible tool description with no hard cap; that new individual context item can exceed the >1k-token manual-review threshold and has no 10K-token bound. Please keep CodeMode as a local gate or cap/truncate the generated description before exposing metadata-driven code mode.
Useful? React with 👍 / 👎.
| tool_mode: model_info | ||
| .tool_mode | ||
| .unwrap_or_else(|| tool_mode_from_features(features)), |
There was a problem hiding this comment.
Preserve tool_mode in lockfile replays
When config-lock replay is used with the default save_fields_resolved_from_model_catalog = true, the lockfile drops model_catalog_json after saving resolved catalog-derived fields, but this new tool_mode selector is not saved anywhere. In that replay/resume context, a later remote catalog change for the same model can make this line choose a different mode and override the lockfile’s materialized feature flags, so the same locked session can gain or lose the code-mode tool surface while validation still passes; persist the resolved selector or encode it into the locked feature state.
Useful? React with 👍 / 👎.
jif-oai
left a comment
There was a problem hiding this comment.
I maintain my comment on the other side that we probably need something more generic to control the config from the model
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2261e0814d
ℹ️ 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".
| #[serde(rename_all = "snake_case")] | ||
| pub enum ToolMode { | ||
| Direct, | ||
| CodeMode, |
There was a problem hiding this comment.
Cover the code_mode catalog selector
The new code_mode selector is a distinct wire value, but the added integration coverage only drives remote metadata through direct and code_mode_only; existing Code Mode tests still reach this path via local feature flags. A regression that deserializes or resolves tool_mode: "code_mode" incorrectly for remote models would pass the current suite, so please add a focused integration test that feeds code_mode metadata and asserts the resulting outbound Code Mode tools/worker behavior.
Useful? React with 👍 / 👎.
Why
Some models need to select their code-execution behavior through model catalog metadata. Models without that metadata must continue to follow the existing
CodeModeandCodeModeOnlyfeature flags, including when a newer server sends an enum value this client does not recognize.What changed
ModelInfo.tool_modemetadata withdirect,code_mode, andcode_mode_onlyNoneNonefrom the existing feature flagsToolModedirectly onTurnContext, outsideConfigCoverage
/v1/modelsand verifies the outbound/responsestools for explicitdirectandcode_mode_onlyselectorsStack