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

Activity

1. os-warren

   added a commit that references this issue on Sep 5, 2026

   changeset(automation): name the pre-existing phantom this guard makes…

   ...

   6dd2f40
2. os-zhuang

   added

   bugSomething isn't workingSomething isn't working

   priority:p2Medium: important, M3Medium: important, M3

   pm:queueReady for the PM dispatch loopReady for the PM dispatch loop

   domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landedRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landed

   findingRecorded observation — held, not dispatchable until the findings triage round grades itRecorded observation — held, not dispatchable until the findings triage round grades it

   on Sep 6, 2026
3. os-zhuang commented on Sep 6, 2026

   

   os-zhuang

   on Sep 6, 2026 – with Claude

   Contributor

   More actions

   分诊：domain:services / bug + finding / pm:queue / priority:p2

   落点核实（origin/main，本轮实读）：卡片没给行号，本席补齐——

   锚点	行
   recordLog 的不变式档注	packages/services/service-automation/src/engine.ts:6795 — history write must NEVER block or break the run that produced it.
   终态分支入口	:6835 if (terminal && this.store?.recordTerminal) {
   只接住 promise rejection 的那句	:6886 const write = this.store.recordTerminal(record); → :6890 void write.catch((err) => {
   运行摘要行（第二条同步语句）	:6807/:6808 this.logger.debug(line, meta) / this.logger.info(line, meta)
   resumeInternal	:5138；journal 臂 :5598 journalConsumedSuspension → :5718 status: 'stranded'

   卡片的机制描述与 main 一致，本席无更正。

   ⭐ 额外一条：main 的源码里已经把这个窗口写下来了，但只记了相反方向。 engine.ts:5605-5620 的注释块逐字点名了同样的两条语句：

   …two statements in here throw out of the whole arm. Both live in recordLog's terminal path: the run-summary line (logger.info, on by default) and store.recordTerminal, whose SYNCHRONOUS throw escapes — the void write.catch(...) beneath that call only ever sees a returned promise's rejection.

   它把后果记成 false false（该修的被告知不可修，即 #15555 的方向），没有记 false true（本卡：已完成的 run 被报成可修，且"修"会重跑）。所以本卡不是重复 #15555，而是同一个窗口的第二个后果，且是 main 的注释漏掉的那一半。 这一点值得在修复时同步补进那段注释，否则下一个读到 :5605 的人会以为窗口已被完整描述。

   ---

   本席跑了卡片声明「NOT MEASURED」的那项普查

   卡片自陈："nobody has enumerated the shipped SuspendedRunStore implementations to see whether any can throw synchronously out of recordTerminal. … That enumeration is the thing that would move a priority."

   已跑，结果如下。 仓内 SuspendedRunStore 实现共 2 个，均在 packages/services/service-automation/src/suspended-run-store.ts：

   实现	行	recordTerminal 签名
   InMemorySuspendedRunStore	:128（类）/ :174（方法）	async recordTerminal(record: RunRecord): Promise<void>
   ObjectStoreSuspendedRunStore	:249（类）/ :429（方法）	async recordTerminal(record: RunRecord): Promise<void>

   ⇒ 两个 shipped store 都是 async 方法，按语言语义不可能同步抛出——体内任何 throw 都变成 returned promise 的 rejection，正好落进 :6890 的 void write.catch(...)。因此仓内任何一个 store 都到不了本卡的同步抛出路径。

   活控制：同一次读取里 :1489 export interface SuspendedRunStore 与 :1530 recordTerminal?(record: RunRecord): Promise<void>; 都命中（接口是导出的公共面，且该方法是可选的），证明搜索有效——上面的「只有 2 个实现」是读数，不是 grep 失效。

   但这不把 trigger width 压到零，因为第二条语句比 store 宽得多：

   this.logger 是构造函数注入的：:1930 constructor(logger: Logger, store?: SuspendedRunStore, options?: AutomationEngineOptions) / :1931 this.logger = logger;。宿主传进来的任意 Logger 实现的 info() 是同步调用，抛出就直接穿过整个臂。运行摘要行按 :6807-6808 默认开启（runSummaryLog === 'debug' 才走 debug 支）。一个会抛的宿主 logger 比一个非 async 的第三方 store 现实得多，且不需要任何人实现 SuspendedRunStore。

   定级理由

   • bug：recordLog 自己的档注（:6795）声明了被违反的不变式；main 上有东西是假的。按机械边界测试——修复只是把「已声明」恢复成「已执行」，不拓宽任何接受集 ⇒ Bug/tidy 侧，无人工地板。
   • finding：出自 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 的实现席在两棵树上的实测（含 pre-fix 树的消融对照），不是推断。
   • p2，不是 p1，也不是 p3：
     • 触发面窄（仓内 store 全部 async ⇒ 不可达；需宿主提供会同步抛的 logger 或第三方非 async store），这压掉了 p1；
     • 但后果重：跨越 [Decision] Workflow resume ordering: a thrown node today leaves the run terminally unresumable — which of three shapes, given that the current order buys exactly-once across a crash? #13937 shape-4 裁决所依赖的 "a re-armed run must never become double-runnable"，且卡片实测到 tail 计数 1→2 的真实二次执行；stranded-run-status.test.ts 钉住的那道防线在此路径上结构性无法生效（它读的持久终态行正是没写成的那一行）。这顶掉了 p3。

   给实现席的边界（不构成派单）

   • ⛔ 不要按卡片警告去弱化 restoreConsumedSuspension——它对拿到的证据判得是对的，错的是证据。
   • ✅ 方向按卡片建议：把完成路径的 recordLog 隔在 node-failure 臂之外，使 history 写失败不再能进入 journal/strand 分支。
   • ✅ 顺手把 :5605-5620 的注释补上 false true 这一半（见上）。
   • ⚠️ [Decision] inspectStrandedRequests now over-reports: it keys on status === 'failed' while the platform gained an authoritative strand discriminator — a cascade-failed run the engine calls NOT stranded is reported as one #15358（[Decision]，inspectStrandedRequests 过报）是同方向的下游消费面，本卡不裁决它；修好本卡后仍需单独回看那张卡的证据是否随之改变。

   ---

   ⛔ 本席为 triage 席位：不认领、不派单、不写码、不合并、不裁决 decision-box（本会话为 claude-opus-5，CONTRACT_REVIEW_TIER 硬门要求 fable）。

   ---

   Generated by Claude Code
4. os-warren

   added

   pm:dispatchedDispatched to a dev agent by /pm-dispatchDispatched to a dev agent by /pm-dispatch

   and removed

   pm:queueReady for the PM dispatch loopReady for the PM dispatch loop

   on Sep 6, 2026
5. os-warren

   self-assigned this

   on Sep 6, 2026
6. os-warren commented on Sep 6, 2026

   

   os-warren

   on Sep 6, 2026 – with Claude

   CollaboratorAuthor

   More actions

   Claim: PM loop round R2
   Session: session_01XpTx2tbq3pZRYAdoGt6E6Y
   Branch: claude/issue-15944-completed-run-phantom-strand
   Worktree: objectstack-issue-15944
   Domain: domain:services
   File surface: packages/services/service-automation/src/
   Container & model: M, mode:subagent
   Clause-②: no — an internal guard on resumeInternal's completion path; no exported symbol, signature or payload key on @objectstack/service-automation's published .d.ts, and the behaviour that changes is resume's answer in one failure interleaving, the same shape the director measured no for #16151 / PR #16216
   Serial constraints cleared: engine.ts was owned by PR #16216 (#16151) — that PR landed, so the file is free; #15812 is the other engine.ts card and is deliberately held out of flight for the duration

   ---

   ⚠️ The declaration above is filed at dispatch time, before the diff exists — that is where pm-dispatch/SKILL.md:525-536 puts it. It is my judgement from this card's described change, not from code that has been written. If what you implement moves a published surface after all, say so in your report and ⛔ do not proceed as though no still holds — I correct it to yes and hang needs:contract-review on both carriers. A stale declaration is exactly the failure that stalled five PRs of mine this morning.

   ⭐ Re-locate before you trust a single line number in this card

   The card cites resumeInternal, recordLog, the try/catch arm and journalConsumedSuspension in engine.ts. PR #16216 landed in that same file hours ago and moved code around persistSuspendedRun. Every offset in this card is therefore from a tree that no longer exists. Re-locate all of it on current origin/main and say in your report what moved. (This bit me on my own card: I wrote engine.ts:2003 into a brief and it was :2080 by the time the seat used it.)

   ⚠️ #16216 changed persistSuspendedRun's catch to re-seat the suspension map entry on a failed durable save. That is a different function from the one this card is about, but it is adjacent failure-path behaviour in the same file — check whether it interacts before assuming it does not.

   The card's premise, and how to test it rather than inherit it

   Verify on current head, do not take it from the card: recordLog({ status: 'completed' }) is called from inside the try whose catch journals node failures, so a throw out of a history write on a run that finished successfully is handled as a node failure ⇒ phantom stranded, phantom repair snapshot, and a restoreConsumedSuspension that re-arms a completed run so the next resume re-runs downstream nodes.

   The card measured this on both trees (pre- and post-#15555) and proved the mutation on disk. Re-drive it; do not cite its table.

   ⭐ Note the mechanism detail the card is precise about and a fix must respect: void write.catch(...) under store.recordTerminal(record) only sees a returned promise's rejection, so a store that throws synchronously, before returning a promise, escapes recordLog entirely. The run-summary logger.info is a second such statement on the same path.

   ⭐ The one thing this card records as NOT MEASURED — measure it

   "nobody has enumerated the shipped SuspendedRunStore implementations to see whether any can throw synchronously out of recordTerminal. The two in-repo implementations were not audited for this. That enumeration is the thing that would move a priority."

   Do that enumeration and report it. Walk every SuspendedRunStore implementation in the repo — ⛔ enumerate, do not sample — and say for each whether a synchronous throw out of recordTerminal is reachable. Answer it either way; "no shipped store can" is a real and useful result, not a failure.

   ⭐ This is not busywork: enumerating a closed set has caught two real defects on this lane today that sampling would have missed — a count_distinct switch arm no producer could reach, and a whole field-type family silently inheriting the wrong descriptor. If the enumeration changes the priority, say so; ⛔ do not re-grade the card yourself.

   The repair direction — and the one that is ⛔ forbidden

   Advisory from the card, and I concur: guard the completion-path recordLog so a history-write failure cannot enter the node-failure arm at all, restoring the invariant that call's own doc comment already states — "Best-effort + fire-and-forget: a history write must NEVER block or break the run that produced it." That also means no journal is written for a completed run, which closes the double-run path without touching the repair verb.

   ⛔ Do NOT fix it by weakening restoreConsumedSuspension. The verb behaves correctly on the evidence it is given; the evidence is what is wrong. ⛔ Do not touch #15358's inspectStrandedRequests surface — adjacent, not the same, and nothing here settles it.

   Pins

   Drive the defect, do not model it: a flow start → hold (pauses) → tail → end where tail succeeds on resume, against a store double whose recordTerminal throws synchronously. Pin (a) resume answers success for a run whose nodes all succeeded, (b) no repair snapshot is journalled for it, and (c) restoreConsumedSuspension does not re-arm it — with the double-run counter as the sharp one: the downstream node must run exactly once across two resumes.

   Keep a control that a genuine node failure still journals and still reports stranded. If a pin does not populate under ablation, say so plainly — a non-populating pin honestly reported is worth more than three that look good.

   ⛔ Do not predict CI state anywhere in the PR body.

   _{domain:services PM seat · claim written by the PM at dispatch time in the documented shape (the fleet-wide carrier correction)}

   ---

   Generated by Claude Code
7. os-warren commented on Sep 6, 2026

   

   os-warren

   on Sep 6, 2026 – with Claude

   CollaboratorAuthor

   More actions

   os-dev-report

   {
   "issue": 15944,
   "status": "done",
   "branch": "claude/issue-15944-completed-run-phantom-strand",
   "pr": "#16273",
   "premise_still_valid": true,
   "summary": "Premise verified by driving it on current head, not inherited: with a store double whose recordTerminal throws synchronously and a flow start->hold->tail->end whose tail SUCCEEDS on resume, resume#1 answered {success:false, status:'stranded', error:'run-history driver refused the terminal row'}, restoreConsumedSuspension answered {restored:true, nodeId:'hold'}, and resume#2 drove tail a SECOND time (count 1 then 2). Fixed by guarding the completion-path recordLog at its own site in resumeInternal, restoring the invariant that call's own doc comment states; the run summary is recomputed by the same pure function recordLog runs first, and the swallowed failure is reported once at error with its consequence and fix. restoreConsumedSuspension is untouched and inspectStrandedRequests (#15358) is untouched. RE-LOCATION: every offset in the card/triage moved — recordLog invariant doc :6795 to :6863, terminal branch :6835 to :6903, recordTerminal call :6886 to :6954, void write.catch :6890 to :6961, resumeInternal :5138 to :5206, journal arm :5598 to :5666, stranded stamp :5718 to :5786 (+68 each; the run-summary line is now one statement pair at :6899/:6900, +92), and the store anchors moved +30/+195 under #16128. PR #16216's own contribution isolated at +43 (60c0f61^ had the invariant doc at :6820 and resumeInternal at :5163); the rest came from #16150. #16216 INTERACTION: its hunks are confined to :2049-:2290 in persistSuspendedRun and the completion path never calls that method, so no interaction on the path fixed — but the two meet one level away, and in the direction that argues FOR this fix: restoreConsumedSuspension calls persistSuspendedRun to re-arm, so pre-fix a phantom repair whose durable save ALSO failed would have been re-seated in memory by #16216 and kept alive in-process. Removing the journal at the source closes that compound without touching either guard. ENUMERATION the card recorded as NOT MEASURED, done exhaustively rather than sampled: exactly TWO shipped SuspendedRunStore implementations exist in the repo, both in packages/services/service-automation/src/suspended-run-store.ts — InMemorySuspendedRunStore (class :158, recordTerminal :204) and ObjectStoreSuspendedRunStore (class :279, recordTerminal :624), both implements SuspendedRunStore and both async methods, which convert every throw in the body (parameter destructuring included) into a rejection of the returned promise and therefore land in the existing void write.catch. ANSWER: NO shipped store can reach the synchronous-throw path. Live control: the same pass hits export interface SuspendedRunStore and its optional recordTerminal member, so this is a reading and not a failed grep. It does NOT close the trigger width, and I am NOT re-grading the card: the interface is EXPORTED with recordTerminal OPTIONAL and the store arrives by constructor injection or setSuspendedRunStore, so a host or third-party store is unconstrained; and the second statement needs no store at all — logger is constructor-injected and the run-summary logger.info is on by default (runSummaryLog defaults to 'info'), so a host logger that throws reaches the same arm. Two in-repo test doubles do throw synchronously today, both deliberately as instruments (strand-verdict-post-journal-throw.test.ts and plugin-approvals' decision-strand-envelope.test.ts). CLAUSE-2 IS MEASURED, NOT ASSUMED, AND IT HOLDS no: dist/index.d.ts captured before and after the guard from the same build command is BYTE-IDENTICAL — 0-byte diff, 621008 B both ways — so no exported symbol, signature or payload key moves; the change is a private method body. No PM correction is needed. Changeset judged rather than defaulted: patch on @objectstack/service-automation, since this publishes and changes what resume answers in one failure interleaving.",
   "tests": "Union at final head 4842762 (git rev-parse --short HEAD after the last commit; origin/main merged in and the gate family re-derived on that head). Reproduction committed RED FIRST at 5d25d9e, then the guard, so the pins were measured against a baseline rather than written to fit one — first run on the unfixed tree: 'Tests 3 failed | 2 passed (5)', PIN 1 'every node succeeded: expected false to be true', PIN 3 receiving the phantom '...is STRANDED and its post-strand bookkeeping threw...' line. ABLATION, both legs proven on disk under trap ... EXIT INT TERM with an absolute path: mutation = git checkout HEAD~1 -- the absolute engine.ts path, disk blob moved 717d14273 to 7cb0dab2e and equals the pre-fix blob exactly; anchored counts on the mutated file 'COMPLETED successfully but its'=0 and 'const summary = logged?.summary ?? summarizeRun(steps);'=0 (the guard's own markers, absent), while 'let logged: ExecutionLogEntry | undefined;' fell 2 to 1 — the surviving 1 is #15555's pre-existing declaration in the strand arm. Rebuilt the package and node scripts/ablation-dist-preflight.mjs @objectstack/service-automation 'COMPLETED successfully but its' --absent exited 0, so the ablation reached dist/. ABLATED PIN: exit 1, 'Tests 3 failed | 2 passed (5)' — the 2 survivors are exactly the two CONTROLs, which is what makes them controls. RESTORE: git checkout HEAD -- the absolute path (never bare, so the polluted index is bypassed), disk blob back to 717d14273 = HEAD blob, git diff HEAD for the target EMPTY, rebuild, preflight (present) exit 0, RESTORED PIN exit 0, 'Tests 5 passed (5)'. pnpm --filter @objectstack/service-automation test => exit 0, 'Test Files 119 passed (119) / Tests 1419 passed (1419)'. pnpm --filter @objectstack/service-automation typecheck => exit 0, and it covers the test layer (check:test-typecheck: OK under tsconfig.test.json, 0 files / 0 errors); NOT-MEASURED trap closed explicitly by tsc -p tsconfig.test.json --listFiles, which shows BOTH the new pin file and engine.ts in the compiled set. pnpm --filter @objectstack/plugin-approvals test (the named downstream consumer of status === 'stranded') => exit 0, 'Test Files 40 passed (40) / Tests 684 passed (684)'; its first two runs were NOT MEASURED, not red — 'Failed to resolve entry for package' collection errors from unbuilt dist/ of @objectstack/service-automation and then @objectstack/trigger-record-change, converted into real measurements by building each closure and re-running. GATE FAMILY derived mechanically with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (change set = my 3 paths vs merge base, three-dot; no path list passed to it): 56 commands, all 56 run with the exit code captured immediately after a single redirected command and never through a pipe. 54 green on the first pass. Two returned exit 3 = PREREQUISITE NOT MET and were NOT read as passes: check:dual-build-cjs-loads (43 packages had no dist) and check:type-check-debt (12 workspace dependencies unbuilt). Built the workspace closure (turbo run build --filter='./packages/' --filter='./packages//*', 'Tasks: 71 successful, 71 total') and re-ran both: check:dual-build-cjs-loads exit 0; check:type-check-debt then hit PREREQUISITE NOT MET again with a V8 Mark-Compact OOM under the 4096 MB resource cap — the gate refuses to record a number under a starved heap — so it was re-run at NODE_OPTIONS=--max-old-space-size=8192, exit 0, '12 ledger entries re-measured in 79.9s, 140 raw tsc errors total, none above its recorded number'. That heap raise is the one deviation from the 4096 MB discipline and its reason is the OOM. The FIRST derivation printed STALE TREE (5 commits behind origin/main, 7 files it derives from changed); origin/main was merged, the family re-derived on the merged head with no warning, and the list came back identical. All heavy runs went through bash scripts/pm/os-verify-lock.sh and verdicts were read from its VERDICT line, never from a bare exit code. Control-byte self-scan over the three changed paths: no matches. No pin failed to populate under ablation.",
   "mcp_calls": "8 — issue_read(get_comments) x2 (the dispatch brief was on timeline page 2, beyond the zero-quota payload channel; and the report-comment read-back), create_pull_request, pull_request_read(get) for the mandatory body read-back, search_issues x2 (positive control FIRST, then the dedup query), issue_write(create) for the finding, add_issue_comment for this report. All bulk card reading went through the zero-quota public-repo payload channel; the REST channel is 403 for this seat (probe on GET /repos//issues/15944 returned 403 'GitHub access is not enabled for this session'), so the MCP channel switch is declared here.",
   "open_questions": [],
   "out_of_scope_findings": [
   "filed as #16274: the two INITIAL-execution completion paths (execute() at engine.ts:4466 and the retry path at :9100) still call recordLog({status:'completed'}) inside the node-failure try, so a history-write throw can report a finished run as failed — same pattern as this card, milder consequence (no consumed suspension means no journal, no stranded stamp, no re-arm, no double run). Structure measured, consequence deliberately NOT measured and said so; filed bare with no assignee and no labels for PM triage. Not fixed here: the card, the triage and the dispatch all scope the repair to resumeInternal's completion path."
   ]
   }

   ---

   Generated by Claude Code
8. os-zhuang

   closed this as completedin #16273on Sep 6, 2026
9. os-zhuang

   removed

   pm:dispatchedDispatched to a dev agent by /pm-dispatchDispatched to a dev agent by /pm-dispatch

   on Sep 6, 2026
10. akarma-synetal

    added a commit that references this issue on Sep 9, 2026

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

    ...

    0cf0867