From 0591b50b6febf325bdffc9fef84dfe6be1f2fb5d Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Thu, 23 Jul 2026 14:22:30 -0700 Subject: [PATCH] Scope defaults to visible harness fallbacks Signed-off-by: kenny lopez --- .../src-tauri/src/commands/agents_tests.rs | 18 ++++++ .../src/managed_agents/global_config/mod.rs | 47 +++++++++++++++ .../src/managed_agents/global_config/tests.rs | 57 +++++++++++++++++++ desktop/src-tauri/src/managed_agents/mod.rs | 4 +- desktop/src/features/agents/AGENTS.md | 9 ++- .../lib/instanceInputForDefinition.test.mjs | 4 ++ .../agents/lib/instanceInputForDefinition.ts | 15 ++++- .../lib/runtimeVisibilityPreference.test.mjs | 8 ++- .../agents/lib/runtimeVisibilityPreference.ts | 20 +++++-- .../agents/ui/AddTeamToChannelDialog.tsx | 5 ++ 10 files changed, 174 insertions(+), 13 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index e32fc1cfe..3897c99d8 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -217,6 +217,24 @@ fn deploy_resolver_returns_none_for_orphaned_instance() { ); } +#[test] +fn deploy_resolver_ignores_defaults_from_a_different_implicit_runtime() { + let mut record = bare_agent_record(Some("p1"), None, None); + record.agent_command_override = Some("goose".to_string()); + let personas = vec![persona_record("p1", None, None)]; + let global = crate::managed_agents::GlobalAgentConfig { + model: Some("auto".to_string()), + provider: Some("relay-mesh".to_string()), + preferred_runtime: Some("buzz-agent".to_string()), + ..Default::default() + }; + + assert_eq!( + resolve_deploy_model_provider(&record, &personas, &global), + (None, None) + ); +} + #[test] fn normalize_relay_mesh_rejects_empty_model_ref() { let config = RelayMeshConfig { diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 162f44798..4d4d51423 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -207,6 +207,53 @@ pub fn save_global_agent_config(app: &AppHandle, config: &GlobalAgentConfig) -> atomic_write_json_restricted(&path, &payload) } +/// Return the global provider/model values that are safe for this record. +/// +/// A runtime-less definition can be started on a fallback runtime when its +/// saved global preference is hidden or unavailable. In that case the selected +/// command is pinned on the record, and provider/model defaults belonging to a +/// different preferred runtime must not cross the harness boundary. Explicitly +/// configured definitions and standalone agents keep normal global inheritance. +pub(crate) fn global_model_provider_for_record<'a>( + record: &ManagedAgentRecord, + personas: &[AgentDefinition], + global: &'a GlobalAgentConfig, +) -> (Option<&'a str>, Option<&'a str>) { + let global_values = (global.model.as_deref(), global.provider.as_deref()); + if record.persona_id.is_none() { + return global_values; + } + + let definition_runtime = record.runtime.as_deref().or_else(|| { + record + .persona_id + .as_deref() + .and_then(|id| personas.iter().find(|persona| persona.id == id)) + .and_then(|persona| persona.runtime.as_deref()) + }); + if definition_runtime.is_some_and(|runtime| !runtime.trim().is_empty()) { + return global_values; + } + + let Some(selected_runtime) = record + .agent_command_override + .as_deref() + .and_then(crate::managed_agents::known_acp_runtime) + else { + return global_values; + }; + let preferred_runtime = global + .preferred_runtime + .as_deref() + .and_then(crate::managed_agents::known_acp_runtime); + + if preferred_runtime.is_some_and(|preferred| std::ptr::eq(preferred, selected_runtime)) { + global_values + } else { + (None, None) + } +} + /// Resolve the effective model and provider for an agent. /// /// Delegates to `effective_config::resolve_effective_config` which enforces diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 33b93d8a5..5f50cdca4 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -472,6 +472,63 @@ fn resolve_global_fallback_when_record_and_persona_have_none() { ); } +#[test] +fn runtime_fallback_does_not_inherit_defaults_from_a_different_preferred_runtime() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + record.agent_command_override = Some("goose".to_string()); + let personas = vec![persona("p1", None, None)]; + let global = GlobalAgentConfig { + model: Some("auto".to_string()), + provider: Some("relay-mesh".to_string()), + preferred_runtime: Some("buzz-agent".to_string()), + ..Default::default() + }; + + assert_eq!( + resolve_effective_model_provider(&record, &personas, &global), + (None, None) + ); +} + +#[test] +fn runtime_fallback_inherits_defaults_when_it_matches_the_preferred_runtime() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + record.agent_command_override = Some("goose".to_string()); + let personas = vec![persona("p1", None, None)]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + preferred_runtime: Some("goose".to_string()), + ..Default::default() + }; + + assert_eq!( + resolve_effective_model_provider(&record, &personas, &global), + (Some("global-model"), Some("global-provider")) + ); +} + +#[test] +fn explicit_runtime_keeps_global_defaults_when_another_runtime_is_preferred() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + record.runtime = Some("goose".to_string()); + let personas = vec![persona("p1", None, None)]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + preferred_runtime: Some("buzz-agent".to_string()), + ..Default::default() + }; + + assert_eq!( + resolve_effective_model_provider(&record, &personas, &global), + (Some("global-model"), Some("global-provider")) + ); +} + /// Tier 4 — no persona linked: record.persona_id is None, record has no /// model/provider; global defaults must still fill in (persona lookup skipped). #[cfg(feature = "mesh-llm")] diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index b0e86f8ed..105f9c191 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -52,8 +52,8 @@ pub use env_vars::*; pub(crate) use git_bash::git_bash_available; pub(crate) use git_bash::{discover_git_bash, GitBashPrerequisite}; pub(crate) use global_config::{ - load_global_agent_config, resolve_effective_model_provider, save_global_agent_config, - validate_global_config, GlobalAgentConfig, + global_model_provider_for_record, load_global_agent_config, resolve_effective_model_provider, + save_global_agent_config, validate_global_config, GlobalAgentConfig, }; pub(crate) use managed_node_paths::*; pub use nest::*; diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index bae3389d5..7a226b8a5 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -93,9 +93,12 @@ with a TypeScript lookup table or an id comparison in a component. runtime off must not uninstall it, stop it, or invalidate an existing agent that already uses it. If the disabled runtime was the saved global default, consumers immediately ignore that preference and the defaults - editor persists its visible fallback on the next save. Runtime-less agent - starts and team deploys must filter through the same visible-runtime set; - definitions already pinned to a hidden runtime remain runnable. + editor persists its visible fallback on the next save. Its dependent + provider/model defaults are also ignored for new implicit fallback agents, + without changing the persisted configuration used by existing agents. + Runtime-less agent starts and team deploys must filter through the same + visible-runtime set; definitions already pinned to a hidden runtime remain + runnable. 10. **The defaults modal is progressively disclosed.** An unset global config starts on the Buzz Agent-first deployment fallback and carries that visible harness into the next saved edit. The `progressive-defaults` disclosure diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs index d79c7abcf..4496b01ce 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs @@ -5,6 +5,7 @@ import { availableRuntimesForStart, buildInstanceInputForDefinition, resolveStartRuntimeForDefinition, + shouldPinSelectedRuntimeForDefinition, } from "./instanceInputForDefinition.ts"; // ── Phase 1B.3.5: the single definition→instance mapping ──────────────────── @@ -74,6 +75,9 @@ test("row 4: create input never contains definition env vars", async () => { }); test("row 2: harnessOverride follows the backend-aligned formula", async () => { + assert.equal(shouldPinSelectedRuntimeForDefinition(undefined, "goose"), true); + assert.equal(shouldPinSelectedRuntimeForDefinition("claude", "goose"), false); + const match = await buildInstanceInputForDefinition( persona({ runtime: "goose" }), gooseRuntime, diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.ts b/desktop/src/features/agents/lib/instanceInputForDefinition.ts index 591930922..e5012e467 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.ts +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.ts @@ -88,6 +88,16 @@ export type BackendIntent = { config: Record; }; +/** Keep every definition-start surface aligned on runtime pinning semantics. */ +export function shouldPinSelectedRuntimeForDefinition( + definitionRuntimeId: string | null | undefined, + selectedRuntimeId: string, +): boolean { + return ( + !definitionRuntimeId || definitionRuntimeId.trim() === selectedRuntimeId + ); +} + /** * The single definition→instance mapping (Phase 1B.3.5 rows 2–4). Every * surface that creates a running instance from a definition builds its @@ -151,7 +161,10 @@ export async function buildInstanceInputForDefinition( // at top of this function). agentArgs: [], mcpCommand: runtime.mcpCommand ?? "", - harnessOverride: !persona.runtime || persona.runtime === runtime.id, + harnessOverride: shouldPinSelectedRuntimeForDefinition( + persona.runtime, + runtime.id, + ), model: persona.model ?? undefined, provider: persona.provider ?? undefined, spawnAfterCreate: true, diff --git a/desktop/src/features/agents/lib/runtimeVisibilityPreference.test.mjs b/desktop/src/features/agents/lib/runtimeVisibilityPreference.test.mjs index 629f28559..662fbeb2d 100644 --- a/desktop/src/features/agents/lib/runtimeVisibilityPreference.test.mjs +++ b/desktop/src/features/agents/lib/runtimeVisibilityPreference.test.mjs @@ -66,16 +66,18 @@ test("stored runtime visibility is read from the versioned device key", () => { assert.deepEqual(readDisabledAcpRuntimeIds(storage), ["claude"]); }); -test("a disabled saved runtime is removed from the effective preference", () => { +test("a disabled saved runtime and its dependent defaults are masked", () => { const config = { env_vars: {}, - provider: null, - model: null, + provider: "relay-mesh", + model: "auto", preferred_runtime: "Goose", }; assert.deepEqual(maskDisabledAcpRuntimePreference(config, ["goose"]), { ...config, + provider: null, + model: null, preferred_runtime: null, }); assert.equal(maskDisabledAcpRuntimePreference(config, ["claude"]), config); diff --git a/desktop/src/features/agents/lib/runtimeVisibilityPreference.ts b/desktop/src/features/agents/lib/runtimeVisibilityPreference.ts index dcda74a41..361259b39 100644 --- a/desktop/src/features/agents/lib/runtimeVisibilityPreference.ts +++ b/desktop/src/features/agents/lib/runtimeVisibilityPreference.ts @@ -110,13 +110,20 @@ export function runtimesForImplicitAcpSelection( } /** - * Prevent a disabled runtime from remaining the effective global preference. + * Prevent a disabled runtime and its dependent defaults from remaining + * effective for new implicit selections. * * The persisted config is left untouched until the user next saves defaults; - * consumers immediately fall back through the normal runtime selection path. + * existing agents keep their configuration while new consumers immediately + * fall back through the normal runtime selection path without carrying a + * provider or model selected for the hidden harness. */ export function maskDisabledAcpRuntimePreference< - T extends { preferred_runtime: string | null }, + T extends { + model: string | null; + preferred_runtime: string | null; + provider: string | null; + }, >(config: T, disabledRuntimeIds: readonly string[]): T { const preferredRuntime = config.preferred_runtime; if ( @@ -128,7 +135,12 @@ export function maskDisabledAcpRuntimePreference< return config; } - return { ...config, preferred_runtime: null }; + return { + ...config, + model: null, + preferred_runtime: null, + provider: null, + }; } function getDisabledRuntimeIdsSnapshot(): readonly string[] { diff --git a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx index 8919666a3..c105389ae 100644 --- a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx +++ b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx @@ -11,6 +11,7 @@ import { emptyResolvedTeamPersonas, resolveTeamPersonas, } from "@/features/agents/lib/teamPersonas"; +import { shouldPinSelectedRuntimeForDefinition } from "@/features/agents/lib/instanceInputForDefinition"; import { useSelectableAcpRuntimes } from "@/features/agents/lib/runtimeVisibilityPreference"; import { collectRuntimeWarnings, @@ -147,6 +148,10 @@ export function AddTeamToChannelDialog({ name: persona.displayName, systemPrompt: persona.systemPrompt, avatarUrl: persona.avatarUrl ?? undefined, + harnessOverride: shouldPinSelectedRuntimeForDefinition( + persona.runtime, + runtimeToUse.id, + ), model: persona.model ?? undefined, personaId: persona.id, teamId: team.id,