From d3ebf3b55c11dfc6c592f96ac24ea99d1234e16c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 01:22:37 +0000 Subject: [PATCH 1/4] fix(approvals): auto-cancel a record's pending approvals when the record is deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi --- .../approval-auto-cancel-on-record-delete.md | 85 ++++ .../plugin-approvals/src/approval-service.ts | 166 ++++++++ .../plugin-approvals/src/approvals-plugin.ts | 15 +- .../plugin-approvals/src/lifecycle-hooks.ts | 130 +++++++ .../record-delete-cancel.integration.test.ts | 368 ++++++++++++++++++ .../src/sys-approval-request.object.ts | 37 +- .../approval-status-vocabulary.test.ts | 6 + .../src/translations/en.objects.generated.ts | 13 +- .../translations/es-ES.objects.generated.ts | 13 +- .../translations/ja-JP.objects.generated.ts | 13 +- .../translations/zh-CN.objects.generated.ts | 13 +- packages/spec/api-surface/contracts.json | 3 + packages/spec/export-origins/contracts.json | 3 + .../spec/src/contracts/approval-service.ts | 73 ++++ 14 files changed, 926 insertions(+), 12 deletions(-) create mode 100644 .changeset/approval-auto-cancel-on-record-delete.md create mode 100644 packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts 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/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 3fbf4ebae9..865e1eb20c 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 @@ -292,6 +293,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). * @@ -467,6 +482,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), @@ -2890,6 +2909,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..6f129e8a30 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts @@ -0,0 +1,368 @@ +// 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'; +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, + } as any) as Promise; + + const actionsFor = (requestId: string) => + engine.find('sys_approval_action', { + where: { request_id: requestId }, context: SYSTEM, + } as any) as Promise; + + const approverIndexFor = (requestId: string) => + engine.find('sys_approval_approver', { + where: { request_id: requestId }, context: SYSTEM, + } as any) 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 } as any)) + .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, + } as any) 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; /** From 408716c18cac64a65cfe2435f46a7d64d01c21dd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 01:50:57 +0000 Subject: [PATCH 2/4] 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 Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi --- .../docs/permissions/tenant-audit-census.mdx | 28 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 16 +++++------ 2 files changed, 22 insertions(+), 22 deletions(-) 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 | From a334b2c332e8b881c74279b70b2ec91647af2c3d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 02:23:56 +0000 Subject: [PATCH 3/4] test(approvals): declare the new integration test's read options instead of erasing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi --- .../record-delete-cancel.integration.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) 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 index 6f129e8a30..b0d33c626a 100644 --- a/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/record-delete-cancel.integration.test.ts @@ -46,6 +46,12 @@ 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'; @@ -116,17 +122,17 @@ describe('a deleted record auto-cancels its pending approvals (#13568)', () => { engine.find('sys_approval_request', { where: { object_name: 'crm_leave_request', record_id: recordId }, context: SYSTEM, - } as any) as Promise; + } satisfies EngineQueryOptions) as Promise; const actionsFor = (requestId: string) => engine.find('sys_approval_action', { where: { request_id: requestId }, context: SYSTEM, - } as any) as Promise; + } satisfies EngineQueryOptions) as Promise; const approverIndexFor = (requestId: string) => engine.find('sys_approval_approver', { where: { request_id: requestId }, context: SYSTEM, - } as any) as Promise; + } satisfies EngineQueryOptions) as Promise; const openOn = (recordId: string, runId: string) => svc.openNodeRequest({ object: 'crm_leave_request', recordId, runId, nodeId: 'manager_review', @@ -316,8 +322,9 @@ describe('a deleted record auto-cancels its pending approvals (#13568)', () => { 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 } as any)) - .toEqual([]); + expect(await engine.find('crm_leave_request', { + where: { id: 'LR6' }, context: SYSTEM, + } satisfies EngineQueryOptions)).toEqual([]); boom.mockRestore(); }); }); @@ -358,7 +365,7 @@ describe('#13568 control — without the linkage the same delete strands the req const rows = await engine.find('sys_approval_request', { where: { object_name: 'crm_leave_request', record_id: 'LR6' }, context: SYSTEM, - } as any) as any[]; + } 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); From 1112d50aa93f68cfd6eb88ae7fad224f51ef912d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 03:06:27 +0000 Subject: [PATCH 4/4] docs(permissions): re-anchor the system-context census after the approvals insertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_015adLit3ZYASJiXwxKG78Wi --- content/docs/permissions/system-context.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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` |