service-automation: the two INITIAL-execution completion paths still call `recordLog({ status: 'completed' })` inside the node-failure `try` — a history-write throw reports a finished run as `failed`

[os-warren]

Found while implementing #15944 (PR #16273). ⛔ Deliberately not fixed there: that card and its dispatch scope the repair to resumeInternal's completion path, and these are the initial-execution paths with a different consequence. Filed bare — domain:*, type and priority are triage's.

⚠️ Structure is measured; the consequence is NOT. I read the shape on origin/main and did not drive it. That distinction is the point of filing rather than asserting.

The shape

#15944 was the resumeInternal instance of one pattern: a completion-path recordLog({ status: 'completed' }) called from inside the try whose catch exists for node failures, so a throw out of a history write on a run that already finished is handled as though a node had thrown. PR #16273 guards that one site.

The same structural shape survives at two more completion sites in packages/services/service-automation/src/engine.ts, both on the initial-execution path (line numbers on 48427620a, which includes #16273's guard — re-locate before trusting them):

site	completion recordLog	its enclosing catch	path
execute()	:4466, status: 'completed' at :4470	:4510 } catch (err: unknown) {	first execution of a flow
the retry path	:9100, status: 'completed' at :9104	:9122 } catch (err: unknown) {	a RETRY attempt (#9510)

The two statements that reach those catch arms are the same ones #15944 names, and neither is an in-repo surface:

• store.recordTerminal(record) when it throws synchronously — the void write.catch(...) beneath the call only ever sees a returned promise's rejection. (A store returning a non-thenable escapes identically: write.catch is then a synchronous TypeError.) Both stores shipped in this package are async methods and cannot; SuspendedRunStore is an exported interface with an optional recordTerminal, so a host store is unconstrained.
• The run-summary line logger.info(line, meta), on by default (runSummaryLog: 'info') and calling a host-injected Logger. This one needs no store at all.

Why this is NOT a duplicate of #15944, and why it is milder

Neither of these paths has consumed a suspension, so none of #15944's sharp consequences are reachable here: no journalConsumedSuspension, no status: 'stranded' stamp, no restoreConsumedSuspension re-arm, and therefore no double run. What appears reachable instead is the narrower half of the same invariant violation — a run whose nodes all succeeded being answered failed, carrying the history driver's own text as the run's error, and being recorded failed in a history row (if that write is the one that landed).

recordLog's own doc comment states the invariant in the file: "Best-effort + fire-and-forget: a history write must NEVER block or break the run that produced it." On these two paths it can still break the run that produced it.

What is NOT measured, and would move a priority

• Whether a throw at either site actually reaches the generic failure arm rather than an earlier branch — the arms test isSuspendSignal(err) first, and the retry path has its own additional structure (bug(service-automation): a retry attempt that PAUSES is recorded as failed and its suspension is never persisted — executeWithoutRetry has no isSuspendSignal arm #9510).
• What the caller is finally told in each case, and whether anything downstream distinguishes it from a genuine node failure.
• Whether the retry path can then re-attempt a run that already completed. This is the one worth driving first — if it can, the consequence is closer to service-automation: a run whose nodes ALL succeeded is journalled and reported stranded when its terminal history write throws — and restoring it RE-RUNS the flow #15944's than the paragraph above assumes.

Driving it needs no new instrument: the doubles in completed-run-history-throw.test.ts (added by PR #16273) are a sync-throwing store and a throwing-info logger, and the flow only has to complete without pausing.

Suggested direction (advisory — the disposition is the lane's)

If it measures as described, the repair is the same shape PR #16273 already landed and #15555 landed before it: guard the completion-path recordLog at its own site, recompute the summary with the same pure function, and report the swallowed failure once at error with its consequence and fix. ⛔ Nothing here argues for widening any catch arm's meaning, and ⛔ nothing here bears on restoreConsumedSuspension or on inspectStrandedRequests (#15358).

Refs: #15944 (the resumeInternal instance, guarded in PR #16273) · #15555 (the failure-side guard on the same window) · #9510 (the retry path's own pause arm).