diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 09c464675..ab9baa719 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -11,7 +11,9 @@ pub(crate) struct KnownAcpRuntime { pub commands: &'static [&'static str], pub aliases: &'static [&'static str], pub avatar_url: &'static str, - /// MCP server binary for this runtime, or `None` for no MCP server. + /// MCP server binary to use instead of the default `sprout-mcp-server`. + /// `None` means this runtime does not need a Sprout MCP server — + /// no MCP tools will be registered for the agent session. pub mcp_command: Option<&'static str>, /// Whether to enable MCP hook tools (`_Stop`, `_PostCompact`) for this agent. pub mcp_hooks: bool, @@ -32,12 +34,16 @@ pub(crate) struct KnownAcpRuntime { /// pointing to the canonical `.agents/skills/sprout-cli`. `None` → this /// runtime reads the canonical path directly or has no skill support. pub skill_dir: Option<&'static str>, + // Phase 3: these fields are consumed by runtime.rs spawn logic to replace ad-hoc env var injection. + /// Whether this runtime supports ACP model switching mid-session. pub supports_acp_model_switching: bool, + /// Environment variable name used to set the model for this runtime, if any. pub model_env_var: Option<&'static str>, - #[allow(dead_code)] + /// Environment variable name used to set the LLM provider for this runtime, if any. pub provider_env_var: Option<&'static str>, - #[allow(dead_code)] + /// Whether the LLM provider is locked (not user-selectable) for this runtime. pub provider_locked: bool, + /// Default environment variables injected when spawning agents using this runtime. pub default_env: &'static [(&'static str, &'static str)], } diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 8b1436eb2..975c27d00 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -6,6 +6,11 @@ //! `SPROUT_SHARE_IDENTITY=1` and `SPROUT_PRIVATE_KEY` is set. All dev //! instances share the same physical files — edits in any worktree are //! immediately visible to all others. +//! +//! **Provider reconciliation** (`reconcile_provider_mcp_commands`): Per-launch +//! fix-up of `mcp_command` values in `managed-agents.json` against the +//! discovery table. Ensures known providers always have their canonical +//! `mcp_command`; unknown/custom agents are left untouched. use std::path::{Path, PathBuf}; use tauri::Manager; @@ -259,6 +264,53 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) { } } +fn reconcile_mcp_commands_in_file(path: &Path) { + patch_json_records(path, |obj| { + let agent_command = match obj.get("agent_command").and_then(|v| v.as_str()) { + Some(cmd) => cmd.to_string(), + None => return false, + }; + let Some(runtime) = crate::managed_agents::known_acp_runtime(&agent_command) else { + return false; + }; + let expected = runtime.mcp_command.unwrap_or(""); + let current = obj + .get("mcp_command") + .and_then(|v| v.as_str()) + .unwrap_or(""); + if current != expected { + eprintln!( + "sprout-desktop: runtime-reconcile: {:?} ({:?}): mcp_command {:?} → {:?}", + obj.get("name").and_then(|v| v.as_str()).unwrap_or("?"), + agent_command, + current, + expected, + ); + obj.insert( + "mcp_command".to_string(), + serde_json::Value::String(expected.to_string()), + ); + true + } else { + false + } + }); +} + +/// Reconcile `mcp_command` values in managed-agents.json against the +/// discovery table. Known runtimes get their canonical mcp_command; +/// unknown/custom agents are left untouched. +pub fn reconcile_provider_mcp_commands(app: &tauri::AppHandle) { + let Ok(dir) = app.path().app_data_dir() else { + return; + }; + let path = dir.join("agents/managed-agents.json"); + if !path.exists() { + return; + } + reconcile_mcp_commands_in_file(&path); +} + fn reconcile_pack_paths_in_file(path: &Path, canonical_dir: &Path) { let canonical_packs = canonical_dir.join("agents/packs"); patch_json_records(path, |obj| { @@ -638,6 +690,169 @@ mod tests { serde_json::from_str(&content).unwrap() } + #[test] + fn reconcile_clears_mcp_command_for_goose() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Scout", + "agent_command": "goose", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], ""); + } + + #[test] + fn reconcile_clears_mcp_command_for_claude() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Claude Agent", + "agent_command": "claude-agent-acp", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], ""); + } + + #[test] + fn reconcile_preserves_sprout_dev_mcp() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": "sprout-dev-mcp" + }]), + ); + let before = + std::fs::read_to_string(dir.path().join("agents/managed-agents.json")).unwrap(); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let after = std::fs::read_to_string(dir.path().join("agents/managed-agents.json")).unwrap(); + assert_eq!( + before, after, + "file should not be rewritten when already correct" + ); + } + + #[test] + fn reconcile_fixes_sprout_agent_if_stale() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } + + #[test] + fn reconcile_leaves_unknown_agent_untouched() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Custom Bot", + "agent_command": "my-custom-agent", + "mcp_command": "my-custom-mcp" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "my-custom-mcp"); + } + + #[test] + fn reconcile_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Scout", + "agent_command": "goose", + "mcp_command": "sprout-mcp-server" + }]), + ); + let path = dir.path().join("agents/managed-agents.json"); + reconcile_mcp_commands_in_file(&path); + let after_first = std::fs::read_to_string(&path).unwrap(); + reconcile_mcp_commands_in_file(&path); + let after_second = std::fs::read_to_string(&path).unwrap(); + assert_eq!(after_first, after_second); + } + + #[test] + fn reconcile_handles_mixed_records() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([ + {"name": "Scout", "agent_command": "goose", "mcp_command": "sprout-mcp-server"}, + {"name": "Claude", "agent_command": "claude-agent-acp", "mcp_command": "sprout-mcp-server"}, + {"name": "Solo", "agent_command": "sprout-agent", "mcp_command": "sprout-dev-mcp"}, + {"name": "Custom", "agent_command": "my-bot", "mcp_command": "my-mcp"}, + {"name": "Codex", "agent_command": "codex-acp", "mcp_command": "sprout-mcp-server"} + ]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "", "goose should be cleared"); + assert_eq!(records[1]["mcp_command"], "", "claude should be cleared"); + assert_eq!( + records[2]["mcp_command"], "sprout-dev-mcp", + "sprout-agent preserved" + ); + assert_eq!( + records[3]["mcp_command"], "my-mcp", + "custom agent untouched" + ); + assert_eq!(records[4]["mcp_command"], "", "codex should be cleared"); + } + + #[test] + fn reconcile_adds_mcp_command_when_key_absent() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } + + #[test] + fn reconcile_treats_null_mcp_command_as_empty() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": null + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } + #[test] fn sync_creates_packs_directory_symlink() { let (_parent, canonical, worktree) = setup_sync_layout(); diff --git a/desktop/src/features/channels/ui/AddChannelBotDialog.tsx b/desktop/src/features/channels/ui/AddChannelBotDialog.tsx index 139931e63..f58945a25 100644 --- a/desktop/src/features/channels/ui/AddChannelBotDialog.tsx +++ b/desktop/src/features/channels/ui/AddChannelBotDialog.tsx @@ -29,15 +29,16 @@ import { DropdownMenuContent, DropdownMenuRadioGroup, DropdownMenuRadioItem, - DropdownMenuSeparator, DropdownMenuTrigger, } from "@/shared/ui/dropdown-menu"; import { coerceConfigValues, ProviderConfigFields, } from "@/features/agents/ui/ProviderConfigFields"; -import { resolvePersonaRuntime } from "@/features/agents/lib/resolvePersonaRuntime"; -import { useEffectiveRuntimes } from "@/features/channels/ui/useEffectiveRuntimes"; +import { + collectRuntimeWarnings, + resolvePersonaRuntime, +} from "@/features/agents/lib/resolvePersonaRuntime"; import { getActivePersonas } from "@/features/agents/lib/catalog"; import { getUsableTeams } from "@/features/agents/lib/teamPersonas"; import { useLastRuntime } from "@/features/agents/lib/useLastRuntime"; @@ -55,8 +56,6 @@ type AddChannelBotDialogProps = { onOpenChange: (open: boolean) => void; }; -const RUNTIME_NONE_SENTINEL = "__none__"; - function defaultBotName(runtime: AcpRuntime | null) { if (!runtime) { return ""; @@ -104,7 +103,7 @@ export function AddChannelBotDialog({ onAdded, onOpenChange, }: AddChannelBotDialogProps) { - const { setLastRuntime } = useLastRuntime(); + const { lastRuntimeId, setLastRuntime } = useLastRuntime(); const personasQuery = usePersonasQuery(); const teamsQuery = useTeamsQuery(); const inChannelPersonaIds = useInChannelPersonaIds( @@ -153,13 +152,11 @@ export function AddChannelBotDialog({ const selectedRuntime = React.useMemo( () => - selectedRuntimeId - ? (providers.find((runtime) => runtime.id === selectedRuntimeId) ?? - null) - : null, + providers.find((runtime) => runtime.id === selectedRuntimeId) ?? + providers[0] ?? + null, [providers, selectedRuntimeId], ); - const isOverrideActive = selectedRuntime !== null; const selectedPersonas = React.useMemo( () => personas.filter((persona) => selectedPersonaIds.includes(persona.id)), [personas, selectedPersonaIds], @@ -169,18 +166,21 @@ export function AddChannelBotDialog({ const reusableAgent = useReusableAgentDetection( channelId, open && channelId !== null, - selectedRuntime ?? providers[0] ?? null, + selectedRuntime, selectedPersonas, includeGeneric, customPrompt, ); - const { runtimeWarnings, effectiveRuntimes } = useEffectiveRuntimes( - personas, - selectedPersonas, - providers, - selectedRuntime, - isOverrideActive, + // Surface warnings when a persona's preferred runtime differs from the + // user-selected runtime. In this dialog the user explicitly picks a + // runtime via the dropdown, so the fallback is `selectedRuntime` (their + // choice), NOT `providers[0]`. This differs intentionally from + // AddTeamToChannelDialog which has no runtime selector and falls back + // to the first available runtime. + const runtimeWarnings = React.useMemo( + () => collectRuntimeWarnings(selectedPersonas, providers, selectedRuntime), + [selectedPersonas, providers, selectedRuntime], ); const isProviderMode = runOn !== "local"; @@ -197,6 +197,19 @@ export function AddChannelBotDialog({ ); }, [isProviderMode, probedProvider, providerConfig]); + React.useEffect(() => { + if (!open) { + return; + } + + if (!selectedRuntimeId && providers.length > 0) { + const remembered = lastRuntimeId + ? providers.find((p) => p.id === lastRuntimeId) + : null; + setSelectedRuntimeId(remembered ? remembered.id : providers[0].id); + } + }, [open, providers, selectedRuntimeId, lastRuntimeId]); + React.useEffect(() => { if (!selectedRuntime || hasEditedCustomName) { return; @@ -305,7 +318,7 @@ export function AddChannelBotDialog({ } async function handleSubmit() { - if (providers.length === 0 || selectedCount === 0) { + if (!selectedRuntime || selectedCount === 0) { return; } @@ -333,7 +346,7 @@ export function AddChannelBotDialog({ ...(includeGeneric ? [ { - runtime: selectedRuntime ?? providers[0], + runtime: selectedRuntime, name: customName, systemPrompt: customPrompt, role: "bot" as const, @@ -344,15 +357,13 @@ export function AddChannelBotDialog({ ] : []), ...selectedPersonas.map((persona) => { - const effectiveFallback = selectedRuntime ?? providers[0] ?? null; const resolved = resolvePersonaRuntime( persona.runtime, providers, - effectiveFallback, - isOverrideActive, + selectedRuntime, ); return { - runtime: resolved.runtime ?? effectiveFallback ?? providers[0], + runtime: resolved.runtime ?? selectedRuntime, name: persona.displayName, personaId: persona.id, systemPrompt: persona.systemPrompt, @@ -413,7 +424,7 @@ export function AddChannelBotDialog({ respondTo !== "allowlist" || respondToAllowlist.length > 0; const canSubmit = - (selectedRuntime !== null || providers.length > 0) && + selectedRuntime !== null && selectedCount > 0 && (!includeGeneric || customName.trim().length > 0) && respondToValid && @@ -425,11 +436,9 @@ export function AddChannelBotDialog({ const canChooseProvider = providers.length > 0 && !providersLoading && !createBotsMutation.isPending; const canToggleSelections = !createBotsMutation.isPending; - const runtimeTriggerLabel = providersLoading + const providerTriggerLabel = providersLoading ? "Loading runtimes..." - : providers.length === 0 - ? "No runtimes found" - : (selectedRuntime?.label ?? "Use persona defaults"); + : (selectedRuntime?.label ?? "No runtimes found"); const addButtonLabel = createBotsMutation.isPending ? selectedCount > 1 ? `Adding ${selectedCount}...` @@ -529,7 +538,7 @@ export function AddChannelBotDialog({ type="button" variant="ghost" > - {runtimeTriggerLabel} + {providerTriggerLabel} @@ -540,19 +549,11 @@ export function AddChannelBotDialog({ > { - if (id === RUNTIME_NONE_SENTINEL) { - setSelectedRuntimeId(""); - } else { - setSelectedRuntimeId(id); - setLastRuntime(id); - } + setSelectedRuntimeId(id); + setLastRuntime(id); }} - value={selectedRuntime?.id ?? RUNTIME_NONE_SENTINEL} + value={selectedRuntime?.id ?? ""} > - - Use persona defaults - - {providers.map((provider) => ( {provider.label} @@ -562,9 +563,9 @@ export function AddChannelBotDialog({

- {isOverrideActive - ? "All agents will use this runtime, overriding persona preferences." - : "Each persona uses its preferred runtime. Choose a runtime above to override all."} + {selectedPersonas.some((p) => p.runtime) + ? "Personas with a preferred runtime will use their own instead of this selection." + : "Default runtime for all deployed agents."}

@@ -582,7 +583,6 @@ export function AddChannelBotDialog({