Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/spec-approval-continue-restored-contract.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@objectstack/spec': minor
---

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.

`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 live request — `pending` for decide and send-back, `returned` for resubmit, and `pending` or the revise window for recall — and the stranding call leaves the row where none of them can issue the continuation it owes. 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.

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.
36 changes: 25 additions & 11 deletions packages/plugins/plugin-approvals/src/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4726,10 +4726,11 @@ export class ApprovalService implements IApprovalService {
* `AutomationEngine.restoreConsumedSuspension` puts a stranded approval run
* back on its pause and tells the operator to *re-issue the continuation* —
* but for an `approval` node the only issuers are this service's doors, and
* every one of them guards on a `pending` request that the stranding call
* itself just made terminal. Re-opening the row is excluded (it would let a
* decided request be decided again), so what is kept instead is the SIGNAL:
* the exact `branchLabel` + `output` the failed resume carried.
* every one of them guards on a live status — `pending`, or `returned` for
* the revise-window doors — which the stranding call left in no state to
* issue the continuation it owes. Re-opening the row is excluded (it would
* let a decided request be decided again), so what is kept instead is the
* SIGNAL: the exact `branchLabel` + `output` the failed resume carried.
*
* ⚠️ Best-effort by construction, and it must stay that way: the decision is
* already durable and its caller is already owed a `RESUME_FAILED` throw. A
Expand Down Expand Up @@ -5052,9 +5053,14 @@ export class ApprovalService implements IApprovalService {
* `true` — and its own reason string tells the operator to *re-issue the
* continuation*. For an `approval` node there was then nobody who could:
*
* - `decide` / `recall` / `sendBack` / `resubmit` all guard on a `pending`
* request, and the row is terminal — written by the very call that
* stranded the run;
* - `decide` / `recall` / `sendBack` / `resubmit` each guard on a LIVE
* request — `pending` for `decide` and `sendBack`, `returned` for
* `resubmit`, and `pending` or the revise window for `recall` — and the
* stranding call left the row where none of them can issue the
* continuation it owes. ⚠️ Not because the row is never `returned`: a
* stranded send-back leaves it exactly there, and `resubmit` is still
* no way back — submitter-only, and it owes the `resubmit` edge where
* the stranded continuation was the `revise` one;
* - the generic `engine.resume` refuses, because the `approval` node
* declares `resumeAuthority: 'service'` and the #3801 gate turns away any
* resume that is not the tail of a decision this service authorized.
Expand All @@ -5068,7 +5074,7 @@ export class ApprovalService implements IApprovalService {
* ## What it deliberately does NOT do
*
* ⛔ It does not re-open, re-decide, or rewrite the request row: all four
* `pending` guards stay exactly as they are, and no status, mirror field or
* status guards stay exactly as they are, and no status, mirror field or
* audit row is written. A person decided this once; this replays what they
* decided onto the pause that was put back, and replays nothing else.
* ⛔ It does not relax `resumeAuthority: 'service'` — the resume goes through
Expand Down Expand Up @@ -5100,9 +5106,17 @@ export class ApprovalService implements IApprovalService {
*
* Deliberately shaped like the engine verb it completes: an in-process
* operator repair, reachable from a host or a console script, with no REST
* route and no entry in the spec `ApprovalService` contract — exactly as
* `restoreConsumedSuspension` is a class method on `AutomationEngine` and
* appears in no contract. It authorizes nothing new: the decision it replays
* route. ⚠️ That last part is where it DIFFERS from
* `restoreConsumedSuspension`, which does have a platform-operator door —
* `POST /:name/runs/:runId/restore-suspension`, the #13953 services half.
* This verb has none: the #15389 ruling of 2026-09-09 refused a REST/CLI
* route for it, so a door for it would be a new card. Both verbs are
* DECLARED on their spec contracts as OPTIONAL members —
* `IAutomationService.restoreConsumedSuspension` by #16495, and this one on
* `IApprovalService` by that same ruling — which declares the
* capability without opening a door: a caller reaching this verb through the
* contract must probe for presence and refuse fail-closed when it is absent.
* It authorizes nothing new: the decision it replays
* was authorized and recorded when it was made, and re-authorizing it here
* against a present-day actor would be a different and wrong question (the
* original approver may be long gone). `requestedBy` / `reason` ride the log
Expand Down
189 changes: 188 additions & 1 deletion packages/spec/src/contracts/approval-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,16 @@
// ?? null` — a resolved org id, or `null` when none resolved, or absent on
// rows written before stamping existed).

import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';

import { describe, it, expect, expectTypeOf } from 'vitest';
import type { ApprovalActionRow, ApprovalRequestRow } from './approval-service';
import type {
ApprovalActionRow,
ApprovalRequestRow,
ApprovalStatus,
IApprovalService,
} from './approval-service';

describe('approval row organization_id declaration (#10331)', () => {
it('is readable off ApprovalRequestRow without a cast, at the stamped shape', () => {
Expand Down Expand Up @@ -54,3 +62,182 @@ describe('approval row organization_id declaration (#10331)', () => {
expect(read({ ...minimal, organization_id: 'o_plant' })).toBe('o_plant');
});
});

// ---------------------------------------------------------------------------
// [#15389] `continueRestoredRun` — the approvals half of the operator repair
// pair, declared on the contract.
//
// The maintainer ruling of 2026-09-09 (decision batch #106, item 3) settled
// option A: the verb is declared on `IApprovalService` as an OPTIONAL member,
// with the shape and docblock discipline #16495 gave
// `IAutomationService.cancelRun` / `restoreConsumedSuspension`. These pins are
// that block's sibling, and they hold the three things the ruling actually
// decided: the member exists, it is optional, and it carries the ruled posture
// in its docblock — including the note that promising a repair verb that will
// refuse is worse than promising nothing.
//
// The type-level identities are exported aliases for the same reason the
// #16495 block's are: an unread alias inside a test body is TS6196, and a pin
// no program compiles is no pin at all. `check:test-typecheck` compiles this
// file.
// ---------------------------------------------------------------------------

type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
type Assert<T extends true> = T;
type ContinueRestoredRun = NonNullable<IApprovalService['continueRestoredRun']>;

/**
* `continueRestoredRun(requestId, options?)` — who asked and why travel
* through the contract, exactly as they do on the engine verb this completes.
*/
export type ContinueTakesRequestIdAndOptions = Assert<
Eq<Parameters<ContinueRestoredRun>, [requestId: string, options?: { requestedBy?: string; reason?: string }]>
>;

/**
* The replay result: what moved, which run, which outcome and edge, and
* whether the signal was the literal one or was rebuilt. A dropped or widened
* member turns this alias red.
*/
export type ContinueAnswersTheReplayResult = Assert<
Eq<
Awaited<ReturnType<ContinueRestoredRun>>,
{
resumed: boolean;
runId: string;
decision: string;
branchLabel?: string;
source: 'journal' | 'reconstructed';
resumeError?: string;
}
>
>;

/** A conforming request row, at the minimum the contract requires. */
const requestRow = (id: string, status: ApprovalStatus): ApprovalRequestRow => ({
id,
process_name: 'flow:expense_review',
object_name: 'expense',
record_id: 'rec_1',
status,
});

/**
* The smallest thing that satisfies `IApprovalService` — every REQUIRED member
* and nothing else. It is the population the optionality pin is about: a
* service with no operator repair verb still conforms.
*/
const minimalService = (): IApprovalService => ({
listRequests: async () => [],
countRequests: async () => 0,
getRequest: async () => null,
decide: async (requestId) => ({ request: requestRow(requestId, 'approved'), finalized: true, decision: 'approve' }),
recall: async (requestId) => ({ request: requestRow(requestId, 'recalled') }),
sendBack: async (requestId) => ({ request: requestRow(requestId, 'returned') }),
resubmit: async (requestId) => ({ request: requestRow(requestId, 'returned') }),
reassign: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
remind: async (requestId) => ({ request: requestRow(requestId, 'pending'), notified: 0 }),
requestInfo: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
comment: async (requestId) => ({ request: requestRow(requestId, 'pending') }),
listActions: async () => [],
});

describe('[#15389] continueRestoredRun — the approvals operator repair verb, declared', () => {
it('is optional: the minimal implementation still conforms and has no operator door', () => {
const service = minimalService();

// The ruled optionality. A door standing in front of THIS service has
// to probe and refuse fail-closed — it may not call and report success.
expect(service.continueRestoredRun).toBeUndefined();
});

it('carries the signature through the contract — who asked, and why, reach the implementation', async () => {
const seen: Array<Record<string, unknown>> = [];
const service: IApprovalService = {
...minimalService(),
continueRestoredRun: async (requestId, options) => {
seen.push({ requestId, ...options });
return requestId === 'req_journalled'
? {
resumed: true,
runId: 'run_stranded',
decision: 'reject',
branchLabel: 'reject',
source: 'journal',
}
: {
resumed: true,
runId: 'run_stranded',
decision: 'reject',
branchLabel: 'reject',
source: 'reconstructed',
resumeError: 'a concurrent resume is already advancing this run',
};
},
};

const exact = await service.continueRestoredRun!('req_journalled', {
requestedBy: 'ops@example.com',
reason: 'notify node fixed; re-issuing the recorded rejection',
});
expect(exact.resumed).toBe(true);
expect(exact.runId).toBe('run_stranded');
// The outcome is REPLAYED, never re-decided: it is the one already on
// the row, and the edge it walks is the one it always walked.
expect(exact.decision).toBe('reject');
expect(exact.branchLabel).toBe('reject');
// `journal` is the literal re-issue; `reconstructed` is the inferred
// one. A caller that cannot tell them apart cannot say what it trusts.
expect(exact.source).toBe('journal');
expect(exact.resumeError).toBeUndefined();

const rebuilt = await service.continueRestoredRun!('req_pre_journal');
expect(rebuilt.source).toBe('reconstructed');
expect(rebuilt.resumeError).toContain('concurrent resume');

// The optional parameters ARE the reason the signature follows the
// implementation: an operator repair records who asked and why.
expect(seen).toEqual([
{
requestId: 'req_journalled',
requestedBy: 'ops@example.com',
reason: 'notify node fixed; re-issuing the recorded rejection',
},
{ requestId: 'req_pre_journal' },
]);
});

it('refuses a replay result that omits `source` (compile-time, under check:test-typecheck)', () => {
const service: IApprovalService = {
...minimalService(),
// @ts-expect-error — `source` is required: a caller told a run moved, but not whether the
// signal was the literal one or a rebuild, cannot tell an exact replay from an inferred one.
continueRestoredRun: async (_requestId) => ({ resumed: true, runId: 'run_stranded', decision: 'reject' }),
};

expect(service.continueRestoredRun).toBeDefined();
});

it('the docblock carries the ruled posture: no re-decision, no door, and the promising-nothing note', () => {
const source = readFileSync(fileURLToPath(new URL('./approval-service.ts', import.meta.url)), 'utf8');
const at = source.indexOf('continueRestoredRun?(');
expect(at).toBeGreaterThan(-1);
// The doc block immediately above the declaration — from its last `/**`.
const doc = source.slice(source.lastIndexOf('/**', at), at);

// The sibling this was ruled to copy, named where a later author reads it.
expect(doc).toContain('#16495');
// What the engine verb leaves undone, which is the whole reason this exists.
expect(doc).toMatch(/the[\s*]+continuation[\s*]+must[\s*]+be[\s*]+re-issued/);
// ⛔ It replays a recorded outcome; it does not re-decide.
expect(doc).toMatch(/does not re-open, re-decide or rewrite the request row/);
expect(doc).toMatch(/does not relax the node's `resumeAuthority: 'service'`/);
// Optional ⇒ absent means no door, and the door refuses fail-closed.
expect(doc).toMatch(/NO[\s*]+operator door/);
expect(doc).toMatch(/refuse[\s*]+fail-closed/);
// The #16495 note, in its own words — the reason optionality is not a shrug.
expect(doc).toMatch(/promising a repair verb that will refuse is worse[\s*]+than promising nothing/);
// C was refused: declaring the member is not declaring a route.
expect(doc).toMatch(/refused a REST\/CLI route/);
});
});
Loading
Loading