fix(approvals): auto-cancel a record's pending approvals when the record is deleted

[claude]

Fixes #13568

Deleting a record left every pending approval it had opened sitting in the approvers' inbox — counted in the pending total, openable, and pointing at a record_id that resolves to nothing. Nothing about it was module-specific: an approval node that declares lockRecord blocks the edit, so "delete and recreate" is the only route left to an author who needs to fix a submitted record, and every such delete added another orphan.

Maintainer ruling 2026-08-31 (总监席第 5 场决裁批 #5, verbatim 「同意」): pending requests auto-cancel on record delete — status cancelled plus a machine-readable reason, rows kept for audit, out of the pending count and the inbox's default view. Landing is the approval service's record-delete lifecycle linkage, platform-level and once, so every "approval + lockRecord" combination benefits.

What landed

Contract (packages/spec/src/contracts/approval-service.ts)

• APPROVAL_STATUSES gains cancelled (+ its label). Its own terminal state rather than a re-use of recalled: a recall is an act by the submitter, and filing a platform-initiated void as one attributes a withdrawal to a person who never performed it.
• New APPROVAL_CANCEL_REASONS / ApprovalCancelReason / APPROVAL_CANCEL_REASON_LABELS, single entry record_deleted. A vocabulary rather than free text because the reason has a non-human consumer, and a class per the ruling's wording, so the next platform-initiated cancellation cause extends this list instead of minting a second terminal status.
• APPROVAL_ACTION_KINDS gains cancel — the only kind with no human actor, by construction.
• ApprovalRequestRow.cancel_reason declared, optional-nullable.

Plugin (packages/plugins/plugin-approvals)

• sys_approval_request.cancel_reason, a select derived from the contract vocabulary and never re-typed. On the row, not on the audit entry, so a plain list view reads WHY without joining the append-only action log.
• bindRecordDeleteCancelHook — a global afterDelete registration beside the existing global record-lock hook. It needs no row-set plumbing: the engine binds the deleted row's pre-image on the by-id path and fans afterDelete out per matched row on a predicate delete, so a bulk delete is covered by the same handler. The five approvals tables are excluded at registration, so they do not pay the delete-side pre-image read.
• ApprovalService.cancelForDeletedRecord writes the transition: one append-only sys_approval_action row (action: 'cancel', no actor), status: 'cancelled' + cancel_reason: 'record_deleted' + completed_at, and a sys_approval_approver index clear — that last one is not garnish, it is what actually empties the inbox, because the approver filter resolves through that index rather than through status.
• The Completed list view now includes cancelled, so a kept audit row is visible in the one curated terminal view rather than only under All.

Held to the ruling, explicitly

• ⛔ No flow resume and no status mirror-back. A cancellation is a status write plus a reason, not a decision. The mirror-back is skipped by construction, not by a swallowed error: it is an update_record against the row that was just deleted — the exact write this card's forensics caught failing elsewhere. The suspended run is reported at warn with its id and otherwise left alone; 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 / POST /api/v1/approvals/requests/{id}/reject returns 500 while its effect lands AND strands the workflow run — three inconsistent outcomes from one call #13807 are not addressed here and remain open.
• ⛔ The delete is never refused. The "forbid delete while an approval is pending" direction was vetoed in the same ruling. Nothing in the hook throws; a failure degrades to the pre-existing state and is logged.
• ⛔ Terminal rows untouched. approved / rejected / recalled / returned keep their recorded outcome. The tombstone presentation of their now-dead record reference is the separate console-side card, out of scope here.
• returned is deliberately out of the cancel set. The ruling names pending, the record lock is keyed on pending, and the inbox count filters on pending — so a returned request is not part of the reported symptom. Called out rather than silently decided.

Seam correction

The dispatch anchored the landing site at packages/services/plugin-approvals. That path does not exist — the package lives at packages/plugins/plugin-approvals. Same package, different directory; no behavioural consequence. Reported in the claim comment before the first edit.

Clause-2: YES (content-limb)

Declared from the actual diff, upward only. No existing declared field honestly carried a machine-readable cancellation reason: submitter_comment is submitter prose and sys_approval_action.comment is free text — the "not machine-readable" shape the ruling asks to avoid. needs:contract-review is attached and this PR stays draft pending that review.

Verification

Gate union and the suites below were run on the final commit, 408716c1.

• New pins — packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts, 11 cases on a real ObjectQL over @objectstack/driver-sql + better-sqlite3 :memory:, the real sys_approval_* schemas and the real ApprovalService (requests opened by openNodeRequest, so the approver-index rows the tests assert the disappearance of were written by production code). Positive: the row survives as cancelled + record_deleted, one no-actor audit row, the pending count and inbox both empty, the approver index cleared, the reason surfaced through the service read path, and a predicate/bulk delete covering every matched row. Negative: a terminal request about the same record untouched; a pending request about a different record untouched; no resume and no cancelRun on the automation spy; and the delete still succeeds when the bookkeeping write fails.
• Control case, in the same file — the same delete with the hook not bound reproduces the reported symptom exactly (row still pending, count still 1). Without it, the positives would also pass against an engine cancelling for some unrelated reason.
• Ablation — engine.registerHook('afterDelete', …) in bindRecordDeleteCancelHook neutered on a committed tree, mutation confirmed on disk by anchor counts (1 injected, 0 unmutated originals) and by blob hash (10159f22… to f913e884…): 8 of 11 red. The 3 that stayed green are the two negatives and the control, i.e. exactly the cases that must be insensitive to the linkage. Restored via git checkout HEAD -- path under a trap with absolute paths; restore proved by on-disk hash equal to the HEAD blob and an empty git diff HEAD, then re-run green 11/11. No rebuild leg: the ablated subject is reached through a relative source import, not through a package exports map, so nothing in dist/ participates — and the red proves the mutation reached the resolution path.
• Suites — pnpm --filter @objectstack/plugin-approvals exec vitest run → 35 files / 643 tests passed. typecheck green for @objectstack/spec, @objectstack/plugin-approvals and @objectstack/client.
• Contract reverse-verification — from a consumer package against the rebuilt .d.ts: ApprovalStatus = 'cancelled' and ApprovalCancelReason = 'record_deleted' compile (exit 0); mutating the reason to a bogus literal goes red with TS2322: Type '"record_vanished"' is not assignable to type '"record_deleted"'. So the green above is a reading of the new declarations, not of a cached build.
• Generated artifacts — pnpm --filter @objectstack/spec check:generated reported api-surface/ and export-origins/ stale; both regenerated with --fix. The four locale bundles were regenerated with the plugin's own os i18n extract command, and zh-CN / ja-JP / es-ES carry authored translations for the new leaves (已作废 / 無効化済み / Anulada) rather than English source fills — re-running the extract afterwards returned the source-hash companions to their pre-change contents, which is the instrument confirming the leaves are no longer fills.
• Gate family — derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (16 paths, merge base e612dd0f5), re-derived after the census edits pulled in the docs family: 55 of 58 green. check-tenant-audit-census was a genuine red — cancelForDeletedRecord adds two write call sites, moving the census 215 to 217; regenerated with node scripts/tenant-audit-census.mjs --write and the eight hand-written prose figures the gate holds to it updated in the same commit.
• NOT MEASURED, not green — three gates exit on an unmet prerequisite in this container and say so themselves: check-dev-prereqs and check:dual-build-cjs-loads need a fully built workspace (exit 3, "this is NOT a pass: nothing was measured"), and check-test-completeness needs a CI turbo run test log it cannot produce locally (exit 3, explicitly not a finding). CI owns all three.
• Repo-wide lint — pnpm lint (eslint . --no-inline-config) run in full, exit 0. No narrowing to declare.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/spec, touching 28 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

21 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json afbf271143715707a96d6255aee08261bf9ac15f.

⛔ 2 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see

• 2 changed file(s) yielded no anchor (packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json) — pages documenting those are invisible to this run
• 3 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 — 128 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 afbf271143715707a96d6255aee08261bf9ac15f → packageMentionDocs.

Which tree this was computed on

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

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

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