Skip to content

Commit dda969c

Browse files
claude[bot]claude
andauthored
fix(approvals): auto-cancel a record's pending approvals when the record is deleted (#14061)
* fix(approvals): auto-cancel a record's pending approvals when the record is deleted Deleting a record left its pending `sys_approval_request` rows in the approvers' inbox — counted, openable, and pointing at a record id that no longer resolves. Any object whose approval node declares `lockRecord` walks the same path, because the lock makes "delete and recreate" the author's only route to fixing a submitted record. Per the maintainer ruling: pending requests now transition to a new terminal `cancelled` status carrying a machine-readable `cancel_reason`, the rows are KEPT as audit evidence, and they leave the pending count and the inbox's default view (status write plus a `sys_approval_approver` index clear — the index is what the approver filter actually resolves through). The linkage is one global `afterDelete` hook beside the existing global record-lock hook, so it is platform-level and every "approval + lockRecord" combination benefits at once. It runs no flow node and mirrors no status back onto the deleted record; the suspended run is reported and left to the automation service. Terminal rows are untouched, and the delete itself is never refused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi * chore(audits): re-certify the tenant write-call-site census after the cancel path `cancelForDeletedRecord` adds two write call sites (the append-only `sys_approval_action` insert and the `sys_approval_request` status update), so the generated census moves 215 -> 217 and its hand-written prose figures move with it. Regenerated with `node scripts/tenant-audit-census.mjs --write`; the prose numbers the gate holds to the census (215/143/97) are updated in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi * test(approvals): declare the new integration test's read options instead of erasing them `check:query-options-erasure` counted 5 new sites (240 -> 245) from record-delete-cancel.integration.test.ts. None of them is deliberately off-contract — they are plain `where` + `context` reads — so the remedy the gate names is to type them, not to spell them `as unknown as EngineQueryOptions` (which would claim a contract bypass that is not happening) and not to raise the ratchet. Each of the five `engine.find` calls now carries `satisfies EngineQueryOptions`. Verified it is a real check rather than decoration: the file compiles clean under a tsconfig that includes it, and planting the #4674 shape the rule exists for (`direction: 'desc'`, an undeclared key) turns it red with TS2353. The ratchet is back at its 240 ceiling, unraised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi * docs(permissions): re-anchor the system-context census after the approvals insertions `check-system-context-census` reported 20 problems (10 site-without-a-row + 10 anchor-is-not-a-read-site) across plugin-approvals. Triaged all 20 against `origin/main` before touching anything: every one of the 10 sites exists verbatim in main's copy of the same file, and the 10 stale anchors are exactly main's line numbers for those same sites. So this is pure line rot from the `cancelForDeletedRecord` / `RECORD_DELETE_CANCEL_LIMIT` / `bindRecordDeleteCancelHook` insertions pushing existing reads down — zero new elevation reads on this branch, which the gate confirms independently by holding the site count at 109. In particular `lifecycle-hooks.ts:570` is NOT new code: it is the pre-existing `bindDelegationWriteGuard` system bypass (#1322 / #4839), main line 440. Repaired with the sanctioned `node scripts/check-system-context-census.mjs --fix` — anchors only. No `isSystem` check was deleted, weakened or re-worded, and no row's prose changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 96e25a8 commit dda969c

17 files changed

Lines changed: 958 additions & 37 deletions
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/plugin-approvals': minor
4+
---
5+
6+
fix(approvals): a deleted record's pending approvals auto-cancel instead of stranding in the inbox (#13568)
7+
8+
Deleting a record left every `pending` approval it had opened sitting in the
9+
approvers' inbox — counted in the pending total, openable, and pointing at a
10+
`record_id` that resolves to nothing. Nothing about it was module-specific:
11+
an approval node that declares `lockRecord` blocks the EDIT, so "delete and
12+
recreate" is the only route left to an author who needs to fix a submitted
13+
record, and every such delete added another orphan. Maintainer ruling
14+
2026-08-31 (`总监席第 5 场决裁批 #5`, verbatim 「同意」): pending requests
15+
auto-cancel on record delete — status `cancelled` plus a machine-readable
16+
reason, rows KEPT for audit, out of the pending count and the inbox's default
17+
view.
18+
19+
**Graded `minor`, and deliberately not `patch`.** The repair itself is a
20+
defect fix, but it lands by WIDENING two published vocabularies and adding a
21+
declared column, and this repo's convention grades a shipped service's
22+
accept-set/behaviour move as `minor`. **No `BREAKING` banner**: nothing is
23+
narrowed and no metadata that used to be accepted is now refused — the one
24+
consequence a consumer can feel is that `ApprovalStatus` and
25+
`ApprovalActionKind` each gained a member, so an exhaustive `switch` with no
26+
default, or a `satisfies Record<ApprovalStatus, …>` map outside this repo,
27+
now has a case to add. That is the same shape `returned` had when ADR-0044
28+
landed it.
29+
30+
**Spec (`@objectstack/spec/contracts/approval-service`)**
31+
32+
- `APPROVAL_STATUSES` gains `cancelled` (+ its `APPROVAL_STATUS_LABELS`
33+
entry). Its own terminal state rather than a re-use of `recalled`: a recall
34+
is an ACT by the submitter, and filing a platform-initiated void as one
35+
attributes a withdrawal to a person who never performed it.
36+
- New `APPROVAL_CANCEL_REASONS` / `ApprovalCancelReason` /
37+
`APPROVAL_CANCEL_REASON_LABELS`, single entry `record_deleted`. A
38+
VOCABULARY, not free text, because the reason has a non-human consumer (the
39+
inbox and the tombstone presentation branch on it) — and a CLASS, per the
40+
ruling's wording, so the next platform-initiated cancellation cause extends
41+
this list instead of minting a second terminal status for itself.
42+
- `APPROVAL_ACTION_KINDS` gains `cancel` — the only kind with no human actor,
43+
by construction.
44+
- `ApprovalRequestRow.cancel_reason` declared, optional-nullable.
45+
46+
**Plugin (`@objectstack/plugin-approvals`)**
47+
48+
- `sys_approval_request.cancel_reason`, a select derived from the contract
49+
vocabulary and never re-typed (the #3786 rule the `status` column already
50+
follows). On the row rather than on the audit entry, so a plain list view
51+
can read WHY without joining the append-only action log.
52+
- `bindRecordDeleteCancelHook` — a GLOBAL `afterDelete` registration beside
53+
the existing global record-lock hook, so one platform-level linkage covers
54+
every "approval + `lockRecord`" object at once. It needs no row-set
55+
plumbing: the engine binds the deleted row's pre-image on the by-id path
56+
and fans `afterDelete` out per matched row on a predicate delete, so a bulk
57+
delete is covered by the same handler. The approvals tables are excluded at
58+
registration, so they do not pay the delete-side pre-image read.
59+
- `ApprovalService.cancelForDeletedRecord` writes the transition: one
60+
append-only `sys_approval_action` row (`action: 'cancel'`, no actor),
61+
`status: 'cancelled'` + `cancel_reason: 'record_deleted'` +
62+
`completed_at`, and a `sys_approval_approver` index clear — that last one
63+
is not optional garnish, it is what actually empties the inbox, because the
64+
approver filter resolves through that index rather than through `status`.
65+
- The `Completed` list view now includes `cancelled`, so a kept audit row is
66+
visible in the one curated terminal view rather than only under `All`.
67+
-**No flow resume and no status mirror-back.** A cancellation is a status
68+
write plus a reason, not a decision, so there is no branch to resume down.
69+
The mirror-back is skipped by construction rather than by a swallowed
70+
error: it is an `update_record` against the row that was just deleted — the
71+
exact write this card's forensics caught failing elsewhere. The suspended
72+
run the request gated is reported at `warn` with its id and otherwise left
73+
alone; what becomes of it belongs to the automation service.
74+
-**The delete is never refused.** The "forbid delete while an approval is
75+
pending" direction was vetoed in the same ruling — `lockRecord` already
76+
blocks the edit, and blocking the delete too locks an author onto a record
77+
they cannot fix. Nothing in the hook throws; a failure degrades to the
78+
pre-existing state (the stale row) and is logged.
79+
- Terminal rows are untouched. `approved` / `rejected` / `recalled` /
80+
`returned` requests about the deleted record keep their recorded outcome —
81+
history stays history, and rendering their now-dead record reference is a
82+
separate console-side change.
83+
84+
zh-CN / ja-JP / es-ES bundles carry authored translations for the new leaves
85+
(已作废 / 無効化済み / Anulada), not source fills.

content/docs/permissions/system-context.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ The largest single consumer — **20 of the 109 sites**.
143143

144144
| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
145145
|:--|:---|:---|:---|:---|
146-
| 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:333` |
147-
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
148-
| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
146+
| 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` |
147+
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` |
148+
| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:950`, `:1059`, `:3163`, `:3309`, `:3476`, `:3547`, `:3736`, `:3776` |
149149
| 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` |
150150
| 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` |
151151
| 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` |

content/docs/permissions/tenant-audit-census.mdx

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way.
9898

9999
The same holds twice over for the context. An options argument spelled as a
100100
literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
101-
forwarding shim cannot, and **67 of the 215 sites are spelled that way**. A
101+
forwarding shim cannot, and **67 of the 217 sites are spelled that way**. A
102102
context resolved from an inline literal or a local `const` can be tested for
103103
`isSystem`; one arriving from a helper call cannot.
104104

@@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
147147

148148
| carried figure | where it survives | this census |
149149
| :--- | :--- | ---: |
150-
| 175 write call sites | quoted in the merged changeset | **215** |
150+
| 175 write call sites | quoted in the merged changeset | **217** |
151151
| 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable |
152-
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **143 of 215** decidable, **72** undecidable |
153-
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 97 decidably elevated, 0 decidably not, 101 undecidable |
152+
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **145 of 217** decidable, **72** undecidable |
153+
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 99 decidably elevated, 0 decidably not, 101 undecidable |
154154
| 141 and 132, two independent re-derivations | the card that filed this work ||
155155

156156
**The differences are not reconciled, and deliberately so.** The old census's
@@ -167,11 +167,11 @@ would report a smaller number and would not say so.
167167

168168
The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
169169
figure has no surviving corroboration anywhere in the tree.** This census reads
170-
97 of 215 (45%) as decidably elevated, with 101 more whose elevation is a
170+
99 of 217 (45%) as decidably elevated, with 101 more whose elevation is a
171171
run-time fact — so the claim is neither confirmed nor refuted, and the honest
172172
answer is that a static reading cannot settle it.
173173

174-
**Cite `9 / 215`, and say what it is**: the sites whose options argument was
174+
**Cite `9 / 217`, and say what it is**: the sites whose options argument was
175175
READ and holds no tenant context, against a decidably tenancy-enabled object.
176176
That is the control's provable yield surface. ⛔ Do not cite it as "the sites
177177
without tenant context" — **32 further sites** have an options argument this
@@ -183,28 +183,28 @@ cannot read, and they are neither in nor out.
183183

184184
| what | count |
185185
| :--- | ---: |
186-
| write call sites on the application surface | **215** |
187-
| …whose object name is statically decidable | 143 |
186+
| write call sites on the application surface | **217** |
187+
| …whose object name is statically decidable | 145 |
188188
| …whose object name is chosen at run time | 72 |
189-
| …against an object with tenancy ENABLED | 143 |
189+
| …against an object with tenancy ENABLED | 145 |
190190
| …against an object that declares tenancy off | 0 |
191-
| threading a tenant context | 131 |
191+
| threading a tenant context | 133 |
192192
| PROVABLY carrying none (options read, no context key) | **17** |
193193
| …of those, against a decidably tenancy-enabled object | **9** |
194194
| options argument UNREADABLE — may or may not carry one | 67 |
195195
| …of those, against a decidably tenancy-enabled object | 32 |
196-
| threading a decidably ELEVATED (`isSystem`) context | 97 |
196+
| threading a decidably ELEVATED (`isSystem`) context | 99 |
197197
| threading a context that is decidably NOT elevated | 0 |
198198
| threading a context whose elevation is a run-time fact | 101 |
199199

200200
| how the instrument reached the site | count |
201201
| :--- | ---: |
202-
| receiver carried a readable engine type | 170 |
202+
| receiver carried a readable engine type | 172 |
203203
| receiver erased, placed by the object NAME | 19 |
204204
| receiver erased, placed by an `object: string` PARAMETER | 15 |
205205
| receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 |
206206

207-
| object name spelled inline | 106 |
207+
| object name spelled inline | 108 |
208208
| object name spelled through a `const` | 37 |
209209
| object name is an `object: string` parameter | 19 |
210210
| object name is some other run-time expression | 53 |
@@ -224,7 +224,7 @@ holds still. They are required to be HERE and to say WHEN they were true;
224224
their values are not compared. The reasoning, and the measurement behind it,
225225
are in `scripts/check-tenant-audit-census.mjs`.
226226

227-
Measured on 2026-08-31 at `fc8858a24`.
227+
Measured on 2026-09-01 at `d3ebf3b55`.
228228

229229
| corpus scale (not enforced) | count |
230230
| :--- | ---: |

docs/audits/2026-08-tenant-audit-write-call-sites.counts.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
2929

3030
| Measure | Value |
3131
|---|---:|
32-
| Write call sites | 215 |
33-
| Object name statically decidable | 143 |
32+
| Write call sites | 217 |
33+
| Object name statically decidable | 145 |
3434
| Object name chosen at run time | 72 |
35-
| Against a tenancy-enabled object | 143 |
35+
| Against a tenancy-enabled object | 145 |
3636
| Against an object declaring tenancy off | 0 |
37-
| Threading a tenant context | 131 |
37+
| Threading a tenant context | 133 |
3838
| Provably carrying none | 17 |
3939
| …and decidably tenancy-enabled | 9 |
4040
| Options argument unreadable | 67 |
4141
| …and decidably tenancy-enabled | 32 |
42-
| Threading a decidably elevated context | 97 |
42+
| Threading a decidably elevated context | 99 |
4343
| Threading a decidably non-elevated context | 0 |
4444
| Threading a context of undecidable elevation | 101 |
4545

@@ -52,7 +52,7 @@ holds still. They are required to be HERE and to say WHEN they were true;
5252
their values are not compared. The reasoning, and the measurement behind it,
5353
are in `scripts/check-tenant-audit-census.mjs`.
5454

55-
Measured on 2026-08-31 at `fc8858a24`.
55+
Measured on 2026-09-01 at `d3ebf3b55`.
5656

5757
| corpus scale (not enforced) | count |
5858
| :--- | ---: |
@@ -66,11 +66,11 @@ Measured on 2026-08-31 at `fc8858a24`.
6666
| file | verb | object | tenancy | tenant context | n |
6767
|---|---|---|---|---|---:|
6868
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `object` | undecidable | context, elevation undecidable | 1 |
69-
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_action` | enabled | elevated | 13 |
69+
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_action` | enabled | elevated | 14 |
7070
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `delete` | `sys_approval_approver` | enabled | elevated | 2 |
7171
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_approver` | enabled | elevated | 2 |
7272
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_request` | enabled | elevated | 1 |
73-
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_request` | enabled | elevated | 8 |
73+
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_request` | enabled | elevated | 9 |
7474
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_token` | enabled | elevated | 1 |
7575
| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_token` | enabled | elevated | 1 |
7676
| `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 |

0 commit comments

Comments
 (0)