Skip to content

Commit 5bbf818

Browse files
os-billclaude
andcommitted
feat(spec): declare continueRestoredRun on the IApprovalService contract
The approvals half of the operator repair pair. `restoreConsumedSuspension` re-arms a consumed pause and deliberately does not replay the resume signal; for an approval suspension nothing could re-issue the continuation, because every front door guards on a `pending` request and the row is terminal. The issuer exists as a class member on `plugin-approvals`; declaring it optional on the contract puts the same verb family on the contract rather than half on a contract and half on a class. Shape and docblock discipline follow `IAutomationService.cancelRun` / `restoreConsumedSuspension`: optional, structural result declared inline, and the note that promising a repair verb that will refuse is worse than promising nothing. No REST or CLI route is declared. Also corrects the class docblock's posture paragraph, which claimed this verb has no contract entry and that `restoreConsumedSuspension` appears in no contract — both false as of this change and of #16495 respectively. Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH Co-authored-by: Claude <noreply@anthropic.com>
1 parent ad715ac commit 5bbf818

4 files changed

Lines changed: 297 additions & 4 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
Declare `continueRestoredRun` on the `IApprovalService` contract, so the approvals half of the operator repair pair is reachable through the published interface rather than only off the implementation class.
6+
7+
`IAutomationService.restoreConsumedSuspension` re-arms the pause a failed resume consumed and, by its own contract, does not replay the resume signal — the continuation must be re-issued. For an approval suspension nothing could re-issue it: every front door guards on a `pending` request and the row is terminal, written by the very call that stranded the run. The issuer landed as a class member on `plugin-approvals`; this declares it, so a caller programs against the contract instead of importing the implementation.
8+
9+
Additive and OPTIONAL, the way `cancelRun` / `restoreConsumedSuspension` are declared on `IAutomationService`: an existing implementation still conforms, and a service that does not declare the member has no operator door for it — a caller must probe for presence and refuse fail-closed rather than answer success for a verb it could not dispatch, because promising a repair verb that will refuse is worse than promising nothing. No REST or CLI route is declared or implied.

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5100,9 +5100,13 @@ export class ApprovalService implements IApprovalService {
51005100
*
51015101
* Deliberately shaped like the engine verb it completes: an in-process
51025102
* operator repair, reachable from a host or a console script, with no REST
5103-
* route and no entry in the spec `ApprovalService` contract — exactly as
5104-
* `restoreConsumedSuspension` is a class method on `AutomationEngine` and
5105-
* appears in no contract. It authorizes nothing new: the decision it replays
5103+
* route — exactly as `restoreConsumedSuspension` is reached. Both verbs are
5104+
* DECLARED on their spec contracts as OPTIONAL members —
5105+
* `IAutomationService.restoreConsumedSuspension` by #16495, and this one on
5106+
* `IApprovalService` by the #15389 ruling of 2026-09-09 — which declares the
5107+
* capability without opening a door: a caller reaching this verb through the
5108+
* contract must probe for presence and refuse fail-closed when it is absent.
5109+
* It authorizes nothing new: the decision it replays
51065110
* was authorized and recorded when it was made, and re-authorizing it here
51075111
* against a present-day actor would be a different and wrong question (the
51085112
* original approver may be long gone). `requestedBy` / `reason` ride the log

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

Lines changed: 188 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,16 @@
99
// ?? null` — a resolved org id, or `null` when none resolved, or absent on
1010
// rows written before stamping existed).
1111

12+
import { readFileSync } from 'node:fs';
13+
import { fileURLToPath } from 'node:url';
14+
1215
import { describe, it, expect, expectTypeOf } from 'vitest';
13-
import type { ApprovalActionRow, ApprovalRequestRow } from './approval-service';
16+
import type {
17+
ApprovalActionRow,
18+
ApprovalRequestRow,
19+
ApprovalStatus,
20+
IApprovalService,
21+
} from './approval-service';
1422

1523
describe('approval row organization_id declaration (#10331)', () => {
1624
it('is readable off ApprovalRequestRow without a cast, at the stamped shape', () => {
@@ -54,3 +62,182 @@ describe('approval row organization_id declaration (#10331)', () => {
5462
expect(read({ ...minimal, organization_id: 'o_plant' })).toBe('o_plant');
5563
});
5664
});
65+
66+
// ---------------------------------------------------------------------------
67+
// [#15389] `continueRestoredRun` — the approvals half of the operator repair
68+
// pair, declared on the contract.
69+
//
70+
// The maintainer ruling of 2026-09-09 (decision batch #106, item 3) settled
71+
// option A: the verb is declared on `IApprovalService` as an OPTIONAL member,
72+
// with the shape and docblock discipline #16495 gave
73+
// `IAutomationService.cancelRun` / `restoreConsumedSuspension`. These pins are
74+
// that block's sibling, and they hold the three things the ruling actually
75+
// decided: the member exists, it is optional, and it carries the ruled posture
76+
// in its docblock — including the note that promising a repair verb that will
77+
// refuse is worse than promising nothing.
78+
//
79+
// The type-level identities are exported aliases for the same reason the
80+
// #16495 block's are: an unread alias inside a test body is TS6196, and a pin
81+
// no program compiles is no pin at all. `check:test-typecheck` compiles this
82+
// file.
83+
// ---------------------------------------------------------------------------
84+
85+
type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
86+
type Assert<T extends true> = T;
87+
type ContinueRestoredRun = NonNullable<IApprovalService['continueRestoredRun']>;
88+
89+
/**
90+
* `continueRestoredRun(requestId, options?)` — who asked and why travel
91+
* through the contract, exactly as they do on the engine verb this completes.
92+
*/
93+
export type ContinueTakesRequestIdAndOptions = Assert<
94+
Eq<Parameters<ContinueRestoredRun>, [requestId: string, options?: { requestedBy?: string; reason?: string }]>
95+
>;
96+
97+
/**
98+
* The replay result: what moved, which run, which outcome and edge, and
99+
* whether the signal was the literal one or was rebuilt. A dropped or widened
100+
* member turns this alias red.
101+
*/
102+
export type ContinueAnswersTheReplayResult = Assert<
103+
Eq<
104+
Awaited<ReturnType<ContinueRestoredRun>>,
105+
{
106+
resumed: boolean;
107+
runId: string;
108+
decision: string;
109+
branchLabel?: string;
110+
source: 'journal' | 'reconstructed';
111+
resumeError?: string;
112+
}
113+
>
114+
>;
115+
116+
/** A conforming request row, at the minimum the contract requires. */
117+
const requestRow = (id: string, status: ApprovalStatus): ApprovalRequestRow => ({
118+
id,
119+
process_name: 'flow:expense_review',
120+
object_name: 'expense',
121+
record_id: 'rec_1',
122+
status,
123+
});
124+
125+
/**
126+
* The smallest thing that satisfies `IApprovalService` — every REQUIRED member
127+
* and nothing else. It is the population the optionality pin is about: a
128+
* service with no operator repair verb still conforms.
129+
*/
130+
const minimalService = (): IApprovalService => ({
131+
listRequests: async () => [],
132+
countRequests: async () => 0,
133+
getRequest: async () => null,
134+
decide: async (requestId) => ({ request: requestRow(requestId, 'approved'), finalized: true, decision: 'approve' }),
135+
recall: async (requestId) => ({ request: requestRow(requestId, 'recalled') }),
136+
sendBack: async (requestId) => ({ request: requestRow(requestId, 'returned') }),
137+
resubmit: async (requestId) => ({ request: requestRow(requestId, 'returned') }),
138+
reassign: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
139+
remind: async (requestId) => ({ request: requestRow(requestId, 'pending'), notified: 0 }),
140+
requestInfo: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
141+
comment: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
142+
listActions: async () => [],
143+
});
144+
145+
describe('[#15389] continueRestoredRun — the approvals operator repair verb, declared', () => {
146+
it('is optional: the minimal implementation still conforms and has no operator door', () => {
147+
const service = minimalService();
148+
149+
// The ruled optionality. A door standing in front of THIS service has
150+
// to probe and refuse fail-closed — it may not call and report success.
151+
expect(service.continueRestoredRun).toBeUndefined();
152+
});
153+
154+
it('carries the signature through the contract — who asked, and why, reach the implementation', async () => {
155+
const seen: Array<Record<string, unknown>> = [];
156+
const service: IApprovalService = {
157+
...minimalService(),
158+
continueRestoredRun: async (requestId, options) => {
159+
seen.push({ requestId, ...options });
160+
return requestId === 'req_journalled'
161+
? {
162+
resumed: true,
163+
runId: 'run_stranded',
164+
decision: 'reject',
165+
branchLabel: 'reject',
166+
source: 'journal',
167+
}
168+
: {
169+
resumed: true,
170+
runId: 'run_stranded',
171+
decision: 'reject',
172+
branchLabel: 'reject',
173+
source: 'reconstructed',
174+
resumeError: 'a concurrent resume is already advancing this run',
175+
};
176+
},
177+
};
178+
179+
const exact = await service.continueRestoredRun!('req_journalled', {
180+
requestedBy: 'ops@example.com',
181+
reason: 'notify node fixed; re-issuing the recorded rejection',
182+
});
183+
expect(exact.resumed).toBe(true);
184+
expect(exact.runId).toBe('run_stranded');
185+
// The outcome is REPLAYED, never re-decided: it is the one already on
186+
// the row, and the edge it walks is the one it always walked.
187+
expect(exact.decision).toBe('reject');
188+
expect(exact.branchLabel).toBe('reject');
189+
// `journal` is the literal re-issue; `reconstructed` is the inferred
190+
// one. A caller that cannot tell them apart cannot say what it trusts.
191+
expect(exact.source).toBe('journal');
192+
expect(exact.resumeError).toBeUndefined();
193+
194+
const rebuilt = await service.continueRestoredRun!('req_pre_journal');
195+
expect(rebuilt.source).toBe('reconstructed');
196+
expect(rebuilt.resumeError).toContain('concurrent resume');
197+
198+
// The optional parameters ARE the reason the signature follows the
199+
// implementation: an operator repair records who asked and why.
200+
expect(seen).toEqual([
201+
{
202+
requestId: 'req_journalled',
203+
requestedBy: 'ops@example.com',
204+
reason: 'notify node fixed; re-issuing the recorded rejection',
205+
},
206+
{ requestId: 'req_pre_journal' },
207+
]);
208+
});
209+
210+
it('refuses a replay result that omits `source` (compile-time, under check:test-typecheck)', () => {
211+
const service: IApprovalService = {
212+
...minimalService(),
213+
// @ts-expect-error — `source` is required: a caller told a run moved, but not whether the
214+
// signal was the literal one or a rebuild, cannot tell an exact replay from an inferred one.
215+
continueRestoredRun: async (_requestId) => ({ resumed: true, runId: 'run_stranded', decision: 'reject' }),
216+
};
217+
218+
expect(service.continueRestoredRun).toBeDefined();
219+
});
220+
221+
it('the docblock carries the ruled posture: no re-decision, no door, and the promising-nothing note', () => {
222+
const source = readFileSync(fileURLToPath(new URL('./approval-service.ts', import.meta.url)), 'utf8');
223+
const at = source.indexOf('continueRestoredRun?(');
224+
expect(at).toBeGreaterThan(-1);
225+
// The doc block immediately above the declaration — from its last `/**`.
226+
const doc = source.slice(source.lastIndexOf('/**', at), at);
227+
228+
// The sibling this was ruled to copy, named where a later author reads it.
229+
expect(doc).toContain('#16495');
230+
// What the engine verb leaves undone, which is the whole reason this exists.
231+
expect(doc).toMatch(/the continuation must be[\s*]+re-issued/);
232+
// ⛔ It replays a recorded outcome; it does not re-decide.
233+
expect(doc).toMatch(/does not re-open, re-decide or rewrite the request row/);
234+
expect(doc).toMatch(/does not relax the node's `resumeAuthority: 'service'`/);
235+
// Optional ⇒ absent means no door, and the door refuses fail-closed.
236+
expect(doc).toMatch(/NO[\s*]+operator door/);
237+
expect(doc).toMatch(/refuse[\s*]+fail-closed/);
238+
// The #16495 note, in its own words — the reason optionality is not a shrug.
239+
expect(doc).toMatch(/promising a repair verb that will refuse is worse[\s*]+than promising nothing/);
240+
// C was refused: declaring the member is not declaring a route.
241+
expect(doc).toMatch(/refused a REST\/CLI route/);
242+
});
243+
});

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,4 +944,97 @@ export interface IApprovalService {
944944

945945
/** Audit trail for a request. */
946946
listActions(requestId: string, context: ExecutionContext): Promise<ApprovalActionRow[]>;
947+
948+
/**
949+
* **Operator verb — re-issue the continuation for a run an operator has
950+
* re-armed** (#15389; the maintainer ruling of 2026-09-09, decision batch
951+
* #106 item 3, declared here exactly as #16495 declared
952+
* `IAutomationService.cancelRun` / `restoreConsumedSuspension`).
953+
*
954+
* The missing half of `IAutomationService.restoreConsumedSuspension`, for
955+
* approvals. That verb re-arms the pause a failed resume consumed and, by
956+
* its own contract, deliberately does NOT replay the resume signal — the
957+
* continuation must be re-issued. For an `approval` suspension there was
958+
* then nobody who could: {@link decide}, {@link recall}, {@link sendBack}
959+
* and {@link resubmit} all guard on a `pending` request, and the row is
960+
* terminal — written by the very call that stranded the run — while a
961+
* generic engine resume is refused at a node declaring
962+
* `resumeAuthority: 'service'`. The only verb left was cancel, which
963+
* discards the branch's downstream work. This is the issuer that exits that
964+
* dead end.
965+
*
966+
* What it does, exactly, and what it does not:
967+
* - it replays the outcome the request ALREADY recorded onto the pause that
968+
* was put back, and replays nothing else;
969+
* - ⛔ it does not re-open, re-decide or rewrite the request row — no
970+
* status, no mirror field and no audit row is written, and a decided
971+
* request still cannot be decided again through the front door;
972+
* - ⛔ it does not relax the node's `resumeAuthority: 'service'`: the resume
973+
* is the implementation's own, through the same single call site that
974+
* stamps that marker;
975+
* - it never answers a silent `false` — a resume that fails again throws
976+
* the same `RESUME_FAILED` envelope the original decision did,
977+
* `repairable` and all, so a second restore-and-continue is possible.
978+
*
979+
* **Why it takes no {@link ExecutionContext}** — deliberately shaped like
980+
* the engine verb it completes. The decision it replays was authorized and
981+
* recorded when it was made; re-authorizing it here against a present-day
982+
* actor would be a different and wrong question, because the original
983+
* approver may be long gone. `requestedBy` / `reason` ride the
984+
* implementation's log for the reason they do on the restore: an operator
985+
* repair records who asked, and why.
986+
*
987+
* **No door is declared here, and none is ruled** — the 2026-09-09 ruling
988+
* refused a REST/CLI route for this verb. It is an in-process operator
989+
* repair, reachable from a host or a console script; a route for it is a new
990+
* card, never a widening at a call site.
991+
*
992+
* **Optional, deliberately** — for the reason `cancelRun` and
993+
* `restoreConsumedSuspension` are. Re-issuing a consumed continuation is a
994+
* capability of an approvals implementation that can identify the pause it
995+
* was refused on; a service that does not declare this member has NO
996+
* operator door for it, and a door MUST probe for presence and refuse
997+
* fail-closed when it is absent — never answer success for a verb it could
998+
* not dispatch, because promising a repair verb that will refuse is worse
999+
* than promising nothing.
1000+
*
1001+
* @param requestId - The terminal request whose recorded outcome is to be
1002+
* re-issued; the run is the one that request has always named
1003+
* @param options.requestedBy - Who asked; logged
1004+
* @param options.reason - Why; logged the same way
1005+
* @returns what was replayed, and whether the run moved
1006+
*/
1007+
continueRestoredRun?(
1008+
requestId: string,
1009+
options?: { requestedBy?: string; reason?: string },
1010+
): Promise<{
1011+
/** True when the restored pause was consumed and the flow moved on. */
1012+
resumed: boolean;
1013+
/** The run this continued — the one the request has always named. */
1014+
runId: string;
1015+
/**
1016+
* The outcome that was replayed, exactly as it was first recorded.
1017+
* Free-form by design, as it is on `StrandedDecisionDetails`: the
1018+
* vocabulary belongs to the producing service, not to this contract.
1019+
*/
1020+
decision: string;
1021+
/** The edge it walked, when the replayed signal names one. */
1022+
branchLabel?: string;
1023+
/**
1024+
* Whether the replayed signal was the literal one the failing door sent
1025+
* (`journal`), or was rebuilt from the row's recorded outcome
1026+
* (`reconstructed`) — so a caller can tell an EXACT replay from an
1027+
* inferred one before it trusts what moved.
1028+
*
1029+
* ⚠️ Enumerated here, unlike `restoreConsumedSuspension`'s `refusal`
1030+
* (#16495 route (i)), and the difference is the point: that one is the
1031+
* implementation's own open refusal vocabulary, which this contract would
1032+
* have to keep in step with; this is a closed, binary property of the
1033+
* replay itself and the caller's branch point. A third provenance is a
1034+
* spec card, never a widening at a call site.
1035+
*/
1036+
source: 'journal' | 'reconstructed';
1037+
/** Set only on the tolerated non-failure: a concurrent resume already had it. */
1038+
resumeError?: string;
1039+
}>;
9471040
}

0 commit comments

Comments
 (0)