diff --git a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs index 25acfb1d0a..e7908ee3a6 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs @@ -565,6 +565,30 @@ fn resolve_subagent_model_target( } } +/// Resolve the model target for an internally created (or reused) subagent. +/// +/// `subagents.enforce_default` is the user's hard guarantee that a delegated +/// child never lands on another model: it wins over the Task tool's explicit +/// `model_id`, over `model_id: "inherit"`, over `subagents.fork` and over every +/// per-builtin or custom-agent override. When the flag is off this is exactly +/// [`resolve_subagent_model_target`]. +/// +/// Callers must skip this helper for a session that carries an approved +/// external immutable model binding: such a session keeps its approved model +/// (the fresh path returns before reaching it, the fork and reuse paths guard +/// on the binding policy). +fn resolve_internal_subagent_model_target( + defaults: &AgentModelSelections, + explicit: Option<&str>, + configured: &AgentModelSelection, + parent: Option, +) -> OpenBitFunResult { + if defaults.subagents.enforce_default { + return resolve_subagent_model_target(None, &defaults.subagents.default, parent); + } + resolve_subagent_model_target(explicit, configured, parent) +} + /// Whether a turn belongs to the review phase of a review child session. /// /// Only `CodeReview`/`DeepReview` receive the `deep_review_run_manifest` @@ -12516,7 +12540,8 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet .map(AgentModelSelection::from) .unwrap_or(defaults.builtin(agent_type)) }; - let target = resolve_subagent_model_target( + let target = resolve_internal_subagent_model_target( + &defaults, if inherit_parent_model { None } else { @@ -12721,7 +12746,44 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet task_description = text.clone(); } } - let requested_target = if inherit_parent_model { + // A reused child is still an internally created subagent: + // `subagents.enforce_default` also stops `send_input` from + // switching an existing child onto another model. Sessions + // that hold an approved external immutable binding keep + // their own approved model. The probe is best-effort so that + // reading the flag never turns a previously working + // follow-up into a failure. + let enforced_target = match Self::agent_model_defaults().await.ok().as_ref() { + Some(defaults) + if defaults.subagents.enforce_default + && session.config.model_binding_policy + != SessionModelBindingPolicy::ApprovedImmutable => + { + Some( + Self::normalize_model_target( + resolve_internal_subagent_model_target( + defaults, + None, + &defaults.subagents.default, + // An `inherit` default resolves against + // the parent session, exactly like the + // fresh and fork paths: the child may + // still hold the model this flag exists + // to exclude. + Some(Self::model_target_from_config( + &parent_session.config, + defaults, + )?), + )?, + ) + .await?, + ) + } + _ => None, + }; + let requested_target = if let Some(enforced_target) = enforced_target { + Some(enforced_target) + } else if inherit_parent_model { Some( Self::normalize_model_target(self.parent_model_target( &parent_session_id, @@ -12931,18 +12993,33 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet } else { defaults.subagents.fork.clone() }; - let target = resolve_subagent_model_target( - if inherit_parent_model { - None - } else { - model_id.as_deref() - }, - &configured, - Some(Self::model_target_from_config( - &snapshot.session_config, + let explicit = if inherit_parent_model { + None + } else { + model_id.as_deref() + }; + let forked_parent_target = + Self::model_target_from_config(&snapshot.session_config, &defaults)?; + // A fork child clones the parent's session config, so it can + // carry an approved external immutable binding and that + // binding's fingerprint. Enforcement must not re-pin it: the + // approved model would then fail the later binding check. + let target = if snapshot.session_config.model_binding_policy + == SessionModelBindingPolicy::ApprovedImmutable + { + resolve_subagent_model_target( + explicit, + &configured, + Some(forked_parent_target), + )? + } else { + resolve_internal_subagent_model_target( &defaults, - )?), - )?; + explicit, + &configured, + Some(forked_parent_target), + )? + }; let target = Self::normalize_model_target(target).await?; let mut session_config = snapshot.build_child_session_config(None); apply_session_model_target(&mut session_config, target); @@ -24371,6 +24448,426 @@ pub(crate) mod tests { .await; } + #[tokio::test] + async fn enforced_subagent_default_wins_over_explicit_inherit_fork_and_builtin_choices() { + assert_subagent_default_enforcement(true).await; + } + + #[tokio::test] + async fn disabled_subagent_default_enforcement_keeps_todays_resolution() { + assert_subagent_default_enforcement(false).await; + } + + /// `subagents.enforce_default` must pin every internally created (or reused) + /// subagent to `subagents.default`; with the flag off every existing rule + /// keeps winning instead. + async fn assert_subagent_default_enforcement(enforce_default: bool) { + let (coordinator, manager) = test_persistent_coordinator(); + let workspace = tempfile::tempdir().unwrap(); + let record = crate::service::workspace::legacy_compat::register_local_fixture( + workspace.path(), + None, + ) + .await; + let workspace_path = workspace.path().to_string_lossy().into_owned(); + let parent = manager + .create_session( + "Parent".into(), + "Standard".into(), + SessionConfig { + model_id: Some("primary".into()), + workspace_id: Some(record.id), + workspace_path: Some(workspace_path.clone()), + ..Default::default() + }, + ) + .await + .unwrap(); + manager + .replace_context_messages( + &parent.session_id, + vec![Message::user("Inspect the workspace".into())], + ) + .await; + let child = coordinator + .create_hidden_agent_session( + None, + "Reused subagent".to_string(), + "Standard".to_string(), + SessionConfig { + model_id: Some("primary".to_string()), + workspace_path: Some(workspace_path), + ..Default::default() + }, + Some(format!("session-{}", parent.session_id)), + SessionKind::Subagent, + ) + .await + .unwrap(); + + let request = |target_session_id: Option<&str>, + fork: bool, + model_id: Option<&str>, + inherit_parent_model: bool| SubagentExecutionRequest { + task_description: "Inspect files".into(), + requested_agent_id: None, + context_mode: if fork { + SubagentContextMode::Fork + } else { + SubagentContextMode::Fresh + }, + target_session_id: target_session_id.map(str::to_string), + subagent_type: if fork || target_session_id.is_some() { + None + } else { + Some("Explore".into()) + }, + logical_subagent_type: None, + continuation_policy: SessionContinuationPolicy::Reusable, + model_binding_policy: SessionModelBindingPolicy::Mutable, + workspace_path: None, + model_id: model_id.map(str::to_string), + inherit_parent_model, + subagent_parent_info: SubagentParentInfo { + session_id: parent.session_id.clone(), + dialog_turn_id: "turn".into(), + tool_call_id: "task".into(), + }, + context: HashMap::new(), + permission_runtime_ceiling: PermissionRuntimeCeiling::default(), + delegation_policy: DelegationPolicy::top_level().spawn_child(), + external_generation_lease: None, + }; + + let mut defaults = AgentModelSelections::default(); + defaults.subagents.default = AgentModelSelection::Model { + model_ref: "fast".into(), + }; + // Both the per-builtin override and the fork default name the model the + // user wants kept away from subagents. + defaults.subagents.builtin.insert( + "Explore".into(), + AgentModelSelection::Model { + model_ref: "primary".into(), + }, + ); + defaults.subagents.fork = AgentModelSelection::Model { + model_ref: "primary".into(), + }; + defaults.subagents.enforce_default = enforce_default; + + let expected = if enforce_default { "fast" } else { "primary" }; + let expected_target = || ModelSelection::Model { + model_ref: expected.into(), + }; + + TEST_AGENT_MODEL_DEFAULTS + .scope(defaults, async { + // (a) An explicit Task model_id is ignored. + assert_eq!( + coordinator + .resolve_fresh_subagent_model_target( + Some("primary"), + false, + "Explore", + &parent.session_id, + ) + .await + .unwrap(), + expected_target() + ); + // (b) `model_id: "inherit"` is ignored. + assert_eq!( + coordinator + .resolve_fresh_subagent_model_target( + None, + true, + "Explore", + &parent.session_id, + ) + .await + .unwrap(), + expected_target() + ); + // (c) The per-builtin override is ignored. + assert_eq!( + coordinator + .resolve_fresh_subagent_model_target( + None, + false, + "Explore", + &parent.session_id, + ) + .await + .unwrap(), + expected_target() + ); + // (d) A fork child ignores `subagents.fork` and its own model_id. + let forked = coordinator + .resolve_hidden_subagent_execution_request(request( + None, + true, + Some("primary"), + false, + )) + .await + .unwrap(); + assert_eq!(forked.session_config.model_id.as_deref(), Some(expected)); + let inherited_fork = coordinator + .resolve_hidden_subagent_execution_request(request(None, true, None, true)) + .await + .unwrap(); + assert_eq!( + inherited_fork.session_config.model_id.as_deref(), + Some(expected) + ); + // (e) `send_input` must not hand an existing child another model. + let reused = coordinator + .resolve_hidden_subagent_execution_request(request( + Some(&child.session_id), + false, + Some("primary"), + false, + )) + .await + .unwrap(); + assert_eq!(reused.session_config.model_id.as_deref(), Some(expected)); + let reused_without_switch = coordinator + .resolve_hidden_subagent_execution_request(request( + Some(&child.session_id), + false, + None, + false, + )) + .await + .unwrap(); + assert_eq!( + reused_without_switch.session_config.model_id.as_deref(), + Some(expected) + ); + assert_eq!( + manager + .get_session(&child.session_id) + .and_then(|session| session.config.model_id.clone()) + .as_deref(), + Some(expected) + ); + }) + .await; + } + + #[tokio::test] + async fn fork_child_with_an_approved_immutable_binding_skips_default_enforcement() { + let (coordinator, manager) = test_persistent_coordinator(); + let workspace = tempfile::tempdir().unwrap(); + let record = crate::service::workspace::legacy_compat::register_local_fixture( + workspace.path(), + None, + ) + .await; + let workspace_path = workspace.path().to_string_lossy().into_owned(); + // A fork child clones its parent's session config, including this + // approved external immutable binding and its fingerprint. + let parent = manager + .create_session( + "Approved parent".into(), + "Standard".into(), + SessionConfig { + model_id: Some("approved-model".into()), + model_binding_policy: SessionModelBindingPolicy::ApprovedImmutable, + model_binding_fingerprint: Some("approved-fingerprint".into()), + workspace_id: Some(record.id), + workspace_path: Some(workspace_path), + ..Default::default() + }, + ) + .await + .unwrap(); + manager + .replace_context_messages( + &parent.session_id, + vec![Message::user("Inspect the workspace".into())], + ) + .await; + + let mut defaults = AgentModelSelections::default(); + defaults.subagents.default = AgentModelSelection::Model { + model_ref: "fast".into(), + }; + defaults.subagents.fork = AgentModelSelection::Model { + model_ref: "approved-model".into(), + }; + defaults.subagents.enforce_default = true; + + // An approved external binding always names a concrete configured + // model, so the fork target must resolve against a real catalog entry. + use crate::service::config::types::{AIConfig, AIModelConfig}; + let mut ai_config = AIConfig::default(); + ai_config.models = ["approved-model"] + .into_iter() + .map(|id| AIModelConfig { + id: id.into(), + enabled: true, + ..Default::default() + }) + .collect(); + + TEST_MODEL_RESOLUTION_AI_CONFIG + .scope( + ai_config, + TEST_AGENT_MODEL_DEFAULTS.scope(defaults, async { + let forked = coordinator + .resolve_hidden_subagent_execution_request(SubagentExecutionRequest { + task_description: "Inspect files".into(), + requested_agent_id: None, + context_mode: SubagentContextMode::Fork, + target_session_id: None, + subagent_type: None, + logical_subagent_type: None, + continuation_policy: SessionContinuationPolicy::Reusable, + model_binding_policy: SessionModelBindingPolicy::Mutable, + workspace_path: None, + model_id: None, + inherit_parent_model: false, + subagent_parent_info: SubagentParentInfo { + session_id: parent.session_id.clone(), + dialog_turn_id: "turn".into(), + tool_call_id: "task".into(), + }, + context: HashMap::new(), + permission_runtime_ceiling: PermissionRuntimeCeiling::default(), + delegation_policy: DelegationPolicy::top_level().spawn_child(), + external_generation_lease: None, + }) + .await + .unwrap(); + // The approved binding keeps its own model: pinning the + // child to `subagents.default` would fail later with + // "Approved model binding changed before execution". + assert_eq!( + forked.session_config.model_id.as_deref(), + Some("approved-model") + ); + assert_ne!(forked.session_config.model_id.as_deref(), Some("fast")); + assert_eq!( + forked.session_config.model_binding_policy, + SessionModelBindingPolicy::ApprovedImmutable + ); + assert_eq!( + forked.session_config.model_binding_fingerprint.as_deref(), + Some("approved-fingerprint") + ); + }), + ) + .await; + } + + #[tokio::test] + async fn enforced_inherit_default_resolves_a_reused_child_against_the_parent_model() { + let (coordinator, manager) = test_persistent_coordinator(); + let workspace = tempfile::tempdir().unwrap(); + let record = crate::service::workspace::legacy_compat::register_local_fixture( + workspace.path(), + None, + ) + .await; + let workspace_path = workspace.path().to_string_lossy().into_owned(); + let parent = manager + .create_session( + "Parent".into(), + "Standard".into(), + SessionConfig { + model_id: Some("primary".into()), + workspace_id: Some(record.id), + workspace_path: Some(workspace_path.clone()), + ..Default::default() + }, + ) + .await + .unwrap(); + let child = coordinator + .create_hidden_agent_session( + None, + "Expensive reused subagent".to_string(), + "Standard".to_string(), + SessionConfig { + model_id: Some("expensive-model".to_string()), + workspace_path: Some(workspace_path), + ..Default::default() + }, + Some(format!("session-{}", parent.session_id)), + SessionKind::Subagent, + ) + .await + .unwrap(); + + let mut defaults = AgentModelSelections::default(); + defaults.subagents.default = AgentModelSelection::Inherit; + defaults.subagents.enforce_default = true; + + // The child's previous model is a concrete catalog entry, so resolving + // it against the child must succeed before it can be detected. + use crate::service::config::types::{AIConfig, AIModelConfig}; + let mut ai_config = AIConfig::default(); + ai_config.models = ["expensive-model"] + .into_iter() + .map(|id| AIModelConfig { + id: id.into(), + enabled: true, + ..Default::default() + }) + .collect(); + + TEST_MODEL_RESOLUTION_AI_CONFIG + .scope( + ai_config, + TEST_AGENT_MODEL_DEFAULTS.scope(defaults, async { + let reused = coordinator + .resolve_hidden_subagent_execution_request(SubagentExecutionRequest { + task_description: "Continue the investigation".into(), + requested_agent_id: None, + context_mode: SubagentContextMode::Fresh, + target_session_id: Some(child.session_id.clone()), + subagent_type: None, + logical_subagent_type: None, + continuation_policy: SessionContinuationPolicy::Reusable, + model_binding_policy: SessionModelBindingPolicy::Mutable, + workspace_path: None, + model_id: None, + inherit_parent_model: false, + subagent_parent_info: SubagentParentInfo { + session_id: parent.session_id.clone(), + dialog_turn_id: "turn".into(), + tool_call_id: "task".into(), + }, + context: HashMap::new(), + permission_runtime_ceiling: PermissionRuntimeCeiling::default(), + delegation_policy: DelegationPolicy::top_level().spawn_child(), + external_generation_lease: None, + }) + .await + .unwrap(); + // `inherit` resolves against the parent session, exactly + // like the fresh and fork paths; resolving it against the + // child would keep the expensive model the flag exists to + // exclude. + assert_eq!(reused.session_config.model_id.as_deref(), Some("primary")); + assert_ne!( + reused.session_config.model_id.as_deref(), + Some("expensive-model") + ); + assert_eq!( + manager + .get_session(&child.session_id) + .and_then(|session| session.config.model_id.clone()) + .as_deref(), + Some("primary") + ); + }), + ) + .await; + } + #[tokio::test] async fn fresh_subagent_request_can_explicitly_inherit_parent_model() { let (coordinator, session_manager) = test_coordinator(); diff --git a/src/crates/assembly/core/src/service/config/providers.rs b/src/crates/assembly/core/src/service/config/providers.rs index 087787cdf6..8b2405c700 100644 --- a/src/crates/assembly/core/src/service/config/providers.rs +++ b/src/crates/assembly/core/src/service/config/providers.rs @@ -1012,6 +1012,48 @@ mod tests { assert!(error.to_string().contains("agent_model_defaults.mode")); } + #[tokio::test] + async fn enforced_subagent_default_flag_validates_and_is_never_dropped() { + // The legacy surface only validates while no runtime selections exist. + let mut config = AIConfig::default(); + config.agent_model_defaults.subagents.enforce_default = true; + let value = serde_json::to_value(&config).unwrap(); + assert_eq!( + value["agent_model_defaults"]["subagents"]["enforce_default"], + serde_json::json!(true) + ); + AIConfigProvider + .validate_config(&value) + .await + .expect("a boolean enforcement flag has nothing to validate"); + let round_tripped: AIConfig = serde_json::from_value(value).unwrap(); + assert!(round_tripped.agent_model_defaults.subagents.enforce_default); + + // The runtime surface keeps the flag through validation as well. + let mut selections = + openbitfun_config_contracts::model_pool::AgentModelSelections::default(); + selections.subagents.enforce_default = true; + let mut config = AIConfig::default(); + config.set_model_selections(selections).unwrap(); + let value = serde_json::to_value(&config).unwrap(); + assert_eq!( + value["agent_model_selections"]["subagents"]["enforce_default"], + serde_json::json!(true) + ); + AIConfigProvider + .validate_config(&value) + .await + .expect("the runtime selections remain valid"); + let round_tripped: AIConfig = serde_json::from_value(value).unwrap(); + assert!( + round_tripped + .effective_model_selections() + .unwrap() + .subagents + .enforce_default + ); + } + #[tokio::test] async fn rejects_invalid_canonical_reasoning_actions() { for (action, expected) in [ diff --git a/src/crates/assembly/core/src/service/config/service.rs b/src/crates/assembly/core/src/service/config/service.rs index 0a5c45edb8..a0e4dfeaa1 100644 --- a/src/crates/assembly/core/src/service/config/service.rs +++ b/src/crates/assembly/core/src/service/config/service.rs @@ -137,6 +137,17 @@ fn translate_legacy_model_selection_write( SubagentModelSelection, >(value)?))? } + // A raw boolean is not expressible as a legacy model selection, so it is + // passed through unchanged instead of being projected through the enum. + ".subagents.enforce_default" => { + if value.is_boolean() { + value + } else { + return Err(OpenBitFunError::validation( + "ai.agent_model_defaults.subagents.enforce_default must be a boolean", + )); + } + } _ if suffix.starts_with(".subagents.builtin.") => { if value.is_null() { value @@ -1262,6 +1273,98 @@ mod tests { ); } + #[tokio::test] + async fn legacy_subagent_enforce_default_write_lands_on_runtime_selections() { + let (service, dir) = test_service("subagent-enforce-default").await; + service + .add_ai_model(runtime_model("chosen", "fixture-key")) + .await + .unwrap(); + let legacy_before: serde_json::Value = service + .get_config(Some("ai.agent_model_defaults")) + .await + .unwrap(); + assert!(!service + .get_config::(Some("ai.agent_model_selections.subagents.enforce_default")) + .await + .unwrap()); + + assert!(service + .set_config_if_changed( + "ai.agent_model_defaults.subagents.enforce_default", + serde_json::json!(true), + ) + .await + .unwrap()); + assert!(service + .get_config::(Some("ai.agent_model_selections.subagents.enforce_default")) + .await + .unwrap()); + let selections: openbitfun_config_contracts::model_pool::AgentModelSelections = service + .get_config(Some("ai.agent_model_selections")) + .await + .unwrap(); + assert!(selections.subagents.enforce_default); + // The frozen legacy surface is read-only and must not be rewritten. + assert_eq!( + service + .get_config::(Some("ai.agent_model_defaults")) + .await + .unwrap(), + legacy_before + ); + + // Whole-object writes carry the flag through the same translation. + service + .set_config( + "ai.agent_model_defaults.subagents", + serde_json::json!({ + "default": {"kind": "fixed", "model_id": "chosen"}, + "builtin": {}, + "fork": {"kind": "inherit"}, + "enforce_default": true, + }), + ) + .await + .unwrap(); + let selections: openbitfun_config_contracts::model_pool::AgentModelSelections = service + .get_config(Some("ai.agent_model_selections")) + .await + .unwrap(); + assert!(selections.subagents.enforce_default); + assert_eq!( + selections.subagents.default, + openbitfun_config_contracts::model_pool::AgentModelSelection::Model { + model_ref: "chosen".into() + } + ); + + // The flag survives a restart and clearing it restores today's rules. + let restarted = restart_test_service(&dir, "subagent-enforce-default").await; + assert!(restarted + .get_config::(Some("ai.agent_model_selections.subagents.enforce_default")) + .await + .unwrap()); + restarted + .set_config( + "ai.agent_model_defaults.subagents.enforce_default", + serde_json::json!(false), + ) + .await + .unwrap(); + assert!(!restarted + .get_config::(Some("ai.agent_model_selections.subagents.enforce_default")) + .await + .unwrap()); + assert!(service + .set_config( + "ai.agent_model_defaults.subagents.enforce_default", + serde_json::json!("true"), + ) + .await + .is_err()); + } + fn realtime_voice_fixture() -> VoiceCallConfig { VoiceCallConfig { api_key: "fixture-realtime-voice-key".to_string(), diff --git a/src/crates/contracts/config-contracts/src/model_pool.rs b/src/crates/contracts/config-contracts/src/model_pool.rs index 5cc8a27231..5814f2d9eb 100644 --- a/src/crates/contracts/config-contracts/src/model_pool.rs +++ b/src/crates/contracts/config-contracts/src/model_pool.rs @@ -95,6 +95,10 @@ pub struct SubagentModelSelections { pub default: AgentModelSelection, pub builtin: BTreeMap, pub fork: AgentModelSelection, + /// Mirrors `ai.agent_model_defaults.subagents.enforce_default`: pin every + /// internally created subagent to `default`. Missing data means `false`. + #[serde(default)] + pub enforce_default: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -163,6 +167,7 @@ impl Default for AgentModelSelections { AgentModelSelection::Inherit, )]), fork: AgentModelSelection::Inherit, + enforce_default: false, }, } } @@ -183,6 +188,7 @@ impl AgentModelSelections { .map(|(id, choice)| (id.clone(), choice.clone().into())) .collect(), fork: legacy.subagents.fork.clone().into(), + enforce_default: legacy.subagents.enforce_default, }, } } @@ -330,6 +336,60 @@ mod tests { ); } + #[test] + fn subagent_enforce_default_is_absent_by_default_and_round_trips() { + // A document written before the field existed must deserialize to false. + let legacy = json!({ + "mode": {"kind": "model", "model_ref": "primary"}, + "subagents": { + "default": {"kind": "model", "model_ref": "fast"}, + "builtin": {}, + "fork": {"kind": "inherit"} + } + }); + let selections: AgentModelSelections = serde_json::from_value(legacy.clone()).unwrap(); + assert!(!selections.subagents.enforce_default); + assert_eq!( + serde_json::to_value(&selections).unwrap()["subagents"]["enforce_default"], + json!(false) + ); + + let mut explicit = selections.clone(); + explicit.subagents.enforce_default = true; + let saved = serde_json::to_value(&explicit).unwrap(); + assert_eq!(saved["subagents"]["enforce_default"], json!(true)); + assert_eq!( + serde_json::from_value::(saved).unwrap(), + explicit + ); + + // An explicit `false` is still a valid document and keeps today's rules. + let mut disabled = legacy; + disabled["subagents"]["enforce_default"] = json!(false); + assert!( + !serde_json::from_value::(disabled) + .unwrap() + .subagents + .enforce_default + ); + } + + #[test] + fn from_legacy_carries_the_enforced_subagent_default_flag() { + for enforce_default in [false, true] { + let mut legacy = crate::types::AIConfig::default(); + legacy.agent_model_defaults.subagents.enforce_default = enforce_default; + let migrated = AgentModelSelections::from_legacy(&legacy.agent_model_defaults); + assert_eq!(migrated.subagents.enforce_default, enforce_default); + assert_eq!( + migrated.subagents.default, + AgentModelSelection::Model { + model_ref: "fast".into() + } + ); + } + } + #[test] fn malformed_new_selection_is_retained_and_never_read_as_legacy() { for malformed in [json!(null), json!({}), json!({"mode":{"kind":"future"}})] { diff --git a/src/crates/contracts/config-contracts/src/types.rs b/src/crates/contracts/config-contracts/src/types.rs index 884b96f659..2be03002b1 100644 --- a/src/crates/contracts/config-contracts/src/types.rs +++ b/src/crates/contracts/config-contracts/src/types.rs @@ -1066,6 +1066,10 @@ pub struct SubagentModelDefaultsConfig { pub builtin: HashMap, /// Default choice for a child created from the parent's context. pub fork: SubagentModelSelection, + /// When true every internally created subagent uses `default`, ignoring + /// explicit requests, `fork` and per-builtin overrides. Absent means false. + #[serde(default)] + pub enforce_default: bool, } impl Default for SubagentModelDefaultsConfig { @@ -1077,6 +1081,7 @@ impl Default for SubagentModelDefaultsConfig { SubagentModelSelection::Inherit, )]), fork: SubagentModelSelection::Inherit, + enforce_default: false, } } } @@ -4137,6 +4142,53 @@ mod tests { ); } + #[test] + fn subagent_enforce_default_defaults_off_and_round_trips() { + assert!(!super::SubagentModelDefaultsConfig::default().enforce_default); + assert!( + !AgentModelDefaultsConfig::default() + .subagents + .enforce_default + ); + + // Absent field (older configuration) keeps today's resolution rules. + for payload in [ + serde_json::json!({}), + serde_json::json!({"subagents": {}}), + serde_json::json!({"subagents": {"default": {"kind": "inherit"}}}), + ] { + let config: AgentModelDefaultsConfig = serde_json::from_value(payload).unwrap(); + assert!(!config.subagents.enforce_default); + } + + let config: AgentModelDefaultsConfig = serde_json::from_value(serde_json::json!({ + "subagents": {"enforce_default": true} + })) + .unwrap(); + assert!(config.subagents.enforce_default); + let saved = serde_json::to_value(&config).unwrap(); + assert_eq!( + saved["subagents"]["enforce_default"], + serde_json::json!(true) + ); + let reloaded: AgentModelDefaultsConfig = serde_json::from_value(saved).unwrap(); + assert!(reloaded.subagents.enforce_default); + + // A damaged value must stay an error instead of silently enforcing. + for invalid in [ + serde_json::json!("true"), + serde_json::Value::Null, + serde_json::json!(1), + ] { + assert!( + serde_json::from_value::(serde_json::json!({ + "subagents": {"enforce_default": invalid} + })) + .is_err() + ); + } + } + #[test] fn code_font_defaults_follow_frontend_and_preserve_explicit_legacy_fonts() { let editor: EditorConfig = serde_json::from_str("{}").unwrap(); diff --git a/src/web-ui/src/app/scenes/settings/pages/ai/ExecutionSettingsPage.tsx b/src/web-ui/src/app/scenes/settings/pages/ai/ExecutionSettingsPage.tsx index a8d0f49d3e..5cc3ec2e9d 100644 --- a/src/web-ui/src/app/scenes/settings/pages/ai/ExecutionSettingsPage.tsx +++ b/src/web-ui/src/app/scenes/settings/pages/ai/ExecutionSettingsPage.tsx @@ -77,6 +77,7 @@ const ExecutionSettingsPage: React.FC = () => { } const hasSeededPageData = seed.loaded && Boolean(seededSelections); const [subagentDefaultModel, setSubagentDefaultModel] = useState(() => seededSelections?.subagents.default ?? { kind: 'inherit' }); + const [enforceSubagentDefaultModel, setEnforceSubagentDefaultModel] = useState(() => seededSelections?.subagents.enforce_default === true); const [modelPools, setModelPools] = useState(() => { const document = seed.get('ai.model_pools', null); return document ? readableModelPools(document) : []; @@ -224,6 +225,31 @@ const ExecutionSettingsPage: React.FC = () => { } }; + const handleSubagentModelEnforcementChange = async (checked: boolean) => { + const scope = getActiveSurfaceScope(); + const previousEnforcement = enforceSubagentDefaultModel; + setEnforceSubagentDefaultModel(checked); + setToolExecConfigLoading(true); + try { + if (modelSelectionsConfigPath() === 'ai.agent_model_selections') { + await configManager.setConfig('ai.agent_model_selections.subagents.enforce_default', checked); + } else { + await configManager.setConfig('ai.agent_model_defaults.subagents.enforce_default', checked); + } + if (!scope.isCurrent()) return; + notificationService.success(tTools('messages.saveSuccess'), { duration: 2000 }); + } catch (error) { + if (!scope.isCurrent()) return; + log.error('Failed to save subagent model enforcement', error); + setEnforceSubagentDefaultModel(previousEnforcement); + notificationService.error( + `${tTools('messages.saveFailed')}: ${error instanceof Error ? error.message : String(error)}` + ); + } finally { + if (scope.isCurrent()) setToolExecConfigLoading(false); + } + }; + const handleSwarmMaxConcurrencyChange = async (input: number) => { if (!Number.isFinite(input)) return; const value = Math.min(SWARM_MAX_CONCURRENCY_LIMIT, Math.max(1, Math.round(input))); @@ -332,6 +358,7 @@ const ExecutionSettingsPage: React.FC = () => { supportsModelPools() ? configManager.getOptionalConfig('ai.model_pools') : Promise.resolve(undefined), ]); setSubagentDefaultModel(loadedSubagentDefaultModel.subagents.default); + setEnforceSubagentDefaultModel(loadedSubagentDefaultModel.subagents.enforce_default === true); setModelPools(poolDocument ? readableModelPools(poolDocument) : []); setConfiguredModels(loadedModels ?? []); setEnableDeferredToolLoading(deferredToolLoadingEnabled ?? true); @@ -512,6 +539,21 @@ const ExecutionSettingsPage: React.FC = () => { /> + +
+ void handleSubagentModelEnforcementChange(event.target.checked)} + disabled={toolExecConfigLoading} + /> +
+ { Object.defineProperty(window, '__TAURI__', { configurable: true, value: {} }); return { - invoke: vi.fn(), setConfig: vi.fn(), setEnabled: vi.fn(), getConfig: vi.fn(), error: vi.fn(), + invoke: vi.fn(), setConfig: vi.fn(), setEnabled: vi.fn(), getConfig: vi.fn(), getOptionalConfig: vi.fn(), error: vi.fn(), t: (key: string) => key, workspace: null as { workspaceKind: string } | null, }; }); + +/** Same fixture the previous inline mock returned for the runtime model-selection path. */ +const defaultOptionalConfig = async (path: string) => path === 'ai.agent_model_selections' + ? { mode: { kind: 'model', model_ref: 'primary' }, subagents: { default: { kind: 'model', model_ref: 'fast' }, builtin: {}, fork: { kind: 'inherit' } } } + : true; vi.mock('@/infrastructure/config/hooks', () => ({ useAIExperienceSettings: () => ({ settings: { enable_workspace_search: true }, isLoading: false, error: null, reload: vi.fn() }) })); vi.mock('@/infrastructure/api', () => ({ globalAPI: {}, workspaceAPI: {} })); vi.mock('@/infrastructure/contexts/WorkspaceContext', () => ({ useCurrentWorkspace: () => ({ workspace: mocks.workspace }) })); @@ -26,7 +32,7 @@ vi.mock('@/infrastructure/i18n', () => ({ vi.mock('@/infrastructure/api/service-api/ApiClient', () => ({ api: { invoke: mocks.invoke } })); vi.mock('@/infrastructure/api/service-api/SystemAPI', () => ({ systemAPI: { getSystemInfo: async () => ({ platform: 'macos' }) } })); vi.mock('../../../../../infrastructure/config/services/ConfigManager', () => ({ configManager: { hasCachedConfig: () => false, getCachedConfig: () => undefined, - getConfig: mocks.getConfig, getOptionalConfig: async (path: string) => path === 'ai.agent_model_selections' ? { mode: { kind: 'model', model_ref: 'primary' }, subagents: { default: { kind: 'model', model_ref: 'fast' }, builtin: {}, fork: { kind: 'inherit' } } } : true, setConfig: mocks.setConfig, + getConfig: mocks.getConfig, getOptionalConfig: mocks.getOptionalConfig, setConfig: mocks.setConfig, } })); vi.mock('../../../../../infrastructure/config/hooks/useComputerUseEnabled', () => ({ useComputerUseEnabled: () => ({ computerUseEnabled: false, setComputerUseEnabled: mocks.setEnabled }) })); vi.mock('../../../../../infrastructure/config/services/AIExperienceConfigService', () => ({ aiExperienceConfigService: { getCachedSettings: () => null, getSettingsAsync: async () => ({ enable_workspace_search: true }) } })); @@ -68,6 +74,7 @@ beforeEach(() => { (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; status = { ...readyStatus }; mocks.getConfig.mockReset().mockResolvedValue(null); + mocks.getOptionalConfig.mockReset().mockImplementation(defaultOptionalConfig); mocks.setConfig.mockReset().mockResolvedValue(undefined); mocks.error.mockReset(); mocks.invoke.mockReset().mockImplementation(async (command: string) => { @@ -304,3 +311,63 @@ describe('Flashgrep settings availability', () => { expect(container.textContent?.includes('features.workspaceSearch.enable')).toBe(workspaceKind === 'normal'); }); }); + +describe('Subagent model enforcement', () => { + const enforcementRow = () => Array.from(container.querySelectorAll('[data-openbitfun-part="row"]')) + .find(row => row.querySelector('[data-openbitfun-part="rowLabel"]')?.textContent === 'config.subagentModelEnforcement')!; + const enforceSwitch = () => enforcementRow().querySelector('input[role="switch"]')!; + const rowDescription = () => enforcementRow().querySelector('[data-openbitfun-part="rowDescription"]')!.textContent; + + function runtimeSelections(subagents: Record) { + return { mode: { kind: 'model', model_ref: 'primary' }, subagents: { default: { kind: 'model', model_ref: 'fast' }, builtin: {}, fork: { kind: 'inherit' }, ...subagents } }; + } + + it('renders the stored state and writes the runtime path on a local host', async () => { + mocks.getOptionalConfig.mockImplementation(async (path: string) => path === 'ai.agent_model_selections' + ? runtimeSelections({ enforce_default: true }) + : defaultOptionalConfig(path)); + await act(async () => root.render()); + expect(enforceSwitch().checked).toBe(true); + expect(rowDescription()).toContain('config.subagentModelEnforcementDesc'); + mocks.setConfig.mockClear(); + + await act(async () => enforceSwitch().click()); + expect(mocks.setConfig).toHaveBeenCalledExactlyOnceWith('ai.agent_model_selections.subagents.enforce_default', false); + expect(enforceSwitch().checked).toBe(false); + expect(mocks.error).not.toHaveBeenCalled(); + }); + + it('restores the switch and reports the host error when the write is rejected', async () => { + mocks.setConfig.mockRejectedValue(new Error('Host rejected the subagent policy')); + await act(async () => root.render()); + await act(async () => enforceSwitch().click()); + expect(mocks.error).toHaveBeenCalledWith('messages.saveFailed: Host rejected the subagent policy'); + expect(enforceSwitch().checked).toBe(false); + }); + + it('reads and writes the legacy typed path on a host without runtime selections', async () => { + await act(async () => activateSurface('peer-legacy')); + try { + mocks.getOptionalConfig.mockImplementation(async (path: string) => path === 'ai.agent_model_defaults' + ? { mode: 'primary', subagents: { default: { kind: 'fixed', model_id: 'fast' }, builtin: {}, fork: { kind: 'inherit' }, enforce_default: true } } + : defaultOptionalConfig(path)); + await act(async () => root.render()); + expect(enforceSwitch().checked).toBe(true); + + await act(async () => enforceSwitch().click()); + expect(mocks.setConfig).toHaveBeenCalledExactlyOnceWith('ai.agent_model_defaults.subagents.enforce_default', false); + expect(mocks.error).not.toHaveBeenCalled(); + } finally { + await act(async () => activateSurface('local')); + } + }); + + it('warns when enforcement would leave subagents on the session model', async () => { + mocks.getOptionalConfig.mockImplementation(async (path: string) => path === 'ai.agent_model_selections' + ? runtimeSelections({ default: { kind: 'inherit' }, enforce_default: true }) + : defaultOptionalConfig(path)); + await act(async () => root.render()); + expect(enforceSwitch().checked).toBe(true); + expect(rowDescription()).toContain('config.subagentModelEnforcementInheritWarning'); + }); +}); diff --git a/src/web-ui/src/infrastructure/config/services/modelSelections.test.ts b/src/web-ui/src/infrastructure/config/services/modelSelections.test.ts index a3a10a2005..5193c51dc3 100644 --- a/src/web-ui/src/infrastructure/config/services/modelSelections.test.ts +++ b/src/web-ui/src/infrastructure/config/services/modelSelections.test.ts @@ -13,8 +13,14 @@ describe('typed model preferences', () => { expect(result.mode).toEqual({ kind: 'model', model_ref: 'a' }); expect(result.subagents.default).toEqual({ kind: 'model', model_ref: 'fast' }); expect(result.subagents.builtin.ResearchSpecialist).toEqual({ kind: 'inherit' }); + expect(result.subagents.enforce_default).toBe(false); expect(legacy.mode).toBe('a'); }); + it('carries the legacy enforcement flag next to the converted selection', () => { + const enforced: AgentModelDefaultsConfig = { ...legacy, subagents: { ...legacy.subagents, enforce_default: true } }; + expect(effectiveModelSelections(enforced, 'ai.agent_model_defaults').subagents.enforce_default).toBe(true); + expect(legacy.subagents.enforce_default).toBeUndefined(); + }); it('rejects malformed selections and read failures instead of falling back', async () => { expect(() => effectiveModelSelections({} as any)).toThrow('Invalid'); mocks.get.mockRejectedValue(new Error('offline')); diff --git a/src/web-ui/src/infrastructure/config/services/modelSelections.ts b/src/web-ui/src/infrastructure/config/services/modelSelections.ts index b579bb8b1b..338c19c4bc 100644 --- a/src/web-ui/src/infrastructure/config/services/modelSelections.ts +++ b/src/web-ui/src/infrastructure/config/services/modelSelections.ts @@ -31,6 +31,9 @@ export function effectiveModelSelections(config?: AgentModelDefaultsConfig | Age default: convert(defaults?.subagents?.default ?? { kind: 'fixed', model_id: 'fast' }), builtin: Object.fromEntries(Object.entries(defaults?.subagents?.builtin ?? {}).map(([id, choice]) => [id, convert(choice)])), fork: convert(defaults?.subagents?.fork ?? { kind: 'inherit' }), + // The host copies this flag when it migrates the legacy typed path; mirror + // that so readers see the enforcement state on both dialects. + enforce_default: defaults?.subagents?.enforce_default === true, }, }; } diff --git a/src/web-ui/src/infrastructure/config/types/index.ts b/src/web-ui/src/infrastructure/config/types/index.ts index e64b01be6f..96c23dbd01 100644 --- a/src/web-ui/src/infrastructure/config/types/index.ts +++ b/src/web-ui/src/infrastructure/config/types/index.ts @@ -474,6 +474,8 @@ export interface AgentModelSelections { default: AgentModelTargetSelection; builtin: Record; fork: AgentModelTargetSelection; + /** `ai.agent_model_selections.subagents.enforce_default`; absent means false. */ + enforce_default?: boolean; }; } @@ -483,6 +485,8 @@ export interface AgentModelDefaultsConfig { default: SubagentModelSelection; builtin: Record; fork: SubagentModelSelection; + /** `ai.agent_model_defaults.subagents.enforce_default`; absent means false. */ + enforce_default?: boolean; }; } diff --git a/src/web-ui/src/locales/en-US/settings/agentic-tools.json b/src/web-ui/src/locales/en-US/settings/agentic-tools.json index 33d06d81b1..17bd875b19 100644 --- a/src/web-ui/src/locales/en-US/settings/agentic-tools.json +++ b/src/web-ui/src/locales/en-US/settings/agentic-tools.json @@ -42,7 +42,10 @@ "subagentModelSettings": "Go to the Agents page to set models for individual subagents", "subagentModelInherit": "Inherit parent session", "subagentDefaultModel": "Default subagent model", - "subagentDefaultModelDesc": "Used by subagents without an individual model override." + "subagentDefaultModelDesc": "Used by subagents without an individual model override.", + "subagentModelEnforcement": "Always use this model for subagents", + "subagentModelEnforcementDesc": "When on, every subagent, including forked child sessions, is locked to the model above: the session's own model and any model requested by a task are ignored", + "subagentModelEnforcementInheritWarning": "The model above is set to inherit the parent session, so subagents would still follow the session model. Choose a specific model first" }, "filters": { "all": "All", diff --git a/src/web-ui/src/locales/zh-CN/settings/agentic-tools.json b/src/web-ui/src/locales/zh-CN/settings/agentic-tools.json index 58e046be99..b95ba98153 100644 --- a/src/web-ui/src/locales/zh-CN/settings/agentic-tools.json +++ b/src/web-ui/src/locales/zh-CN/settings/agentic-tools.json @@ -42,7 +42,10 @@ "subagentModelSettings": "前往智能体页面为子智能体单独设置模型", "subagentModelInherit": "继承父会话", "subagentDefaultModel": "子智能体默认模型", - "subagentDefaultModelDesc": "未单独指定模型的子智能体使用此设置。" + "subagentDefaultModelDesc": "未单独指定模型的子智能体使用此设置。", + "subagentModelEnforcement": "强制子智能体固定使用该模型", + "subagentModelEnforcementDesc": "开启后，所有子智能体（含分叉出的子会话）都固定使用上方选择的模型，会话自身模型和任务级模型请求都会被忽略", + "subagentModelEnforcementInheritWarning": "上方选择的是“继承父会话”，开启后子智能体仍会跟随会话模型。请先改选一个具体模型" }, "filters": { "all": "全部", diff --git a/src/web-ui/src/locales/zh-TW/settings/agentic-tools.json b/src/web-ui/src/locales/zh-TW/settings/agentic-tools.json index 4a4be6da39..6363a2638b 100644 --- a/src/web-ui/src/locales/zh-TW/settings/agentic-tools.json +++ b/src/web-ui/src/locales/zh-TW/settings/agentic-tools.json @@ -42,7 +42,10 @@ "subagentModelSettings": "前往智慧體頁面為子智慧體個別設定模型", "subagentModelInherit": "繼承父會話", "subagentDefaultModel": "子智能體預設模型", - "subagentDefaultModelDesc": "未個別指定模型的子智慧體使用此設定。" + "subagentDefaultModelDesc": "未個別指定模型的子智慧體使用此設定。", + "subagentModelEnforcement": "強制子智慧體固定使用該模型", + "subagentModelEnforcementDesc": "開啟後，所有子智慧體（含分叉出的子會話）都固定使用上方選擇的模型，會話自身模型與任務級模型請求都會被忽略", + "subagentModelEnforcementInheritWarning": "上方選擇的是「繼承父會話」，開啟後子智慧體仍會跟隨會話模型。請先改選一個具體模型" }, "filters": { "all": "全部",