From 7bd533fb46607145579fcc35cfc91fd497cd52a1 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 10:08:47 -0400 Subject: [PATCH 01/11] feat(config-nudge): distinguish install/auth state in cli_login nudge cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits the collapsed Requirement::CliLogin into four distinct failure states using the existing AcpAvailabilityStatus classifier from Doctor (classify_runtime in discovery.rs). The nudge card now shows actionable per-state copy and a deep-link CTA to Settings → Doctor instead of routing every harness issue through Edit Agent. Backend (Rust): - readiness.rs: rework cli_login_requirements to call classify_runtime before deciding whether to probe. States: Available → run probe; AdapterMissing/CliMissing/NotInstalled → skip probe, emit CliLogin with the precise state. availability field added to Requirement::CliLogin. - discovery.rs: make find_command and classify_runtime pub(crate) for reuse in readiness.rs. - types.rs: add Deserialize to AcpAvailabilityStatus derive. - runtime.rs: include availability in the cli_login serde JSON payload. - Tests: 4 new unit tests covering each arm + updated codex_not_ready test to use make_cli_runtime stub (function now requires a runtime arg). Frontend (TS): - configNudge.ts: add required availability field to cli_login union arm. Type guard rejects old-format payloads without the field. - config-nudge-attachment.tsx: per-state copy via cliLoginMessage(); (A) pure cli_login card routes whole card to Doctor; (B) mixed card routes card to Edit Agent + each cli_login row gets inline Open Doctor CTA with stopPropagation. Always-visible CTA label replaces opacity-0 fade-in hint. cursor-pointer + hover:shadow-sm affordances. - AppShellContext.tsx/AppShell.tsx: onOpenSettings added to context so config-nudge-attachment can deep-link to Doctor via useAppShell(). - configNudge.test.mjs: update cli_login fixtures to include availability, add test pinning rejection of old-format payloads. - check-file-sizes.mjs: bump readiness.rs override to 1370. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 6 +- .../src-tauri/src/managed_agents/discovery.rs | 4 +- .../src-tauri/src/managed_agents/readiness.rs | 249 +++++++++++++++--- .../src-tauri/src/managed_agents/runtime.rs | 2 + desktop/src-tauri/src/managed_agents/types.rs | 2 +- desktop/src/app/AppShell.tsx | 1 + desktop/src/app/AppShellContext.tsx | 6 + desktop/src/shared/lib/configNudge.test.mjs | 21 ++ desktop/src/shared/lib/configNudge.ts | 19 +- .../src/shared/ui/config-nudge-attachment.tsx | 112 ++++++-- 10 files changed, 360 insertions(+), 62 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index d03e67e2b1..c419ea6d0f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -116,7 +116,11 @@ const overrides = new Map([ // +2 readiness integration tests for flat-DATABRICKS_HOST canonicalization fix. // +1 cargo fmt whitespace reformat (readiness.rs closures inline after rebase). // +2 unit tests for cli_login_requirements resolve_command integration (DMG PATH fix). - ["src-tauri/src/managed_agents/readiness.rs", 1215], + // Doctor-CTA: reworked cli_login_requirements to carry AcpAvailabilityStatus, + // skip login probe for not-installed/adapter-missing/cli-missing states, and + // added 4 unit tests covering each arm. Load-bearing discoverability fix. + // Updated existing codex_not_ready test to use make_cli_runtime stub. + ["src-tauri/src/managed_agents/readiness.rs", 1370], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 09ac897f3c..059ab988f1 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -678,7 +678,7 @@ pub fn login_shell_path() -> Option { .clone() } -fn find_command(command: &str) -> Option { +pub(crate) fn find_command(command: &str) -> Option { resolve_command(command) } @@ -701,7 +701,7 @@ pub fn missing_command_message(command: &str, role: &str) -> String { ) } -fn classify_runtime( +pub(crate) fn classify_runtime( adapter_result: Option<(&str, PathBuf)>, underlying_cli: Option<&str>, underlying_cli_found: bool, diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c5f3ef440c..0b94eae4e7 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -45,9 +45,11 @@ use serde::{Deserialize, Serialize}; use crate::managed_agents::{ agent_env::baked_build_env, config_bridge::read_goose_file_config, - discovery::{known_acp_runtime, resolve_command, KnownAcpRuntime}, + discovery::{ + classify_runtime, find_command, known_acp_runtime, resolve_command, KnownAcpRuntime, + }, env_vars::merged_user_env, - types::{ManagedAgentRecord, PersonaRecord}, + types::{AcpAvailabilityStatus, ManagedAgentRecord, PersonaRecord}, }; // ── EffectiveAgentEnv ───────────────────────────────────────────────────────── @@ -151,6 +153,11 @@ pub enum Requirement { /// Human-readable instruction for completing the login /// (e.g. `"run \`codex login --with-api-key\`"`). setup_copy: String, + /// Granular install/auth state for this runtime — distinguishes + /// "not installed" from "logged out" from "adapter missing". + /// Carried to the FE so the nudge card can show the right message + /// and route to Doctor with accurate context. + availability: AcpAvailabilityStatus, }, } @@ -242,10 +249,12 @@ fn collect_missing_requirements( "claude" => cli_login_requirements( &["claude", "auth", "status"], "complete Claude Code authentication by running the Claude CLI", + rt, ), "codex" => cli_login_requirements( &["codex", "login", "status"], "run `codex login --with-api-key`", + rt, ), _ => vec![], } @@ -422,39 +431,72 @@ fn goose_requirements( /// Requirements for CLI-login runtimes (claude, codex). /// -/// We probe the CLI's login-status command synchronously. These probes are +/// Probes the CLI's login-status command synchronously. These probes are /// fast (<300ms) and the results are memoized by the caller for the session /// lifetime if desired. -fn cli_login_requirements(probe_args: &[&str], setup_copy: &str) -> Vec { - // Resolve the binary through the full PATH-search + login-shell path so - // the probe works in a packaged macOS DMG where the GUI PATH lacks - // npm/homebrew directories (where `claude` / `codex` typically live). - // - // If the binary genuinely does not exist, stay NotReady — the user needs - // to install it. If it exists but is not on the GUI PATH, resolve_command - // finds it via the login-shell fallback. - let Some(binary_path) = resolve_command(probe_args[0]) else { - // Binary not found → not installed → NotReady. - return vec![Requirement::CliLogin { - probe_args: probe_args.iter().map(|s| s.to_string()).collect(), - setup_copy: setup_copy.to_string(), - }]; - }; - - // Run the probe at the resolved absolute path so the GUI-PATH gap is bypassed. - let logged_in = std::process::Command::new(&binary_path) - .args(&probe_args[1..]) - .output() - .map(|o| o.status.success()) +/// +/// Computes a granular `AcpAvailabilityStatus` by running the same +/// classifier as Doctor (`classify_runtime`) before deciding whether to +/// probe — this lets the nudge card distinguish "not installed" from +/// "adapter missing" from "logged out" without a new backend probe. +fn cli_login_requirements( + probe_args: &[&str], + setup_copy: &str, + runtime: &KnownAcpRuntime, +) -> Vec { + // Resolve each adapter command to find the ACP adapter binary. + let adapter_result = runtime + .commands + .iter() + .find_map(|cmd| find_command(cmd).map(|path| (*cmd, path))); + + // Check whether the underlying CLI itself (e.g. "claude", "codex") is on PATH. + let underlying_cli_found = runtime + .underlying_cli + .map(|cli| find_command(cli).is_some()) .unwrap_or(false); - if logged_in { - vec![] - } else { - vec![Requirement::CliLogin { + let (availability, _cmd, _path) = + classify_runtime(adapter_result, runtime.underlying_cli, underlying_cli_found); + + match availability { + AcpAvailabilityStatus::Available => { + // Both adapter and CLI are present — probe login status. + // Resolve via the full login-shell PATH so the probe works in a + // packaged macOS DMG where the GUI PATH lacks npm/homebrew. + let Some(binary_path) = resolve_command(probe_args[0]) else { + // Unexpectedly not resolvable (race or PATH edge case). + return vec![Requirement::CliLogin { + probe_args: probe_args.iter().map(|s| s.to_string()).collect(), + setup_copy: setup_copy.to_string(), + availability: AcpAvailabilityStatus::Available, + }]; + }; + + let logged_in = std::process::Command::new(&binary_path) + .args(&probe_args[1..]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false); + + if logged_in { + vec![] + } else { + vec![Requirement::CliLogin { + probe_args: probe_args.iter().map(|s| s.to_string()).collect(), + setup_copy: setup_copy.to_string(), + availability: AcpAvailabilityStatus::Available, + }] + } + } + // Tooling is not fully installed — emit CliLogin with the precise + // state so the nudge card can show the right message. Skip the probe + // (can't run a missing or misconfigured CLI). + other => vec![Requirement::CliLogin { probe_args: probe_args.iter().map(|s| s.to_string()).collect(), setup_copy: setup_copy.to_string(), - }] + availability: other, + }], } } @@ -804,11 +846,13 @@ mod tests { fn codex_not_ready_copy_does_not_mention_openai_api_key() { // codex uses its own credential store via `codex login --with-api-key`. // The nudge copy must NOT say "set OPENAI_API_KEY". - // We test the static not-logged-in path by constructing the requirement - // directly (the binary may or may not be installed in CI). + // Use a not-installed runtime so the requirement is always emitted + // regardless of whether codex is on the test machine's PATH. + let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], None); let reqs = cli_login_requirements( &["codex", "login", "status"], "run `codex login --with-api-key`", + &rt, ); // Whether codex is installed or not, the copy (if any) must not mention OPENAI_API_KEY. for req in &reqs { @@ -827,14 +871,54 @@ mod tests { // ── cli_login_requirements: resolve_command integration ───────────── + /// Construct a minimal `KnownAcpRuntime` stub for testing cli_login_requirements. + /// `commands` are the adapter binaries; `underlying_cli` is the CLI name. + fn make_cli_runtime( + commands: &'static [&'static str], + underlying_cli: Option<&'static str>, + ) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "test-cli-runtime", + label: "Test CLI", + commands, + aliases: &[], + avatar_url: "", + mcp_command: None, + mcp_hooks: false, + underlying_cli, + cli_install_commands: &[], + adapter_install_commands: &[], + install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + config_file_path: None, + config_file_format: None, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + } + } + #[test] fn cli_login_requirements_missing_binary_is_not_ready() { - // A binary that cannot possibly exist on any system → binary not found - // → resolve_command returns None → function must return CliLogin - // requirement (NotReady), not panic or return Ready. + // Both adapter and underlying CLI are nonexistent → NotInstalled state + // → must return a CliLogin requirement with availability=NotInstalled. + let rt = make_cli_runtime( + &["__buzz_nonexistent_adapter_abc123__"], + Some("__buzz_nonexistent_cli_abc123__"), + ); let reqs = cli_login_requirements( &["__buzz_nonexistent_binary_abc123__", "status"], "install the tool first", + &rt, ); assert!( !reqs.is_empty(), @@ -845,21 +929,78 @@ mod tests { "requirement must be CliLogin; got {:?}", reqs[0] ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::NotInstalled, + "both missing → NotInstalled" + ); + } + } + + #[test] + fn cli_login_requirements_adapter_missing_emits_adapter_missing() { + // Underlying CLI present (`true` is always on PATH), adapter absent. + // → AdapterMissing state → no probe run → CliLogin{AdapterMissing}. + let rt = make_cli_runtime( + &["__buzz_nonexistent_adapter_xyz789__"], + Some("true"), // "true" is always on PATH on POSIX + ); + let reqs = cli_login_requirements(&["true", "status"], "install the adapter", &rt); + assert!( + !reqs.is_empty(), + "adapter missing must produce a CliLogin requirement" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::AdapterMissing, + "adapter absent, CLI present → AdapterMissing" + ); + } + } + + #[test] + fn cli_login_requirements_cli_missing_emits_cli_missing() { + // Adapter present (`true` stands in for the adapter binary), CLI absent. + // → CliMissing state → no probe run → CliLogin{CliMissing}. + let rt = make_cli_runtime( + &["true"], // adapter found via "true" + Some("__buzz_nonexistent_cli_abc123__"), // underlying CLI missing + ); + let reqs = cli_login_requirements(&["true", "status"], "install the CLI", &rt); + assert!( + !reqs.is_empty(), + "CLI missing must produce a CliLogin requirement" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::CliMissing, + "adapter present, CLI absent → CliMissing" + ); + } } #[test] fn cli_login_requirements_resolvable_binary_runs_probe_at_resolved_path() { - // `true` is a POSIX built-in available on every CI runner and always - // exits 0. Use it as a probe: probe_args = ["true", "--probe-arg"] - // → resolved path probed → exit 0 → logged_in = true → requirements - // is empty (Ready). This exercises the resolve_command fast path - // (binary found on PATH) + the probe-at-resolved-path branch. - // - // `true` is universally resolvable on POSIX/macOS/Linux CI, so we - // assert the ready (empty) outcome directly rather than hedging. + // Both adapter and CLI present (`true` stands in for both), probe exits 0 + // → logged_in = true → requirements is empty (Ready). + // `true` is universally resolvable on POSIX/macOS/Linux CI. + let rt = make_cli_runtime(&["true"], Some("true")); let reqs = cli_login_requirements( &["true", "--probe-arg"], "this should not show (true always succeeds)", + &rt, ); assert!( reqs.is_empty(), @@ -870,6 +1011,29 @@ mod tests { ); } + #[test] + fn cli_login_requirements_logged_out_emits_available() { + // Both adapter and CLI present, but probe exits non-zero (logged out). + // Use `false` as the probe — always exits 1 on POSIX. + // → CliLogin{Available} (tooling installed, needs login). + let rt = make_cli_runtime(&["true"], Some("true")); + let reqs = cli_login_requirements(&["false", "status"], "run `tool login`", &rt); + assert!( + !reqs.is_empty(), + "non-zero probe must produce a CliLogin requirement (logged out)" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::Available, + "tooling installed, probe fails → Available (logged-out)" + ); + } + } + // ── custom/unknown command ───────────────────────────────────────────── #[test] @@ -930,6 +1094,7 @@ mod tests { "status".to_string(), ], setup_copy: "run `codex login --with-api-key`".to_string(), + availability: crate::managed_agents::AcpAvailabilityStatus::Available, }; let json = serde_json::to_value(&r).unwrap(); assert_eq!(json["surface"], "cli_login"); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 3fa953dd05..557bdccbdb 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1635,10 +1635,12 @@ pub fn spawn_agent_child( Requirement::CliLogin { probe_args, setup_copy, + availability, } => serde_json::json!({ "surface": "cli_login", "probe_args": probe_args, "setup_copy": setup_copy, + "availability": availability, }), }) .collect(); diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 633e74eea5..16e4cfc79e 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -544,7 +544,7 @@ pub struct ManagedAgentLogResponse { pub log_path: String, } -#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum AcpAvailabilityStatus { Available, diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index e35c41c0d2..02d0574e54 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -619,6 +619,7 @@ export function AppShell() { threadActivityItems, threadActivityFeedItems, feedItemState, + onOpenSettings: handleOpenSettings, }} > diff --git a/desktop/src/app/AppShellContext.tsx b/desktop/src/app/AppShellContext.tsx index 3266210fd9..30d67fd56b 100644 --- a/desktop/src/app/AppShellContext.tsx +++ b/desktop/src/app/AppShellContext.tsx @@ -3,6 +3,7 @@ import type { ContextParentResolver } from "@/features/channels/readState/readSt import type { ThreadActivityItem } from "@/features/channels/useUnreadChannels"; import type { FeedItemState } from "@/features/home/useFeedItemState"; import type { FeedItem } from "@/shared/api/types"; +import type { SettingsSection } from "@/features/settings/ui/SettingsPanels"; const EMPTY_SET = new Set(); @@ -45,6 +46,10 @@ type AppShellContextValue = { threadActivityItems: ThreadActivityItem[]; threadActivityFeedItems: FeedItem[]; feedItemState: FeedItemState; + // Open the Settings panel at the given section. Available on all surfaces + // that render under AppShell (channel, home, projects, pulse, agents). + // Used by config-nudge cards to deep-link to Settings → Doctor. + onOpenSettings: ((section: SettingsSection) => void) | null; }; const AppShellContext = React.createContext({ @@ -75,6 +80,7 @@ const AppShellContext = React.createContext({ undoUnread: () => {}, unreadSet: EMPTY_SET, }, + onOpenSettings: null, }); export function AppShellProvider({ diff --git a/desktop/src/shared/lib/configNudge.test.mjs b/desktop/src/shared/lib/configNudge.test.mjs index adee297f75..8bb8263eaf 100644 --- a/desktop/src/shared/lib/configNudge.test.mjs +++ b/desktop/src/shared/lib/configNudge.test.mjs @@ -63,12 +63,32 @@ test("extractConfigNudge parses cli_login requirement", () => { surface: "cli_login", probe_args: ["codex", "login", "status"], setup_copy: "run `codex login --with-api-key`", + availability: "available", }, ], }; assert.deepEqual(extractConfigNudge(withSentinel("prose", payload)), payload); }); +test("extractConfigNudge returns null for cli_login without availability", () => { + // availability is required — old-format payloads (no availability field) + // must not parse so stale nudge JSON from before the Doctor-CTA update + // does not silently render a broken card. + const payload = { + agent_name: "Codex", + agent_pubkey: CODEX_PUBKEY, + requirements: [ + { + surface: "cli_login", + probe_args: ["codex", "login", "status"], + setup_copy: "run `codex login --with-api-key`", + // no availability field + }, + ], + }; + assert.equal(extractConfigNudge(withSentinel("prose", payload)), null); +}); + test("extractConfigNudge parses multiple requirements of mixed types", () => { const payload = { agent_name: "Atlas", @@ -80,6 +100,7 @@ test("extractConfigNudge parses multiple requirements of mixed types", () => { surface: "cli_login", probe_args: ["codex", "login"], setup_copy: "run `codex login`", + availability: "not_installed", }, ], }; diff --git a/desktop/src/shared/lib/configNudge.ts b/desktop/src/shared/lib/configNudge.ts index 627e0f6e8c..5b71c8162a 100644 --- a/desktop/src/shared/lib/configNudge.ts +++ b/desktop/src/shared/lib/configNudge.ts @@ -25,6 +25,19 @@ export type ConfigNudgeRequirement = surface: "cli_login"; probe_args: string[]; setup_copy: string; + /** + * Granular install/auth state — mirrors `AcpAvailabilityStatus` from Rust. + * Determines which message and CTA the nudge card shows: + * - "available" → tooling installed, needs login + * - "adapter_missing" → CLI installed but ACP adapter missing + * - "cli_missing" → ACP adapter installed but CLI missing + * - "not_installed" → neither adapter nor CLI found + */ + availability: + | "available" + | "adapter_missing" + | "cli_missing" + | "not_installed"; }; /** @@ -114,7 +127,11 @@ function isConfigNudgeRequirement(v: unknown): v is ConfigNudgeRequirement { return ( Array.isArray(r.probe_args) && r.probe_args.every((a) => typeof a === "string") && - typeof r.setup_copy === "string" + typeof r.setup_copy === "string" && + (r.availability === "available" || + r.availability === "adapter_missing" || + r.availability === "cli_missing" || + r.availability === "not_installed") ); default: return false; diff --git a/desktop/src/shared/ui/config-nudge-attachment.tsx b/desktop/src/shared/ui/config-nudge-attachment.tsx index 767a83417a..f8e76c22d0 100644 --- a/desktop/src/shared/ui/config-nudge-attachment.tsx +++ b/desktop/src/shared/ui/config-nudge-attachment.tsx @@ -1,6 +1,7 @@ import { AlertTriangle } from "lucide-react"; import { requestOpenEditAgent } from "@/features/agents/openEditAgentEvent"; +import { useAppShell } from "@/app/AppShellContext"; import type { ConfigNudgePayload } from "@/shared/lib/configNudge"; import { cn } from "@/shared/lib/cn"; import { useProfilePanel } from "@/shared/context/ProfilePanelContext"; @@ -32,6 +33,40 @@ function requirementKey( } } +/** + * Returns true when every requirement in the nudge is a `cli_login` surface. + * In that case the card routes to Doctor (install/login can't be fixed in + * Edit Agent), per design decision (A). + */ +function isAllCliLogin(reqs: ConfigNudgePayload["requirements"]): boolean { + return reqs.length > 0 && reqs.every((r) => r.surface === "cli_login"); +} + +/** + * Per-state human-readable copy for a cli_login requirement. + * Uses the probe_args[0] as a best-effort harness name. + */ +function cliLoginMessage( + req: Extract< + ConfigNudgePayload["requirements"][number], + { surface: "cli_login" } + >, +): string { + const harness = req.probe_args[0] ?? "the CLI tool"; + switch (req.availability) { + case "not_installed": + return `${harness} isn't installed`; + case "cli_missing": + return `${harness} CLI is missing`; + case "adapter_missing": + return `${harness} ACP adapter isn't installed`; + case "available": + // Tooling is present but authentication is needed — fall back to + // the backend-supplied copy which has the exact login command. + return req.setup_copy; + } +} + /** * Inline card rendered when the desktop detects a `buzz:config-nudge` * sentinel in a kind:9 message body. @@ -40,11 +75,12 @@ function requirementKey( * variant so it is visually distinct and consistent with other error states in * the system. * - * The card's trigger opens the Edit Agent dialog for the agent by: - * 1. Calling `openProfilePanel(pubkey)` (from ProfilePanelContext) to ensure - * the profile panel is visible. - * 2. Dispatching `requestOpenEditAgent(pubkey)` so that `UserProfilePanel` - * auto-opens the edit dialog once it mounts with that pubkey. + * Routing: + * (A) When ALL requirements are `cli_login`, the card trigger opens + * Settings → Doctor (install/login can't be fixed in Edit Agent). + * (B) Otherwise, the card trigger opens Edit Agent, and each `cli_login` row + * renders its own "Open Doctor →" inline CTA that routes to Doctor + * regardless of the card-level trigger. */ export function ConfigNudgeCard({ className, @@ -54,15 +90,40 @@ export function ConfigNudgeCard({ nudge: ConfigNudgePayload; }) { const { openProfilePanel } = useProfilePanel(); + const { onOpenSettings } = useAppShell(); + + const allCliLogin = isAllCliLogin(nudge.requirements); const handleOpen = () => { - openProfilePanel?.(nudge.agent_pubkey); - requestOpenEditAgent(nudge.agent_pubkey); + if (allCliLogin) { + // (A) Pure cli_login card — route to Doctor. + onOpenSettings?.("doctor"); + } else { + openProfilePanel?.(nudge.agent_pubkey); + requestOpenEditAgent(nudge.agent_pubkey); + } }; + const handleOpenDoctor = (e: React.MouseEvent) => { + // (B) Inline Doctor CTA — stop propagation so the card trigger doesn't + // double-fire (which would also open Edit Agent on mixed cards). + e.stopPropagation(); + onOpenSettings?.("doctor"); + }; + + // CTA label shown in AttachmentActions. + // Always-visible pill (replaces the opacity-0 fade-in hint) so the card + // reads as clickable at rest (affordance #3). + const ctaLabel = allCliLogin ? "Open Doctor →" : "Edit Agent →"; + return ( @@ -75,17 +136,24 @@ export function ConfigNudgeCard({
{nudge.requirements.map((req, i) => ( - + ))}
- - Edit Agent - + {/* Affordance #3: always-visible CTA text (was opacity-0 fade-in). */} + {ctaLabel} @@ -93,8 +161,10 @@ export function ConfigNudgeCard({ } function RequirementRow({ + onOpenDoctor, requirement, }: { + onOpenDoctor: (e: React.MouseEvent) => void; requirement: ConfigNudgePayload["requirements"][number]; }) { switch (requirement.surface) { @@ -117,8 +187,20 @@ function RequirementRow({ ); case "cli_login": return ( -
- {requirement.setup_copy} +
+ + {cliLoginMessage(requirement)} + + {/* (B) Inline Doctor CTA on every cli_login row — routes to Doctor + regardless of the card-level trigger (handles mixed-requirement + cards). stopPropagation prevents double-fire. */} +
); } From ad5f539fa9cd00b0b625ab6b2bb883067d05d8ef Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 10:24:45 -0400 Subject: [PATCH 02/11] fix(doctor-cta): carry availability through buzz-acp setup-mode bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cli_login sentinel the FE parses was missing the `availability` field because `RequirementPayload::CliLogin` in setup_mode.rs did not have the field — serde silently dropped it during deserialization of the desktop-emitted JSON, so nudge_body() re-serialized a struct that never had it. Add an `AcpAvailabilityStatus` mirror enum (snake_case, Serialize + Deserialize) and add `availability` to `RequirementPayload::CliLogin`. The field now round-trips desktop → buzz-acp → sentinel → FE intact. Add 4 boundary tests that exercise the full desktop→from_raw_env_value →nudge_body→sentinel extraction path for each availability state. Update `instruction()` prose fallback to name the specific install gap (adapter missing / CLI missing / not installed) for non-card clients instead of always emitting the login instruction. FE MINOR: replace the inline availability union in configNudge.ts with the shared `AcpAvailabilityStatus` import from api/types.ts. The runtime validator still enumerates wire literals explicitly (correct); the type annotation now uses the shared type (no more drift point). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/setup_mode.rs | 157 +++++++++++++++++++++++++- desktop/src/shared/lib/configNudge.ts | 8 +- 2 files changed, 159 insertions(+), 6 deletions(-) diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index 9fe51f415f..41a9fe7083 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -43,6 +43,31 @@ use nostr::EventId; use serde::{Deserialize, Serialize}; use uuid::Uuid; +// ── Availability mirror ──────────────────────────────────────────────────────── + +/// Granular install/auth state for a CLI-backed ACP harness. +/// +/// Mirrors the desktop `AcpAvailabilityStatus` enum (and the FE +/// `AcpAvailabilityStatus` type in `api/types.ts`). Carried on +/// `RequirementPayload::CliLogin` so the sentinel JSON the desktop parses +/// contains the exact wire literals the FE expects. +/// +/// buzz-acp is a separate crate and must NOT depend on desktop types — +/// this explicit mirror is the correct pattern (same as the rest of +/// `RequirementPayload` as "the Rust counterpart to desktop's `Requirement`"). +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub(crate) enum AcpAvailabilityStatus { + /// Adapter + CLI both present; may still need login. + Available, + /// ACP adapter binary missing; underlying CLI may be present. + AdapterMissing, + /// CLI binary missing; ACP adapter may be present. + CliMissing, + /// Neither adapter nor CLI found. + NotInstalled, +} + use crate::{ author_allowed, config::Config, @@ -72,6 +97,11 @@ pub(crate) enum RequirementPayload { CliLogin { probe_args: Vec, setup_copy: String, + /// Granular install/auth state — determines copy and CTA routing on + /// the desktop card. `Available` means tooling is present but login + /// is needed; the other three variants mean the tooling itself is + /// missing and the probe was skipped. + availability: AcpAvailabilityStatus, }, } @@ -85,7 +115,40 @@ impl RequirementPayload { RequirementPayload::EnvKey { key } => { format!("set `{}` in Edit Agent → Environment variables", key) } - RequirementPayload::CliLogin { setup_copy, .. } => setup_copy.clone(), + RequirementPayload::CliLogin { + setup_copy, + availability, + probe_args, + } => match availability { + AcpAvailabilityStatus::Available => setup_copy.clone(), + AcpAvailabilityStatus::AdapterMissing => { + let harness = probe_args + .first() + .map(String::as_str) + .unwrap_or("the agent"); + format!( + "install the {} ACP adapter (open Doctor in Settings to diagnose)", + harness + ) + } + AcpAvailabilityStatus::CliMissing => { + let harness = probe_args + .first() + .map(String::as_str) + .unwrap_or("the agent"); + format!( + "install {} CLI (open Doctor in Settings to diagnose)", + harness + ) + } + AcpAvailabilityStatus::NotInstalled => { + let harness = probe_args + .first() + .map(String::as_str) + .unwrap_or("the agent"); + format!("install {} (open Doctor in Settings to diagnose)", harness) + } + }, } } } @@ -589,6 +652,7 @@ mod tests { "status".to_string(), ], setup_copy: "run `codex login --with-api-key`".to_string(), + availability: AcpAvailabilityStatus::Available, }], }; let body = payload.nudge_body(); @@ -655,6 +719,7 @@ mod tests { RequirementPayload::CliLogin { probe_args: vec!["codex".to_string(), "login".to_string()], setup_copy: "run `codex login`".to_string(), + availability: AcpAvailabilityStatus::Available, }, ], }; @@ -766,4 +831,94 @@ mod tests { "replay of the same event-id must be rejected (dedup)" ); } + + // ── availability round-trip tests ───────────────────────────────────────── + // + // These tests prove the desktop→buzz-acp→sentinel path preserves the + // `availability` field. They simulate what actually happens at runtime: + // desktop serializes a `cli_login` JSON blob → buzz-acp parses it via + // `from_raw_env_value` → `nudge_body()` re-serializes into the sentinel → + // the sentinel JSON is extracted and checked for the `availability` field. + // + // This is the exact path Thufir's CRITICAL finding identified as broken + // (the old `RequirementPayload::CliLogin` had no `availability` field, so + // serde silently dropped it during deserialization). + + fn extract_sentinel_json(body: &str) -> String { + let fence_open = "```buzz:config-nudge\n"; + let fence_close = "\n```"; + let start = body + .rfind(fence_open) + .expect("sentinel open fence not found") + + fence_open.len(); + let end = body[start..] + .rfind(fence_close) + .expect("sentinel close fence not found") + + start; + body[start..end].to_string() + } + + fn make_desktop_cli_login_json(availability: &str) -> String { + // Simulate the JSON desktop's runtime.rs emits — a full SetupPayload + // with one cli_login requirement carrying the given availability state. + format!( + r#"{{"agent_name":"TestAgent","agent_pubkey":"aa","requirements":[{{"surface":"cli_login","probe_args":["claude"],"setup_copy":"run claude login","availability":"{availability}"}}]}}"# + ) + } + + #[test] + fn cli_login_availability_available_survives_sentinel_round_trip() { + let raw = make_desktop_cli_login_json("available"); + let payload = SetupPayload::from_raw_env_value(Some(raw)) + .unwrap() + .expect("must parse"); + let body = payload.nudge_body(); + let sentinel_json = extract_sentinel_json(&body); + assert!( + sentinel_json.contains(r#""availability":"available""#), + "sentinel must carry availability=available; got: {sentinel_json:?}" + ); + } + + #[test] + fn cli_login_availability_adapter_missing_survives_sentinel_round_trip() { + let raw = make_desktop_cli_login_json("adapter_missing"); + let payload = SetupPayload::from_raw_env_value(Some(raw)) + .unwrap() + .expect("must parse"); + let body = payload.nudge_body(); + let sentinel_json = extract_sentinel_json(&body); + assert!( + sentinel_json.contains(r#""availability":"adapter_missing""#), + "sentinel must carry availability=adapter_missing; got: {sentinel_json:?}" + ); + } + + #[test] + fn cli_login_availability_cli_missing_survives_sentinel_round_trip() { + let raw = make_desktop_cli_login_json("cli_missing"); + let payload = SetupPayload::from_raw_env_value(Some(raw)) + .unwrap() + .expect("must parse"); + let body = payload.nudge_body(); + let sentinel_json = extract_sentinel_json(&body); + assert!( + sentinel_json.contains(r#""availability":"cli_missing""#), + "sentinel must carry availability=cli_missing; got: {sentinel_json:?}" + ); + } + + #[test] + fn cli_login_availability_not_installed_survives_sentinel_round_trip() { + let raw = make_desktop_cli_login_json("not_installed"); + let payload = SetupPayload::from_raw_env_value(Some(raw)) + .unwrap() + .expect("must parse"); + let body = payload.nudge_body(); + let sentinel_json = extract_sentinel_json(&body); + assert!( + sentinel_json.contains(r#""availability":"not_installed""#), + "sentinel must carry availability=not_installed; got: {sentinel_json:?}" + ); + } } diff --git a/desktop/src/shared/lib/configNudge.ts b/desktop/src/shared/lib/configNudge.ts index 5b71c8162a..a806489481 100644 --- a/desktop/src/shared/lib/configNudge.ts +++ b/desktop/src/shared/lib/configNudge.ts @@ -17,6 +17,8 @@ // ── Types ───────────────────────────────────────────────────────────────────── +import type { AcpAvailabilityStatus } from "@/shared/api/types"; + /** A single missing requirement — mirrors the Rust `RequirementPayload` enum. */ export type ConfigNudgeRequirement = | { surface: "normalized_field"; field: string } @@ -33,11 +35,7 @@ export type ConfigNudgeRequirement = * - "cli_missing" → ACP adapter installed but CLI missing * - "not_installed" → neither adapter nor CLI found */ - availability: - | "available" - | "adapter_missing" - | "cli_missing" - | "not_installed"; + availability: AcpAvailabilityStatus; }; /** From 07bb11df2de6f1fd3cff4b510ec67eda2bed8e37 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 10:33:39 -0400 Subject: [PATCH 03/11] chore(doctor-cta): reword test comment to state invariant Replace review-thread reference with a description of the invariant being guarded. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/setup_mode.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index 41a9fe7083..ecdc36d90f 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -840,9 +840,9 @@ mod tests { // `from_raw_env_value` → `nudge_body()` re-serializes into the sentinel → // the sentinel JSON is extracted and checked for the `availability` field. // - // This is the exact path Thufir's CRITICAL finding identified as broken - // (the old `RequirementPayload::CliLogin` had no `availability` field, so - // serde silently dropped it during deserialization). + // This guards the prior regression where `RequirementPayload::CliLogin` + // had no `availability` field, so serde silently dropped it during + // deserialization and the desktop card never rendered. fn extract_sentinel_json(body: &str) -> String { let fence_open = "```buzz:config-nudge\n"; From c98af39c4a7dc274447981d6cb093b0a4d8bf8ce Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 10:58:57 -0400 Subject: [PATCH 04/11] test(doctor-cta): add screenshot spec for config-nudge card states Captures the four availability states the Doctor-CTA nudge card can render in: available (logged-out), not_installed, adapter_missing, and cli_missing. Also covers the mixed-requirements case showing the inline per-row Doctor CTA alongside the card-level Edit Agent trigger. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/playwright.config.ts | 1 + .../tests/e2e/doctor-cta-screenshots.spec.ts | 295 ++++++++++++++++++ 2 files changed, 296 insertions(+) create mode 100644 desktop/tests/e2e/doctor-cta-screenshots.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 8ca25b81d2..4beec218f0 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -40,6 +40,7 @@ export default defineConfig({ "**/local-archive-screenshots.spec.ts", "**/agent-readiness-screenshots.spec.ts", "**/edit-agent.spec.ts", + "**/doctor-cta-screenshots.spec.ts", "**/pubkey-display-screenshots.spec.ts", "**/file-attachment.spec.ts", "**/image-attachment-gallery.spec.ts", diff --git a/desktop/tests/e2e/doctor-cta-screenshots.spec.ts b/desktop/tests/e2e/doctor-cta-screenshots.spec.ts new file mode 100644 index 0000000000..28cd92cd07 --- /dev/null +++ b/desktop/tests/e2e/doctor-cta-screenshots.spec.ts @@ -0,0 +1,295 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +const SHOTS = "test-results/doctor-cta"; + +// Channel ID for the seeded #general mock channel. +const GENERAL_CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"; + +// Tyler is a managed agent seed in the mock bridge — used as the nudge sender. +const AGENT_PUBKEY = TEST_IDENTITIES.tyler.pubkey; +const AGENT_NAME = "Tyler Agent"; + +/** Settle CSS / Web Animations before capture. */ +async function settleAnimations(page: import("@playwright/test").Page) { + await page.evaluate(() => + Promise.all(document.getAnimations().map((a) => a.finished)), + ); +} + +/** + * Wait for the Tauri invoke mock to be available, then call a mock command. + * Mirrors the helper used in config-bridge-screenshots.spec.ts. + */ +async function invokeMockCommand( + page: import("@playwright/test").Page, + command: string, + payload?: Record, +): Promise { + await page.waitForFunction( + () => { + const w = window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: unknown; + __TAURI_INTERNALS__?: { invoke?: unknown }; + }; + return ( + typeof w.__BUZZ_E2E_INVOKE_MOCK_COMMAND__ === "function" || + typeof w.__TAURI_INTERNALS__?.invoke === "function" + ); + }, + null, + { timeout: 5_000 }, + ); + return page.evaluate( + async ({ command: cmd, payload: pl }) => { + const w = window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: ( + command: string, + payload?: Record, + ) => Promise; + __TAURI_INTERNALS__?: { + invoke?: ( + command: string, + payload?: Record, + ) => Promise; + }; + }; + const invoke = + w.__BUZZ_E2E_INVOKE_MOCK_COMMAND__ ?? w.__TAURI_INTERNALS__?.invoke; + if (!invoke) throw new Error("Mock invoke bridge is unavailable."); + return invoke(cmd, pl); + }, + { command, payload }, + ); +} + +/** + * Build a `buzz:config-nudge` sentinel body from a requirements array. + * Mirrors the format nudge_body() in setup_mode.rs produces. + */ +function makeNudgeSentinel( + agentName: string, + agentPubkey: string, + requirements: unknown[], +): string { + const payload = JSON.stringify({ + agent_name: agentName, + agent_pubkey: agentPubkey, + requirements, + }); + return `**${agentName}** needs configuration before it can respond.\n\n\`\`\`buzz:config-nudge\n${payload}\n\`\`\``; +} + +/** + * Inject a nudge message from the agent into #general, then navigate to the + * channel so the card renders in the message list. + */ +async function injectNudgeAndNavigate( + page: import("@playwright/test").Page, + content: string, +): Promise { + await invokeMockCommand(page, "send_managed_agent_channel_message", { + agentPubkey: AGENT_PUBKEY, + channelId: GENERAL_CHANNEL_ID, + content, + }); + + // Navigate to #general — the injected message is live-pushed so the card + // renders in the current message list. + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); +} + +test.describe("doctor CTA nudge card screenshots", () => { + test.use({ viewport: { width: 1280, height: 900 } }); + + test.beforeEach(async ({ page }) => { + page.on("pageerror", (err) => { + console.error( + "PAGE ERROR:", + err.message, + err.stack?.split("\n").slice(0, 5).join("\n"), + ); + }); + page.on("console", (msg) => { + if (msg.type() === "error") { + console.error("CONSOLE ERROR:", msg.text().slice(0, 500)); + } + }); + }); + + /** + * 01 — pure cli_login card (all requirements are cli_login, availability=available): + * The whole card routes to Doctor; CTA label reads "Open Doctor →". + * This is the "installed but logged-out" state — the most actionable case. + */ + test("01-cli-login-available-all-doctor-routing", async ({ page }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "running" as const, + channelNames: ["general"], + }, + ], + }); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + + const content = makeNudgeSentinel(AGENT_NAME, AGENT_PUBKEY, [ + { + surface: "cli_login", + probe_args: ["claude"], + setup_copy: "run `claude auth login` to authenticate", + availability: "available", + }, + ]); + + await injectNudgeAndNavigate(page, content); + + // Wait for the nudge card to render. + const card = page.locator("[data-config-nudge]").last(); + await expect(card).toBeVisible({ timeout: 10_000 }); + await expect(card.getByText("Open Doctor →").first()).toBeVisible(); + + await card.scrollIntoViewIfNeeded(); + await settleAnimations(page); + + await card.screenshot({ + path: `${SHOTS}/01-cli-login-available-all-doctor-routing.png`, + }); + }); + + /** + * 02 — not_installed state: neither adapter nor CLI found. + * Card shows "claude isn't installed" copy + inline "Open Doctor →" CTA. + */ + test("02-cli-login-not-installed-state", async ({ page }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped" as const, + channelNames: ["general"], + }, + ], + }); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + + const content = makeNudgeSentinel(AGENT_NAME, AGENT_PUBKEY, [ + { + surface: "cli_login", + probe_args: ["claude"], + setup_copy: "install Claude Code", + availability: "not_installed", + }, + ]); + + await injectNudgeAndNavigate(page, content); + + const card = page.locator("[data-config-nudge]").last(); + await expect(card).toBeVisible({ timeout: 10_000 }); + await expect(card.getByText(/isn't installed/)).toBeVisible(); + + await card.scrollIntoViewIfNeeded(); + await settleAnimations(page); + + await card.screenshot({ + path: `${SHOTS}/02-cli-login-not-installed-state.png`, + }); + }); + + /** + * 03 — mixed card: one cli_login (adapter_missing) + one env_key requirement. + * Card trigger routes to Edit Agent; each cli_login row shows inline + * "Open Doctor →" with stopPropagation. CTA label reads "Edit Agent →". + */ + test("03-mixed-requirements-inline-doctor-cta", async ({ page }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped" as const, + channelNames: ["general"], + }, + ], + }); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + + const content = makeNudgeSentinel(AGENT_NAME, AGENT_PUBKEY, [ + { + surface: "cli_login", + probe_args: ["claude"], + setup_copy: "install the Claude Code ACP adapter", + availability: "adapter_missing", + }, + { + surface: "env_key", + key: "ANTHROPIC_API_KEY", + }, + ]); + + await injectNudgeAndNavigate(page, content); + + const card = page.locator("[data-config-nudge]").last(); + await expect(card).toBeVisible({ timeout: 10_000 }); + // Mixed card shows inline Doctor CTA on the cli_login row. + await expect(card.getByText("Open Doctor →")).toBeVisible(); + // And the card-level CTA says "Edit Agent →". + await expect(card.getByText("Edit Agent →", { exact: true })).toBeVisible(); + + await card.scrollIntoViewIfNeeded(); + await settleAnimations(page); + + await card.screenshot({ + path: `${SHOTS}/03-mixed-requirements-inline-doctor-cta.png`, + }); + }); + + /** + * 04 — cli_missing state: ACP adapter present but underlying CLI absent. + * Shows "claude CLI is missing" copy. + */ + test("04-cli-login-cli-missing-state", async ({ page }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped" as const, + channelNames: ["general"], + }, + ], + }); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + + const content = makeNudgeSentinel(AGENT_NAME, AGENT_PUBKEY, [ + { + surface: "cli_login", + probe_args: ["claude"], + setup_copy: "install the Claude CLI", + availability: "cli_missing", + }, + ]); + + await injectNudgeAndNavigate(page, content); + + const card = page.locator("[data-config-nudge]").last(); + await expect(card).toBeVisible({ timeout: 10_000 }); + await expect(card.getByText(/CLI is missing/)).toBeVisible(); + + await card.scrollIntoViewIfNeeded(); + await settleAnimations(page); + + await card.screenshot({ + path: `${SHOTS}/04-cli-login-cli-missing-state.png`, + }); + }); +}); From 05b3d728b13003d6180285389df97badae3ac952 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 11:49:29 -0400 Subject: [PATCH 05/11] fix(config-nudge): suppress redundant per-row Doctor CTA on pure cli_login cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On pure allCliLogin cards the card-level trigger already routes to Doctor ("Open Doctor →" in AttachmentActions). The per-row "Open Doctor →" button in the cli_login RequirementRow was additionally rendering for each row, producing two identical Doctor CTAs visible at once. Thread showRowDoctorCta={!allCliLogin} into RequirementRow and guard the inline button behind it. Mixed cards (card CTA = "Edit Agent →") are unchanged — each cli_login row still renders its own inline Doctor CTA. Pure cli_login cards now show exactly one "Open Doctor →" (the card action). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src/shared/ui/config-nudge-attachment.tsx | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/desktop/src/shared/ui/config-nudge-attachment.tsx b/desktop/src/shared/ui/config-nudge-attachment.tsx index f8e76c22d0..a4d8ee246d 100644 --- a/desktop/src/shared/ui/config-nudge-attachment.tsx +++ b/desktop/src/shared/ui/config-nudge-attachment.tsx @@ -140,6 +140,7 @@ export function ConfigNudgeCard({ key={requirementKey(req, i)} onOpenDoctor={handleOpenDoctor} requirement={req} + showRowDoctorCta={!allCliLogin} /> ))}
@@ -163,9 +164,11 @@ export function ConfigNudgeCard({ function RequirementRow({ onOpenDoctor, requirement, + showRowDoctorCta, }: { onOpenDoctor: (e: React.MouseEvent) => void; requirement: ConfigNudgePayload["requirements"][number]; + showRowDoctorCta: boolean; }) { switch (requirement.surface) { case "env_key": @@ -191,16 +194,20 @@ function RequirementRow({ {cliLoginMessage(requirement)} - {/* (B) Inline Doctor CTA on every cli_login row — routes to Doctor - regardless of the card-level trigger (handles mixed-requirement - cards). stopPropagation prevents double-fire. */} - + {/* (B) Inline Doctor CTA — shown only on mixed cards where the + card-level trigger opens Edit Agent. When allCliLogin is true the + card trigger already routes to Doctor; the per-row button is + redundant and is suppressed. stopPropagation prevents double-fire + on mixed cards where both card and row CTAs are visible. */} + {showRowDoctorCta && ( + + )} ); } From 09d245635a04e80d7ad1cea584bafe7ad366d7cd Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 12:31:13 -0400 Subject: [PATCH 06/11] =?UTF-8?q?fix(desktop):=20bump=20readiness.rs=20fil?= =?UTF-8?q?e-size=20override=201370=20=E2=86=92=201374?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1640 persona-env-vars-refresh rebase added live-persona env merge path changes that grew readiness.rs by 4 lines beyond the existing override. Availability-classification growth, not generic debt. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index c419ea6d0f..96b7c1fe6e 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -120,7 +120,9 @@ const overrides = new Map([ // skip login probe for not-installed/adapter-missing/cli-missing states, and // added 4 unit tests covering each arm. Load-bearing discoverability fix. // Updated existing codex_not_ready test to use make_cli_runtime stub. - ["src-tauri/src/managed_agents/readiness.rs", 1370], + // +4 lines: #1640 persona-env-vars-refresh rebase added availability-classification + // growth in the live-persona env merge path. Feature plumbing, not generic debt. + ["src-tauri/src/managed_agents/readiness.rs", 1374], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing From 3e87d61fcdd4a42dc4e094ca074cdd877346e54c Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 8 Jul 2026 14:18:00 -0400 Subject: [PATCH 07/11] fix(config-nudge): address review feedback on doctor-cta branch Q1: change codex setup_copy from 'codex login --with-api-key' to plain 'codex login'. The --with-api-key flag is wrong for the majority of users (ChatGPT OAuth) and over-prescribes the login method. Updated all 8 sites: the production line in readiness.rs, the doc-comment, test comment, three Rust fixtures, and two TS fixtures in configNudge.test.mjs. Q2: fix Windows CI portability for cli_login_requirements tests. The four new tests used POSIX 'true'/'false' as stand-in binaries which are not guaranteed on the Windows/Git-for-Windows PATH. Replace with std::env::current_exe() via a new present_binary_str() helper that returns the absolute path of the running test binary (always present, resolves via path.exists() regardless of platform). Success probe uses --list (exits 0 on all platforms); failure probe uses an unrecognized flag (libtest exits non-zero for unknown flags on all platforms). Bump readiness.rs file-size override 1374 to 1403. Q3: fix config-nudge card visual issues in mixed-card layout. (a) Row CTA was styled text-destructive (red), making the navigation button look like a danger action. Changed to text-muted-foreground to match the card-level CTA color. (b) AttachmentActions now has items-end self-end so the 'Edit Agent' pill bottom-anchors in its panel rather than vertically centering alongside requirement rows, which was causing false visual pairing with the per-row 'Open Doctor' button. Nit: extract openDoctor() helper in ConfigNudgeCard and add a console.warn guard so onOpenSettings null is not a silent dead click on surfaces without AppShell context. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/setup_mode.rs | 2 +- desktop/scripts/check-file-sizes.mjs | 5 +- .../src-tauri/src/managed_agents/readiness.rs | 78 +++++++++++-------- desktop/src/shared/lib/configNudge.test.mjs | 4 +- .../src/shared/ui/config-nudge-attachment.tsx | 17 +++- 5 files changed, 66 insertions(+), 40 deletions(-) diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index ecdc36d90f..2f162d84f3 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -651,7 +651,7 @@ mod tests { "login".to_string(), "status".to_string(), ], - setup_copy: "run `codex login --with-api-key`".to_string(), + setup_copy: "run `codex login`".to_string(), availability: AcpAvailabilityStatus::Available, }], }; diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 96b7c1fe6e..7b3af6b0cf 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -122,7 +122,10 @@ const overrides = new Map([ // Updated existing codex_not_ready test to use make_cli_runtime stub. // +4 lines: #1640 persona-env-vars-refresh rebase added availability-classification // growth in the live-persona env merge path. Feature plumbing, not generic debt. - ["src-tauri/src/managed_agents/readiness.rs", 1374], + // Windows-CI portability: replaced POSIX true/false probes with current_exe() + // stand-in + present_binary_str()/static_commands() helpers (+29 lines). + // Tests now pass on windows-latest CI shard without POSIX shell utilities. + ["src-tauri/src/managed_agents/readiness.rs", 1403], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 0b94eae4e7..b69a97b57d 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -151,7 +151,7 @@ pub enum Requirement { /// Arguments for the login-status probe (e.g. `["claude", "auth", "status"]`). probe_args: Vec, /// Human-readable instruction for completing the login - /// (e.g. `"run \`codex login --with-api-key\`"`). + /// (e.g. `"run \`codex login\`"`). setup_copy: String, /// Granular install/auth state for this runtime — distinguishes /// "not installed" from "logged out" from "adapter missing". @@ -251,11 +251,7 @@ fn collect_missing_requirements( "complete Claude Code authentication by running the Claude CLI", rt, ), - "codex" => cli_login_requirements( - &["codex", "login", "status"], - "run `codex login --with-api-key`", - rt, - ), + "codex" => cli_login_requirements(&["codex", "login", "status"], "run `codex login`", rt), _ => vec![], } } @@ -844,16 +840,12 @@ mod tests { #[test] fn codex_not_ready_copy_does_not_mention_openai_api_key() { - // codex uses its own credential store via `codex login --with-api-key`. + // codex uses its own credential store via `codex login` (OAuth or API key). // The nudge copy must NOT say "set OPENAI_API_KEY". // Use a not-installed runtime so the requirement is always emitted // regardless of whether codex is on the test machine's PATH. let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], None); - let reqs = cli_login_requirements( - &["codex", "login", "status"], - "run `codex login --with-api-key`", - &rt, - ); + let reqs = cli_login_requirements(&["codex", "login", "status"], "run `codex login`", &rt); // Whether codex is installed or not, the copy (if any) must not mention OPENAI_API_KEY. for req in &reqs { if let Requirement::CliLogin { setup_copy, .. } = req { @@ -907,6 +899,23 @@ mod tests { } } + /// Returns the absolute path of the currently-running test binary as a + /// `&'static str`. Host-portable stand-in for a "present" binary: + /// the path is absolute so `find_command` resolves it via `path.exists()` + /// rather than searching `PATH`, and the file always exists on the host. + /// + /// The tiny allocation is intentionally leaked — this runs at most once per + /// test process and the process exits immediately after tests complete. + fn present_binary_str() -> &'static str { + let path = std::env::current_exe().expect("current_exe must be available in tests"); + Box::leak(path.to_string_lossy().into_owned().into_boxed_str()) + } + + /// Leak a runtime slice of `'static` strs for use in `make_cli_runtime`. + fn static_commands(commands: Vec<&'static str>) -> &'static [&'static str] { + Box::leak(commands.into_boxed_slice()) + } + #[test] fn cli_login_requirements_missing_binary_is_not_ready() { // Both adapter and underlying CLI are nonexistent → NotInstalled state @@ -943,13 +952,13 @@ mod tests { #[test] fn cli_login_requirements_adapter_missing_emits_adapter_missing() { - // Underlying CLI present (`true` is always on PATH), adapter absent. + // Underlying CLI present (use the running test binary as a portable + // stand-in — it's always present and resolves via absolute path), + // adapter absent. // → AdapterMissing state → no probe run → CliLogin{AdapterMissing}. - let rt = make_cli_runtime( - &["__buzz_nonexistent_adapter_xyz789__"], - Some("true"), // "true" is always on PATH on POSIX - ); - let reqs = cli_login_requirements(&["true", "status"], "install the adapter", &rt); + let exe = present_binary_str(); + let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], Some(exe)); + let reqs = cli_login_requirements(&[exe, "--list"], "install the adapter", &rt); assert!( !reqs.is_empty(), "adapter missing must produce a CliLogin requirement" @@ -968,13 +977,15 @@ mod tests { #[test] fn cli_login_requirements_cli_missing_emits_cli_missing() { - // Adapter present (`true` stands in for the adapter binary), CLI absent. + // Adapter present (use the running test binary as a portable stand-in), + // underlying CLI absent. // → CliMissing state → no probe run → CliLogin{CliMissing}. + let exe = present_binary_str(); let rt = make_cli_runtime( - &["true"], // adapter found via "true" + static_commands(vec![exe]), // adapter found via absolute path Some("__buzz_nonexistent_cli_abc123__"), // underlying CLI missing ); - let reqs = cli_login_requirements(&["true", "status"], "install the CLI", &rt); + let reqs = cli_login_requirements(&[exe, "--list"], "install the CLI", &rt); assert!( !reqs.is_empty(), "CLI missing must produce a CliLogin requirement" @@ -993,20 +1004,21 @@ mod tests { #[test] fn cli_login_requirements_resolvable_binary_runs_probe_at_resolved_path() { - // Both adapter and CLI present (`true` stands in for both), probe exits 0 + // Both adapter and CLI present (use the running test binary as a + // portable stand-in — always present, resolves via absolute path), + // probe exits 0 (run with `--list` which lists tests and exits 0). // → logged_in = true → requirements is empty (Ready). - // `true` is universally resolvable on POSIX/macOS/Linux CI. - let rt = make_cli_runtime(&["true"], Some("true")); + let exe = present_binary_str(); + let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); let reqs = cli_login_requirements( - &["true", "--probe-arg"], - "this should not show (true always succeeds)", + &[exe, "--list"], + "this should not show (probe exits 0)", &rt, ); assert!( reqs.is_empty(), "expected Ready (no requirements) when probe binary resolves and exits 0; \ - got {:?} — `true` is always on PATH on POSIX/macOS/Linux CI so this \ - must be empty", + got {:?}", reqs ); } @@ -1014,10 +1026,12 @@ mod tests { #[test] fn cli_login_requirements_logged_out_emits_available() { // Both adapter and CLI present, but probe exits non-zero (logged out). - // Use `false` as the probe — always exits 1 on POSIX. + // Use the test binary with an unrecognized argument as the probe — + // libtest exits non-zero for unknown flags on all platforms. // → CliLogin{Available} (tooling installed, needs login). - let rt = make_cli_runtime(&["true"], Some("true")); - let reqs = cli_login_requirements(&["false", "status"], "run `tool login`", &rt); + let exe = present_binary_str(); + let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); + let reqs = cli_login_requirements(&[exe, "--buzz-probe-fail-xyz"], "run `tool login`", &rt); assert!( !reqs.is_empty(), "non-zero probe must produce a CliLogin requirement (logged out)" @@ -1093,7 +1107,7 @@ mod tests { "login".to_string(), "status".to_string(), ], - setup_copy: "run `codex login --with-api-key`".to_string(), + setup_copy: "run `codex login`".to_string(), availability: crate::managed_agents::AcpAvailabilityStatus::Available, }; let json = serde_json::to_value(&r).unwrap(); diff --git a/desktop/src/shared/lib/configNudge.test.mjs b/desktop/src/shared/lib/configNudge.test.mjs index 8bb8263eaf..a2967fd956 100644 --- a/desktop/src/shared/lib/configNudge.test.mjs +++ b/desktop/src/shared/lib/configNudge.test.mjs @@ -62,7 +62,7 @@ test("extractConfigNudge parses cli_login requirement", () => { { surface: "cli_login", probe_args: ["codex", "login", "status"], - setup_copy: "run `codex login --with-api-key`", + setup_copy: "run `codex login`", availability: "available", }, ], @@ -81,7 +81,7 @@ test("extractConfigNudge returns null for cli_login without availability", () => { surface: "cli_login", probe_args: ["codex", "login", "status"], - setup_copy: "run `codex login --with-api-key`", + setup_copy: "run `codex login`", // no availability field }, ], diff --git a/desktop/src/shared/ui/config-nudge-attachment.tsx b/desktop/src/shared/ui/config-nudge-attachment.tsx index a4d8ee246d..2199f833e1 100644 --- a/desktop/src/shared/ui/config-nudge-attachment.tsx +++ b/desktop/src/shared/ui/config-nudge-attachment.tsx @@ -94,10 +94,19 @@ export function ConfigNudgeCard({ const allCliLogin = isAllCliLogin(nudge.requirements); + const openDoctor = () => { + if (!onOpenSettings) { + console.warn( + "[ConfigNudgeCard] onOpenSettings is null — Doctor deep-link unavailable on this surface", + ); + } + onOpenSettings?.("doctor"); + }; + const handleOpen = () => { if (allCliLogin) { // (A) Pure cli_login card — route to Doctor. - onOpenSettings?.("doctor"); + openDoctor(); } else { openProfilePanel?.(nudge.agent_pubkey); requestOpenEditAgent(nudge.agent_pubkey); @@ -108,7 +117,7 @@ export function ConfigNudgeCard({ // (B) Inline Doctor CTA — stop propagation so the card trigger doesn't // double-fire (which would also open Edit Agent on mixed cards). e.stopPropagation(); - onOpenSettings?.("doctor"); + openDoctor(); }; // CTA label shown in AttachmentActions. @@ -145,7 +154,7 @@ export function ConfigNudgeCard({ ))} - + {/* Affordance #3: always-visible CTA text (was opacity-0 fade-in). */} {ctaLabel} @@ -201,7 +210,7 @@ function RequirementRow({ on mixed cards where both card and row CTAs are visible. */} {showRowDoctorCta && ( + )} ); case "normalized_field": return ( -
- Set the {requirement.field} field in Edit Agent - dropdowns +
+ + Set the {requirement.field} field in Edit Agent + dropdowns + + {!allCliLogin && ( + + )}
); case "cli_login": @@ -203,12 +244,12 @@ function RequirementRow({ {cliLoginMessage(requirement)} - {/* (B) Inline Doctor CTA — shown only on mixed cards where the + {/* (B) Per-row Doctor CTA — shown only on mixed cards where the card-level trigger opens Edit Agent. When allCliLogin is true the card trigger already routes to Doctor; the per-row button is redundant and is suppressed. stopPropagation prevents double-fire on mixed cards where both card and row CTAs are visible. */} - {showRowDoctorCta && ( + {!allCliLogin && (