Skip to content

Commit ca326b5

Browse files
claude[bot]claude
andauthored
docs(spec): IApprovalService.recall names every actor who may recall, scoped by status (#15645)
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. Claude-Session: https://claude.ai/code/session_01G4138K1EG7kQ81FNba5Kp4 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 58f30d9 commit ca326b5

2 files changed

Lines changed: 44 additions & 9 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`IApprovalService.recall`'s contract prose names every actor who may recall, and scopes each one by status (#14670)
6+
7+
**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.
8+
9+
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:
10+
11+
- **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.
12+
- **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.
13+
14+
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.
15+
16+
`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.
17+
18+
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.
19+
20+
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.

packages/spec/src/contracts/approval-service.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -540,9 +540,14 @@ export interface ApprovalDecisionInput {
540540
outputs?: Record<string, unknown>;
541541
}
542542

543-
/** Input for recalling (withdrawing) a pending request. */
543+
/** Input for recalling (withdrawing) an undecided request. */
544544
export interface ApprovalRecallInput {
545-
/** Must be the request's submitter (or a system context). */
545+
/**
546+
* The actor the recall is recorded against. On a `pending` request: the
547+
* submitter, a system context, or a #3424 override actor. On a `returned`
548+
* request the submitter alone — see {@link IApprovalService.recall} for why
549+
* the two sets differ.
550+
*/
546551
actorId: string;
547552
comment?: string;
548553
}
@@ -709,14 +714,24 @@ export interface IApprovalService {
709714
decide(requestId: string, input: ApprovalDecisionInput, context: ExecutionContext): Promise<ApprovalDecisionResult>;
710715

711716
/**
712-
* Withdraw a pending request. Only the submitter (or a system context) may
713-
* recall. Finalises the request as `recalled` and resumes the owning flow
714-
* run down the `reject` branch with `output.decision = 'recall'`.
717+
* Withdraw a request that has not been decided. Finalises it as `recalled`
718+
* and resumes the owning flow run down the `reject` branch with
719+
* `output.decision = 'recall'`.
720+
*
721+
* Who may recall is scoped BY STATUS, and the two sets are not the same —
722+
* the ADR-0044 widening below is the submitter's alone, never a widening of
723+
* who may act:
715724
*
716-
* ADR-0044: also valid on the LATEST `returned` request of its (run, node)
717-
* — the submitter abandons the revision window instead of resubmitting; the
718-
* request flips `returned → recalled` and the run resumes down `reject` the
719-
* same way.
725+
* - `pending` — the submitter, a system context, or a #3424 override actor
726+
* (the stuck-request recovery path: the same privilege carried by
727+
* {@link ApprovalRequestRow}'s `viewer.can_override`, and scoped to
728+
* `pending` for the same reason).
729+
* - `returned` (ADR-0044) — the submitter ALONE, and only on the LATEST
730+
* `returned` request of its (run, node): they abandon the revision window
731+
* instead of resubmitting, the request flips `returned → recalled`, and
732+
* the run resumes down `reject` the same way. Neither the override nor the
733+
* system arm reaches a `returned` request (#12775); an override actor is
734+
* refused there exactly as any other non-submitter.
720735
*/
721736
recall(requestId: string, input: ApprovalRecallInput, context: ExecutionContext): Promise<ApprovalRecallResult>;
722737

0 commit comments

Comments
 (0)