Skip to content

feat(spec): declare the operator run-lifecycle verbs cancelRun and restoreConsumedSuspension on IAutomationService (contract half of the #13953 ruling) - #16563

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-16495-automation-service-cancel-restore-contract
Sep 7, 2026
Merged

feat(spec): declare the operator run-lifecycle verbs cancelRun and restoreConsumedSuspension on IAutomationService (contract half of the #13953 ruling)#16563
huangyiirene merged 2 commits into
mainfrom
claude/issue-16495-automation-service-cancel-restore-contract

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #16495

Clause-②: yes

The contract half of the #13953 ruling A (director seat, decision batch #42, comment 5548737008 — maintainer 「13753 我让别人处理了,其他同意」). IAutomationService gains the two operator run-lifecycle verbs as optional members, typed as the engine implements them, with the ruling's persistent-face statement in both docblocks, plus a pin test and an @objectstack/spec minor changeset. The services half — the REST doors, the platform_admin check, the ADR-0112 envelope, any lister — stays with #13953 and is not addressed here. No CLI, no implementation.

What changed

  • packages/spec/src/contracts/automation-service.ts — two members appended to IAutomationService after getSuspendedScreen:

    • cancelRun?(runId: string, reason?: string) answering a Promise of boolean
    • restoreConsumedSuspension?(runId: string, options?: { requestedBy?: string; reason?: string }) answering a Promise of { restored: boolean; runId: string; refusal?: string; reason: string }

    Both docblocks quote the ruling verbatim — "listing and acting go through sys_automation_run (the persistent face), never engine memory" — and its permission posture ("gated on the existing platform_admin position (no new permission type, no per-run ownership — a run belongs to the environment, not a user)"), and state that a service not declaring a verb has NO operator door for it: the door must probe for presence and refuse fail-closed.

  • packages/spec/src/contracts/automation-service.test.ts — the spec: name the terminally-failed run state on AutomationResult.status — contract half of #13937 (shape 4 ruling) #14384-shaped pin: four exported type-level identities (parameter tuples and return types of both verbs), a minimal-implementation case (both verbs absent), a typed full-implementation case proving reason / requestedBy travel through the contract, a @ts-expect-error refusing a result without reason (compiled by check:test-typecheck), and a docblock-reading case asserting the ruling's phrases sit above each declaration.

  • .changeset/automation-service-operator-verbs-contract.md@objectstack/spec minor.

The three calls the ruling left open — made as the director's grading comment (5567526365) rules, with the reasons

  1. Signatures follow the engine, not the ruling's verb(runId) shorthand. cancelRun(runId, reason?) and restoreConsumedSuspension(runId, options?: { requestedBy?, reason? }). A door calling through the contract must be able to say who asked and why: restore's trace records exactly those and writes not recorded when requestedBy is absent; cancel's reason lands on the terminal cancelled log's error. Both engine methods were re-located by symbol on f48f3f1b21: AutomationEngine implements IAutomationService at engine.ts:1718, cancelRun at :6262, restoreConsumedSuspension at :6551 — the card's anchors hold today.
  2. Return type: route (i), an inline structural result — no new export. The engine's SuspensionRestoreResult / SuspensionRestoreRefusal (engine.ts:1451 / :1431) stay with the engine; the contract declares { restored, runId, refusal?: string, reason } inline, using the engine's own member names so the wider type satisfies the contract under implements without any rename, and refusal typed string rather than an enumeration this contract would have to keep in step (the director: no second consumer needs the eight codes yet; one that does is a card). Inline rather than a named exported alias on purpose: listSuspendedRuns on this same interface is the house precedent, and a named export would add a row to api-surface/contracts.json and export-origins/contracts.json — the shards PR api-surface/ records a name declared as BOTH a const and a type under (type) only — deleting the value half of any of 132 such exports is invisible to check:api-surface #15919's in-flight repair rewrites wholesale. Measured after the build: check:api-surface "public API surface + factory signatures unchanged", check:export-origins "5277 exports across 17 entry points resolve exactly as recorded" — the card's own baseline numbers; zero baselines moved. Deliberately omitted from the contract result: the engine's flowName / nodeId / consumedAt — the door's shape per the ruling is restored + refusal + reason; if the REST door wants to echo the re-armed node, that widening is the services half's to raise.
  3. Optional members. 13 of the interface's 15 members are optional; the "minimal implementation = { execute, listFlows }" pin stays green, zero implementors break, and the docblocks turn absence into a rule: no member, no door, fail-closed. Required would have redded that pin plus the typed test literals across spec, service-automation and runtime, for no consumer that needs it.

Zone 2 — who calls the verbs today (re-measured on f48f3f1b21)

  • The only non-test implements IAutomationService is AutomationEngine (engine.ts:1718); the only non-test 'automation' slot registration is plugin.ts:582.
  • cancelRun: one in-process caller, plugin-approvals' revise-window recall — through its own duck-typed ApprovalResumeSurface (approval-service.ts:134), not through this contract. Unchanged by this PR.
  • restoreConsumedSuspension: zero non-test call sites (mentions in engine prose and log text only).
  • Typed consumers of the contract: runtime/src/domains/automation.ts:631 (a Partial of the contract) and :855; spec/src/contracts/core-service-contracts.ts:80. Each sees two new optional members and nothing else.
  • ADR-0087 does not trigger (additive optional members): check:adr-0087-registration — "this PR adds no declared-breaking changeset".

Verification (all on 60aaf369b6, the only commit; base f48f3f1b21)

  • Build: pnpm --filter @objectstack/spec build under the shared verify lock (held 322 s, exit 0). Generated footprint: zero tracked files changed by the build (git status --porcelain empty); gitignored only — packages/spec/dist/, packages/spec/json-schema/, .turbo/.

  • pnpm --filter @objectstack/spec check:generated — "All 15 generated artifacts are up to date" (react-declaration-parity cannot run here, as designed).

  • Spec gates by name: check:api-surface unchanged · check:export-origins 5277 / 17 exactly as recorded · check:exported-any · check:dual-source-exports · check:docs 228 in sync · check:authorable-surface · check:test-typecheck OK with the ledger held at 54 files / 261 errors / 145 signatures (this file's one pre-existing TS2739 entry unchanged, so the @ts-expect-error fired) · browser-reachable-entries · entry-nameability · llms-txt · duration-unit-keys · empty-state · liveness · objectui-pin-citations · skill-refs · strictness-ledger · variant-docs · yaml-examples — all exit 0.

  • Spec typecheck, all three parts of its script: tsc --noEmit -p tsconfig.json 0 errors · check:scripts-typecheck 0 errors · check:test-typecheck OK.

  • Tests: every spec test that reads this contract file (enumerated by grep over src/**/*.test.ts for the import or the source path; 6 files, the fs-reading automation-result-status.pin.test.ts among them as the control) — vitest run --maxWorkers=2 ⇒ 6 files / 199 tests passed. Declared narrowing of the full spec suite: vitest strips types, so only a test that imports or reads this file can observe the change; CI runs the suite whole.

  • Root families from dispatch-gates --ran: 75 derived, 74 run, 1 unrun — check:type-check-debt (a cross-package --re-measure; declared to CI: an additive optional member cannot raise a consumer's tsc error count). check:dual-build-cjs-loads answered exit 3 — PREREQUISITE NOT MET (other packages' dist absent) — recorded as NOT MEASURED, not as a pass.

  • Cross-package reverse verification (the engine's implements), narrowed by declaration: service-automation's typecheck needs its 21-package dependency closure built, beyond the foreground cap under this container's lock queue. Substitute, measured against the rebuilt dist/contracts/index.d.mts: a class carrying the engine's exact method signatures (copied from engine.ts, the wider SuspensionRestoreResult included) compiles under implements IAutomationService (exit 0); the control with cancelRun answering a number and a reason-less restore result is refused on both members (TS2416 ×2) — before this change neither member existed on the interface, so the control proves the rebuilt declaration is what was read. CI's TypeScript Type Check job covers the closure.

  • Lint, narrowed with the three evidences: population = eslint . under eslint.config.mjs (five global ignores: node_modules, dist, build, .next, .turbo; the two files fall under its **/*.{ts,…} and packages/**/*.{ts,…} blocks); files linted = 2 (read from --format json), 0 errors / 0 warnings; invariance = the config never enables type-aware linting ("no parserOptions.project, no typed @typescript-eslint rules" — its own comment at lines 326–328), so this diff cannot move any untouched file's verdict.

  • Ablations, each with an on-disk landing proof and a HEAD-anchored restore proven by git status --porcelain empty and the blob hash back to HEAD 784aa349…:

    • (a) both occurrences of "never engine memory" replaced (count 2 → 0; mutated blob 19067c74…) ⇒ the docblock pin fails on exactly that phrase (vitest exit 1: 1 failed / 14 passed). Restored.
    • (b) cancelRun narrowed to the ruling's shorthand cancelRun?(runId: string) (count 1 → 0; mutated blob a36ecc5e…) ⇒ check:test-typecheck red, naming two ARRIVED signatures on the test file: TS2344 (the identity pin) and TS2554 (the two-argument call). Restored.

    Neither leg needs a build — the pins read source and compile under tsconfig.test.json — so the false-green build-regeneration trap the card warns about does not apply to them.

  • Byte hygiene: check:nul-bytes OK over 8144 files; control-character self-scan of the three touched files: no hits.

Not in this PR

The REST routes, the platform_admin check, the ADR-0112 refusal envelope, any lister, any CLI — #13953's services half, which remains open. The A/B/C fork is ruled and not reopened. No out-of-scope findings were met.

needs:contract-review is on both carriers; this PR stays draft and is not enqueued by its author.

🤖 Generated with Claude Code


Generated by Claude Code

…ationService

The contract half of #13953's ruling A: both operator run-lifecycle verbs
are declared as optional members, typed as the engine implements them
(cancelRun(runId, reason?) -> boolean; restoreConsumedSuspension(runId,
options?) -> a narrower structural result), with the ruling's
persistent-face statement in their docblocks and a pin test in
automation-service.test.ts. @objectstack/spec minor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
@github-actions github-actions Bot added the size/m label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 3 documentable anchor(s).

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

  • content/docs/releases/v17.mdx (via IAutomationService (symbol, a top-level interface))

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

Coarse fallback — 130 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 ffca0df9cb09cd61410b92f3d952226b309a58c8packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json ffca0df9cb09cd61410b92f3d952226b309a58c8

⚠️ 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 ffca0df9cb09cd61410b92f3d952226b309a58c8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Contract review at CONTRACT_REVIEW_TIER (claude-fable-5-1) — PASS, no blocking findings

Anchors: head 60aaf369b6 (one commit), branch point f48f3f1b21, origin/main 90e7e6de11. main has not touched any of the six relevant files since the branch point (measured: empty diff). All measurements in a detached worktree at head, --offline, no build — so nothing below rests on the author's build or on any exit 99 leg.

The three unsettled calls all hold

Signatures. The engine's members at service-automation/src/engine.ts:1718/:6262/:6551 match the contract character-for-character and are byte-stable between base and origin/main. Verified the way it should be: an engine-shaped probe class compiles under implements IAutomationService, and the control — numeric cancelRun, reason-less restore result — is refused TS2416 ×2. The declaration emitter reproduces both members and both docblocks, so the dist leg is covered without a build.

Route (i). check:export-origins reports 5277/17 "exactly as recorded" — zero baselines moved, as the card's own probe predicted. One deviation from the grading is correct: the ruling glossed the refusal shape as code/message, but only the engine's own names (refusal/reason) let the wider type satisfy the contract under implements without forcing an engine rename. Taking the engine's names over the ruling's shorthand was the right call.

Optional members. The {execute, listFlows} minimal-implementation pin stays green (23/23). The fail-closed claim is neither kept nor false — it is half-delegated: the contract keeps the half it can (absence is observable as undefined, now pinned), and the door's half is exactly what grading 5567526365 §3 assigns to #13953. Worst case for an unprobed door is a thrown TypeError, never a silent 200.

The pin was checked for firing for the right reason, not merely firing: an (a-one) ablation — mutating just one of the two docblocks — still fails the same single test, proving the pin reads each docblock independently rather than passing on an aggregate.

⛔ Holding this PR unarmed for a one-sentence repair

Advisory 2 is small in size and not small in kind. automation-service.ts:691-692 says:

Answers true only when a suspension was consumed by THIS call.

Measured, and it is not true of the only implementation: the engine has no cancelling guard (0 mentions — only resuming/restoring exist), and cancelRun's forgetSuspendedRun(run, 'cancelled') omits the CAS flag, so the delete is unconditional, unlike resume's claimAdvance. Two overlapping cancels of one run both answer true and both write a cancelled log.

That is an exclusivity guarantee stated in a published contract that the implementation does not provide — and this contract exists precisely so #13953's door can be built against it. A caller entitled to read "I was the one who cancelled" out of true would send its notification twice. The engine's own docblock claims nothing of the kind, and the @returns line ("when this call cancelled a suspended run") is already defensible, so the repair is: soften that one sentence to match what the engine does.

The reviewer graded it advisory and offered "soften it, or note it for #13953's door." I am taking the first. A contract docblock is read as a guarantee, and this lane holds PRs for false claims in exactly this position — the same call I made on #16531's A4. Queued as the next dev dispatch when a slot frees; dev concurrency is at its cap of 3.

Also carried out before enqueue, and now done: advisory 1, the clause-② carrier. check-clause2-carriers --pair 16563 returned exit 4, NOT clause-② legible — my own claim comment on card #16495 declared the clause as a ## Clause ②: heading, which is a near-miss twice over (space instead of the hyphenated Clause-②, and ## is not an admitted prefix). Amended on the card as comment 5569882834, with the fixed spelling checked against the script's actual matcher rather than assumed. That was my defect, not this PR's.

Everything else checked and clean

@ts-expect-error fired (ledger held at 54/261/145; an unfired directive would have added TS2578 and turned the file red). Changeset minor parses; ADR-0087 does not trigger. The level axis was measured, not declared — feeding the PR's real labels and body as --event passes. Both NOT MEASURED legs were checked for what might hide in them and nothing does: the sole implementor is the engine, and plugin-approvals types via its own ApprovalAutomationSurface generic at the call site, never IAutomationService.

Two pre-notes routed to #13953 rather than ridden in here

The refusal vocabulary being refusal?: string means an ADR-0112 envelope needs a non-exhaustive string switch, and #13953's acceptance should pin the absent-member probe (absent verb → refusal envelope, never 200). Both belong to that card; neither widens this one.


Generated by Claude Code

The contract docblock said `true` is answered "only when a suspension was
consumed by THIS call". The only implementation gives no such guarantee:
the engine has no `cancelling` guard (only `resuming` / `restoring`), and
`cancelRun` consumes through `forgetSuspendedRun(run, 'cancelled')`, whose
store delete is by id and unconditional — nothing like `resume`'s advance
claim. Two cancels of one run overlapping in time therefore both answer
`true` and both record the terminal `cancelled` log.

Say what the engine does: `true` means this call cancelled a suspended run
(the `@returns` line, unchanged), NOT that it was the only one — a caller
may not read sole authorship out of it, nor use it as an idempotency token
for a once-only side effect. Prose only; the `@returns` line, the pin, the
changeset and the result shape are untouched. Whether the engine should
grow a cancel-side compare-and-set is the services half's call, not this
contract's.

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

Copy link
Copy Markdown
Collaborator Author

Repaired and armed — the exclusivity claim is gone, and the engine facts were re-measured independently

Head 60aaf369b6bd193c657b, one file, +11/−7, all inside the one docblock.

The repair seat did not take the reviewer's measurement on trust — it re-derived both facts on the branch tree (engine.ts unchanged against merge-base f48f3f1b21):

grep -c 'cancelling' engine.ts → 0        'resuming' → 25        'restoring' → 6

and traced the delete: cancelRun (engine.ts:6262) calls forgetSuspendedRun(run, 'cancelled') with no third argument and no claimAdvance; that function's store write is this.store.delete(run.runId) — by id, unconditional — unless durableRecordAlreadyConsumed is passed, and only resume passes it, after its compare-and-set (:5566, :5626). Two overlapping cancels therefore both read the row, both delete it (the second a no-op by id), both recordLog('cancelled'), and both return true. Confirmed.

⭐ Worth recording: the engine's own cancelRun docblock (engine.ts:6226-6232) never claimed exclusivity — it says only that false means no suspended run. The false guarantee existed only in the contract, which is precisely the surface #13953's door gets built against.

Before → after:

Answers true only when a suspension was consumed by THIS call, and false when…

Answers true when it cancelled a suspended run, and false when no suspended run exists under the id … true is NOT exclusive to this call — this contract carries no cancel-side exclusivity guarantee, so two cancels of one run overlapping in time can each answer true (and each record the terminal log): a caller may not read true as sole authorship, nor use it as an idempotency token for a once-only side effect.

The @returns line was already defensible and is unchanged. restoreConsumedSuspension, the pin, the changeset, the result shape and packages/services are untouched; main not merged forward.

⚠️ The correction is unprotected, and that is reported rather than papered over

The existing docblock pin asserts /idempotent/ and /could not READ/ on the cancelRun doc. /idempotent/ happens to land on a retained clause of the edited sentence — so the pin reads the sentence but asserts nothing about exclusivity. It was green on the old wording and is green on the new. ⇒ This correction can drift back silently. No pin was added, per the repair fence; the fact is recorded here instead, because a pin that would be right today is the wrong pin if the engine later grows the CAS.

Verification

All 34 check runs read, every one success or skipped, Lint & Repo Gates included. check:generated 15/15 with a clean tree; check:export-origins "5277 exports across 17 entry points resolve exactly as recorded"; check:api-surface unchanged. 75 gate families derived, 75 run, 0 UNRUN; 73 exit 0, two NOT MEASURED (check:dual-build-cjs-loads, check:type-check-debt, both exit 3 PREREQUISITE NOT MET) and declared to CI rather than counted.

One distinction the seat drew carefully and correctly: the docblock reaches no tracked generated artifact (content/docs/references/** has zero mentions of cancelRun or IAutomationServicebuild-docs.ts enumerates .zod.ts files, not contracts), but the gitignored dist/contracts/index.d.ts does carry the new sentence — so the check:api-surface verdict above was read against a dist containing the edit, not a stale one.

The engine question is routed, not dropped: whether cancelRun should grow a cancel-side compare-and-set mirroring resume's claimAdvance is the services half's call, and it is on #13953 (comment 5570352879) with both measurements and the trade-off spelled out. ⛔ Deliberately not filed as its own card and not done here.

needs:contract-review struck (review PASS, repair landed — card #16495 still carries it until merge, so the carrier pair stays satisfied while this PR is open). Ready flipped, auto-merge on.


Generated by Claude Code

@huangyiirene
huangyiirene added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit abb140c Sep 7, 2026
39 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-16495-automation-service-cancel-restore-contract branch September 7, 2026 12:31
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[spec] Declare cancelRun and restoreConsumedSuspension on IAutomationService — #13953's ruled contract half, step (1), never filed

2 participants