diff --git a/.changeset/approval-recall-docstring-override-scope.md b/.changeset/approval-recall-docstring-override-scope.md new file mode 100644 index 0000000000..82dfc4d475 --- /dev/null +++ b/.changeset/approval-recall-docstring-override-scope.md @@ -0,0 +1,20 @@ +--- +"@objectstack/spec": patch +--- + +`IApprovalService.recall`'s contract prose names every actor who may recall, and scopes each one by status (#14670) + +**Documentation only — no key, no accepted value, no runtime behaviour moves.** The implementation has been correct since #12775; only the contract's description of it was stale. + +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, in opposite directions: + +- **The list was not exhaustive.** A #3424 override actor — a platform or tenant admin holding no approver slot — may recall a `pending` request. That is the in-product recovery path for an approval routed to an unstaffed position, and this same file already documented it 387 lines above the sentence denying it: the docblock on `ApprovalRequestRow.viewer.can_override` spells the override's levers as `(approve / reject / reassign / recall it)`. One file, two contradicting sentences about the same verb. +- **The ADR-0044 widening read as though 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; an override actor is refused there exactly as any other non-submitter (#12775, maintainer ruling 2026-09-02). Abandoning a revision window is the submitter's alone. + +The rewrite makes **status** the axis instead of appending a caveat, so the second defect cannot come back on a re-read: each status carries its own admitted set, and the `returned` bullet says outright that the submitter is alone in it. + +`ApprovalRecallInput.actorId` carried the same stale sentence (*"Must be the request's submitter (or a system context)"*) and is corrected with it. Fixing only the method docstring would have left the contradiction alive on the very input type the corrected method takes. + +The two sibling docstrings sharing that phrasing are **correct and unchanged**: `ApprovalSendBackInput.actorId` and `ApprovalResubmitInput.actorId`. `isOverrideActor` is called from exactly five places in `plugin-approvals` — `decideNode`, `reassign`, `recall`, `attachViewers` and `visibleRequestIds` — and neither `sendBack` nor `resubmit` is among them, so no override actor reaches either. + +The published prose already described the corrected rule (`content/docs/automation/approvals.mdx`: an admin "may act on any `pending` request — approve, reject, reassign it to a real approver, or recall it"). This docstring was the one surface that had not kept up. diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index ca49305cd2..f6019f3f6b 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -540,9 +540,14 @@ export interface ApprovalDecisionInput { outputs?: Record; } -/** Input for recalling (withdrawing) a pending request. */ +/** Input for recalling (withdrawing) an undecided request. */ export interface ApprovalRecallInput { - /** Must be the request's submitter (or a system context). */ + /** + * The actor the recall is recorded against. On a `pending` request: the + * submitter, a system context, or a #3424 override actor. On a `returned` + * request the submitter alone — see {@link IApprovalService.recall} for why + * the two sets differ. + */ actorId: string; comment?: string; } @@ -709,14 +714,24 @@ export interface IApprovalService { decide(requestId: string, input: ApprovalDecisionInput, context: ExecutionContext): Promise; /** - * Withdraw a pending request. Only the submitter (or a system context) may - * recall. Finalises the request as `recalled` and resumes the owning flow - * run down the `reject` branch with `output.decision = 'recall'`. + * Withdraw a request that has not been decided. Finalises it as `recalled` + * and resumes the owning flow run down the `reject` branch with + * `output.decision = 'recall'`. + * + * Who may recall is scoped BY STATUS, and the two sets are not the same — + * the ADR-0044 widening below is the submitter's alone, never a widening of + * who may act: * - * ADR-0044: also valid on the LATEST `returned` request of its (run, node) - * — the submitter abandons the revision window instead of resubmitting; the - * request flips `returned → recalled` and the run resumes down `reject` the - * same way. + * - `pending` — the submitter, a system context, or a #3424 override actor + * (the stuck-request recovery path: the same privilege carried by + * {@link ApprovalRequestRow}'s `viewer.can_override`, and scoped to + * `pending` for the same reason). + * - `returned` (ADR-0044) — the submitter ALONE, and only on the LATEST + * `returned` request of its (run, node): they abandon the revision window + * instead of resubmitting, the request flips `returned → recalled`, and + * the run resumes down `reject` the same way. Neither the override nor the + * system arm reaches a `returned` request (#12775); an override actor is + * refused there exactly as any other non-submitter. */ recall(requestId: string, input: ApprovalRecallInput, context: ExecutionContext): Promise;