diff --git a/.changeset/approval-auto-cancel-on-record-delete.md b/.changeset/approval-auto-cancel-on-record-delete.md new file mode 100644 index 0000000000..47bb691feb --- /dev/null +++ b/.changeset/approval-auto-cancel-on-record-delete.md @@ -0,0 +1,85 @@ +--- +'@objectstack/spec': minor +'@objectstack/plugin-approvals': minor +--- + +fix(approvals): a deleted record's pending approvals auto-cancel instead of stranding in the inbox (#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. + +**Graded `minor`, and deliberately not `patch`.** The repair itself is a +defect fix, but it lands by WIDENING two published vocabularies and adding a +declared column, and this repo's convention grades a shipped service's +accept-set/behaviour move as `minor`. **No `BREAKING` banner**: nothing is +narrowed and no metadata that used to be accepted is now refused — the one +consequence a consumer can feel is that `ApprovalStatus` and +`ApprovalActionKind` each gained a member, so an exhaustive `switch` with no +default, or a `satisfies Record` map outside this repo, +now has a case to add. That is the same shape `returned` had when ADR-0044 +landed it. + +**Spec (`@objectstack/spec/contracts/approval-service`)** + +- `APPROVAL_STATUSES` gains `cancelled` (+ its `APPROVAL_STATUS_LABELS` + entry). 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, not free text, because the reason has a non-human consumer (the + inbox and the tombstone presentation branch on it) — 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 for itself. +- `APPROVAL_ACTION_KINDS` gains `cancel` — the only kind with no human actor, + by construction. +- `ApprovalRequestRow.cancel_reason` declared, optional-nullable. + +**Plugin (`@objectstack/plugin-approvals`)** + +- `sys_approval_request.cancel_reason`, a select derived from the contract + vocabulary and never re-typed (the #3786 rule the `status` column already + follows). On the row rather than on the audit entry, so a plain list view + can read WHY without joining the append-only action log. +- `bindRecordDeleteCancelHook` — a GLOBAL `afterDelete` registration beside + the existing global record-lock hook, so one platform-level linkage covers + every "approval + `lockRecord`" object at once. 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 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 optional 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`. +- ⛔ **No flow resume and no status mirror-back.** A cancellation is a status + write plus a reason, not a decision, so there is no branch to resume down. + The mirror-back is skipped by construction rather than 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 the request gated is reported at `warn` with its id and otherwise left + alone; what becomes of it belongs to the automation service. +- ⛔ **The delete is never refused.** The "forbid delete while an approval is + pending" direction was vetoed in the same ruling — `lockRecord` already + blocks the edit, and blocking the delete too locks an author onto a record + they cannot fix. Nothing in the hook throws; a failure degrades to the + pre-existing state (the stale row) and is logged. +- Terminal rows are untouched. `approved` / `rejected` / `recalled` / + `returned` requests about the deleted record keep their recorded outcome — + history stays history, and rendering their now-dead record reference is a + separate console-side change. + +zh-CN / ja-JP / es-ES bundles carry authored translations for the new leaves +(已作废 / 無効化済み / Anulada), not source fills. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b61dcf5983..b050766be2 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -143,9 +143,9 @@ The largest single consumer — **20 of the 109 sites**. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 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` | -| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` | -| 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` | +| 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` | +| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` | +| 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` | | 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` | | 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` | | 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` | diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 61bb8254ae..87c684e694 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **67 of the 215 sites are spelled that way**. A +forwarding shim cannot, and **67 of the 217 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **215** | +| 175 write call sites | quoted in the merged changeset | **217** | | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **143 of 215** decidable, **72** undecidable | -| 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 | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **145 of 217** decidable, **72** undecidable | +| 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 | | 141 and 132, two independent re-derivations | the card that filed this work | — | **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. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -97 of 215 (45%) as decidably elevated, with 101 more whose elevation is a +99 of 217 (45%) as decidably elevated, with 101 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `9 / 215`, and say what it is**: the sites whose options argument was +⇒ **Cite `9 / 217`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -183,28 +183,28 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **215** | -| …whose object name is statically decidable | 143 | +| write call sites on the application surface | **217** | +| …whose object name is statically decidable | 145 | | …whose object name is chosen at run time | 72 | -| …against an object with tenancy ENABLED | 143 | +| …against an object with tenancy ENABLED | 145 | | …against an object that declares tenancy off | 0 | -| threading a tenant context | 131 | +| threading a tenant context | 133 | | PROVABLY carrying none (options read, no context key) | **17** | | …of those, against a decidably tenancy-enabled object | **9** | | options argument UNREADABLE — may or may not carry one | 67 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 97 | +| threading a decidably ELEVATED (`isSystem`) context | 99 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 101 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 170 | +| receiver carried a readable engine type | 172 | | receiver erased, placed by the object NAME | 19 | | receiver erased, placed by an `object: string` PARAMETER | 15 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 | -| object name spelled inline | 106 | +| object name spelled inline | 108 | | object name spelled through a `const` | 37 | | object name is an `object: string` parameter | 19 | | 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; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-08-31 at `fc8858a24`. +Measured on 2026-09-01 at `d3ebf3b55`. | corpus scale (not enforced) | count | | :--- | ---: | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 7706b6adc2..f9458b7a03 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -29,17 +29,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 215 | -| Object name statically decidable | 143 | +| Write call sites | 217 | +| Object name statically decidable | 145 | | Object name chosen at run time | 72 | -| Against a tenancy-enabled object | 143 | +| Against a tenancy-enabled object | 145 | | Against an object declaring tenancy off | 0 | -| Threading a tenant context | 131 | +| Threading a tenant context | 133 | | Provably carrying none | 17 | | …and decidably tenancy-enabled | 9 | | Options argument unreadable | 67 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 97 | +| Threading a decidably elevated context | 99 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 101 | @@ -52,7 +52,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-08-31 at `fc8858a24`. +Measured on 2026-09-01 at `d3ebf3b55`. | corpus scale (not enforced) | count | | :--- | ---: | @@ -66,11 +66,11 @@ Measured on 2026-08-31 at `fc8858a24`. | file | verb | object | tenancy | tenant context | n | |---|---|---|---|---|---:| | `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `object` | undecidable | context, elevation undecidable | 1 | -| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_action` | enabled | elevated | 13 | +| `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_action` | enabled | elevated | 14 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `delete` | `sys_approval_approver` | enabled | elevated | 2 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_approver` | enabled | elevated | 2 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_request` | enabled | elevated | 1 | -| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_request` | enabled | elevated | 8 | +| `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_request` | enabled | elevated | 9 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 | diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index d016b8bf9f..fd8a406788 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -40,6 +40,7 @@ import type { ApprovalResubmitInput, ApprovalResubmitResult, ApprovalStatus, + ApprovalCancelReason, } from '@objectstack/spec/contracts'; // [#7135] The full `resolveAuthzContext` envelope — what `IApprovalService` // declares for every one of these context parameters since #6523 (the #6206 @@ -373,6 +374,20 @@ export type ActionTokenOutcome = */ const SYSTEM_CTX: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; +/** + * [#13568] Bound on the pending requests one deleted record can carry into + * {@link ApprovalService.cancelForDeletedRecord}. + * + * Not a safety valve like `lifecycle-hooks.ts`' `PENDING_LOCK_LIMIT` — that one + * fails a WRITE closed when it cannot decide row by row, and there is no such + * decision here: the delete has already landed, and going over the bound + * cancels fewer rows rather than refusing anything. It exists so a pathological + * row set cannot turn one delete into an unbounded write loop. In practice a + * record carries one pending request (`sys_approval_request`'s own guard on + * submit), so this is two orders of magnitude of headroom over the real shape. + */ +const RECORD_DELETE_CANCEL_LIMIT = 200; + /** * Who is acting, for the purpose of a data write made on their behalf (#3783). * @@ -548,6 +563,10 @@ function rowFromRequest(row: any): ApprovalRequestRow { submitter_id: row.submitter_id ?? undefined, submitter_comment: row.submitter_comment ?? undefined, status: (row.status as ApprovalStatus) ?? 'pending', + // [#13568] Why a cancelled row was cancelled. Surfaced on every read for + // the same reason `status` is: a client that can see the terminal state and + // not the machine-readable cause is back to guessing from prose. + cancel_reason: (row.cancel_reason as ApprovalCancelReason) ?? undefined, current_step: row.current_step ?? undefined, current_step_index: row.current_step_index ?? undefined, pending_approvers: csvSplit(row.pending_approvers), @@ -2971,6 +2990,153 @@ export class ApprovalService implements IApprovalService { return { request: fresh, runId, resumed, ...(resumeError ? { resumeError } : {}) }; } + // ── Record-delete lifecycle linkage (#13568) ───────────────── + + /** + * Void every `pending` request about a record that has just been DELETED + * (#13568, maintainer ruling 2026-08-31 「同意」). + * + * ## What the ruling asked for, and what each half costs + * + * status → `cancelled`, a machine-readable `cancel_reason`, the row KEPT + * for audit, and the request out of the pending count and the inbox's + * default view. + * + * The last clause needs both writes below, not just the status one: the + * inbox's "My Pending" tab pages the request table by `status`, but the + * approver filter resolves through the normalized `sys_approval_approver` + * index (#1745), which is keyed on nothing but `request_id`. Leaving the + * index rows behind would keep the request in `approverRequestIds`' answer + * for every approver forever — so {@link syncApproverIndex} clears them, the + * same call every other exit from `pending` makes. + * + * ## Three things this deliberately does NOT do + * + * 1. **No status mirror-back.** Every other terminal transition calls + * {@link mirrorStatusField} to write the outcome onto the subject record's + * `approvalStatusField`. Here the subject record is precisely what no + * longer exists, so that write is guaranteed to fail — it is the very + * `update_record(...) failed: Record ... not found` this card's own + * forensics recorded on the reject door. Skipped by construction, not by a + * swallowed error. + * + * 2. **No flow resume, and no run cancel.** A cancellation is a status write + * plus a reason; it is not a decision, so there is no branch to resume + * down and no downstream node that should run. The suspended run this + * request gated is reported (below) and left alone — what becomes of an + * approval run whose request was voided is a lifecycle question that + * belongs to the automation service, not to a delete hook, and answering + * it here by guessing would be exactly the kind of consumer-side + * accommodation PD #12 refuses. + * + * 3. **No refusal of the delete.** The delete has already landed by the time + * this runs, and the "forbid delete while an approval is pending" + * direction was VETOED in the same ruling (`lockRecord` already blocks the + * edit; blocking the delete too locks an author onto a record they cannot + * fix). So nothing here throws — a failure degrades to a warning and the + * stale row, which is the pre-existing state, never to a failed delete. + * + * Terminal rows are untouched: the `where` below names `status: 'pending'`, + * so an `approved` / `rejected` / `recalled` / `returned` request about the + * same record keeps its recorded outcome. That is the ruling's second half — + * history is kept; only the presentation of its dead record reference is + * someone else's card. + * + * @returns the request ids cancelled and the suspended run ids left behind. + */ + async cancelForDeletedRecord( + objectName: string, + recordId: string, + ): Promise<{ cancelled: string[]; suspendedRuns: string[] }> { + const empty = { cancelled: [] as string[], suspendedRuns: [] as string[] }; + const object = String(objectName ?? '').trim(); + const record = String(recordId ?? '').trim(); + if (!object || !record) return empty; + + let pending: any[]; + try { + const rows = await this.engine.find('sys_approval_request', { + where: { object_name: object, record_id: record, status: 'pending' }, + limit: RECORD_DELETE_CANCEL_LIMIT, + context: SYSTEM_CTX, + }); + pending = Array.isArray(rows) ? rows : []; + } catch (err: any) { + // Reading the bookkeeping is the one step with no partial outcome: if it + // fails, nothing was changed and the rows stay exactly as they were. + this.logger?.warn?.( + '[approvals] could not read the pending requests of a deleted record — they stay in the inbox ' + + 'until the record is deleted again or an operator finalises them', + { object, record, error: err?.message ?? String(err) }, + ); + return empty; + } + if (pending.length === 0) return empty; + + const now = this.clock.now().toISOString(); + const cancelled: string[] = []; + const suspendedRuns: string[] = []; + + for (const raw of pending) { + const requestId = String(raw?.id ?? ''); + if (!requestId) continue; + const org = raw?.organization_id ?? null; + const nodeId: string | null = raw?.flow_node_id ?? raw?.current_step ?? null; + try { + // Audit first, same ordering as every other write path here: if the + // status write fails, the trail still records that the platform tried + // to void the request, rather than the reverse (a cancelled row whose + // history says nothing happened). + // + // `actor_id` is null on purpose and is the point of the `cancel` kind: + // no person did this. Attributing it to a submitter (`recall`) or an + // approver (`reject`) would file a decision nobody made. + await this.engine.insert('sys_approval_action', { + id: uid('aact'), request_id: requestId, organization_id: org, + step_name: nodeId, step_index: 0, action: 'cancel', + actor_id: null, + comment: `Auto-cancelled: the ${object} record '${record}' this request is about was deleted`, + created_at: now, + }, { context: SYSTEM_CTX }); + + await this.engine.update('sys_approval_request', { + id: requestId, + status: 'cancelled', + cancel_reason: 'record_deleted', + pending_approvers: null, + completed_at: now, + updated_at: now, + }, { context: SYSTEM_CTX }); + + // See the docstring: the status write alone does not empty the inbox. + await this.syncApproverIndex(requestId, [], org, now); + cancelled.push(requestId); + + const runId = raw?.flow_run_id ? String(raw.flow_run_id) : ''; + if (runId) suspendedRuns.push(runId); + } catch (err: any) { + // Per request, so one unwritable row cannot skip the rest. + this.logger?.warn?.( + '[approvals] could not auto-cancel a pending request whose record was deleted — it stays in ' + + 'the inbox pointing at a record that no longer exists', + { object, record, request: requestId, error: err?.message ?? String(err) }, + ); + } + } + + if (suspendedRuns.length) { + // Reported, never repaired here (docstring point 2). `warn`: a run parked + // on an approval that can no longer be decided is a real functional + // degradation, and naming the run ids is what makes it actionable. + this.logger?.warn?.( + '[approvals] auto-cancelled approval request(s) for a deleted record — the automation run(s) ' + + 'they gated stay suspended and are not resumed by a cancellation', + { object, record, requests: cancelled, runs: suspendedRuns }, + ); + } + return { cancelled, suspendedRuns }; + } + // ── Send back for revision / resubmit (ADR-0044) ───────────── /** diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index b2bb2b815c..90837bfc4a 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -20,7 +20,12 @@ import { type ApprovalEngine, type ApprovalMessagingSurface, } from './approval-service.js'; -import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js'; +import { + bindApprovalLockHook, + bindDelegationWriteGuard, + bindRecordDeleteCancelHook, + unbindAllHooks, +} from './lifecycle-hooks.js'; import { bindSnapshotRedactionMiddleware } from './payload-redaction-middleware.js'; import type { FieldVisibilitySource } from './payload-redaction.js'; import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js'; @@ -244,13 +249,19 @@ export class ApprovalsServicePlugin implements Plugin { }); // Record lock: block edits to a record while it has a pending request. + // Record-delete cancel (#13568): void a record's pending requests when the + // record itself is deleted — the other half of the lock's lifecycle, and + // the reason `disableAutoHooks` now suppresses BOTH (a deployment that + // wants no engine-level approval wiring wants neither half; leaving the + // lock off and the cancel on would be a shape nobody asked for). // Delegation write-guard: a self-service OOO delegation may only name the - // acting user as delegator (#1322 follow-up). Both bind under the same + // acting user as delegator (#1322 follow-up). All bind under the same // package id, so unbindAllHooks clears them together. if (!this.options.disableAutoHooks) { try { unbindAllHooks(engine); bindApprovalLockHook(engine, ctx.logger); + bindRecordDeleteCancelHook(engine, this.service, ctx.logger); bindDelegationWriteGuard(engine, ctx.logger); // [#10749] Generic-door snapshot redaction. Registered here so the // service door and the generic data door narrow together — see diff --git a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts index 193badf4d2..10159f2215 100644 --- a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts +++ b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts @@ -77,6 +77,7 @@ export const APPROVALS_HOOK_PACKAGE = 'plugin-approvals:lock'; interface MinimalEngine { registerHook(event: string, handler: (ctx: any) => any | Promise, options?: { object?: string | string[]; + excludeObjects?: string | string[]; priority?: number; packageId?: string; }): void; @@ -84,6 +85,19 @@ interface MinimalEngine { find(object: string, args: any, opts?: any): Promise; } +/** + * The one thing {@link bindRecordDeleteCancelHook} needs from the approvals + * service — declared structurally so the hook can be bound (and tested) against + * anything that answers it, and so this module keeps depending on no more of + * the service than it uses. + */ +export interface RecordDeleteCancelSurface { + cancelForDeletedRecord( + objectName: string, + recordId: string, + ): Promise<{ cancelled: string[]; suspendedRuns: string[] }>; +} + interface MinimalLogger { debug?: (msg: any, ...rest: any[]) => void; info?: (msg: any, ...rest: any[]) => void; @@ -384,6 +398,122 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg logger?.info?.('[approvals] record-lock hook bound'); } +/** + * The approvals plugin's own bookkeeping tables. + * + * The lock hook above skips them with a `startsWith('sys_approval')` test + * because it is a `beforeUpdate` guard and the cheap test is enough. The delete + * linkage below excludes them at REGISTRATION instead, which is strictly + * stronger: `hookMatchesObject` subtracts `excludeObjects` before + * `hasHooksFor` answers, so these objects do not even pay the delete-side + * pre-image read the hook's presence would otherwise demand of them. + * + * Literal names, not a prefix: `excludeObjects` matches literally by design + * (`'*'` is refused at registration), so a sixth approvals object added later + * must be added here too — which is the loud direction. A missing entry costs + * one pointless lookup, never a wrong answer, because + * {@link bindRecordDeleteCancelHook} would find no request whose + * `object_name` names an approvals table anyway. + */ +const APPROVALS_OWN_OBJECTS = [ + 'sys_approval_request', + 'sys_approval_action', + 'sys_approval_approver', + 'sys_approval_token', + 'sys_approval_delegation', +] as const; + +/** + * Bind the global record-delete → approval-cancel linkage (#13568, maintainer + * ruling 2026-08-31 「同意」). + * + * ## Why this is a GLOBAL hook and not a per-module rule + * + * The card reached us from a leave-request module, but nothing about it is + * specific to leave: any object whose approval node declares `lockRecord` puts + * its authors on the same path — the lock blocks the EDIT, so "delete and + * recreate" becomes the only way to fix a wrong record, and every such delete + * used to leave its pending request sitting in the approvers' inbox, counted, + * openable, and pointing at a record that no longer exists. So the linkage + * belongs exactly where the lock already lives: one global registration, and + * every "approval + lockRecord" combination is covered at once. + * + * ## Why `afterDelete`, and why it needs no row-set plumbing + * + * `after`, because a cancellation must not be written for a delete that then + * fails; the delete has landed by the time this runs. And the engine hands it + * the rows outright: a by-id delete binds `previous` from the pre-image it + * already reads for the not-found gate (#7867), and a predicate delete reads + * the doomed rows ONCE before they are gone and dispatches this hook per row + * with each row bound (#5038 / #5574, `bulkPerRowRows` in ObjectQL's + * `delete()`). So there is no "which rows was this?" question left to answer — + * no before-hook stash, no predicate re-resolution, no unbounded branch. + * + * ## Cost + * + * A global delete-side registration makes `hasHooksFor('afterDelete', object)` + * true for every non-excluded object, which is what makes the engine read the + * doomed rows on a predicate delete. That is not a new cost on any real + * deployment: `plugin-sharing`, `service-storage` and `plugin-audit` already + * register global delete-side hooks, so the read was already demanded of every + * object (ObjectQL's `engine-delete-prior-read-scope.test.ts` enumerates them). + * + * ## Failure posture + * + * Nothing here throws. The delete already happened; failing this hook would + * turn a successful delete into an error for the caller while leaving the row + * gone. A failure degrades to the PRE-EXISTING state (the stale request stays) + * and is logged — see {@link RecordDeleteCancelSurface}'s implementation for + * the per-request warning. + */ +export function bindRecordDeleteCancelHook( + engine: MinimalEngine, + service: RecordDeleteCancelSurface, + logger?: MinimalLogger, +): void { + engine.registerHook('afterDelete', async (ctx: any) => { + const object = (ctx?.object ?? ctx?.objectName) as string | undefined; + if (!object) return; + + // The deleted row's own id. `previous` first: on the per-row dispatch it IS + // the deleted row, and on the by-id path it is the pre-image the engine + // proved present before deleting. `input.id` is the fallback for a + // hand-built context (a direct handler call, a test) that carries no + // pre-image. + const raw = (ctx?.previous as any)?.id ?? ctx?.input?.id; + const recordId = raw == null ? '' : String(raw); + if (!recordId) { + // "We do not know which row this was" — never read as "no row changed". + // Unreachable on the engine paths above (both bind an id), so a sighting + // is a real finding about a caller this hook has not met. + logger?.warn?.( + '[approvals] a delete reached the approval-cancel hook with no record id — pending approvals ' + + 'for the deleted record (if any) stay in the inbox', + { object }, + ); + return; + } + + try { + await service.cancelForDeletedRecord(String(object), recordId); + } catch (err: any) { + // Belt: the service already swallows its own per-request failures, so + // reaching here means something outside them threw. The delete stands. + logger?.warn?.( + '[approvals] the approval-cancel linkage failed for a deleted record — its pending approvals ' + + 'stay in the inbox pointing at a record that no longer exists', + { object, record: recordId, error: err?.message ?? String(err) }, + ); + } + }, { + excludeObjects: [...APPROVALS_OWN_OBJECTS], + packageId: APPROVALS_HOOK_PACKAGE, + priority: 50, + }); + + logger?.info?.('[approvals] record-delete cancel hook bound'); +} + /** The self-service out-of-office delegation object (#1322). */ export const DELEGATION_OBJECT = 'sys_approval_delegation'; diff --git a/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts b/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts new file mode 100644 index 0000000000..b0d33c626a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts @@ -0,0 +1,375 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13568 — a record's PENDING approvals are voided when the record is deleted. + * + * ## The defect, as it was found + * + * A leave-request module declared an approval node with `lockRecord: true`. The + * lock blocks the EDIT, so an author who needs to change a submitted record has + * exactly one route left — delete it and build it again. That delete left the + * pending `sys_approval_request` row behind: still `pending`, still counted in + * the approver's inbox, still openable, and pointing at a `record_id` that no + * longer resolves to anything. Nothing about that is specific to leave + * requests; every "approval + `lockRecord`" object walks the same path, which + * is why the fix is one global lifecycle linkage rather than a module rule. + * + * The maintainer's 2026-08-31 ruling: 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. Historical + * terminal rows are kept as they are (their dead-reference PRESENTATION is a + * separate objectui card), the "forbid delete while pending" direction was + * vetoed, and a cancellation is a status write — not a flow resume. + * + * ## Why this file boots the real engine + * + * The seam under test is the ENGINE's delete dispatch. Whether a delete arrives + * as one by-id call or as a predicate write fanned out per row, and whether the + * deleted row's pre-image is bound on the way past, are decisions ObjectQL + * makes — the whole defect lives in what the hook is handed. A fake engine + * would be a fixture written by the same author as the assertion, deciding the + * one thing the test exists to measure. So: a real {@link ObjectQL} over + * `@objectstack/driver-sql` + better-sqlite3 `:memory:`, the real + * `sys_approval_*` schemas, and the real {@link ApprovalService} — the requests + * are opened by `openNodeRequest`, so the `sys_approval_approver` index rows + * this file asserts the disappearance of were written by production code. + * + * ## The control case is load-bearing + * + * `describe('control — without the linkage')` at the bottom runs the same + * delete with the hook NOT bound and asserts the row stays `pending`. Without + * it, every positive assertion above would also pass against an engine that + * cancels requests for some unrelated reason, and a linkage that quietly + * stopped being registered would go on reading green forever. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +// The read options below are DECLARED rather than erased to `any` (#4674 / +// #4918). None of these queries is deliberately off-contract — they are plain +// `where` + `context` reads — so the contract type is the right instrument and +// `as unknown as EngineQueryOptions` (the sanctioned spelling for input a test +// means to be invalid) would be a false claim here. +import type { EngineQueryOptions } from '@objectstack/spec/data'; +import { ApprovalService } from './approval-service.js'; +import { bindRecordDeleteCancelHook } from './lifecycle-hooks.js'; +import { SysApprovalRequest } from './sys-approval-request.object.js'; +import { SysApprovalAction } from './sys-approval-action.object.js'; +import { SysApprovalApprover } from './sys-approval-approver.object.js'; +import { SysApprovalDelegation } from './sys-approval-delegation.object.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any; +const SUBMITTER = { userId: 'submitter', positions: [], permissions: [] } as any; +const APPROVER = { userId: 'approver', positions: [], permissions: [] } as any; + +/** The business object the approvals are about. */ +const leaveRequest = { + name: 'crm_leave_request', + label: 'Leave Request', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + approval_status: { name: 'approval_status', label: 'Approval Status', type: 'text' as const }, + }, +}; + +/** + * The node config from the card: an approval that LOCKS its record. `lockRecord` + * is what makes "delete and recreate" the author's only route, so it is the + * shape the linkage exists for — even though the cancel path itself never reads + * it. + */ +const nodeConfig = { + approvers: [{ type: 'user' as const, value: 'approver' }], + behavior: 'first_response' as const, + lockRecord: true, + approvalStatusField: 'approval_status', +}; + +function makeSqliteDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +/** + * A stand-in for the automation service that RECORDS instead of acting. + * + * Its purpose is a negative: the ruling says a cancellation is a status write + * plus a reason, NOT a flow resume, and this is how "the downstream nodes did + * not run" is measured rather than asserted. A spy that is never called proves + * nothing unless the same spy is reachable — `openNodeRequest` below is handed + * this same object, so the wiring is live throughout. + */ +function makeAutomationSpy() { + return { + resume: vi.fn(async () => ({ status: 'completed' })), + cancelRun: vi.fn(async () => undefined), + getRun: vi.fn(async (runId: string) => ({ id: runId, status: 'suspended' })), + }; +} + +describe('a deleted record auto-cancels its pending approvals (#13568)', () => { + let engine: ObjectQL; + let svc: ApprovalService; + let automation: ReturnType; + let warnings: Array<{ msg: string; meta: any }>; + + const requestsFor = (recordId: string) => + engine.find('sys_approval_request', { + where: { object_name: 'crm_leave_request', record_id: recordId }, + context: SYSTEM, + } satisfies EngineQueryOptions) as Promise; + + const actionsFor = (requestId: string) => + engine.find('sys_approval_action', { + where: { request_id: requestId }, context: SYSTEM, + } satisfies EngineQueryOptions) as Promise; + + const approverIndexFor = (requestId: string) => + engine.find('sys_approval_approver', { + where: { request_id: requestId }, context: SYSTEM, + } satisfies EngineQueryOptions) as Promise; + + const openOn = (recordId: string, runId: string) => svc.openNodeRequest({ + object: 'crm_leave_request', recordId, runId, nodeId: 'manager_review', + flowName: 'leave_approval', config: nodeConfig, submitterId: 'submitter', + record: { id: recordId, title: 'Leave' }, + }, SUBMITTER) as Promise; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + beforeEach(async () => { + engine = new ObjectQL(); + engine.registerDriver(makeSqliteDriver(), true); + await engine.init(); + for (const def of [ + leaveRequest, SysApprovalRequest, SysApprovalAction, SysApprovalApprover, SysApprovalDelegation, + ]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + // Real DDL through the real driver — including the `cancel_reason` column + // this card adds, so a write of it that the schema does not carry fails + // here rather than passing against a permissive fake. + await engine.syncSchemas(); + + warnings = []; + automation = makeAutomationSpy(); + svc = new ApprovalService({ + engine: engine as any, + automation: automation as any, + logger: { + warn: (msg: any, meta?: any) => { warnings.push({ msg: String(msg), meta }); }, + } as any, + }); + + await engine.insert('crm_leave_request', { id: 'LR6', title: 'Leave' }, { context: SYSTEM } as any); + await engine.insert('crm_leave_request', { id: 'LR7', title: 'Other leave' }, { context: SYSTEM } as any); + + bindRecordDeleteCancelHook(engine as any, svc); + }); + + // ── positive ──────────────────────────────────────────────────── + + it('the card verbatim: deleting the record cancels its pending request, and the row survives', async () => { + const opened = await openOn('LR6', 'run_1'); + expect((await requestsFor('LR6'))[0].status).toBe('pending'); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + const rows = await requestsFor('LR6'); + // KEPT for audit — the ruling's first clause. A cancel that deleted the row + // would satisfy every inbox assertion below and destroy the evidence that + // an approval was ever opened. + expect(rows).toHaveLength(1); + expect(rows[0].id).toBe(opened.id); + expect(rows[0].status).toBe('cancelled'); + // The MACHINE-READABLE reason, read off the column rather than parsed out + // of prose. + expect(rows[0].cancel_reason).toBe('record_deleted'); + expect(rows[0].completed_at).toBeTruthy(); + // Cleared with the exit from `pending`, exactly as every other terminal + // transition clears it. + expect(rows[0].pending_approvers ?? null).toBeNull(); + }); + + it('writes one append-only audit row, attributed to no person', async () => { + const opened = await openOn('LR6', 'run_1'); + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + const cancels = (await actionsFor(opened.id)).filter((a) => a.action === 'cancel'); + expect(cancels).toHaveLength(1); + // The point of the `cancel` kind: nobody decided this. Recording it as + // `recall` would file a submitter withdrawal that never happened, and as + // `reject` an approver decision that never happened. + expect(cancels[0].actor_id ?? null).toBeNull(); + expect(String(cancels[0].comment)).toMatch(/deleted/i); + }); + + it('leaves the pending count and the approver inbox — both halves', async () => { + await openOn('LR6', 'run_1'); + // Before: the row the card's approver saw. + expect(await svc.countRequests({ status: 'pending', approverId: 'approver' }, APPROVER)).toBe(1); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + expect(await svc.countRequests({ status: 'pending', approverId: 'approver' }, APPROVER)).toBe(0); + expect(await svc.listRequests({ status: 'pending', approverId: 'approver' }, APPROVER)).toEqual([]); + }); + + it('clears the normalized approver index, not only the status', async () => { + const opened = await openOn('LR6', 'run_1'); + // The index is what the inbox's approver filter actually resolves through + // (#1745) — a status-only cancel would leave these rows behind and the + // request would stay reachable by every approver filter forever. + expect((await approverIndexFor(opened.id)).length).toBeGreaterThan(0); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + expect(await approverIndexFor(opened.id)).toEqual([]); + }); + + it('surfaces the reason through the service read path, not only in the table', async () => { + const opened = await openOn('LR6', 'run_1'); + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + const row = await svc.getRequest(opened.id, SYSTEM); + expect(row?.status).toBe('cancelled'); + expect(row?.cancel_reason).toBe('record_deleted'); + }); + + it('covers a predicate (bulk) delete — every deleted row, not just the first', async () => { + const a = await openOn('LR6', 'run_1'); + const b = await openOn('LR7', 'run_2'); + + // The shape the record lock once walked straight past (#4778): no scalar + // `where.id`, so the engine routes to `deleteMany` and fans `afterDelete` + // out per matched row. Both requests must be cancelled, not one. + await engine.delete('crm_leave_request', { + where: { id: { $in: ['LR6', 'LR7'] } }, multi: true, context: SYSTEM, + } as any); + + expect((await requestsFor('LR6'))[0].status).toBe('cancelled'); + expect((await requestsFor('LR7'))[0].status).toBe('cancelled'); + expect((await approverIndexFor(a.id))).toEqual([]); + expect((await approverIndexFor(b.id))).toEqual([]); + }); + + // ── negative ──────────────────────────────────────────────────── + + it('does NOT touch a terminal request about the same record', async () => { + const opened = await openOn('LR6', 'run_1'); + await svc.decide(opened.id, { decision: 'approve', actorId: 'approver' }, APPROVER); + const before = (await requestsFor('LR6'))[0]; + expect(before.status).toBe('approved'); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + const after = (await requestsFor('LR6'))[0]; + // The ruling's second clause: history is kept as history. Rewriting a + // recorded decision into `cancelled` would erase the fact that someone + // approved this — the audit trail's whole job. + expect(after.status).toBe('approved'); + expect(after.cancel_reason ?? null).toBeNull(); + expect((await actionsFor(opened.id)).filter((x) => x.action === 'cancel')).toEqual([]); + }); + + it('does NOT touch a request about a DIFFERENT record of the same object', async () => { + await openOn('LR6', 'run_1'); + await openOn('LR7', 'run_2'); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + expect((await requestsFor('LR6'))[0].status).toBe('cancelled'); + // Per-record, never "this object has a delete happening". + expect((await requestsFor('LR7'))[0].status).toBe('pending'); + expect(await svc.countRequests({ status: 'pending', approverId: 'approver' }, APPROVER)).toBe(1); + }); + + it('runs no flow node and mirrors no status back', async () => { + await openOn('LR6', 'run_1'); + automation.resume.mockClear(); + automation.cancelRun.mockClear(); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + // A cancellation is a status write plus a reason. Resuming would run the + // approval's downstream nodes for a decision nobody made; the mirror-back + // would be an `update_record` against a row that no longer exists — the + // exact write this card's forensics caught failing on the reject door. + expect(automation.resume).not.toHaveBeenCalled(); + expect(automation.cancelRun).not.toHaveBeenCalled(); + // The suspended run is REPORTED rather than repaired — the datum an + // operator (and the automation-side card) needs, without this hook + // guessing at run lifecycle. + expect(warnings.some((w) => w.msg.includes('stay suspended') + && Array.isArray(w.meta?.runs) && w.meta.runs.includes('run_1'))).toBe(true); + }); + + it('does not fail the delete when the approvals bookkeeping cannot be written', async () => { + await openOn('LR6', 'run_1'); + const boom = vi.spyOn(svc as any, 'cancelForDeletedRecord') + .mockRejectedValue(new Error('bookkeeping unavailable')); + + // The delete already landed by the time the hook runs; failing it here + // would report an error for a row that is gone. The pre-existing state + // (a stale request) is the safe degradation, and it is logged. + await expect( + engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any), + ).resolves.toBeDefined(); + expect(await engine.find('crm_leave_request', { + where: { id: 'LR6' }, context: SYSTEM, + } satisfies EngineQueryOptions)).toEqual([]); + boom.mockRestore(); + }); +}); + +// ── the control ─────────────────────────────────────────────────── + +describe('#13568 control — without the linkage the same delete strands the request', () => { + let engine: ObjectQL; + let svc: ApprovalService; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + beforeEach(async () => { + engine = new ObjectQL(); + engine.registerDriver(makeSqliteDriver(), true); + await engine.init(); + for (const def of [ + leaveRequest, SysApprovalRequest, SysApprovalAction, SysApprovalApprover, SysApprovalDelegation, + ]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + await engine.syncSchemas(); + svc = new ApprovalService({ engine: engine as any }); + await engine.insert('crm_leave_request', { id: 'LR6', title: 'Leave' }, { context: SYSTEM } as any); + // ⛔ deliberately NOT bound — that is the whole case. + }); + + it('reproduces the reported symptom exactly, so the pins above measure the hook', async () => { + await svc.openNodeRequest({ + object: 'crm_leave_request', recordId: 'LR6', runId: 'run_1', nodeId: 'manager_review', + flowName: 'leave_approval', config: nodeConfig, submitterId: 'submitter', + record: { id: 'LR6', title: 'Leave' }, + }, SUBMITTER); + + await engine.delete('crm_leave_request', { where: { id: 'LR6' }, context: SYSTEM } as any); + + const rows = await engine.find('sys_approval_request', { + where: { object_name: 'crm_leave_request', record_id: 'LR6' }, context: SYSTEM, + } satisfies EngineQueryOptions) as any[]; + // The card's screenshot, in one assertion: the record is gone and the + // request is still waiting for a decision about it. + expect(rows).toHaveLength(1); + expect(rows[0].status).toBe('pending'); + expect(await svc.countRequests({ status: 'pending', approverId: 'approver' }, APPROVER)).toBe(1); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 7739f9a63d..51ec92d3cb 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -1,7 +1,12 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; -import { APPROVAL_STATUSES, APPROVAL_STATUS_LABELS } from '@objectstack/spec/contracts'; +import { + APPROVAL_STATUSES, + APPROVAL_STATUS_LABELS, + APPROVAL_CANCEL_REASONS, + APPROVAL_CANCEL_REASON_LABELS, +} from '@objectstack/spec/contracts'; /** * sys_approval_request — Live approval instance. @@ -11,6 +16,7 @@ import { APPROVAL_STATUSES, APPROVAL_STATUS_LABELS } from '@objectstack/spec/con * * `pending` → (per-approver decisions) → `approved` | `rejected` * `pending` → recalled by submitter → `recalled` + * `pending` → its record was deleted → `cancelled` + `cancel_reason` * * `flow_run_id` / `flow_node_id` tie the request back to the suspended run so a * decision can resume it; `current_step` mirrors the node id. `node_config_json` @@ -76,7 +82,10 @@ export const SysApprovalRequest = ObjectSchema.create({ label: 'Completed', data: { provider: 'object', object: 'sys_approval_request' }, columns: ['process_name', 'object_name', 'record_id', 'status', 'submitter_id', 'completed_at'], - filter: [{ field: 'status', operator: 'in', value: ['approved', 'rejected', 'recalled'] }], + // Every terminal state, `cancelled` included (#13568) — a platform-voided + // request is kept as audit evidence, and a terminal state absent from the + // only curated terminal view is evidence nobody can find. + filter: [{ field: 'status', operator: 'in', value: ['approved', 'rejected', 'recalled', 'cancelled'] }], sort: [{ field: 'completed_at', order: 'desc' }], pagination: { pageSize: 25 }, }, @@ -212,6 +221,30 @@ export const SysApprovalRequest = ObjectSchema.create({ }, ), + // [#13568] Why a `cancelled` request was cancelled — the machine-readable + // half of the maintainer's 2026-08-31 ruling ("状态转 cancelled + 机器可读 + // 原因"). Derived from `APPROVAL_CANCEL_REASONS`, never re-typed, on the + // same #3786 rule the `status` column above follows. + // + // On the ROW rather than on the `sys_approval_action` audit entry, because + // the readers are the inbox and the tombstone presentation, which page the + // request table: making them join the append-only action log to learn why a + // row they already hold is cancelled buys nothing and would make the reason + // unavailable to a plain list view. The action row still records the event + // (`action: 'cancel'`); this column records the STATE. + // + // Null on every non-cancelled row, and on rows written before the column + // existed — "not recorded", never "cancelled for no reason". + cancel_reason: Field.select( + APPROVAL_CANCEL_REASONS.map((value) => ({ value, label: APPROVAL_CANCEL_REASON_LABELS[value] })), + { + label: 'Cancellation Reason', + required: false, + description: 'Why the platform voided this request (set only when the status is cancelled)', + group: 'State', + }, + ), + current_step: Field.text({ label: 'Current Step', required: false, diff --git a/packages/plugins/plugin-approvals/src/translations/approval-status-vocabulary.test.ts b/packages/plugins/plugin-approvals/src/translations/approval-status-vocabulary.test.ts index afad083f46..03d5ca8e48 100644 --- a/packages/plugins/plugin-approvals/src/translations/approval-status-vocabulary.test.ts +++ b/packages/plugins/plugin-approvals/src/translations/approval-status-vocabulary.test.ts @@ -92,6 +92,7 @@ describe('approval status vocabulary (#7232)', () => { rejected: 'Rejected', recalled: 'Recalled', returned: 'Returned', + cancelled: 'Cancelled', }); }); @@ -102,6 +103,11 @@ describe('approval status vocabulary (#7232)', () => { rejected: '已拒绝', recalled: '已撤回', returned: '已退回修改', + // #13568. 已作废, deliberately not 已取消: `recalled` is already 已撤回 + // (the submitter withdrew), and a platform-initiated void has to read as + // something nobody chose to do — the same distinction the English pair + // Recalled / Cancelled carries. + cancelled: '已作废', }); }); diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index 3e64d45594..8c8972d68e 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -51,7 +51,15 @@ export const enObjects: NonNullable = { approved: "Approved", rejected: "Rejected", recalled: "Recalled", - returned: "Returned" + returned: "Returned", + cancelled: "Cancelled" + } + }, + cancel_reason: { + label: "Cancellation Reason", + help: "Why the platform voided this request (set only when the status is cancelled)", + options: { + record_deleted: "Related record deleted" } }, current_step: { @@ -232,7 +240,8 @@ export const enObjects: NonNullable = { comment: "Comment", revise: "Revise", resubmit: "Resubmit", - ooo_substitute: "Out-of-Office Substitution" + ooo_substitute: "Out-of-Office Substitution", + cancel: "Cancel" } }, actor_id: { diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 890c99a9dd..17a69af7dd 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -51,7 +51,15 @@ export const esESObjects: NonNullable = { approved: "Aprobada", rejected: "Rechazada", recalled: "Retirada", - returned: "Devuelta para revisión" + returned: "Devuelta para revisión", + cancelled: "Anulada" + } + }, + cancel_reason: { + label: "Motivo de anulación", + help: "Por qué la plataforma anuló esta solicitud (solo se establece cuando el estado es Anulada)", + options: { + record_deleted: "Registro relacionado eliminado" } }, current_step: { @@ -232,7 +240,8 @@ export const esESObjects: NonNullable = { comment: "Comentario", revise: "Devolución", resubmit: "Reenvío", - ooo_substitute: "Sustitución por ausencia" + ooo_substitute: "Sustitución por ausencia", + cancel: "Anulación" } }, actor_id: { diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index 6a608a9b58..e1e3571230 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -51,7 +51,15 @@ export const jaJPObjects: NonNullable = { approved: "承認済み", rejected: "却下済み", recalled: "取り消し済み", - returned: "差し戻し済み" + returned: "差し戻し済み", + cancelled: "無効化済み" + } + }, + cancel_reason: { + label: "無効化理由", + help: "プラットフォームがこの申請を無効化した理由(ステータスが無効化済みの場合のみ設定)", + options: { + record_deleted: "関連レコードが削除されました" } }, current_step: { @@ -232,7 +240,8 @@ export const jaJPObjects: NonNullable = { comment: "コメント", revise: "差し戻し", resubmit: "再提出", - ooo_substitute: "不在時代理" + ooo_substitute: "不在時代理", + cancel: "無効化" } }, actor_id: { diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index 537a51a405..3367c89278 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -51,7 +51,15 @@ export const zhCNObjects: NonNullable = { approved: "已批准", rejected: "已拒绝", recalled: "已撤回", - returned: "已退回修改" + returned: "已退回修改", + cancelled: "已作废" + } + }, + cancel_reason: { + label: "作废原因", + help: "平台作废该审批单的原因(仅在状态为「已作废」时写入)", + options: { + record_deleted: "关联记录已删除" } }, current_step: { @@ -232,7 +240,8 @@ export const zhCNObjects: NonNullable = { comment: "评论", revise: "退回修改", resubmit: "重新提交", - ooo_substitute: "不在岗改派" + ooo_substitute: "不在岗改派", + cancel: "作废" } }, actor_id: { diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index ec7d051b15..bf819e6114 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -15,6 +15,8 @@ "AIToolResult (type)", "APPROVAL_ACTION_KINDS (const)", "APPROVAL_ACTION_KIND_LABELS (const)", + "APPROVAL_CANCEL_REASONS (const)", + "APPROVAL_CANCEL_REASON_LABELS (const)", "APPROVAL_STATUSES (const)", "APPROVAL_STATUS_LABELS (const)", "AdapterContext (interface)", @@ -27,6 +29,7 @@ "ApprovalActionAttachment (interface)", "ApprovalActionKind (type)", "ApprovalActionRow (interface)", + "ApprovalCancelReason (type)", "ApprovalDecisionInput (interface)", "ApprovalDecisionResult (interface)", "ApprovalRecallInput (interface)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index d3fce9f12b..33cb67d8d7 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -15,6 +15,8 @@ "AIToolResult": "src/contracts/ai-service.ts#AIToolResult (type)", "APPROVAL_ACTION_KINDS": "src/contracts/approval-service.ts#APPROVAL_ACTION_KINDS (const)", "APPROVAL_ACTION_KIND_LABELS": "src/contracts/approval-service.ts#APPROVAL_ACTION_KIND_LABELS (const)", + "APPROVAL_CANCEL_REASONS": "src/contracts/approval-service.ts#APPROVAL_CANCEL_REASONS (const)", + "APPROVAL_CANCEL_REASON_LABELS": "src/contracts/approval-service.ts#APPROVAL_CANCEL_REASON_LABELS (const)", "APPROVAL_STATUSES": "src/contracts/approval-service.ts#APPROVAL_STATUSES (const)", "APPROVAL_STATUS_LABELS": "src/contracts/approval-service.ts#APPROVAL_STATUS_LABELS (const)", "AdapterContext": "src/contracts/knowledge-adapter.ts#AdapterContext (interface)", @@ -27,6 +29,7 @@ "ApprovalActionAttachment": "src/contracts/approval-service.ts#ApprovalActionAttachment (interface)", "ApprovalActionKind": "src/contracts/approval-service.ts#ApprovalActionKind (type)", "ApprovalActionRow": "src/contracts/approval-service.ts#ApprovalActionRow (interface)", + "ApprovalCancelReason": "src/contracts/approval-service.ts#ApprovalCancelReason (type)", "ApprovalDecisionInput": "src/contracts/approval-service.ts#ApprovalDecisionInput (interface)", "ApprovalDecisionResult": "src/contracts/approval-service.ts#ApprovalDecisionResult (interface)", "ApprovalRecallInput": "src/contracts/approval-service.ts#ApprovalRecallInput (interface)", diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 68ae52f175..ca49305cd2 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -43,6 +43,14 @@ import type { ExecutionContext } from '../kernel/execution-context.zod.js'; * terminal for THIS request/round; the flow walks the `revise` edge to a wait * point, and a later resubmit opens a fresh `pending` request (next round). * Distinct from `recalled` (submitter-initiated withdrawal). + * + * `cancelled` (#13568): the platform voided a pending request because the thing + * it was about stopped existing — nobody decided it and nobody withdrew it. + * Terminal, and deliberately its own state rather than a re-use of `recalled`: + * a recall is an ACT by the submitter, and reporting a platform-initiated void + * as one attributes a withdrawal to a person who never performed it. The + * machine-readable cause rides {@link APPROVAL_CANCEL_REASONS} on the row, so + * the vocabulary here does not have to grow a state per cause. */ export const APPROVAL_STATUSES = [ 'pending', @@ -50,6 +58,7 @@ export const APPROVAL_STATUSES = [ 'rejected', 'recalled', 'returned', + 'cancelled', ] as const; /** Lifecycle state of an approval request — derived from {@link APPROVAL_STATUSES}. */ @@ -78,8 +87,51 @@ export const APPROVAL_STATUS_LABELS = { rejected: 'Rejected', recalled: 'Recalled', returned: 'Returned', + cancelled: 'Cancelled', } as const satisfies Record; +/** + * Why a request reached {@link APPROVAL_STATUSES}' `cancelled` — the + * machine-readable half of a platform-initiated void (#13568, maintainer + * ruling 2026-08-31). + * + * A VALUE for the same reason as {@link APPROVAL_STATUSES}, and a VOCABULARY + * rather than free text because the reason has a consumer that is not a human + * reader: the inbox and the tombstone presentation branch on WHY a row is + * cancelled, and `sys_approval_action.comment` — the only other place the + * cause could have ridden — is submitter/approver prose that no client can + * narrow. A free-text reason on a shipped audit row is the declared-≠-enforced + * shape PD #10 exists to refuse. + * + * One entry today, and the vocabulary still earns its keep: the ruling asked + * for a reason "class", so the next platform-initiated cancellation cause + * extends THIS list instead of minting a second terminal status for itself. + * + * `record_deleted`: the record the request was about was deleted while the + * request was still `pending`. The row is kept as audit evidence — it records + * that an approval was opened and never decided — but it leaves the pending + * count and the inbox's default view, because there is nothing left to decide. + */ +export const APPROVAL_CANCEL_REASONS = [ + 'record_deleted', +] as const; + +/** Why a request was cancelled — derived from {@link APPROVAL_CANCEL_REASONS}. */ +export type ApprovalCancelReason = (typeof APPROVAL_CANCEL_REASONS)[number]; + +/** + * Authored English display label per {@link APPROVAL_CANCEL_REASONS} entry — + * same contract-first shape as {@link APPROVAL_STATUS_LABELS}: the + * `sys_approval_request.cancel_reason` column derives its option labels from + * this map and never re-types them, so the `en` bundle regenerates from the + * contract's own text. + * + * `satisfies` is exhaustive in both directions. + */ +export const APPROVAL_CANCEL_REASON_LABELS = { + record_deleted: 'Related record deleted', +} as const satisfies Record; + /** Live request row. */ export interface ApprovalRequestRow { id: string; @@ -106,6 +158,16 @@ export interface ApprovalRequestRow { submitter_id?: string; submitter_comment?: string; status: ApprovalStatus; + /** + * Why the request was cancelled (#13568) — present only on rows whose + * {@link ApprovalRequestRow.status} is `cancelled`, absent everywhere else. + * + * Optional-nullable rather than required-on-cancelled because a TypeScript + * optional cannot be conditioned on a sibling's value, and because a row + * written before this column existed carries no value: absent reads as "not + * recorded", never as "cancelled for no reason". + */ + cancel_reason?: ApprovalCancelReason | null; /** The flow node id that opened the request (mirrors `flow_node_id`). */ current_step?: string; current_step_index?: number; @@ -299,9 +361,18 @@ export interface ApprovalRequestRow { * its own `submit`) * ooo_substitute #1322 M1: an out-of-office approver's slot was auto-rerouted * to their delegate at resolution time + * cancel #13568: the PLATFORM voided a pending request — nobody + * decided it and nobody withdrew it. The only kind with no + * human actor (`actor_id` is null on it, by construction), + * which is exactly why it must not be recorded as `recall`: + * that would file a submitter withdrawal nobody performed. + * The cause rides `sys_approval_request.cancel_reason` + * ({@link APPROVAL_CANCEL_REASONS}). * * `reassign` / `remind` / `request_info` / `comment` / `ooo_substitute` are * thread interactions and never move the flow; `revise` / `resubmit` do. + * `cancel` moves nothing either — it records that the request stopped being + * decidable. */ export const APPROVAL_ACTION_KINDS = [ 'submit', @@ -316,6 +387,7 @@ export const APPROVAL_ACTION_KINDS = [ 'revise', 'resubmit', 'ooo_substitute', + 'cancel', ] as const; /** Kinds of entries on a request's audit trail — derived from {@link APPROVAL_ACTION_KINDS}. */ @@ -349,6 +421,7 @@ export const APPROVAL_ACTION_KIND_LABELS = { revise: 'Revise', resubmit: 'Resubmit', ooo_substitute: 'Out-of-Office Substitution', + cancel: 'Cancel', } as const satisfies Record; /**