fix(automation,approvals): gate the generic run-resume route on the suspended node (#3801)

.changeset/automation-resume-authority-gate.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/spec": minor
+"@objectstack/service-automation": minor
+"@objectstack/plugin-approvals": minor
+"@objectstack/runtime": minor
+"@objectstack/client": patch
+---
+
+fix(automation,approvals): the run-resume route is gated by the node the run is parked on (#3801)
+
+`POST /api/v1/automation/:name/runs/:runId/resume` forwarded a caller-supplied
+`{ inputs, output, branchLabel }` straight into `AutomationEngine.resume`, and
+`resumeInternal` validated **machine state only** — the concurrent-resume latch,
+the run exists, the flow exists, the suspended node still exists. Nothing asked
+*who was calling*.
+
+Approval nodes suspend and resume through exactly that mechanism. So a resume
+carrying `branchLabel: 'approve'` walked the approve edge with **no approver
+check, no `sys_approval_action` row and no status mirror** — the
+`sys_approval_request` row and the run then disagreed permanently. The only
+thing standing between the route and the approvals rules was convention; the
+showcase spelled it out in a comment ("decide via the approvals API, never a raw
+engine `resume`"), and a comment in an example is not an access control.
+
+Removing the route was not the fix: it is load-bearing for **screen flows** —
+the UI flow-runner posts `{ inputs }` there to advance a paused `screen` node.
+The gate therefore keys on **what the run is parked on**:
+
+- `ActionDescriptor.resumeAuthority` (`'any'` | `'service'`, default `'any'`) —
+  a pausing node declares who may continue it. `approval` declares `'service'`.
+- The engine refuses a `'service'` suspension unless the signal carries
+  `RESUME_AUTHORITY_SERVICE` (`@objectstack/spec/contracts`), a **symbol** the
+  owning service stamps in-process — a JSON body can never produce one, so the
+  transport cannot forge it. `ApprovalService` stamps it on the tail of a
+  decision it has already authorized and recorded.
+- The gate follows a **subflow** pause down to the child the signal would
+  actually reach, so resuming the parent is not a way around it.
+- Refusal returns `{ success: false, code: 'forbidden' }` and the route answers
+  **403**. Nothing is consumed — the request stays pending and the run stays
+  parked, so the real decision still lands.
+
+`screen` and `wait` pauses are unchanged, as is every path that already went
+through the approvals API. What changes for consumers:
+
+- **FROM:** finishing an approval with
+  `client.automation.resume(flow, runId, { branchLabel: 'approve' })`
+  **TO:** `client.approvals.approve(requestId, …)` (or `.reject` / `.recall`).
+  The old call now answers 403 and changes nothing.
+- Registering your own pausing node whose continuation belongs to a service
+  rather than to whoever holds the run id? Declare `resumeAuthority: 'service'`
+  on its descriptor and stamp `RESUME_AUTHORITY_SERVICE` on the signal from that
+  service.
+
+A suspension now records the node type that produced it
+(`SuspendedRun.nodeType` / `sys_automation_run.node_type`), captured at suspend
+time so a flow republished mid-pause cannot re-type the node out from under the
+gate; rows written before this fall back to the flow definition.

content/docs/automation/approvals.mdx

@@ -288,7 +288,13 @@ curl -b cookies.txt -X POST \
 `actorId` defaults to the caller. The actor **must** be in `pending_approvers`
 or the call returns 403 (`FORBIDDEN: actor '…' is not a pending approver`); a
 request that isn't pending returns 409 (`INVALID_STATE`). Always go through
-these endpoints — never resume the flow run directly.
+these endpoints — never resume the flow run directly, and since #3801 you
+**cannot**: `POST /api/v1/automation/{flow}/runs/{runId}/resume` answers 403 for
+a run parked on an `approval` node (including via a `subflow` pause) and changes
+nothing — the request stays pending and the run stays parked, so the real
+decision still lands. The approver slate, the `sys_approval_action` row and the
+status mirror all live on this path; it is the only one that produces a
+consistent outcome.
 
 A decision may carry **file attachments** — `attachments: string[]` of `sys_file`
 ids — recorded on its audit row (e.g. a signed contract on the approve):

content/docs/automation/flows.mdx

@@ -389,11 +389,34 @@ POST /api/v1/automation/{flow}/runs/{runId}/resume
 
 | Pausing node | Suspends until… | Resumed by |
 | :--- | :--- | :--- |
-| `approval` | a human decision | the approvals service (`POST /api/v1/approvals/requests/:id/approve\|reject`) — resumes down the matching `approve` / `reject` edge. **Always decide through the approvals API**, never by calling `resume` directly: a direct resume strands the pending `sys_approval_request`, so the record stays locked and later approvals on the same record hit `DUPLICATE_REQUEST`. |
+| `approval` | a human decision | the approvals service (`POST /api/v1/approvals/requests/:id/approve\|reject`) — resumes down the matching `approve` / `reject` edge. **Decide through the approvals API**; the resume route above **refuses** an approval pause outright (see below). |
 | `screen` | a user submits the form | the UI runner posting the collected `inputs`; a `paused` response carrying the next `screen` chains multi-step wizards under one stable `runId` |
 | `wait` (timer) | an ISO-8601 duration elapses | **automatically** — a one-shot job resumes the run; after a cold boot the engine re-arms pending timers from the durable store (overdue timers resume immediately) |
 | `wait` (signal) | a named external event | any caller invoking `resume(runId)` |
 
+### Who may resume — the gate is the suspended node
+
+The resume route is generic, so **the node the run is parked on** decides
+whether a raw resume is a legitimate continuation. Every action descriptor
+carries a `resumeAuthority`:
+
+- **`'any'`** (the default — `screen`, `wait`, your own pausing nodes): the
+  caller supplies the continuation and the route is the intended door.
+- **`'service'`** (`approval`): continuing is a *side effect* of a decision
+  that some service must authorize and record first, so only that service may
+  drive it. `ApprovalService.decide` enforces the approver slate, writes the
+  `sys_approval_action` row, mirrors the status field — **then** resumes.
+
+A resume of a `'service'` pause through the route answers **403** and changes
+nothing: the request stays pending and the run stays parked, so the real
+decision can still land. (Before this gate a raw resume walked the `approve`
+edge with no decision recorded, leaving the `sys_approval_request` row and the
+run permanently disagreeing.) The gate follows a **subflow** pause down to the
+child the signal would actually reach, so resuming the parent is no way around
+it. Registering a pausing node of your own? Declare
+`resumeAuthority: 'service'` on its descriptor when the decision to continue
+belongs to your service rather than to whoever holds the run id.
+
 ### Parallel approvals — one aggregating node, not two pauses
 
 "Finance **and** legal must both sign off, concurrently" is **one `approval`
@@ -469,7 +492,8 @@ the chain resolves from either end:
   `${nodeId}.output` / `outputVariable` mapping as a synchronous subflow;
 - resuming the **parent** run id (what a UI holds from the original launch)
   **delegates** down to the suspended child — multi-screen child wizards keep
-  the parent run id stable.
+  the parent run id stable. The resume gate delegates with it: if the child is
+  parked on an `approval`, resuming the parent is refused too.
 
 A child that fails terminally after the pause fails every waiting ancestor, so
 no run is stranded as resumable-forever. See the worked example pair in the

content/docs/references/automation/node-executor.mdx

@@ -76,6 +76,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018)
 | **supportsRetry** | `boolean` | ✅ | Supports retry on failure |
 | **needsOutbox** | `boolean` | ✅ | Dispatch via service-messaging outbox (retry/idempotency/dead-letter) |
 | **isAsync** | `boolean` | ✅ | Suspends the flow awaiting an external reply |
+| **resumeAuthority** | `Enum<'any' \| 'service'>` | ✅ | Who may resume a run this node suspended: 'any' (the generic resume route) or 'service' (only the owning service, e.g. approvals) |
 | **maturity** | `Enum<'ga' \| 'beta' \| 'reserved'>` | ✅ | Runtime maturity: ga (shipped), beta, or reserved (contract only — designers grey this out) |
 | **source** | `Enum<'builtin' \| 'plugin'>` | ✅ | builtin = platform baseline; plugin = third-party contributed |
 | **deprecated** | `boolean` | ✅ | Deprecated alias kept for back-compat |

docs/adr/0019-approval-as-flow-node.md

@@ -195,3 +195,49 @@ not catch a *post-pause* child failure (the parent run fails terminally instead)
 does not count across a suspension; a crash exactly between child completion and the parent
 bubble leaves the parent paused — an operator can compensate with a manual
 `resume(parentRunId, { output })` (outbox-grade exactly-once chaining is future work).
+
+## Addendum (2026-07-28, #3801) — the resume seam is a trust boundary: `resumeAuthority`
+
+Collapsing approval onto the flow engine made `AutomationEngine.resume` the **one** way any pause
+continues — including an approval decision. That is the right execution model, but it left the
+authorization model behind: `POST /api/v1/automation/:name/runs/:runId/resume` forwards a
+caller-supplied `{ inputs, output, branchLabel }` straight through, and `resumeInternal` validated
+**machine state only** (the concurrent-resume latch, the run exists, the flow exists, the suspended
+node still exists). Nothing asked *who*. A raw resume with `branchLabel: 'approve'` therefore walked
+the approve edge with **no approver check, no `sys_approval_action` row and no status mirror** —
+leaving the `sys_approval_request` row and the run permanently disagreeing. The only thing standing
+between the route and the approvals rules was convention, spelled out in a showcase comment. A
+comment in an example is not an access control.
+
+Deleting the route was never an option: it is load-bearing for **screen flows** — the UI flow-runner
+posts `{ inputs }` to advance a paused `screen` node, which is its whole reason to exist. So the gate
+discriminates by **what the run is parked on**, not by the route:
+
+- **`ActionDescriptor.resumeAuthority`** (ADR-0018 registry, `'any'` | `'service'`, default `'any'`).
+  A pausing node declares who may continue it. `approval` declares `'service'`; `screen` / `wait`
+  keep the default and behave exactly as before.
+- **The suspension carries its own node type.** `SuspendedRun.nodeType` (column
+  `sys_automation_run.node_type`) is captured at suspend time rather than re-derived from the live
+  flow, so a flow republished mid-pause cannot re-type the node out from under the gate. Rows
+  written before this shipped fall back to the flow definition.
+- **The marker is a symbol.** `RESUME_AUTHORITY_SERVICE` (`@objectstack/spec/contracts`) is stamped
+  on the `ResumeSignal` by the owning service — `ApprovalService` does it in one place
+  (`serviceResume`), on the tail of a decision it already authorized and recorded. The transport
+  builds its signal out of a JSON body, and no JSON body can produce a symbol-keyed property, so the
+  route cannot forge it; the gate does not need to trust the caller's claims about itself.
+- **The gate follows the subflow chain.** A parent parked on a `subflow` node *delegates* the signal
+  to its suspended child (see the 2026-06-10 addendum), so the gate resolves the effective suspension
+  first and judges the node the signal actually lands on. Resuming the parent is not a way around it.
+- **Refusal is an authorization answer.** `resume` returns `{ success: false, code: 'forbidden' }`
+  and the route answers **403** — not a 200 carrying `success: false`, which reads as "your resume
+  ran and the flow failed". Nothing is consumed: the request stays pending and the run stays parked,
+  so the real decision still lands.
+
+Engine-internal continuations (subflow delegation and up-bubble, `map` re-entry, the wait-timer wake)
+go through the private `resumeInternal` and are **not** re-gated — they continue work an
+already-authorized call started.
+
+**Known gap, tracked separately:** ADR-0044's revise window parks the run on an ordinary `wait` node
+(signal flavor) placed by the flow author, which the type-keyed gate cannot distinguish from any
+other wait. A raw resume there still forces a resubmit without a `sys_approval_action` row. Filed as
+a follow-up — closing it needs a per-suspension owner claim rather than a node-type one.

examples/app-showcase/src/automation/flows/index.ts

@@ -987,8 +987,10 @@ export const ResilientSyncFlow = defineFlow({
  * rejection resumes down `reject`. One node, one suspend/resume, no token tree —
  * the multi-instance pattern Camunda and Step Functions use for exactly this.
  *
- * Decide via the approvals API (never a raw engine `resume`):
- *   POST /api/v1/automation/showcase_invoice_signoff/runs/{runId}/...  ← no
+ * Decide via the approvals API — a raw engine `resume` is refused, not merely
+ * discouraged: the `approval` node declares `resumeAuthority: 'service'`, so
+ * the generic run-resume route answers 403 for a run parked on one (#3801).
+ *   POST /api/v1/automation/showcase_invoice_signoff/runs/{runId}/resume  ← 403
  *   POST /api/v1/approvals/requests/{id}/approve  { actorId: 'position:finance' }
  *   POST /api/v1/approvals/requests/{id}/approve  { actorId: 'position:legal' }   ← now it continues
  */

packages/client/src/index.ts

@@ -2720,7 +2720,7 @@ export class ObjectStackClient {
           return this.unwrapResponse(res) as Promise<T>;
       },
       /**
-       * Resume a run suspended at a `screen` (or `approval`) node — the
+       * Resume a run suspended at a `screen` (or `wait`) node — the
        * screen-flow runtime's second half (ADR-0019 durable pause).
        *
        * `execute()` returns `{ status: 'paused', runId, screen }` when a flow
@@ -2729,6 +2729,13 @@ export class ObjectStackClient {
        * NEXT `{ status: 'paused', screen }` of a multi-step wizard or the
        * terminal `AutomationResult`. Without this method a paused run can only
        * be finished by hand-rolling the HTTP call (#3528).
+       *
+       * **Not the door for an approval (#3801).** A run parked on an
+       * `approval` node — directly, or as the child of a `subflow` pause — is
+       * resumable only through the approvals API
+       * ({@link ObjectStackClient.approvals}: `approve` / `reject` / `recall`),
+       * which authorizes the decision and records it first. This call answers
+       * **403** for one and changes nothing.
        */
       resume: async <T = any>(
           flowName: string,
@@ -4423,9 +4430,10 @@ export class ScopedProjectClient {
       return this.parent._unwrap<T>(res);
     },
     /**
-     * Resume a run suspended at a `screen` / `approval` node with the collected
+     * Resume a run suspended at a `screen` / `wait` node with the collected
      * input (ADR-0019 durable pause). Mirrors the unscoped
-     * `client.automation.resume`.
+     * `client.automation.resume` — including its refusal (403) to resume an
+     * `approval` pause, which belongs to the approvals API (#3801).
      */
     resume: async <T = any>(
       flowName: string,

packages/plugins/plugin-approvals/src/approval-node.test.ts

@@ -154,6 +154,46 @@ describe('Approval node bridge (ADR-0019)', () => {
     expect(paused.runId).toBeDefined();
   });
 
+  // ── resume authorization gate (#3801) ───────────────────────────────
+  //
+  // `POST /automation/:name/runs/:runId/resume` reaches
+  // `AutomationEngine.resume` with a caller-supplied signal. Before the gate,
+  // the only thing between that route and the approvals rules was convention —
+  // a comment in the showcase. These pin the enforcement.
+
+  it('declares the approval node resumable only by its owning service', () => {
+    const approval = automation.getActionDescriptors().find(d => d.type === 'approval');
+    expect(approval!.resumeAuthority).toBe('service');
+  });
+
+  it('refuses a raw engine resume of an approval pause, leaving the request untouched', async () => {
+    registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
+    const paused = await automation.execute('deal_approval', {
+      object: 'crm_deal', record: { id: 'd1' }, userId: 'submitter',
+    });
+    const request = (await fake.find('sys_approval_request', { where: { status: 'pending' } }))[0];
+
+    // Exactly the signal the resume route builds from `{ branchLabel }`.
+    const refused = await automation.resume(paused.runId!, {
+      branchLabel: 'approve', output: { decision: 'approve' },
+    });
+
+    expect(refused).toMatchObject({ success: false, code: 'forbidden' });
+    // The approve branch did NOT run…
+    expect(marks).toHaveLength(0);
+    // …the request is still pending, with no decision recorded…
+    const stillPending = (await fake.find('sys_approval_request', { where: { id: request.id } }))[0];
+    expect(stillPending.status).toBe('pending');
+    const actions = await fake.find('sys_approval_action', { where: { request_id: request.id } });
+    expect(actions.map((a: any) => a.action)).toEqual(['submit']);
+    // …and the run is still parked, so the real decision can still land.
+    expect(automation.listSuspendedRuns()).toHaveLength(1);
+
+    const out = await service.decide(request.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX);
+    expect(out).toMatchObject({ finalized: true, resumed: true });
+    expect(marks).toEqual(['on_approved']);
+  });
+
   it('resumes down the reject branch on rejection', async () => {
     registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
     await automation.execute('deal_approval', { object: 'crm_deal', record: { id: 'd1' } });

packages/plugins/plugin-approvals/src/approval-node.ts

@@ -104,6 +104,15 @@ export function registerApprovalNode(
       // Human decision: the run suspends here awaiting an external reply.
       supportsPause: true,
       isAsync: true,
+      // #3801: this pause is NOT resumable through the generic run-resume
+      // route. Continuing an approval is a side effect of a DECISION, and the
+      // decision is the thing that must be authorized (the approver slate),
+      // recorded (`sys_approval_action`) and mirrored (the status field) —
+      // all of which lives in `ApprovalService.decide`. The engine now refuses
+      // any resume of an approval suspension that does not carry the service's
+      // in-process marker, so "decide via the approvals API, never a raw engine
+      // resume" is enforced rather than merely documented.
+      resumeAuthority: 'service',
       // Publish the node's config contract (ADR-0018 §configSchema) so the
       // Studio flow designer renders the Approval property form from the engine
       // rather than a hardcoded client form — the engine owns the shape.