fix(automation): keep the stranded verdict when post-journal bookkeeping throws — a repairable strand must not report repairable: false

[os-warren]

Fixes #15555 — the throw window between the repair journal and the status: 'stranded' stamp, reproduced first and then closed.

The card reproduces, at the door and in the engine

resumeInternal's catch arm journals the consumed suspension — the snapshot restoreConsumedSuspension puts back — and only then stamps status: 'stranded'. Between them sit statements that throw out of the whole arm: recordLog's terminal run-summary line (logger.info, on by default), store.recordTerminal(record) whose synchronous throw escapes (the void write.catch(...) beneath it only ever sees a returned promise's rejection), and then await this.failAncestors(...).

So "a repair snapshot was journalled" and "the engine said stranded" were not one fact. Every consumer derives repairability from the second one — plugin-approvals computes its operator-facing flag as repairable = status === 'stranded' — so a throw in that window reported repairable: false about a run the repair verb recovers.

Measured at the pre-fix shape (the M1 leg below drives exactly it):

	pre-fix	with this change
resume, store's terminal write throwing	threw the store's error, no run-state discriminator at all	returns { success: false, status: 'stranded' }, error naming the node
restoreConsumedSuspension on that same run	restored: true	restored: true
the approvals decision door	repairable: false	repairable: true
the durable history row	never landed	never landed — the failure is real, not simulated away

The first and third rows together are what make this a false negative rather than a conservative default: the operator was told not to attempt a repair that succeeds. The usual worry is a false true; this is the other direction.

What repairable keys on, and why the journal — not the stamp — is the fact

The dispatch asked the question directly. The journal is what makes a run repairable; the stamp is a separate write that can be lost. This change makes the stamp key on the journal by construction: the guard opens on the line after journalConsumedSuspension returns, so only a run that demonstrably has a snapshot can reach the stamp.

⛔ Not "assume repairable when the failure is unknown" — that would invert the ruling's honest default and promise a repair for a lost run. A throw from the journal call itself still propagates, and every exit above the consumption point still carries no status at all (pinned).

⛔ And the journal is not moved down beside the stamp instead, the card's other suggested shape. recordLog is what carries the snapshot into the durable row, so journalling after it would leave a recordLog failure with no snapshot anywhere — turning this false negative into a true one by destroying the repair rather than by reporting it.

What this changes

One window in resumeInternal's catch arm. The bookkeeping may still fail; it now says so at error, naming the run, what did not land, and the verb that repairs the strand — while the verdict ships. Consequence and fix in the first line, said once per stranded run, per AGENTS.md's durability rule; the Error slot stays empty and the thrown text goes to the structured slot.

⛔ No contract moves. AutomationResult and ApprovalDecisionResult are untouched — status: 'stranded' is the existing #13937 discriminator on the exit that already produced it. ⛔ Cascade-failed ancestors are untouched: they go through failSuspendedRun, journal nothing, and stay correctly non-repairable.

Pins, and both mutation legs

New: packages/services/service-automation/src/strand-verdict-post-journal-throw.test.ts (5 cases — the window driven through both statements that reach it, the loudness of the swallowed failure, and two controls). Extended: decision-strand-envelope.test.ts PIN 4, which drives the window through the real ApprovalService.decide door.

Every leg proved on disk by a git hash-object delta plus marker counts, restored with git checkout HEAD -- ABSOLUTE_PATH under a trap … EXIT INT TERM, and each restore proved by an empty git diff HEAD and the blob back at HEAD's e6fa6ff8f. The M1 leg rebuilt dist/ and proved the mutation reached the artifact (ablation-dist-preflight … --absent, exit 0), because the door suite resolves @objectstack/service-automation through exports to dist/.

leg	engine.ts blob	marker	engine suite	door suite
GREEN	e6fa6ff8f (= HEAD)	1	exit 0 — 5 passed	exit 0 — 4 passed
M1 guard removed (the pre-fix shape)	976d8ec78	1 → 0	exit 1 — 3 failed / 2 passed (PIN 1, 2, 3)	exit 1 — PIN 4, repairable: expected false to be true
M2 guard kept, its logger.error deleted	2dc4b81eb	1 → 0	exit 1 — 2 failed / 3 passed (PIN 2, 3)	not run
M3 the summary fallback removed	09f67bd25	1 → 1	exit 1 — 2 failed / 3 passed (PIN 1, 2)	not run

M1 is the reproduction, re-taken mechanically: it reddens the pins and only the pins, and it is the leg that shows repairable: false at the door. M2 shows the loudness half is pinned separately from the verdict half. M3 shows the second hunk is load-bearing. Both controls stay green under all three.

⛔ What this does NOT do, and one finding it surfaced

repairable stays a point-in-time fact. This does not make it durable, and the changeset says so: in the driven case the terminal history row is exactly what failed, so the repair rides on the in-memory journal and a restart loses it. That residual is recorded on the card as explicitly not the card, and the wording nit it implies belongs to PR #15436.

A separate defect, in the opposite direction, is filed as #15944 rather than folded in here. The completion-path recordLog sits inside the same try, so a history-write throw on a run whose nodes all succeeded is handled as a node failure: it journals a repair snapshot, reports stranded, and restoreConsumedSuspension then re-arms a completed run — measured re-running the downstream node a second time, which crosses #13937's "never double-runnable" invariant.

⚠️ Stated plainly because it interacts with this PR: that phantom is pre-existing. The M1 leg measured it on the pre-fix tree and the reading is byte-identical there — a store refusing only the completed row already answered status: 'stranded' with restored: true before this change. What this change alters is only which store failures report the phantom instead of throwing over it. #15944 carries both trees' readings and an advisory direction; ⛔ nothing here settles it.

#15358 is adjacent and untouched. It is the same over-reporting direction on the inspectStrandedRequests sweep, and #15944 is the engine-side producer of the same class. ⛔ Neither is decided here. ⛔ Nothing here folds in #15221 either.

Verification

Every exit code captured right after a single redirected command (cmd > log 2>&1; EXIT=$?), never through a pipe. All of it re-run on the final commit, after merging origin/main.

• Suites, on closures rebuilt after the merge: @objectstack/service-automation 114 files / 1382 tests passed; @objectstack/plugin-approvals 38 files / 672 tests passed. An earlier approvals run reported one file failing to load — Failed to resolve entry for package "@objectstack/trigger-record-change", an unbuilt dependency of that suite, not a test verdict; building the approvals closure cleared it, and nothing red remains to attribute.
• Typecheck: both packages, exit 0. Recorded precisely: plugin-approvals's base tsconfig.json does not include decision-strand-envelope.test.ts (checked with --listFiles), so that leg says nothing about it; the third leg of the package's typecheck script does — check:test-typecheck compiles the test layer under tsconfig.test.json, exit 0. service-automation's base config does reach the new test file (--listFiles, 1 hit).
• Gates, family re-derived from the actual changed files with scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (exit 0, no stale-tree warning after the merge). Run locally, all exit 0: check:nul-bytes · check:durability-log-level (29 seams, all loud) · check:engine-double-contract (789 pinned / 133 debt / 3 exempt; no new double to pin, the added store subclass is not one) · check:cross-package-test-inputs · check:test-source-alias · check:changeset-gate-self-tests · check:doc-authoring · check:logger-receiver-detach · check:optional-error-sink · check:resume-authority-declared · check:error-code-casing · check:partof-closing-keyword.
• node scripts/check-adr-0087-registration.mjs --base origin/main --head e9ee5a226 → exit 0, with --self-test → exit 0 as its control in the same session.
• ⚠️ Declared narrowing: the derived family lists roughly sixty check names; the twelve above are the ones this diff actually touches. The rest — and the repo-wide pnpm lint sweep — are left to CI, which runs the farm exactly once regardless. That is a narrowing, stated, not an omission.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 97afcb5ccc061eb271cb37fda3bc468459eac7c2 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2082feb6bcb6102fbc14b63783c4df538125d87d — the merge of head 6dd2f40a0f6aa5eed180b018d9f36716829152ff into base 97afcb5ccc061eb271cb37fda3bc468459eac7c2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2082feb6bcb6102fbc14b63783c4df538125d87d && git checkout 2082feb6bcb6102fbc14b63783c4df538125d87d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97afcb5ccc061eb271cb37fda3bc468459eac7c2 6dd2f40a0f6aa5eed180b018d9f36716829152ff && git checkout -B drift-repro 97afcb5ccc061eb271cb37fda3bc468459eac7c2 && git merge --no-ff 6dd2f40a0f6aa5eed180b018d9f36716829152ff

node scripts/docs-audit/affected-docs.mjs --json 97afcb5ccc061eb271cb37fda3bc468459eac7c2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

[os-warren]

Clause-② contract review — PR #15949 (card #15555): PASS

Tier. CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:9852 on this head). This review ran under an explicit model: fable override attested by the domain:services PM seat, and self-reports as claude-fable-5-1 — override + self-report, not an exact-match reading.
Independence. The dev was a separate os-dev subagent of the same PM session (the five commits carry that session's trailer); the dev line is the branch claude/issue-15555-strand-repairable-window, so this is not the self-review case.
Subject. head 6dd2f40a0, merge-base f1e91595f, the four named files. Own detached worktree /home/user/objectstack-review-15949; fresh pnpm install --frozen-lockfile --offline (exit 0) and turbo run build --filter=@objectstack/plugin-approvals... (exit 0, 23 tasks). Every exit code below was taken right after a single redirected command.
STALE TREE. origin/main was a2051fa4e at measurement time (6 commits ahead; git merge-base HEAD origin/main = f1e91595f). git diff --name-only f1e91595f origin/main = 26 files, 0 under packages/services/service-automation or packages/plugins/plugin-approvals; engine.ts blob at origin/main = merge-base = 3a44f8da7. The dev's check holds; staleness is immaterial to everything measured here.

1. The false negative, end to end at the real door — PASS (blocking bar)

GREEN at HEAD: door suite decision-strand-envelope.test.ts exit 0, 4 passed — PIN 4 drives ApprovalService.decide against a SyncThrowTerminalStore and asserts details.repairable === true, then restoreConsumedSuspension → restored: true, hasSuspendedRun true again, on the same run. Engine pins exit 0, 5 passed; PIN 1 additionally asserts store.loadTerminal(runId) falsy, so the secondary failure was real.
PRE (engine.ts = merge-base blob 3a44f8da7, dist/ rebuilt): door exit 1, 1 failed / 3 passed — PIN 4 a journalled strand is repairable even when its bookkeeping failed: expected false to be true.
All three legs together: resume returns { success: false, status: 'stranded' } on HEAD vs threw on PRE; door repairable true vs false; restoreConsumedSuspension restored: true on both trees. That is the false negative, measured at the door and not a stub: plugin-approvals/vitest.config.ts aliases only metadata-core and metadata-protocol, and check-test-source-alias.mjs lists plugin-approvals → service-automation as unaliased, so the suite consumes dist/.

2. Guard placement — the stamp keys on the journal, in both directions — PASS (blocking bar)

By reading: the try opens on the statement after journalConsumedSuspension returns (engine.ts 5517 → 5556); every refusal above forgetSuspendedRun (refuseInvalidScreenInput, INVALID_SIGNAL, claim lost / unavailable) returns before it.
By measurement — my boundary-15949-review.test.ts (4 cases; green on HEAD, and green on PRE with the one tree-dependent reading flipped):

• B1 the journal itself throws (monkeypatched): resume throws, no status; restore refuses NO_CONSUMED_SUSPENSION; no guard log. Both trees.
• B2 a completed run: no status; restore refuses RUN_COMPLETED. Both trees.
• B3 cascade on a healthy store (real registerSubflowNode): child stranded and restorable; parent pause consumed, restore refuses NO_CONSUMED_SUSPENSION. Both trees.
• B4 over-reach probe — a store refusing only the parent's row (the write inside failAncestors): HEAD → child returns stranded, error names the child node not the driver, exactly 1 guard log naming the child run with the driver text in meta; PRE → the child's resume throws the parent's row failure, 0 logs. Both trees: child restores true; parent hasSuspendedRun false, restore refuses NO_CONSUMED_SUSPENSION, loadTerminal(parent) falsy. The guard does not make a never-journalled run repairable.
  Plus the PR's own CONTROL (INVALID_SIGNAL → no status, restore RUN_SUSPENDED).

3. The synchronous-throw claim — PASS (blocking bar)

recordLog (engine.ts 6710–6717): const write = this.store.recordTerminal(record); … void write.then(…); void write.catch(…). A synchronous throw leaves write unassigned, so neither handler is ever attached and the throw exits recordLog — and, pre-fix, the whole arm. The run-summary this.logger.info(line, meta) at 6655 is the second unguarded statement in the same path. Measured: PRE PIN 1 expected 'threw' to be 'returned'; GREEN PIN 1 loadTerminal falsy. The window is where the PR says it is.

4. Ablation legs — PASS; the mutation blobs match the dev's

leg	engine.ts blob	marker	engine pins	door
GREEN	e6fa6ff8f (= HEAD)	1 in src, 1 in dist/index.js	exit 0 — 5 passed	exit 0 — 4 passed
M1/PRE — full revert to the merge-base blob (the pre-fix shape by definition; a superset of the dev's 976d8ec78 guard-removal)	3a44f8da7	1 → 0 (src and dist)	exit 1 — 3 failed (PIN 1, 2, 3) / 2 passed	exit 1 — PIN 4 repairable false
M2 — guard's logger.error deleted (anchor regex unique, catch (bookkeeping) kept)	2dc4b81eb = dev's	1 → 0	exit 1 — 2 failed (PIN 2, 3: expected +0 to be 1) / 3 passed	—
M3 — logged?.summary ?? summarizeRun(steps) → logged!.summary (anchor count 1)	09f67bd25 = dev's	1 → 0	exit 1 — 2 failed (PIN 1, 2: 'threw') / 3 passed	—

The preflight is what it claims. scanDist greps every non-sourcemap file under the resolved package's dist/ for the literal; verdict needs ≥1 hit in present mode and 0 in --absent; treeVerdict runs git status --porcelain -z and, on a restore leg, reds any dirty path that does not carry the marker. Readings: M1 --absent exit 0 — "marker absent from all 6 built files", tree classified mutate leg (4 paths differ, 1 carries the marker: engine.ts plus my three untracked scratch tests). Restore leg present exit 0 — "marker present in 2 built files … working tree clean against HEAD" (scratch moved aside for that reading, since it rightly counts unaccounted dirt as RED there); --self-test exit 0. Hand-counted grep -c on dist/index.js: 1 → 0 → 1. Door after the restore rebuild: exit 0, 4 passed.
Every restore: git checkout HEAD -- <absolute path> under trap … EXIT INT TERM, then the blob back at e6fa6ff8f and git diff HEAD empty.

5. The reproduction, run unfixed — PASS

The pin file at its reproduction commit (a29f75465, blob b291868ad, staged under a -15949-review name) on the PRE tree: exit 1, 4 failed / 1 passed, headline resume must REPORT the strand, not throw the secondary failure. The fourth failure is that commit's "widens nothing" CONTROL, which re-resumed the same run through the throwing store on the completion path — exactly the #15944 territory; HEAD's version drives a healthy store and says why. HEAD's pin file on PRE: 3 failed / 2 passed.
PIN 3 is not a tautology. On PRE it reads expected +0 to be 1 — zero error logs pre-fix, so the silence was measured. M2 reddens PIN 3 (and PIN 2) while leaving PIN 1's verdict half green, so the loudness is pinned separately from the verdict; the clean-strand CONTROL (errors empty on a healthy store) shows the engine does not log error on every strand.

6. The changeset and the phantom disclosure — PASS

Standing guard: no unqualified "unchanged / as before / in both directions". The two "still" clauses name what they claim (the verdict ships; a journal throw propagates; exits above the consumption point carry no status; cascade-failed ancestors "which journal nothing" stay non-repairable) — each measured in §2. The durability caveat is accurate: PIN 1's run has no terminal row and the repair rides the in-memory journal.
The #15944 disclosure is accurate, measured with phantom-15944-15949-review.test.ts on both trees, flow start → hold → tail (succeeds) → end:

• store refusing only the completed row: resume returns { success: false, status: 'stranded', error: '…refused the COMPLETED row' }, restoreConsumedSuspension → restored: true, re-resume runs tail a second time — identical on PRE and HEAD;
• store refusing every terminal row: PRE threw, HEAD returns stranded; on both trees restore → true and tail runs twice.
  So the phantom, its repair snapshot and the double run all pre-exist; this PR changes only whether the all-rows case reports the phantom rather than throwing over it. Nothing here creates the hazard. 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 stays open and is not settled here.

7. Scope — PASS

The file set is the four named; nothing under packages/spec/; AutomationResult and ApprovalDecisionResult untouched. status: 'stranded' is the existing #13937 discriminator on the exit that already produced it. The door's status code and finalized are untouched (PIN 4 asserts both), so the #15556 A/B/C/D ruling is not pre-empted.

Regression control (not asked; cheap once built)

At HEAD after all restores: @objectstack/service-automation 114 files / 1382 passed (exit 0); @objectstack/plugin-approvals 38 files / 672 passed (exit 0).

Non-blocking observations

• B4 on HEAD: when failAncestors throws after failSuspendedRun has consumed the parent's pause, the parent is left with no pause, no journal and no row — correctly non-repairable, and named by the guard's "and/or its subflow ancestors were not failed". That condition pre-exists; the PR only turns it from a throw out of the child's resume into one error line. Not this card.
• My M1 is the merge-base revert rather than the dev's 976d8ec78 guard-removal; the readings agree on every column.

Tree state. /home/user/objectstack-review-15949 at 6dd2f40a0: git status --short empty, git diff HEAD empty, engine.ts blob e6fa6ff8f; scratch tests removed from src/ (copies kept in the review scratchpad). No push, undraft, merge, or stash. Nothing NOT MEASURED.

Verdict: PASS.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

---

Generated by Claude Code

[os-warren]

PM sign-off — undrafted and armed

PASS, and this is the most thoroughly re-taken review of the session: the reviewer did not check the numbers, it rebuilt them.

The three things I most wanted independently established

The disclosure I required is accurate, and that was verified rather than accepted. I made the implementer add a paragraph stating that this change makes a pre-existing fault visible rather than creating it — a completed run journalled and reported stranded when its completed history row throws, whose repair re-runs the flow (#15944). ⇒ The reviewer wrote its own test and ran it on both trees: a completed-only-throwing store gives identical stranded + restored: true + double run on PRE and HEAD; an all-rows store threw on PRE and returns stranded on HEAD, double-running on both. ⭐ The PR exposes, does not create, the hazard — which is exactly the claim I would have held it for had it been wrong, and it is now measured rather than argued.

The guard does not over-reach, which was the way this fix could have been worse than the bug. Four constructed boundary cases, green on HEAD and PRE: a journal throw still propagates (NO_CONSUMED_SUSPENSION); a completed run refuses (RUN_COMPLETED); a cascade parent refuses (NO_CONSUMED_SUSPENSION); and with a store that throws only the parent row, the child still ships stranded with exactly one guard log while the parent stays non-repairable on both trees. ⇒ A run that never journalled does not come back repairable.

Two ablation blobs came out byte-identical to the implementer's — M2 2dc4b81eb, M3 09f67bd25. ⭐ That is a stronger result than agreeing totals: it means the mutations were reproduced exactly, not merely re-derived to the same verdict.

The subtle mechanism, confirmed two ways

The window exists because const write = this.store.recordTerminal(record) attaches its handlers only to a returned promise, so a recordTerminal that throws synchronously escapes the void write.catch(...) beneath it. Confirmed by reading and by the pre-fix pin answering 'threw'. If that had been wrong, the guard would be in the wrong place and everything above it would be measuring the wrong window.

⭐ And PIN 3 is not a tautology — it pins the absence of an error log, which is the assertion most likely to pass for free, and M2 (deleting the guard's logger.error) reds it. Pinning silence only means something when something can break the silence.

Method notes worth keeping

• The door suite genuinely consumes dist/ — no alias for service-automation, and the ledger lists the pair unaliased — so ablation-dist-preflight.mjs was load-bearing, not ceremony. The reviewer checked the preflight is what it claims: a real grep of dist/ plus whole-tree git status accounting, with --self-test exit 0, and hand-counted the dist marker 1 → 0 → 1 across mutate and restore.
• The STALE TREE warning was verified immaterial rather than merged away: 26 upstream files, 0 under either package, and origin/main's engine.ts blob equals the merge-base. ⭐ The implementer checked this rather than reflexively merging, and the reviewer re-checked it — the right handling of a warning that is usually, but not always, noise.
• Changeset carries zero unqualified stability claims, and volunteers the durability caveat that repairable stays a point-in-time fact.

Non-blocking, recorded not folded in

Boundary case B4: a parent whose cascade row throws after its pause is consumed ends with no pause, no journal and no row. Pre-existing; it is now reported at error instead of thrown. ⛔ Not this card's, and correctly not fixed here.

⛔ This does not close #15556. It is Part of: the log half ships, the door still answers full success over a stranded parent, and the A/B/C/D contract decision is the maintainer's.

Undrafted and auto-merge armed.

---

Generated by Claude Code