mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
refactor(desktop): retire dead install-gate, npm-preflight, and codex drift machinery
Since the bundling series emptied every runtime's adapter_install_commands, three pieces of machinery in the earlier cleanup plan were dead code. Delete them, and fix the Phase-3 install verification hint that the emptying broke for goose: - node_required install gate: runtime_needs_npm was constant false, so the node_required computation never fired and Doctor's amber "Node.js is required to install this adapter" callout could never render. Deleted the trigger (discovery.rs), the wire field (types.rs, tauri.ts, types.ts), the callout component + the nodeRequired half of the Install-button gate (DoctorSettingsPanel.tsx), and the fixture fields (e2eBridge.ts, agentReadiness.test.mjs, doctor-states.spec.ts). E2e test 04-node-required force-fed the retired state and is deleted; the bundled bridges' real Node.js requirement bites at spawn, not install, and is covered by the node-runtime Doctor section (07-node-runtime-warn). The cleanup plan's per-row spawn-time callout derived from nodeRuntimeCheck remains open as a follow-up. - npm preflight/EACCES machinery: Phase 2 of install_acp_runtime_blocking applied npm handling only to adapter_install_commands — empty everywhere. Deleted is_npm_global_install (+7 tests), npm_eacces_hint/npm_eacces_guidance/ NPM_MISSING_HINT (+5 tests), npm_preflight_check/resolve_npm_prefix/ npm_install_target_is_writable/unix_is_writable (+3 tests), the Phase-2 npm branches, and the stale Phase-1 preflight note. - codex adapter-availability drift stamp: built to catch a manual npm install/downgrade flipping the adapter under a running agent — impossible now that bare-name resolution prefers the bundle. Deleted the cache + availability_drift predicate (+5 tests), the codex-only discovery warm, the spawn-time stamp, the availability_drift half of needs_restart (hash_drift stays), and the ManagedAgentProcess field with its finish_spawn plumbing. Goose verify hint fix: adapter_verification_step received bundled = adapter_install_commands.is_empty(), now true for all four runtimes — a goose curl install that succeeded but left `goose` unresolvable claimed "The Goose ACP adapter ships with the Buzz desktop app… Reinstall Buzz", wrong on both counts. The new runtime_adapter_is_bundled seam requires cli_install_commands AND adapter_install_commands to be empty (goose has a curl CLI installer; claude/codex/buzz-agent have neither), with catalog-driven tests pinning goose to the check-the-step-output hint. That hint also drops its "and your npm global prefix" tail — no npm-installed adapters remain in the catalog. File-size ledger ratcheted down to bank the deletions: agent_discovery.rs 1410 -> 1021, discovery.rs 1147 -> 1046, runtime.rs 2216 -> 2079, tauri.ts 1342 -> 1340, types.ts 1066 -> 1064. Covers sections 1 and 3 of the post-bundling cleanup note; the adapter_missing retirement (section 2) and copy/fixture staleness (section 4) are separate follow-ups. Verification: - cargo test --lib (desktop/src-tauri): 1423 passed, 0 failed (1436 at head - 15 deleted npm/drift tests + 2 new bundled-flag tests) - cargo clippy --lib --tests -D warnings: clean; cargo fmt --check: clean - tsc --noEmit: clean; biome: no new diagnostics - pnpm test (desktop): 2792 passed, 0 failed - playwright doctor-states.spec.ts: 8 passed; doctor-cta-screenshots.spec.ts: 3 passed - node scripts/check-file-sizes.mjs + check-px-text.mjs: pass Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
Matt Toohey
co-authored by
Claude Fable 5
Claude Fable 5
parent
b3aed32a88
commit
f60138463f
@@ -121,7 +121,11 @@ const overrides = new Map([
|
||||
// record_provider param + applies persona_field_with_record_fallback. +5 lines.
|
||||
// global-agent-config: spawn_agent_child loads global config and merges as
|
||||
// lowest env layer (+8 lines). Queued to split.
|
||||
["src-tauri/src/managed_agents/runtime.rs", 2216],
|
||||
// acp-dead-machinery retirement: codex adapter-availability spawn stamp +
|
||||
// the availability_drift half of needs_restart deleted (the bundled bridge
|
||||
// can't drift out-of-band); ratcheted to bank the deletions (main's
|
||||
// team-instructions spawn-hash growth stays).
|
||||
["src-tauri/src/managed_agents/runtime.rs", 2087],
|
||||
// config-bridge setup-payload env-boundary fix adds readiness wiring in
|
||||
// spawn_agent_child; load-bearing security fix, queued to split.
|
||||
["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016],
|
||||
@@ -205,7 +209,9 @@ const overrides = new Map([
|
||||
// split with the rest of this file.
|
||||
// bundled-adapter-doctor-copy: adapter_bundled field on
|
||||
// RawAcpRuntimeCatalogEntry + mapper passthrough (+3 lines).
|
||||
["src/shared/api/tauri.ts", 1343],
|
||||
// acp-dead-machinery retirement: node_required wire field deleted;
|
||||
// ratcheted 1343 -> 1341.
|
||||
["src/shared/api/tauri.ts", 1341],
|
||||
// doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line).
|
||||
// doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/
|
||||
// loginHint fields on AcpRuntimeCatalogEntry (+14 lines). Load-bearing new feature.
|
||||
@@ -227,7 +233,9 @@ const overrides = new Map([
|
||||
// AcpRuntimeCatalogEntry (+2 lines).
|
||||
// bundled-cli-probes: "cli_missing" availability retired; +4 doc lines on
|
||||
// AcpAvailabilityStatus explaining why the state no longer exists.
|
||||
["src/shared/api/types.ts", 1075],
|
||||
// acp-dead-machinery retirement: nodeRequired field deleted;
|
||||
// ratcheted 1075 -> 1073.
|
||||
["src/shared/api/types.ts", 1073],
|
||||
// readiness-gate: PersonaDialog.tsx threads computeLocalModeGate +
|
||||
// requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog
|
||||
// shows required markers and credential amber rows (parity with
|
||||
@@ -294,7 +302,10 @@ const overrides = new Map([
|
||||
// bundled-cli-probes: resolve_probe_binary (bundled-CLI-first probe
|
||||
// resolution) + classify_runtime/underlying_cli doc rewrites for the
|
||||
// CliMissing retirement (+8 lines).
|
||||
["src-tauri/src/managed_agents/discovery.rs", 1267],
|
||||
// acp-dead-machinery retirement: adapter-availability cache +
|
||||
// availability_drift, runtime_needs_npm/is_npm_global_install, and the
|
||||
// node_required computation deleted; ratcheted 1267 -> 1166.
|
||||
["src-tauri/src/managed_agents/discovery.rs", 1166],
|
||||
// rebase over codex-acp-package-swap: its version-probe tests union with the
|
||||
// doctor-install-reliability nvm/login-shell/semver tests — each side alone
|
||||
// stayed under the 1000 default; the union exceeds it.
|
||||
@@ -470,7 +481,12 @@ const overrides = new Map([
|
||||
// bundle-acps: adapter_verification_step post-install gate + its test
|
||||
// quartet (+120 lines, partly offset by the version-gate retirement's
|
||||
// deletions in this file). Queued to split.
|
||||
["src-tauri/src/commands/agent_discovery.rs", 1576],
|
||||
// acp-dead-machinery retirement: npm EACCES preflight (resolve_npm_prefix,
|
||||
// npm_preflight_check, npm_eacces_hint) + its Phase-2 branches, test
|
||||
// groups, and the availability_drift tests deleted; ratcheted
|
||||
// 1576 -> 1184 to bank the deletions (main's Windows install-shell
|
||||
// machinery and tests stay).
|
||||
["src-tauri/src/commands/agent_discovery.rs", 1184],
|
||||
// draft-persistence predicate: submit-time `loadDraft` check + inline comment
|
||||
// + deps-array entry in submitMessage closes the never-persisted-boundary
|
||||
// defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to
|
||||
|
||||
@@ -4,9 +4,9 @@ use tauri::State;
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{
|
||||
command_availability, is_npm_global_install, AcpRuntimeCatalogEntry,
|
||||
DiscoverManagedAgentPrereqsRequest, InstallRuntimeResult, InstallStepResult,
|
||||
ManagedAgentPrereqsInfo, RelayAgentInfo, DEFAULT_ACP_COMMAND,
|
||||
command_availability, AcpRuntimeCatalogEntry, DiscoverManagedAgentPrereqsRequest,
|
||||
InstallRuntimeResult, InstallStepResult, ManagedAgentPrereqsInfo, RelayAgentInfo,
|
||||
DEFAULT_ACP_COMMAND,
|
||||
},
|
||||
nostr_convert,
|
||||
relay::query_relay,
|
||||
@@ -55,8 +55,8 @@ pub(crate) fn plan_adapter_install<'c>(
|
||||
/// Returns `None` when there is nothing to verify (`commands` is empty) or any
|
||||
/// adapter command resolves. Otherwise returns a failed synthetic "verify"
|
||||
/// step whose hint points at reinstalling Buzz when the adapter is bundled
|
||||
/// (`bundled`, i.e. the catalog carries no install commands) or at the install
|
||||
/// step output otherwise.
|
||||
/// (`bundled`, see [`runtime_adapter_is_bundled`]) or at the install step
|
||||
/// output otherwise.
|
||||
fn adapter_verification_step(
|
||||
commands: &[&str],
|
||||
label: &str,
|
||||
@@ -74,7 +74,7 @@ fn adapter_verification_step(
|
||||
} else {
|
||||
format!(
|
||||
"The {label} ACP adapter still could not be found after the install steps completed. \
|
||||
Check the step output above and your npm global prefix."
|
||||
Check the step output above."
|
||||
)
|
||||
};
|
||||
Some(InstallStepResult {
|
||||
@@ -88,6 +88,14 @@ fn adapter_verification_step(
|
||||
})
|
||||
}
|
||||
|
||||
/// A runtime's adapter ships with the Buzz desktop app when its catalog entry
|
||||
/// carries no install commands at all — neither CLI nor adapter. Goose has a
|
||||
/// curl CLI installer (and its CLI *is* its adapter), so a failed goose verify
|
||||
/// must not claim the adapter is bundled and point at reinstalling Buzz.
|
||||
fn runtime_adapter_is_bundled(runtime: &crate::managed_agents::KnownAcpRuntime) -> bool {
|
||||
runtime.cli_install_commands.is_empty() && runtime.adapter_install_commands.is_empty()
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub async fn discover_acp_providers() -> Result<Vec<AcpRuntimeCatalogEntry>, String> {
|
||||
tokio::task::spawn_blocking(|| {
|
||||
@@ -184,11 +192,6 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
|
||||
let mut steps = Vec::new();
|
||||
|
||||
// Phase 1: Install CLI if missing and commands are available.
|
||||
// NOTE: the npm EACCES preflight and `npm_eacces_hint` classifier only run
|
||||
// in Phase 2 below. Today every entry in `cli_install_commands` is a
|
||||
// curl-pipe; all `npm install -g` commands live in `adapter_install_commands`.
|
||||
// If a future runtime adds an npm-global CLI install it must also add the
|
||||
// preflight and classifier to this loop.
|
||||
if let Some(cli) = runtime.underlying_cli {
|
||||
if crate::managed_agents::resolve_command(cli).is_none() {
|
||||
for cmd in runtime.cli_install_commands_for_os() {
|
||||
@@ -218,21 +221,7 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
|
||||
plan_adapter_install(adapter_path.as_deref(), runtime.adapter_install_commands)
|
||||
{
|
||||
for cmd in cmds {
|
||||
if is_npm_global_install(cmd) {
|
||||
if let Some(step) = npm_preflight_check("adapter", cmd) {
|
||||
steps.push(step);
|
||||
return Ok(InstallRuntimeResult {
|
||||
success: false,
|
||||
steps,
|
||||
restarted_count: 0,
|
||||
failed_restart_count: 0,
|
||||
});
|
||||
}
|
||||
}
|
||||
let mut result = run_install_command("adapter", cmd);
|
||||
if !result.success && result.hint.is_none() && is_npm_global_install(cmd) {
|
||||
result.hint = npm_eacces_hint(&result.stderr, cmd);
|
||||
}
|
||||
let result = run_install_command("adapter", cmd);
|
||||
let success = result.success;
|
||||
steps.push(result);
|
||||
if !success {
|
||||
@@ -255,7 +244,7 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
|
||||
if let Some(step) = adapter_verification_step(
|
||||
runtime.commands,
|
||||
runtime.label,
|
||||
runtime.adapter_install_commands.is_empty(),
|
||||
runtime_adapter_is_bundled(runtime),
|
||||
crate::managed_agents::resolve_command,
|
||||
) {
|
||||
steps.push(step);
|
||||
@@ -591,10 +580,8 @@ fn persist_last_error_on_install(
|
||||
}
|
||||
|
||||
/// Build a login-shell `Command` for `command` with the hermit env vars
|
||||
/// stripped and the user's PATH set. This is the single source of truth for
|
||||
/// the shell selection and environment cleanup shared by `run_install_command`
|
||||
/// and `resolve_npm_prefix` — keeping them in sync so the hermit-strip list
|
||||
/// can't drift between the two paths.
|
||||
/// stripped and the user's PATH set, so install scripts run against the
|
||||
/// user's normal environment rather than the project-local hermit paths.
|
||||
///
|
||||
/// On Windows, resolves Git Bash via `resolve_bash_path` (skips `BUZZ_SHELL`
|
||||
/// since install commands require bash syntax). Returns `Err` when no shell
|
||||
@@ -844,233 +831,6 @@ fn floor_char_boundary(s: &str, mut index: usize) -> usize {
|
||||
index
|
||||
}
|
||||
|
||||
// ── npm EACCES preflight ──────────────────────────────────────────────────────
|
||||
|
||||
/// Guidance text for the EACCES / unwritable-prefix case.
|
||||
fn npm_eacces_guidance(command: &str) -> String {
|
||||
format!(
|
||||
"npm's global install directory isn't writable by your user.\n\
|
||||
\n\
|
||||
Fix (no sudo):\n\
|
||||
1. Run: npm config set prefix ~/.npm-global\n\
|
||||
2. Add to ~/.zprofile: export PATH=\"$HOME/.npm-global/bin:$PATH\"\n\
|
||||
3. Restart Buzz, then click Install again.\n\
|
||||
\n\
|
||||
Or install manually, then click Refresh:\n\
|
||||
sudo {command}"
|
||||
)
|
||||
}
|
||||
|
||||
/// Guidance text shown when npm / Node.js is not found in the login-shell PATH.
|
||||
const NPM_MISSING_HINT: &str = "Node.js / npm was not found. Install Node.js \
|
||||
(https://nodejs.org or your version manager), restart Buzz, then click Install again.\n\
|
||||
If npm works in your terminal, make sure your Node version manager is initialized in \
|
||||
~/.zprofile (not only ~/.zshrc) — Buzz resolves tools via non-interactive login shells.";
|
||||
|
||||
/// Result of probing `npm prefix -g` in the hermit-stripped login shell.
|
||||
#[cfg(unix)]
|
||||
enum NpmPrefix {
|
||||
/// npm responded with a parseable prefix path.
|
||||
Found(std::path::PathBuf),
|
||||
/// npm was not found, the spawn failed, the command returned a non-zero
|
||||
/// exit, or the output could not be parsed.
|
||||
Unavailable,
|
||||
/// The probe exceeded the 30-second deadline (e.g. a version-manager init
|
||||
/// that blocks on `/dev/tty`). The install should proceed so the stderr
|
||||
/// classifier remains the backstop.
|
||||
TimedOut,
|
||||
}
|
||||
|
||||
/// Spawn the same login shell used by `run_install_command` and run
|
||||
/// `npm prefix -g` to discover where npm would install global packages.
|
||||
#[cfg(unix)]
|
||||
fn resolve_npm_prefix() -> NpmPrefix {
|
||||
let mut cmd = match install_shell_command("npm prefix -g") {
|
||||
Ok(cmd) => cmd,
|
||||
Err(_) => return NpmPrefix::Unavailable,
|
||||
};
|
||||
let mut child = match cmd
|
||||
.stdin(std::process::Stdio::null())
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.spawn()
|
||||
{
|
||||
Ok(c) => c,
|
||||
Err(_) => return NpmPrefix::Unavailable,
|
||||
};
|
||||
|
||||
// Drain stdout/stderr on background threads to prevent pipe-buffer deadlock.
|
||||
let stdout_pipe = child.stdout.take();
|
||||
let stderr_pipe = child.stderr.take();
|
||||
let stdout_thread = std::thread::spawn(move || {
|
||||
let mut buf = Vec::new();
|
||||
if let Some(mut pipe) = stdout_pipe {
|
||||
let _ = pipe.read_to_end(&mut buf);
|
||||
}
|
||||
buf
|
||||
});
|
||||
let stderr_thread = std::thread::spawn(move || {
|
||||
// Drain stderr so the child doesn't block on a full pipe.
|
||||
if let Some(mut pipe) = stderr_pipe {
|
||||
let _ = std::io::copy(&mut pipe, &mut std::io::sink());
|
||||
}
|
||||
});
|
||||
|
||||
let child_pid = child.id();
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let wait_thread = std::thread::spawn(move || {
|
||||
let status = child.wait();
|
||||
let _ = tx.send(status);
|
||||
});
|
||||
|
||||
// 30-second timeout — plenty for `npm prefix -g`; intentionally shorter
|
||||
// than the 5-minute install budget in `run_install_command`.
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
|
||||
let raw_bytes: Option<Vec<u8>> = loop {
|
||||
let remaining = deadline.saturating_duration_since(std::time::Instant::now());
|
||||
if remaining.is_zero() {
|
||||
// Timed out: send SIGTERM, clean up threads, signal the caller to
|
||||
// fall through to the install path rather than abort.
|
||||
unsafe { libc::kill(child_pid as i32, libc::SIGTERM) };
|
||||
drop(rx);
|
||||
let _ = wait_thread.join();
|
||||
let _ = stdout_thread.join();
|
||||
let _ = stderr_thread.join();
|
||||
eprintln!(
|
||||
"buzz: npm prefix probe timed out after 30s; \
|
||||
proceeding to install (stderr classifier is the backstop)"
|
||||
);
|
||||
return NpmPrefix::TimedOut;
|
||||
}
|
||||
match rx.recv_timeout(std::time::Duration::from_millis(200).min(remaining)) {
|
||||
Ok(Ok(status)) => {
|
||||
let _ = wait_thread.join();
|
||||
let stdout = stdout_thread.join().unwrap_or_default();
|
||||
let _ = stderr_thread.join();
|
||||
break if status.success() { Some(stdout) } else { None };
|
||||
}
|
||||
Ok(Err(_)) | Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
|
||||
let _ = wait_thread.join();
|
||||
let _ = stdout_thread.join();
|
||||
let _ = stderr_thread.join();
|
||||
break None;
|
||||
}
|
||||
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => continue,
|
||||
}
|
||||
};
|
||||
|
||||
let bytes = match raw_bytes {
|
||||
Some(b) => b,
|
||||
None => return NpmPrefix::Unavailable,
|
||||
};
|
||||
let raw = String::from_utf8_lossy(&bytes).into_owned();
|
||||
// Version managers can print banner lines before the real prefix — take the
|
||||
// last non-empty line to skip any preamble.
|
||||
let prefix = match raw.lines().rfind(|l| !l.trim().is_empty()) {
|
||||
Some(l) => l.trim().to_string(),
|
||||
None => return NpmPrefix::Unavailable,
|
||||
};
|
||||
if prefix.is_empty() {
|
||||
return NpmPrefix::Unavailable;
|
||||
}
|
||||
NpmPrefix::Found(std::path::PathBuf::from(prefix))
|
||||
}
|
||||
|
||||
/// Check write access to a file-system path using the POSIX `access(2)` syscall.
|
||||
#[cfg(unix)]
|
||||
fn unix_is_writable(path: &std::path::Path) -> bool {
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
let bytes = path.as_os_str().as_bytes();
|
||||
let Ok(c_path) = std::ffi::CString::new(bytes) else {
|
||||
return false;
|
||||
};
|
||||
// SAFETY: `access` is a pure read-only syscall; we pass a valid NUL-terminated
|
||||
// path and a standard flag constant. This mirrors the existing `setsid`/`kill`
|
||||
// usage in this file.
|
||||
unsafe { libc::access(c_path.as_ptr(), libc::W_OK) == 0 }
|
||||
}
|
||||
|
||||
/// Returns true when the directory where npm would write global packages is
|
||||
/// writable by the current process user.
|
||||
///
|
||||
/// On non-unix platforms always returns `true` — the EACCES preflight is a
|
||||
/// no-op there; the stderr classifier still applies.
|
||||
fn npm_install_target_is_writable(prefix: &std::path::Path) -> bool {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
// Probe the most specific candidate that exists; fall back up the tree.
|
||||
for candidate in &[
|
||||
prefix.join("lib/node_modules"),
|
||||
prefix.join("lib"),
|
||||
prefix.to_path_buf(),
|
||||
] {
|
||||
if candidate.exists() {
|
||||
return unix_is_writable(candidate);
|
||||
}
|
||||
}
|
||||
// Nothing exists — npm couldn't create it either.
|
||||
unix_is_writable(prefix)
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
let _ = prefix;
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
/// Inspect `stderr` for known npm EACCES patterns and return actionable
|
||||
/// guidance if matched, or `None` when the error is unrelated.
|
||||
fn npm_eacces_hint(stderr: &str, command: &str) -> Option<String> {
|
||||
if stderr.contains("EACCES: permission denied") || stderr.contains("npm error EACCES") {
|
||||
Some(npm_eacces_guidance(command))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the npm preflight before executing an npm global install command.
|
||||
/// Returns `Some(failed InstallStepResult)` to abort, or `None` to proceed.
|
||||
fn npm_preflight_check(step: &str, command: &str) -> Option<InstallStepResult> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
match resolve_npm_prefix() {
|
||||
NpmPrefix::Unavailable => Some(InstallStepResult {
|
||||
step: step.to_string(),
|
||||
command: command.to_string(),
|
||||
success: false,
|
||||
stdout: String::new(),
|
||||
stderr: String::new(),
|
||||
exit_code: None,
|
||||
hint: Some(NPM_MISSING_HINT.to_string()),
|
||||
}),
|
||||
NpmPrefix::Found(prefix) if !npm_install_target_is_writable(&prefix) => {
|
||||
Some(InstallStepResult {
|
||||
step: step.to_string(),
|
||||
command: command.to_string(),
|
||||
success: false,
|
||||
stdout: String::new(),
|
||||
stderr: format!(
|
||||
"npm global prefix '{}' is not writable by the current user.",
|
||||
prefix.display()
|
||||
),
|
||||
exit_code: None,
|
||||
hint: Some(npm_eacces_guidance(command)),
|
||||
})
|
||||
}
|
||||
// `Found` + writable, or `TimedOut` — proceed; let the install run and
|
||||
// the stderr classifier serve as the backstop.
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
let _ = (step, command);
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
// ── end npm preflight ─────────────────────────────────────────────────────────
|
||||
|
||||
#[tauri::command]
|
||||
pub async fn discover_managed_agent_prereqs(
|
||||
input: DiscoverManagedAgentPrereqsRequest,
|
||||
@@ -1123,124 +883,6 @@ pub async fn list_relay_agents(state: State<'_, AppState>) -> Result<Vec<RelayAg
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// ── is_npm_global_install ─────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_accepts_catalog_claude_command() {
|
||||
assert!(is_npm_global_install(
|
||||
"npm install -g @agentclientprotocol/claude-agent-acp"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_accepts_catalog_codex_command() {
|
||||
assert!(is_npm_global_install(
|
||||
"npm install -g @agentclientprotocol/codex-acp"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_accepts_short_flag() {
|
||||
assert!(is_npm_global_install("npm i -g some-package"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_accepts_leading_whitespace() {
|
||||
assert!(is_npm_global_install(" npm install -g foo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_rejects_curl_pipe() {
|
||||
assert!(!is_npm_global_install(
|
||||
"curl -fsSL https://example.com/install.sh | bash"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_rejects_non_global_install() {
|
||||
assert!(!is_npm_global_install("npm install foo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_npm_global_install_rejects_unrelated_command() {
|
||||
assert!(!is_npm_global_install("cargo install some-tool"));
|
||||
}
|
||||
|
||||
// ── npm_eacces_hint ───────────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_detects_old_format() {
|
||||
let stderr = "npm ERR! code EACCES\nnpm ERR! syscall mkdir\nnpm ERR! path /usr/local/lib/node_modules\nnpm ERR! errno -13\nnpm ERR! Error: EACCES: permission denied, mkdir '/usr/local/lib/node_modules'";
|
||||
assert!(npm_eacces_hint(stderr, "npm install -g foo").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_detects_new_format() {
|
||||
let stderr = "npm error EACCES: permission denied, mkdir '/usr/local/lib/node_modules'";
|
||||
assert!(npm_eacces_hint(stderr, "npm install -g foo").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_returns_none_for_404_stderr() {
|
||||
let stderr = "npm error 404 Not Found - GET https://registry.npmjs.org/no-such-pkg";
|
||||
assert!(npm_eacces_hint(stderr, "npm install -g no-such-pkg").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_guidance_contains_npm_global_path() {
|
||||
let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
|
||||
assert!(hint.contains("~/.npm-global"), "hint: {hint}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_guidance_contains_zprofile() {
|
||||
let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
|
||||
assert!(hint.contains("~/.zprofile"), "hint: {hint}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_eacces_hint_guidance_contains_sudo_command() {
|
||||
let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
|
||||
assert!(hint.contains("sudo npm install -g foo"), "hint: {hint}");
|
||||
}
|
||||
|
||||
// ── npm_install_target_is_writable ────────────────────────────────────────
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_npm_install_target_is_writable_true_on_writable_dir() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
assert!(npm_install_target_is_writable(dir.path()));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_npm_install_target_is_writable_false_when_lib_node_modules_unwritable() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let lib = dir.path().join("lib");
|
||||
let node_modules = lib.join("node_modules");
|
||||
std::fs::create_dir_all(&node_modules).unwrap();
|
||||
// Make node_modules read-only.
|
||||
std::fs::set_permissions(&node_modules, std::fs::Permissions::from_mode(0o555)).unwrap();
|
||||
let result = npm_install_target_is_writable(dir.path());
|
||||
// Restore before the dir is dropped so cleanup can delete it.
|
||||
std::fs::set_permissions(&node_modules, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
// Skip this assertion when running as root (root can write to 0o555).
|
||||
if unsafe { libc::getuid() } != 0 {
|
||||
assert!(!result);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_npm_install_target_is_writable_walks_up_to_lib() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
// Create only `lib/` — no `lib/node_modules`.
|
||||
std::fs::create_dir(dir.path().join("lib")).unwrap();
|
||||
assert!(npm_install_target_is_writable(dir.path()));
|
||||
}
|
||||
|
||||
// ── plan_adapter_install ──────────────────────────────────────────────────
|
||||
|
||||
/// plan_adapter_install is the pure install-plan seam used by
|
||||
@@ -1335,68 +977,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── badge availability-drift (Phase 2) ───────────────────────────────────
|
||||
//
|
||||
// `availability_drift` is a pure predicate over two `Option` values —
|
||||
// no global state, no parallelism hazard.
|
||||
|
||||
/// Both sides known and different → drift detected.
|
||||
#[test]
|
||||
fn test_availability_drift_detected_when_stamped_differs_from_current() {
|
||||
use crate::managed_agents::{availability_drift, AcpAvailabilityStatus};
|
||||
assert!(
|
||||
availability_drift(
|
||||
Some(&AcpAvailabilityStatus::Available),
|
||||
Some(AcpAvailabilityStatus::AdapterMissing),
|
||||
),
|
||||
"Available stamped vs AdapterMissing current must be detected as drift"
|
||||
);
|
||||
}
|
||||
|
||||
/// Both sides known and equal → no drift.
|
||||
#[test]
|
||||
fn test_availability_drift_no_drift_when_stamped_equals_current() {
|
||||
use crate::managed_agents::{availability_drift, AcpAvailabilityStatus};
|
||||
assert!(
|
||||
!availability_drift(
|
||||
Some(&AcpAvailabilityStatus::Available),
|
||||
Some(AcpAvailabilityStatus::Available),
|
||||
),
|
||||
"matching stamped and current must not show drift"
|
||||
);
|
||||
}
|
||||
|
||||
/// Stamped is None (cold cache at spawn) → no drift regardless of current.
|
||||
#[test]
|
||||
fn test_availability_drift_none_stamp_never_drifts() {
|
||||
use crate::managed_agents::{availability_drift, AcpAvailabilityStatus};
|
||||
assert!(
|
||||
!availability_drift(None, Some(AcpAvailabilityStatus::Available)),
|
||||
"None stamp (cold cache at spawn) must never signal drift"
|
||||
);
|
||||
}
|
||||
|
||||
/// Current is None (cache cold now) → no drift regardless of stamp.
|
||||
#[test]
|
||||
fn test_availability_drift_none_current_never_drifts() {
|
||||
use crate::managed_agents::{availability_drift, AcpAvailabilityStatus};
|
||||
assert!(
|
||||
!availability_drift(Some(&AcpAvailabilityStatus::Available), None),
|
||||
"None current (cache cold) must never signal drift"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-codex agent (stamp is None) → no drift (None case).
|
||||
#[test]
|
||||
fn test_availability_drift_non_codex_none_never_drifts() {
|
||||
use crate::managed_agents::{availability_drift, AcpAvailabilityStatus};
|
||||
// Non-codex agents have `adapter_availability = None` — must never flip.
|
||||
assert!(
|
||||
!availability_drift(None, Some(AcpAvailabilityStatus::AdapterMissing)),
|
||||
"non-codex agent (None stamp) must never trigger drift badge"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Phase A: install shell selection ─────────────────────────────────────
|
||||
|
||||
/// On Unix, resolve_install_shell always succeeds (returns zsh or bash).
|
||||
@@ -1562,6 +1142,34 @@ mod tests {
|
||||
"no adapter commands means nothing to verify"
|
||||
);
|
||||
}
|
||||
|
||||
// ── runtime_adapter_is_bundled ────────────────────────────────────────────
|
||||
//
|
||||
// Guards the Phase-3 hint selection against catalog drift: goose installs
|
||||
// via a curl script (its CLI is its adapter), so a failed goose verify must
|
||||
// get the check-the-step-output hint, never the reinstall-Buzz one.
|
||||
|
||||
#[test]
|
||||
fn test_runtime_adapter_is_bundled_false_for_goose() {
|
||||
let goose = crate::managed_agents::known_acp_runtime_exact("goose")
|
||||
.expect("goose must be in the catalog");
|
||||
assert!(
|
||||
!runtime_adapter_is_bundled(goose),
|
||||
"goose has a curl CLI installer — its adapter is not bundled with Buzz"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_runtime_adapter_is_bundled_true_for_bundled_bridges() {
|
||||
for id in ["claude", "codex", "buzz-agent"] {
|
||||
let runtime = crate::managed_agents::known_acp_runtime_exact(id)
|
||||
.unwrap_or_else(|| panic!("{id} must be in the catalog"));
|
||||
assert!(
|
||||
runtime_adapter_is_bundled(runtime),
|
||||
"{id} carries no install commands — its adapter ships with Buzz"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the Windows-only Git Bash prerequisite used by buzz-agent's shell MCP.
|
||||
|
||||
@@ -547,73 +547,6 @@ pub fn resolve_command(command: &str) -> Option<PathBuf> {
|
||||
pub fn clear_resolve_cache() {
|
||||
let mut guard = resolve_cache().lock().unwrap_or_else(|e| e.into_inner());
|
||||
guard.clear();
|
||||
// Also invalidate the adapter-availability cache so a freshly-installed
|
||||
// adapter is reflected the next time the summary builder checks the badge.
|
||||
clear_adapter_availability_cache();
|
||||
}
|
||||
|
||||
// ── Adapter availability cache (Phase-2 badge fallback) ─────────────────────
|
||||
//
|
||||
// `build_managed_agent_summary` needs to compare the spawn-time adapter
|
||||
// availability against the *current* availability without re-running command
|
||||
// resolution on every poll cycle. This cache stores the last availability
|
||||
// status of the codex-acp binary at its resolved path. It is warmed by
|
||||
// `discover_acp_runtimes` (which already resolves), so the badge path reads
|
||||
// warm data, and is invalidated by `clear_resolve_cache`
|
||||
// (called on every Doctor install and every `discover_acp_providers` call).
|
||||
|
||||
fn adapter_availability_cache() -> &'static std::sync::Mutex<Option<AcpAvailabilityStatus>> {
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
static CACHE: OnceLock<Mutex<Option<AcpAvailabilityStatus>>> = OnceLock::new();
|
||||
CACHE.get_or_init(|| Mutex::new(None))
|
||||
}
|
||||
|
||||
fn clear_adapter_availability_cache() {
|
||||
if let Ok(mut guard) = adapter_availability_cache().lock() {
|
||||
*guard = None;
|
||||
}
|
||||
}
|
||||
|
||||
/// Cache the current codex-acp adapter availability status.
|
||||
///
|
||||
/// Called by `discover_acp_runtimes` after it probes the codex adapter so the
|
||||
/// badge path has a warm value without re-probing.
|
||||
pub(crate) fn cache_adapter_availability(status: AcpAvailabilityStatus) {
|
||||
if let Ok(mut guard) = adapter_availability_cache().lock() {
|
||||
*guard = Some(status);
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the most recently cached codex-acp adapter availability, or
|
||||
/// `None` if no discovery has run yet.
|
||||
///
|
||||
/// This is a **read from cache only** — it never spawns a subprocess. The
|
||||
/// value is populated by `discover_acp_runtimes` and invalidated by
|
||||
/// `clear_resolve_cache`. When the cache is cold, returning `None` defers
|
||||
/// the drift check until discovery has produced a real value, preventing
|
||||
/// a fabricated `AdapterMissing` stamp from triggering a false restart badge
|
||||
/// on a newly restarted process.
|
||||
pub(crate) fn adapter_availability_cached() -> Option<AcpAvailabilityStatus> {
|
||||
adapter_availability_cache()
|
||||
.lock()
|
||||
.ok()
|
||||
.and_then(|g| g.clone())
|
||||
}
|
||||
|
||||
/// Pure predicate: does the stamped adapter availability differ from the
|
||||
/// current cached availability?
|
||||
///
|
||||
/// Returns `false` whenever either side is `None` (unknown) — "no data" is
|
||||
/// not evidence of drift. This is extracted for unit testing without global
|
||||
/// state and used by `build_managed_agent_summary`.
|
||||
pub(crate) fn availability_drift(
|
||||
stamped: Option<&AcpAvailabilityStatus>,
|
||||
current: Option<AcpAvailabilityStatus>,
|
||||
) -> bool {
|
||||
match (stamped, current) {
|
||||
(Some(s), Some(c)) => *s != c,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Return all candidate basenames for `command` on the current platform.
|
||||
@@ -943,24 +876,6 @@ pub(crate) fn find_command(command: &str) -> Option<PathBuf> {
|
||||
resolve_command(command)
|
||||
}
|
||||
|
||||
/// Returns true when the runtime has at least one adapter install step that
|
||||
/// is an npm global install. Used to determine whether Node.js is required.
|
||||
fn runtime_needs_npm(runtime: &KnownAcpRuntime) -> bool {
|
||||
runtime
|
||||
.adapter_install_commands
|
||||
.iter()
|
||||
.any(|cmd| is_npm_global_install(cmd))
|
||||
}
|
||||
|
||||
/// Returns `true` when `cmd` is an `npm install -g` invocation.
|
||||
///
|
||||
/// Used by Doctor to determine whether Node.js is required before running an
|
||||
/// install step, and by the npm EACCES preflight in the install command path.
|
||||
pub(crate) fn is_npm_global_install(cmd: &str) -> bool {
|
||||
let t = cmd.trim_start();
|
||||
t.starts_with("npm install -g ") || t.starts_with("npm i -g ")
|
||||
}
|
||||
|
||||
/// Resolve the binary for a CLI auth probe (`claude`, `codex`): the pinned
|
||||
/// CLI vendored inside the bundled bridge wins — it is the exact binary agent
|
||||
/// sessions run and reads the same credential store — falling back to the
|
||||
@@ -1136,13 +1051,6 @@ pub fn discover_acp_runtimes() -> Vec<AcpRuntimeCatalogEntry> {
|
||||
let (availability, command, binary_path) =
|
||||
classify_runtime(adapter_result, runtime.underlying_cli, underlying_cli_found);
|
||||
|
||||
// Warm the adapter-availability cache for the badge fallback.
|
||||
// The cache is scoped to the codex runtime; other runtimes leave it
|
||||
// unchanged. Invalidated by `clear_resolve_cache`.
|
||||
if runtime.id == "codex" {
|
||||
cache_adapter_availability(availability.clone());
|
||||
}
|
||||
|
||||
let underlying_cli_path = runtime
|
||||
.underlying_cli
|
||||
.and_then(find_command)
|
||||
@@ -1172,14 +1080,6 @@ pub fn discover_acp_runtimes() -> Vec<AcpRuntimeCatalogEntry> {
|
||||
}
|
||||
};
|
||||
|
||||
// node_required: an npm adapter step is pending AND node/npm are absent.
|
||||
let node_required = matches!(
|
||||
availability,
|
||||
AcpAvailabilityStatus::AdapterMissing | AcpAvailabilityStatus::NotInstalled
|
||||
) && runtime_needs_npm(runtime)
|
||||
&& resolve_command("npm").is_none()
|
||||
&& resolve_command("node").is_none();
|
||||
|
||||
PartialEntry {
|
||||
runtime,
|
||||
entry: AcpRuntimeCatalogEntry {
|
||||
@@ -1196,7 +1096,6 @@ pub fn discover_acp_runtimes() -> Vec<AcpRuntimeCatalogEntry> {
|
||||
install_instructions_url: runtime.install_instructions_url.to_string(),
|
||||
can_auto_install,
|
||||
underlying_cli_path,
|
||||
node_required,
|
||||
// Filled in by the probe phase below.
|
||||
auth_status: AuthStatus::Unknown,
|
||||
login_hint: None,
|
||||
|
||||
@@ -135,7 +135,6 @@ pub fn finish_spawn(
|
||||
log_path: std::path::PathBuf,
|
||||
spawn_config_hash: u64,
|
||||
setup_mode: bool,
|
||||
adapter_availability: Option<super::AcpAvailabilityStatus>,
|
||||
agent_name: &str,
|
||||
) -> super::ManagedAgentProcess {
|
||||
let job = create_job_for_child(child.id());
|
||||
@@ -150,7 +149,6 @@ pub fn finish_spawn(
|
||||
log_path,
|
||||
spawn_config_hash,
|
||||
setup_mode,
|
||||
adapter_availability,
|
||||
job,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1321,31 +1321,20 @@ pub fn build_managed_agent_summary(
|
||||
// at launch; recompute from current disk state and flag drift. Only a
|
||||
// tracked live process can drift — stopped agents spawn fresh, and
|
||||
// adopted (runtime_pid-only) processes have no stamped hash to compare.
|
||||
//
|
||||
// Additionally, for runtimes with an adapter version gate (codex only),
|
||||
// check whether the cached adapter availability has drifted from the value
|
||||
// stamped at spawn. This catches out-of-band adapter changes (manual
|
||||
// npm install/downgrade) that Phase-1 auto-restart doesn't cover. The
|
||||
// cache is read-only here — no subprocess is spawned.
|
||||
let needs_restart = runtimes.get(&record.pubkey).is_some_and(|runtime| {
|
||||
use tauri::Manager;
|
||||
let state = app.state::<crate::app_state::AppState>();
|
||||
let global_for_hash =
|
||||
crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
let teams_for_hash = crate::managed_agents::load_teams(app).unwrap_or_default();
|
||||
let hash_drift = runtime.spawn_config_hash
|
||||
runtime.spawn_config_hash
|
||||
!= crate::managed_agents::spawn_hash::spawn_config_hash(
|
||||
record,
|
||||
personas,
|
||||
&teams_for_hash,
|
||||
&crate::relay::relay_ws_url_with_override(&state),
|
||||
&global_for_hash,
|
||||
);
|
||||
let availability_drift = super::availability_drift(
|
||||
runtime.adapter_availability.as_ref(),
|
||||
super::adapter_availability_cached(),
|
||||
);
|
||||
hash_drift || availability_drift
|
||||
)
|
||||
});
|
||||
|
||||
// Resolve the effective harness the same way, then derive args/mcp from it,
|
||||
@@ -1891,20 +1880,6 @@ pub fn spawn_agent_child(
|
||||
&global,
|
||||
);
|
||||
|
||||
// Stamp the adapter availability for runtimes with a version gate (codex
|
||||
// only). The summary builder compares this against the current cached value
|
||||
// to detect out-of-band adapter changes after spawn (Phase-2 badge fallback).
|
||||
// Non-codex runtimes get `None` — nothing changes for them.
|
||||
// When the cache is cold (e.g. Doctor just installed and cleared the cache),
|
||||
// `adapter_availability_cached()` returns `None`, so the stamp is `None` and
|
||||
// the drift check is skipped until discovery warms the cache — preventing a
|
||||
// false restart badge immediately after auto-restart.
|
||||
let spawned_adapter_availability = if runtime_meta.is_some_and(|r| r.id == "codex") {
|
||||
super::adapter_availability_cached()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let _ = super::write_agent_pid_file(app, &record.pubkey, child.id());
|
||||
|
||||
// Windows: assign the harness to a Job Object so its whole tree dies with
|
||||
@@ -1915,7 +1890,6 @@ pub fn spawn_agent_child(
|
||||
log_path,
|
||||
spawn_config_hash,
|
||||
spawned_setup_mode,
|
||||
spawned_adapter_availability,
|
||||
&record.name,
|
||||
));
|
||||
#[cfg(not(windows))]
|
||||
@@ -1924,7 +1898,6 @@ pub fn spawn_agent_child(
|
||||
log_path,
|
||||
spawn_config_hash,
|
||||
setup_mode: spawned_setup_mode,
|
||||
adapter_availability: spawned_adapter_availability,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -435,12 +435,6 @@ pub struct ManagedAgentProcess {
|
||||
/// `install_acp_runtime` to target only stuck agents for auto-restart,
|
||||
/// excluding healthy in-pool agents.
|
||||
pub setup_mode: bool,
|
||||
/// Adapter availability status stamped at spawn time for runtimes with a
|
||||
/// version gate (currently codex only; `None` for all others). Runtime-only
|
||||
/// — never persisted. The summary builder compares this against the current
|
||||
/// cached availability and sets `needs_restart` on drift, catching out-of-
|
||||
/// band adapter changes that Phase-1 auto-restart doesn't cover.
|
||||
pub adapter_availability: Option<AcpAvailabilityStatus>,
|
||||
/// Win32 Job Object owning the harness + its entire process tree. Closing
|
||||
/// the handle (via `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`) kills the whole
|
||||
/// tree — the Windows mirror of the Unix process-group teardown. `None`
|
||||
@@ -581,9 +575,6 @@ pub struct AcpRuntimeCatalogEntry {
|
||||
/// true when at least one automated install step is available
|
||||
pub can_auto_install: bool,
|
||||
pub underlying_cli_path: Option<String>,
|
||||
/// true when an npm adapter step is pending but Node.js / npm is absent.
|
||||
/// The UI hides the Install button and shows a Node.js install callout.
|
||||
pub node_required: bool,
|
||||
/// Login/authentication status for CLI-based runtimes.
|
||||
pub auth_status: AuthStatus,
|
||||
/// Hint for completing authentication, shown when `auth_status` is not `logged_in`.
|
||||
|
||||
@@ -19,7 +19,6 @@ function makeRuntime(overrides = {}) {
|
||||
installInstructionsUrl: "https://example.com",
|
||||
canAutoInstall: false,
|
||||
underlyingCliPath: null,
|
||||
nodeRequired: false,
|
||||
loginHint: null,
|
||||
...overrides,
|
||||
};
|
||||
|
||||
@@ -83,7 +83,7 @@ function InstallActions({
|
||||
onInstall: () => void;
|
||||
runtime: AcpRuntimeCatalogEntry;
|
||||
}) {
|
||||
const showInstall = runtime.canAutoInstall && !runtime.nodeRequired;
|
||||
const showInstall = runtime.canAutoInstall;
|
||||
|
||||
return (
|
||||
<div className="mt-2 flex items-center gap-2">
|
||||
@@ -117,46 +117,6 @@ function InstallActions({
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Node.js callout when required, or the install actions when it is not.
|
||||
* Used for both `adapter_missing` and `not_installed` availability states.
|
||||
*/
|
||||
function NodeRequiredOrInstall({
|
||||
hasError,
|
||||
isInstalling,
|
||||
onInstall,
|
||||
runtime,
|
||||
}: {
|
||||
hasError: boolean;
|
||||
isInstalling: boolean;
|
||||
onInstall: () => void;
|
||||
runtime: AcpRuntimeCatalogEntry;
|
||||
}) {
|
||||
if (runtime.nodeRequired) {
|
||||
return (
|
||||
<p className="mt-2 rounded-lg border border-amber-500/30 bg-amber-500/10 px-3 py-1.5 text-sm text-amber-700 dark:text-amber-400">
|
||||
Node.js is required to install this adapter.{" "}
|
||||
<button
|
||||
className="underline underline-offset-2 hover:no-underline"
|
||||
onClick={() => void openUrl("https://nodejs.org")}
|
||||
type="button"
|
||||
>
|
||||
Install Node.js
|
||||
</button>
|
||||
, then click Re-run.
|
||||
</p>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<InstallActions
|
||||
hasError={hasError}
|
||||
isInstalling={isInstalling}
|
||||
onInstall={onInstall}
|
||||
runtime={runtime}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
function RuntimeRow({
|
||||
installError,
|
||||
installSuccess,
|
||||
@@ -262,7 +222,7 @@ function RuntimeRow({
|
||||
<p className="mt-1 text-sm font-normal text-muted-foreground">
|
||||
{runtime.installHint}
|
||||
</p>
|
||||
<NodeRequiredOrInstall
|
||||
<InstallActions
|
||||
hasError={installError !== null}
|
||||
isInstalling={isInstalling}
|
||||
onInstall={onInstall}
|
||||
@@ -277,7 +237,7 @@ function RuntimeRow({
|
||||
<p className="mt-1 text-sm font-normal text-muted-foreground">
|
||||
{runtime.installHint}
|
||||
</p>
|
||||
<NodeRequiredOrInstall
|
||||
<InstallActions
|
||||
hasError={installError !== null}
|
||||
isInstalling={isInstalling}
|
||||
onInstall={onInstall}
|
||||
|
||||
@@ -233,7 +233,6 @@ export type RawAcpRuntimeCatalogEntry = {
|
||||
install_instructions_url: string;
|
||||
can_auto_install: boolean;
|
||||
underlying_cli_path: string | null;
|
||||
node_required: boolean;
|
||||
/** Tagged union with snake_case status values — same shape as `AuthStatus`. */
|
||||
auth_status: AuthStatus;
|
||||
login_hint?: string;
|
||||
@@ -928,7 +927,6 @@ function fromRawAcpRuntimeCatalogEntry(
|
||||
installInstructionsUrl: entry.install_instructions_url,
|
||||
canAutoInstall: entry.can_auto_install,
|
||||
underlyingCliPath: entry.underlying_cli_path,
|
||||
nodeRequired: entry.node_required,
|
||||
authStatus: entry.auth_status,
|
||||
loginHint: entry.login_hint ?? null,
|
||||
};
|
||||
|
||||
@@ -587,8 +587,6 @@ export type AcpRuntimeCatalogEntry = {
|
||||
installInstructionsUrl: string;
|
||||
canAutoInstall: boolean;
|
||||
underlyingCliPath: string | null;
|
||||
/** True when an npm adapter step is pending but Node.js / npm is absent. */
|
||||
nodeRequired: boolean;
|
||||
/** Login/auth status for CLI-based runtimes. */
|
||||
authStatus: AuthStatus;
|
||||
/** Hint for completing authentication; null when not applicable or already logged in. */
|
||||
|
||||
@@ -6434,7 +6434,6 @@ async function handleDiscoverAcpRuntimes(
|
||||
install_instructions_url: "https://block.github.io/goose/",
|
||||
can_auto_install: true,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "not_applicable" },
|
||||
login_hint: undefined,
|
||||
},
|
||||
@@ -6452,7 +6451,6 @@ async function handleDiscoverAcpRuntimes(
|
||||
"https://www.npmjs.com/package/@anthropic-ai/claude-agent-acp",
|
||||
can_auto_install: true,
|
||||
underlying_cli_path: "/usr/local/bin/claude",
|
||||
node_required: false,
|
||||
auth_status: { status: "unknown" },
|
||||
login_hint: undefined,
|
||||
},
|
||||
@@ -6470,7 +6468,6 @@ async function handleDiscoverAcpRuntimes(
|
||||
install_instructions_url: "https://github.com/openai/codex",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "unknown" },
|
||||
login_hint: undefined,
|
||||
},
|
||||
@@ -6487,7 +6484,6 @@ async function handleDiscoverAcpRuntimes(
|
||||
install_instructions_url: "https://github.com/block/buzz",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "not_applicable" },
|
||||
login_hint: undefined,
|
||||
},
|
||||
|
||||
@@ -25,7 +25,6 @@ const GOOSE_AVAILABLE = {
|
||||
install_instructions_url: "https://block.github.io/goose/",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "not_applicable" },
|
||||
};
|
||||
|
||||
@@ -43,7 +42,6 @@ const BUZZ_AGENT_AVAILABLE = {
|
||||
install_instructions_url: "https://github.com/block/buzz",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "not_applicable" },
|
||||
};
|
||||
|
||||
@@ -68,13 +66,12 @@ const CLAUDE_AVAILABLE_LOGGED_IN = {
|
||||
"https://github.com/agentclientprotocol/claude-agent-acp",
|
||||
can_auto_install: false,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "logged_in" },
|
||||
};
|
||||
|
||||
/**
|
||||
* Codex not-installed base — tweak `availability`, `auth_status`, and
|
||||
* `node_required` in each test as needed.
|
||||
* Codex not-installed base — tweak `availability` and `auth_status` in each
|
||||
* test as needed.
|
||||
*/
|
||||
const CODEX_NOT_INSTALLED = {
|
||||
id: "codex",
|
||||
@@ -89,7 +86,6 @@ const CODEX_NOT_INSTALLED = {
|
||||
install_instructions_url: "https://github.com/zed-industries/codex-acp",
|
||||
can_auto_install: true,
|
||||
underlying_cli_path: null,
|
||||
node_required: false,
|
||||
auth_status: { status: "unknown" },
|
||||
};
|
||||
|
||||
@@ -206,43 +202,12 @@ test.describe("Doctor panel state screenshots", () => {
|
||||
await row.screenshot({ path: `${SHOTS}/03-auth-config-error.png` });
|
||||
});
|
||||
|
||||
/**
|
||||
* 04 — adapter_missing runtime with node_required: true: the amber "Node.js
|
||||
* is required…" callout replaces the Install button so the user cannot
|
||||
* inadvertently trigger a doomed npm install.
|
||||
/*
|
||||
* 04-node-required retired with the per-row `node_required` install gate:
|
||||
* no runtime carries npm adapter install commands anymore, so the amber
|
||||
* "Node.js is required…" callout has no trigger. The Node.js requirement
|
||||
* for the bundled bridges is covered by 07-node-runtime-warn below.
|
||||
*/
|
||||
test("04-node-required", async ({ page }) => {
|
||||
await installMockBridge(page, {
|
||||
acpRuntimesCatalog: [
|
||||
GOOSE_AVAILABLE,
|
||||
CLAUDE_AVAILABLE_LOGGED_IN,
|
||||
{
|
||||
...CODEX_NOT_INSTALLED,
|
||||
availability: "adapter_missing",
|
||||
underlying_cli_path: "/usr/local/bin/codex",
|
||||
node_required: true,
|
||||
install_hint:
|
||||
"Install the Codex ACP adapter: npm install -g @zed-industries/codex-acp",
|
||||
},
|
||||
BUZZ_AGENT_AVAILABLE,
|
||||
],
|
||||
});
|
||||
|
||||
await page.goto("/", { waitUntil: "domcontentloaded" });
|
||||
await openSettings(page, "doctor");
|
||||
|
||||
const row = page.getByTestId("doctor-runtime-codex");
|
||||
await expect(row).toBeVisible({ timeout: 10_000 });
|
||||
await expect(row).toContainText("Node.js is required");
|
||||
// Exact-name match so "Install Node.js" (inside the callout) is not counted.
|
||||
await expect(
|
||||
row.getByRole("button", { name: "Install", exact: true }),
|
||||
).toHaveCount(0);
|
||||
|
||||
await row.scrollIntoViewIfNeeded();
|
||||
await waitForAnimations(page);
|
||||
await row.screenshot({ path: `${SHOTS}/04-node-required.png` });
|
||||
});
|
||||
|
||||
/**
|
||||
* 05 — a failed install renders a "Retry" button; clicking Retry succeeds.
|
||||
@@ -261,7 +226,6 @@ test.describe("Doctor panel state screenshots", () => {
|
||||
{
|
||||
...CODEX_NOT_INSTALLED,
|
||||
can_auto_install: true,
|
||||
node_required: false,
|
||||
},
|
||||
BUZZ_AGENT_AVAILABLE,
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user