From 7e611c2bea7111bab1a9fc09e0fc30281d333916 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 12 Sep 2026 16:26:34 +0200 Subject: [PATCH 1/3] feat(kernel,sdk): preserve failed deterministic attempt output in step.completed (#292) The daemon captured {exit_code, stdout_tail, stderr_tail} on every attempt but dropped it to Value::Null on both the retry and terminal failure paths (machine.rs:405,414) before journaling the completion. The CLI diagnostic merged in #366 could only read the shape through the trajectory_tail workaround; genuine failure records carried only free-text detail in verification.detail. Preserve result.output across every branch of completion_actions so the journal now carries the structured shape verbatim. Prefer output over trajectory_tail in the SDK CLI reader (trajectory_tail remains a fallback for records emitted before this fix). Closes #292. Unblocks #276's acceptance criterion (already merged as #366) against the real kernel journal shape. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- kernel/relayflowd-core/src/machine.rs | 12 ++-- kernel/relayflowd-core/src/machine/tests.rs | 67 +++++++++++++++++++ packages/sdk/src/cli/deterministic-failure.ts | 7 +- .../deterministic-failure-diagnostic.test.ts | 23 +++++++ 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/kernel/relayflowd-core/src/machine.rs b/kernel/relayflowd-core/src/machine.rs index e7579e714..3b61fc5f3 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,9 @@ 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` across every branch so a failed deterministic + // attempt journals the captured `{exit_code, stdout_tail, stderr_tail}` + // instead of dropping the diagnostic on the floor (#292 unblocks #276). let (reason, disposition, output, next_attempt_at_ms) = if verified { ( CompletionReason::Success, @@ -402,7 +406,7 @@ pub fn completion_actions( .failure_reason .unwrap_or(CompletionReason::VerificationFailed), Disposition::Retry, - Value::Null, + result.output, Some(now_ms.saturating_add(delay as i64)), ) } else { @@ -411,7 +415,7 @@ pub fn completion_actions( .failure_reason .unwrap_or(CompletionReason::RetriesExhausted), Disposition::StepDone, - Value::Null, + result.output, 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/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', + }); + }); }); From 9f9fa9fd2e00282f392f3fd35692c4cb48cbb558 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 12 Sep 2026 16:48:34 +0200 Subject: [PATCH 2/3] test(kernel): update exec_det evidence-survives test for #292 payload preservation The pre-#292 assertion pinned output=Null (the bug this slice fixes). Flip the assertion to match the corrected behavior: structured output survives on failed deterministic completions. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- kernel/relayflowd/src/exec_det.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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"], From 40f1bc6bc9bcf7c6cee4aaa663ec56ccb9bb431a Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 12 Sep 2026 17:20:09 +0200 Subject: [PATCH 3/3] fix(kernel): scope #292 output preservation to deterministic steps only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial change preserved result.output across all step types on failure, which broke the PII invariant for LLM/agent verification failures (test 'hn-monitor analyze-story FAILS verification when the CLI omits required schema fields' in live-kernel.test.ts pins output=null on json_schema rejection). Restrict preservation to deterministic steps — that was the original #292 scope, matching the {exit_code, stdout_tail, stderr_tail} capture in exec_det.rs. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- kernel/relayflowd-core/src/machine.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/kernel/relayflowd-core/src/machine.rs b/kernel/relayflowd-core/src/machine.rs index 3b61fc5f3..255b53645 100644 --- a/kernel/relayflowd-core/src/machine.rs +++ b/kernel/relayflowd-core/src/machine.rs @@ -388,9 +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` across every branch so a failed deterministic - // attempt journals the captured `{exit_code, stdout_tail, stderr_tail}` - // instead of dropping the diagnostic on the floor (#292 unblocks #276). + // 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, @@ -406,7 +410,7 @@ pub fn completion_actions( .failure_reason .unwrap_or(CompletionReason::VerificationFailed), Disposition::Retry, - result.output, + if preserve_failure_output { result.output } else { Value::Null }, Some(now_ms.saturating_add(delay as i64)), ) } else { @@ -415,7 +419,7 @@ pub fn completion_actions( .failure_reason .unwrap_or(CompletionReason::RetriesExhausted), Disposition::StepDone, - result.output, + if preserve_failure_output { result.output } else { Value::Null }, None, ) };