mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix: address code review findings from provider→runtime rename
localStorage key migration: useLastRuntime now reads the legacy "sprout:last-runtime-provider" key as a fallback and clears it on first write, preventing silent loss of saved runtime preference on upgrade. Remaining stale local variable names (selectedProviderId, acpProvidersQuery, etc.) renamed for consistency across 12 TS files. Migration and template serde alias backward-compat tests added.
This commit is contained in:
@@ -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)],
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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"
|
||||
>
|
||||
<span className="truncate">{runtimeTriggerLabel}</span>
|
||||
<span className="truncate">{providerTriggerLabel}</span>
|
||||
<ChevronDown className="h-3.5 w-3.5 text-muted-foreground" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
@@ -540,19 +549,11 @@ export function AddChannelBotDialog({
|
||||
>
|
||||
<DropdownMenuRadioGroup
|
||||
onValueChange={(id) => {
|
||||
if (id === RUNTIME_NONE_SENTINEL) {
|
||||
setSelectedRuntimeId("");
|
||||
} else {
|
||||
setSelectedRuntimeId(id);
|
||||
setLastRuntime(id);
|
||||
}
|
||||
setSelectedRuntimeId(id);
|
||||
setLastRuntime(id);
|
||||
}}
|
||||
value={selectedRuntime?.id ?? RUNTIME_NONE_SENTINEL}
|
||||
value={selectedRuntime?.id ?? ""}
|
||||
>
|
||||
<DropdownMenuRadioItem value={RUNTIME_NONE_SENTINEL}>
|
||||
Use persona defaults
|
||||
</DropdownMenuRadioItem>
|
||||
<DropdownMenuSeparator />
|
||||
{providers.map((provider) => (
|
||||
<DropdownMenuRadioItem key={provider.id} value={provider.id}>
|
||||
{provider.label}
|
||||
@@ -562,9 +563,9 @@ export function AddChannelBotDialog({
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{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."}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
@@ -582,7 +583,6 @@ export function AddChannelBotDialog({
|
||||
|
||||
<AddChannelBotPersonasSection
|
||||
canToggleSelections={canToggleSelections}
|
||||
effectiveRuntimes={effectiveRuntimes}
|
||||
inChannelPersonaIds={inChannelPersonaIds}
|
||||
includeGeneric={includeGeneric}
|
||||
isLoading={personasQuery.isLoading}
|
||||
|
||||
Reference in New Issue
Block a user