[codex] Add model multi-agent system overlay#25155
Conversation
d0bfbd8 to
8f6c1ed
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6c1eda81
ℹ️ 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".
| } | ||
| }) | ||
| inherited_multi_agent_version | ||
| .or_else(|| model_info.and_then(|model_info| model_info.multi_agent_version)) |
There was a problem hiding this comment.
Honor model-selected V2 concurrency limits
When a backend model sets multi_agent_version = V2 while the local Feature::MultiAgentV2 flag is disabled, this path now enables the V2 tool surface, but Config::load still derives agent_max_threads only from the feature flag (config/mod.rs uses the legacy agents.max_threads/default in that case). Since spawn_agent enforces config.agent_max_threads via reserve_spawn_slot, these model-selected V2 sessions can advertise multi_agent_v2.max_concurrent_threads_per_session but still reject spawns at the legacy cap, e.g. a model-marked V2 session with the feature disabled and a configured cap of 17 is limited to the default 6 children.
Useful? React with 👍 / 👎.
| &config, | ||
| &session_source, | ||
| history.get_multi_agent_version(), | ||
| /*model_info*/ None, | ||
| ); |
There was a problem hiding this comment.
Use model selectors for interrupted forks
For interrupted forks whose stored history does not already contain multi_agent_version (for example legacy rollouts), this pre-spawn resolution cannot see a backend model marked as V2 when the local MultiAgentV2 flag is off, so fork_history_from_snapshot appends the legacy contextual-user interruption marker. The new session is then initialized through Codex::spawn, where the same model metadata can resolve the thread to V2, leaving a V2 thread with the wrong interrupted-turn marker in its model context; the live V2 interrupt path uses the developer marker instead.
Useful? React with 👍 / 👎.
| &config, | ||
| &session_source, | ||
| stored_thread.multi_agent_version, | ||
| /*model_info*/ None, |
There was a problem hiding this comment.
Avoid legacy watchers for model-selected V2 resumes
When resuming a stored child thread that predates persisted multi_agent_version, this resolution ignores the child model metadata, so a backend-marked V2 child with the local V2 feature disabled is treated as non-V2 here and gets maybe_start_completion_watcher attached. The resumed session itself can still resolve to V2 during Codex::spawn, so final turns are reported both by the V2 terminal-turn forwarding path and by this legacy watcher, causing duplicate completion messages to the parent for those resumed subagents.
Useful? React with 👍 / 👎.
8f6c1ed to
72b1232
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72b1232ae9
ℹ️ 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".
| } | ||
| }) | ||
| inherited_multi_agent_version | ||
| .or_else(|| model_info.and_then(|model_info| model_info.multi_agent_version)) |
There was a problem hiding this comment.
Keep V1-selected sessions on V1 thread limits
When a backend model selects multi_agent_version = V1 while Feature::MultiAgentV2 is enabled, this resolves the session to the V1 tool surface but leaves Config::load having derived config.agent_max_threads from features.multi_agent_v2.max_concurrent_threads_per_session and rejecting agents.max_threads; spawn_agent then enforces that mismatched V2-derived cap via reserve_spawn_slot. In that configuration a model intentionally pinned to V1 cannot use the legacy V1 concurrency setting and may allow/reject spawns according to the wrong limit, even though the tools and prompts are V1.
Useful? React with 👍 / 👎.
d617fd1 to
88f1739
Compare
e7021b0 to
f32ec83
Compare
Why
New root threads need a constrained catalog selector for the multi-agent system without expanding
Config. PR #25168 owns the thread-scoped session lock; this PR adds one catalog input when that lock is initially chosen.What changed
ModelInfo.multi_agent_versionwith knownv1andv2values.None, so they follow the existing flag fallback.Verification
Added protocol coverage for omitted, known, and unknown values. Added request-boundary integration coverage for explicit catalog overrides and omitted or unknown fallback, plus focused coverage that inherited parent state wins over conflicting catalog metadata. Ran
just fmt. Validation is running in online CI.Stack
Supersedes #25032.