From 105fd1b3057fe605df2295dcd2a3dae8e95bbc8f Mon Sep 17 00:00:00 2001 From: Ryan SVIHLA Date: Fri, 28 Aug 2026 18:49:47 +0200 Subject: [PATCH] Scope session config changes to their session; mjconfig owns defaults Session settings now follow one rule: a change made anywhere in mj applies only to the session it was made in, and only /mjconfig also changes the defaults new sessions start from. - Live /model, /effort, and session-option changes are session-local: delete persist_accepted_session_config and the session_config[].models routes layer, so accepted values are never written back to config.toml and can no longer leak into new sessions or subagents. Leftover [session_config.*.models] tables in old configs parse and are ignored. - save_user_config_preserving_session_routes becomes a plain locked save_user_config: with no live write-back there is nothing to merge. - The web /mjconfig save carries the invoking session id; the server reloads that one session's auxiliary routes instead of broadcasting ReloadAuxiliaryAgents to every running session. - A save that replaces the primary still updates the invoking session's reviewer and subagent lanes: both runtime handlers re-pair auto seats against the still-running primary (new rebind_auto_subagents_for_primary) instead of bailing, and the TUI always sends the aux reload while the primary-route check gates only the switch-primary prompt. - Server clear/new re-reads saved defaults from disk via the new SavedSessionConfigReload source, so an mjconfig save made after launch reaches fresh sessions on the same runtime. Co-Authored-By: Claude Fable 5 --- docs/src/content/docs/configuration.md | 35 ++- mj-agents/src/subagent.rs | 5 +- mj-core/src/acp.rs | 313 +++++++++++++++++-------- mj-core/src/config.rs | 212 ++++------------- mj-core/src/roster.rs | 114 +++++++++ mj-core/src/side.rs | 3 +- mj-remote/src/remote.rs | 45 +++- mj-remote/src/remote_viewer.html | 8 +- mj-tui/src/settings.rs | 13 - mj-tui/src/ui.rs | 98 ++++++-- src/headless.rs | 2 +- src/main.rs | 17 +- src/remote_host.rs | 74 ++++-- 13 files changed, 596 insertions(+), 343 deletions(-) diff --git a/docs/src/content/docs/configuration.md b/docs/src/content/docs/configuration.md index 13dc1f14..98caf048 100644 --- a/docs/src/content/docs/configuration.md +++ b/docs/src/content/docs/configuration.md @@ -101,12 +101,22 @@ configures the default backing for `create_subagent`; set `model = "disabled"` | `subagents.progress_wake_minutes` | Minutes a primary parked on running subagents may go without a report before it is woken with their progress alone; default 20, `0` disables. Config file only | | `voice_auto_send` | `off` (default), `two_seconds`, `four_seconds`, `six_seconds`, or `eight_seconds`; submit a recognized voice prompt after that much detected silence | -In an active primary session, model and reasoning-effort entries in `/mjconfig` -and the `/model` and `/effort` commands update the current ACP session without -a restart when the connected agent advertises the corresponding selectors; -changes made during a turn apply after it finishes. Team and ACP routing changes -still apply to a new session. A `max_parallel` above 16 is a configuration -error, not a silently clamped value. +Session settings follow one rule: a change made anywhere in mj applies to the +session it was made in, and only `/mjconfig` also changes the defaults that +new sessions start from. Concretely: + +- `/model`, `/effort`, and the session-options picker update the current ACP + session in place (when the connected agent advertises the corresponding + selectors; changes made during a turn apply after it finishes). They are + session-local: nothing is written to the config file, and neither other + running sessions nor future sessions are affected. +- Saving `/mjconfig` updates the session it was opened from the same way, and + persists the chosen models and session options as the defaults for every + session started afterwards. Other running sessions are never touched. +- Team and ACP routing changes still apply to a new session. + +A `max_parallel` above 16 is a configuration error, not a silently clamped +value. Onboarding, the **Team** tab in `/mjconfig`, and **Shift+Tab** during a session all offer the same four configurations: @@ -254,12 +264,13 @@ between completed turns; disable them under **Appearance** or set The **Agent**, **Reviewer**, and **Subagents** tabs list the selectable session options advertised by that role's selected ACP source. Each role stores its -defaults separately. Compatible primary changes are also sent to the running -primary session when `/mjconfig` is saved; the UI calls out the active value -when it differs from the selected default. Team, reviewer, and subagent changes -apply only to sessions started later, never to ones that are already running. A -saved value that a newly selected adapter no longer advertises stays intact and -is shown as unavailable until you select a compatible value. +defaults separately. Saving `/mjconfig` sends compatible changes to the session +the panel was opened from — primary options directly, reviewer and subagent +routes via a reload — and the UI calls out the active value when it differs +from the selected default. Other running sessions keep the settings they have; +the saved defaults reach them only as new sessions start. A saved value that a +newly selected adapter no longer advertises stays intact and is shown as +unavailable until you select a compatible value. The same role-scoped defaults can be written directly in TOML: diff --git a/mj-agents/src/subagent.rs b/mj-agents/src/subagent.rs index 1ca5d5d0..6a84a4e6 100644 --- a/mj-agents/src/subagent.rs +++ b/mj-agents/src/subagent.rs @@ -1086,7 +1086,6 @@ fn spawn_subagent_runtime( mj_core::config::load_saved_session_config( &mj_core::config::default_config_path(), &role.adapter_source_id, - &role.model_id, match config.usage_seat { Seat::Primary => mj_core::config::SessionConfigSeat::Primary, Seat::Subagent => mj_core::config::SessionConfigSeat::Subagent, @@ -1114,8 +1113,10 @@ fn spawn_subagent_runtime( fs_max_text_bytes: context.fs_max_text_bytes, access_mode: context.access_mode, agent_source_id, - config_path: Some(mj_core::config::default_config_path()), saved_session_config, + // Delegated lanes are one-shot runtimes: every launch reloads the + // saved defaults above, and clear/new is never sent to them. + saved_session_config_reload: None, role_config, subagents: None, memory, diff --git a/mj-core/src/acp.rs b/mj-core/src/acp.rs index 0f3f1302..27eb4335 100644 --- a/mj-core/src/acp.rs +++ b/mj-core/src/acp.rs @@ -54,6 +54,21 @@ pub enum SessionRestoreMode { Replay, } +/// Where a runtime re-reads its seat's saved `/mjconfig` session-option +/// defaults when it starts a fresh session mid-run. +#[derive(Debug, Clone)] +pub struct SavedSessionConfigReload { + pub config_path: PathBuf, + pub source_id: String, + pub seat: crate::config::SessionConfigSeat, +} + +impl SavedSessionConfigReload { + fn load(&self) -> HashMap { + crate::config::load_saved_session_config(&self.config_path, &self.source_id, self.seat) + } +} + pub struct AcpRuntimeConfig { pub command: PathBuf, pub args: Vec, @@ -81,12 +96,16 @@ pub struct AcpRuntimeConfig { pub fs_max_text_bytes: u64, /// Host capabilities exposed to the agent for this runtime. pub access_mode: RuntimeAccessMode, - /// Stable configured agent id used for per-agent session-config memory. + /// Stable configured agent id ("codex-acp", ...) identifying the adapter + /// this runtime launched. pub agent_source_id: Option, - /// Config file to update when a prompt snapshots current session options. - pub config_path: Option, - /// Values remembered from the last prompt submitted for this agent. + /// Saved `/mjconfig` session-option defaults applied to fresh sessions. pub saved_session_config: HashMap, + /// Where to re-read those defaults when this runtime starts another + /// fresh session mid-run (the server's clear/new flow), so a `/mjconfig` + /// save made after launch reaches new starts. Absent, the defaults + /// captured at launch are reused. + pub saved_session_config_reload: Option, /// Seat configuration applied before the first substantive prompt. pub role_config: Option, /// Optional model-visible subagent MCP service. Interactive TUI sessions @@ -1274,9 +1293,8 @@ pub async fn run( fatal_emitted.clone(), cfg.fs_max_text_bytes, cfg.access_mode, - cfg.agent_source_id.clone(), - cfg.config_path.clone(), cfg.saved_session_config.clone(), + cfg.saved_session_config_reload.clone(), cfg.role_config.clone(), cfg.subagents.clone(), cfg.memory.clone(), @@ -1986,12 +2004,11 @@ where fatal_emitted, DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), None, None, None, + None, false, None, ) @@ -2022,12 +2039,11 @@ where fatal_emitted, DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), None, None, None, + None, false, None, ) @@ -2059,12 +2075,11 @@ where fatal_emitted, DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), None, None, None, + None, false, None, ) @@ -2084,9 +2099,8 @@ async fn drive_client_with_fs_limit( fatal_emitted: Arc, fs_max_text_bytes: u64, access_mode: RuntimeAccessMode, - agent_source_id: Option, - config_path: Option, saved_session_config: HashMap, + saved_session_config_reload: Option, role_config: Option, subagents: Option>, memory: Option, @@ -2352,9 +2366,8 @@ where drive_terminals, access_mode, fs_max_text_bytes, - agent_source_id, - config_path, saved_session_config, + saved_session_config_reload, role_config, subagents, memory, @@ -2401,9 +2414,8 @@ async fn drive_session( terminals: Arc, access_mode: RuntimeAccessMode, fs_max_text_bytes: u64, - agent_source_id: Option, - config_path: Option, saved_session_config: HashMap, + saved_session_config_reload: Option, role_config: Option, subagents: Option>, memory: Option, @@ -2814,9 +2826,6 @@ async fn drive_session( ui_rx, &mut deferred_prompts, &mut deferred_config_updates, - config_path.as_deref(), - agent_source_id.as_deref(), - role_config.as_ref().map(|role| role.model_id.as_str()), ) .await? { @@ -2857,6 +2866,13 @@ async fn drive_session( tokio::task::spawn_blocking(move || session_memory.synchronize_native()) .await; } + // A fresh session is a new start: re-read the `/mjconfig` + // defaults from disk so a save made after this runtime + // launched applies, instead of replaying the launch snapshot. + let fresh_session_defaults = saved_session_config_reload + .as_ref() + .map(SavedSessionConfigReload::load) + .unwrap_or_else(|| saved_session_config.clone()); match start_fresh_session( &conn, &session_id, @@ -2865,7 +2881,7 @@ async fn drive_session( &mcp_servers, &init_resp.auth_methods, role_config.as_ref(), - &saved_session_config, + &fresh_session_defaults, &session_state, &terminals, &hidden_config_ids, @@ -5583,23 +5599,13 @@ async fn drive_config_update( ui_rx: &mut mpsc::UnboundedReceiver, deferred_prompts: &mut VecDeque<(String, Vec, Vec)>, deferred_config_updates: &mut VecDeque<(SessionConfigTarget, SessionConfigValueId)>, - config_path: Option<&Path>, - agent_source_id: Option<&str>, - model_id: Option<&str>, ) -> Result { - let persistable = session_config - .options - .iter() - .zip(session_config.targets.iter()) - .find(|(_, candidate)| *candidate == &target) - .is_some_and(|(option, candidate)| session_config_option_is_persistable(option, candidate)); let update = send_config_update(conn, session_id, target.clone(), value.clone()); tokio::pin!(update); loop { tokio::select! { result = &mut update => { - let accepted = result.is_ok(); match result { Ok(Some(options)) => { session_config.targets = config_option_targets(&options); @@ -5629,22 +5635,9 @@ async fn drive_config_update( ))); } } - if accepted - && persistable - && let (Some(path), Some(source_id), Some(model_id)) = - (config_path, agent_source_id, model_id) - && let Err(error) = crate::config::persist_accepted_session_config( - path, - source_id, - model_id, - session_config_target_key(&target), - value.to_string(), - ) - { - let _ = ui_tx.send(UiEvent::Warning(format!( - "session config changed but could not be saved: {error:#}" - ))); - } + // Deliberately no write-back to the config file: a live + // change belongs to this session alone. Defaults for new + // sessions change only through `/mjconfig`. return Ok(true); } maybe_cmd = ui_rx.recv() => { @@ -8729,9 +8722,8 @@ mod tests { Arc::new(AtomicBool::new(false)), DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::ReadOnly, - Some("codex-acp".to_string()), - None, HashMap::from([("config:mode".to_string(), "agent".to_string())]), + None, Some(RuntimeRoleConfig { label: "reviewer".to_string(), model_id: "model-a".to_string(), @@ -10563,9 +10555,8 @@ mod tests { Arc::new(AtomicBool::new(false)), DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), + None, Some(role_config), None, None, @@ -12472,8 +12463,11 @@ mod tests { agent_task.abort(); } + /// A live in-session change belongs to that session alone: it must apply + /// to the running ACP session and never be written back to the config + /// file, so other sessions and future sessions are unaffected. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn accepted_live_config_update_persists_for_the_exact_route() { + async fn accepted_live_config_update_is_never_persisted() { let (client_side, agent_side) = tokio::io::duplex(64 * 1024); let (cr, cw) = split(client_side); let client_transport = ByteStreams::new(cw.compat_write(), cr.compat()); @@ -12494,9 +12488,8 @@ mod tests { Arc::new(AtomicBool::new(false)), DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - Some("codex-acp".to_string()), - Some(config_path.clone()), HashMap::new(), + None, Some(RuntimeRoleConfig { label: "primary".to_string(), model_id: "model-a".to_string(), @@ -12527,38 +12520,174 @@ mod tests { value: SessionConfigValueId::new("priority"), }) .expect("send config update"); - cmd_tx - .send(UiCommand::SetSessionConfigOption { - target: SessionConfigTarget::ConfigOption { - config_id: SessionConfigId::new("response_style"), - }, - value: SessionConfigValueId::new("concise"), - }) - .expect("queue second config update"); - let deadline = tokio::time::Instant::now() + Duration::from_secs(5); - loop { - let loaded = crate::config::Config::load(&config_path).expect("load config"); - let route = loaded - .session_config - .get("codex-acp") - .and_then(|saved| saved.models.get("model-a")); - if route - .and_then(|values| values.get("config:service_tier")) - .is_some_and(|value| value == "priority") - && route - .and_then(|values| values.get("config:response_style")) - .is_some_and(|value| value == "concise") - { - break; + // Session start publishes the options once; the next publish only + // happens after the agent accepted the update. Wait for the second. + let mut options_published = 0; + while options_published < 2 { + let ev = tokio::time::timeout(EVENT_DEADLINE, ui_rx.recv()) + .await + .expect("timeout waiting for config acceptance") + .expect("channel closed"); + if matches!(ev, UiEvent::SessionConfigOptions { .. }) { + options_published += 1; } - assert!( - tokio::time::Instant::now() < deadline, - "accepted value was not persisted" - ); - tokio::time::sleep(Duration::from_millis(10)).await; } + // The accepted value stays session-local: no config file appears. + assert!( + !config_path.exists(), + "a live session change must not be written to the config file" + ); + + cmd_tx.send(UiCommand::Shutdown).expect("shutdown"); + tokio::time::timeout(EVENT_DEADLINE, client_task) + .await + .expect("client shutdown timeout") + .expect("client task") + .expect("client result"); + agent_task.abort(); + } + + async fn run_mock_agent_recording_config_updates( + stream: tokio::io::DuplexStream, + updates: Arc>>, + ) { + let (r, w) = split(stream); + let transport = ByteStreams::new(w.compat_write(), r.compat()); + let _ = AgentRole + .builder() + .on_receive_request( + async move |_req: agent_client_protocol::schema::v1::InitializeRequest, + responder, + _cx| { + responder.respond(InitializeResponse::new( + agent_client_protocol::schema::ProtocolVersion::V1, + )) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |_req: agent_client_protocol::schema::v1::NewSessionRequest, + responder, + _cx| { + responder.respond( + NewSessionResponse::new(SessionId::new("test-session")) + .config_options(slow_config_options()), + ) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |req: SetSessionConfigOptionRequest, responder, _cx| { + let value = match &req.value { + agent_client_protocol::schema::v1::SessionConfigOptionValue::ValueId { + value, + } => value.to_string(), + agent_client_protocol::schema::v1::SessionConfigOptionValue::Boolean { + value, + } => value.to_string(), + other => panic!("unexpected config option value: {other:?}"), + }; + updates + .lock() + .expect("updates lock") + .push((req.config_id.to_string(), value)); + responder.respond(SetSessionConfigOptionResponse::new(slow_config_options())) + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_with(transport, |_cx| async move { + futures::future::pending::<()>().await; + Ok(()) + }) + .await; + } + + /// A `/mjconfig` save made after the runtime launched must reach the next + /// fresh session (the server's clear/new flow): the reload source re-reads + /// the saved defaults from disk instead of replaying the launch snapshot. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn fresh_session_reloads_saved_defaults_from_disk() { + let (client_side, agent_side) = tokio::io::duplex(64 * 1024); + let (cr, cw) = split(client_side); + let client_transport = ByteStreams::new(cw.compat_write(), cr.compat()); + let updates = Arc::new(std::sync::Mutex::new(Vec::new())); + let agent_task = tokio::spawn(run_mock_agent_recording_config_updates( + agent_side, + updates.clone(), + )); + let config_dir = tempfile::tempdir().expect("config dir"); + let config_path = config_dir.path().join("config.toml"); + let (ui_tx, mut ui_rx) = mpsc::unbounded_channel::(); + let (cmd_tx, cmd_rx) = mpsc::unbounded_channel::(); + let client_task = tokio::spawn(drive_client_with_fs_limit( + client_transport, + std::env::temp_dir(), + Vec::new(), + Vec::new(), + None, + SessionRestoreMode::Continue, + ui_tx, + cmd_rx, + Arc::new(AtomicBool::new(false)), + DEFAULT_FS_TEXT_BYTES, + RuntimeAccessMode::Full, + HashMap::new(), + Some(SavedSessionConfigReload { + config_path: config_path.clone(), + source_id: "codex-acp".to_string(), + seat: crate::config::SessionConfigSeat::Primary, + }), + None, + None, + None, + false, + None, + )); + + while !matches!( + tokio::time::timeout(EVENT_DEADLINE, ui_rx.recv()) + .await + .expect("handshake timeout") + .expect("channel closed"), + UiEvent::SessionStarted { .. } + ) {} + assert!( + updates.lock().expect("updates lock").is_empty(), + "no defaults were saved yet, so nothing is applied at launch" + ); + + // The `/mjconfig` save happens while the runtime is already running. + let mut saved = crate::config::Config::default(); + saved + .session_config + .entry("codex-acp".to_string()) + .or_default() + .defaults + .insert("config:service_tier".to_string(), "priority".to_string()); + saved.save(&config_path).expect("save config"); + + let (responder, response) = oneshot::channel(); + cmd_tx + .send(UiCommand::NewSession { responder }) + .expect("send new session"); + assert_eq!( + tokio::time::timeout(EVENT_DEADLINE, response) + .await + .expect("new session timeout") + .expect("new session response"), + LoadSessionResult::Switched + ); + + assert!( + updates + .lock() + .expect("updates lock") + .contains(&("service_tier".to_string(), "priority".to_string())), + "the fresh session applies the defaults saved after launch" + ); + cmd_tx.send(UiCommand::Shutdown).expect("shutdown"); tokio::time::timeout(EVENT_DEADLINE, client_task) .await @@ -12712,8 +12841,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -12775,8 +12904,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -12936,8 +13065,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -13053,8 +13182,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -13085,8 +13214,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -13113,8 +13242,8 @@ mod tests { fs_max_text_bytes: DEFAULT_FS_TEXT_BYTES, access_mode: RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory: None, @@ -13713,12 +13842,11 @@ mod tests { fatal_emitted.clone(), DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), None, None, None, + None, false, Some(stderr_tail), )); @@ -13927,11 +14055,10 @@ mod tests { Arc::new(AtomicBool::new(false)), DEFAULT_FS_TEXT_BYTES, RuntimeAccessMode::Full, - None, - None, HashMap::new(), None, None, + None, Some(crate::memory::SessionMemory { store_path: store.clone(), config_path: None, diff --git a/mj-core/src/config.rs b/mj-core/src/config.rs index bc8914c4..cc1e75ef 100644 --- a/mj-core/src/config.rs +++ b/mj-core/src/config.rs @@ -42,7 +42,6 @@ const V5_CONFIG_VERSION: u32 = 5; const V6_CONFIG_VERSION: u32 = 6; /// Saved ACP session defaults are scoped to the seat that will consume them. -/// Live accepted values remain in the top-level `session_config` cache. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SessionConfigSeat { Primary, @@ -336,11 +335,10 @@ pub struct Config { #[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] pub struct AcpSessionConfig { /// Defaults chosen in `/mjconfig` for future sessions on this server. + /// Live in-session changes are deliberately never written back here: + /// they apply to that session alone. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub defaults: BTreeMap, - /// Values accepted by live sessions, keyed by configured model identity. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub models: BTreeMap>, } impl Default for Config { @@ -1747,7 +1745,6 @@ pub fn default_config_path() -> PathBuf { pub fn load_saved_session_config( path: &Path, source_id: &str, - model_id: &str, seat: SessionConfigSeat, ) -> HashMap { match Config::load(path) { @@ -1755,9 +1752,6 @@ pub fn load_saved_session_config( let mut values = HashMap::new(); if let Some(saved) = config.session_config.get(source_id) { values.extend(saved.defaults.clone()); - if let Some(route) = saved.models.get(model_id) { - values.extend(route.clone()); - } } let scoped = match seat { SessionConfigSeat::Primary => config.agent.session_defaults.get(source_id), @@ -1782,56 +1776,12 @@ pub fn load_saved_session_config( static SESSION_CONFIG_WRITE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); -pub fn save_user_config_preserving_session_routes(path: &Path, config: &mut Config) -> Result<()> { +/// Save the user config under the shared write lock so concurrent saves (the +/// TUI menu and the web `/mjconfig` page) serialize instead of interleaving. +pub fn save_user_config(path: &Path, config: &Config) -> Result<()> { let _guard = SESSION_CONFIG_WRITE_LOCK .lock() .unwrap_or_else(|error| error.into_inner()); - let latest = Config::load(path)?; - for (source_id, saved) in latest.session_config { - let changed_defaults = config - .session_config - .get(&source_id) - .map(|edited| { - edited - .defaults - .iter() - .filter(|(key, value)| saved.defaults.get(*key) != Some(*value)) - .map(|(key, _)| key.clone()) - .collect::>() - }) - .unwrap_or_default(); - if !saved.models.is_empty() { - let routes = &mut config.session_config.entry(source_id).or_default().models; - routes.clone_from(&saved.models); - for route in routes.values_mut() { - for key in &changed_defaults { - route.remove(key); - } - } - } - } - config.save(path) -} - -pub fn persist_accepted_session_config( - path: &Path, - source_id: &str, - model_id: &str, - key: String, - value: String, -) -> Result<()> { - let _guard = SESSION_CONFIG_WRITE_LOCK - .lock() - .unwrap_or_else(|error| error.into_inner()); - let mut config = Config::load(path)?; - config - .session_config - .entry(source_id.to_string()) - .or_default() - .models - .entry(model_id.to_string()) - .or_default() - .insert(key, value); config.save(path) } @@ -2897,34 +2847,50 @@ kimi = "disabled" } #[test] - fn saved_session_config_merges_server_defaults_with_model_route() { + fn saved_session_config_loads_server_defaults() { let dir = tempfile::tempdir().expect("tempdir"); let path = dir.path().join("config.toml"); let mut cfg = Config::default(); - let saved = cfg - .session_config + cfg.session_config .entry("codex-acp".to_string()) - .or_default(); - saved + .or_default() .defaults .insert("config:service_tier".to_string(), "default".to_string()); - saved - .models - .entry("model-a".to_string()) - .or_default() - .insert("config:service_tier".to_string(), "priority".to_string()); cfg.save(&path).expect("save"); assert_eq!( - load_saved_session_config(&path, "codex-acp", "model-a", SessionConfigSeat::Primary,)["config:service_tier"], - "priority" - ); - assert_eq!( - load_saved_session_config(&path, "codex-acp", "model-b", SessionConfigSeat::Primary,)["config:service_tier"], + load_saved_session_config(&path, "codex-acp", SessionConfigSeat::Primary)["config:service_tier"], "default" ); } + /// Older builds wrote live-accepted values into per-model route tables. + /// Those are session-local now: a leftover table still parses (so old + /// configs keep loading) but never reaches a new session's defaults. + #[test] + fn stale_model_route_tables_are_ignored() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("config.toml"); + std::fs::write( + &path, + format!( + r#" +version = {CONFIG_VERSION} + +[session_config.codex-acp.defaults] +"config:service_tier" = "default" + +[session_config.codex-acp.models.model-a] +"config:service_tier" = "priority" +"# + ), + ) + .expect("write"); + + let saved = load_saved_session_config(&path, "codex-acp", SessionConfigSeat::Primary); + assert_eq!(saved["config:service_tier"], "default"); + } + #[test] fn saved_session_config_keeps_role_defaults_separate() { let dir = tempfile::tempdir().expect("tempdir"); @@ -2948,126 +2914,48 @@ kimi = "disabled" cfg.save(&path).expect("save"); assert_eq!( - load_saved_session_config(&path, "codex-acp", "model-a", SessionConfigSeat::Primary,)["config:mode"], + load_saved_session_config(&path, "codex-acp", SessionConfigSeat::Primary)["config:mode"], "primary" ); assert_eq!( - load_saved_session_config(&path, "codex-acp", "model-a", SessionConfigSeat::Subagent,) - ["config:mode"], + load_saved_session_config(&path, "codex-acp", SessionConfigSeat::Subagent)["config:mode"], "subagent" ); assert_eq!( - load_saved_session_config(&path, "codex-acp", "model-a", SessionConfigSeat::Review,)["config:mode"], + load_saved_session_config(&path, "codex-acp", SessionConfigSeat::Review)["config:mode"], "review" ); } + /// A `/mjconfig` save writes the edited defaults verbatim; there is no + /// merge with live-session state because live changes are never persisted. #[test] - fn accepted_session_config_is_route_isolated_and_merge_safe() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("config.toml"); - let mut cfg = Config { - theme: TerminalThemeKind::Ansi, - ..Config::default() - }; - cfg.agent.discrete_review = false; - cfg.save(&path).expect("save initial config"); - - persist_accepted_session_config( - &path, - "codex-acp", - "model-a", - "config:service_tier".to_string(), - "priority".to_string(), - ) - .expect("persist model a"); - persist_accepted_session_config( - &path, - "codex-acp", - "model-b", - "config:service_tier".to_string(), - "economy".to_string(), - ) - .expect("persist model b"); - - let loaded = Config::load(&path).expect("load merged config"); - assert_eq!(loaded.theme, TerminalThemeKind::Ansi); - assert!(!loaded.agent.discrete_review); - assert_eq!( - loaded.session_config["codex-acp"].models["model-a"]["config:service_tier"], - "priority" - ); - assert_eq!( - loaded.session_config["codex-acp"].models["model-b"]["config:service_tier"], - "economy" - ); - } - - #[test] - fn settings_save_preserves_a_concurrent_accepted_route() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("config.toml"); - let mut editor_snapshot = Config::default(); - editor_snapshot - .session_config - .entry("codex-acp".to_string()) - .or_default() - .defaults - .insert("config:service_tier".to_string(), "default".to_string()); - editor_snapshot.save(&path).expect("save editor snapshot"); - - persist_accepted_session_config( - &path, - "codex-acp", - "model-a", - "config:service_tier".to_string(), - "priority".to_string(), - ) - .expect("persist accepted route"); - editor_snapshot.theme = TerminalThemeKind::Ansi; - save_user_config_preserving_session_routes(&path, &mut editor_snapshot) - .expect("save settings"); - - let loaded = Config::load(&path).expect("load merged config"); - assert_eq!(loaded.theme, TerminalThemeKind::Ansi); - assert_eq!( - loaded.session_config["codex-acp"].models["model-a"]["config:service_tier"], - "priority" - ); - } - - #[test] - fn changing_a_default_clears_that_key_from_saved_routes() { + fn user_config_save_round_trips_edited_defaults() { let dir = tempfile::tempdir().expect("tempdir"); let path = dir.path().join("config.toml"); let mut config = Config::default(); - let saved = config + config .session_config .entry("codex-acp".to_string()) - .or_default(); - saved + .or_default() .defaults .insert("config:service_tier".to_string(), "default".to_string()); - saved - .models - .entry("model-a".to_string()) - .or_default() - .insert("config:service_tier".to_string(), "priority".to_string()); config.save(&path).expect("save initial config"); + config.theme = TerminalThemeKind::Ansi; config .session_config .get_mut("codex-acp") .unwrap() .defaults .insert("config:service_tier".to_string(), "economy".to_string()); - save_user_config_preserving_session_routes(&path, &mut config) - .expect("save changed default"); + save_user_config(&path, &config).expect("save settings"); let loaded = Config::load(&path).expect("load config"); - assert!( - !loaded.session_config["codex-acp"].models["model-a"] - .contains_key("config:service_tier") + assert_eq!(loaded.theme, TerminalThemeKind::Ansi); + assert_eq!( + loaded.session_config["codex-acp"].defaults["config:service_tier"], + "economy" ); } diff --git a/mj-core/src/roster.rs b/mj-core/src/roster.rs index f4f495a5..410fe40c 100644 --- a/mj-core/src/roster.rs +++ b/mj-core/src/roster.rs @@ -86,6 +86,35 @@ pub fn subagent_failover_roles(roster: &Roster) -> Vec { failover_roles(initial, &available, false, &roster.subagent_acp_priority) } +/// Re-derive an `auto` subagent seat against `roster.primary`, mirroring +/// [`rebind_auto_review_for_primary`]. Explicit subagent pins are untouched. +pub fn rebind_auto_subagents_for_primary(roster: &mut Roster, config: &Config) { + if config.subagents.model != "auto" { + return; + } + let available = source_candidates(&roster.available, config.subagents.acp_source.as_deref()); + let rows = roster + .choices + .iter() + .filter(|choice| choice.ranked) + .map(|choice| Row { + model: choice.model.clone(), + reasoning_effort: None, + pass_at_1: choice.pass_at_1, + mean_cost_usd: choice.mean_cost_usd, + }) + .collect::>(); + roster.subagent_default = choose_secondary_auto( + &roster.primary, + &rows, + &available, + &config.subagents.acp_priority, + ); + if let Some(subagent_default) = roster.subagent_default.as_mut() { + subagent_default.reasoning_effort = config.subagents.reasoning_effort.clone(); + } +} + pub fn rebind_auto_review_for_primary(roster: &mut Roster, config: &Config) { if !config.agent.needs_review_route() { roster.review_supervisor = None; @@ -1841,6 +1870,91 @@ mod tests { assert_eq!(review.reasoning_effort.as_deref(), Some("high")); } + #[test] + fn auto_subagents_rebind_after_primary_is_pinned() { + let mut config = Config::default(); + config.subagents.reasoning_effort = Some("medium".to_string()); + let gpt = role("gpt-5-6-sol", 0.70); + let claude = role("claude-fable-5", 0.64); + let mut roster = Roster { + primary: gpt.clone(), + review_supervisor: None, + subagent_default: Some(claude.clone()), + available: vec![gpt, claude.clone()], + choices: vec![ + ModelChoice { + model: "gpt-5-6-sol".to_string(), + pass_at_1: 0.70, + mean_cost_usd: 1.0, + available: true, + disabled_reason: None, + adapter: Some("codex-acp".to_string()), + ranked: true, + }, + ModelChoice { + model: "claude-fable-5".to_string(), + pass_at_1: 0.64, + mean_cost_usd: 1.0, + available: true, + disabled_reason: None, + adapter: Some("claude-acp".to_string()), + ranked: true, + }, + ModelChoice { + model: "claude-sonnet-5".to_string(), + pass_at_1: 0.60, + mean_cost_usd: 7.0, + available: false, + disabled_reason: None, + adapter: None, + ranked: true, + }, + ], + warnings: Vec::new(), + inventory: AcpInventory::default(), + subagent_acp_priority: Vec::new(), + subagent_acp_source: None, + }; + + roster.primary = claude; + rebind_auto_subagents_for_primary(&mut roster, &config); + + let subagent = roster.subagent_default.expect("subagent rebound"); + assert_eq!(subagent.model.model, "gpt-5-6-sol"); + assert_eq!(subagent.reasoning_effort.as_deref(), Some("medium")); + } + + #[test] + fn explicit_subagents_are_not_rebound_after_primary_is_pinned() { + let mut config = Config::default(); + config.subagents.model = "claude-fable-5".to_string(); + let gpt = role("gpt-5-6-sol", 0.70); + let claude = role("claude-fable-5", 0.64); + let mut roster = Roster { + primary: gpt.clone(), + review_supervisor: None, + subagent_default: Some(claude.clone()), + available: vec![gpt, claude.clone()], + choices: Vec::new(), + warnings: Vec::new(), + inventory: AcpInventory::default(), + subagent_acp_priority: Vec::new(), + subagent_acp_source: None, + }; + + roster.primary = claude; + rebind_auto_subagents_for_primary(&mut roster, &config); + + assert_eq!( + roster + .subagent_default + .expect("explicit subagent kept") + .model + .model, + "claude-fable-5" + ); + } + #[test] fn explicit_review_is_not_rebound_after_primary_is_pinned() { let mut config = Config::default(); diff --git a/mj-core/src/side.rs b/mj-core/src/side.rs index 42df9e01..6b61fc38 100644 --- a/mj-core/src/side.rs +++ b/mj-core/src/side.rs @@ -163,8 +163,8 @@ pub fn isolated_runtime_config( fs_max_text_bytes, access_mode: acp::RuntimeAccessMode::Full, agent_source_id: None, - config_path: None, saved_session_config: std::collections::HashMap::new(), + saved_session_config_reload: None, role_config: None, subagents: None, memory, @@ -233,7 +233,6 @@ mod tests { assert!(cfg.subagents.is_none()); assert!(cfg.role_config.is_none()); assert!(cfg.agent_source_id.is_none()); - assert!(cfg.config_path.is_none()); assert!(cfg.saved_session_config.is_empty()); assert!(cfg.side_prompt_policy); assert_eq!(cfg.resume_session.as_deref(), Some("child-session")); diff --git a/mj-remote/src/remote.rs b/mj-remote/src/remote.rs index 3702d0d6..76fc6c95 100644 --- a/mj-remote/src/remote.rs +++ b/mj-remote/src/remote.rs @@ -1763,9 +1763,10 @@ pub trait ServerSessionManager: Send + Sync { fn resume_session(&self, cwd: PathBuf, session_id: String) -> u64; fn owns_session(&self, session_id: &str) -> bool; async fn archive_session(&self, session_id: &str) -> bool; - /// Re-resolve reviewer and subagent routes for active sessions whose - /// primary route still matches their running ACP process. - async fn reload_auxiliary_agents(&self); + /// Re-resolve reviewer and subagent routes for the one session a + /// `/mjconfig` save was made from. Other running sessions are never + /// touched; they keep the routes they started with. + async fn reload_auxiliary_agents(&self, session_id: &str); async fn refresh_for_config( &self, config_path: &Path, @@ -5534,6 +5535,10 @@ struct MjLoginStatus { #[derive(Debug, Default, Deserialize)] #[serde(deny_unknown_fields)] struct MjConfigApplyRequest { + /// The session the `/mjconfig` panel was opened from. Live updates + /// (auxiliary-route reloads) apply to this session only; absent, the + /// save changes defaults for new sessions and touches no live session. + session_id: Option, /// One of the four supported coder/reviewer team ids. team: Option, primary_model: Option, @@ -6464,8 +6469,9 @@ fn mjconfig_option_controls_reasoning_effort( async fn mjconfig_apply( State(state): State, - Json(request): Json, + Json(mut request): Json, ) -> std::result::Result, (StatusCode, String)> { + let invoking_session = request.session_id.take(); let mut config = mjconfig_load(&state); if let Some(warning) = config.newer_build_notice() { return Err((StatusCode::CONFLICT, warning)); @@ -6492,7 +6498,7 @@ async fn mjconfig_apply( &choices, active_models.as_ref(), )?; - config::save_user_config_preserving_session_routes(&state.mjconfig.config_path, &mut config) + config::save_user_config(&state.mjconfig.config_path, &config) .map_err(|error| internal_error(format!("save config: {error:#}")))?; let notice = if reroute_notices.is_empty() { "Saved".to_string() @@ -6511,7 +6517,14 @@ async fn mjconfig_apply( { Ok(Some(roster)) => { state.mjconfig.update_from_roster(&roster); - state.session_manager.reload_auxiliary_agents().await; + // Live updates reach only the session the save was made from; + // every other running session keeps its current routes. + if let Some(session_id) = invoking_session.as_deref() { + state + .session_manager + .reload_auxiliary_agents(session_id) + .await; + } } Ok(None) => {} Err(error) => warn!("saved configuration does not bind a roster yet: {error}"), @@ -10676,7 +10689,7 @@ mod tests { #[derive(Default)] struct TestServerSessionManager { roster_refresh_requested: AtomicBool, - auxiliary_reloads: AtomicU64, + auxiliary_reloads: Mutex>, roster_refresh_lock: tokio::sync::Mutex<()>, launches: Mutex>, next_launch: AtomicU64, @@ -10731,8 +10744,11 @@ mod tests { true } async fn shutdown_all(&self) {} - async fn reload_auxiliary_agents(&self) { - self.auxiliary_reloads.fetch_add(1, Ordering::Release); + async fn reload_auxiliary_agents(&self, session_id: &str) { + self.auxiliary_reloads + .lock() + .expect("auxiliary reloads") + .push(session_id.to_string()); } async fn refresh_for_config( &self, @@ -11489,7 +11505,10 @@ mod tests { .oneshot(mjconfig_request( "POST", Some(token), - Some(serde_json::json!({ "team": "codex" })), + Some(serde_json::json!({ + "team": "codex", + "session_id": "mjconfig-session", + })), )) .await .expect("response"); @@ -11506,9 +11525,9 @@ mod tests { let discovery = runtime.discovery.lock().expect("discovery lock"); assert_eq!(discovery.choices.len(), 1); assert_eq!( - manager.auxiliary_reloads.load(Ordering::Acquire), - 1, - "a successful mjconfig rebind reloads active server sessions' auxiliary routes" + *manager.auxiliary_reloads.lock().expect("auxiliary reloads"), + vec!["mjconfig-session".to_string()], + "a successful mjconfig rebind reloads only the invoking session's auxiliary routes" ); } diff --git a/mj-remote/src/remote_viewer.html b/mj-remote/src/remote_viewer.html index cb1c5fe4..2be94a32 100644 --- a/mj-remote/src/remote_viewer.html +++ b/mj-remote/src/remote_viewer.html @@ -4996,10 +4996,16 @@

Load session

mjSetStatus("Saving…"); try { const edits = mjcfg.edits; + // The save carries the session it was made from: live updates + // (auxiliary-route reloads, session-option reconciliation) apply + // to that session only, never to other running sessions. + const payload = selectedSessionId + ? { ...edits, session_id: selectedSessionId } + : edits; const response = await apiFetch("/api/mjconfig", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(edits), + body: JSON.stringify(payload), }); if (!response.ok) { mjSetStatus((await response.text()) || `HTTP ${response.status}`, true); diff --git a/mj-tui/src/settings.rs b/mj-tui/src/settings.rs index 77f56cca..18af8214 100644 --- a/mj-tui/src/settings.rs +++ b/mj-tui/src/settings.rs @@ -2252,15 +2252,6 @@ mod tests { SessionConfigSelectOption::new("priority", "Priority"), ], )]; - editor - .config - .session_config - .entry(server_id.clone()) - .or_default() - .models - .entry("model-a".to_string()) - .or_default() - .insert("config:service_tier".to_string(), "default".to_string()); editor.config.agent.acp_source = Some(server_id.clone()); editor.tab = SettingsTab::Agents; editor.selected = 1; @@ -2280,10 +2271,6 @@ mod tests { editor.config.agent.session_defaults[&server_id]["config:service_tier"], "priority" ); - assert!( - editor.config.session_config[&server_id].models["model-a"] - .contains_key("config:service_tier") - ); } #[test] diff --git a/mj-tui/src/ui.rs b/mj-tui/src/ui.rs index 8d3389d0..71102e08 100644 --- a/mj-tui/src/ui.rs +++ b/mj-tui/src/ui.rs @@ -6151,7 +6151,7 @@ fn set_memory_toggle(state: &mut AppState, toggle: MemoryToggle, enabled: bool) MemoryToggle::Use => config.memory.use_memories = enabled, MemoryToggle::Generate => config.memory.generate_memories = enabled, } - match config::save_user_config_preserving_session_routes(&path, &mut config) { + match config::save_user_config(&path, &config) { Ok(()) => state.record_status_message( StatusKind::Info, format!( @@ -6232,11 +6232,17 @@ fn persist_mjconfig_selection( let reroute_notices = crate::settings::reset_unroutable_models(&mut config, &state.model_choices); let team_changed = initial_config.team != config.team; - let auxiliary_agents_update_live = primary_route_stays_active(state, &config); - let primary_team_switch_pending = team_changed && !auxiliary_agents_update_live; + // The switch prompt keys off whether the saved primary still matches the + // running process; the auxiliary reload does not. Reviewer and subagent + // lanes re-resolve from the saved config for this session even when the + // primary change itself can only apply on /new or a confirmed switch. + // `cmd_tx` addresses only this session's runtime, so the reload can never + // reach another session. + let primary_route_live = primary_route_stays_active(state, &config); + let primary_team_switch_pending = team_changed && !primary_route_live; let live_session_updates = live_primary_session_config_updates(state, &config); if let Some(path) = state.config_path.clone() { - match config::save_user_config_preserving_session_routes(&path, &mut config) { + match config::save_user_config(&path, &config) { Ok(()) => { state.configured_models = config.model_names(); state.acp_inventory = @@ -6257,9 +6263,7 @@ fn persist_mjconfig_selection( max_correction_rounds: config.agent.max_correction_rounds, }); } - if auxiliary_agents_update_live { - let _ = cmd_tx.send(UiCommand::ReloadAuxiliaryAgents); - } + let _ = cmd_tx.send(UiCommand::ReloadAuxiliaryAgents); for (target, value) in live_session_updates { let _ = cmd_tx.send(UiCommand::SetSessionConfigOption { target, value }); } @@ -6282,7 +6286,7 @@ fn persist_mjconfig_selection( } let mut message = if primary_team_switch_pending { "config saved; choose whether to switch the primary now".to_string() - } else if team_changed && auxiliary_agents_update_live { + } else if team_changed { "config saved; reviewer and subagent configuration is updating now".to_string() } else { format!( @@ -7503,20 +7507,18 @@ fn persist_team_picker_selection( } }; preset.apply(&mut config); - // Presets reset the primary selector to `auto`. Reload the reviewer and - // subagent routes only when that post-preset route is still the primary - // process already running in this session. - let apply_auxiliaries_live = primary_config_matches_active_route(state, &config); - match config::save_user_config_preserving_session_routes(path, &mut config) { + // Whether the post-preset primary route is still the process already + // running decides only the switch-primary prompt. The reviewer and + // subagent lanes reload from the saved config for this session either way. + let primary_unchanged = primary_config_matches_active_route(state, &config); + match config::save_user_config(path, &config) { Ok(()) => { state.configured_models = config.model_names(); state.acp_inventory = crate::roster::rediscover_inventory(&config, &state.acp_inventory); - if apply_auxiliaries_live { - let _ = cmd_tx.send(UiCommand::ReloadAuxiliaryAgents); - } + let _ = cmd_tx.send(UiCommand::ReloadAuxiliaryAgents); state.record_status_message(StatusKind::Info, format!("{} team saved", preset.label())); - Some(apply_auxiliaries_live) + Some(primary_unchanged) } Err(error) => { state.record_status_message( @@ -19558,6 +19560,12 @@ mod tests { Some(config::TeamPreset::Claude) ); assert!(!state.review_enabled, "active session policy is unchanged"); + // The reviewer/subagent lanes still reload for this session; the + // review policy itself is untouched until the transfer completes. + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); assert!(cmd_rx.try_recv().is_err(), "no live policy update is sent"); handle_crossterm(&mut state, &cmd_tx, key(KeyCode::Enter)); @@ -20011,7 +20019,13 @@ mod tests { .as_ref() .is_some_and(|picker| { picker.step == TeamPickerStep::SwitchPrimary }) ); - assert!(cmd_rx.try_recv().is_err(), "no live reload is sent"); + // The auxiliary lanes reload for this session; the primary repin + // itself still waits for the new-session step. + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!(cmd_rx.try_recv().is_err(), "the primary is not reloaded"); } #[test] @@ -20702,6 +20716,13 @@ mod tests { assert_eq!(picker.step, TeamPickerStep::SwitchPrimary); assert_eq!(picker.selected, 1, "Claude is selected"); assert!(picker.switch_primary_now); + // The reviewer and subagent lanes update for this session even while + // the primary switch is still pending confirmation; the primary + // itself is not reloaded. + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); assert!( cmd_rx.try_recv().is_err(), "the old primary is not reloaded" @@ -20752,6 +20773,13 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, initial_config, config); + // A combined save — team change replacing the primary plus a session + // option — still applies both to this session: the auxiliary reload + // and the live option update. + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); assert!(matches!( cmd_rx.try_recv(), Ok(UiCommand::SetSessionConfigOption { @@ -20796,6 +20824,10 @@ mod tests { let picker = state.team_picker.as_ref().expect("primary switch prompt"); assert_eq!(picker.step, TeamPickerStep::SwitchPrimary); assert!(picker.switch_primary_now); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); assert!(cmd_rx.try_recv().is_err()); handle_crossterm(&mut state, &cmd_tx, key(KeyCode::Enter)); @@ -20861,6 +20893,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!(matches!( cmd_rx.try_recv(), Ok(UiCommand::SetSessionConfigOption { @@ -20901,6 +20938,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!( cmd_rx.try_recv().is_err(), "a saved value that matches the active session must not be re-sent" @@ -20940,6 +20982,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!( matches!( cmd_rx.try_recv(), @@ -20980,6 +21027,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!( cmd_rx.try_recv().is_err(), "the selected seat effort belongs to another provider's route" @@ -21014,6 +21066,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!(matches!( cmd_rx.try_recv(), Ok(UiCommand::SetSessionConfigOption { @@ -21054,6 +21111,11 @@ mod tests { persist_mjconfig_selection(&mut state, &cmd_tx, config.clone(), config); + assert!(matches!( + cmd_rx.try_recv(), + Ok(UiCommand::ReloadAuxiliaryAgents) + )); + assert!(matches!( cmd_rx.try_recv(), Ok(UiCommand::SetSessionConfigOption { diff --git a/src/headless.rs b/src/headless.rs index a9739cdf..3294560b 100644 --- a/src/headless.rs +++ b/src/headless.rs @@ -138,8 +138,8 @@ pub async fn run(cfg: RunConfig) -> Result<()> { fs_max_text_bytes: cfg.fs_max_text_bytes, access_mode: acp::RuntimeAccessMode::Full, agent_source_id: Some(format!("roster:{}", primary.model.model)), - config_path: Some(config_path), saved_session_config: HashMap::new(), + saved_session_config_reload: None, role_config: Some(acp::RuntimeRoleConfig { label: "primary".to_string(), model_id: primary.model.model.clone(), diff --git a/src/main.rs b/src/main.rs index 0bafca22..86d054d9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2742,13 +2742,16 @@ async fn run_session( fs_max_text_bytes: runtime_options.fs_max_text_bytes, access_mode: acp::RuntimeAccessMode::Full, agent_source_id: Some(roster.primary.launch.source_id.clone()), - config_path: Some(config::default_config_path()), saved_session_config: config::load_saved_session_config( &config::default_config_path(), &roster.primary.launch.source_id, - &roster.primary.model.model, config::SessionConfigSeat::Primary, ), + saved_session_config_reload: Some(acp::SavedSessionConfigReload { + config_path: config::default_config_path(), + source_id: roster.primary.launch.source_id.clone(), + seat: config::SessionConfigSeat::Primary, + }), role_config: Some(acp::RuntimeRoleConfig { label: "primary".to_string(), model_id: roster.primary.model.model.clone(), @@ -3023,7 +3026,7 @@ async fn run_session( continue; } }; - let updated_roster = match roster::resolve(&updated_config, &side_cwd).await { + let mut updated_roster = match roster::resolve(&updated_config, &side_cwd).await { Ok(roster) => roster, Err(error) => { let _ = side_ui_event_tx.send(UiEvent::Warning(format!( @@ -3033,11 +3036,17 @@ async fn run_session( } }; if !primary_route_matches(&command_primary, &updated_roster.primary) { + // The primary itself only changes on /new or /clear, but + // the reviewer and subagent lanes still follow the saved + // config for this session. Auto seats re-pair against the + // primary that keeps running. + updated_roster.primary = command_primary.clone(); + roster::rebind_auto_review_for_primary(&mut updated_roster, &updated_config); + roster::rebind_auto_subagents_for_primary(&mut updated_roster, &updated_config); let _ = side_ui_event_tx.send(UiEvent::Info( "primary agent changed; start /new or /clear to apply that route" .to_string(), )); - continue; } let (roles, codex_home) = match isolated_subagent_roles( roster::subagent_failover_roles(&updated_roster), diff --git a/src/remote_host.rs b/src/remote_host.rs index e9565dd7..4e659904 100644 --- a/src/remote_host.rs +++ b/src/remote_host.rs @@ -426,19 +426,22 @@ impl RootServerSessionManager { } } - async fn reload_auxiliary_agents(&self) { - let commands = self - .sessions - .lock() - .map(|mut sessions| { - sessions.retain(|session| !session.task.is_finished()); - sessions - .iter() - .map(|session| session.command_tx.clone()) - .collect::>() - }) - .unwrap_or_default(); - for command_tx in commands { + async fn reload_auxiliary_agents(&self, session_id: &str) { + // Only the session the `/mjconfig` save came from re-resolves its + // reviewer and subagent routes; other running sessions are left alone. + let command_tx = self.sessions.lock().ok().and_then(|mut sessions| { + sessions.retain(|session| !session.task.is_finished()); + sessions + .iter() + .find(|session| { + session + .session_id + .lock() + .is_ok_and(|current| current.as_deref() == Some(session_id)) + }) + .map(|session| session.command_tx.clone()) + }); + if let Some(command_tx) = command_tx { let _ = command_tx.send(UiCommand::ReloadAuxiliaryAgents); } } @@ -469,8 +472,8 @@ fn start_server_agent_session( let session_id = Arc::new(Mutex::new(resume_session.clone())); let published_session_id = Arc::clone(&session_id); // The adapter source id ("codex-acp", ...) — not the synthetic - // `roster:{model}` launch id — so saved session options load from and - // accepted live values persist to the same buckets the TUI uses. + // `roster:{model}` launch id — so saved session options load from the + // same buckets the TUI uses. let agent_source_id = roster.as_ref().map_or_else( || agent.source_id.clone(), |resolved| resolved.primary.launch.source_id.clone(), @@ -481,10 +484,19 @@ fn start_server_agent_session( config::load_saved_session_config( &config_path, &resolved.primary.launch.source_id, - &resolved.primary.model.model, config::SessionConfigSeat::Primary, ) }); + // Clear/new reuses this long-lived runtime, so fresh sessions re-read the + // saved defaults from disk: a `/mjconfig` save after launch reaches them. + let saved_session_config_reload = + roster + .as_ref() + .map(|resolved| acp::SavedSessionConfigReload { + config_path: config_path.clone(), + source_id: resolved.primary.launch.source_id.clone(), + seat: config::SessionConfigSeat::Primary, + }); let project_label = mj_core::paths::project_label_from_cwd(&cwd); let worktree_label = mj_core::paths::worktree_name_from_cwd(&cwd); // With a roster the session has a real primary model; align the published @@ -633,8 +645,8 @@ fn start_server_agent_session( fs_max_text_bytes, access_mode: mj_core::acp::RuntimeAccessMode::Full, agent_source_id: Some(agent_source_id), - config_path: Some(config_path), saved_session_config, + saved_session_config_reload, role_config, subagents, memory: session_memory, @@ -850,7 +862,8 @@ fn start_server_agent_session( continue; } }; - let updated_roster = match roster::resolve(&updated_config, &side_cwd).await + let mut updated_roster = match roster::resolve(&updated_config, &side_cwd) + .await { Ok(roster) => roster, Err(error) => { @@ -861,11 +874,24 @@ fn start_server_agent_session( } }; if !crate::primary_route_matches(command_primary, &updated_roster.primary) { + // The primary itself only changes with a new + // server session, but the reviewer and subagent + // lanes still follow the saved config for this + // one. Auto seats re-pair against the primary + // that keeps running. + updated_roster.primary = command_primary.clone(); + roster::rebind_auto_review_for_primary( + &mut updated_roster, + &updated_config, + ); + roster::rebind_auto_subagents_for_primary( + &mut updated_roster, + &updated_config, + ); tracker.observe_event(&UiEvent::Info( "primary agent changed; start a new server session to apply that route" .to_string(), )); - continue; } let (roles, codex_home) = match crate::isolated_subagent_roles( roster::subagent_failover_roles(&updated_roster), @@ -1126,8 +1152,8 @@ impl remote::ServerSessionManager for RootServerSessionManager { async fn shutdown_all(&self) { RootServerSessionManager::shutdown_all(self).await } - async fn reload_auxiliary_agents(&self) { - RootServerSessionManager::reload_auxiliary_agents(self).await + async fn reload_auxiliary_agents(&self, session_id: &str) { + RootServerSessionManager::reload_auxiliary_agents(self, session_id).await } async fn refresh_for_config( &self, @@ -1213,7 +1239,11 @@ mod tests { task, }); - manager.reload_auxiliary_agents().await; + // A reload addressed to another session must not reach this one. + manager.reload_auxiliary_agents("some-other-session").await; + assert!(command_rx.try_recv().is_err()); + + manager.reload_auxiliary_agents("server-session").await; assert!(matches!( command_rx.try_recv(),