docs(spec): IApprovalService.recall names every actor who may recall, and scopes each one by status (#14670) - #15645
Conversation
…l, scoped by status The docstring said "Only the submitter (or a system context) may recall", then widened to `returned` requests in a second paragraph. Both halves were wrong: - The list was not exhaustive. A #3424 override actor (platform or tenant admin, holding no approver slot) may recall a `pending` request. The same file already documented that 387 lines above, where the docblock on `ApprovalRequestRow.viewer.can_override` spells the override's levers as `(approve / reject / reassign / recall it)`. - The ADR-0044 widening read as if it applied to that whole list. It does not: the override and system arms are ANDed with `status === 'pending'` where they are computed, so neither reaches a `returned` request (#12775, maintainer ruling 2026-09-02). Abandoning a revision window is the submitter's alone. Status is now the axis rather than a caveat appended to a general rule, so each status carries its own admitted set. `ApprovalRecallInput.actorId` carried the same stale sentence and is corrected with it. `ApprovalSendBackInput.actorId` and `ApprovalResubmitInput.actorId` share the phrasing but are correct, since `isOverrideActor` is never called from `sendBack` or `resubmit`. Prose only: no key, no accepted value, no runtime behaviour moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4138K1EG7kQ81FNba5Kp4
📓 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 — 129 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 a00d4b251bb599bb536a4fb2ec0c80990b57599b && git checkout a00d4b251bb599bb536a4fb2ec0c80990b57599b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e944fdb246d6baa41133bb78c5a07af1e5279eed f50678c0e848fc874957381fd3fcf70f428ce102 && git checkout -B drift-repro e944fdb246d6baa41133bb78c5a07af1e5279eed && git merge --no-ff f50678c0e848fc874957381fd3fcf70f428ce102
node scripts/docs-audit/affected-docs.mjs --json e944fdb246d6baa41133bb78c5a07af1e5279eed
|
Fixes #14670
Prose only — no key, no accepted value, no runtime behaviour moves.
packages/specand a changeset; nothing inplugin-approvalsis touched.Clause-②: no. Judged from content, not from the path. The path limb fires (
SUSPECT_TIER_GLOBSdeclarespackages/spec/src/**), but that declaration's own docblock calls the path reading "a HINT, never a verdict". This corrects a JSDoc block: nothing about the accept/reject set moves and no public surface widens. Lane precedent: #14586 claimednofor a describe-string correction.Governed surface: no.
node scripts/pm/check-governed-merges.mjs --testover the final file list, exit 0 — "0 of 2 path(s) hit the register".The defect: one file disagreeing with itself
The card frames this as a docstring that "predates" the #3424 admin override. Measured, it is sharper —
packages/spec/src/contracts/approval-service.tsalready documented the override 387 lines above the sentence that denied it, and it names the very verb:origin/maind30ccb9bd):325on a *pending* request (approve / reject / reassign / recall it) despite:338can_override: boolean;:497-:513via_override), naming #3424:712So
:325says an override actor may recall, and:712says only the submitter may. That is the argument for fixing this rather than filing it as a nicety.The second paragraph made it worse, not better. Structured as a general rule plus an ADR-0044 widening, it invites the reader to apply the widening to whoever the general rule admits — so a reader who takes both paragraphs together concludes an admin can recall a
returnedrequest. They cannot.What the source actually says (verified, not inherited)
The override arm.
plugin-approvals/src/approval-service.ts:961,private isOverrideActor(context, requestOrg?). Its own docblock (:944-:952) enumerates the levers: "approve, reject, reassign it to a real approver, or recall it", and:956adds "A system context always passes".The pending-only scope.
recall()'s own gate,:2987:and the viewer flag it is spelled to match,
attachViewersat:5283:Because
isOverrideActorreturnstrueoncontext.isSystem(:963), the system arm enters through the same gate and carries the samependingscope. The card says this ("the override and system arms"); it is worth restating because a rewrite that scopes only the override arm leaves "or a system context" hanging in the general rule and reproduces the defect on the other arm.The published prose was already correct and is the tie-breaker:
content/docs/automation/approvals.mdx:550-:559— an admin "may act on anypendingrequest — approve, reject, reassign it to a real approver, or recall it". This docstring was the one surface that had not kept up.The fix
Status is now the axis, rather than a general rule with corrections appended. Each status carries its own admitted set, so the inheritance misreading is not available on a re-read:
pending— the submitter, a system context, or a Approval routed to an empty position permanently locks the record (no admin override, no recovery) #3424 override actor.returned(ADR-0044) — the submitter alone, and only on the latestreturnedrequest of its (run, node).A second stale docstring, in scope and named here
ApprovalRecallInput.actorId(:545) read/** Must be the request's submitter (or a system context). */— the same false claim, on the same operation, 167 lines above the one the card cites. It is corrected in this PR under the bounded-in-place rule, all four conditions measured:returnedapproval: an ADR-0044 side effect to retire, or a capability to keep? The gate, the prose and the viewer flag disagree three ways #12775's maintainer ruling of 2026-09-02.approval. (The dispatch note's observation about feat(spec)!: duration-shaped number keys carry their unit in the key name — no-baseline gate + seven ADR-0087 renames (timeoutMs, ttlSeconds/ttlMs, *TimeoutSeconds) #15626 holdingcontracts/job-service.tsis consistent: same directory, different file.)Fixing only
:712would have left the contradiction alive on the very input type the corrected method takes.Deliberately NOT changed
ApprovalSendBackInput.actorId(:572) andApprovalResubmitInput.actorId(:601) carry the same "(or a system context)" phrasing and are correct.isOverrideActorhas exactly five call sites in the service —decideNode(:2457),reassign(:3553),recall(:2988),attachViewers(:5284) andvisibleRequestIds(:4865) — and neithersendBacknorresubmitis among them, so no override actor reaches either.Verification
Everything below ran on the final commit,
f50678c0e(git rev-parse --short HEAD), with a clean working tree, so the green is a reading of the tree being merged.pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2— 472 files / 12,686 tests passed; lock wrapperVERDICT command-exit 0.pnpm --filter @objectstack/spec typecheck—VERDICT command-exit 0(tsc --noEmit, scripts project, and the test layer).pnpm --filter '@objectstack/spec...' build—VERDICT command-exit 0; 34/34 declaration files emitted.pnpm lint(whole repo,eslint . --no-inline-config) — exit 0. Run in full rather than narrowed.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(the script derived its own change set; 65 runnable families). All run families exit 0, each verified to have echoed its script name so a zero-match--filtercould not pass silently. Includescheck:api-surface,check:authorable-surface,check:docs,check:llms-txt,check:skill-refs,check:export-origins,check:declaration-map,check:liveness,check:variant-docs,check:strictness-ledger,check:doc-authoring,check:system-context-census,check:cross-package-test-inputs,check:published-files, and the changeset family (check-empty-changeset,check-changeset-no-major,check-adr-0087-registration, all with their self-tests).node scripts/check-nul-bytes.mjs— OK, 7,572 files scanned, no raw control bytes.The docstring ships. It is in the published declarations, not only in source —
packages/spec/dist/contracts/index.d.ts:8307-:8325carries the new text, which is why this PR takes apatchchangeset rather than theskip-changesetlabel.Two results that are NOT MEASURED, and are not being reported as green.
check:doc-formula-expressionsexits 3 — its own PREREQUISITE-NOT-MET code, because@objectstack/formulais not built in this worktree ("Nothing was measured", in the gate's own words).check:skill-examplesexits 1 for the same reason one layer over (packages/client-react/distholds no declarations); it is not in this card's derived gate list, and this diff adds no marked example block. CI builds the whole tree, so both are answered there.One note for whoever runs these gates next
check:skill-examplesleft 233 extracted.tsfiles underpackages/spec/.examples-build*/when it aborted on that prerequisite, andscripts/docs-audit/affected-docs.mjswalks the filesystem without skipping those dirs — so the next gate in the sweep,check-affected-docs, failed its own self-test with a message pointing nowhere near the cause. Removing the residue turns it green again (568 self-test cases pass); reverting this PR's source change does not, which is how it was ruled out as mine. That interaction is already filed as #15446 and was independently reproduced here, so nothing new was filed for it.Out of scope, filed
ApprovalService.recall's docblock opens "(submitter only)" and contradicts itself 12 lines later, where the #3424 override is stated correctly #15643 — the implementation's own docblock (plugin-approvals/src/approval-service.ts:2933) opensWithdraw a pending request (submitter only).and contradicts itself at:2945, where the Approval routed to an empty position permanently locks the record (no admin override, no recovery) #3424 override is stated correctly. Same defect class as this card, one surface over, in thedomain:serviceslane's territory rather than this one's. Unassigned, left for triage; plugin-approvals:ApprovalService.recall's docblock opens "(submitter only)" and contradicts itself 12 lines later, where the #3424 override is stated correctly #15643 is not addressed here.🤖 Generated with Claude Code
https://claude.ai/code/session_01G4138K1EG7kQ81FNba5Kp4
Generated by Claude Code