fix(service-automation): persist the terminal run status distinction (cancelled / timed_out survive a restart) - #17008
Conversation
…our members Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
`RunRecord.status` declared two members while `recordLog`'s terminal predicate admitted four and `ExecutionStatus` declares them all. Both ends of the durable store folded to match the narrower declaration, so a cancelled or timed-out run's distinction was destroyed at write time: the same run read `cancelled` in-process and `failed` after a restart. - write side: `recordLog` records the status its own terminal predicate admitted, resolved once through the newly declared `TERMINAL_RUN_STATUSES` - read side: the row's status is resolved once in the terminal gate and handed to `deserializeTerminal`, which no longer folds it; `listHistory`'s second copy of the two-member list now asks the same predicate - stored column: `sys_automation_run.status` options and the retention `onlyWhen` scope carry all four terminal members - pins: the distinction survives a fresh store over the same rows, on `getRun`, `listRuns` (with its `?status=` filter) and `listHistory` Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ation token The body explained why no breaking-change banner is owed, and spelled the token to say so — which `check:adr-0087-registration` reads as the declaration itself (its detector is token-based and blind to the negation, the same shape as the closing-keyword trap). The reasoning is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
📓 Docs Drift Check13 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 745a37e065652fc69c6a0bf0a08cc82b09871d81 && git checkout 745a37e065652fc69c6a0bf0a08cc82b09871d81
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dd2fd2003485df584092d3a35d774d3543c70686 0ddabd202d6273ac02c45ebc232215794316c9e5 && git checkout -B drift-repro dd2fd2003485df584092d3a35d774d3543c70686 && git merge --no-ff 0ddabd202d6273ac02c45ebc232215794316c9e5
node scripts/docs-audit/affected-docs.mjs --json dd2fd2003485df584092d3a35d774d3543c70686 |
Fixes #15223
Clause-②: yes
The durable run-history row records the terminal status a run actually reached, so a restart no longer changes a run's answer. Gate union reconciled on
0ddabd202.🛑 The census came back POSITIVE — this may be a
priority:p1Triage wrote the regrade trigger down: 若发现任何消费者(平台内或应用侧)依赖持久的
cancelled/timed_out做分支,升 p1. It fires. I have applied no label — the seat regrades.Method. Two passes, joined. (1) Enumerate every read that a durable row can feed — the callers of
getRun/listRuns/loadTerminal/listHistoryacrosspackages/,apps/,examples/,scripts/— then read each for a per-terminal-member branch. (2) Grep every'cancelled'/'timed_out'literal over the same trees plus the two reachable consuming apps (objectstack-ai/ats,objectstack-ai/hotcrm), and triage all 36 hits by which status vocabulary they belong to.Positive control (it fires). The method has to find a durable-fed
cancelledbranch that is already independently measured, or its zero is NOT MEASURED. It findsAutomationEngine.restoreConsumedSuspension'sRUN_CANCELLEDarm — the one triage measured onstranded-run-status.test.ts— atengine.ts. For the app repos the control is a term known present (sys_automation_run, 2 hits;automation, 8+ files): the grep can return non-zero there, and for a status branch it returns zero.Result — two platform consumers branch on a persistent
cancelled, and the second one was not accounted for.restoreConsumedSuspensionrefusal ladder (engine.ts)getRun→loadTerminallogged.status === 'cancelled'→RUN_CANCELLEDApprovalService.inspectStrandedRequests→classifyStrandedRunState(approval-service.ts)automation.getRun(runId)→loadTerminalcase 'cancelled': return undefined(SKIP) vscase 'failed': return 'failed'(REPORT)releasePendingForTerminalRunsdead-run sweepTERMINAL_RUN_STATUSESsetThe second row is the sharp one, and it is not the sweep the card excluded — that one is
releasePendingForTerminalRunsand it treats every terminal state alike.classifyStrandedRunStatebranches per member, and its own comment states why: "Deliberately terminated by an operator (cancelRun, ADR-0044). The run stopping is the intended outcome … reporting it would bury the real findings under expected ones." On any replica that did not itself see the cancel, the folded row saysfailed, the skip does not fire, and the row is reported — then handed to the third oracle, which on a foreign replica holds no snapshot and grades itunrepairable.The strongest single piece of evidence is an existing green test.
packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts, "does NOT report a run that was CANCELLED — stopping it was the intent", feedshistory: { run_1: { status: 'cancelled' } }and assertsstranded: []. It pins a history-row state that, before this PR, the persistence layer could not produce. It passes only because its fake store does not fold. That is the whole shape of the card: the consumer's pinned contract and the row's real content had diverged, and no gate could see it.App-side: zero. Both consumers are in-platform.
🛑 Backfill: rows already stored are left as
failed, and there is nothing to recover them fromStated out loud because silence is not an answer. Rows written before this release had their distinction destroyed at write time —
recordLogmappedcancelledandtimed_outtofailedbefore the row was ever built, and the row carries no other column that discriminates. A backfill would need a source, and there is none: the ring buffer that still knew is per-process and long gone, and nothing else on the row (error,node_id,steps_json) distinguishes an operator's cancellation from a failure —cancelRun'sreasonlands in the sameerrorcolumn a thrown node writes to.So: no backfill, no migration, no "undecidable" marking. Marking the old rows undecidable was considered and rejected — it would need a write over the whole history table to replace one wrong answer (
failed) with a differently wrong one (unknown), and it would break the two consumers above in the opposite direction:classifyStrandedRunState'sdefaultarm skips an unrecognised status, so every genuinely failed historical strand would silently stop being reported. Rows written from this release forward carry the distinction; the ones before it do not, and the PR says so rather than letting a reader assume they heal.The narrowing question: INHERITED, not deliberate — and now written beside the declaration
RunRecord.statusdeclared two members whilerecordLog's own terminal predicate admitted four andExecutionStatusdeclared them all. Neither file explained it. It was inherited:Field.selectthat stores the string whatever its width — there was no storage cost to buy, which was the hypothesis worth testing.cancelRun(ADR-0044), andtimed_outhas been in the spec vocabulary the whole time.The reason is now recorded at the declaration itself (
RunRecord.status), with a ⛔ against re-narrowing it to make a downstreamswitchexhaustive.What changed
TERMINAL_RUN_STATUSES/TerminalRunStatus/isTerminalRunStatus(engine.ts). Three sites had a private copy of the list — the writer's predicate, the store's row gate,listHistory's filter — and a fourth lives in the object schema.recordLogresolves the status once into theconstthat also decides whether a row is written, and stores it. The fold is gone; a cast that would let it back is impossible by construction.loadTerminal/listHistoryresolve the row's status in the gate that already decides whether the row is terminal, and hand the member todeserializeTerminal, which no longer re-reads or folds it — and has no unreachable fallback pretending to guard an arm that cannot occur.listHistory's filter. The second two-member copy. Left alone, widening the writer would have replaced a wrong status with a missing row — cancelled runs would have vanished from the Runs list entirely.sys_automation_run.statusacceptscancelled/timed_out, andlifecycle.retention.onlyWhencounts them as terminal — a widened writer over a two-member sweep scope would have left those rows never ageing out, on a table whose whole retention posture (ADR-0057) is that history is telemetry.refuseddeliberately NOT added.ExecutionStatusdeclares it (A flow cannot REFUSE with per-record text: the only channel that interpolates is a screendescription, and a message-only screen still renders Submit and toasts "completed" #14945) but no engine path produces it, and norecordLogterminal arm admits it. An option nothing can write is declared-but-inert metadata (ADR-0078). Noted below rather than fixed here.Verification
The restart pin (
suspended-run-store.test.ts, "the persisted terminal status distinction (#15223)"). Every assertion reads through a second store over the same rows, because the defect is invisible in-process:getRunprefers the ring entry and has always saidcancelled. Three tests: a cancelled run read from a process that never saw the cancel (getRun,listRuns, and the wire's?status=filter, #7359); all four members round-tripping through the row and survivinglistHistory's gate; and the ladder reading below.Ablation — the two folds are independent, and each is proven separately. Anchor counts and
git hash-objectvs the HEAD blob on both legs; restored undertrap … EXIT INT TERMwithgit checkout HEAD -- ABSPATH, proven by blob equality and an emptygit diff HEAD. The suite resolves./engine.jsin-package, so vitest readssrc/— nodist/sits between the mutation and the assertion.status: terminalStatus→entry.status === 'completed' ? 'completed' : 'failed'expected 'failed' to be 'cancelled',expected 'NO_CONSUMED_SUSPENSION' to be 'RUN_CANCELLED'status(the parameter) →row.status === 'failed' ? 'failed' : 'completed'expected 'completed' to be 'cancelled'×2,expected 'RUN_COMPLETED' to be 'RUN_CANCELLED'The store-level round-trip stays GREEN under the write leg and turns RED under the read leg: the two sites really are independent, and a fix to either alone would have masked the other. The read leg also shows the read fold's own signature — it makes the ladder answer
RUN_COMPLETEDfor a cancelled run, which is worse than today'sNO_CONSUMED_SUSPENSION.Reverse validation of the cross-package type change (proves the dependent reads the rebuilt
.d.ts, not a cache): a probe inplugin-approvalsassigningRunRecord['status']to the four-member union compiles GREEN; the same probe assigning to the OLD'completed' | 'failed'is RED withTS2322: Type '"completed" | "failed" | "cancelled" | "timed_out"' is not assignable to type '"completed" | "failed"'. Probe deleted,git status --porcelainempty. No out-of-package source namesRunRecordorTerminalRunStatustoday.Runs.
pnpm --filter @objectstack/service-automation test— 1496 passed / 127 files (1493 before; +3).pnpm --filter @objectstack/service-automation typecheck— green, including the test layer (check:test-typecheck: OK — 0 file(s) / 0 error(s)).turbo run build --filter='...@objectstack/service-automation'— 68 successful, 68 total.scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack→ 58 families, all run, all green.--ranreconciliation:✓ 58 derived famil(ies) accounted for — 58 run, 0 NOT-MEASURED.check:dual-build-cjs-loads(built the five packages it named, then green — 104 require entry points across 67 packages) andcheck:type-check-debt(OOM under a 4096 MB ceiling; re-run at 9216 MB, green — 5 ledger entries re-measured, 55 raw errors, none above its recorded number).check:route-envelope(dispatch-gates: a whole-tree-walk gate whose workflownames:lists only its CURRENT members is placed Silent, so it is never derived for the card that adds a new member — measured on check:route-envelope / PR #16730 #16828, outside the derived union) is not owed: no file in this diff writesc.json(…)orres.json(…)— measured, 0 hits across all six.pnpm lint(eslint . --no-inline-config) ran in full and passed. No narrowing to declare.check:adr-0087-registrationfirst went RED on this changeset: the body explained why no breaking-change banner is owed and spelled the token to say so, which its detector reads as the declaration itself (token-based, blind to the negation — the closing-keyword trap in another costume). Reworded to state the same conclusion without the token; green.The reading triage asked for: does the ladder now answer
RUN_CANCELLEDon a foreign replica?Half yes, and the half that did not move is worth naming. ⛔ The ladder is unchanged by this PR — triage ruled it honest and the row the defect.
RUN_CANCELLED, pinned in the new test. Before:NO_CONSUMED_SUSPENSION.failedring entry (the shapestranded-run-status.test.tsmeasures): stillNO_CONSUMED_SUSPENSION. The ladder testscancelledagainstgetRun, which is ring-first, while it consults the durable row forcompletedalone. The row can support the real answer now; that arm does not read it. Recorded in the test's comment so the assertion is not mistaken for a fix — the reason it holds has narrowed from two to one.Dispositions
sys_metadataand the conversion chain replayed at rehydration.sys_automation_runis an engine-owned system data table with no authorable surface,objectstack migrate metahas nothing to rewrite for it, and every value already stored stays valid under the widened option set. No column added, no type changed.**BREAKING**banner: not owed. The published contract —IAutomationService.getRun/listRunsreturnExecutionLog, whosestatusisExecutionStatus— has declared all four members since before this row existed. The implementation stops under-reporting one the contract already promised; a consumer written against the declared contract is unaffected. The in-repo readers that would have been narrowed by the widening (listHistory, the retention scope, the row gate) are all widened in this same change.@objectstack/service-automation: minor— an additive widening of a published package's surface, and the level the clause-② declaration requires.packages/spec/src/**path, so the fix(analytics): ask the object-level read grant before serving an inline dataset — one admission verdict on every driver #16860 collision the dispatch fenced did not arise.ExecutionStatusalready declared everything needed.验收备注 (acceptance notes — observations, not filed)
ExecutionStatusdeclares a fifth terminal member,refused(A flow cannot REFUSE with per-record text: the only channel that interpolates is a screendescription, and a message-only screen still renders Submit and toasts "completed" #14945), that no engine path produces and norecordLogterminal arm admits — so arefusedrun would write no durable history row at all, not merely a folded one. Adjacent to this card, deliberately out of its scope, and not filed: it needs the A flow cannot REFUSE with per-record text: the only channel that interpolates is a screendescription, and a message-only screen still renders Submit and toasts "completed" #14945 author's reading of whether the member is live yet.restoreConsumedSuspension'scancelledarm readsgetRun(ring-first) while the arm above it reads the durable row directly, which is why the stale-hot replica still cannot name a cancellation. Triage explicitly reserved the ladder; not filed, recorded above and in the test.AutomationEngine.getRunreturnsnullboth for "no such run" and for a swallowed store failure (its own comment names this ascheck:durability-log-level结构性看不见「读接缝把故障答成空值」这一类 —— #4825 / #5108 全家都在闸门盲区里 #5186's shape, remedy out of finding(service-automation): engine.ts has 13 MORE log messages splicing uncontrolled thrown text — #6299's "last batch" premise was wrong, and persistSuspendedRun sits 25 lines from one it fixed #6499's scope). Untouched, pre-existing, not filed.🤖 Generated with Claude Code
https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Generated by Claude Code