fix: pass bootstrapped node path to ACP defaults

This commit is contained in:
Bryan Helmkamp 2026-05-11 14:31:33 -04:00
parent a25a0b46d4
commit 2f8958ccaa
No known key found for this signature in database
2 changed files with 129 additions and 7 deletions

View file

@ -86,15 +86,20 @@ impl AgentAcpBackend {
let command = resolve_acp_command(provider, explicit_command)
.map_err(|err| Error::handler_with_source("Failed to resolve ACP command", &err))?;
if explicit_command.is_none()
let node_runtime_env = if explicit_command.is_none()
&& command.program() == default_acp_command(provider).program()
{
node_runtime::ensure_node_runtime(sandbox, &cancel_token).await?;
}
Some(node_runtime::ensure_node_runtime(sandbox, &cancel_token).await?)
} else {
None
};
let launch_env = self
let mut launch_env = self
.launch_env(provider, emitter, sandbox, &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>
@ -342,6 +347,7 @@ mod tests {
use fabro_agent::{LocalSandbox, Sandbox, shell_quote};
use fabro_graphviz::graph::{AttrValue, Node};
use fabro_model::Provider;
use fabro_sandbox::test_support::MockSandbox;
use fabro_types::EventBody;
use tokio_util::sync::CancellationToken;
@ -570,6 +576,52 @@ mod tests {
assert!(!command.contains("secret-key"));
}
#[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();
let sandbox = Arc::new(sandbox);
let sandbox_dyn: Arc<dyn Sandbox> = sandbox.clone();
let mut node = Node::new("work");
node.attrs.insert(
"provider".to_string(),
AttrValue::String("openai".to_string()),
);
node.attrs
.insert("backend".to_string(), AttrValue::String("acp".to_string()));
let backend = AgentAcpBackend::new_from_env("fake-acp".to_string(), Provider::OpenAi);
let result = backend
.run(
&node,
"write hello",
&Context::new(),
None,
&Arc::new(Emitter::default()),
&sandbox_dyn,
None,
CancellationToken::new(),
)
.await;
assert!(
result.is_err(),
"mock stdio transport should not complete 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")
);
}
fn fake_agent_script() -> &'static str {
r#"
import json

View file

@ -1,10 +1,18 @@
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 {
"export PATH=\"$HOME/.local/bin:$PATH\" && \
(node --version >/dev/null 2>&1 && npm --version >/dev/null 2>&1 && npx --version >/dev/null 2>&1 || \
@ -15,8 +23,12 @@ pub fn ensure_node_runtime_shell() -> String {
pub async fn ensure_node_runtime(
sandbox: &Arc<dyn Sandbox>,
cancel_token: &CancellationToken,
) -> Result<(), Error> {
let command = ensure_node_runtime_shell();
) -> 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,
@ -29,7 +41,10 @@ pub async fn ensure_node_runtime(
.map_err(|err| Error::handler_with_source("Failed to ensure Node runtime", &err))?;
if result.is_success() {
Ok(())
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 {}",
@ -37,3 +52,58 @@ pub async fn ensure_node_runtime(
)))
}
}
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")
);
}
}