Uh oh!
There was an error while loading. Please reload this page.
fix(approvals): the #3424 override reaches a pending request only — refuse override-recall of a returned request (#12775) - #14571
Conversation
…efuse override-recall of a returned request `ApprovalService.recall`'s override short-circuit sat above ADR-0044's widened state check with no status test of its own, so a platform or tenant admin could recall a `returned` request — a reach no UI ever offered (`viewer.can_override` is ANDed with `status === 'pending'` where it is computed) and no pin ever held, while the `isOverrideActor` doc block and the viewer flag both said pending-only. Re-scope the short-circuit to `pending`, spelled exactly as the viewer flag, so the gate, the doc block and the flag agree at one point. On `returned` an override actor is judged as any other non-submitter and receives the existing refusal (`FORBIDDEN:` + the catalog sentence); the developer log line now names the status it refused on. The submitter's revise-window recall and the override's pending-request rescue are untouched. Pins in approval-revise.test.ts: the narrowing (both override postures, nothing moves — status, actions, lock, run), the reverse checks on `pending`, the submitter's ADR-0044 recall, and a control that the refusal shape is unchanged. Changeset declares the narrowing as an explicit behaviour change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…ll gate edit Pure line rot: the recall-gate hunk in approval-service.ts moved the six `context.isSystem` read sites below it by +20 lines. Rewritten by the gate's own `--fix` (`pnpm gen:system-context-census`); no row content changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…me log string `check:doc-authoring` (maintainer ruling 2026-08-12) refuses a new internal issue id inside sibling-package string prose; the anchor lives in the adjacent comment, where the reader who can resolve it reads it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…erride-recall-pending-only
… tree Regeneration commit after merging origin/main (the merge driver deferred content/docs/permissions/system-context.mdx). Both sides had re-anchored row 42 for approval-service.ts line shifts — this branch's recall-gate hunk and #14542's JSDoc hunk — so the anchors are re-derived from the merged code by the gate's own --fix; no row content changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5b97d874264945f7494d12240ee5eb67c744ac46 && git checkout 5b97d874264945f7494d12240ee5eb67c744ac46
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7286dd58e806ed321cbdfc23a1c455db8f80b1ad 9da369ffe1d114b4d1684977fe4ca579336365f7 && git checkout -B drift-repro 7286dd58e806ed321cbdfc23a1c455db8f80b1ad && git merge --no-ff 9da369ffe1d114b4d1684977fe4ca579336365f7
node scripts/docs-audit/affected-docs.mjs --json 7286dd58e806ed321cbdfc23a1c455db8f80b1ad
|
os-sales
commented
Sep 2, 2026
Landing provenance — ready + auto-merge at head |
Fixes#12775
Ruling of record (verbatim, untranslated)
Maintainer, 2026-09-02, via the director seat (12775#issuecomment-5504841854), replying to the decision batch whose item for this card recommended 「(ii),附条件:实施 PR 的 changeset 把这条收窄写成显式行为变更;回退 (i)」:
Read as (ii): the audit reading is unobtainable and counts as zero; the 2026-08-29 ruling stands — B: re-scope the status-blind override short-circuit in
recalltopendingso the gate, the prose and the viewer flag agree at one point; A remains the fallback; the changeset declares the narrowing as an explicit behaviour change so one revert restores it; one pin on the narrowing (returned+ override actor ⇒ refused) with the reverse check; #12716 (landed as PR #12791) is unaffected. The card's own facet block (the shape agreed on 2026-08-29):The hard precondition is discharged by ruling (ii): no audit reading was attempted and no audit instrument was built.
What changed
ApprovalService.recall(packages/plugins/plugin-approvals/src/approval-service.ts): the #3424 override short-circuit is now conditioned onraw.status === 'pending'— spelled exactly asattachViewerscomputesviewer.can_override— so onreturnedan override actor is judged as any other non-submitter and gets the existing refusal (message prefixFORBIDDEN:+ the operation catalog'sapproval_recall_not_submittersentence; over REST,handleApprovalErrormaps that prefix to403+ codeFORBIDDEN). No new error code, no new envelope shape, no new export. The comment block above the gate (the old#3424/#11993text, whose sentence "the GATE is untouched" is no longer true) is rewritten to state the pending-only scope and this card; therecalldoc block gains the same statement. The developer half of the refusal (thelogger.warnline, level unchanged) now also names the status it refused on — its text carries no tracker id (check:doc-authoring).pendingand onreturned(ADR-0044 revise window): untouched.pending(the Approval routed to an empty position permanently locks the record (no admin override, no recovery) #3424 stuck-record rescue, lock release included): untouched.isOverrideActordoc block ("may always act on a PENDING request … or recall it") is now true without an edit;attachViewersandsys-approval-request.object.tsare not touched.Design note — system context.
isOverrideActor's first arm iscontext.isSystem, so the same expression also scopes a system caller's override topending. A machine caller keeps a path onreturnedby naming the submitter asactorId(resolveActorreturns a system caller's explicit actor), which is the ADR-0044 semantics. No in-repo caller invokesrecallunder a system context (packages/rest/src/rest-server.tsis the only call site, with the request's own context); the#13568record-delete path cancels, it does not recall. Flagged in the report's open questions as a note, not a blocker — spelling the gate as the viewer flag is what the ruling asked for.Premise verification (on
origin/mainfed4fa409, before any edit)recalladmittedpendingandreturned, and the gateif (!this.isOverrideActor(context, raw.organization_id ?? null) && raw.submitter_id && …)passed an override actor regardless ofraw.status(:2904).attachViewerscomputedcan_override: row.status === 'pending' && this.isOverrideActor(...)(:5195); theisOverrideActordoc block said PENDING (:938).approval_recall'svisible(PR fix(plugin-approvals): showapproval_recallfor the #3424 admin override (#12716) #12791) ORsrecord.viewer.can_override, pending-only through the flag; no UI path offered override-recall onreturned.packages/specchange needed.returnedrequest. Every existing admin-recall pin (approval-service.test.ts,admin-exemption-retired.test.ts,recall-refusal-user-copy.test.ts) opens apendingrequest;approval-revise.test.ts:357already asserted the non-submitter refusal onreturnedfor a non-admin. Nothing to reverse or register.Hypotheses
returnedfalls into the existing refusal branch with its existing catalog sentence, developer message and wire prefix.Errorwhose message starts withFORBIDDEN:; the ADR-0112 envelope (code: 'FORBIDDEN', HTTP403) is minted at the REST seam byhandleApprovalError(rest-server.ts:11477, regex^FORBIDDEN), the same way every approvals refusal reaches the wire. The pins assert the prefix the mapping keys on (the service-seam half of the contract) and the catalog sentence; a REST-level live-emission pin for that row is outside this card's file surface and is recorded as an out-of-scope finding (see below). No code minted, nothing underpackages/spectouched.returned → unlockedand the submitter's revise-window recall pins inapproval-revise.test.tsstay green (24/24 file, 662/662 package).decideNode,loadPendingRow); all four override levers are now pending-only and theisOverrideActordoc block is true without edits.Pins (
packages/plugins/plugin-approvals/src/approval-revise.test.ts, newdescribeblock driving the REAL automation engine and the REAL record-lock hook)posture) on a genuinelyreturnedrow:viewer.can_overrideisfalse,recallrejects with anErrorwhose message matches^FORBIDDEN:and equalsFORBIDDEN:+BUILTIN_OPERATION_MESSAGES.en.approval_recall_not_submitter; afterwards the status is stillreturned, norecallaction row exists, the record is still unlocked (the refusal touched no lock), the run status is unchanged and notcancelled, and the submitter can still resubmit (round 2 opens).pending:can_overrideistrue, the record is locked,recalllands (recalled,resumed: true,recallaction row, lock released).pendingtoo (the narrowing is about status, not posture).returned— lands,resumed: false, runcancelled.returnedis refused exactly as before, and the override actor's refusal is byte-identical to it (no new refusal shape).Changeset
.changeset/approval-override-recall-pending-only.md—@objectstack/plugin-approvals: patch. Level reason: no exported symbol is added, removed or changed (measured:git diff -U0 fed4fa409...HEAD | grep exportmatches only theexport class ApprovalServicehunk-context lines). The body states the narrowing as an explicit behaviour change in plain words (override-recall of areturnedrequest is now refused; onlypendingrequests are override-recallable; the submitter's revise-window recall is unchanged; why), per the ruling's condition. Nomajor/ breaking declaration, so no ADR-0087 marker is owed (check:adr-0087-registrationgreen).Clause-② declaration (from the ACTUAL diff)
Yes — accept-set narrowing on a live endpoint (
POST /api/v1/approvals/requests/:id/recall: an override actor on areturnedrequest was admitted, is now refused with403 FORBIDDEN). Export surface: unchanged (git diff -U0 fed4fa409...HEAD | grep export→ hunk-context lines only).needs:contract-reviewhangs on this PR and on #12775.Ablation (on the committed tree, HEAD
971c8cabf)Mutation:
const overrideAdmits = raw.status === 'pending'→const overrideAdmits = true /* ABLATION-12775: status-blind */(line-neutral). On disk:injected=1 removed_needle_remaining=0. Run:Tests 2 failed | 22 passed (24)— pins 1 (platform admin: expected null to be an instance of Error) and 5 (Cannot read properties of null (reading 'message')on the admin refusal) red, pins 2–4 green — direction: red, as predicted. Restore:git checkout HEAD -- ABSOLUTE_PATHinside atrap … EXIT INT TERM; proven bygit hash-objectequal to the HEAD blob (fed51d81…),git diff HEADempty, marker count 0. Nodist/is involved (the suite imports./approval-service.jsfrom source), soablation-dist-preflightdoes not apply.Verification on the final head
9da369ffe(after mergingorigin/main310dd01a0; merge-tree clean against7286dd58e)All exits captured before any pipe; verdict lines quoted from the tools.
pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2→Test Files 36 passed (36)/Tests 662 passed (662)pnpm --filter @objectstack/plugin-approvals typecheck→check:test-typecheck: OK — @objectstack/plugin-approvals's test layer compiles … 8 file(s) / 324 error(s) / 27 pinned signature(s)(test layer unchanged at 324;tsc --noEmitcoverssrc/**minus tests, the test layer is covered bytsconfig.test.jsonviacheck:test-typecheck)pnpm check:system-context-census→OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read.pnpm check:doc-authoring→sibling-package prose ids hold the baseline — 831 pinned site(s) across 231 file(s) … no growth, no burn-down unrecorded.pnpm check:engine-double-contract→OK — 753 pinned, 134 in the DEBT ledger, 3 exempt.pnpm check:objectql-double-limit→baseline key set verified against 310dd01: no files added.pnpm check:nul-bytes→OK (scanned 7949 text file(s) … no raw ASCII control bytes).Derived gate family (
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, from the merge base; 61 commands) run in five lock-serialised batches on971c8cabf/9f08922f6(the doc-authoring fix landed as9f08922f6; the merge and census regeneration after it touched no gate input other than the census page, which is re-verified above):@objectstack/client-reactclosure — first run was PREREQUISITE NOT MET) / strictness-ledger / variant-docs / yaml-examples, changeset-gate-self-tests, corpus-claim-drift, doc-anchors, doc-authoring (red on the first run: a tracker id in the new log string — fixed in9f08922f6), docs-audit-scope, docs-redirects, docs-single-h1, engine-double-contract, i18n-stale-fill, logger-receiver-detach, merge-driver, objectql-double-limit, objectui-changeset, page-declaration-shape, published-files, published-readme-links, query-options-erasure, react-page-adapter-contract, role-word, skill-identifier-liveness, slot-lookup, test-source-alias, type-check-coverage, type-source-resolution, vendor-version-stamps, where-matcher, nul-bytes.check-test-completeness(exit 3:PREREQUISITE NOT MET — this gate grades a saved turbo run test log),check:dual-build-cjs-loads(exit 3:PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/),check:i18n(exit 1:PREREQUISITE NOT MET — the workspace CLI is not built),check:type-check-debt(exit 3:PREREQUISITE NOT MET — --re-measure cannot run: 20 workspace dependenc(ies) … have no built type entry point).Adjacent mechanical fix:
content/docs/permissions/system-context.mdxrow 42 re-anchored by the gate's own--fix(pnpm gen:system-context-census) — first for this branch's +20-line hunk (971c8cabf), then again on the merged tree afterorigin/mainbrought #14542's hunk in the same file (9da369ffe, the os-regen deferral discharged as its own commit perscripts/pm/os-regen-merge.sh). No row content changed.Out of scope
packages/rest'shandleApprovalErrorFORBIDDEN → 403row has no live-emission pin inrest-approvals-wire-codes.test.ts(that file pinsTHROTTLED,READ_BACK_FAILEDand the template-generated 500s). Filed as afinding, see the report comment on [Decision] Override-recall of areturnedapproval: an ADR-0044 side effect to retire, or a capability to keep? The gate, the prose and the viewer flag disagree three ways #12775. approvals: ten service endpoints returnrequest: fresh!— a non-null assertion that is false whenever the read-back is org-filtered out, shipping{ "request": null }with HTTP 200 #12769 and approvals: theapproval_recallaction hides the #3424 admin-override recall — the service admits it, the visible predicate never shows it #12716 remain untouched by this PR.slaDueAtJSDoc, :600-630) was not touched; it has since landed onmainand merged cleanly here.Generated by Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code