Expose thread-level multi-agent mode#28792
Conversation
511fa89 to
1f54314
Compare
f0b05d4 to
5a272c7
Compare
1f54314 to
5d78618
Compare
5a272c7 to
a113393
Compare
5d78618 to
ddb088f
Compare
a113393 to
180fa9b
Compare
ddb088f to
a50ab75
Compare
180fa9b to
07d718f
Compare
a50ab75 to
ec2f4a9
Compare
f4d3cce to
c992e15
Compare
ec2f4a9 to
9ac7c38
Compare
c992e15 to
6a0aad7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a0aad7247
ℹ️ 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".
| let multi_agent_mode = | ||
| initial_multi_agent_mode.or_else(|| conversation_history.get_multi_agent_mode()); |
There was a problem hiding this comment.
Preserve selected mode across cold resume
When thread/start.multiAgentMode is supplied, this only seeds the live session; cold resume reconstructs the setting from conversation_history.get_multi_agent_mode(), which reads the persisted effective turn mode rather than the selected thread mode. In inspected app-server resume/fork paths, this means a thread started with proactive can later resume as null if it had no turns or as explicitRequestOnly/null when the selected mode was not applicable or was downgraded for the turn, so clients cannot recover the selected thread-level mode the new lifecycle API promises. Persist and restore the selected mode separately from the effective model-visible mode.
AGENTS.md reference: AGENTS.md:L103-L110
Useful? React with 👍 / 👎.
| forked_from_thread_id, | ||
| ) | ||
| .await; | ||
| let inherited_multi_agent_mode = match Self::inherited_thread_id_for_spawn( |
There was a problem hiding this comment.
This also runs for InitialHistory::Resumed. For a thread-spawn child the helper picks the live parent, so a residency reload replaces the child’s own persisted mode with the parent’s current selection
There was a problem hiding this comment.
Updated this so that we push the responsibility for determining the multi_agent_mode on spawn the the initiator.
| }, | ||
| None => None, | ||
| }; | ||
| let initial_multi_agent_mode = initial_multi_agent_mode.or(inherited_multi_agent_mode); |
There was a problem hiding this comment.
We lose an authoritative inherited None here
## Why Clients can select multi-agent mode through `thread/start` and `turn/start`, but they cannot currently change a loaded thread's selection without starting a turn. `thread/settings/update` already exists for changing sticky next-turn settings independently of model execution, so multi-agent mode should use the same path. ## What changed - Add the optional experimental `thread/settings/update.multiAgentMode` field with `explicitRequestOnly` and `proactive` values. - Pass the selected mode through the existing thread-settings update pipeline so it becomes the loaded session's selection for subsequent turns. - Return the updated selection through the existing `thread/settings/updated` notification without starting a model request or adding transcript items. - Document the new request field in the app-server API overview. - Add end-to-end coverage verifying that a proactive settings update is reported immediately and, between two eligible turns, appends exactly one proactive developer update while retaining the prior explicit baseline without duplication. Omitting `multiAgentMode` leaves the loaded session's selection unchanged. As with `turn/start`, this API does not currently provide a distinct operation for clearing an existing selection back to unset. ## Verification - `CARGO_INCREMENTAL=0 just test -p codex-app-server-protocol` - `CARGO_INCREMENTAL=0 just test -p codex-app-server thread_settings_update` - `CARGO_INCREMENTAL=0 just test -p codex-app-server thread_settings_update_multi_agent_mode_applies_to_future_turns` - `CARGO_INCREMENTAL=0 just fix -p codex-app-server-protocol -p codex-app-server` - `just fmt` ## Stack Stacked on #28792, which adds thread-start initialization and lifecycle/settings observability for multi-agent mode. The underlying per-turn mode behavior is introduced in #28685.
8798467 to
4fe4b98
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fe4b982df
ℹ️ 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".
| | RolloutItem::Compacted(_) | ||
| | RolloutItem::EventMsg(_) => None, | ||
| }) | ||
| .and_then(|turn_context| turn_context.multi_agent_mode) |
There was a problem hiding this comment.
Persist selected multi-agent mode across resumes
Because this restores the thread’s selected mode only from the latest persisted TurnContext, any explicit selection made by thread/start or thread/settings/update is lost if the thread is unloaded before another turn writes a context item; conversely, an unselected thread that merely ran an eligible turn can be resumed as if explicitRequestOnly had been selected. In those resume-from-rollout cases, thread/resume reports the wrong multiAgentMode and the next turn may run with the wrong selected baseline, so the selected value needs to be persisted separately from the effective per-turn value.
AGENTS.md reference: AGENTS.md:L103-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
that is okay - we want to resume on the last effective multi agent mode based on last turn context.
4fe4b98 to
d035949
Compare
## Why Multi-agent v2 currently carries an explicit-request-only delegation rule in its static usage hint. That provides a safe default, but it prevents clients from selecting proactive delegation per turn without changing static guidance or rewriting prior model context. This change makes delegation mode a session selection that can be updated through `turn/start`, while deriving the effective model-visible mode separately for each turn. Eligible multi-agent v2 turns remain explicit-request-only unless proactive mode is both selected and enabled. ## What changed - Add the experimental `turn/start.multiAgentMode` parameter with `explicitRequestOnly` and `proactive` values. Omission retains the loaded session's current optional selection. - Add the default-off `features.multi_agent_mode` feature gate. Eligible multi-agent v2 turns use the selected mode when enabled; an unset selection or disabled gate resolves to `explicitRequestOnly`. - Treat mode prompting as inapplicable for multi-agent v1 and other unsupported session configurations, producing no multi-agent mode developer message rather than rejecting the turn. - Move the explicit-request-only rule out of the static v2 usage hint and into a bounded, tagged developer context fragment. - Emit the effective mode in initial context and only when that effective mode changes on later turns. - Persist the effective mode in `TurnContextItem` as the durable baseline for resume and context-update comparisons. Historical rollout items are not rewritten. Later mode developer messages establish the current rule incrementally. ## Not covered - Initial selection through `thread/start` and selected-mode reporting from thread lifecycle/settings APIs; those are isolated in the stacked #28792. - A TUI control or slash command for selecting the mode. - Persisting a preferred mode to `config.toml`; selection remains session/turn scoped. - Changes to multi-agent concurrency limits, tool availability, or model catalog capability declarations. - Rewriting historical rollout prompt items. Cold resume restores the latest persisted effective mode when available while leaving historical developer messages intact. ## Verification - `CARGO_INCREMENTAL=0 just test -p codex-core multi_agent_mode` - Focused app-server coverage verifies that `turn/start.multiAgentMode` produces proactive developer instructions for an eligible v2 turn. ## Stack Followed by #28792, which adds `thread/start` initialization and lifecycle/settings observability.
## Why Clients can select multi-agent mode through `thread/start` and `turn/start`, but they cannot currently change a loaded thread's selection without starting a turn. `thread/settings/update` already exists for changing sticky next-turn settings independently of model execution, so multi-agent mode should use the same path. ## What changed - Add the optional experimental `thread/settings/update.multiAgentMode` field with `explicitRequestOnly` and `proactive` values. - Pass the selected mode through the existing thread-settings update pipeline so it becomes the loaded session's selection for subsequent turns. - Return the updated selection through the existing `thread/settings/updated` notification without starting a model request or adding transcript items. - Document the new request field in the app-server API overview. - Add end-to-end coverage verifying that a proactive settings update is reported immediately and, between two eligible turns, appends exactly one proactive developer update while retaining the prior explicit baseline without duplication. Omitting `multiAgentMode` leaves the loaded session's selection unchanged. As with `turn/start`, this API does not currently provide a distinct operation for clearing an existing selection back to unset. ## Verification - `CARGO_INCREMENTAL=0 just test -p codex-app-server-protocol` - `CARGO_INCREMENTAL=0 just test -p codex-app-server thread_settings_update` - `CARGO_INCREMENTAL=0 just test -p codex-app-server thread_settings_update_multi_agent_mode_applies_to_future_turns` - `CARGO_INCREMENTAL=0 just fix -p codex-app-server-protocol -p codex-app-server` - `just fmt` ## Stack Stacked on #28792, which adds thread-start initialization and lifecycle/settings observability for multi-agent mode. The underlying per-turn mode behavior is introduced in #28685.
d035949 to
964fbd4
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 964fbd4305
ℹ️ 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".
| RolloutItem::SessionMeta(_) | ||
| | RolloutItem::ResponseItem(_) | ||
| | RolloutItem::InterAgentCommunication(_) | ||
| | RolloutItem::Compacted(_) | ||
| | RolloutItem::EventMsg(_) => None, |
There was a problem hiding this comment.
Preserve selected multi-agent mode on cold resume
When a client sets multiAgentMode via thread/start or via thread/settings/update and the thread unloads before another turn writes a TurnContext, cold resume/fork reconstructs initial_multi_agent_mode only from turn contexts and ignores the persisted settings event/session metadata. In that scenario thread/resume reports null or an older mode and the first post-resume turn runs with the wrong thread-level setting.
AGENTS.md reference: AGENTS.md:L103-L110
Useful? React with 👍 / 👎.
| let multi_agent_mode = crate::session::multi_agents::effective_multi_agent_mode( | ||
| turn.multi_agent_version, | ||
| &turn.config.multi_agent_v2, | ||
| &turn.session_source, | ||
| turn.multi_agent_mode, |
There was a problem hiding this comment.
Inherit selected mode instead of effective prompt mode
This stores the effective model-visible mode in SpawnAgentOptions::initial_multi_agent_mode, but that field seeds the child thread's selected mode. When the parent has no selected mode, eligible v2 turns default to explicitRequestOnly, so every child is reported as explicitly selected instead of null; when the parent selected proactive but the mode feature is disabled, the child is downgraded to explicitRequestOnly. Pass the selected turn.multi_agent_mode here and let the child compute its own effective prompt mode.
Useful? React with 👍 / 👎.
| /// v2 turns still default to `explicitRequestOnly`. | ||
| #[experimental("thread/start.multiAgentMode")] | ||
| #[ts(optional = nullable)] | ||
| pub multi_agent_mode: Option<MultiAgentMode>, |
There was a problem hiding this comment.
Split the multi-agent rollout into stages
This commit is 656 changed lines across 46 files, and the edits are not mechanical: the option is threaded through protocol/schema/docs, app-server start/resume/fork/settings behavior, core spawn/resume/fork inheritance, analytics/TUI call sites, and test suites. Because that is a complex logic change over the 500-line target, split it into smaller reviewable stages, such as landing settings/API plumbing separately from AgentControl/ThreadManager lifecycle inheritance.
AGENTS.md reference: AGENTS.md:L125-L131
Useful? React with 👍 / 👎.
## Why Multi-agent v2 currently carries an explicit-request-only delegation rule in its static usage hint. That provides a safe default, but it prevents clients from selecting proactive delegation per turn without changing static guidance or rewriting prior model context. This change makes delegation mode a session selection that can be updated through `turn/start`, while deriving the effective model-visible mode separately for each turn. Eligible multi-agent v2 turns remain explicit-request-only unless proactive mode is both selected and enabled. ## What changed - Add the experimental `turn/start.multiAgentMode` parameter with `explicitRequestOnly` and `proactive` values. Omission retains the loaded session's current optional selection. - Add the default-off `features.multi_agent_mode` feature gate. Eligible multi-agent v2 turns use the selected mode when enabled; an unset selection or disabled gate resolves to `explicitRequestOnly`. - Treat mode prompting as inapplicable for multi-agent v1 and other unsupported session configurations, producing no multi-agent mode developer message rather than rejecting the turn. - Move the explicit-request-only rule out of the static v2 usage hint and into a bounded, tagged developer context fragment. - Emit the effective mode in initial context and only when that effective mode changes on later turns. - Persist the effective mode in `TurnContextItem` as the durable baseline for resume and context-update comparisons. Historical rollout items are not rewritten. Later mode developer messages establish the current rule incrementally. ## Not covered - Initial selection through `thread/start` and selected-mode reporting from thread lifecycle/settings APIs; those are isolated in the stacked openai#28792. - A TUI control or slash command for selecting the mode. - Persisting a preferred mode to `config.toml`; selection remains session/turn scoped. - Changes to multi-agent concurrency limits, tool availability, or model catalog capability declarations. - Rewriting historical rollout prompt items. Cold resume restores the latest persisted effective mode when available while leaving historical developer messages intact. ## Verification - `CARGO_INCREMENTAL=0 just test -p codex-core multi_agent_mode` - Focused app-server coverage verifies that `turn/start.multiAgentMode` produces proactive developer instructions for an eligible v2 turn. ## Stack Followed by openai#28792, which adds `thread/start` initialization and lifecycle/settings observability.
jif-oai
left a comment
There was a problem hiding this comment.
tbh I'm still a bit skeptical about the Option<MultiAgentMode> as it represents some blurry states. But this is a larger discussion we can have later if this needs to land fast
I’d rather persist one thread-owned selected_mode, use an explicit Keep/Clear/Set (or equivalent) patch for updates, and derive effective_mode only when constructing a turn
Anyway, pre-approving after my small nits
| fork_mode: args.fork_context.then_some(SpawnAgentForkMode::FullHistory), | ||
| parent_thread_id: Some(session.thread_id), | ||
| environments: Some(turn.environments.to_selections()), | ||
| initial_multi_agent_mode: None, |
There was a problem hiding this comment.
OOC why not passing turn.multi_agent_mode through these paths too?
I think this only break backward compatibility though
| /// Select the multi-agent mode for subsequent turns. | ||
| #[experimental("thread/settings/update.multiAgentMode")] | ||
| #[ts(optional = nullable)] | ||
| pub multi_agent_mode: Option<MultiAgentMode>, |
There was a problem hiding this comment.
None is a meaningful lifecycle state in the design, but here it merge it with omission. This feels odd to me?
Why
Once multi-agent mode can be selected per turn, clients also need to choose the initial selection when creating a thread and observe that selection through lifecycle and settings APIs.
The selected value is intentionally distinct from the effective model-visible value: no client selection is represented as
null, even though an eligible multi-agent v2 turn derivesexplicitRequestOnlyas its effective default.What changed
thread/start.multiAgentModeparameter and pass it through thread creation.explicitRequestOnly.thread/startselection to the first turn through the session configuration established at thread creation.multiAgentModefromthread/start,thread/resume,thread/fork, and thread settings, usingnullwhen no mode is selected.Not covered
turn/start; omitted ornullcurrently retains the session's selection.config.tomlpreference.Verification
CARGO_INCREMENTAL=0 just test -p codex-app-server-protocolCARGO_INCREMENTAL=0 just test -p codex-app-server multi_agent_modeThe focused app-server coverage verifies explicit
thread/startinitialization, first-turn prompting, nullable reporting for an omitted selection, and retention of selections that are not currently runtime-eligible.Stack
Stacked on #28685. This PR contains only the thread initialization and lifecycle/settings API layer.