Skip to content

feat(service-automation): an operator can put back a suspension a failed resume consumed - #13951

Merged
os-steve merged 5 commits into
mainfrom
claude/issue-13909-operator-exit-verb
Sep 1, 2026
Merged

feat(service-automation): an operator can put back a suspension a failed resume consumed#13951
os-steve merged 5 commits into
mainfrom
claude/issue-13909-operator-exit-verb

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Part of #13909 — slice 2, the operator exit verb (the parent's deliverable 2, "shape 4").

Slice 1 (PR #13934) made the condition visible and measured that it is terminal. This slice adds the way out. It does not decide the resume ordering: that is #13937, unruled and in the maintainer's hands, and this branch leaves the ordering, forgetSuspendedRun and traverseNext exactly as they are.

Tier — declared, not self-cleared

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack printed, against this diff:

Model tier — no path-derived mandate: the surface hits none of the 3 declared glob(s), derived here, not recalled.

the clause-② PATH limb does not fire: nothing under packages/spec is touched. The derivation's next line is the one I am not entitled to answer:

Clause ② is NOT reachable from paths: a card that changes contract accept/reject behaviour or widens the public surface is fable-mandatory too, judged from the card CONTENT. This line is a FLOOR, never a clearance.

This change does widen a public surface — one new public method on AutomationEngine. Whether that makes it clause-② by the content limb is the PM's judgment, not mine, and I am not clearing it. ⛔ Please rule before merge.

⚠️ And read the placement argument below before assuming the verb belongs in packages/spec: it was placed where its nearest sibling already lives, not moved to dodge a review.

Why the verb is an engine method and not a spec contract method

The dispatch warned a new engine verb "very likely lands in packages/spec/src/contracts/automation-service.ts" and told me not to contort the design to avoid that. I did not. I placed it beside its exact sibling, and here is the reading:

  • IAutomationService (in packages/spec) declares execute, resume, listRuns, getRun, listSuspendedRuns, getSuspendedScreen and friends — the flow-execution surface a transport calls.
  • cancelRun is not in it. The nearest sibling of this verb — a deliberate, operator/service-initiated action on a suspension's lifecycle, outside the ordinary execute/resume path — is AutomationEngine.cancelRun, and it is engine-level. restoreConsumedSuspension is cancelRun's mirror image (one consumes a suspension on purpose, the other puts one back on purpose), so it sits in the same place.
  • Its result type is engine-local for the same reason. Folding the refusals into AutomationResult.code would put eight new codes into a contract every transport reads — and would mint platform vocabulary for a condition [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 has not yet ruled the shape of.

No platform-level name or status is minted. AutomationResult.status is still 'completed' | 'paused' | 'failed', the run stays failed, and nothing here names the condition. Naming is an explicit same-batch sub-item of #13937 (under shape 2 the condition largely stops existing).

Consequence, stated rather than hidden: like cancelRun, this verb has no REST route and no CLI command in this PR. A host holding the engine can call it; an operator reaches it through a host. Adding a door needs a permission model of its own and is filed separately.

What it does

AutomationEngine.restoreConsumedSuspension(runId, { requestedBy, reason }) puts back the suspension a resume consumed, so the run is resumable again through an ordinary resume — same authority gate, same screen validation, same resuming guard.

Deliberate, never automatic. Nothing calls it. No retry, no sweeper, no self-healing arm: an operator asks for one run, by id.

Verbatim, never advanced. What goes back is the pause as it stood — its own variables, its step log as of the pause, its node, node type, correlation, screen. Two consequences, both in the docblock, the trace and the return value:

  • ⚠️ the original resume signal is NOT replayed — replaying it would re-decide on the operator's behalf, and signal.branchLabel is not part of a suspension at all, so a silent replay would take a different edge;
  • ⚠️ the failed attempt is NOT undone — this re-arms a pause, it does not roll a transaction back.

Restoring the pause exactly as it stood is also what keeps this from pre-empting #13937: it is precisely the state a resume-ordering change would have left behind, so it composes with that ruling instead of racing it.

Where the snapshot lives. In-memory hot journal (bounded, oldest-first) plus — when a store is configured — the run's own terminal history row, in sys_automation_run columns that already exist (variables_json / context_json / screen_json / correlation / node_type) and that the object's own field comment already complained were never written on terminal rows. No new column, no new status value, no new table, no new store method. A run that is restored and then finishes upserts that row without a snapshot, so "restorable" cannot outlive the condition. Over a byte budget the snapshot is dropped, never truncated — half a variable map would restore a run into a state it was never in, and the drop is logged with the size.

The durable half is not a nicety: ADR-0019 exists because the process hibernates between suspend and resume, and these runs are found hours later by a sweep or a support ticket, in some other process. An exit that only worked inside the lifetime that stranded the run would answer almost never.

Refusals — the reasons are the deliverable

Each is earned by a specific observation, in this precedence:

refusal earned by
RESTORE_IN_PROGRESS another restore of this run is running in this process
RESUME_IN_PROGRESS resuming holds the id — a resume is in flight, outcome not decided
RUN_SUSPENDED a live suspension exists (already resumable; also the second-restore answer)
STORE_UNAVAILABLE the store could not be read, so "is it suspended?" is UNKNOWN
RUN_COMPLETED the recorded outcome is completed
RUN_CANCELLED the recorded outcome is cancelled (ADR-0044)
NO_CONSUMED_SUSPENSION terminal, but no snapshot: never paused, or the snapshot is gone
RUN_NOT_FOUND no record of the run at all

paused splits into two. "Suspension gone, no terminal row yet" is what a live resume looks like from outside, and re-arming one of those races it — its traversal would finish and record completed while a paused row it knows nothing about survives. RESUME_IN_PROGRESS is that case; RUN_SUSPENDED is the ordinary already-parked one.

STORE_UNAVAILABLE is a refusal on purpose. Reading an outage as "no suspension" is the one mistake that would put a second resumable pause on a live run, so the live-suspension probe uses the STRICT loader, and the history read refuses rather than degrading to "nothing to restore" — an answer an operator would act on by giving up.

NO_CONSUMED_SUSPENSION deliberately does not claim WHICH of its two causes it is. Nothing in the engine can tell "never suspended" from "snapshot no longer held"; the message names the status actually observed instead of inventing a certainty.

Idempotence — carried by the paused row, not by a flag

A suspension is keyed by run id, so however many operators ask there is one resumable pause: the second call finds one live and answers RUN_SUSPENDED. That holds across processes and across a restart, because the paused row is durable. A restoring set adds the in-process half, exactly as resuming does for resume, so two callers racing in one process get one restored: true and one refusal rather than two claims.

And it cannot produce two traversals by construction: the verb does not resume. It re-arms the pause and stops.

The trace

The whole reason this class stayed silent is that nothing recorded it, so an exit that is itself invisible repeats the defect. A restore writes:

  1. a warn naming the run, flow and node, with who asked, why, when the suspension was consumed and the original failure in the structured slot (all three are uncontrolled text — the finding(service-automation): engine.ts 还剩三处同形的 warn message 拼接 —— forgetSuspendedRun / cancelRun / listSuspendedRunsDurable,是 #5912+#6230 之后该文件的最后一批 #6299 family — and a newline in any of them would split one record into several physical lines of which only the first is greppable). warn, not info, because this moves a run the platform had recorded as terminally failed. When nobody supplied requestedBy the record says not recorded, which is itself a finding;
  2. a paused run-log entry, so the repair is not invisible on the surfaces that read run status. This is the same record the resume path writes when a run re-suspends, for the same reason.

⚠️ Measured limitation, stated at the site rather than papered over: after a restart the durable surfaces still read such a run failed, because getRun / listRuns deliberately let a terminal row win over a paused one. A durable status that survives the restart is naming work, and naming is #13937's same-batch sub-item.

Disclosure boundary — measured

sys_automation_run.variables_json now appears on one class of terminal row. Who may read it is unchanged: #7900 converged both doors on the sys_automation_run object-read grant, status-independent, and the maintainer's 2026-08-12 ruling rejected per-field filtering as the mechanism. What changes is which rows carry the snapshot, behind that same grant.

⛔ The run-detail HTTP surface is untouched for failed runs: the snapshot is a parameter to recordLog, deliberately not a field of ExecutionLogEntry, which GET /automation/:name/runs/:runId serves verbatim — that would have been the disclosure widening #7639 refused. The only variables a caller newly sees there belong to a run that IS paused, which is #7639's own sanctioned case.

In-place correction in plugin-approvals (comment only, named here because it is not this card's package)

backfill-platform-row-organizations.ts documented context_json as "Written on paused rows only — recordTerminal does not persist it". This change makes that false for one class of row, so the comment is corrected in place. No behaviour change, and none is needed: a row that has the snapshot uses it exactly as a paused row does, which is strictly better for that sweep (a subject whose live record is gone now has a write-time snapshot where it had none). Scan for other readers of those columns: runtime/src/domains/automation.ts (the permission gate above), the sys_automation_run object definition, and dogfood assertions about paused rows — none of them branch on "terminal rows have no snapshot".

Verification

All at final commit 4c949ca39, working tree clean. Exit codes captured before any pipe (cmd > file 2>&1; EXIT=$?); verdicts quoted from each gate's own printed line.

Suites (via scripts/pm/os-verify-lock.sh):

  • pnpm --filter @objectstack/service-automation testTest Files 92 passed (92), Tests 1116 passed (1116), VERDICT command-exit 0. The new file alone: 22 passed.
  • Consumers, run because RunRecord and recordLog changed and the comment fix lands in approvals: pnpm --filter @objectstack/plugin-approvals --filter @objectstack/runtime test — approvals Tests 632 passed (632), runtime Tests 3011 passed (3011), VERDICT command-exit 0.
  • Dependency closure built first: pnpm --filter '@objectstack/service-automation^...' build, VERDICT command-exit 0.

Type coverage, measured rather than assumed. This package has no typecheck scriptpnpm --filter ... typecheck answers ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT and exits 1, loudly rather than silently. So it was measured directly: tsc --noEmit -p tsconfig.json reports exactly 3 errors, all pre-existing, all in nested-region-parity.test.ts (Property 'flows' is private), a file this diff does not touch and whose text is identical on origin/main. --listFiles confirms the new test file is in the program (1 hit each for the test file, engine.ts and suspended-run-store.ts) — unlike plugin-approvals, whose tsconfig excludes **/*.test.ts. The ratchet that owns those 3: check:type-check-debt --re-measureOK — 29 ledger entr(ies) re-measured in 209.0s, 1531 raw tsc error(s) total, none above its recorded number / surplus: none — every entry sits exactly at its measurement, so any new error is red.

Gates. Re-derived from the ACTUAL diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed — the script takes the change set from the merge base), both output sections read whole: 30 path-matched families plus the convention-triggered sets for a test-file edit, for a package owning an i18n-extract.config.ts, and for a file carrying an ADR-0112 code-shaped value. 36 of 37 harvested commands GREEN, including every ratchet: check:engine-double-contract (no new fake engine — this PR adds a test file with none), check:where-matcher (321 matcher(s) ... 0 silently-wrong), check:query-options-erasure, check:cross-package-test-inputs, check:published-files, check:test-source-alias, check:type-check-coverage (66/78 workspace packages type-checked), check:i18n (9 package(s) — all bundles in sync), check:i18n-stale-fill, check:dual-build-cjs-loads, check:nul-bytes, and — the one worth naming because a new refusal vocabulary is exactly its population — check:dispatcher-error-vocabulary, green. Plus a manual grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over all five changed files: 0 hits.

⚠️ pnpm lint (repo-wide eslint . --no-inline-config) run whole under the lock, VERDICT command-exit 0 — so there is no narrowing to declare.

ONE gate NOT MEASURED, by its own printed verdict — neither a pass nor a red: check-test-completeness.mjs exit 3, PREREQUISITE NOT MET — this gate grades a saved 'turbo run test' log, and no log was named. CI tees that log; a local run cannot produce it.

⚠️ check:type-check-debt first answered exit 1 with a build prerequisite, not a red: --re-measure cannot run: 1 workspace dependenc(ies) ... have a type entry point OLDER than their own sources (the ablation legs had moved engine.ts's mtime after the closure build). That closure was cleared exactly as the gate named it (pnpm --filter @objectstack/service-automation build) and the gate re-run green, rather than reported as a failure.

Ablation — two legs, direction predicted before each run, both matched exactly

The implementation was committed first (4c949ca39), then mutated in the working tree.

No rebuild is in the resolution path, and here is why: the test imports ./engine.js and ./suspended-run-store.js relatively, from the same package and directory, so vitest resolves them to src, never through a package exports map to dist. The colour changes below are themselves the corroboration — a stale artefact cannot redden ten named tests and then green them again.

Mutation confirmed ON DISK, never by an editor's exit code. HEAD blob 883fc7fb00cfd31157467d0f402738d04983888b. Both mutation scripts refuse unless every anchor matches exactly once, both abort if the mutated blob equals the HEAD blob, and both count the injected marker AND the deleted text on disk. (Leg B's first attempt aborted on that count — the deleted-text pattern also matched resumeInternal's own RESUME_IN_PROGRESS guard, so the anchor was tightened and re-run rather than waved through.)

  • A — the journal is never recorded (so no snapshot can exist and no exit is possible). Mutated blob 6d66db03a2ec7ef3c15ac4455da8090b528b0ef6, injected marker 2, deleted text 0. Predicted: the ten restore-dependent pins redden; every refusal pin stays green, because none of them depends on a snapshot existing. Observed: Tests 10 failed | 12 passed (22) — the ten being the three exit pins, both idempotence-restore pins, both trace pins, and the three across-a-restart pins.
  • B — the in-flight guard removed. Mutated blob 346ec81e3d069839df7375d8e627abd22c332241, injected marker 1, deleted text 0. Predicted: exactly one pin reddens, the in-flight refusal. Observed: Tests 1 failed | 21 passed (22), expected 'NO_CONSUMED_SUSPENSION' to be 'RESUME_IN_PROGRESS'.

⭐ What leg B actually measured, reported honestly rather than overclaimed: the guard's contribution here is the reason, not the outcome. With it removed, a restore attempted mid-traversal still cannot re-arm anything, because the journal entry is written in the catch arm and does not yet exist — the durable design makes that instant structurally unreachable. The guard is what makes the engine say the true thing instead of "nothing to restore". ⚠️ There is a narrower window it does cover for real — between the journal write and resuming's finally — which is guarded but not pinned; pinning it needs a seam inside the catch arm that this slice does not add.

DECLARED CONTROLS — green in BOTH directions, reported as controls and NOT as ablation evidence: under A, all eight refusal pins, "does not itself traverse" (with no journal the restores refuse, so the traversal count is 1 either way), and the three "what this slice deliberately does NOT do" pins. Under B, everything except the one named pin.

RESTORE PROVEN BY STATE after each leg, not by an exit code: git diff HEAD empty, git hash-object back to 883fc7fb..., residual ablation markers 0. The harness carries trap restore EXIT INT TERM with absolute paths resolved from git rev-parse --show-toplevel, and restores with git checkout HEAD -- ABSOLUTE_PATH — never a bare git checkout --, which reads from the index the mutation itself wrote.

The pins earned their keep before any ablation. An early revision of this branch silently dropped the await this.forgetSuspendedRun(run, 'resumed') call while inserting the step-count capture beside it — i.e. it accidentally implemented #13937 shape 2, the very thing this slice must not pre-empt. leaves the resume ordering exactly as it is is the pin that caught it, and it is in the file for that reason.

NOT MEASURED — needs the deployment, not this repo

How many runs are in this state in any real deployment cannot be read from this repo, and no in-repo number here is a deployment answer. Slice 1 measured that the in-product inspector would have answered 0 before its widening, so an in-product zero was not evidence either. Sizing still needs an operator census over sys_automation_run (status='failed') joined against sys_approval_request — and note this verb changes nothing about that: a run stranded before this ships carries no snapshot, so it answers NO_CONSUMED_SUSPENSION. ⚠️ This exit is available to runs stranded from here on, not retroactively. Rescuing already-stranded rows is a data question (their variables were deleted with their paused row) and is not something code in this repo can answer.

Boundaries honoured

⛔ The resume ordering, forgetSuspendedRun and traverseNext are untouched — pinned, not asserted. ⛔ No platform-level name or status for the condition. ⛔ The approvals reject door is untouched, and so is the ledger #13568 covers. ⛔ Nothing under content/docs/releases/. #13937 stays open and unruled; #13807 is not addressed here.


Generated by Claude Code

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/service-automation, touching 35 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via runId (symbol, a field of interface SuspensionRestoreResult))
  • content/docs/automation/approvals.mdx (via nodeId (symbol, a field of interface SuspensionRestoreResult), runId (symbol, a field of interface SuspensionRestoreResult), /automation/:name/runs/:runId (route, a path literal in AutomationEngine))
  • content/docs/automation/flows.mdx (via flowName (symbol, a field of interface SuspensionRestoreResult), nodeId (symbol, a field of interface SuspensionRestoreResult), runId (symbol, a field of interface SuspensionRestoreResult), getRun (sdk, the bare tail of client method automation.getRun, bound to GET /automation/:name/runs/:runId), /automation/:name/runs/:runId (route, a path literal in AutomationEngine))
  • content/docs/automation/jobs.mdx (via flowName (symbol, a field of interface SuspensionRestoreResult))
  • content/docs/kernel/cluster.mdx (via nodeId (symbol, a field of interface SuspensionRestoreResult))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17.mdx (via AutomationEngine (symbol, a top-level class), flowName (symbol, a field of interface SuspensionRestoreResult), nodeId (symbol, a field of interface SuspensionRestoreResult), runId (symbol, a field of interface SuspensionRestoreResult), getRun (sdk, the bare tail of client method automation.getRun, bound to GET /automation/:name/runs/:runId), /automation/:name/runs/:runId (route, a path literal in AutomationEngine))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 6 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 836a29c27aa9dbac27cc643106e5b2bf68cc96c1packageMentionDocs.

Which tree this was computed on

This run read content/docs from 34b191f34ce9b8cf0cc822dfcbc5dbb989fc1dc0 — the merge of head f422d353a47903e24282f415bce247084a3c7e64 into base 836a29c27aa9dbac27cc643106e5b2bf68cc96c1, 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 34b191f34ce9b8cf0cc822dfcbc5dbb989fc1dc0 && git checkout 34b191f34ce9b8cf0cc822dfcbc5dbb989fc1dc0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 836a29c27aa9dbac27cc643106e5b2bf68cc96c1 f422d353a47903e24282f415bce247084a3c7e64 && git checkout -B drift-repro 836a29c27aa9dbac27cc643106e5b2bf68cc96c1 && git merge --no-ff f422d353a47903e24282f415bce247084a3c7e64

node scripts/docs-audit/affected-docs.mjs --json 836a29c27aa9dbac27cc643106e5b2bf68cc96c1

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 836a29c27aa9dbac27cc643106e5b2bf68cc96c1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

⚖️ 契约复审裁决 — FAIL。逐字采纳,⛔ 标签不清,交回返工

domain:services 派发席 #6021 记录。本裁决由达档子代理产出并经转录核验,下方原文逐字转录 —— ⛔ 未改写、未删节、未润色(父会话仅两个合法动作:逐字采纳或整体作废)。

核验读数(采信的前提)

CONTRACT_REVIEW_TIER                = claude-fable-5      (dispatch-gates.mjs:6895,单源)
本派发会话 last_served_model        = claude-opus-5       ⇒ ⛔ 自会话不达档,自会话复审整场跳过
复审子代理转录 harness 盖章统计:
  assistant 轮次 ............ 40
  盖章 "claude-fable-5" ..... 40
  非 fable 盖章(回退证据) ... 0
⇒ 逐轮达档,核验通过,裁决合法。

CONTRACT REVIEW — PR #13951 @ 4c949ca39
VERDICT: FAIL
INCREMENT REVIEWED: The published-surface widening of @objectstack/service-automation on the content limb — the new public method AutomationEngine.restoreConsumedSuspension and its eight-value refusal vocabulary, the RunRecord.consumedSuspension widening, the changed terminal-row write/read behavior of ObjectStoreSuspendedRunStore, and the claimed no-new-column/no-new-status durability posture — not the ablation, not the test design, not #13937's ordering question.
FINDINGS:
  1. packages/services/service-automation/src/index.ts — the refusal vocabulary is published de facto but unnameable de jure. The barrel (untouched by this PR; zero mentions of the new names) does not export SuspensionRestoreResult or SuspensionRestoreRefusal, and package.json's exports map publishes only "." — so a consumer can call the barrel-reachable restoreConsumedSuspension and receive the eight values at runtime, but cannot import the type to annotate a result, type a handler parameter, or write an exhaustive switch over the union. That contradicts the increment's own stated purpose ("an operator whose repair is refused has to be able to tell…" — consumers branch on these), and it breaks the package's measured convention: every other non-primitive result type of a public engine method is nameable (AutomationResult via spec; SuspendedRun, RunRecord, FlowShadowingRecord etc. in the barrel), and the cancelRun precedent needed no export only because it returns Promise<boolean>. The gap has no test witness because consumed-suspension-restore.test.ts imports from './engine.js', not the barrel. Fix: add `export type { SuspensionRestoreResult, SuspensionRestoreRefusal }` to the barrel's engine.js type block. (ConsumedSuspension may stay unexported — it appears in no barrel-reachable signature; RunRecord.consumedSuspension is typed SuspendedRun.)
  2. packages/services/service-automation/src/sys-automation-run.object.ts — the published object declaration (itself a barrel export, SysAutomationRun; its description strings feed operator-facing help) now asserts invariants this PR falsifies: node_type's description says it is "Null … on terminal history rows", and the trigger-attribution comment says "`context_json` is not even written on terminal history rows" — both false since this PR for the consumed-suspension class of terminal row, and the new load-bearing meaning (variables_json present on a terminal row ⇔ restorable snapshot; it is the deserializer's discriminator) is recorded only in suspended-run-store.ts internals, nowhere at the declaration. The PR proves it knows this staleness class — it corrected the identical claim in plugin-approvals/src/backfill-platform-row-organizations.ts — but skipped the declaration its own engine.ts docblock cites as authority ("`sys_automation_run` says so at the field itself"). Fix: the same one-class correction at the node_type description and the trigger-attribution comment, plus one sentence (at variables_json or the object header) naming the presence-discriminator.
  minor (non-blocking): engine.ts's NO_CONSUMED_SUSPENSION docblock claims "the run exists and is terminal", but the fall-through arm is also reachable when getRun observes a non-terminal status ('running'/'pending'); the result's reason string honestly names the observed status, so only the docblock over-claims.
  minor (non-blocking, stated in-code as a measured limitation): after a restart the durable getRun/listRuns read a restored run 'failed' while listSuspendedRuns lists it live — a deliberately minted state the terminal-wins rule previously reserved for corruption; acceptable only because it is stated at the site and the durable naming is #13937's same-batch sub-item.
BASIS:
  - Merge-base 0f63965ea; changed files: .changeset/automation-consumed-suspension-restore.md, plugin-approvals/src/backfill-platform-row-organizations.ts (comment only), service-automation/{engine.ts, suspended-run-store.ts, consumed-suspension-restore.test.ts}. No packages/spec path (path limb confirmed silent); `git diff … -- src/index.ts` empty (barrel untouched); package.json exports map publishes "." only.
  - Q1 enumeration from the barrel: NEW reachable = AutomationEngine.restoreConsumedSuspension(runId, {requestedBy, reason}?) → Promise<SuspensionRestoreResult> (public, engine.ts:5374 at head) and RunRecord.consumedSuspension?: SuspendedRun (both type names already exported); CHANGED behavior of existing export ObjectStoreSuspendedRunStore (terminal upsert now always writes variables_json/context_json/screen_json/node_type/correlation — value or explicit NULL, so a later terminal record self-clears; reads rebuild the snapshot keyed off variables_json). NOT nameable: SuspensionRestoreResult (engine.ts:1274), SuspensionRestoreRefusal (:1255), ConsumedSuspension (:1207), MAX_CONSUMED_SUSPENSIONS (:633) — exported from engine.ts, absent from index.ts (grep count 0), deep import blocked.
  - Q2: eight values counted (RESTORE_IN_PROGRESS, RESUME_IN_PROGRESS, RUN_SUSPENDED, STORE_UNAVAILABLE, RUN_COMPLETED, RUN_CANCELLED, NO_CONSUMED_SUSPENSION, RUN_NOT_FOUND); each earned by a distinct observation with a distinct remedy and each pinned by its own test (test list read). STORE_UNAVAILABLE spans two read sites with one remedy (distinct reason strings); RUN_SUSPENDED spans "never stranded"/"already restored" with one operator action — both benign. NO_CONSUMED_SUSPENSION's refusal to pick a cause measured HONEST: pre-#13909 rows carry no snapshot, over-budget snapshots are dropped-not-truncated (logged with size at write time, suspended-run-store.ts), store-less eviction (MAX_CONSUMED_SUSPENSIONS=50) leaves no tombstone — no discriminator exists short of the new column this PR rightly declines pending #13937; the reason names the observed status instead. Method never throws; every path returns a result. Live-pause check covers store-less engines (loadSuspendedRunStrict reads the in-memory suspendedRuns map first, engine.ts:4521), and persistSuspendedRun re-arms hot map + store, so the second-pause hazard is closed on both engine shapes.
  - Q3: precedent verified — cancelRun is a public engine method at merge-base (engine.ts:4984) and absent from IAutomationService (packages/spec/src/contracts/automation-service.ts:387→end; interface stops at getSuspendedScreen; grep: no cancelRun, no restoreConsumedSuspension). Holds for this verb: semantics defined entirely by engine internals the contract does not describe (consume-before-traverse), no transport exposure added, contract unions are explicitly closed and grown by the spec seat, and engine placement leaves #13937's naming space unclaimed. Placement sound despite the stronger re-arming semantics — the safeguards (restoring guard, strict store reads, paused-row-keyed idempotence, resume re-entering the ordinary authority gate) are engine-local too.
  - Q4: sys-automation-run.object.ts diff vs merge-base is empty; all five columns pre-exist as declared fields; status select stays ['running','paused','completed','failed'] — restore writes an ordinary paused row plus an in-memory 'paused' log entry, no history row, snapshot cleared by explicit NULLs on the next terminal upsert (recordLog terminal branch). No new column, no new status: claim CONFIRMED. Reuse mechanically sound (columns hold exactly the shape they were declared for; only this path writes variables_json on terminal rows) — but the declaration's own prose now contradicts it: Finding 2.
  - Q5: spec diff empty; AutomationResult.status verified `'completed' | 'paused' | 'failed'` (automation-service.ts:281); ExecutionStatus already contains 'paused'/'cancelled' (execution.zod.ts:25). A restored run is describable as 'paused' and the engine says so in-process (pinned: "records the run as paused again"); the restart-window 'failed' reading is the second minor above. The one published thing that does acquire an unsaid new meaning is the terminal-row column presence — folded into Finding 2.

派发席的处置(⛔ 与裁决分开,裁决在上,原文未动)

⭐ 值得单说:Finding 2 抓的是「这个 PR 自己证明它知道这一类过期」 —— 它在 backfill-platform-row-organizations.ts 里更正了一模一样的说法,却跳过了它自己的 docblock 引为权威的那个声明("sys_automation_run says so at the field itself")。⇒ 不是没想到,是漏了一处;而漏的那一处恰是已发布的对象声明,其 description 会喂给运维可见的帮助文本。


Generated by Claude Code

claude added 2 commits August 31, 2026 23:58
… and true up the sys_automation_run declaration

Contract-review rework (ruling recorded on PR 13951; behaviour unchanged):

- index.ts: export type SuspensionRestoreResult / SuspensionRestoreRefusal in
  the engine.js type block. The verb was already barrel-reachable, so the
  eight refusal values were published de facto while the union was unnameable
  de jure — no exhaustive switch, no annotated result, no typed handler
  parameter. ConsumedSuspension deliberately stays unexported (it appears in
  no barrel-reachable signature).
- consumed-suspension-restore.test.ts: barrel-import pin. The existing suite
  imports from './engine.js', which is why the gap had no witness. The pin
  annotates a real result with the BARREL type and writes the exhaustive
  switch (never-typed default), so removing the export breaks tsc; the
  runtime half pins the verb on the class the barrel itself exports.
- sys-automation-run.object.ts: the declaration asserted invariants this PR
  falsified. One-class corrections only — the node_type description and the
  trigger-attribution comment now carve out the consumed-suspension class of
  terminal row (the same correction shape this PR already applied in
  plugin-approvals' backfill), and variables_json's description names the
  presence-discriminator the store's deserializer keys off.
- engine.ts: the NO_CONSUMED_SUSPENSION docblock no longer claims "exists and
  is terminal" — the arm also answers on a non-terminal logged status, and
  the reason string was already honest about that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…ntime strings

check:doc-authoring is right: the two descriptions the rework corrected
carried '#13909' inside runtime string prose, which reaches operators who
cannot resolve a tracker id. The carve-out text stays; the ids move to
adjacent comments (the reader who can resolve them reads the source).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Sep 1, 2026

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

契约复审(返工复核):PASS —— 逐字采纳,双载体标签已清

转录核验(采信前必做):

assistant 轮次:                    67
盖章 "claude-fable-5":             67
非 fable 盖章(回退证据):            0

⇒ 每一轮都在档、零回退 ⇒ 裁决合法,取逐字采纳。以下为复审席原文,⛔ 未改写、未删节、未润色:


契约复审(返工复核):PASS

结论一行

两项阻塞均已实缴:桶导出与经桶见证成立,对象声明三处与读写代码逐一相符,准予发布。

非阻塞

  1. 注释里的函数定位写法不精确——packages/services/service-automation/src/sys-automation-run.object.ts:236 说判别器 "lives in ObjectStoreSuspendedRunStore.deserializeConsumedSuspension",但 deserializeConsumedSuspensionsuspended-run-store.ts:623模块级函数(由类方法 deserializeTerminal:465 调用),不是类方法。实质无误(一写一读、以此列为键都对),只是点号路径会让人找错一层。清除方式:改写为 "suspended-run-store.ts 的 deserializeConsumedSuspension(经 ObjectStoreSuspendedRunStore.deserializeTerminal 调用)"。仅注释文字,不动契约。

我核了什么 / 我没核什么

核了(全部读 4c949ca39..f422d353a 区间及 f422d353a 全量源码,未运行任何套件):

  • R1 已实缴。 桶导出:src/index.ts:45-46 导出 SuspensionRestoreResult / SuspensionRestoreRefusal(engine.ts:1258、1277 处有真实声明)。见证的模块说明符是 ./index.js(consumed-suspension-restore.test.ts:59-63),不是 ./engine.js——值半边(BarrelAutomationEngine === AutomationEngine + 原型上有该动词)在 vitest 落地;类型半边(type-only 导入 + 注解 + 对 8 个拒绝值的穷尽 switch,与 engine.ts:1258-1266 的并集逐值对齐,含 never 兜底)依赖 tsc。我核实了该 tsc 通道真实存在:此包无 typecheck 脚本,但在 scripts/check-type-check-coverage.mjs:591 的 DEBT 台账上(errors: 3,tsconfig include: ["src"] 把测试收进同一 program),lint.yml 的 typecheck-debt 道对每个台账项重测且"any new error is red on arrival"——删掉桶导出会以 TS2305 把 3 顶破,CI 变红。见证不是盲点复刻。
  • R2 已实缴。 三处声明逐一对到代码:①终端行写入方 recordTerminal(suspended-run-store.ts:298-361)基础列不含 node_type/context_json/variables_json,四列只经 serializeConsumedSuspension(:570-610)写入,无快照时显式写 NULL(upsert 清除旧快照);②读方 deserializeConsumedSuspension(:623-624)恰以 variables_json 判空为键;③consumedSuspension唯一生产者是 resume 消耗暂停后下游节点抛错的那一处(engine.ts:4915-4927,12 个 recordLog 调用点里仅它传第三参,且 status 恒为 'failed')——所以 completed 行不会带 variables_json,⇔ 双向成立。两处细微窗口也核过、均不证伪该句(句子明确限定"the row carries"):restore 后历史行保留快照期间,二次 restore 得到诚实的 RUN_SUSPENDED 拒绝;超字节预算时行上无快照但同进程热日志仍可 restore——行本身确实"carries none"。node_type 保留声明与 :607 node_type: run.nodeType ?? null 相符,automation: the generic run-resume route needs an authorization gate keyed on the suspended node #3801 门经 restore 写回的 paused 行重新上膛。
  • R3 无新增负债。 增量之外只有:engine.ts 的 NO_CONSUMED_SUSPENSION 文档纠真(旧文"exists and is terminal"是过度声明;新文说 fall-through 不要求终态,与 :5479-5501 实现相符——running/pending 落此臂可达);第二个提交仅把 "(service-automation: a resume consumes the pause BEFORE running downstream nodes, so any node that throws leaves the run terminally unresumable — and the only inspector for it reports all clear #13909)"/"(since service-automation: a resume consumes the pause BEFORE running downstream nodes, so any node that throws leaves the run terminally unresumable — and the only inspector for it reports all clear #13909)" 两个 tracker id 从两条 description 字符串挪到相邻代码注释,carve-out 叙述原文未动、准确性未损、可追溯性由注释保留。测试文件新增(remedyFor + 两个 it)是 test-only,不是契约面。桶注释里 "ConsumedSuspension 不导出,因为不出现在任何桶可达签名"核实为真(仅私有成员使用;RunRecord.consumedSuspension 的类型是已导出的 SuspendedRun)。
  • R4 排除项未越界。 区间只动 4 个文件;engine.ts 的 hunk 全部是 JSDoc 行,forgetSuspendedRun/traverseNext/resume 顺序零触碰;重启窗口 durable naming 的"Measured limitation"注释(engine.ts:5545 附近,归 [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)未被区间改动。
  • STALE TREE 核实,不构成阻塞。 merge-base 0f63965ea,现落后 origin/main 53 个提交(dev 量得 44 时为真,main 在动)。其中触及 packages/services/service-automation只有一个:db9c460cf(test(service-automation): pin that a node failing mid-resume strands the run (#13807 step 1 measurement) #13899,+180 行纯新增测试 resume-node-failure-strands-run.test.ts,只从 ./engine.js 导入,断言 strand 经 resume/cancelRun 不可达)。桶、类型、对象声明零触碰——发布的契约文字不被落后证伪。

没核(NOT MEASURED,及归属):

  • 任何套件/构建/tsc 的实际运行——verify 锁饱和,遵令未跑。dev 报的 1116 → 1118 绿与 check:doc-authoring 通过,由 CI 度量。
  • 合并树上 db9c460cf 的 strand pin 与本 PR 共存是否仍绿——静态读断言(strand 仍记 failed、resume 仍 RUN_NOT_FOUND、cancelRun 仍 false)不受本 PR 改变,但合并后套件由合并时 CI 度量。
  • typecheck-debt 道对本分支的实测计数(3 是否恰好保持)——由 CI 的 typecheck-debt lane 度量;我核的是通道存在与红线机制,不是本次读数。
  • 首轮已裁定项(placement、拒绝词汇表、ablation、披露边界、changeset)一律未重审,遵派工令。

PM 侧收口

非阻塞项 1 的处置:不再开一轮返工

⛔ 不为一行注释的点号路径再走一趟「返工 + 再复审」—— 那一轮的成本远超收益,而复审席自己判它 非阻塞

⇒ 按仓内既有惯例记成待搭车项:sys-automation-run.object.ts:236 那句应改为「suspended-run-store.tsdeserializeConsumedSuspension(经 ObjectStoreSuspendedRunStore.deserializeTerminal 调用)」,由下一张动到该文件的卡顺手带走。⚠️ 记在这里而不是只留在复审正文里,是因为今晚本车道刚为同一形状付过学费:#7401 的更正只活在评论里,结果解锁扫描照旧读 body、又误放行了一次。


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review September 1, 2026 04:49
@os-steve
os-steve enabled auto-merge September 1, 2026 04:49
@os-steve
os-steve added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit 4bb09e8 Sep 1, 2026
37 checks passed
@os-steve
os-steve deleted the claude/issue-13909-operator-exit-verb branch September 1, 2026 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants