Merge origin/main into claude/issue-12758-datasource-credentials-ref

Brings the branch onto the current type-check-debt ledger sweep (#12798) and
the dispatch-gates hintCovers fix (#12794), so the gate-family derivation below
reads a tree someone is actually on rather than a 5-commit-stale one.

Part of #12758

Author: claude

.changeset/approval-recall-admin-override-arm.md

@@ -0,0 +1,34 @@
+---
+"@objectstack/plugin-approvals": patch
+---
+
+fix(plugin-approvals): the `approval_recall` action shows for the #3424 admin override (#12716)
+
+`ApprovalService.recall` has admitted two callers since #3424 — the submitter,
+and a platform/tenant admin releasing a stuck request — and `isOverrideActor`'s
+own doc block names recall as one of the four override levers in so many words.
+The declared action that reaches that endpoint did not agree: `approval_recall`'s
+`visible` predicate was submitter-only, while its three siblings
+(`approval_approve` / `approval_reject` / `approval_reassign`) each OR in
+`record.viewer.can_override`.
+
+So recall was the one lever the override covers whose button never appeared. An
+admin rescuing an approval routed to an unstaffed position could approve or
+reject their way out — writing a decision nobody made — or reassign it, but
+could not simply withdraw it. This is declared-vs-enforced drift in the less
+usual direction: a capability the server grants that no UI entry exposed.
+
+`approval_recall`'s `visible` now ORs in `record.viewer.can_override`, spelled
+byte-identically to the three siblings.
+
+Not a permission change: the service's authorisation set is untouched, and
+`can_override` was already computed server-side for every viewer.
+
+**Pending-only, and enforced rather than asserted.** The new arm carries no
+status test of its own — neither do the siblings — because the flag is already
+status-scoped where it is computed: `attachViewers` sets
+`can_override: row.status === 'pending' && isOverrideActor(...)`, ANDed, so the
+flag can never be true off `pending` and the arm is pending-only in effect
+however CEL groups the expression. The submitter's own `returned` (revise
+window) arm is unchanged. Pinned in both directions, with the flag's own scoping
+pinned against the real service on a genuinely `returned` row.

packages/plugins/plugin-approvals/src/action-predicate-sparse-face.test.ts

@@ -121,14 +121,54 @@ describe('#8990 — the fail-closed intent survives as a real false, and the lev
     expect(evaluate(visibleOf('approval_remind'), approver)).toBe(false);
   });
 
-  it('an override-only admin still gets the three core decision levers and nothing else (#3424)', () => {
+  it('an override-only admin gets the four levers the override covers, and nothing else (#3424, +recall #12716)', () => {
+    // Re-expressed for #12716. This pin previously read "the three core decision
+    // levers and nothing else" — true when written, and it is the shape of pin
+    // the recall override arm was expected to turn red. It did not go red (it
+    // never asserted anything about recall), but its TITLE became false the
+    // moment recall joined the set, so it is restated rather than left to read
+    // as a claim the code no longer honours.
     const admin = { status: 'pending', viewer: { can_act: false, can_override: true, is_submitter: false } };
     expect(evaluate(visibleOf('approval_approve'), admin)).toBe(true);
     expect(evaluate(visibleOf('approval_reject'), admin)).toBe(true);
     expect(evaluate(visibleOf('approval_reassign'), admin)).toBe(true);
+    // #12716 — the fourth lever. An override admin is a NON-SUBMITTER who now
+    // sees Recall: the service has authorised them since #3424, and this is the
+    // one lever that releases a stuck request without writing a decision on
+    // someone else's behalf.
+    expect(evaluate(visibleOf('approval_recall'), admin)).toBe(true);
     // `can_override` was never OR'd into the secondary levers, and still is not.
     expect(evaluate(visibleOf('approval_send_back'), admin)).toBe(false);
     expect(evaluate(visibleOf('approval_request_info'), admin)).toBe(false);
+    // Nor into the remaining submitter levers — recall is the only one that
+    // moved, and remind/resubmit stay shut for an actor who is not the submitter.
+    expect(evaluate(visibleOf('approval_remind'), admin)).toBe(false);
+    expect(evaluate(visibleOf('approval_resubmit'), { ...admin, status: 'returned' })).toBe(false);
+  });
+
+  it('the recall override arm is pending-only in effect, because `can_override` is itself pending-scoped (#12716)', () => {
+    // The negative direction, and the reason the arm needs no status test of its
+    // own. `attachViewers` computes
+    //   can_override: row.status === 'pending' && isOverrideActor(...)
+    // — ANDed — so for the SAME override actor the flag the service attaches is
+    // true on `pending` and false on `returned`. Both rows below are viewer
+    // blocks the service really emits: forcing `can_override: true` onto a
+    // `returned` row would pin a state the server never produces, and would
+    // measure CEL's grouping rather than the product's behaviour.
+    //
+    // That is what makes "pending-only" enforced rather than asserted in prose.
+    // The flag's own scoping is pinned against the REAL service — an override
+    // admin reading a genuinely `returned` request — in `approval-revise.test.ts`;
+    // this pair is the predicate half of the same claim.
+    const onPending = { status: 'pending', viewer: { can_act: false, can_override: true, is_submitter: false } };
+    const onReturned = { status: 'returned', viewer: { can_act: false, can_override: false, is_submitter: false } };
+    expect(evaluate(visibleOf('approval_recall'), onPending)).toBe(true);
+    expect(evaluate(visibleOf('approval_recall'), onReturned)).toBe(false);
+    // And the submitter's own `returned` arm is untouched by the new OR — the
+    // revise-window withdraw stays exactly as available as it was.
+    expect(evaluate(visibleOf('approval_recall'), {
+      status: 'returned', viewer: { can_act: false, can_override: false, is_submitter: true },
+    })).toBe(true);
   });
 
   it('the submitter still gets remind / recall on pending and resubmit / recall on returned', () => {

packages/plugins/plugin-approvals/src/approval-revise.test.ts

@@ -315,6 +315,39 @@ describe('Send back for revision (ADR-0044)', () => {
     await expect(editAttempt()).rejects.toThrow(/RECORD_LOCKED/);          // round 2 pending → re-locked
   });
 
+  it('viewer.can_override drops on `returned`, which is what keeps the recall override arm pending-only (#12716)', async () => {
+    // `sys_approval_request`'s `approval_recall` action ORs in
+    // `record.viewer.can_override`, spelled exactly like its three siblings and
+    // with no status test of its own. What makes that arm pending-only is not
+    // the predicate — it is `attachViewers`, which ANDs the status in when it
+    // COMPUTES the flag. `action-predicate-sparse-face.test.ts` pins the
+    // predicate half against fixture viewer blocks; this pins the half those
+    // fixtures stand for, against a genuinely `returned` row produced by the
+    // real send-back path. Without it, "pending-only" would be a property of
+    // hand-written fixtures rather than of the service.
+    //
+    // Relaxing the flag is also not available as a fix: it feeds the three
+    // sibling predicates too, and THEIR endpoints are pending-only
+    // (`decideNode`, and reassign via `loadPendingRow`), so widening it would
+    // put those three buttons on statuses their services refuse.
+    registerReviseFlow();
+    const { req } = await startFlow();
+    // A platform admin with no tenant scope: `loadRequest` narrows by the
+    // CALLER's org, and this harness's requests carry none.
+    const ADMIN = { isSystem: false, userId: 'root', positions: [], permissions: ['admin_full_access'] } as any;
+
+    // Positive control first — the same actor, same request, on `pending`.
+    const whilePending = await service.getRequest(req.id, ADMIN);
+    expect(whilePending!.status).toBe('pending');
+    expect(whilePending!.viewer!.can_override).toBe(true);
+
+    await service.sendBack(req.id, { actorId: 'u1' }, asUser('u1'));
+
+    const whileReturned = await service.getRequest(req.id, ADMIN);
+    expect(whileReturned!.status).toBe('returned');
+    expect(whileReturned!.viewer!.can_override).toBe(false);
+  });
+
   it('recall crossing the revise window cancels the run (returned → recalled)', async () => {
     registerReviseFlow();
     const { runId, req } = await startFlow();

packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts

@@ -13,8 +13,11 @@
  *   • every `type:'api'` target resolves `{id}` and points at a route that the
  *     REST server actually registers (approve/reject/reassign/recall/remind/
  *     request-info/revise/resubmit) — a typo'd verb would 404 silently in the UI;
- *   • submitter-only levers (remind/recall/resubmit) gate on
- *     `submitter_id == ctx.user.id` so a non-submitter never sees them.
+ *   • submitter levers (remind/recall/resubmit) gate on the server-computed
+ *     `record.viewer.is_submitter`, so a plain non-submitter never sees them.
+ *     `recall` additionally ORs in the #3424 admin override (#12716) — an
+ *     override admin is therefore the one non-submitter who does see it, and
+ *     is a caller `ApprovalService.recall` already authorises.
  */
 
 import { describe, it, expect } from 'vitest';
@@ -74,17 +77,40 @@ describe('sys_approval_request declared actions', () => {
     }
   });
 
-  it('the core decision levers OR in the admin override (#3424) so a stuck request is recoverable', () => {
-    // approve / reject / reassign additionally show for a platform/tenant admin
-    // (`record.viewer.can_override`) so an approval routed to an unstaffed
-    // position — otherwise undecidable, locking the record forever — can be
-    // rescued in-product. The secondary approver levers stay slot-only.
-    for (const name of ['approval_approve', 'approval_reject', 'approval_reassign']) {
+  it('the levers the admin override covers OR it in (#3424, +recall #12716) so a stuck request is recoverable', () => {
+    // approve / reject / reassign / recall additionally show for a
+    // platform/tenant admin (`record.viewer.can_override`) so an approval routed
+    // to an unstaffed position — otherwise undecidable, locking the record
+    // forever — can be rescued in-product. Recall joined the set in #12716: the
+    // service has admitted the override caller on that endpoint since #3424
+    // (`isOverrideActor`'s doc block names recall as one of the four levers),
+    // and it is the only one that RELEASES the record without recording a
+    // decision on someone else's behalf. The secondary approver levers and the
+    // remaining submitter levers stay slot-only.
+    for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_recall']) {
       expect(vis(name)).toContain('record.viewer.can_override');
     }
-    for (const name of ['approval_send_back', 'approval_request_info']) {
+    for (const name of ['approval_send_back', 'approval_request_info', 'approval_remind', 'approval_resubmit']) {
       expect(vis(name)).not.toContain('can_override');
     }
+
+    // Exhaustive, so a FIFTH lever cannot quietly join the override set without
+    // this pin moving: the two loops above only constrain the names they name.
+    expect(
+      actions.filter((a) => vis(a.name).includes('can_override')).map((a) => a.name).sort(),
+      'exactly these levers OR in the #3424 override',
+    ).toEqual(['approval_approve', 'approval_reassign', 'approval_recall', 'approval_reject']);
+
+    // ONE spelling of the arm, not four. This file has a spelling-drift history,
+    // and a fourth wording of the same idea is its own defect — `toContain` on
+    // the whole arm (leading ` || ` included) is what makes drift fail here
+    // rather than in review.
+    const OVERRIDE_ARM =
+      ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true';
+    for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_recall']) {
+      expect(vis(name), `${name} must spell the override arm exactly as its siblings do`)
+        .toContain(OVERRIDE_ARM);
+    }
   });
 
   it('recall stays available while a returned request is still the submitter\'s to abandon', () => {

packages/plugins/plugin-approvals/src/sys-approval-request.object.ts

@@ -328,11 +328,14 @@ export const SysApprovalRequest = ObjectSchema.create({
   // per-viewer block (#3310): approver actions on `record.viewer.can_act`
   // (the caller is a current pending approver — same check the service
   // authorizes a decision with, so position/team approvers resolve correctly),
-  // submitter actions on `record.viewer.is_submitter`. The core decision levers
-  // (approve/reject/reassign) additionally OR in `record.viewer.can_override`
-  // (#3424) so a platform/tenant admin can rescue a request routed to an
+  // submitter actions on `record.viewer.is_submitter`. The four levers the
+  // #3424 override covers (approve/reject/reassign, and recall since #12716)
+  // additionally OR in `record.viewer.can_override`
+  // so a platform/tenant admin can rescue a request routed to an
   // unstaffed position — otherwise undecidable, locking the record forever — by
-  // approving, rejecting, or reassigning it to a real approver. `viewer` is
+  // approving, rejecting, reassigning it to a real approver, or recalling it
+  // (the lever that releases the record without recording a decision nobody
+  // made). `viewer` is
   // attached by getRequest/listRequests; where it is absent the predicate fails
   // closed.
   //
@@ -476,8 +479,13 @@ export const SysApprovalRequest = ObjectSchema.create({
     // Remind / recall (pending) and resubmit / recall (returned). These are the
     // submitter's own levers, so `visible` gates on `record.viewer.is_submitter`
     // (server-computed on the current viewer). The service re-checks ownership;
-    // the predicate keeps a non-submitter from ever seeing a button they cannot
-    // use.
+    // the predicate keeps a plain non-submitter from ever seeing a button they
+    // cannot use.
+    //
+    // `recall` is the one exception, and it is not a widening: it ALSO ORs in
+    // the #3424 admin override (#12716), because an override admin is a caller
+    // `ApprovalService.recall` already authorises. Remind and resubmit keep no
+    // override arm.
     {
       name: 'approval_remind',
       label: 'Send reminder',
@@ -508,9 +516,29 @@ export const SysApprovalRequest = ObjectSchema.create({
       ],
       // Recall applies while the request is live for the submitter — pending
       // (withdraw) or returned (abandon the revision instead of resubmitting).
+      //
+      // The second arm is the #3424 admin override, spelled byte-identically to
+      // the three core decision levers above (#12716). `ApprovalService.recall`
+      // has admitted the override caller since #3424 — `isOverrideActor`'s own
+      // doc block names recall as one of the four levers — so until this arm
+      // landed, recall was the one authorised capability with no button: an
+      // admin could approve or reject their way out of a stuck request (writing
+      // a decision that did not happen) or reassign it, but could not withdraw.
+      //
+      // The override arm carries no status test of its own, on purpose, because
+      // it does not need one and the siblings do not have one either: the flag
+      // is already status-scoped where it is COMPUTED. `attachViewers` in
+      // `approval-service.ts` sets
+      // `can_override: row.status === 'pending' && isOverrideActor(...)` —
+      // ANDed — so `record.viewer.can_override` can never be true off `pending`,
+      // and this arm is pending-only in effect however CEL groups the
+      // expression. Pinned in both directions in
+      // `action-predicate-sparse-face.test.ts`, with the flag's own scoping
+      // pinned against the real service in `approval-revise.test.ts`.
       visible:
         'has(record.status) && (record.status == "pending" || record.status == "returned")' +
-        ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true',
+        ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true' +
+        ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true',
       locations: ['record_section'],
       successMessage: 'Recalled.',
       refreshAfter: true,