Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions crates/buzz-acp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,42 @@ All configuration is via environment variables (or CLI flags — every env var h
| `BUZZ_PRIVATE_KEY` | **yes** | — | Agent's Nostr private key (`nsec1...`). Used for relay auth and agent identity. |
| `BUZZ_RELAY_URL` | no | `ws://localhost:3000` | Relay WebSocket URL. |
| `BUZZ_ACP_AGENT_COMMAND` | no | `goose` | Agent binary to spawn. |
| `BUZZ_ACP_AGENT_ARGS` | no | `acp` | Agent arguments (comma-separated). |
| `BUZZ_ACP_AGENT_ARGS` | no | `acp` | Agent arguments (comma-separated). Legacy behavior — entries are not shell-split. |
| `BUZZ_ACP_AGENT_ARGS_JSON` | no | — | Structured agent arguments as a JSON array of strings. Takes precedence over `BUZZ_ACP_AGENT_ARGS` when set. Preserves arguments containing spaces, backslashes, and quotes without shell reinterpretation. Example: `["-m","my-model","--reasoning","low"]` |
| `BUZZ_ACP_MCP_COMMAND` | no | `""` (empty) | Path to an optional MCP server binary to provide to the agent subprocess. |
| `BUZZ_ACP_IDLE_TIMEOUT` | no | `620` | Idle timeout: max seconds of silence before cancelling a turn. Resets on any agent stdout activity. |
| `BUZZ_ACP_MAX_TURN_DURATION` | no | `7200` | Absolute wall-clock cap per turn (safety valve). |
| `BUZZ_API_TOKEN` | no | — | API token (required if relay enforces token auth). |

**Note:** `BUZZ_ACP_AGENT_ARGS` splits on commas. For args with values, use: `-c,key="value"`.
**Note:** `BUZZ_ACP_AGENT_ARGS` splits on commas. Each comma-delimited entry is
preserved as-is — no shell splitting is applied. For arguments containing
spaces, backslashes, or quotes, use `BUZZ_ACP_AGENT_ARGS_JSON` instead:

```bash
# Legacy comma-separated (no spaces in values):
export BUZZ_ACP_AGENT_ARGS='-c,key=value'

# Structured JSON (handles spaces, backslashes, quotes):
export BUZZ_ACP_AGENT_ARGS_JSON='["-m","my-model","--reasoning","low"]'

# Single argument containing a space:
export BUZZ_ACP_AGENT_ARGS_JSON='["--label=hello world"]'

# Windows path with backslashes:
export BUZZ_ACP_AGENT_ARGS_JSON='["--config=C:\\Program Files\\Agent\\config.toml"]'
```

When both `BUZZ_ACP_AGENT_ARGS_JSON` and a non-default `BUZZ_ACP_AGENT_ARGS` are
set, startup fails with an error explaining the conflict. Invalid JSON also
fails startup — the agent does not launch with ambiguous arguments.

**Producer behavior:** Desktop and the Kubernetes backend automatically select
the appropriate transport. When no argument contains a comma, they write
`BUZZ_ACP_AGENT_ARGS` (comma-joined). When any argument contains a comma, they
serialize the argument list as JSON into `BUZZ_ACP_AGENT_ARGS_JSON` and set
`BUZZ_ACP_AGENT_ARGS` to the default `"acp"`, which the harness treats as
"not configured." This is transparent to the user — no manual env var
configuration is needed.

**Legacy env vars:** `BUZZ_ACP_PRIVATE_KEY`, `BUZZ_ACP_API_TOKEN`, and `BUZZ_ACP_TURN_TIMEOUT` (replaced by `BUZZ_ACP_IDLE_TIMEOUT`) are still accepted as fallbacks.

Expand Down
194 changes: 191 additions & 3 deletions crates/buzz-acp/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@ pub struct CliArgs {
)]
pub agent_args: Vec<String>,

/// Structured agent arguments as a JSON array of strings.
/// When set, takes precedence over `BUZZ_ACP_AGENT_ARGS` (which must be
/// unset or left at its default). Parses strictly with no shell splitting,
/// preserving arguments containing spaces, backslashes, and quotes.
/// Example: `BUZZ_ACP_AGENT_ARGS_JSON='["-m","my-model","--reasoning","low"]'`
#[arg(long, env = "BUZZ_ACP_AGENT_ARGS_JSON")]
pub agent_args_json: Option<String>,

#[arg(long, env = "BUZZ_ACP_MCP_COMMAND", default_value = "")]
pub mcp_command: String,

Expand Down Expand Up @@ -871,8 +879,14 @@ pub fn codex_network_env(agent_command: &str, relay_url: &str) -> Option<(String
pub fn normalize_agent_args(command: &str, agent_args: Vec<String>) -> Vec<String> {
let normalized = agent_args
.into_iter()
.map(|arg| arg.trim().to_string())
.filter(|arg| !arg.is_empty())
.flat_map(|arg| {
let trimmed = arg.trim().to_string();
if trimmed.is_empty() {
Vec::new()
} else {
vec![trimmed]
}
})
.collect::<Vec<_>>();

let Some(default_args) = default_agent_args(command) else {
Expand All @@ -894,6 +908,59 @@ pub fn normalize_agent_args(command: &str, agent_args: Vec<String>) -> Vec<Strin
normalized
}

/// Resolve agent arguments from the legacy comma-delimited env var and the
/// structured JSON env var.
///
/// Selection is explicit:
/// - neither set → no configured args (default-value handling follows in
/// `normalize_agent_args`)
/// - JSON only → parse strictly as `Vec<String>`, no shell splitting
/// - legacy only → today's comma-delimited behavior, unchanged
/// - both set → startup error explaining the conflict
/// - invalid JSON → startup error; do not launch the agent
pub fn resolve_agent_args(
command: &str,
legacy_args: Vec<String>,
json_args: Option<&str>,
) -> Result<Vec<String>, ConfigError> {
match json_args {
Some(json) if !json.trim().is_empty() => {
if !(legacy_args.is_empty()
|| legacy_args.len() == 1 && legacy_args[0] == "acp")
{
return Err(ConfigError::ConfigFile(
"BUZZ_ACP_AGENT_ARGS and BUZZ_ACP_AGENT_ARGS_JSON are both set; \
use only one. JSON is the canonical structured transport; \
the legacy comma-delimited variable must be unset or removed."
.into(),
));
}
let parsed: Vec<String> = serde_json::from_str(json).map_err(|e| {
ConfigError::ConfigFile(format!(
"BUZZ_ACP_AGENT_ARGS_JSON is not a valid JSON array of strings: {e}"
))
})?;
// Apply the same default-value fallback as normalize_agent_args,
// but do NOT filter empty strings — JSON is a lossless transport
// where an explicit "" is a meaningful argv element.
let Some(default_args) = default_agent_args(command) else {
return Ok(parsed);
};
if parsed.is_empty() {
return Ok(default_args);
}
if parsed.len() == 1
&& parsed[0].eq_ignore_ascii_case("acp")
&& default_args.is_empty()
{
return Ok(default_args);
}
Ok(parsed)
}
_ => Ok(normalize_agent_args(command, legacy_args)),
}
}

/// Propagate legacy env-var aliases to their canonical names.
///
/// Must be called **before** the tokio runtime starts — i.e. from the sync
Expand Down Expand Up @@ -1002,7 +1069,11 @@ impl Config {
));
}

let agent_args = normalize_agent_args(&agent_command, args.agent_args);
let agent_args = resolve_agent_args(
&agent_command,
args.agent_args,
args.agent_args_json.as_deref(),
)?;

if let Some(ref channels) = args.channels {
for ch in channels {
Expand Down Expand Up @@ -1688,6 +1759,123 @@ mod tests {
);
}

// --- BUZZ_ACP_AGENT_ARGS_JSON structured transport (#6017) ---

#[test]
fn resolve_agent_args_json_parses_array() {
// JSON array is parsed strictly — no shell splitting.
let args = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some(r#"["-m","my-model","--reasoning","low"]"#),
)
.unwrap();
assert_eq!(args, vec!["-m", "my-model", "--reasoning", "low"]);
}

#[test]
fn resolve_agent_args_json_preserves_spaces_in_values() {
// An argument containing a space is one argv element.
let args = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some(r#"["--label=hello world"]"#),
)
.unwrap();
assert_eq!(args, vec!["--label=hello world"]);
}

#[test]
fn resolve_agent_args_json_preserves_windows_paths() {
// Backslashes in Windows paths are preserved, not interpreted as
// POSIX escape characters.
let args = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some(r#"["--config=C:\\Program Files\\Agent\\config.toml"]"#),
)
.unwrap();
assert_eq!(
args,
vec!["--config=C:\\Program Files\\Agent\\config.toml"]
);
}

#[test]
fn resolve_agent_args_json_preserves_empty_args_and_quotes() {
// Empty strings and values starting with -- are preserved exactly.
let args = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some(r#"["","--verbose","--label=has \"quotes\""]"#),
)
.unwrap();
assert_eq!(args, vec!["", "--verbose", "--label=has \"quotes\""]);
}

#[test]
fn resolve_agent_args_legacy_only_unchanged() {
// Legacy comma-delimited behavior is unchanged — no shell splitting.
let args = resolve_agent_args(
"custom-agent",
vec!["-c".into(), "model=gpt-5".into()],
None,
)
.unwrap();
assert_eq!(args, vec!["-c", "model=gpt-5"]);
}

#[test]
fn resolve_agent_args_both_set_is_error() {
// Both JSON and non-default legacy args is a startup error.
let result = resolve_agent_args(
"custom-agent",
vec!["-c".into(), "model=gpt-5".into()],
Some(r#"["--verbose"]"#),
);
assert!(result.is_err());
let msg = format!("{}", result.unwrap_err());
assert!(msg.contains("both set"), "error should mention conflict: {msg}");
}

#[test]
fn resolve_agent_args_json_with_default_legacy_is_ok() {
// JSON takes precedence when legacy is at its default ("acp").
let args = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some(r#"["--verbose"]"#),
)
.unwrap();
assert_eq!(args, vec!["--verbose"]);
}

#[test]
fn resolve_agent_args_invalid_json_is_error() {
let result = resolve_agent_args(
"custom-agent",
vec!["acp".into()],
Some("not valid json"),
);
assert!(result.is_err());
let msg = format!("{}", result.unwrap_err());
assert!(msg.contains("not a valid JSON array"), "error should mention invalid JSON: {msg}");
}

#[test]
fn resolve_agent_args_neither_set_uses_default() {
// No JSON, legacy at default for goose → ["acp"].
let args = resolve_agent_args("goose", vec!["acp".into()], None).unwrap();
assert_eq!(args, vec!["acp"]);
}

#[test]
fn resolve_agent_args_empty_json_uses_default() {
// Empty JSON array → provider default.
let args = resolve_agent_args("goose", vec!["acp".into()], Some("[]")).unwrap();
assert_eq!(args, vec!["acp"]);
}

#[test]
fn normalizes_buzz_agent_args_to_empty() {
assert_eq!(
Expand Down
38 changes: 32 additions & 6 deletions crates/buzz-backend-kubernetes/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use std::collections::BTreeMap;
/// before writing its own values, so a key the authoritative tier has no value
/// for is **removed** rather than left holding a lower-tier value. Plain
/// overwrite is not enough — most of these are written conditionally
/// (`BUZZ_ACP_AGENT_ARGS` only when `launch.args` is non-empty,
/// (`BUZZ_ACP_AGENT_ARGS` / `BUZZ_ACP_AGENT_ARGS_JSON` only when `launch.args`
/// is non-empty,
/// `BUZZ_ACP_RESPOND_TO` only when set), and without the clear, a lower tier
/// could supply the value for exactly the cases the authoritative tier stays
/// silent on. Clearing is also what the local spawn does: the desktop strips
Expand All @@ -28,6 +29,7 @@ const AUTHORITATIVE_KEYS: &[&str] = &[
"BUZZ_ACP_AGENT_OWNER",
"BUZZ_ACP_AGENT_COMMAND",
"BUZZ_ACP_AGENT_ARGS",
"BUZZ_ACP_AGENT_ARGS_JSON",
"BUZZ_ACP_RESPOND_TO",
"BUZZ_ACP_RESPOND_TO_ALLOWLIST",
"BUZZ_ACP_MCP_COMMAND",
Expand Down Expand Up @@ -258,11 +260,15 @@ pub fn build_env(
env.insert("BUZZ_ACP_AGENT_COMMAND".into(), command.to_string());
}
if !launch.args.is_empty() {
// Comma-joined because that is what the harness's CLI parser decodes,
// and what the desktop's local spawn does. An argument containing a
// comma is unrepresentable in both paths; inventing an escaping
// scheme here would produce args the harness cannot decode.
env.insert("BUZZ_ACP_AGENT_ARGS".into(), launch.args.join(","));
if launch.args.iter().any(|a| a.contains(',')) {
env.insert(
"BUZZ_ACP_AGENT_ARGS_JSON".into(),
serde_json::to_string(&launch.args).expect("Vec<String> is always serializable"),
);
env.insert("BUZZ_ACP_AGENT_ARGS".into(), "acp".into());
} else {
env.insert("BUZZ_ACP_AGENT_ARGS".into(), launch.args.join(","));
}
}
env.insert("BUZZ_ACP_MCP_COMMAND".into(), "buzz-dev-mcp".into());

Expand Down Expand Up @@ -589,11 +595,30 @@ mod tests {
}));
let env = build(&agent).unwrap();
assert_eq!(env["BUZZ_ACP_AGENT_ARGS"], "run,--no-session");
assert!(!env.contains_key("BUZZ_ACP_AGENT_ARGS_JSON"));

let agent = payload_json(serde_json::json!({
"launch": {"command": "goose", "args": [], "owner_pubkey": "b"}
}));
assert!(!build(&agent).unwrap().contains_key("BUZZ_ACP_AGENT_ARGS"));
assert!(!build(&agent).unwrap().contains_key("BUZZ_ACP_AGENT_ARGS_JSON"));
}

#[test]
fn args_containing_commas_use_json_transport() {
let agent = payload_json(serde_json::json!({
"launch": {
"command": "goose",
"args": ["--config=a,b", "--safe", "on"],
"owner_pubkey": "b"
}
}));
let env = build(&agent).unwrap();
assert_eq!(
env["BUZZ_ACP_AGENT_ARGS_JSON"],
r#"["--config=a,b","--safe","on"]"#
);
assert_eq!(env["BUZZ_ACP_AGENT_ARGS"], "acp");
}

/// The top-level `model`/`provider` fields are display inputs; their
Expand Down Expand Up @@ -678,6 +703,7 @@ mod tests {
// merely different.
for absent in [
"BUZZ_ACP_AGENT_ARGS",
"BUZZ_ACP_AGENT_ARGS_JSON",
"BUZZ_ACP_RESPOND_TO",
"BUZZ_ACP_RESPOND_TO_ALLOWLIST",
] {
Expand Down
5 changes: 3 additions & 2 deletions desktop/src-tauri/src/commands/agent_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,9 @@ fn run_buzz_acp_auth_command_with_paths<const N: usize>(
let mut command = Command::new(acp_path);
command
.args(args)
.env("BUZZ_ACP_AGENT_COMMAND", adapter_path.as_os_str())
.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(","))
.env("BUZZ_ACP_AGENT_COMMAND", adapter_path.as_os_str());
crate::managed_agents::set_agent_args_env(&mut command, &agent_args);
command
.stdout(Stdio::piped())
.stderr(Stdio::piped());
if let Some(workdir) = default_agent_workdir() {
Expand Down
4 changes: 2 additions & 2 deletions desktop/src-tauri/src/commands/agent_model_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ pub(super) async fn run_agent_models_command(
}
cmd.arg("models")
.arg("--json")
.env("BUZZ_ACP_AGENT_COMMAND", &agent_command)
.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(","));
.env("BUZZ_ACP_AGENT_COMMAND", &agent_command);
crate::managed_agents::set_agent_args_env(&mut cmd, &agent_args);
if let Some(meta) = known_acp_runtime(&agent_command) {
for (key, value) in meta.default_env {
if std::env::var(key).is_err() {
Expand Down
Loading