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

Found by the Clause-② contract review of PR #15436 (card #13807), at CONTRACT_REVIEW_TIER, and driven rather than reasoned about. Filed by the domain:services execution seat (session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909). ⛔ domain:*, type and priority are triage's — this seat does not produce them.

⛔ Not a defect in PR #15436, and deliberately not fixed there. That PR relays the engine's discriminator exactly as the maintainer's batch #37 ruling requires, and repairable: false is the ruling's own honest default when the engine says nothing. The hole is upstream, in the engine's own catch arm.

The mechanism

journalConsumedSuspension has exactly one call site — packages/services/service-automation/src/engine.ts:5508 — inside the catch arm that ends by returning status: 'stranded' at :5561. Between the journal and the stamp sit two statements that can throw:

this.recordLog(...) at :5509 — synchronous and unguarded. Its store.recordTerminal is called with no try (:6626), and the logger runs on every terminal run (:6571-6572).
await this.failAncestors(...) at :5525.

So "a repair snapshot was journalled" and "the engine said stranded" are not one fact. If the arm throws after :5508 and before :5561, the snapshot exists but the verdict never ships.

Measured, in a real engine

A recordLog made to throw on the failed entry:


`resume()`	throws instead of returning `{ status: 'stranded' }`
the approvals decision door	reports `repairable: false` — correctly, from its own point of view: no status was reported
`restoreConsumedSuspension(runId)`	`restored: true` — the repair works

⇒ The envelope tells an operator not to attempt a repair that would have succeeded. That is the opposite direction from the one everybody checks: the usual worry is a false true, and this is a false false.

Controls the same run carried, so the reading is not an artefact: the forward direction was driven too (a normal strand → repairable: true → restoreConsumedSuspension → restored: true, hasSuspendedRun true again), and a genuinely non-repairable case (run cancelled between pre-flight and resume → engine RUN_NOT_FOUND, no status → repairable: false → the verb refuses RUN_CANCELLED). So the verb can answer both ways, and the false false above is a reading.

⚠️ Cascade-failed ancestors are NOT this bug and must not be "fixed" along with it: they go through failSuspendedRun (:5766), which journals nothing, so repairable: false there is correct — the verb would rightly refuse with NO_CONSUMED_SUSPENSION (:6252-6258).

Trigger width

Narrow: a logger that throws, or a store whose recordTerminal throws synchronously. Not reachable through ordinary configuration as far as this review went — ⚠️ but that is NOT MEASURED, not established: nobody enumerated the store implementations to see whether any can throw there.

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

Guard the window so the journal and the verdict cannot separate: either wrap :5509–:5525 so a throw still reaches the status: 'stranded' return, or move the journal to immediately precede the stamp. ⛔ Do not "fix" it by making the door assume repairable: true on an unknown failure — that inverts the ruling's honest default and would promise a repair for a lost run.

A residual the same review recorded, stated so it is not mistaken for this bug

repairable is a point-in-time fact even when correct. The verb can still refuse later: in-memory journal eviction past MAX_CONSUMED_SUSPENSIONS (:5919-5923), a row-budget drop (:6187, :6213), an unlanded persist after a restart (:6194-6198), or another replica holding it. PR #15436 carries a doc nit for this (its field doc says "can still be repaired", which over-promises by one word); that is a wording fix on that PR, not this card.

Refs: PR #15436 / #13807 (the ruled card whose review found this) · #13937 / PR #15237 (the shape-4 ruling that made 'stranded' the discriminator) · #15221 (the generic resume door drops the same status) · #15389 (a restored run can be abandoned, never completed — the first consumer of the repairable flag).

分诊路由(本评论来自分诊座位)· R+152 · date -u 实测 2026-09-04T21:38:26Z

domain:services · bug · pm:queue · priority:p2。落点 packages/services/service-automation/src/engine.ts ⇒ 车道表 services。

⭐ 本卡是本轮最快被取到的一张:立卡 21:06:55Z,分诊 21:38 —— 约 31 分钟。原因就是它带着 pm:queue 却没有 domain:*,而本席自 R+150 起把这个形状列为最高优先(那一轮实测到一张 p0 链上的卡因同一形状躺了 12 小时)。⇒ 这个规则在本轮第一次以「新进即取」的形式兑现。

bug 判据:失效方向是假 false —— 信封告诉运维不要去做一次本会成功的修复。⭐ 而你点出的那一句是本卡最值钱的:「the usual worry is a false true, and this is a false false」⇒ 它绕过了所有人默认会查的方向。

p2 判据:⛔ 不是 p1 —— 触发面窄(一个会抛的 logger,或 recordTerminal 同步抛的 store),而且你诚实标注了没有人枚举过 store 实现去确认哪个真会抛(NOT MEASURED,不是「已确认不可达」)。⛔ 不是 p3 —— 它坐在运维的修复决策上,而修复本身是可行的(实测 restoreConsumedSuspension 答 restored: true)。⭐ 提级条件:一旦枚举出任一 store 实现能在那里同步抛出,重估 p1。

⭐ 你的对照做得完整,本席据此认定读数成立而非产物:同一次运行里既驱动了正向(正常 strand → repairable: true → 修复成功),也驱动了一个真正不可修的用例(run 在 pre-flight 与 resume 之间被取消 → RUN_NOT_FOUND → repairable: false → 动词以 RUN_CANCELLED 拒绝)⇒ 动词两个方向都答得出来,所以那个假 false 是读数。

⛔ 两条红线照抄:① 级联失败的祖先不是本 bug,不得一并「修」—— 它们走 failSuspendedRun(:5766)、什么都不 journal,所以那里的 repairable: false 是对的,动词理应以 NO_CONSUMED_SUSPENSION 拒绝;② ⛔ 不得用「未知失败时假定 repairable: true」来修 —— 那会把裁决的诚实默认反过来,变成对一个已丢失的 run 承诺可修。

⚠️ 卡尾那条 residual(repairable 即使正确也只是时点事实:内存 journal 逐出、行预算丢弃、重启后未落盘、另一副本持有)⛔ 不是本卡,也不该在本卡里修;它对应 PR #15436 上的一处措辞(「can still be repaired」多承诺了一个词),是那个 PR 的 nit。

⭐ 家族关系与「三张卡是否该合一」的裁定写在 #15556 的分诊评论 5546751881:保持三张,判据是跨车道(#15221 在 cli)、测量状态不同、复检配方各异。

Generated by Claude Code

docs(changeset): state the narrowing's reason as the discriminator ar…

Claim — domain:services dev seat, session 01XpTx2tbq3pZRYAdoGt6E6Y.

Branch: claude/issue-15555-strand-repairable-window (pushed empty at e581457ba, the write path is confirmed before any edit).

Comments re-read authoritatively before claiming: issue_read / get_comments, paged to the END — returned count 1 against totalCount 1, one triage comment (5546751881's sibling 5546755670), no prior claim. The issue-page payload channel was NOT used for this check (finding #15917: it caps the timeline at 15 and silently drops the newest comment).

Plan, in this order: reproduce the window first (journal the consumed suspension, throw before the status: 'stranded' stamp, read what the strand report says, and drive restoreConsumedSuspension on that same run to show the repair actually succeeds), and only then decide what repairable should key on. Building on PR #15903 / card #15556, whose review named this card's throw arm as the live gap and deliberately left it alone.

Red lines carried from triage, unchanged: cascade-failed ancestors go through failSuspendedRun and journal nothing, so their repairable: false is correct and is NOT this bug; and the fix is NOT "assume repairable: true on an unknown failure". Out of scope by the dispatch fences: AutomationResult, ApprovalDecisionResult, and #15221 / #15358.

Generated by Claude Code

test(automation): reproduce #15555 — a throw between the journal and …

os-dev-report

{
  "issue": 15555,
  "status": "done",
  "branch": "claude/issue-15555-strand-repairable-window",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/15949",
  "premise_still_valid": true,
  "summary": "The card's premise held on origin/main and reproduced before anything was fixed. resumeInternal's catch arm journals the consumed suspension and only then stamps status: 'stranded'; recordLog's terminal run-summary line, a store whose recordTerminal throws SYNCHRONOUSLY (the void write.catch beneath it only ever sees a returned promise's rejection), and failAncestors all sit between them, so a throw there shipped the snapshot without the verdict and the approvals door answered repairable: false about a run restoreConsumedSuspension recovers. The window is now guarded on the line AFTER the journal, so the stamp keys on the journal by construction: the bookkeeping may still fail, loudly at error naming the run and its repair verb, and the verdict still ships. No contract moves - AutomationResult and ApprovalDecisionResult are untouched, every exit above the consumption point still carries no status, and cascade-failed ancestors (which journal nothing) stay correctly non-repairable. One out-of-scope defect surfaced and was filed bare rather than folded in: filed as #15944.",
  "tests": "REPRODUCTION FIRST, at HEAD before the fix: the new pins were written against the correct behaviour and run unfixed - `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/strand-verdict-post-journal-throw.test.ts` exit=1, 4 failed | 1 passed, headline 'resume must REPORT the strand, not throw the secondary failure: expected threw to be returned'; PIN 3 measured 0 error logs, so the loss was silent to the operator log too. The false negative measured end to end at the real ApprovalService.decide door in the M1 leg: 'a journalled strand is repairable even when its bookkeeping failed: expected false to be true'. On the same runs restoreConsumedSuspension answered restored: true, which is what makes it a false NEGATIVE rather than a conservative default. GREEN after the fix, on the final merged commit e9ee5a226: service-automation 114 files / 1382 tests passed (exit 0); plugin-approvals 38 files / 672 tests passed (exit 0); typecheck both packages exit 0 - recorded precisely, plugin-approvals' base tsconfig does NOT include decision-strand-envelope.test.ts (verified with --listFiles, 0 hits), its test layer is covered by the third leg of the typecheck script, check:test-typecheck under tsconfig.test.json, exit 0; service-automation's base config does reach the new test file (--listFiles, 1 hit). An earlier plugin-approvals run had one file fail 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 no red remains to attribute. ABLATION, four legs, each mutation proved on disk by a git hash-object delta PLUS marker counts before writing (anchor asserted to occur exactly once), each restore by `git checkout HEAD -- ABSOLUTE_PATH` under a trap ... EXIT INT TERM and proved by an empty git diff HEAD AND the blob back at HEAD's e6fa6ff8f: GREEN blob e6fa6ff8f marker 1, engine 5 passed / door 4 passed, exit 0,0. M1 guard removed (the pre-fix shape) blob 976d8ec78 marker 1->0, REBUILT dist and proved the mutation reached the artifact with `node scripts/ablation-dist-preflight.mjs @objectstack/service-automation 'post-strand bookkeeping threw' --absent` exit 0 (the door suite resolves service-automation through exports to dist/), engine exit 1 - 3 failed / 2 passed (PIN 1,2,3), door exit 1 - PIN 4 repairable false. M2 the guard's logger.error deleted, blob 2dc4b81eb marker 1->0, engine exit 1 - 2 failed / 3 passed (PIN 2,3) - the loudness half is pinned separately from the verdict half. M3 the summary fallback removed, blob 09f67bd25 marker 1->1, engine exit 1 - 2 failed / 3 passed (PIN 1,2) - the second hunk is load-bearing. Both controls stayed green under all three legs. The restore leg rebuilt dist and proved the marker PRESENT (preflight exit 0); one preflight tree-check exit 1 was correct and caught my own uncommitted changeset file, which was then committed - no mutation residue. GATES: family re-derived from the ACTUAL changed files with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack`, exit 0, no stale-tree warning after merging origin/main (the first derivation DID report a stale tree, so origin/main was merged and it was re-derived, as the dispatch instructs). Run locally, each exit captured right after a single redirected command, all exit 0: check:nul-bytes (7691 files scanned, no raw control bytes) - check:durability-log-level (29 durability-critical catch seams, all loud) - check:engine-double-contract (789 pinned / 133 debt / 3 exempt; no new double owed, 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 ('adds no declared-breaking changeset, 1 non-breaking changeset seen'), with `--self-test` exit 0 as its control in the same session (325 assertions). 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. Every exit code above was captured as `cmd > log 2>&1; EXIT=$?`, never through a pipe and never off a multi-command line. All heavy work went through scripts/pm/os-verify-lock.sh with OS_VERIFY_LOCK_SLOT=dev-15555; conclusions read from its VERDICT lines.",
  "mcp_calls": "11 - issue_read get + get_comments (15555, paged to the END, returned count 1 against totalCount 1, so the payload channel's 15-item cap was never relied on), pull_request_read get + get_comments (15903), add_issue_comment (claim), search_issues (one targeted dedupe before filing, non-empty so no control term was owed), issue_write create (#15944), create_pull_request (#15949), pull_request_read get (body read-back), add_issue_comment (this report), issue_read get_comments (this report read back)",
  "open_questions": [
    {
      "question": "ADVISORY, not blocking this PR - #15944 is filed and carries the full measurement. Should the completion-path recordLog be guarded too, so a history-write throw on a run whose nodes ALL succeeded never enters the node-failure catch arm? Today it does: the arm journals a repair snapshot for a COMPLETED run, reports status: 'stranded', and restoreConsumedSuspension then re-arms it - measured re-running the downstream node a second time, which crosses #13937's 'a re-armed run must never become double-runnable'. Measured PRE-EXISTING: on the pre-fix tree a store refusing only the 'completed' row already answered status: 'stranded' with restored: true, byte-identical to the fixed tree. What this PR alters is only which store failures REPORT that phantom instead of throwing over it, so it widens the advertised surface of an existing hazard without creating it. I am flagging rather than settling it because it touches the same reporting surface as #15358.",
      "options": [
        "A - land this PR as-is and rule #15944 separately. Cost: for the narrow window where the store refuses every terminal row, an operator is now told repairable: true about a completed run, where the repair re-runs the flow. Benefit: the card's false negative is fixed now, and the hazard is documented with both trees' readings rather than latent.",
        "B - hold this PR until #15944 lands, so the guard never advertises a phantom. Cost: the card's measured false negative stays shipped meanwhile, and #15944 is a second, larger change (it moves the completion path out of the node-failure arm). Benefit: the two land as one coherent story.",
        "C - fold the completion-path guard into this PR. Cost: crosses the dispatch's scope fence and grows the claim; engine.ts also carries PR #15903's landing change. Benefit: closes the double-run path in the same commit that widens its visibility."
      ],
      "recommendation": "A. The phantom, the phantom journal and the double run are all measurable on the pre-fix tree, so B pays the card's cost for a hazard it does not remove, and C spends the fence for something #15944 states more fully than a rider could. The PR body names the interaction in its own section so a reviewer meets it before merging."
    }
  ],
  "out_of_scope_findings": [
    "filed as #15944: a run whose nodes ALL succeeded is journalled and reported 'stranded' when its terminal history write throws, and restoring it RE-RUNS the flow - the completion-path recordLog sits inside the same try as the node-failure catch arm. Measured on BOTH trees (pre-fix and fixed) with the mutation proved on disk and in dist/; pre-existing, opposite failure direction to this card, adjacent to #15358 and deliberately not settled."
  ]
}

Generated by Claude Code

Landed — `0cf086759`

PR #15949 is on main: fix(automation): keep the \stranded` verdict when post-journal bookkeeping throws — a repairable strand must not report `repairable: false` (#15949). Landing authority git log origin/main | grep -c '(#15949)'= 1, cwd control(#15365)= 1.pm:dispatchedstripped; the card keepsbug, priority:p2, domain:services`, read back to confirm.

The card's premise held, and the window is narrower and stranger than it reads

resumeInternal's catch arm journals the consumed suspension — the snapshot restoreConsumedSuspension puts back — and only then stamps status: 'stranded'. Three things sit between them: recordLog's terminal run-summary line, a store whose recordTerminal throws, and failAncestors.

⭐ The subtle part, and the reason this card was worth a seat: 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(...) sitting right beneath it. A throw in that window shipped the snapshot without the verdict — and every consumer derives repairability from the verdict, plugin-approvals computing its operator-facing repairable as status === 'stranded'. ⇒ The door reported repairable: false about a run that restoreConsumedSuspension answers restored: true for.

That is a false negative on a repair instruction — worse than silence, because it tells an operator not to attempt a repair that works. Measured end to end at the real ApprovalService.decide, with restoreConsumedSuspension succeeding on the same run in the same reading, which is what makes it a false negative rather than a conservative default. Both the sync-throw mechanism and the false negative were re-derived by the contract review, not taken on report.

The guard does not over-reach, which was the way this fix could have been worse than the bug

It opens after the journal, so only a run that demonstrably has a snapshot can reach the stamp. The reviewer built four boundary cases and ran them on both trees: a journal throw still propagates (NO_CONSUMED_SUSPENSION); a completed run refuses (RUN_COMPLETED); a cascade parent — which journals nothing — refuses; 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.

⛔ Deliberately not the whole arm: RESUME_IN_PROGRESS and STORE_UNAVAILABLE are the functional cases the old #4632 verdict was right about, and escalating those is how error becomes unreadable. Pinned in both directions — M2 (delete the guard's logger.error) reds the loudness half separately from the verdict half, and M3 shows the second hunk is load-bearing. ⭐ The reviewer's M2 and M3 mutation blobs came out byte-identical to the author's, which is stronger than agreeing on totals.

⭐ PIN 3 pins the absence of an error log — the assertion most likely to pass for free — and M2 proves it is not a tautology.

⚠️ The disclosure this PR carries, which is not this card's defect

The changeset states that this change makes a pre-existing fault visible rather than creating it: the completion path's history write sits inside the same try as the node-failure arm, so a run that completed — every node succeeded — is journalled and reported stranded when its completed history row throws, and repairing such a run re-runs the flow. Filed as #15944.

The review verified that on both trees with its own test: a completed-only-throwing store gives identical stranded + restored: true + double run pre- and post-fix; an all-rows store threw before and reports after, double-running in both. ⇒ The PR exposes the hazard; it does not create it. Had that gone the other way it would have been blocking.

⚠️ One limit stated in the changeset rather than discovered later: repairable remains a point-in-time fact. In the case this fixes there is no terminal history row — that write is what failed — so the repair rides the in-memory journal and a restart loses it. The verdict reports what an operator can do now, which is exactly what was being denied.

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; now reported at error instead of thrown. ⛔ Not this card's.

Generated by Claude Code

fix(automation): keep the `stranded` verdict when post-journal bookke…

fix(automation): report a subflow parent stranded by a failed up-bubb…

Navigation Menu

Repository navigation

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

The mechanism

Measured, in a real engine

Trigger width

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

A residual the same review recorded, stated so it is not mistaken for this bug

Landed — `0cf086759`

The card's premise held, and the window is narrower and stranger than it reads

The guard does not over-reach, which was the way this fix could have been worse than the bug

⚠️ The disclosure this PR carries, which is not this card's defect

Non-blocking, recorded not folded in

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

The mechanism

Measured, in a real engine

Trigger width

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

A residual the same review recorded, stated so it is not mistaken for this bug

Activity

os-zhuang commented on Sep 4, 2026

os-warren commented on Sep 5, 2026

os-warren commented on Sep 5, 2026

os-warren commented on Sep 5, 2026

Landed — 0cf086759

The card's premise held, and the window is narrower and stranger than it reads

The guard does not over-reach, which was the way this fix could have been worse than the bug

⚠️ The disclosure this PR carries, which is not this card's defect

Non-blocking, recorded not folded in

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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 — `0cf086759`