Skip to content

Commit 5ccd0ad

Browse files
committed
fix(service-automation): a completed run's history write can no longer strand or re-arm it
`resumeInternal`'s completion path called `recordLog({ status: 'completed' })` from inside the `try` whose `catch` exists for node failures, so a throw out of a history write on a run that finished successfully was handled as a node failure: a repair snapshot was journalled, `status: 'stranded'` stamped, `success: false` answered, and `restoreConsumedSuspension` then honoured the snapshot and re-armed the pause so the next resume re-ran every downstream node. Guard the completion-path `recordLog` at its own site, restoring the invariant that call's own doc comment states. The failure is reported at `error` with its consequence and fix, and the run summary is recomputed by the same pure function `recordLog` runs first. `restoreConsumedSuspension` is untouched: it judged correctly on the evidence it was handed, and no journal is written for a completed run at all now. Also records the false-`true` half of the window in the sibling comment block, which described only the false-`false` direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent 5d25d9e commit 5ccd0ad

1 file changed

Lines changed: 94 additions & 14 deletions

File tree

  • packages/services/service-automation/src

packages/services/service-automation/src/engine.ts

Lines changed: 94 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5580,18 +5580,88 @@ export class AutomationEngine implements IAutomationService {
55805580
}
55815581
}
55825582
const durationMs = Date.now() - run.startTime;
5583-
const logged = this.recordLog({
5584-
id: runId,
5585-
flowName: run.flowName,
5586-
flowVersion: run.flowVersion,
5587-
status: 'completed',
5588-
startedAt: run.startedAt,
5589-
completedAt: new Date().toISOString(),
5590-
durationMs,
5591-
trigger: buildRunTrigger(context),
5592-
steps,
5593-
output,
5594-
}, context);
5583+
// [#15944] THE RUN IS OVER AND IT SUCCEEDED. Everything from
5584+
// here to the return is BOOKKEEPING ABOUT that fact, and the
5585+
// `catch` below this `try` exists for NODE failures — so a
5586+
// throw out of the history write was handled as though a node
5587+
// had thrown: the arm journalled a repair snapshot, stamped
5588+
// `status: 'stranded'`, answered `success: false`, and
5589+
// {@link restoreConsumedSuspension} then honoured that
5590+
// snapshot and re-armed the pause, so the NEXT resume RE-RAN
5591+
// every node after it. Measured: `tail` ran twice.
5592+
//
5593+
// The guard restores the invariant `recordLog`'s own doc
5594+
// states two screens down — "a history write must NEVER block
5595+
// or break the run that produced it" — which that call was
5596+
// relied upon to keep and did not.
5597+
//
5598+
// Two statements inside `recordLog` reach here on the terminal
5599+
// path, and neither is hypothetical: `store.recordTerminal`,
5600+
// whose SYNCHRONOUS throw escapes because the
5601+
// `void write.catch(...)` beneath it only ever sees a returned
5602+
// promise's rejection (both shipped stores are `async` and
5603+
// cannot; the interface is exported, optional, and
5604+
// host-implementable, and a store returning a non-thenable
5605+
// makes `write.catch` itself a synchronous TypeError), and the
5606+
// run-summary line `this.logger.info(line, meta)`, on by
5607+
// default and calling a HOST-INJECTED logger.
5608+
//
5609+
// ⛔ NOT a widening of anything: no exit gains a status it
5610+
// did not have, the node-failure arm below is untouched, and
5611+
// the failure is REPORTED rather than swallowed — see the
5612+
// catch. ⛔ And ⛔ deliberately not fixed at
5613+
// `restoreConsumedSuspension`: that verb judged correctly on
5614+
// the evidence it was handed; the evidence is what was wrong,
5615+
// and no journal is written for a completed run at all now.
5616+
let logged: ExecutionLogEntry | undefined;
5617+
try {
5618+
logged = this.recordLog({
5619+
id: runId,
5620+
flowName: run.flowName,
5621+
flowVersion: run.flowVersion,
5622+
status: 'completed',
5623+
startedAt: run.startedAt,
5624+
completedAt: new Date().toISOString(),
5625+
durationMs,
5626+
trigger: buildRunTrigger(context),
5627+
steps,
5628+
output,
5629+
}, context);
5630+
} catch (bookkeeping) {
5631+
// #4632 verdict: DURABILITY, so `error` — the caller is
5632+
// told the truthful thing (the run completed), which is
5633+
// exactly what makes the rest invisible from the outside:
5634+
// the terminal history row never landed, nothing retries
5635+
// it, and no envelope carries a word about it. After the
5636+
// next restart the run is invisible to the Runs surfaces
5637+
// and the approvals sweeps read the hole —
5638+
// `inspectStrandedRequests` reads "no suspension + no
5639+
// terminal row" as a STRANDED request, and
5640+
// `releasePendingForTerminalRuns` reads "no terminal row"
5641+
// as still-alive. Consequence and fix in the first line,
5642+
// per AGENTS.md. Said ONCE per run, not once per failed
5643+
// write.
5644+
//
5645+
// THIRD argument per `error(message, error?, meta?)`; the
5646+
// `Error` slot stays empty on purpose (#5575), and the
5647+
// thrown text goes to the structured slot rather than into
5648+
// the message (#6499).
5649+
this.logger.error(
5650+
`[Automation] run '${runId}' of flow '${run.flowName}' COMPLETED successfully but its ` +
5651+
`run-history bookkeeping threw, so its terminal history row never landed — nothing ` +
5652+
`retries it, the caller is told the run succeeded, and after the next restart this ` +
5653+
`run is invisible to the Runs surfaces while the approvals sweeps read it as ` +
5654+
`stranded and never-finished. The run itself is COMPLETE and must NOT be repaired ` +
5655+
`or re-run. Fix the history failure in this record's meta.`,
5656+
undefined,
5657+
describeThrownForLog(bookkeeping),
5658+
);
5659+
}
5660+
// [#15944] Recomputed when the guard above had to abandon
5661+
// `recordLog`: the same pure function of the same steps that
5662+
// `recordLog`'s own first statement runs, so the two spellings
5663+
// cannot disagree. Same shape as the strand arm's below.
5664+
const summary = logged?.summary ?? summarizeRun(steps);
55955665

55965666
// ── Subflow up-bubble (nested pause): this run was a subflow
55975667
// child whose parent suspended awaiting it. Auto-resume the
@@ -5600,7 +5670,7 @@ export class AutomationEngine implements IAutomationService {
56005670
// continues the parent itself). Best-effort: the child's own
56015671
// completion stands even if the parent continuation fails.
56025672
if (!skipBubble) {
5603-
await this.bubbleToParent(run, output, logged.summary);
5673+
await this.bubbleToParent(run, output, summary);
56045674
}
56055675

56065676
// Surface the flow's friendly completion message so a screen-flow
@@ -5612,7 +5682,7 @@ export class AutomationEngine implements IAutomationService {
56125682
output,
56135683
durationMs,
56145684
successMessage: flow.successMessage,
5615-
summary: logged.summary,
5685+
summary,
56165686
};
56175687
} catch (err: unknown) {
56185688
// Re-suspended at a downstream node: persist a fresh continuation.
@@ -5687,6 +5757,16 @@ export class AutomationEngine implements IAutomationService {
56875757
// worse than silence, and it is the opposite of the direction
56885758
// everybody checks for.
56895759
//
5760+
// [#15944] THE SAME TWO STATEMENTS HAVE A SECOND CONSEQUENCE,
5761+
// and it is the direction everybody DOES check for. On the
5762+
// COMPLETION path above, a throw out of the same `recordLog`
5763+
// fell into this arm on a run whose nodes ALL succeeded — a
5764+
// false `true`: a journal and a `stranded` stamp for a run
5765+
// that finished, whose "repair" re-armed it and re-ran every
5766+
// node after the pause. That half is guarded at its own site
5767+
// (see the completion path), NOT here: this arm stays the
5768+
// node-failure arm, and reaching it at all was the defect.
5769+
//
56905770
// ⛔ This is NOT "assume repairable when the failure is
56915771
// unknown" — that would invert the honest default and promise
56925772
// a repair for a lost run. The guard opens AFTER the journal,

0 commit comments

Comments
 (0)