fix(sdk): read an authored step's terminal completion, not a retried attempt

Merged
KH
22d ago
+778-32

Bug

readCompletedStepOutput (packages/sdk/src/authored-step-output.ts) took the first step.completed for the step. When an attempt is retried the kernel journals a non-terminal step.completed with disposition: "retry" (crash recovery's crashed, a worker_error) before the attempt that settles the step. So an authored flow killed mid-f.run and then flows resumed failed with journal step "run-2" completed with crashed, even though the child run completed success. This came from Cloud's executor e2e, reproduced here at b65f7928.

The normal path, with no resume, is also broken. The same reader serves f.agent/f.llm, helper, MCP and plugin children. A dead attempt charges no iteration, so even with max_iterations: 1, a worker that disconnects mid-attempt gets crashed/retry and then a successful attempt 2. Before this fix the reader threw crashed on that run too (see the second live test and the mutation output below).

Fix

  • Terminal completion: the reader now uses the step's last step.completed whose disposition is not retry or park. Only that completion is pushed to journalSteps and indexed on the root via recordAuthoredChild/stepFailure. The run-level verdict check is unchanged. If the run settled between attempts (for example it was canceled in backoff), the reader reports the run's reason instead of a protocol violation.
  • Paging: the child journal is now read page by page. Before, one journal.read returned only the first 100 entries, and the last completion can sit past that.
  • Wait for the child to settle (authored-child-run.ts): a child adopted through its admission key can still be mid-attempt or mid-backoff on the request that started it, because run.start returns an existing admission as it stands. If there is no run.completed and a step is still unsettled, the reader waits. The wait:
    • is read-only: it never calls run.resume, because that would presume the inline deterministic attempt dead and run the command again;
    • is bounded by the child's own lease plus the lease-sweep grace, or by its journaled sleep.until, with no fixed deadline;
    • can be canceled through the flow's signal.
  • Parked children: a worker-typed child parked on a worker nobody attached stops the wait at once. A helper drives exactly one attempt, and the retry sent after it dies when its connection closes. Such a child reports its latest attempt that ran to a failed result, and completed_by: kernel deaths rank below it. This keeps the existing helper worker_error behaviour.

Tests

  • tests/authored-step-terminal-completion.test.ts uses fake journals for these cases: retried then success resolves; retried then terminal failure reports the terminal reason; canceled in backoff reports the run's reason; an adopted child mid-backoff is waited out without run.resume; lease expiry is refused; cancel works; a parked helper reports worker_error.
  • tests/authored-retried-child-resume-live.test.ts runs against the real relayflowd:
    1. It runs the Cloud e2e flow, SIGKILLs the CLI and relayflowd mid-step-2, then runs flows resume. The root completes success, count is still ran\n (step 1 was not re-run), the index holds only success for run-2, and the child journal shows [crashed,retry],[success,step_done].
    2. With no daemon restart and no flows resume, an agent worker dies mid-attempt and a second worker completes it. The reader returns attempt 2's output.

Mutation check: I reverted terminalCompletion to first-match and ran the two new files:

   × readCompletedStepOutput on a child run that retried an attempt > resolves the terminal success, not the crashed attempt before it 7ms
     → promise rejected "AuthoredFlowExecutionError: step_failed: … { …(6) }" instead of resolving
   × readCompletedStepOutput on a child run that retried an attempt > reports the terminal failure reason, not the retried attempt's 7ms
     → expected 'step_failed: journal step "run-2" com…' to contain 'journal step "run-2" completed with r…'
   × readCompletedStepOutput on a child run that retried an attempt > reports the run's reason when it settled between attempts 2ms
     → expected 'step_failed: journal step "run-2" com…' to contain 'journal run for step "run-2" complete…'
   × readCompletedStepOutput on an adopted child that is not terminal yet > waits out the retry backoff and the next attempt, without driving the child 202ms
     → promise rejected "AuthoredFlowExecutionError: step_failed: … { …(6) }" instead of resolving
   × an authored step whose child run retried an attempt > resumes after a kill mid-step and reads the terminal completion, not the crashed one 30167ms
     → FAILED [step_failed] journal step "run-2" completed with crashed
   × an authored step whose child run retried an attempt > reads the terminal completion without any resume, when a worker died mid-attempt 122ms
     → promise rejected "AuthoredFlowExecutionError: step_failed: … { …(6) }" instead of resolving
      Tests  6 failed | 3 passed (9)

I restored the fix byte-for-byte (cmp confirmed) and ran them again:

 ✓ tests/authored-step-terminal-completion.test.ts (7 tests) 311ms
 ✓ tests/authored-retried-child-resume-live.test.ts (2 tests) 1088ms
   ✓ an authored step whose child run retried an attempt > resumes after a kill mid-step and reads the terminal completion, not the crashed one 978ms
      Tests  9 passed (9)

npm test in packages/sdk (test:prep, typecheck, build, typecheck:tests, vitest run):

 Test Files  2 failed | 217 passed | 3 skipped (222)
      Tests  1 failed | 3511 passed | 21 skipped (3533)

Both remaining failures also fail on unmodified origin/main in this environment, so they are not caused by this change:

  • authored-node-runtime.test.ts: the local bun is 1.4.2 and the test expects 1.4.0.
  • cloud-run.test.ts "names an authored source that does not load": transport_error, a network failure.

🤖 Generated with Claude Code

[!NOTE]
Medium Risk
Changes core journal interpretation for all f.* child runs (run/agent/MCP/plugin) and resume paths; behavior is heavily tested but mistakes could mis-report success or failure across authored execution.

Overview
Fixes authored child-step reading so retried attempts (disposition: retry) and resume/adopted children no longer produce false failures or premature successes.

readCompletedStepOutput now pages child journals via readChildJournal, picks the step’s last settling step.completed through shared settlingCompletion (skipping retry/park), and optionally waitForTerminalChildRun before accepting success—bounded by lease/backoff, read-only (no run.resume), cancelable via signal, with explicit handling for parked worker children and gates that finish after the producer. Callers pass knownStatus from run.start / classifyOutcome; MCP, plugin, worker, and recorded-run paths are wired through.

The Node IPC durable verifier uses the same settling rule so retried children match the reader. delay and LEASE_SWEEP_GRACE_MS are exported from cli/run.ts for the waiter. New unit and live tests cover crash-resume, worker retry, backoff wait, and gate timing.

Reviewed by Cursor Bugbot for commit 017a537e4c119b71819ce231ada48d03e31b122a. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by cubic

Fixes authored step completion reading so retried attempts no longer fail runs that settled, and so a child's success is never read before its run is terminal.

Bug Fixes

  • Reads the step's last step.completed whose disposition is not retry or park, records and indexes only that one, and has the Bun result verifier judge claims against the same completion.
  • Pages the child journal, since a single read returns only the first 100 entries.
  • Never accepts a step's success before the child run is terminal: it waits unless the caller's own kernel answer is terminal, and refuses a success when the wait ends with the run still running.
  • Waits read-only for an adopted child still mid-attempt or mid-backoff, bounded by its lease plus sweep grace or journaled sleep.until, cancelable via the flow's signal; only a lease past its sweep grace errors, while an elapsed retry wake simply re-polls.
  • A child parked on a worker nobody attached reports its latest attempt that ran to a failed result instead of the kernel-recorded death.