Skip to content
Merged
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
16 changes: 12 additions & 4 deletions kernel/relayflowd-core/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ pub struct AttemptResult {
/// Execution failures bypass verification but still follow retry policy.
pub failure_reason: Option<CompletionReason>,
/// Why the attempt was rejected, in the vocabulary of whoever rejected it.
/// A failed completion journals a null `output` — this is the only place a
/// rejection can name its own cause, so a dropped detail is a lost error.
/// The structured `output` (exit code + captured stdout/stderr tails) also
/// survives into the failed completion record (#292); this human-readable
/// detail complements it rather than being the only surviving cause.
pub failure_detail: Option<String>,
}

Expand Down Expand Up @@ -387,6 +388,13 @@ pub fn completion_actions(
.as_ref()
.is_some_and(|record| record.verdict == crate::entry::VerificationVerdict::Pass);
let may_retry = semantic_executions.saturating_add(1) < step.max_iterations;
// Preserve `result.output` for successful completions, and for FAILED
// deterministic completions specifically — deterministic attempts journal
// `{exit_code, stdout_tail, stderr_tail}` so the CLI can render the
// diagnostic (#292 unblocks #276). LLM/agent step outputs remain nulled on
// verification failure: their `result.output` is the rejected parsed value,
// and the existing invariant is that it never survives to the journal.
let preserve_failure_output = step.step_type() == StepType::Deterministic;
let (reason, disposition, output, next_attempt_at_ms) = if verified {
(
CompletionReason::Success,
Expand All @@ -402,7 +410,7 @@ pub fn completion_actions(
.failure_reason
.unwrap_or(CompletionReason::VerificationFailed),
Disposition::Retry,
Value::Null,
if preserve_failure_output { result.output } else { Value::Null },
Some(now_ms.saturating_add(delay as i64)),
)
} else {
Expand All @@ -411,7 +419,7 @@ pub fn completion_actions(
.failure_reason
.unwrap_or(CompletionReason::RetriesExhausted),
Disposition::StepDone,
Value::Null,
if preserve_failure_output { result.output } else { Value::Null },
None,
)
};
Expand Down
67 changes: 67 additions & 0 deletions kernel/relayflowd-core/src/machine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,73 @@ fn verification_failure_schedules_a_durable_retry() {
));
}

#[test]
fn failed_deterministic_completion_preserves_exit_code_and_stderr(
) {
// #292: failed attempts used to journal `output: null`, so the CLI
// could not surface the actual exit code or stderr excerpt. Both the
// retry branch and the terminal branch must now preserve the captured
// shape verbatim from `AttemptResult.output`.
let spec = retrying_spec();
let mut retryable_step = spec.steps[0].clone();
retryable_step.max_iterations = 2;
let retry_output = json!({
"exit_code": 7,
"stdout_tail": "",
"stderr_tail": "shakedown intentional failure",
});
let retry_actions = completion_actions(
"run",
&retryable_step,
1,
0,
AttemptResult::successful(retry_output.clone(), "kernel"),
1_000,
);
let Action::Append(retry_completed) = &retry_actions[0] else {
panic!("failed attempt must append a typed completion");
};
let retry_payload: StepCompletedPayload =
serde_json::from_value(retry_completed.payload.clone())
.expect("failed retry completion must deserialize");
assert_eq!(retry_payload.disposition, Disposition::Retry);
assert_eq!(
retry_payload.output, retry_output,
"retry path drops the captured exit_code/stderr_tail"
);

let mut terminal_step = spec.steps[0].clone();
terminal_step.max_iterations = 1;
let terminal_output = json!({
"exit_code": 7,
"stdout_tail": "",
"stderr_tail": "final attempt failed",
});
let terminal_actions = completion_actions(
"run",
&terminal_step,
1,
0,
AttemptResult::successful(terminal_output.clone(), "kernel"),
2_000,
);
let Action::Append(terminal_completed) = &terminal_actions[0] else {
panic!("terminal failure must append a typed completion");
};
let terminal_payload: StepCompletedPayload =
serde_json::from_value(terminal_completed.payload.clone())
.expect("terminal failure completion must deserialize");
assert_eq!(terminal_payload.disposition, Disposition::StepDone);
assert_eq!(
terminal_payload.completion_reason,
CompletionReason::RetriesExhausted
);
assert_eq!(
terminal_payload.output, terminal_output,
"terminal failure path drops the captured exit_code/stderr_tail"
);
}

#[test]
fn every_failed_run_terminates_with_declared_completion_reasons() {
let failure_reasons = [
Expand Down
9 changes: 8 additions & 1 deletion kernel/relayflowd/src/exec_det.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,14 @@ mod tests {
let Action::Append(completed) = &actions[0] else {
panic!("expected completion")
};
assert_eq!(completed.payload["output"], serde_json::Value::Null);
// Post-#292: structured output survives on failed deterministic completions
// so the CLI diagnostic (#276 / merged as #366) can render exit code + stderr
// directly, without falling back to trajectory_tail.
assert_eq!(completed.payload["output"]["exit_code"], 7);
assert_eq!(
completed.payload["output"]["stderr_tail"],
"shakedown intentional failure"
);
assert_eq!(completed.payload["trajectory_tail"]["exit_code"], 7);
assert_eq!(
completed.payload["trajectory_tail"]["stderr_tail"],
Expand Down
7 changes: 4 additions & 3 deletions packages/sdk/src/cli/deterministic-failure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ export async function deterministicFailureDetails(
// A later completion supersedes an earlier failed attempt.
failures.delete(stepId);
const payload = record(entry['payload']);
// Failed completions null reusable output; command evidence survives in
// trajectory_tail. Accept output as well for existing completion records.
const output = record(payload?.['trajectory_tail']) ?? record(payload?.['output']);
// Post-#292 the kernel preserves the captured `{exit_code, stdout_tail,
// stderr_tail}` in `output` on failed completions; prefer it. Fall back
// to `trajectory_tail` for journal records emitted before that fix.
const output = record(payload?.['output']) ?? record(payload?.['trajectory_tail']);
const exitCode = output?.['exit_code'];
if (payload?.['disposition'] !== 'step_done' || payload['completionReason'] === 'success'
|| typeof exitCode !== 'number' || !Number.isSafeInteger(exitCode) || exitCode === 0) continue;
Expand Down
23 changes: 23 additions & 0 deletions packages/sdk/tests/deterministic-failure-diagnostic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,27 @@ describe('deterministic failure diagnostic', () => {
kind: 'step_failed', message: expect.stringContaining('invalid journal sequence'),
});
});

it('reads exit_code and stderr_tail from output (post-#292 canonical shape)', async () => {
// Post-#292 the kernel emits the captured shape in `output` on failed
// completions rather than routing it through `trajectory_tail`. The CLI
// must surface the diagnostic from that field even when trajectory_tail
// is absent.
const post292Completion = {
seq: 1, entry_type: 'step.completed', step_id: 'fail-command',
payload: {
completionReason: 'retries_exhausted', disposition: 'step_done',
output: { exit_code: 7, stdout_tail: '', stderr_tail: 'post-292 stderr' },
// trajectory_tail intentionally omitted — output is the canonical
// carrier once the kernel fix has landed.
},
};
const { client } = stub([[post292Completion]]);
const diagnostic = (await classify(client)).report.diagnostics.at(-1) as RunDiagnostic;
expect(diagnostic).toMatchObject({
kind: 'step_failed', stepId: 'fail-command', exitCode: 7,
stderrTail: 'post-292 stderr',
hint: 'flows replay run-failed --at fail-command',
});
});
});
Loading