fix(service-automation): a failed run is still answered in the declared shape when its own history write throws - #17583

Fixes #17562

Both initial-execution paths ended their node-failure catch with an unguarded recordLog({ status: 'failed' }). That catch is the handler for node failures and there is no outer one, so a throw out of the history write escaped the method entirely and left execute() a rejected promise, where its declared return type is an AutomationResult. This is the FAILURE-arm half of PR #17565 (which guarded the COMPLETION arms of these same two paths), and the same shape PR #15555 landed on resumeInternal's failure arm.

The premise held, and it was re-measured rather than inherited

PR #17565's guard markers sit on the completion arms (engine.ts:4868, :4965, :10246, :10300); its author annotated this card's site while working, at :4904, naming the exact case — the catch arm's own recordLog({ status: 'failed' }) threw again. So the site was still open on the post-merge tree, and the card's own prediction is confirmed.

The card's reproduction, re-driven on origin/main @ 216b06682 before a line was written — the identical flow (start -' work -' end, work throws) and the identical node failure, differing only in the store:

store = SYNC-THROW        -> {"kind":"threw","error":"run-history driver refused the terminal row"}
store = HEALTHY (control) -> {"kind":"returned","status":"failed","error":"work blew up","errorMessage":"author failure text","summary":{"selected":0,"acted":0,"skipped":0,"unmeasured":0,"failed":1, ...}}

The control is what makes it a reading: the same node failure against a healthy store returns the declared envelope carrying the NODE's own text. Only the store differs.

What is lost is the SHAPE, not the verdict

The run really did fail, so nothing misleads an operator: no false failed, no double run — which is why this is a narrower defect than #16274's and still a contract violation. A caller that branches on { success: false, status: 'failed' } gets an exception instead, so the transport's status arm (#9378) is bypassed, errorMessage (#9414) and summary (#4354) never arrive, and the node's own error text — the one thing the caller needed — is replaced by the history driver's. A REST route or SDK caller sees a 500-class throw for a run that had a perfectly good failure envelope waiting.

The second site is reached the OPPOSITE way round from #16274's

executeWithoutRetry() is only ever entered from retryExecution(), which execute()'s catch reaches after its own failed row. So with a store that refuses every terminal write, execute() rejects first and the second site is never reached — which is why a store that starts refusing mid-run (the first row lands, the driver's connection then drops) is what puts the throw in that arm and nowhere else. The pin is written that way, so it measures the second site independently of the first rather than through it. Unpatched, that throw rejected out through retryExecution and execute() both, and took the remaining retry budget with it: two node runs instead of three.

What changed

1. Each failure-path recordLog is guarded at its own call site, the summary is recomputed with the same pure function (logged?.summary ?? summarizeRun(steps)), and the swallowed failure is reported once at error with its consequence and its remedy. The shape PR service-automation: a throw between journalConsumedSuspension and the status: 'stranded' stamp makes a REPAIRABLE strand report repairable: false — the operator is told not to attempt a repair that works #15555 landed, twice over; no third design.
2. ⛔ No catch arm's meaning is widened. The suspend arm, the InputSchemaViolationError refusal and the strategy: 'retry' branch are untouched; a genuine node failure still records failed, still carries the node's own text, and is still retried the full 1 + maxRetries times — pinned by two controls.
3. No scripts/** edit was owed, exactly as the dispatch predicted. Verified rather than assumed: see the gate section.

The level question, verified on today's tree rather than inherited

The remedy reports at error, and the standing #13398 ruling forbids raising a site to error where that means growing an optional error? onto a published sink that lacks it. Measured in one read of packages/spec/src/contracts/logger.ts (read-only to this lane, untouched by this diff):

• error(message: string, error?: Error, meta?: Record...): void is declared at line 37 with no ? before its parameter list — a REQUIRED member. Nothing is grown; no sink type changes in this diff.
• ⚠️ One correction to the inherited reading. Both PR fix(service-automation): a completed run is no longer answered failed — or re-executed under strategy: retry — by its own history write #17565's body and the dispatch say "only fatal?, child?, withTrace? are optional". On today's tree the optional members are five, not three: fatal? (45), child? (51), withTrace? (58), log? (65), destroy? (71) — counted mechanically (5 optional vs 4 required: debug, info, warn, error). The load-bearing half is unchanged — error is among the REQUIRED four — so the conclusion stands, but the enumeration it was stated with was incomplete. A fence inherited is an assertion about code as it was.
• Corroborated mechanically: pnpm check:optional-error-sink exit 0.

There is a second reason to state explicitly, because this site looks like the rule's third legal answer ("a failure handed to the CALLER is not a degradation at all"). It is not: what is handed to the caller is the node's failure. The bookkeeping failure is handed to nobody — the envelope, the HTTP arm and every counter read clean while sys_automation_run has no row — so the durability question answers YES and error is right. That is also why recordLog sits in DURABILITY_CRITICAL_CALLEES.

Verification

The reproduction, driven before a line was written (origin/main @ 216b06682, both legs in one process):

sync_throw             {"kind":"threw","error":"run-history driver refused the terminal row"}
healthy_control        {"kind":"returned","status":"failed","error":"work blew up","errorMessage":"author failure text","summary":{"selected":0,"acted":0,"skipped":0,"unmeasured":0,"failed":1,"nodes":[...],"gates":[]}}

Ablation — RED before GREEN, proven by on-disk state. The fix was committed first, then ablated out of committed state (git checkout 216b06682 -- packages/services/service-automation/src/engine.ts), with the mutation proven by hash AND by occurrence counts on the exact text touched, and restored by state:

HEAD blob       : 50fb3b3cae8c38de64ae0601f0025760ef915135
on-disk BEFORE  : 50fb3b3cae8c38de64ae0601f0025760ef915135   (equal -> the tree really was at HEAD)
counts BEFORE   : marker [#17562]=4   catch (bookkeeping)=6   summary-recompute=6
on-disk AFTER   : 8223765305ab4f72b82329c3d1611d9a9addf46c   (= the pre-fix blob; != BEFORE -> not a no-op)
counts AFTER    : marker [#17562]=0   catch (bookkeeping)=4   summary-recompute=4
restore         : git checkout HEAD -- THE FILE ; `git diff HEAD` EMPTY, `git status --porcelain` EMPTY,
                  on-disk hash back to 50fb3b3cae8c38de64ae0601f0025760ef915135

The script carried trap restore EXIT INT TERM with an absolute git rev-parse --show-toplevel path, and an empty hash was treated as FAILURE rather than as "nothing to compare".

The direction OBSERVED (the predicted one, turning red): 5 pins failed, 2 controls passed — the controls passing on the ablated tree is correct, because they assert behaviour this change does not move. Quoted from the run:

PIN 1    AssertionError: a history write must never break the run that produced it: expected 'threw' to be 'returned'
PIN 3    AssertionError: the declared contract is a result, not an exception: expected 'threw' to be 'returned'
PIN 4    AssertionError: said ONCE per abandoned write: expected +0 to be 1
CONTROL  AssertionError: the defect leg: expected 'threw' to be 'returned'

⚠️ Whether a leg resolved through exports into dist/ — measured, not assumed, and the answer is the opposite of PR #17565's. packages/services/service-automation/dist did exist at ablation time (the script printed dist present? : YES; the CI-shaped build below had already run), and the verdict flipped anyway. The pin imports ./engine.js relatively, so vitest resolves it to the TypeScript source rather than through the package's exports; the mutation changing the verdict with a populated dist/ beside it is the proof that source is what ran. Restore leg on the restored tree: 7/7 green.

Runs — every heavy run through scripts/pm/os-verify-lock.sh, quoting its own verdict line. ⚠️ These are SHARED-BOX seconds, as the wrapper says; they exclude other locked runs only.

⚠️ One reading worth recording: turbo is not on the lock shell's PATH, so the CI-shaped build first came back exit 127 — a command that never started, not a red build. Re-run as pnpm exec turbo … to a real 0.

Gates — derived from the ACTUAL diff with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, every derived command run, and reconciled with --ran carrying an exit code for each:

Run reconciliation — 61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 61 derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED
  (a DERIVED zero — all 61 recorded an exit code and none of them is 3).

Two were earned rather than claimed, and ⛔ neither was rounded up:

• pnpm check:dual-build-cjs-loads first returned exit 3 = PREREQUISITE NOT MET = NOT MEASURED ("this gate reads built output, and some package has no dist/ … ⛔ This is NOT a pass: nothing was measured"). The prerequisite was satisfied with the CI-shaped build and the gate re-run to a real exit 0.
• pnpm check:type-check-debt likewise: exit 3 -> prerequisite built -> exit 0 (5 ledger entries re-measured, 55 raw tsc errors, none above its recorded number).

Seven families were run beyond the 61, because the derivation marks their silence as a fact about a list rather than about these paths — the 4 artifact rosters that sit in a directory one of these paths is in (check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity) and the 3 declared-wide families that actually bear on this diff (check:durability-log-level, check:optional-error-sink, check:swallow-census-controls). All exit 0.

The scripts/** question, answered mechanically. pnpm check:durability-log-level judged the two new seams with no gate-script edit at all, exactly as the dispatch predicted:

✓ durability-degradation log levels: 35 durability-critical catch seam(s), all loud, rethrowing
  or propagating to the caller (4 propagating, declared)

35, against the 33 PR #17565 left behind — the two new guards, discovered through the recordLog vocabulary that PR added and judged LOUD. check:swallow-census-controls stayed green too (21 copied gate-vocabulary names still match the gate's declaration), so no by-value copy drifted.

docs-drift — answered on the tree the bot names, and hand-searched blind to the emitter.

# 0 docs name something this change touched (1 anchor(s) — 2 symbol, 0 route, 0 sdk,
  0 literal, 0 command, 0 rule) across 1 changed package(s) since origin/main
  (1 test file(s) excluded — tests cannot make an implementation doc stale)

Zero rows, so there is nothing to answer row by row. The emitter-blind hand search over content/docs, with its wc -l and a firing control, because a page stating this rule by its inputs shares no identifier with the emitter:

The six AutomationResult pages were read: references/automation/flow.mdx (twice, generated) documents successMessage / errorMessage as "carried on AutomationResult for every terminal run" — a claim this change makes more true, not stale, since those two fields previously went missing exactly when the history write threw. No page documents execute() rejecting, and no page states the run-history best-effort rule. No docs edit is owed.

NOT MEASURED, each with its reason — all CI-owned, none a skipped local obligation: the 5 path-scheduled CI jobs whose 27 steps have no local invocation (Build Core, the Test Core shards, the two Dogfood jobs), the families whose argv takes a value only a CI run supplies (check-shard-attestation, check-test-completeness), and the 8 of the 11 declared-wide families whose populations are unrelated roots.

Clause-②: no

Both limbs measured against the diff rather than recalled. ① Accept/reject behaviour — no Zod schema, parse path or accept set is touched, and AutomationResult.status already declares 'failed', so this change makes the declared envelope REACH the caller rather than minting anything. ② Public surface — no exported symbol, no new error code, no new key on any published payload; the change is a try/catch around an existing call plus one logger.error, and summarizeRun / describeThrownForLog / ExecutionLogEntry were all already in the file. No scripts/** edit. This matches the seat's claim-time declaration. node scripts/pm/check-clause2-carriers.mjs --pair 17583 — exit 0: the clause-② declaration is readable in the fixed spelling and both carriers agree, and its diff carries no widening tell. ⛔ Not judged with a hand-rolled regex: the gate's own readClause2Line read it.

⛔ Untouched, as the card and the dispatch both fence: resumeInternal's already-guarded sites, the COMPLETION arms (#16274), restoreConsumedSuspension, inspectStrandedRequests (#15358), and packages/spec/** (read-only).

Authored by Claude Code in session session_01ToDPcx9AESFubJkDiFMtKW — attribution in prose deliberately, because a PR body's trailing rule line plus footer block is eaten by the platform on write while the call still reports success.

Generated by Claude Code

Generated by Claude Code