From fdbafa6977cfb9ee7b01be502d19a4dcb340a8e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Wed, 7 Oct 2026 15:45:14 +0200 Subject: [PATCH 1/4] fix(agent): exit non-zero when an agent command fails agent run, validate, explain and policy test printed their error and exited 0, so a profile that failed closed looked like a success to scripts and CI. Every agent command failure now exits 1; a child's own exit code is still passed through. --- src/exit.rs | 29 +++++++++++++++++++++++++++++ src/handlers/entry/root.rs | 16 ++++++++++++++-- tests/exit_status_cli.rs | 18 ++++++++++-------- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/exit.rs b/src/exit.rs index 4b811a98..f2f7d17b 100644 --- a/src/exit.rs +++ b/src/exit.rs @@ -51,6 +51,16 @@ impl Exit { } } + /// Gives a failure that would exit 0 exit code 1 instead. `agent` + /// commands use this so a profile that fails closed is not reported to + /// scripts as a success. Successes and explicit codes are left alone. + pub fn nonzero_on_failure(self) -> Self { + match self.failure { + Some(_) if self.code == 0 => Self { code: 1, ..self }, + _ => self, + } + } + /// Reports the outcome to telemetry, then exits the process. Telemetry /// never changes the exit code. pub fn terminate(self) -> ! { @@ -185,6 +195,25 @@ mod tests { assert_eq!(Exit::from(anyhow::Error::from(killed)).code, 1); } + #[test] + fn nonzero_on_failure_only_replaces_a_failure_that_exits_0() { + assert_eq!( + Exit::failed(ErrorKind::Validation).nonzero_on_failure(), + Exit { + code: 1, + failure: Some(ErrorKind::Validation) + } + ); + assert_eq!(Exit::ok().nonzero_on_failure(), Exit::ok()); + assert_eq!(Exit::code(3).nonzero_on_failure(), Exit::code(3)); + assert_eq!( + Exit::failed_with(ErrorKind::Other, 7) + .nonzero_on_failure() + .code, + 7 + ); + } + #[test] fn a_normal_return_after_ctrl_c_exits_130_and_nothing_else_changes() { assert_eq!(process_exit_code(0, true), 130); diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index d4474848..af450ade 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -491,6 +491,7 @@ pub async fn handle_cli(args: Cli) -> Exit { // Local commands such as `agent logs` do not need Stashbase authentication. let api_key = api_key.unwrap_or_default(); + let is_agent_command = matches!(args.entity_type, EntityType::Agent(_)); let result: anyhow::Result = match args.entity_type { EntityType::Whoami(WhoamiCommand { format }) => { @@ -1476,7 +1477,7 @@ pub async fn handle_cli(args: Cli) -> Exit { EntityType::Doctor(_) => unreachable!(), }; - match result { + let exit = match result { Ok(exit) => exit, Err(err) => { if REQUEST_ABORTED.load(Ordering::SeqCst) { @@ -1486,8 +1487,14 @@ pub async fn handle_cli(args: Cli) -> Exit { eprintln!("{:?}", err); Exit::from(err) } + }; + if is_agent_command { + exit.nonzero_on_failure() + } else { + exit } } else { + let is_agent_command = matches!(args.entity_type, EntityType::Agent(_)); if let EntityType::Config(cmd) = args.entity_type { if let ConfigSubcommand::Reset(_) = cmd.subcommand { return print_error(handle_config_commands(cmd, &Config::new(), args.raw)); @@ -1502,7 +1509,12 @@ pub async fn handle_cli(args: Cli) -> Exit { // An unreadable or malformed config file: printed, and the command // still exits 0. eprintln!("{:?}", err); - Exit::failed(ErrorKind::Validation) + let exit = Exit::failed(ErrorKind::Validation); + if is_agent_command { + exit.nonzero_on_failure() + } else { + exit + } } } diff --git a/tests/exit_status_cli.rs b/tests/exit_status_cli.rs index 53416a3d..acb5eea7 100644 --- a/tests/exit_status_cli.rs +++ b/tests/exit_status_cli.rs @@ -6,6 +6,8 @@ //! They pin today's behaviour, including paths that print an error and still //! exit 0. Changing one of those exit codes is a product decision, so a test //! here must not change as a side effect of refactoring how the CLI exits. +//! `agent` commands are the exception: every failure exits non-zero, so a +//! profile that fails closed is not reported to scripts as a success. //! //! The tests that apply the macOS sandbox or bind a loopback port skip //! themselves when the OS refuses (for example inside another sandbox). @@ -185,7 +187,7 @@ fn check(label: &str, out: &Output, code: i32, needle: &str, reported: Reported) // ---- config and profile resolution -------------------------------------- #[test] -fn a_malformed_config_toml_fails_every_command_that_reads_it_with_exit_0() { +fn a_malformed_config_toml_fails_every_command_that_reads_it() { let project = Project::new(); project.break_config(); @@ -219,7 +221,7 @@ fn a_malformed_config_toml_fails_every_command_that_reads_it_with_exit_0() { check( "agent init", &project.run(&["agent", "init", "p"]), - 0, + 1, message, Reported::Event("error", Some("validation")), ); @@ -434,7 +436,7 @@ fn generate_prints_its_error_and_exits_0() { // ---- agent init ----------------------------------------------------------- #[test] -fn agent_init_refuses_to_overwrite_and_exits_0() { +fn agent_init_refuses_to_overwrite_and_exits_1() { let project = Project::new(); check( "first", @@ -449,7 +451,7 @@ fn agent_init_refuses_to_overwrite_and_exits_0() { check( "second", &out, - 0, + 1, "Refusing to overwrite", Reported::Event("error", Some("other")), ); @@ -458,7 +460,7 @@ fn agent_init_refuses_to_overwrite_and_exits_0() { // ---- agent run: failures before launch ------------------------------------- #[test] -fn agent_run_failures_before_launch_print_the_error_and_exit_0() { +fn agent_run_failures_before_launch_print_the_error_and_exit_1() { let project = Project::new(); project.write_profile("p", "egress_hosts = [\"example.com\"]\n"); project.write_profile("broken", "workspace = 1\n"); @@ -524,7 +526,7 @@ fn agent_run_failures_before_launch_print_the_error_and_exit_0() { check( label, &project.run(args), - 0, + 1, message, Reported::Event("error", Some(kind)), ); @@ -597,7 +599,7 @@ fn validation_commands_exit_1_when_a_check_fails() { } #[test] -fn agent_policy_test_on_a_missing_profile_prints_the_error_and_exits_0() { +fn agent_policy_test_on_a_missing_profile_prints_the_error_and_exits_1() { let project = Project::new(); let out = project.run(&["agent", "policy", "test", "--profile", "nope"]); @@ -605,7 +607,7 @@ fn agent_policy_test_on_a_missing_profile_prints_the_error_and_exits_0() { check( "agent policy test", &out, - 0, + 1, "was not found in the global or directory config", Reported::Nothing, ); From 439422c5cebb1a1fe83c050de47dbb1d0b3ec0fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Wed, 7 Oct 2026 15:45:14 +0200 Subject: [PATCH 2/4] fix(agent): don't require an API key for agent profiles list and show Both only read local profile files and the local config. --- src/cmd/root.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/cmd/root.rs b/src/cmd/root.rs index e0b58250..8304bd0b 100644 --- a/src/cmd/root.rs +++ b/src/cmd/root.rs @@ -142,6 +142,10 @@ impl EntityType { EntityType::Agent(AgentCommand { subcommand: AgentSubcommand::Policy(_), }) => false, + // Reads local profile files and the local config only. + EntityType::Agent(AgentCommand { + subcommand: AgentSubcommand::Profiles(_), + }) => false, // File-only agent profiles require no Stashbase authentication. // Remote fallback is validated after the profile and local overrides // have been resolved. @@ -208,4 +212,15 @@ mod tests { .is_ok()); } } + + #[test] + fn agent_profiles_commands_do_not_require_an_api_key() { + for args in [ + vec!["stashbase", "agent", "profiles", "list"], + vec!["stashbase", "agent", "profiles", "show", "demo"], + ] { + let cli = Cli::try_parse_from(&args).unwrap(); + assert!(!cli.entity_type.requires_api_key(), "{args:?}"); + } + } } From f7e245b89cad4df375af1de27a685192ba512d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Wed, 7 Oct 2026 15:48:27 +0200 Subject: [PATCH 3/4] fix(agent): keep the bare agent hooks exit codes unchanged Claude Code, Codex and Cursor run `stashbase agent hooks` before tool calls and read its exit code, so it is left out of the exit-1 rule for agent commands. --- src/handlers/entry/root.rs | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index af450ade..544a0e33 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -491,7 +491,7 @@ pub async fn handle_cli(args: Cli) -> Exit { // Local commands such as `agent logs` do not need Stashbase authentication. let api_key = api_key.unwrap_or_default(); - let is_agent_command = matches!(args.entity_type, EntityType::Agent(_)); + let agent_failures_exit_1 = agent_failures_exit_1(&args.entity_type); let result: anyhow::Result = match args.entity_type { EntityType::Whoami(WhoamiCommand { format }) => { @@ -1488,13 +1488,13 @@ pub async fn handle_cli(args: Cli) -> Exit { Exit::from(err) } }; - if is_agent_command { + if agent_failures_exit_1 { exit.nonzero_on_failure() } else { exit } } else { - let is_agent_command = matches!(args.entity_type, EntityType::Agent(_)); + let agent_failures_exit_1 = agent_failures_exit_1(&args.entity_type); if let EntityType::Config(cmd) = args.entity_type { if let ConfigSubcommand::Reset(_) = cmd.subcommand { return print_error(handle_config_commands(cmd, &Config::new(), args.raw)); @@ -1510,7 +1510,7 @@ pub async fn handle_cli(args: Cli) -> Exit { // still exits 0. eprintln!("{:?}", err); let exit = Exit::failed(ErrorKind::Validation); - if is_agent_command { + if agent_failures_exit_1 { exit.nonzero_on_failure() } else { exit @@ -1540,6 +1540,19 @@ fn check_exit(failed: bool) -> Exit { } } +/// `agent` commands exit 1 when they fail, except the bare `agent hooks` +/// invocation: Claude Code, Codex and Cursor run it before tool calls and +/// read its exit code, so its codes stay as they were. +fn agent_failures_exit_1(entity_type: &EntityType) -> bool { + match entity_type { + EntityType::Agent(crate::cmd::agent::AgentCommand { + subcommand: AgentSubcommand::Hooks(command), + }) => command.subcommand.is_some(), + EntityType::Agent(_) => true, + _ => false, + } +} + fn uses_local_dependency_hook_broker(entity_type: &EntityType) -> bool { uses_local_dependency_hook_broker_mode( entity_type, @@ -2337,7 +2350,7 @@ fn spawn_remote_session_rotation( #[cfg(test)] mod tests { use super::{ - audit_binding_sources, codex_mcp_binding_header_overrides, configured_host_matches, + agent_failures_exit_1, audit_binding_sources, codex_mcp_binding_header_overrides, configured_host_matches, dependency_hooks_enabled, directory_profile_git_warning, ensure_replacement_session_is_compatible, infer_remote_agent_type, remote_bindings, remote_codex_command_with_mcp_binding_headers, remote_session_rotation_delay_for, @@ -2417,6 +2430,20 @@ mod tests { )); } + #[test] + fn agent_failures_exit_1_except_the_bare_hook_invocation() { + let parse = |args: &[&str]| Cli::try_parse_from(args).unwrap().entity_type; + + assert!(!agent_failures_exit_1(&parse(&["stashbase", "agent", "hooks"]))); + assert!(agent_failures_exit_1(&parse(&[ + "stashbase", "agent", "hooks", "deps", "install", "codex" + ]))); + assert!(agent_failures_exit_1(&parse(&[ + "stashbase", "agent", "run", "--profile", "p", "--", "true" + ]))); + assert!(!agent_failures_exit_1(&parse(&["stashbase", "pull"]))); + } + #[test] fn dependency_hooks_require_an_api_key() { let profile: AgentProfile = serde_json::from_value(serde_json::json!({ From 9fe4cbe97ae2f929da488712512aed5eb014c965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Wed, 7 Oct 2026 16:03:04 +0200 Subject: [PATCH 4/4] style: apply cargo fmt --- src/handlers/entry/root.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index 544a0e33..8ff45bbc 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -2350,8 +2350,8 @@ fn spawn_remote_session_rotation( #[cfg(test)] mod tests { use super::{ - agent_failures_exit_1, audit_binding_sources, codex_mcp_binding_header_overrides, configured_host_matches, - dependency_hooks_enabled, directory_profile_git_warning, + agent_failures_exit_1, audit_binding_sources, codex_mcp_binding_header_overrides, + configured_host_matches, dependency_hooks_enabled, directory_profile_git_warning, ensure_replacement_session_is_compatible, infer_remote_agent_type, remote_bindings, remote_codex_command_with_mcp_binding_headers, remote_session_rotation_delay_for, remote_session_transport_identity, remote_source_env_names, secret_child_name, @@ -2434,12 +2434,27 @@ mod tests { fn agent_failures_exit_1_except_the_bare_hook_invocation() { let parse = |args: &[&str]| Cli::try_parse_from(args).unwrap().entity_type; - assert!(!agent_failures_exit_1(&parse(&["stashbase", "agent", "hooks"]))); + assert!(!agent_failures_exit_1(&parse(&[ + "stashbase", + "agent", + "hooks" + ]))); assert!(agent_failures_exit_1(&parse(&[ - "stashbase", "agent", "hooks", "deps", "install", "codex" + "stashbase", + "agent", + "hooks", + "deps", + "install", + "codex" ]))); assert!(agent_failures_exit_1(&parse(&[ - "stashbase", "agent", "run", "--profile", "p", "--", "true" + "stashbase", + "agent", + "run", + "--profile", + "p", + "--", + "true" ]))); assert!(!agent_failures_exit_1(&parse(&["stashbase", "pull"]))); }