Found while implementing #16274 (PR to follow in that card), and driven rather than reasoned about — with a firing control. ⛔ Deliberately not fixed there: that card scopes the repair to the COMPLETION-path recordLog inside the node-failure try, and this is the different site, in the catch itself, with a different consequence. Filed bare — domain:*, type and priority are triage's.
The mechanism
Both initial-execution paths in packages/services/service-automation/src/engine.ts end their node-failure arm by recording the failed run:
execute()'s arm, the recordLog({ status: 'failed' }) that feeds its summary;executeWithoutRetry()'s arm, the same statement.
Neither is guarded. So when the history write itself throws, that throw escapes the catch that was handling the node failure — there is no outer handler — and leaves execute() as a rejected promise, where the declared contract is an AutomationResult. The run really did fail; the caller is simply never told in the declared shape.
#16274's guard does not reach this: it guards the COMPLETION write, so after it lands the window is reached by a genuine node failure instead of by a history throw on a successful run.
Reproduction, on the #16274 tree, with a control
Flow start -> work -> end; work throws; the store's recordTerminal throws SYNCHRONOUSLY (the escape recordLog's void write.catch(...) cannot see — both shipped stores are async and cannot do it, but SuspendedRunStore is an exported interface whose recordTerminal is optional, so a host store is unconstrained).
store = SYNC-THROW -> {"kind":"threw","error":"run-history driver refused the terminal row"}
store = HEALTHY (control) -> {"kind":"returned","status":"failed","error":"work blew up"}
The control is what makes this a reading: the identical flow and the identical node failure against a healthy store return the declared envelope with the NODE's own text. Only the store differs.
Why it matters, and why it is not the same card
The consequence is narrower than #16274's and narrower than #15944's — there is no false failed, no double run, and nothing misleads an operator about what happened, because the run genuinely failed. What is lost is the SHAPE: a caller that handles { success: false, status: 'failed' } gets an exception instead, so the transport's #9378 status arm is bypassed, errorMessage (the author's failure text, #9414) and summary (how far the run got before dying, #4354) never reach it, and a REST route or SDK caller sees a 500-class throw for a run that has a perfectly good failure envelope waiting.
The same invariant is the one violated — recordLog's own doc comment: "Best-effort + fire-and-forget: a history write must NEVER block or break the run that produced it."
Suggested direction (advisory — the disposition is the lane's)
The precedent shape is already in the file three times over (#15555 on the resume path's strand arm, #16273 and #16274 on the completion paths): guard the write at its own call site, recompute summary with the same pure function, and report the swallowed failure once at error with its consequence and fix. recordLog is now in DURABILITY_CRITICAL_CALLEES, so pnpm check:durability-log-level will judge the new seam's level automatically.
⛔ Nothing here argues for widening any catch arm's meaning, and ⛔ nothing here bears on restoreConsumedSuspension or on inspectStrandedRequests (#15358).
Refs: #16274 (the completion-path guard on these two paths) · #16273 / #15944 (the resume path's completion site) · #15555 (the resume path's failure side).
Generated by Claude Code
Found while implementing #16274 (PR to follow in that card), and driven rather than reasoned about — with a firing control. ⛔ Deliberately not fixed there: that card scopes the repair to the COMPLETION-path
recordLoginside the node-failuretry, and this is the different site, in thecatchitself, with a different consequence. Filed bare —domain:*, type and priority are triage's.The mechanism
Both initial-execution paths in
packages/services/service-automation/src/engine.tsend their node-failure arm by recording the failed run:execute()'s arm, therecordLog({ status: 'failed' })that feeds itssummary;executeWithoutRetry()'s arm, the same statement.Neither is guarded. So when the history write itself throws, that throw escapes the
catchthat was handling the node failure — there is no outer handler — and leavesexecute()as a rejected promise, where the declared contract is anAutomationResult. The run really did fail; the caller is simply never told in the declared shape.#16274's guard does not reach this: it guards the COMPLETION write, so after it lands the window is reached by a genuine node failure instead of by a history throw on a successful run.
Reproduction, on the #16274 tree, with a control
Flow
start -> work -> end;workthrows; the store'srecordTerminalthrows SYNCHRONOUSLY (the escaperecordLog'svoid write.catch(...)cannot see — both shipped stores areasyncand cannot do it, butSuspendedRunStoreis an exported interface whoserecordTerminalis optional, so a host store is unconstrained).The control is what makes this a reading: the identical flow and the identical node failure against a healthy store return the declared envelope with the NODE's own text. Only the store differs.
Why it matters, and why it is not the same card
The consequence is narrower than #16274's and narrower than #15944's — there is no false
failed, no double run, and nothing misleads an operator about what happened, because the run genuinely failed. What is lost is the SHAPE: a caller that handles{ success: false, status: 'failed' }gets an exception instead, so the transport's #9378statusarm is bypassed,errorMessage(the author's failure text, #9414) andsummary(how far the run got before dying, #4354) never reach it, and a REST route or SDK caller sees a 500-class throw for a run that has a perfectly good failure envelope waiting.The same invariant is the one violated —
recordLog's own doc comment: "Best-effort + fire-and-forget: a history write must NEVER block or break the run that produced it."Suggested direction (advisory — the disposition is the lane's)
The precedent shape is already in the file three times over (#15555 on the resume path's strand arm, #16273 and #16274 on the completion paths): guard the write at its own call site, recompute
summarywith the same pure function, and report the swallowed failure once aterrorwith its consequence and fix.recordLogis now inDURABILITY_CRITICAL_CALLEES, sopnpm check:durability-log-levelwill judge the new seam's level automatically.⛔ Nothing here argues for widening any
catcharm's meaning, and ⛔ nothing here bears onrestoreConsumedSuspensionor oninspectStrandedRequests(#15358).Refs: #16274 (the completion-path guard on these two paths) · #16273 / #15944 (the resume path's completion site) · #15555 (the resume path's failure side).
Generated by Claude Code