fix: stop installing agent CLIs at runtime

Require ACP nodes to provide an explicit acp_command and make CLI backend runs depend on tools already present in the sandbox PATH. Remove the Node runtime bootstrap path and mark old CLI ensure events as legacy.
This commit is contained in:
Bryan Helmkamp 2026-05-11 15:48:25 -04:00
parent 537ab7a511
commit 5d898152a7
No known key found for this signature in database
16 changed files with 199 additions and 560 deletions

View file

@ -1952,6 +1952,8 @@ Emitted when an image or snapshot ensure step fails.
## CLI ensure events
These legacy events may appear in older run logs. Current CLI backend runs do not emit them because Fabro no longer installs or prepares provider CLIs at stage runtime.
### `cli.ensure.started`
```json

View file

@ -401,7 +401,7 @@ V2 keeps the current durable family surface broadly intact.
- `sandbox.*`
- `setup.*`
- `cli.ensure.*`
- `cli.ensure.*` (legacy only)
- `command.*`
- `agent.cli.*`
- `devcontainer.*`

View file

@ -41,6 +41,8 @@ The CLI is selected automatically based on the node's provider:
| OpenAI | `codex` |
| Gemini | `gemini` |
Fabro does not install these CLIs at runtime. Install the selected CLI in the sandbox image or run setup steps before the workflow reaches a `backend="cli"` node.
Set the CLI backend on a node with `backend="cli"` or via a [model stylesheet](/workflows/stylesheets):
```dot
@ -56,26 +58,14 @@ implement [label="Implement", backend="cli"]
Fabro can also run Agent Client Protocol (ACP) stdio agents with `backend="acp"`. ACP agents run inside the active Fabro sandbox, so local and Docker runs keep the same workspace isolation, secret forwarding, cancellation, and file-change tracking behavior as other agent stages.
Set ACP on a node with `backend="acp"`:
```dot
implement [label="Implement", backend="acp", provider="openai"]
```
Fabro chooses a default ACP command from the provider:
| Provider | Default ACP command |
|---|---|
| Anthropic | `npx -y @zed-industries/claude-code-acp@latest` |
| OpenAI and OpenAI-compatible providers | `npx -y @zed-industries/codex-acp@latest` |
| Gemini | `npx -y -- @google/gemini-cli@latest --experimental-acp` |
Use `acp_command="..."` to point a node at a specific ACP stdio command:
Set ACP on a node with `backend="acp"` and an explicit `acp_command`:
```dot
implement [label="Implement", backend="acp", acp_command="python3 tools/fake_acp_agent.py"]
```
Fabro does not install ACP agents, Node.js, npm, or `npx` at runtime. The command must already be available in the sandbox image, repository, or setup steps. You can use `npx ...@latest` as an explicit `acp_command` if that is the behavior you want, but Fabro will treat it like any other user-supplied command.
ACP v1 does not have a portable model-selection request. Fabro records the selected provider and model in events and run projections, but model-specific ACP behavior must be encoded in the chosen command for now. ACP is supported with local and Docker sandboxes; Daytona does not expose bidirectional stdio yet, so ACP nodes fail there with an explicit unsupported-provider error.
### Comparison

View file

@ -207,7 +207,7 @@ Start nodes can also be identified by ID (`start` or `Start`). Exit nodes can be
| `provider` | String | Explicit provider name (overrides stylesheet). Auto-inferred from the model catalog when omitted. |
| `project_memory` | Boolean | When `true` (default), prompt nodes discover and include project docs (`AGENTS.md`, `CLAUDE.md`, etc.) as a system prompt. Set to `false` to disable. |
| `backend` | String | Agent execution backend: `api` (default), `cli`, or `acp`. `api` runs Fabro's tool loop through provider APIs; `cli` delegates to the legacy provider CLI; `acp` runs an Agent Client Protocol stdio agent inside the active sandbox. See [Agents — Backends](/core-concepts/agents#backends). |
| `acp_command` | String | Optional ACP stdio command override for nodes with `backend="acp"`. Defaults are selected from `provider`; model selection is recorded in Fabro but not sent through stable ACP v1. |
| `acp_command` | String | Required for nodes with `backend="acp"`. The value must be a stdio ACP command available in the sandbox. Fabro records model selection but does not send it through stable ACP v1. |
### Command nodes

View file

@ -51,6 +51,10 @@ impl std::fmt::Display for AcpCommand {
pub enum AcpCommandError {
#[error("acp_command must not be empty")]
EmptyOverride,
#[error(
"acp_command is required for backend=\"acp\" because Fabro does not install ACP agents"
)]
MissingOverride,
#[error("only stdio ACP commands are supported")]
UnsupportedTransport,
#[error("failed to parse acp_command")]
@ -63,41 +67,8 @@ impl From<agent_client_protocol::Error> for AcpCommandError {
}
}
#[must_use]
pub fn default_acp_command(provider: Provider) -> AcpCommand {
match provider {
Provider::Anthropic => {
command_from_parts("npx -y @zed-industries/claude-code-acp@latest", "npx", [
"-y",
"@zed-industries/claude-code-acp@latest",
])
}
Provider::Gemini => command_from_parts(
"npx -y -- @google/gemini-cli@latest --experimental-acp",
"npx",
[
"-y",
"--",
"@google/gemini-cli@latest",
"--experimental-acp",
],
),
Provider::OpenAi
| Provider::Kimi
| Provider::Zai
| Provider::Minimax
| Provider::Inception
| Provider::OpenAiCompatible => {
command_from_parts("npx -y @zed-industries/codex-acp@latest", "npx", [
"-y",
"@zed-industries/codex-acp@latest",
])
}
}
}
pub fn resolve_acp_command(
provider: Provider,
_provider: Provider,
override_command: Option<&str>,
) -> Result<AcpCommand, AcpCommandError> {
if let Some(raw) = override_command {
@ -108,7 +79,7 @@ pub fn resolve_acp_command(
return parse_acp_command(trimmed);
}
Ok(default_acp_command(provider))
Err(AcpCommandError::MissingOverride)
}
fn parse_acp_command(raw: &str) -> Result<AcpCommand, AcpCommandError> {
@ -159,19 +130,6 @@ fn reject_non_stdio_json_transport(raw: &str) -> Result<(), AcpCommandError> {
}
}
fn command_from_parts<const N: usize>(
display: impl Into<String>,
program: impl Into<PathBuf>,
args: [&str; N],
) -> AcpCommand {
AcpCommand {
display: display.into(),
program: program.into(),
args: args.into_iter().map(str::to_string).collect(),
env: HashMap::new(),
}
}
#[cfg(test)]
mod tests {
use std::path::Path;
@ -181,35 +139,11 @@ mod tests {
use super::*;
#[test]
fn default_command_for_anthropic_uses_zed_claude_acp() {
assert_eq!(
default_acp_command(Provider::Anthropic).to_string(),
"npx -y @zed-industries/claude-code-acp@latest"
);
}
#[test]
fn default_command_for_openai_compatible_family_uses_zed_codex_acp() {
for provider in [
Provider::OpenAi,
Provider::Kimi,
Provider::Zai,
Provider::Minimax,
Provider::Inception,
Provider::OpenAiCompatible,
] {
assert_eq!(
default_acp_command(provider).to_string(),
"npx -y @zed-industries/codex-acp@latest"
);
}
}
#[test]
fn default_command_for_gemini_uses_experimental_acp() {
assert_eq!(
default_acp_command(Provider::Gemini).to_string(),
"npx -y -- @google/gemini-cli@latest --experimental-acp"
fn missing_acp_command_is_rejected() {
let err = resolve_acp_command(Provider::OpenAi, None).unwrap_err();
assert!(
err.to_string()
.contains("acp_command is required for backend=\"acp\"")
);
}

View file

@ -7,6 +7,6 @@ pub mod test_support;
mod transport;
pub use command::{AcpCommand, AcpCommandError, default_acp_command, resolve_acp_command};
pub use command::{AcpCommand, AcpCommandError, resolve_acp_command};
pub use error::AcpError;
pub use session::{AcpRunRequest, AcpRunResult, run_acp_turn};

View file

@ -472,6 +472,7 @@ mod tests {
use fabro_agent::{AgentEvent, SandboxEvent};
use fabro_llm::types::TokenCounts;
use fabro_model::{ModelRef, Provider};
use fabro_types::run_event::CliEnsureCompletedProps;
use fabro_types::{
MetadataSnapshotFailureKind, MetadataSnapshotPhase, ParallelBranchId, SandboxProvider,
StageId, fixtures,
@ -527,6 +528,24 @@ mod tests {
ui.handle_event(&stored);
}
fn emit_body(ui: &mut ProgressUI, body: fabro_types::EventBody) {
ui.handle_event(&RunEvent {
id: "evt_legacy".to_string(),
ts: Utc::now(),
run_id: fixtures::RUN_1,
node_id: None,
node_label: None,
stage_id: None,
parallel_group_id: None,
parallel_branch_id: None,
session_id: None,
parent_session_id: None,
tool_call_id: None,
actor: None,
body,
});
}
fn agent_event(stage: &str, event: AgentEvent) -> Event {
Event::Agent {
stage: stage.into(),
@ -860,13 +879,16 @@ mod tests {
});
emit(&mut ui, Event::SetupStarted { command_count: 2 });
emit(&mut ui, Event::SetupCompleted { duration_ms: 8200 });
emit(&mut ui, Event::CliEnsureCompleted {
cli_name: "gh".into(),
provider: "github".into(),
already_installed: false,
node_installed: false,
duration_ms: 600,
});
emit_body(
&mut ui,
fabro_types::EventBody::CliEnsureCompleted(CliEnsureCompletedProps {
cli_name: "gh".into(),
provider: "github".into(),
already_installed: false,
node_installed: false,
duration_ms: 600,
}),
);
emit(&mut ui, Event::DevcontainerResolved {
dockerfile_lines: 24,
environment_count: 3,

View file

@ -1,4 +1,4 @@
use fabro_graphviz::graph::{AttrValue, Graph};
use fabro_graphviz::graph::{AttrValue, Graph, Node};
use fabro_types::LlmBackend;
use crate::{Diagnostic, LintRule, Severity};
@ -18,18 +18,36 @@ impl LintRule for Rule {
let mut diagnostics = Vec::new();
for node in graph.nodes.values() {
if let Some(backend) = node.attrs.get("backend").and_then(AttrValue::as_str) {
if backend.parse::<LlmBackend>().is_err() {
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!(
"unsupported LLM backend \"{backend}\"; expected one of: {}",
LlmBackend::EXPECTED
),
node_id: Some(node.id.clone()),
edge: None,
fix: Some(format!("Use one of: {}", LlmBackend::EXPECTED)),
});
match backend.parse::<LlmBackend>() {
Err(_) => {
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!(
"unsupported LLM backend \"{backend}\"; expected one of: {}",
LlmBackend::EXPECTED
),
node_id: Some(node.id.clone()),
edge: None,
fix: Some(format!("Use one of: {}", LlmBackend::EXPECTED)),
});
}
Ok(LlmBackend::Acp) if acp_command_missing(node) => {
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: "backend=\"acp\" requires acp_command because Fabro does \
not install ACP agents"
.to_string(),
node_id: Some(node.id.clone()),
edge: None,
fix: Some(
"Set acp_command to a stdio ACP command available in the sandbox"
.to_string(),
),
});
}
Ok(_) => {}
}
}
}
@ -37,6 +55,13 @@ impl LintRule for Rule {
}
}
fn acp_command_missing(node: &Node) -> bool {
match node.attrs.get("acp_command").and_then(AttrValue::as_str) {
Some(command) => command.trim().is_empty(),
None => true,
}
}
#[cfg(test)]
mod tests {
use fabro_graphviz::graph::{AttrValue, Node};
@ -46,8 +71,8 @@ mod tests {
use crate::{LintRule, Severity};
#[test]
fn backend_valid_accepts_absent_api_cli_and_acp() {
for backend in [None, Some("api"), Some("cli"), Some("acp")] {
fn backend_valid_accepts_absent_api_and_cli() {
for backend in [None, Some("api"), Some("cli")] {
let mut graph = minimal_graph();
let mut node = Node::new("work");
if let Some(backend) = backend {
@ -81,4 +106,35 @@ mod tests {
.contains("unsupported LLM backend \"codex\"; expected one of: api, cli, acp")
);
}
#[test]
fn backend_valid_requires_acp_command_for_acp_backend() {
let mut graph = minimal_graph();
let mut node = Node::new("work");
node.attrs
.insert("backend".to_string(), AttrValue::String("acp".to_string()));
graph.nodes.insert("work".to_string(), node);
let diagnostics = Rule.apply(&graph);
assert_eq!(diagnostics.len(), 1);
assert_eq!(diagnostics[0].severity, Severity::Error);
assert!(diagnostics[0].message.contains(
"backend=\"acp\" requires acp_command because Fabro does not install ACP agents"
));
}
#[test]
fn backend_valid_accepts_acp_backend_with_acp_command() {
let mut graph = minimal_graph();
let mut node = Node::new("work");
node.attrs
.insert("backend".to_string(), AttrValue::String("acp".to_string()));
node.attrs.insert(
"acp_command".to_string(),
AttrValue::String("agent-acp".to_string()),
);
graph.nodes.insert("work".to_string(), node);
assert!(Rule.apply(&graph).is_empty());
}
}

View file

@ -986,38 +986,6 @@ fn event_body_from_event(event: &Event) -> EventBody {
to_model: to_model.clone(),
error: error.clone(),
}),
Event::CliEnsureStarted { cli_name, provider } => {
EventBody::CliEnsureStarted(fabro_types::CliEnsureStartedProps {
cli_name: cli_name.clone(),
provider: provider.clone(),
})
}
Event::CliEnsureCompleted {
cli_name,
provider,
already_installed,
node_installed,
duration_ms,
} => EventBody::CliEnsureCompleted(fabro_types::CliEnsureCompletedProps {
cli_name: cli_name.clone(),
provider: provider.clone(),
already_installed: *already_installed,
node_installed: *node_installed,
duration_ms: *duration_ms,
}),
Event::CliEnsureFailed {
cli_name,
provider,
error,
duration_ms,
exec_output_tail,
} => EventBody::CliEnsureFailed(fabro_types::CliEnsureFailedProps {
cli_name: cli_name.clone(),
provider: provider.clone(),
error: error.clone(),
duration_ms: *duration_ms,
exec_output_tail: exec_output_tail.clone(),
}),
Event::CommandStarted {
script,
command,

View file

@ -495,25 +495,6 @@ pub enum Event {
to_model: String,
error: String,
},
CliEnsureStarted {
cli_name: String,
provider: String,
},
CliEnsureCompleted {
cli_name: String,
provider: String,
already_installed: bool,
node_installed: bool,
duration_ms: u64,
},
CliEnsureFailed {
cli_name: String,
provider: String,
error: String,
duration_ms: u64,
#[serde(default, skip_serializing_if = "Option::is_none")]
exec_output_tail: Option<fabro_types::ExecOutputTail>,
},
CommandStarted {
node_id: String,
script: String,
@ -1268,48 +1249,6 @@ impl Event {
"LLM provider failover"
);
}
Self::CliEnsureStarted {
cli_name, provider, ..
} => {
debug!(cli_name, provider, "CLI ensure started");
}
Self::CliEnsureCompleted {
cli_name,
provider,
already_installed,
node_installed,
duration_ms,
} => {
info!(
cli_name,
provider,
already_installed,
node_installed,
duration_ms,
"CLI ensure completed"
);
}
Self::CliEnsureFailed {
cli_name,
provider,
error,
duration_ms,
exec_output_tail,
} => {
let tail = fabro_types::ExecOutputTail::trace_summary(exec_output_tail.as_ref());
error!(
cli_name,
provider,
error,
duration_ms,
exec_output_tail_present = tail.present,
exec_stdout_tail_bytes = tail.stdout_bytes,
exec_stderr_tail_bytes = tail.stderr_bytes,
exec_stdout_truncated = tail.stdout_truncated,
exec_stderr_truncated = tail.stderr_truncated,
"CLI ensure failed"
);
}
Self::CommandStarted {
node_id,
language,

View file

@ -121,9 +121,6 @@ pub fn event_name(event: &Event) -> &'static str {
Event::ArtifactCaptured { .. } => "artifact.captured",
Event::SshAccessReady { .. } => "ssh.ready",
Event::Failover { .. } => "agent.failover",
Event::CliEnsureStarted { .. } => "cli.ensure.started",
Event::CliEnsureCompleted { .. } => "cli.ensure.completed",
Event::CliEnsureFailed { .. } => "cli.ensure.failed",
Event::CommandStarted { .. } => "command.started",
Event::CommandCompleted { .. } => "command.completed",
Event::AgentCliStarted { .. } => "agent.cli.started",

View file

@ -4,7 +4,7 @@ use std::collections::HashMap;
use std::sync::Arc;
use async_trait::async_trait;
use fabro_acp::{AcpError, AcpRunRequest, default_acp_command, resolve_acp_command};
use fabro_acp::{AcpCommandError, AcpError, AcpRunRequest, resolve_acp_command};
use fabro_agent::{Sandbox, StaticEnvProvider, ToolEnvProvider};
use fabro_auth::CredentialResolver;
use fabro_graphviz::graph::Node;
@ -13,9 +13,9 @@ use fabro_util::time::elapsed_ms;
use tokio_util::sync::CancellationToken;
use super::super::agent::{CodergenBackend, CodergenResult};
use super::changed_files;
use super::cli::AgentCli;
use super::launch_env::{AgentLaunchEnvRequest, resolve_agent_launch_env};
use super::{changed_files, node_runtime};
use crate::context::Context;
use crate::error::Error;
use crate::event::{Emitter, Event, StageScope};
@ -83,19 +83,10 @@ impl AgentAcpBackend {
.provider()
.and_then(|value| value.parse::<Provider>().ok())
.unwrap_or(self.provider);
let explicit_command = node.acp_command();
let command = resolve_acp_command(provider, explicit_command)
.map_err(|err| Error::handler_with_source("Failed to resolve ACP command", &err))?;
let command = resolve_acp_command(provider, node.acp_command())
.map_err(acp_command_error_to_workflow)?;
let node_runtime_env = if explicit_command.is_none()
&& command.program() == default_acp_command(provider).program()
{
Some(node_runtime::ensure_node_runtime(sandbox, &cancel_token).await?)
} else {
None
};
let mut launch_env = resolve_agent_launch_env(AgentLaunchEnvRequest {
let launch_env = resolve_agent_launch_env(AgentLaunchEnvRequest {
provider,
cli: AgentCli::for_provider(provider),
resolver: self.resolver.as_ref(),
@ -107,9 +98,6 @@ impl AgentAcpBackend {
cancel_token: &cancel_token,
})
.await?;
if let Some(runtime_env) = node_runtime_env {
node_runtime::apply_node_runtime_env(&mut launch_env, runtime_env);
}
let on_activity = {
let emitter = Arc::clone(emitter);
Arc::new(move || emitter.touch()) as Arc<dyn Fn() + Send + Sync>
@ -260,6 +248,21 @@ fn stop_reason_to_string(stop_reason: &(impl serde::Serialize + std::fmt::Debug)
.unwrap_or_else(|| format!("{stop_reason:?}"))
}
fn acp_command_error_to_workflow(error: AcpCommandError) -> Error {
match error {
AcpCommandError::EmptyOverride => Error::handler("acp_command must not be empty"),
AcpCommandError::MissingOverride => Error::handler(
"acp_command is required for backend=\"acp\" because Fabro does not install ACP agents",
),
AcpCommandError::UnsupportedTransport => {
Error::handler("only stdio ACP commands are supported")
}
AcpCommandError::Parse(source) => {
Error::handler_with_source("Failed to resolve ACP command", &source)
}
}
}
fn acp_error_to_workflow(error: AcpError) -> Error {
match error {
AcpError::Cancelled => Error::Cancelled,
@ -515,10 +518,8 @@ mod tests {
}
#[tokio::test]
async fn acp_default_command_launch_env_includes_bootstrapped_node_path() {
let mut sandbox = MockSandbox::linux();
sandbox.exec_result.stdout =
"__FABRO_NODE_RUNTIME_PATH=/home/test/.local/bin:/usr/local/bin:/usr/bin\n".to_string();
async fn acp_backend_requires_explicit_acp_command() {
let sandbox = MockSandbox::linux();
let sandbox = Arc::new(sandbox);
let sandbox_dyn: Arc<dyn Sandbox> = sandbox.clone();
@ -543,20 +544,20 @@ mod tests {
CancellationToken::new(),
)
.await;
let Err(err) = result else {
panic!("ACP without acp_command should fail");
};
assert!(
result.is_err(),
"mock stdio transport should not complete ACP"
err.to_string()
.contains("acp_command is required for backend=\"acp\"")
);
let env = sandbox
.captured_env_vars
.lock()
.expect("captured env lock poisoned")
.clone()
.expect("ACP launch env should be captured");
assert_eq!(
env.get("PATH").map(String::as_str),
Some("/home/test/.local/bin:/usr/local/bin:/usr/bin")
assert!(
sandbox
.captured_env_vars
.lock()
.expect("captured env lock poisoned")
.is_none(),
"ACP process should not launch when acp_command is missing"
);
}

View file

@ -40,8 +40,8 @@ fn cli_failure_detail(stdout: &str, stderr: &str, command: &str) -> String {
use super::super::agent::{CodergenBackend, CodergenResult};
use super::acp::AgentAcpBackend;
use super::changed_files;
use super::launch_env::{AgentLaunchEnvRequest, resolve_agent_launch_env};
use super::{changed_files, node_runtime};
use crate::context::Context;
use crate::error::Error;
use crate::event::{Emitter, Event, StageScope};
@ -76,41 +76,20 @@ impl AgentCli {
Self::Gemini => "gemini",
}
}
pub fn npm_package(self) -> &'static str {
match self {
Self::Claude => "@anthropic-ai/claude-code",
Self::Codex => "@openai/codex",
Self::Gemini => "@anthropic-ai/gemini-cli",
}
}
}
/// Ensure the CLI tool for the given provider is installed in the sandbox.
///
/// Checks if the CLI binary exists; if not, installs Node.js (if missing) and
/// the CLI via npm. Emits `CliEnsure*` events for observability.
async fn ensure_cli(
/// Verify the provider CLI exists in the sandbox. Fabro does not install agent
/// CLIs at runtime; sandbox images or setup steps own tool installation.
async fn verify_cli_available(
cli: AgentCli,
provider: Provider,
sandbox: &Arc<dyn Sandbox>,
emitter: &Arc<Emitter>,
cancel_token: &CancellationToken,
) -> Result<(), Error> {
let start = std::time::Instant::now();
let cli_name = cli.name();
let provider_str = <&'static str>::from(provider);
emitter.emit(&Event::CliEnsureStarted {
cli_name: cli_name.to_string(),
provider: provider_str.to_string(),
});
// Check if the CLI is already installed (include ~/.local/bin for npm-installed
// CLIs)
let version_check = sandbox
let availability_check = sandbox
.exec_command(
&format!("PATH=\"$HOME/.local/bin:$PATH\" {cli_name} --version"),
&format!("PATH=\"$HOME/.local/bin:$PATH\" command -v {cli_name}"),
30_000,
None,
None,
@ -118,66 +97,17 @@ async fn ensure_cli(
)
.await
.map_err(|e| {
Error::handler_with_source(format!("Failed to check {cli_name} version"), &e)
Error::handler_with_source(format!("Failed to check {cli_name} availability"), &e)
})?;
if version_check.is_success() {
let duration_ms = elapsed_ms(start);
emitter.emit(&Event::CliEnsureCompleted {
cli_name: cli_name.to_string(),
provider: provider_str.to_string(),
already_installed: true,
node_installed: false,
duration_ms,
});
if availability_check.is_success() {
return Ok(());
}
// Install Node.js (if needed) and the CLI in a single shell so PATH persists
let install_cmd = format!(
"{} && npm install -g {}",
node_runtime::ensure_node_runtime_shell(),
cli.npm_package()
);
let install_result = sandbox
.exec_command(
&install_cmd,
180_000,
None,
None,
Some(cancel_token.child_token()),
)
.await
.map_err(|e| Error::handler_with_source(format!("Failed to install {cli_name}"), &e))?;
let node_installed = true;
if !install_result.is_success() {
let duration_ms = elapsed_ms(start);
let exec_output_tail = install_result.default_redacted_output_tail();
let error_msg = format!(
"{cli_name} install exited with code {}",
install_result.display_exit_code()
);
emitter.emit(&Event::CliEnsureFailed {
cli_name: cli_name.to_string(),
provider: provider_str.to_string(),
error: error_msg.clone(),
duration_ms,
exec_output_tail,
});
return Err(Error::handler(error_msg));
}
let duration_ms = elapsed_ms(start);
emitter.emit(&Event::CliEnsureCompleted {
cli_name: cli_name.to_string(),
provider: provider_str.to_string(),
already_installed: false,
node_installed,
duration_ms,
});
Ok(())
Err(Error::handler(format!(
"CLI backend requires '{cli_name}' to be installed in the sandbox PATH. Install it in the \
sandbox image or setup steps before running backend=\"cli\"."
)))
}
/// Models that are only available through CLI tools (not via API).
@ -492,9 +422,8 @@ impl CodergenBackend for AgentCliBackend {
.and_then(|s| s.parse::<Provider>().ok())
.unwrap_or(self.provider);
// Ensure the CLI tool is installed in the sandbox
let cli = AgentCli::for_provider(provider);
ensure_cli(cli, provider, sandbox, emitter, &cancel_token).await?;
verify_cli_available(cli, sandbox, &cancel_token).await?;
let command = cli_command_for_provider(provider, model, &prompt_path);
let stage_scope = StageScope::for_handler(context, &node.id);
@ -928,14 +857,7 @@ mod tests {
assert_eq!(AgentCli::Gemini.name(), "gemini");
}
#[test]
fn agent_cli_npm_package() {
assert_eq!(AgentCli::Claude.npm_package(), "@anthropic-ai/claude-code");
assert_eq!(AgentCli::Codex.npm_package(), "@openai/codex");
assert_eq!(AgentCli::Gemini.npm_package(), "@anthropic-ai/gemini-cli");
}
// -- ensure_cli --
// -- verify_cli_available --
use std::collections::VecDeque;
use std::sync::Mutex;
@ -1072,113 +994,47 @@ mod tests {
}
#[tokio::test]
async fn ensure_cli_skips_install_when_present() {
async fn verify_cli_available_succeeds_when_present() {
let commands = Arc::new(Mutex::new(Vec::new()));
let sandbox: Arc<dyn Sandbox> = Arc::new(CliMockSandbox::new(
vec![ok_result()],
Arc::clone(&commands),
));
let emitter = Arc::new(Emitter::default());
let result = ensure_cli(
AgentCli::Claude,
Provider::Anthropic,
&sandbox,
&emitter,
&CancellationToken::new(),
)
.await;
let result =
verify_cli_available(AgentCli::Claude, &sandbox, &CancellationToken::new()).await;
assert!(result.is_ok());
let commands = commands.lock().unwrap();
assert_eq!(commands.len(), 1);
assert!(commands[0].contains("claude --version"));
assert!(commands[0].contains("command -v claude"));
}
#[tokio::test]
async fn ensure_cli_installs_when_missing() {
async fn verify_cli_available_fails_when_missing_without_installing() {
let commands = Arc::new(Mutex::new(Vec::new()));
// version check fails, combined install succeeds
let sandbox: Arc<dyn Sandbox> = Arc::new(CliMockSandbox::new(
vec![
fail_result(127), // claude --version
ok_result(), // combined node + npm install
],
vec![fail_result(127)],
Arc::clone(&commands),
));
let emitter = Arc::new(Emitter::default());
let result = ensure_cli(
AgentCli::Claude,
Provider::Anthropic,
&sandbox,
&emitter,
&CancellationToken::new(),
)
.await;
assert!(result.is_ok());
let result =
verify_cli_available(AgentCli::Claude, &sandbox, &CancellationToken::new()).await;
assert!(result.is_err());
assert!(
result
.unwrap_err()
.to_string()
.contains("CLI backend requires 'claude' to be installed")
);
let commands = commands.lock().unwrap();
assert_eq!(commands.len(), 2);
assert!(commands[1].contains("npm install -g @anthropic-ai/claude-code"));
}
#[tokio::test]
async fn ensure_cli_fails_on_install_failure() {
let commands = Arc::new(Mutex::new(Vec::new()));
let sandbox: Arc<dyn Sandbox> = Arc::new(CliMockSandbox::new(
vec![
fail_result(127), // claude --version
fail_result_with_output(1, "install stdout detail", "install stderr detail"),
],
Arc::clone(&commands),
));
let emitter = Arc::new(Emitter::default());
let events = Arc::new(Mutex::new(Vec::<fabro_types::RunEvent>::new()));
emitter.on_event({
let events = Arc::clone(&events);
move |event| events.lock().unwrap().push(event.clone())
});
let result = ensure_cli(
AgentCli::Claude,
Provider::Anthropic,
&sandbox,
&emitter,
&CancellationToken::new(),
)
.await;
assert!(result.is_err());
let error = result.unwrap_err().to_string();
assert!(error.contains("install exited with code 1"));
assert!(!error.contains("install stdout detail"));
assert!(!error.contains("install stderr detail"));
let events = events.lock().unwrap();
let failed = events
.iter()
.find(|event| event.event_name() == "cli.ensure.failed")
.expect("cli ensure failed event");
match &failed.body {
fabro_types::EventBody::CliEnsureFailed(props) => {
assert_eq!(props.error, "claude install exited with code 1");
assert_eq!(
props
.exec_output_tail
.as_ref()
.and_then(|tail| tail.stdout.as_deref()),
Some("install stdout detail")
);
assert_eq!(
props
.exec_output_tail
.as_ref()
.and_then(|tail| tail.stderr.as_deref()),
Some("install stderr detail")
);
}
other => panic!("expected cli ensure failed body, got {other:?}"),
}
assert_eq!(commands.len(), 1);
assert!(commands[0].contains("command -v claude"));
assert!(
!commands
.iter()
.any(|command| command.contains("npm install"))
);
}
// -- Cycle 1: cli_command_for_provider --
@ -1637,8 +1493,8 @@ for line in sys.stdin:
_cancel_token: Option<CancellationToken>,
) -> fabro_sandbox::Result<ExecResult> {
self.commands.lock().unwrap().push(command.to_string());
// Default: success for git/version/cat/rm/ls.
if command.contains("--version") {
// Default: success for CLI availability checks and lightweight setup.
if command.contains("command -v ") {
return Ok(ok_result());
}
Ok(ExecResult {

View file

@ -4,7 +4,6 @@ pub mod api;
pub mod changed_files;
pub mod cli;
pub mod launch_env;
pub mod node_runtime;
pub mod preamble;
pub use acp::AgentAcpBackend;

View file

@ -1,124 +0,0 @@
use std::sync::Arc;
use fabro_agent::Sandbox;
use fabro_static::EnvVars;
use tokio_util::sync::CancellationToken;
use crate::error::Error;
const NODE_RUNTIME_PATH_MARKER: &str = "__FABRO_NODE_RUNTIME_PATH=";
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct NodeRuntimeEnv {
pub path: String,
}
pub fn ensure_node_runtime_shell() -> String {
r#"export PATH="$HOME/.local/bin:$PATH" && \
if node --version >/dev/null 2>&1 && npm --version >/dev/null 2>&1 && npx --version >/dev/null 2>&1; then \
true; \
else \
os="$(uname -s)"; \
if [ "$os" != "Linux" ]; then \
echo "Node.js, npm, and npx are required for default ACP/CLI commands on $os" >&2; \
exit 127; \
fi; \
arch="$(uname -m)"; \
case "$arch" in \
x86_64|amd64) node_arch="x64" ;; \
aarch64|arm64) node_arch="arm64" ;; \
*) echo "Unsupported Linux architecture for Node.js install: $arch" >&2; exit 127 ;; \
esac; \
mkdir -p "$HOME/.local" && \
curl -fsSL "https://nodejs.org/dist/v22.14.0/node-v22.14.0-linux-${node_arch}.tar.gz" | tar -xz --strip-components=1 -C "$HOME/.local"; \
fi"#
.to_string()
}
pub async fn ensure_node_runtime(
sandbox: &Arc<dyn Sandbox>,
cancel_token: &CancellationToken,
) -> Result<NodeRuntimeEnv, Error> {
let command = format!(
"{} && printf '\\n{}%s\\n' \"$PATH\"",
ensure_node_runtime_shell(),
NODE_RUNTIME_PATH_MARKER
);
let result = sandbox
.exec_command(
&command,
180_000,
None,
None,
Some(cancel_token.child_token()),
)
.await
.map_err(|err| Error::handler_with_source("Failed to ensure Node runtime", &err))?;
if result.is_success() {
let path = parse_node_runtime_path(&result.stdout).ok_or_else(|| {
Error::handler("Node runtime install did not report the sandbox PATH".to_string())
})?;
Ok(NodeRuntimeEnv { path })
} else {
Err(Error::handler(format!(
"Node runtime install exited with code {}",
result.display_exit_code()
)))
}
}
pub fn apply_node_runtime_env(
launch_env: &mut std::collections::HashMap<String, String>,
runtime_env: NodeRuntimeEnv,
) {
match launch_env.get_mut(EnvVars::PATH) {
Some(existing_path) if !existing_path.is_empty() => {
*existing_path = format!("{}:{existing_path}", runtime_env.path);
}
_ => {
launch_env.insert(EnvVars::PATH.to_string(), runtime_env.path);
}
}
}
fn parse_node_runtime_path(stdout: &str) -> Option<String> {
stdout
.lines()
.rev()
.find_map(|line| line.strip_prefix(NODE_RUNTIME_PATH_MARKER))
.filter(|path| !path.is_empty())
.map(str::to_string)
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use super::{NodeRuntimeEnv, apply_node_runtime_env, parse_node_runtime_path};
#[test]
fn parse_node_runtime_path_uses_last_reported_marker() {
assert_eq!(
parse_node_runtime_path(
"download output\n__FABRO_NODE_RUNTIME_PATH=/old\n\
__FABRO_NODE_RUNTIME_PATH=/home/test/.local/bin:/usr/bin\n",
),
Some("/home/test/.local/bin:/usr/bin".to_string())
);
}
#[test]
fn apply_node_runtime_env_preserves_existing_path_tail() {
let mut env = HashMap::from([("PATH".to_string(), "/custom/bin".to_string())]);
apply_node_runtime_env(&mut env, NodeRuntimeEnv {
path: "/home/test/.local/bin:/usr/bin".to_string(),
});
assert_eq!(
env.get("PATH").map(String::as_str),
Some("/home/test/.local/bin:/usr/bin:/custom/bin")
);
}
}

View file

@ -9692,11 +9692,10 @@ impl fabro_agent::Sandbox for CliTestEnv {
});
}
// CLI version check during ensure_cli — return success so install path
// is skipped.
if command.contains("--version") {
// CLI availability check.
if command.contains("command -v ") {
return Ok(fabro_agent::ExecResult {
stdout: "1.0.0\n".into(),
stdout: "/usr/local/bin/agent-cli\n".into(),
stderr: String::new(),
exit_code: Some(0),
@ -10005,10 +10004,10 @@ async fn cli_backend_run_fails_on_nonzero_exit() {
duration_ms: 0,
});
}
// CLI version check during ensure_cli — pretend already installed.
if command.contains("--version") {
// CLI availability check.
if command.contains("command -v ") {
return Ok(fabro_agent::ExecResult {
stdout: "1.0.0\n".into(),
stdout: "/usr/local/bin/agent-cli\n".into(),
stderr: String::new(),
exit_code: Some(0),