fix(plugin-approvals): show approval_recall for the #3424 admin override (#12716)

[os-litant]

Fixes#12716

Option A only, per triage's corrected re-ruling. Round 1 returned needs_decision because the premise that triage's original pending-only justification rested on measured false; triage has since corrected that premise on the record and narrowed the ruling to option A. Nothing here re-litigates it, and round 1's premise probe was not re-run.

The change

One CEL arm plus its pins. approval_recall's visible predicate now ORs in record.viewer.can_override, spelled byte-identically to its three siblings.

 && 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',

ApprovalService.recall has admitted the override caller since #3424 and isOverrideActor's doc block names recall as one of the four levers, 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. Not a permission change: the service's authorisation set is untouched.

Measured, with file and line

Every claim below I read first-hand in this worktree, off origin/main at c8be11073.

claim	site	reading
the three siblings each carry the override arm	sys-approval-request.object.ts:381, :411, :434	|| has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true
approval_recall carried no such arm	sys-approval-request.object.ts:511 (pre-change)	is_submitter-only
reverse control on the file	sys-approval-request.object.ts	can_override appeared exactly 4 times: the 3 sibling arms + the :332 doc comment. No fourth arm hiding elsewhere
the flag is pending-scoped where it is COMPUTED	approval-service.ts:4848	can_override: row.status === 'pending' && this.isOverrideActor(...) — ANDed
the package declares the scripts I measured with	packages/plugins/plugin-approvals/package.json	["build","typecheck","test"] — so no --filter run matched zero scripts and exited 0

Inferred, not measured — stated separately so it is not read as a reading:

• That the new arm is behaviourally pending-only follows from the :4848 AND, not from the predicate: the arm carries no status test, exactly as the siblings carry none. I did not enumerate every writer of the viewer block to prove nothing else can set can_override on a non-pending row; I pinned the one computation that does (below).
• Whether the CEL grouping question matters: per the dispatch brief it is behaviour-neutral, and the :4848 reading is consistent with that. I verified :4848 myself and did not spend further time on grouping.

Pin-coupling search — what I found, and what I did not

The standing warning was that any existing pin asserting "a non-submitter does NOT see the recall button" would be turned red by this arm, since an override admin is a non-submitter.

No such assertion exists. Swept is_submitter, approval_recall, and the prose forms (non-submitter / not the submitter / isn't the submitter) across the repo, each zero-hit claim backed by a positive control first. What the sweep found instead:

• action-predicate-sparse-face.test.ts:124 — 'an override-only admin still gets the three core decision levers and nothing else (#3424)'. This is the closest thing to the coupled pin and the one worth naming: its assertions did not go red (it only asserted approval_send_back / approval_request_info are false, never recall), but its title became false the moment recall joined the set. Re-expressed rather than left standing as a claim the code no longer honours, and extended to assert the fourth lever plus the two submitter levers that did not move.
• recall-refusal-user-copy.test.ts ([approvals][console] 「撤回审批」按钮对非提交人恒显示,点击后报未翻译的英文服务端原文 #11993's landed refusal-copy half) — not turned red. It is a service-gate test: zero hits for celEngine / visible / SysApprovalRequest, against a positive control confirming the file is greppable (313 lines, its describe found). Its actors are already scoped correctly — :129 reads "User B in the report: not the submitter, not an admin", and :306"a plain non-submitter is still refused". No re-expression needed.
• sys-approval-request.object.test.ts:64 — asserts not.toContain('can_act') over recall. can_override does not contain the substring can_act, so it stays green.

Three prose sites did become stale and are corrected in this PR: sys-approval-request.object.ts:331 (the doc block naming only three levers), :479 ("the predicate keeps a non-submitter from ever seeing a button they cannot use"), and the test file's header at :17. That header also spelled the gate as submitter_id == ctx.user.id, which the predicates have not used since they moved to record.viewer.is_submitter — corrected in the same sentence I was already rewriting, and called out here rather than left as a silent rider.

Pins added — both directions

• positive — an override-only actor (can_act: false, can_override: true, is_submitter: false) sees approval_recall on pending.
• negative — the same actor does not see it on returned.
• the anchor that makes the negative pin non-circular. At the predicate layer the negative direction is only as good as the fixture's can_override: false, so it would pin nothing on its own. approval-revise.test.ts now pins the flag's own scoping against the real service — a platform admin reading a genuinely returned request produced by the real send-back path — with a positive control on pending first. The predicate fixtures are viewer blocks the service really emits; forcing can_override: true onto a returned row would have measured CEL's grouping rather than the product's behaviour.
• The #3424 string pin is extended to the fourth lever, made exhaustive (a fifth lever cannot quietly join without the pin moving), and now asserts all four spell the arm identically — this file has a spelling-drift history, so a fourth wording fails here rather than in review.
• The submitter's own returned recall arm is pinned unchanged.

Ablation evidence

Both legs wrapped in a script carrying trap … EXIT INT TERM on absolute paths derived from git rev-parse --show-toplevel, run after the implementation was committed so the restore leg points at a HEAD that carries it.

Ablation 1 — drop the arm I added.

PRE : disk=7739f9a63dab88b715398927518c4444b271deca head=7739f9a63… (equal, clean start)
PRE : anchor lines=1 arm occurrences=4
POST : disk=4a45492bf7f86f77c96bc546956dba2f89cf6ea7 arm occurrences=3 anchor lines=0
MUTATION CONFIRMED ON DISK: 7739f9a63… -> 4a45492bf… (arm dropped from recall, 3 siblings intact)
Test Files 2 failed | 1 passed (3)
Tests 3 failed | 38 passed (41)
POST-RESTORE: disk=7739f9a63… head=7739f9a63…
RESTORE PROVEN: hash equal to HEAD blob AND git diff HEAD empty for the target

Exactly the three new assertions went red and no others. approval-revise.test.ts stayed green here, correctly — its pin is about the service's flag, not the predicate.

⚠️The first attempt at this ablation was a no-op that exited 0. A perl -0pi -e substitution quoted the newline as a literal backslash-n, matched zero times, and left the file unmutated. The script's own git hash-object check caught it (FAIL: hash UNCHANGED — the mutation did not reach disk. This ablation DID NOT RUN.) rather than reporting a green ablation. Recorded because the failure mode is invisible from the editor's exit code, which is the whole reason the hash check is there. The mutation was moved to a helper carrying its own match-count guard, so the zero-hit case now fails loudly at two independent points.

Ablation 2 — prove the anchor pin can go red. Relaxed attachViewers's pending-scoping (can_override: row.status === 'pending' && … to unconditional), a temporary measurement fully restored; the shipped diff does not touch can_override.

MUTATION CONFIRMED ON DISK: 6be225552… -> c3f7a5e3f… (can_override no longer pending-scoped)
× viewer.can_override drops on `returned`, … (#12716) [approval-revise.test.ts]
× viewer.can_override reflects the privilege, and drops once finalized [approval-service.test.ts]
Tests 2 failed | 306 passed (308)
RESTORE PROVEN: hash equal to HEAD blob AND git diff HEAD empty for the target

The new pin goes red, alongside the pre-existing drops once finalized pin — exactly the two tests that assert that condition, and nothing else.

Verification

Heavy steps serialized through scripts/pm/os-verify-lock.sh; exit codes captured before any pipe; each verdict quoted from the gate's own judgment line.

• Suite, re-run on the final commit 47f2f1b4b with a clean tree: Test Files 34 passed (34) / Tests 623 passed (623), lock VERDICT command-exit 0.
• Dependency closure built first (pnpm --filter '@objectstack/plugin-approvals^...' build) — VERDICT command-exit 0 · held the lock 263s.
• Package typecheck: tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json, VERDICT command-exit 0. ⚠️This does NOT cover the test files.tsc --listFiles puts 0 of the three edited test files in the program (control: the source file is present, count 1). The measurement that does cover them is check:type-check-debt, below.
• Gate union re-derived live on the actual changed set: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, which reported change set derived from git — 5 path(s) vs merge base c8be11073 and --repo … checked against this checkout's 'origin' remote — it holds. It named no contract family, so the PM's clause-② no is not contradicted. It also reported Model tier — no path-derived mandate.
• All 21 path-matched + 8 convention-triggered families run, all green: check:changeset-gate-self-tests, check:cross-package-test-inputs (both the lint and ci invocations), check:objectql-double-limit, check:objectui-changeset, check:page-declaration-shape, check:pm-half-states, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:where-matcher, check:i18n-stale-fill, check:nul-bytes, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-comment-mask-adoption, check-empty-changeset, check-plugin-teardown-shape, docs-audit/check-affected-docs, docs-audit/check-drift-comment, release-rehearsal-clone --self-test.
• check:i18n — first run exited 1 as a prerequisite failure, in its own words: "Nothing was checked: no bundle was compared and no config was parsed, so this result says NOTHING about whether the committed translation bundles are in sync." Built @objectstack/cli as it instructed and re-ran to a real measurement: check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
• check:type-check-debt — built the workspace closure exactly as lint.yml does, then: check-type-check-coverage --re-measure: OK — 31 ledger entr(ies) re-measured in 242.4s, 1570 raw tsc error(s) total, none above its recorded number. This is the gate that does typecheck the test files, and it reports @objectstack/plugin-approvals: TEST_DEBT records 347, tsc now reports 345 (-2) — my test edits added zero type errors. The -2 surplus is pre-existing (the same run reports surpluses on objectql -103, runtime -10, plugin-auth -3); the gate states outright that lowering is not required to land an improvement, so I did not touch the ledger.

NOT MEASURED (reported as neither pass nor fail)

• scripts/pm/check-half-states.mjs exited 3 for want of a credential, in its own words: "Nothing was swept: no issue was listed, no predicate (H1–H16) ran… It is not a clean board and it is not a dirty one — it is no reading at all." Environmental, and it reads the PM board rather than this diff.

Scope

Untouched, deliberately: can_override itself (it feeds the three sibling predicates whose endpoints are pending-only, so relaxing it would widen those buttons onto statuses their services refuse), and the :2774 override short-circuit (option C — a permission narrowing on a live endpoint, maintainer floor, carried on #12775). No Blocked-by: is declared against #12775: option A is pending-only by construction and is unaffected whichever way that card is ruled. #11993 is not re-scoped here. No file under packages/spec is reached by this change.

Draft on purpose — the PM seat lands it. Auto-merge not armed.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• 1 name(s) were too generic to anchor anything (single lowercase words)
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ead731756fb25549176e178e0ee2e4df20338435 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6e6ca3f4fd509f5cc0cad84dfc33c4278dff925f — the merge of head 47f2f1b4b0fbe3db70f041df554671f9e768b611 into base ead731756fb25549176e178e0ee2e4df20338435, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6e6ca3f4fd509f5cc0cad84dfc33c4278dff925f && git checkout 6e6ca3f4fd509f5cc0cad84dfc33c4278dff925f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ead731756fb25549176e178e0ee2e4df20338435 47f2f1b4b0fbe3db70f041df554671f9e768b611 && git checkout -B drift-repro ead731756fb25549176e178e0ee2e4df20338435 && git merge --no-ff 47f2f1b4b0fbe3db70f041df554671f9e768b611
node scripts/docs-audit/affected-docs.mjs --json ead731756fb25549176e178e0ee2e4df20338435

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.