diff --git a/kernel/relayflowd-core/src/machine.rs b/kernel/relayflowd-core/src/machine.rs index e7579e714..255b53645 100644 --- a/kernel/relayflowd-core/src/machine.rs +++ b/kernel/relayflowd-core/src/machine.rs @@ -60,8 +60,9 @@ pub struct AttemptResult { /// Execution failures bypass verification but still follow retry policy. pub failure_reason: Option, /// 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, } @@ -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, @@ -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 { @@ -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, ) }; diff --git a/kernel/relayflowd-core/src/machine/tests.rs b/kernel/relayflowd-core/src/machine/tests.rs index 3907712e4..26100af4b 100644 --- a/kernel/relayflowd-core/src/machine/tests.rs +++ b/kernel/relayflowd-core/src/machine/tests.rs @@ -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 = [ diff --git a/kernel/relayflowd/src/exec_det.rs b/kernel/relayflowd/src/exec_det.rs index e907945d7..3a6ef16b6 100644 --- a/kernel/relayflowd/src/exec_det.rs +++ b/kernel/relayflowd/src/exec_det.rs @@ -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"], diff --git a/packages/sdk/src/cli/deterministic-failure.ts b/packages/sdk/src/cli/deterministic-failure.ts index 53173d386..3e9176a78 100644 --- a/packages/sdk/src/cli/deterministic-failure.ts +++ b/packages/sdk/src/cli/deterministic-failure.ts @@ -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; diff --git a/packages/sdk/tests/deterministic-failure-diagnostic.test.ts b/packages/sdk/tests/deterministic-failure-diagnostic.test.ts index fbcc060e1..ace8ba0fa 100644 --- a/packages/sdk/tests/deterministic-failure-diagnostic.test.ts +++ b/packages/sdk/tests/deterministic-failure-diagnostic.test.ts @@ -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', + }); + }); });