From bbf9959960de39c7bde18bd60b6fb79ab721b0fd Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Date: Sun, 26 Jul 2026 14:56:53 -0400 Subject: [PATCH] =?UTF-8?q?fix(desktop):=20surface=20BYOH=20availability?= =?UTF-8?q?=20truthfully=20=E2=80=94=20auth=20copy,=20install=20hints,=20a?= =?UTF-8?q?dapter-missing=20presets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three generic fixes for the #2773 hand-test bugs: - Auth-required discovery errors (spec ErrorCode::AuthRequired text, e.g. signed-out cursor-agent) now render sign-in copy naming the agent, threaded from selectedRuntime.label. Matches the message text, never code -32000 (the catch-all fallback). - The agent-definition dialog's not-installed warning now appends a non-empty install_hint (runtimeAvailabilityWarning helper) — presets always carried the right sentence; the requiresExternalCli render gate hid it. - PresetHarness.underlying_cli (Amp: amp-acp wraps amp) feeds preset_catalog_entry, which consumes only the adapter-missing arm of the builtin predicate: CLI-present/adapter-absent now reads AdapterMissing instead of NotInstalled, while adapter-present/ CLI-absent stays Available and selectable (full classify_runtime reuse would have regressed it to CliMissing with backwards copy). Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/scripts/check-file-sizes.mjs | 16 ++- .../src-tauri/src/managed_agents/discovery.rs | 133 +++++++++++++----- .../src/managed_agents/discovery/tests.rs | 88 +++++++++++- .../agents/ui/AgentDefinitionDialog.tsx | 22 ++- .../ui/personaModelDiscoveryStatus.test.mjs | 43 ++++++ .../agents/ui/personaModelDiscoveryStatus.ts | 14 ++ .../ui/runtimeAvailabilityWarning.test.mjs | 88 ++++++++++++ .../agents/ui/runtimeAvailabilityWarning.ts | 32 +++++ .../agents/ui/usePersonaModelDiscovery.ts | 9 +- 9 files changed, 388 insertions(+), 57 deletions(-) create mode 100644 desktop/src/features/agents/ui/runtimeAvailabilityWarning.test.mjs create mode 100644 desktop/src/features/agents/ui/runtimeAvailabilityWarning.ts diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 61dc61da0..55bf334f2 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -326,7 +326,14 @@ const overrides = new Map([ // stale-publish regression is pinned through the REAL discover_acp_runtimes_from // path (Wren's finding: the seam-only tests stayed green under a stale-publish // mutant). Test-only code, zero release-build footprint. - ["src-tauri/src/managed_agents/discovery.rs", 1780], + // +55: #2773 follow-up — PresetHarness.underlying_cli (Amp's amp-acp wraps + // the amp CLI) + preset_catalog_entry helper: adapter presence alone keeps + // deciding Available (adapter-present/CLI-absent stays selectable, Wren's + // regression catch); underlying_cli is consulted only when the adapter is + // absent, so AdapterMissing replaces the misleading NotInstalled. Includes + // the deliberate-divergence doc comments; net after the inline preset + // entries.push block collapsed into the helper. + ["src-tauri/src/managed_agents/discovery.rs", 1835], // BYOH — save_custom_harness_to_dir (backup-swap atomic write) + save_and_warm / // delete_and_warm (persist-mutex serialization for concurrent-safe registry // refresh, B-6). Also: id/collision/load/registry tests (from the file base) + @@ -366,7 +373,12 @@ const overrides = new Map([ // pre_publish_test_hook; verified to red under a stale-publish mutant. // +18: flake fix — lock_path_mutex + registry_test_lock guards (with lock- // order comments) on the four tests that drive discovery's global caches. - ["src-tauri/src/managed_agents/discovery/tests.rs", 1787], + // +84: #2773 follow-up — preset_catalog_entry coverage (Amp-shaped adapter + // preset: AdapterMissing when CLI present, NotInstalled both-missing, + // Available both-present AND adapter-present/CLI-absent — the selectability + // regression guard), bound to an injectable resolver so the tests stay + // PATH-independent. + ["src-tauri/src/managed_agents/discovery/tests.rs", 1871], // identity-import-keyring: the identity resolution state machine's behavioral // matrix (46 tests over FakeIdentityStore — probe × marker × file cells, // adoption / read-back-corruption / marker-failure arms, recovery-mode diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 2941b69ce..2b7264b42 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -1425,6 +1425,91 @@ struct PresetHarness { args: &'static [&'static str], install_instructions_url: &'static str, install_hint: &'static str, + /// Vendor CLI the ACP command wraps, when the preset is an adapter + /// (e.g. Amp's `amp-acp` wraps the separately-installed `amp` CLI). + /// Consulted only when the adapter is absent, so `AdapterMissing` + /// replaces the misleading `NotInstalled` when the CLI is present but + /// the adapter is not. Deliberately NOT fed through the builtins' + /// full `classify_runtime` predicate: that would flip + /// adapter-present/CLI-absent from today's `Available` to `CliMissing` + /// (unselectable), and presets carry a single flat `install_hint`, so + /// the `CliMissing` copy would tell the user to install the adapter + /// they already have. `None` when the command IS the vendor CLI. + underlying_cli: Option<&'static str>, +} + +/// Build the catalog entry for one preset harness through an injectable +/// resolver — the seam the preset loop consumes and tests bind. +/// +/// Availability consumes only the adapter-missing arm of the builtin +/// predicate: adapter presence alone decides `Available` (exactly today's +/// behavior — an `amp-acp` without `amp` stays selectable), and +/// `underlying_cli` is consulted only when the adapter is absent, to +/// distinguish `AdapterMissing` (vendor CLI present) from `NotInstalled` +/// (neither found). See the `underlying_cli` field doc for why the full +/// `classify_runtime` predicate is deliberately not used here. +fn preset_catalog_entry( + def: &PresetHarness, + resolve: impl Fn(&str) -> Option, +) -> AcpRuntimeCatalogEntry { + let (availability, command, binary_path) = match resolve(def.command) { + Some(path) => ( + AcpAvailabilityStatus::Available, + Some(def.command.to_string()), + Some(path.display().to_string()), + ), + None => { + let underlying_cli_found = def + .underlying_cli + .map(|cli| resolve(cli).is_some()) + .unwrap_or(false); + if underlying_cli_found { + (AcpAvailabilityStatus::AdapterMissing, None, None) + } else { + (AcpAvailabilityStatus::NotInstalled, None, None) + } + } + }; + let underlying_cli_path = def + .underlying_cli + .and_then(resolve) + .map(|p| p.display().to_string()); + + let default_args = normalize_agent_args( + def.command, + def.args.iter().map(|s| s.to_string()).collect(), + ); + + AcpRuntimeCatalogEntry { + id: def.id.to_string(), + label: def.label.to_string(), + // No remote URL — all preset icons are bundled assets. + avatar_url: String::new(), + availability, + command, + binary_path, + default_args, + mcp_command: None, + model_env_var: None, + provider_env_var: None, + thinking_env_var: None, + install_hint: def.install_hint.to_string(), + install_instructions_url: def.install_instructions_url.to_string(), + can_auto_install: false, + // Kept false even for adapter presets: presets carry one flat + // install_hint (the adapter's), so the requiresExternalCli + // "CLI is missing" wording would pair the wrong noun with it. + // The builtin path, with per-availability hints, is the only + // consumer of the true case. + requires_external_cli: false, + underlying_cli_path, + node_required: false, + auth_status: AuthStatus::NotApplicable, + login_hint: None, + source: HarnessSource::Preset, + // Preset entries have static, non-editable env; definition_env is empty. + definition_env: Default::default(), + } } const PRESET_HARNESSES: &[PresetHarness] = &[ @@ -1435,6 +1520,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &["acp"], install_instructions_url: "https://cursor.com/downloads", install_hint: "Install Cursor from cursor.com/downloads.", + underlying_cli: None, }, PresetHarness { id: "omp", @@ -1443,6 +1529,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &["acp"], install_instructions_url: "https://github.com/can1357/oh-my-pi", install_hint: "Install Oh My Pi from github.com/can1357/oh-my-pi.", + underlying_cli: None, }, PresetHarness { id: "grok", @@ -1451,6 +1538,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &["agent", "--always-approve", "stdio"], install_instructions_url: "https://build.x.ai/docs", install_hint: "Install Grok Build from build.x.ai.", + underlying_cli: None, }, PresetHarness { id: "opencode", @@ -1459,6 +1547,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &["acp"], install_instructions_url: "https://opencode.ai/docs", install_hint: "Install OpenCode from opencode.ai/docs.", + underlying_cli: None, }, PresetHarness { id: "kimi", @@ -1467,6 +1556,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &["acp"], install_instructions_url: "https://kimi.ai/download", install_hint: "Install Kimi Code from kimi.ai/download.", + underlying_cli: None, }, PresetHarness { id: "amp", @@ -1475,6 +1565,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &[], install_instructions_url: "https://github.com/tao12345666333/amp-acp", install_hint: "Install the amp-acp npm adapter: npm install -g amp-acp.", + underlying_cli: Some("amp"), }, PresetHarness { id: "hermes", @@ -1483,6 +1574,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ args: &[], install_instructions_url: "https://hermes-agent.nousresearch.com", install_hint: "Install Hermes Agent from hermes-agent.nousresearch.com.", + underlying_cli: None, }, PresetHarness { id: "openclaw", @@ -1498,6 +1590,7 @@ const PRESET_HARNESSES: &[PresetHarness] = &[ Gateway's execution environment. If your tools or agent logic \ needs BUZZ_* credentials at execution time, set them on the \ Gateway's own environment separately.", + underlying_cli: None, }, ]; @@ -1626,45 +1719,7 @@ pub fn discover_acp_runtimes_from( } seen_ids.insert(def.id.to_string()); - let (availability, command, binary_path) = match find_command(def.command) { - Some(path) => ( - AcpAvailabilityStatus::Available, - Some(def.command.to_string()), - Some(path.display().to_string()), - ), - None => (AcpAvailabilityStatus::NotInstalled, None, None), - }; - - let default_args = normalize_agent_args( - def.command, - def.args.iter().map(|s| s.to_string()).collect(), - ); - - entries.push(AcpRuntimeCatalogEntry { - id: def.id.to_string(), - label: def.label.to_string(), - // No remote URL — all preset icons are bundled assets. - avatar_url: String::new(), - availability, - command, - binary_path, - default_args, - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - install_hint: def.install_hint.to_string(), - install_instructions_url: def.install_instructions_url.to_string(), - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Preset, - // Preset entries have static, non-editable env; definition_env is empty. - definition_env: Default::default(), - }); + entries.push(preset_catalog_entry(def, find_command)); } // Phase 3: load and append custom harness definitions. diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 2e44a283f..48e8d5479 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -6,8 +6,8 @@ use super::{ codex_adapter_is_outdated, create_time_agent_command_override, default_agent_command, effective_agent_command, find_nvm_default_bin, find_via_login_shell, is_login_shell_path_uninit, is_safe_nvm_tag, managed_agent_avatar_url, normalize_agent_args, - parse_semver_tag, probe_codex_acp_major_version, record_agent_command, - refresh_login_shell_path, try_record_agent_command, BUZZ_AGENT_AVATAR_URL, + parse_semver_tag, preset_catalog_entry, probe_codex_acp_major_version, record_agent_command, + refresh_login_shell_path, try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; @@ -189,6 +189,90 @@ fn classifies_cli_missing_when_adapter_found_but_cli_absent() { assert_eq!(path.as_deref(), Some("/opt/homebrew/bin/codex-acp")); } +/// Amp-shaped preset: an ACP adapter (`amp-acp`) wrapping a separately +/// installed vendor CLI (`amp`). +const ADAPTER_PRESET: PresetHarness = PresetHarness { + id: "amp-test", + label: "Amp Test", + command: "amp-acp", + args: &[], + install_instructions_url: "https://example.com/install", + install_hint: "Install the amp-acp npm adapter.", + underlying_cli: Some("amp"), +}; + +#[test] +fn preset_entry_adapter_missing_when_underlying_cli_present() { + // Vendor CLI resolves, adapter does not — the state Tyler's Amp + // hand-test hit. Must NOT degrade to the misleading NotInstalled. + let entry = preset_catalog_entry(&ADAPTER_PRESET, |cmd| { + (cmd == "amp").then(|| PathBuf::from("/usr/local/bin/amp")) + }); + assert_eq!(entry.availability, AcpAvailabilityStatus::AdapterMissing); + assert!(entry.command.is_none()); + assert!(entry.binary_path.is_none()); + assert_eq!( + entry.underlying_cli_path.as_deref(), + Some("/usr/local/bin/amp") + ); + assert!(!entry.requires_external_cli); + assert_eq!(entry.install_hint, "Install the amp-acp npm adapter."); +} + +#[test] +fn preset_entry_not_installed_when_both_missing() { + let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None); + assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); + assert!(entry.underlying_cli_path.is_none()); + assert!(!entry.requires_external_cli); +} + +#[test] +fn preset_entry_available_when_adapter_and_cli_present() { + let entry = preset_catalog_entry(&ADAPTER_PRESET, |cmd| match cmd { + "amp-acp" => Some(PathBuf::from("/usr/local/bin/amp-acp")), + "amp" => Some(PathBuf::from("/usr/local/bin/amp")), + _ => None, + }); + assert_eq!(entry.availability, AcpAvailabilityStatus::Available); + assert_eq!(entry.command.as_deref(), Some("amp-acp")); + assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); + assert_eq!( + entry.underlying_cli_path.as_deref(), + Some("/usr/local/bin/amp") + ); +} + +#[test] +fn preset_entry_stays_available_when_adapter_present_but_cli_absent() { + // Wren's regression guard: today an `amp-acp` install without `amp` + // is Available and selectable. Feeding underlying_cli through the + // FULL classify_runtime predicate would flip this to CliMissing + // (unselectable, with backwards install copy) — the adapter-missing + // arm is the only one presets consume. + let entry = preset_catalog_entry(&ADAPTER_PRESET, |cmd| { + (cmd == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")) + }); + assert_eq!(entry.availability, AcpAvailabilityStatus::Available); + assert_eq!(entry.command.as_deref(), Some("amp-acp")); + assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); + assert!(entry.underlying_cli_path.is_none()); +} + +#[test] +fn preset_entry_without_underlying_cli_stays_simple() { + // Most presets: the command IS the vendor CLI. No external-CLI flag, + // absent command means plain NotInstalled. + let preset = PresetHarness { + underlying_cli: None, + ..ADAPTER_PRESET + }; + let entry = preset_catalog_entry(&preset, |_| None); + assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); + assert!(!entry.requires_external_cli); + assert!(entry.underlying_cli_path.is_none()); +} + fn persona_with_runtime(id: &str, runtime: Option<&str>) -> crate::managed_agents::AgentDefinition { crate::managed_agents::AgentDefinition { id: id.to_string(), diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 526392085..29cb48c64 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -18,6 +18,7 @@ import { PersonaDropdownField } from "./PersonaDropdownField"; import type { EnvVarsValue } from "./EnvVarsEditor"; import { PersonaAdvancedFields } from "./PersonaAdvancedFields"; import { PersonaModelField } from "./PersonaModelField"; +import { runtimeAvailabilityWarning } from "./runtimeAvailabilityWarning"; import { PersonaProviderApiKeyField } from "./PersonaProviderApiKeyField"; import { canSubmitPersonaDialog, @@ -620,19 +621,14 @@ export function AgentDefinitionDialog({ ); const previewLabel = displayName.trim() || "Agent name"; const previewAvatarUrl = avatarUrl.trim() || null; - const runtimeWarning = - selectedRuntime && selectedRuntime.availability !== "available" ? ( -

- {selectedRuntime.availability === "adapter_missing" - ? `${selectedRuntime.label} CLI is installed but the ACP adapter is missing.` - : selectedRuntime.availability === "adapter_outdated" - ? `${selectedRuntime.label} ACP adapter is outdated — reinstall to continue.` - : selectedRuntime.requiresExternalCli - ? `${selectedRuntime.label} CLI is missing. ${selectedRuntime.installHint}` - : `${selectedRuntime.label} is not installed.`}{" "} - Visit Settings > Agents to set it up. -

- ) : null; + const runtimeWarningText = selectedRuntime + ? runtimeAvailabilityWarning(selectedRuntime) + : null; + const runtimeWarning = runtimeWarningText ? ( +

+ {runtimeWarningText} Visit Settings > Agents to set it up. +

+ ) : null; const advancedFieldsTransition = shouldReduceMotion ? { duration: 0 } : ADVANCED_FIELDS_MOTION_TRANSITION; diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs index bf21bc92f..23d79aaf7 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs @@ -67,3 +67,46 @@ test("model discovery status stays quiet for missing Databricks defaults", () => assert.equal(status, null); }); + +test("auth-required errors name the agent and ask for sign-in", () => { + // Real shape from run_agent_models_command wrapping buzz-acp stderr when + // cursor-agent is signed out (spec ErrorCode::AuthRequired text). + const status = formatModelDiscoveryErrorStatus( + new Error( + "buzz-acp models failed (exit 1): agent communication failed: Agent reported error (code -32000): Authentication required", + ), + "", + "Cursor", + ); + + assert.equal(status?.tone, "warning"); + assert.match(status?.message ?? "", /Cursor requires sign-in/); + assert.match(status?.message ?? "", /Sign in with the Cursor CLI/); +}); + +test("auth-required copy degrades gracefully without an agent label", () => { + const status = formatModelDiscoveryErrorStatus( + new Error("Agent reported error (code -32000): Authentication required"), + "", + ); + + assert.equal(status?.tone, "warning"); + assert.match(status?.message ?? "", /This agent requires sign-in/); +}); + +test("non-auth -32000 errors do NOT get the sign-in copy", () => { + // -32000 is the catch-all fallback code for unclassified agent errors + // (agent_error_from_json unwrap_or(-32000)); only the spec-reserved + // "Authentication required" text may route to the sign-in message. + const status = formatModelDiscoveryErrorStatus( + new Error( + "buzz-acp models failed (exit 1): Agent reported error (code -32000): model catalog fetch timed out", + ), + "anthropic", + "Cursor", + ); + + assert.equal(status?.tone, "warning"); + assert.doesNotMatch(status?.message ?? "", /sign-in/i); + assert.match(status?.message ?? "", /Using built-in model options/); +}); diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts index 38138429f..c6991fdd0 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts @@ -44,6 +44,7 @@ function isEmptySharedComputeError(message: string): boolean { export function formatModelDiscoveryErrorStatus( error: unknown, provider: string, + agentLabel?: string, ): PersonaModelDiscoveryStatus | null { const message = errorMessage(error); @@ -87,6 +88,19 @@ export function formatModelDiscoveryErrorStatus( }; } + // Spec-reserved auth error text (agent-client-protocol ErrorCode::AuthRequired), + // surfaced verbatim through buzz-acp's stderr — generic across conformant + // harnesses (e.g. cursor-agent when not signed in). Match the message text, + // NOT code -32000: that code is also the catch-all fallback for unclassified + // errors, so matching it would swallow unrelated failures into "sign in". + if (message.toLowerCase().includes("authentication required")) { + const label = agentLabel?.trim(); + return { + message: `${label || "This agent"} requires sign-in before models can load. Sign in with the ${label || "agent's"} CLI in a terminal, then try again.`, + tone: "warning", + }; + } + if (message.includes("ANTHROPIC_API_KEY required")) { return { message: "Enter an Anthropic API key to load Anthropic models.", diff --git a/desktop/src/features/agents/ui/runtimeAvailabilityWarning.test.mjs b/desktop/src/features/agents/ui/runtimeAvailabilityWarning.test.mjs new file mode 100644 index 000000000..611d5ff4c --- /dev/null +++ b/desktop/src/features/agents/ui/runtimeAvailabilityWarning.test.mjs @@ -0,0 +1,88 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { runtimeAvailabilityWarning } from "./runtimeAvailabilityWarning.ts"; + +function entry(overrides) { + return { + id: "amp", + label: "Amp", + avatarUrl: "", + availability: "not_installed", + command: null, + binaryPath: null, + defaultArgs: [], + mcpCommand: null, + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + installHint: "Install the amp-acp npm adapter: npm install -g amp-acp.", + installInstructionsUrl: "https://example.com", + canAutoInstall: false, + requiresExternalCli: false, + underlyingCliPath: null, + nodeRequired: false, + authStatus: "not_applicable", + loginHint: null, + source: "preset", + definitionEnv: {}, + ...overrides, + }; +} + +test("available runtimes produce no warning", () => { + assert.equal( + runtimeAvailabilityWarning(entry({ availability: "available" })), + null, + ); +}); + +test("not-installed warning includes the install hint", () => { + // Tyler's Amp hand-test: the hint was carried by the entry but the old + // render gate (requiresExternalCli, hardcoded false for presets) hid it. + const warning = runtimeAvailabilityWarning(entry({})); + assert.equal( + warning, + "Amp is not installed. Install the amp-acp npm adapter: npm install -g amp-acp.", + ); +}); + +test("empty install hint leaves no trailing space", () => { + // Custom harnesses default installHint to "" — an unguarded template + // would render "Amp is not installed. ". + const warning = runtimeAvailabilityWarning(entry({ installHint: "" })); + assert.equal(warning, "Amp is not installed."); +}); + +test("adapter-missing warning names the adapter and includes the hint", () => { + const warning = runtimeAvailabilityWarning( + entry({ availability: "adapter_missing" }), + ); + assert.match( + warning ?? "", + /CLI is installed but the ACP adapter is missing/, + ); + assert.match(warning ?? "", /npm install -g amp-acp/); +}); + +test("cli-missing external-CLI warning keeps the hint", () => { + const warning = runtimeAvailabilityWarning( + entry({ + availability: "cli_missing", + requiresExternalCli: true, + installHint: "Install Codex from example.com.", + label: "Codex", + }), + ); + assert.equal( + warning, + "Codex CLI is missing. Install Codex from example.com.", + ); +}); + +test("adapter-outdated warning stays hint-free reinstall copy", () => { + const warning = runtimeAvailabilityWarning( + entry({ availability: "adapter_outdated" }), + ); + assert.equal(warning, "Amp ACP adapter is outdated — reinstall to continue."); +}); diff --git a/desktop/src/features/agents/ui/runtimeAvailabilityWarning.ts b/desktop/src/features/agents/ui/runtimeAvailabilityWarning.ts new file mode 100644 index 000000000..6f6a03300 --- /dev/null +++ b/desktop/src/features/agents/ui/runtimeAvailabilityWarning.ts @@ -0,0 +1,32 @@ +import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; + +/** + * Availability warning sentence for the agent-definition dialog. + * Returns null when the runtime is available (no warning to show). + * + * Non-empty install hints are appended so a not-installed or + * adapter-missing runtime tells the user the actual next step (presets + * always carry one; custom harnesses may leave it empty — guard on the + * trimmed value to avoid a dangling "X is not installed. "). + */ +export function runtimeAvailabilityWarning( + runtime: AcpRuntimeCatalogEntry, +): string | null { + if (runtime.availability === "available") { + return null; + } + const hint = runtime.installHint.trim(); + const withHint = (base: string) => (hint ? `${base} ${hint}` : base); + switch (runtime.availability) { + case "adapter_missing": + return withHint( + `${runtime.label} CLI is installed but the ACP adapter is missing.`, + ); + case "adapter_outdated": + return `${runtime.label} ACP adapter is outdated — reinstall to continue.`; + default: + return runtime.requiresExternalCli + ? withHint(`${runtime.label} CLI is missing.`) + : withHint(`${runtime.label} is not installed.`); + } +} diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts index 83ebe5396..e7b434288 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts @@ -203,6 +203,7 @@ export function usePersonaModelDiscovery({ // reference from a React Query refetch (same data, unstable ref) does not // abandon and re-issue an in-flight discovery IPC call. const selectedRuntimeAvailability = selectedRuntime?.availability; + const selectedRuntimeLabel = selectedRuntime?.label; const selectedRuntimeDefaultArgs = selectedRuntime?.defaultArgs; const selectedRuntimeDefinitionEnv = selectedRuntime?.definitionEnv; const canDiscoverModelOptions = @@ -252,6 +253,7 @@ export function usePersonaModelDiscovery({ formatModelDiscoveryErrorStatus( new Error(`Runtime not available: ${selectedRuntimeAvailability}`), trimmedProvider, + selectedRuntimeLabel, ), ); setModelDiscoveryStatusKey(null); @@ -320,7 +322,11 @@ export function usePersonaModelDiscovery({ setModelDiscoveryData(null); setModelDiscoveryDataKey(null); setModelDiscoveryStatus( - formatModelDiscoveryErrorStatus(error, trimmedProvider), + formatModelDiscoveryErrorStatus( + error, + trimmedProvider, + selectedRuntimeLabel, + ), ); setModelDiscoveryStatusKey(activeModelDiscoveryKey); }) @@ -355,6 +361,7 @@ export function usePersonaModelDiscovery({ selectedRuntimeAvailability, selectedRuntimeDefaultArgs, selectedRuntimeDefinitionEnv, + selectedRuntimeLabel, shouldDebounceModelDiscovery, trimmedProvider, ]);