Skip to content

Commit 660de59

Browse files
milesjclaude
andcommitted
fix: Propagate a failing task's exit code instead of collapsing it to 1
`moon run`, `moon ci`, and `moon check` always exited with code 1 on task failure, regardless of the underlying process's real exit code. This prevented CI pipelines from distinguishing between failure codes. The exit code now propagates across both pipeline paths: - Bail path (the default): the pipeline bubbles up an `Err`, which starbase otherwise collapses to 1. We now extract the process exit code from the error chain at the CLI boundary (mirroring the existing broken-pipe handling), render the error as before, and exit with the task's real code. - Continue path (`exec --on-failure continue`): `ExecWorkflow::execute` reads the failed action's exit code directly. Signals, aborts, non-task failures, and codes outside the 0-255 range fall back to a generic 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 509caa8 commit 660de59

13 files changed

Lines changed: 276 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535
- Fixed tasks that emit read-only outputs (e.g. `0444` files) failing on every cache hit with a
3636
"Permission denied" error when the `casOutputsCache` experiment is enabled. Caches that already
3737
contain read-only objects are healed automatically.
38+
- Fixed an issue where a failing task would always exit moon with code `1`, instead of propagating
39+
the task's actual exit code. Commands like `moon run`, `moon ci`, and `moon check` now exit with
40+
the underlying process's exit code (matching `make`, `npm`, `just`, etc.), so CI pipelines can
41+
distinguish between failure codes. Signals, aborts, and codes outside the `0-255` range still fall
42+
back to `1`.
3843

3944
## 2.4.2
4045

crates/action/src/action.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,16 @@ impl Action {
153153
miette::miette!("Unknown error!")
154154
}
155155

156+
/// Return the exit code of the last executed task operation, if this
157+
/// action ran a task/process. Returns `None` when no execution occurred
158+
/// (setup/sync actions), or when a code wasn't captured (aborts, signals).
159+
pub fn get_exit_code(&self) -> Option<i32> {
160+
self.operations
161+
.get_last_execution()
162+
.and_then(|operation| operation.get_exec_output())
163+
.and_then(|output| output.exit_code)
164+
}
165+
156166
pub fn get_prefix(&self) -> &str {
157167
match &*self.node {
158168
ActionNode::None => "unknown",

crates/action/tests/action_test.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
use moon_action::{Action, ActionStatus, Operation};
2+
3+
fn task_op(exit_code: Option<i32>, status: ActionStatus) -> Operation {
4+
let mut op = Operation::task_execution("cmd");
5+
6+
if let Some(output) = op.get_exec_output_mut() {
7+
output.exit_code = exit_code;
8+
}
9+
10+
op.finish(status);
11+
op
12+
}
13+
14+
mod get_exit_code {
15+
use super::*;
16+
17+
#[test]
18+
fn none_when_no_operations() {
19+
let action = Action::default();
20+
21+
assert_eq!(action.get_exit_code(), None);
22+
}
23+
24+
#[test]
25+
fn none_when_no_execution_operation() {
26+
let mut action = Action::default();
27+
action.operations.push(Operation::hash_generation());
28+
29+
assert_eq!(action.get_exit_code(), None);
30+
}
31+
32+
#[test]
33+
fn returns_the_execution_exit_code() {
34+
let mut action = Action::default();
35+
action
36+
.operations
37+
.push(task_op(Some(80), ActionStatus::Failed));
38+
39+
assert_eq!(action.get_exit_code(), Some(80));
40+
}
41+
42+
#[test]
43+
fn returns_zero_for_passing_execution() {
44+
let mut action = Action::default();
45+
action
46+
.operations
47+
.push(task_op(Some(0), ActionStatus::Passed));
48+
49+
assert_eq!(action.get_exit_code(), Some(0));
50+
}
51+
52+
#[test]
53+
fn returns_the_last_execution_code_when_retried() {
54+
let mut action = Action::default();
55+
action
56+
.operations
57+
.push(task_op(Some(1), ActionStatus::Failed));
58+
action
59+
.operations
60+
.push(task_op(Some(70), ActionStatus::Failed));
61+
62+
assert_eq!(action.get_exit_code(), Some(70));
63+
}
64+
65+
#[test]
66+
fn returns_negative_one_for_injected_aborts() {
67+
let mut action = Action::default();
68+
action
69+
.operations
70+
.push(task_op(Some(-1), ActionStatus::Aborted));
71+
72+
assert_eq!(action.get_exit_code(), Some(-1));
73+
}
74+
}

crates/app/src/commands/exec.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,15 +285,35 @@ impl ExecWorkflow {
285285
.execute_action_pipeline(action_context, action_graph)
286286
.await?;
287287

288-
let failed = results.into_iter().any(|result| {
289-
if result.has_failed() {
290-
!result.allow_failure
291-
} else {
292-
false
288+
// Determine the exit code to bubble up. When a task fails, propagate its
289+
// actual exit code (like make, npm, and just) so that CI and callers can
290+
// distinguish between failure codes. For aborts, signals, non-task
291+
// failures, or codes that can't be represented, fall back to a generic 1.
292+
let mut any_failed = false;
293+
let mut exit_code = None;
294+
295+
for result in &results {
296+
if !result.has_failed() || result.allow_failure {
297+
continue;
298+
}
299+
300+
any_failed = true;
301+
302+
if let Some(code) = result
303+
.get_exit_code()
304+
.and_then(|code| u8::try_from(code).ok())
305+
.filter(|code| *code != 0)
306+
{
307+
exit_code = Some(code);
308+
break;
293309
}
294-
});
310+
}
311+
312+
if let Some(code) = exit_code {
313+
return Ok(Some(code));
314+
}
295315

296-
if failed {
316+
if any_failed {
297317
return Ok(Some(1));
298318
}
299319

crates/app/src/helpers.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,39 @@ use moon_action_pipeline::ActionPipeline;
1010
use moon_common::Id;
1111
use moon_console::ui::{OwnedOrShared, Progress, ProgressDisplay, ProgressReporter};
1212
use moon_console::{Console, ConsoleError, Level};
13+
use moon_process::ProcessError;
1314
use serde::Serialize;
1415
use starbase_utils::{fs, json, toml, yaml};
1516
use std::ops::Deref;
1617
use std::path::{Path, PathBuf};
1718
use std::sync::Arc;
1819

20+
/// Walk an error chain and return the underlying process exit code, if the
21+
/// failure originated from a task/process that exited with a non-zero status.
22+
/// Signals, aborts, and codes that can't be represented as a `u8` yield `None`,
23+
/// so the caller can fall back to a generic exit code. This lets the pipeline's
24+
/// bubbled-up error propagate the task's real exit code, which `starbase` would
25+
/// otherwise collapse to 1.
26+
pub fn extract_task_exit_code(report: &miette::Report) -> Option<u8> {
27+
report
28+
.chain()
29+
.find_map(|error| {
30+
// The process error is usually boxed within its parent (e.g.
31+
// `TaskRunnerError` wraps `Box<ProcessError>`), which surfaces as a
32+
// `Box<ProcessError>` in the chain, so check both shapes.
33+
error
34+
.downcast_ref::<ProcessError>()
35+
.or_else(|| {
36+
error
37+
.downcast_ref::<Box<ProcessError>>()
38+
.map(|boxed| boxed.as_ref())
39+
})
40+
.and_then(ProcessError::get_exit_code)
41+
})
42+
.and_then(|code| u8::try_from(code).ok())
43+
.filter(|code| *code != 0)
44+
}
45+
1946
pub fn serialize_config_based_on_extension(
2047
plugin_id: &Id,
2148
path: &Path,

crates/app/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ pub mod watchers;
1313

1414
pub use app::*;
1515
pub use app_error::*;
16+
pub use helpers::extract_task_exit_code;
1617
pub use session::*;

crates/app/tests/helpers_test.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
use miette::Diagnostic;
2+
use moon_app::extract_task_exit_code;
3+
use moon_process::ProcessError;
4+
use thiserror::Error;
5+
6+
// Mirrors how `TaskRunnerError::RunFailed` wraps `Box<ProcessError>`, which
7+
// surfaces the process error as a `Box<ProcessError>` in the error chain.
8+
#[derive(Debug, Diagnostic, Error)]
9+
#[error("task failed to run")]
10+
struct WrappedError {
11+
#[source]
12+
error: Box<ProcessError>,
13+
}
14+
15+
fn exit_nonzero(code: Option<i32>) -> ProcessError {
16+
ProcessError::ExitNonZero {
17+
bin: "bash".into(),
18+
status: "exit code".into(),
19+
code,
20+
}
21+
}
22+
23+
fn wrapped(code: Option<i32>) -> miette::Report {
24+
WrappedError {
25+
error: Box::new(exit_nonzero(code)),
26+
}
27+
.into()
28+
}
29+
30+
mod extract_task_exit_code {
31+
use super::*;
32+
33+
#[test]
34+
fn extracts_from_boxed_process_error() {
35+
assert_eq!(extract_task_exit_code(&wrapped(Some(80))), Some(80));
36+
}
37+
38+
#[test]
39+
fn extracts_from_bare_process_error() {
40+
let report: miette::Report = exit_nonzero(Some(2)).into();
41+
42+
assert_eq!(extract_task_exit_code(&report), Some(2));
43+
}
44+
45+
#[test]
46+
fn propagates_codes_above_127() {
47+
assert_eq!(extract_task_exit_code(&wrapped(Some(200))), Some(200));
48+
}
49+
50+
#[test]
51+
fn none_for_signal_without_code() {
52+
assert_eq!(extract_task_exit_code(&wrapped(None)), None);
53+
}
54+
55+
#[test]
56+
fn none_for_injected_abort_code() {
57+
// Aborts before a process spawns are recorded with an exit code of -1
58+
assert_eq!(extract_task_exit_code(&wrapped(Some(-1))), None);
59+
}
60+
61+
#[test]
62+
fn none_for_zero_code() {
63+
assert_eq!(extract_task_exit_code(&wrapped(Some(0))), None);
64+
}
65+
66+
#[test]
67+
fn none_for_out_of_u8_range() {
68+
// e.g. large Windows exit codes that can't map to a u8
69+
assert_eq!(extract_task_exit_code(&wrapped(Some(300))), None);
70+
}
71+
72+
#[test]
73+
fn none_for_non_process_error() {
74+
let report = miette::miette!("something else went wrong");
75+
76+
assert_eq!(extract_task_exit_code(&report), None);
77+
}
78+
}

crates/cli/src/shared.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,5 +295,17 @@ pub async fn run_cli(args: Vec<OsString>) -> MainResult {
295295
return Ok(ExitCode::from(BROKEN_PIPE_EXIT_CODE));
296296
}
297297

298+
// A task failed with a specific exit code. Render the error as the
299+
// top-level handler would, then exit with the task's actual code so
300+
// callers can distinguish between failure codes (like make, npm, and
301+
// just). `starbase` otherwise collapses every error to exit code 1.
302+
if let Some(error) = &outcome.error
303+
&& let Some(code) = moon_app::extract_task_exit_code(error)
304+
{
305+
eprintln!("Error: {error:?}");
306+
307+
return Ok(ExitCode::from(code));
308+
}
309+
298310
outcome.into_exit_result()
299311
}

crates/cli/tests/exec_test.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,20 @@ mod exec {
169169
.stderr(predicate::str::contains("stderr"));
170170
}
171171

172+
// The script exits with a distinct code (2) so we can verify moon
173+
// propagates the task's actual exit code, instead of collapsing it to 1.
174+
#[test]
175+
#[cfg(unix)]
176+
fn propagates_process_exit_code() {
177+
let sandbox = create_pipeline_sandbox();
178+
179+
let assert = sandbox.run_bin(|cmd| {
180+
cmd.arg("exec").arg(target("exitNonZeroInline"));
181+
});
182+
183+
assert.failure().code(2);
184+
}
185+
172186
#[test]
173187
fn passes_args_through() {
174188
let sandbox = create_pipeline_sandbox();

crates/process/src/output.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ impl Output {
6767

6868
pub fn to_error(&self, bin: impl AsRef<str>, with_message: bool) -> ProcessError {
6969
let bin = bin.as_ref().to_owned();
70+
let code = self.code();
7071

7172
let status = match &self.exit {
7273
ChildExit::Completed(status) => match status.code() {
@@ -79,7 +80,7 @@ impl Output {
7980
};
8081

8182
if !with_message {
82-
return ProcessError::ExitNonZero { bin, status };
83+
return ProcessError::ExitNonZero { bin, status, code };
8384
}
8485

8586
let mut message = output_to_trimmed_string(&self.stderr);
@@ -96,6 +97,7 @@ impl Output {
9697
ProcessError::ExitNonZeroWithOutput {
9798
bin,
9899
status,
100+
code,
99101
output: message,
100102
}
101103
}

0 commit comments

Comments
 (0)