fix(approvals): the ADR-0044 revise window is a service-owned node type (#3823)

.changeset/approval-revise-window-service-owned.md

@@ -0,0 +1,60 @@
+---
+"@objectstack/spec": minor
+"@objectstack/plugin-approvals": minor
+"@objectstack/lint": minor
+---
+
+fix(approvals): the ADR-0044 revise window is a service-owned node type, not a bare `wait` (#3823)
+
+#3801 gated `POST /api/v1/automation/:name/runs/:runId/resume` on the **node type**
+that produced the suspension: an `approval` pause declares
+`resumeAuthority: 'service'`, so it continues only through `ApprovalService`.
+ADR-0044's **revise window** was the same trust boundary in a shape that key
+could not see. Send-back parked the run on an ordinary `wait` node the flow
+author placed — correctly `resumeAuthority: 'any'`, because a signal wait is
+*meant* to be resumable by an external producer — and `ApprovalService.resubmit`
+was the only thing that checked anything about continuing it.
+
+Demonstrated (not reasoned) against the real engine: a raw `resume(runId)` with
+an **empty body**, from any caller, walked the `resubmit` back-edge into the
+approval node and opened round N+1 with **no submitter check and no `resubmit`
+audit row** (`['submit','revise']` — no third row, ever). Worse, when another
+request was already pending on the record — the exact case `resubmit` refuses
+with `DUPLICATE_REQUEST` *specifically to keep the run alive* — the raw resume
+went around that guard: the approval node's re-entry failed **after** the engine
+consumed the suspension, and the run was **permanently destroyed** with its
+round-N request stuck `returned` and no resubmit able to reach it.
+
+The revise pause is therefore its own node type:
+
+- **`approval_revise`** (`APPROVAL_REVISE_NODE_TYPE`), registered by
+  `@objectstack/plugin-approvals` alongside the `approval` node, declaring
+  `resumeAuthority: 'service'`. It stays a first-class box on the canvas, in the
+  run log and in the suspended-run store — only the *reuse* of `wait` was wrong.
+  It takes **no config**: the window ends on the submitter's explicit resubmit,
+  never on a signal or timer. The `resumeAuthority` gate itself is unchanged.
+- `sendBack` refuses a `revise` edge whose target is not an `approval_revise`
+  node, **before any mutation** (like the existing missing-`revise`-edge check),
+  so no run can be parked in a window something else can advance.
+- New gating lint `flow-approval-revise-target-not-service-owned`
+  (severity `error`, on `os build` / `os validate` / `os lint` and the runtime
+  metadata publish gate) rejects the old shape at authoring time.
+
+**Upgrading a flow authored against the original ADR-0044 D3.** One token:
+
+- **FROM:** `{ id: 'wait_revision', type: 'wait', waitEventConfig: { eventType: 'signal', … } }`
+- **TO:** `{ id: 'wait_revision', type: 'approval_revise' }` — drop
+  `waitEventConfig` / any `config`; the window has no event to wait on.
+
+Until you do, such a flow keeps registering and running and its approvals stay
+decidable (`approve` / `reject` / `recall` / `reassign` are untouched), but
+**send-back is refused** with a message naming the node and this fix, and
+re-publishing it reports the lint error. A run *already parked* in a legacy
+revise window keeps its recorded node type (a republish never re-types a live
+pause) and is drained by `resubmit` or `recall` as usual.
+
+ADR-0044's 2026-07-28 amendment records the reversal of its D3 and of its
+`Alternatives` rejection of a service-owned revise pause, with the evidence
+above; the implementation section there records what shipped, why the approval
+node does not re-suspend itself instead, and why no ADR-0087 conversion was
+added for the old shape.

content/docs/automation/approvals.mdx

@@ -422,11 +422,23 @@ continuity levers on the same request. All are `POST
 
 **Send-back → resubmit (ADR-0044).** An approver who wants changes rather than a
 hard reject calls `revise`: the round finalizes `returned`, the record unlocks,
-and the run parks at a wait point. The submitter reworks the record and
-`resubmit`s (a fresh round opens for all approvers) or `recall`s (abandons it).
-Past the node's `maxRevisions` budget (default 3) a send-back **auto-rejects**
-instead. The flow's approval node must declare a `revise` edge for send-back to
-be available.
+and the run parks in the **revise window** — an `approval_revise` node on the
+flow's `revise` edge. The submitter reworks the record and `resubmit`s (a fresh
+round opens for all approvers) or `recall`s (abandons it). Past the node's
+`maxRevisions` budget (default 3) a send-back **auto-rejects** instead. The
+flow's approval node must declare a `revise` edge, and that edge must target an
+`approval_revise` node, for send-back to be available.
+
+The window is deliberately **not** an ordinary `wait`: `resubmit` is what
+authorizes (submitter-only), orders (latest round) and records (an audit row) the
+continuation, and refuses when another request is already pending on the record —
+so the pause it parks on declares `resumeAuthority: 'service'` and the generic
+`POST /api/v1/automation/:name/runs/:runId/resume` route answers **403** for it.
+ADR-0044 D3 originally prescribed a `wait` here; its 2026-07-28 amendment
+reversed that (#3823). A flow still carrying the old shape publishes with an
+error (`flow-approval-revise-target-not-service-owned`) and its `revise` verb is
+refused at runtime until the node's `type` is changed — approve/reject are
+unaffected.
 
 ### Acting on requests in the console
 

docs/adr/0044-approval-send-back-for-revision.md

@@ -1,6 +1,6 @@
 # ADR-0044: Flow-level send-back-for-revision — `revise` branch + typed back-edge re-entry
 
-**Status**: Accepted — engine + model implemented; designer pending (objectui) (proposed 2026-06-12 · calibrated 2026-06-12 · **amended 2026-07-28 (#3823): the revise pause moves to a service-owned node — D3's generic `wait` is superseded, see the amendment below**)
+**Status**: Accepted — engine + model implemented; designer pending (objectui) (proposed 2026-06-12 · calibrated 2026-06-12 · **amended 2026-07-28 (#3823): the revise pause moves to a service-owned node — D3's generic `wait` is superseded; amendment ratified by the maintainer and implemented 2026-08-05 as the `approval_revise` node type, see the amendment below**)
 **Deciders**: ObjectStack Protocol Architects
 **Builds on**: [ADR-0019](./0019-approval-as-flow-node.md) (approval as a durable-pause flow node), [ADR-0039](./0039-token-scope-tree-execution.md) (single-program-counter suspend model), thread interactions (#1740), [ADR-0042](./0042-approval-sla-escalation.md) (audit-first discipline)
 **Closes**: [#1744](https://github.com/objectstack-ai/objectstack/issues/1744)
@@ -78,7 +78,9 @@ stores all already understand it), not an invisible service limbo.
 > made `resume` authorization-bearing, that became exploitable (unauthorized
 > resubmit with no audit row; a colliding request can permanently destroy the
 > run). The revise pause moves to a **dedicated service-owned node** — still
-> visible on the canvas, no longer raw-resumable. See the amendment below.
+> visible on the canvas, no longer raw-resumable. Shipped 2026-08-05 as
+> **`approval_revise`**; read D3 as "the revise edge targets an
+> `approval_revise` node". See the amendment below.
 
 Resubmit is an explicit REST verb by the submitter:
 
@@ -147,7 +149,7 @@ them, under explicit constraints:
 | moment | request status | lock |
 |---|---|---|
 | round N pending | `pending` | locked |
-| revise window (run at wait node) | `returned` | **unlocked** (hook keys on pending) |
+| revise window (run at the `approval_revise` node) | `returned` | **unlocked** (hook keys on pending) |
 | after resubmit (round N+1) | new row `pending` | re-locked |
 
 - **unanimous × revise**: one approver's send-back finalizes the request
@@ -158,7 +160,7 @@ them, under explicit constraints:
   `recall` on the *latest `returned`* request (the one normal recall
   precondition `pending` doesn't cover) flips it `returned → recalled`
   (the one sanctioned terminal→terminal transition) and audits `recall`.
-  The run is paused at the *wait node*, which has no `reject` out-edge to
+  The run is paused at the *revise window* node, which has no `reject` out-edge to
   resume down — so this lands the engine's first **run-cancel primitive**:
   `cancelRun(runId, reason)` consumes the continuation and records a
   terminal `cancelled` log (`ExecutionStatus` already reserves the value).
@@ -269,6 +271,13 @@ no new machinery. The real axis was never visibility-vs-enforcement; it was
 **reuse-vs-a-new-type.** ADR-0044 chose reuse (no new node type), and reuse is
 what seated a generic node in a privileged position.
 
+**Ruling (2026-08-05).** The maintainer approved this reversal. The criterion set
+for the implementation was that the revise pause become visible to the existing
+#3801 `resumeAuthority` type gate **with zero new machinery**; the owner-claim
+alternative (a per-suspension capability) was rejected outright, its
+screen-inheritance hazard being part of why. See *Implementation* below for what
+shipped.
+
 **Decision of record (the short-term fix).**
 
 - The `revise` edge targets a **dedicated service-owned pause** — a distinct node
@@ -322,3 +331,64 @@ case that matters; the two deferred directions are how the platform would
 generalise if a third case appears.
 
 Refs #3801, #3853, #3879; security lineage in ADR-0019's #3801 / #3879 addenda.
+
+### Implementation (2026-08-05, #3823)
+
+Of the two equivalent shapes the amendment allowed, the **dedicated node type**
+shipped:
+
+- **`approval_revise`** (`APPROVAL_REVISE_NODE_TYPE`, `spec/automation/approval.zod.ts`)
+  — registered by `plugin-approvals` alongside the `approval` node, one call site
+  so no deployment can hold half the feature. Its descriptor declares
+  `resumeAuthority: 'service'`, `supportsPause`, `isAsync`, `category: 'human'`
+  and **no `configSchema`**: the window is pure position in the graph, with no
+  signal and no timer, so nothing invents an authorable surface that has no
+  reader. Its executor suspends and arms nothing, hence no
+  `onSuspensionReleased` pairing (contrast the wait node's timer one-shot).
+- **Nothing in the engine changed.** The #3801 gate keys on the suspended node's
+  registry type; a node type that declares service ownership is covered as-is.
+  That was the ruling's criterion and it held literally — the diff touches no
+  file in `service-automation`.
+- **Two refusals, both prescriptive.** `ApprovalService.sendBack` refuses a
+  `revise` edge whose target is not `approval_revise` **before any mutation**
+  (alongside the existing missing-edge check), so a run can never be parked in a
+  window something else can advance; and
+  `flow-approval-revise-target-not-service-owned` (`@objectstack/lint`,
+  severity `error`) rejects the shape at authoring time — `os build` / `os
+  validate` / `os lint` and the runtime metadata publish gate, via the already-wired
+  `lintFlowPatterns` entry. It qualifies for `error` under that module's stated
+  bar ("the runtime refuses"), which is why the deliberately-narrow lint promotion
+  needed no new rule wiring either.
+
+**Why the approval node does not re-suspend itself.** The equivalent shape was
+available and cheaper by one node type, but it would skip the author's `revise`
+edge — every node on that branch (a `notify`, a status update) would stop running,
+and the window would vanish from the canvas and the run log, which is the property
+D3 chose the generic `wait` for. Only the *reuse* was wrong.
+
+**Backward compatibility, stated plainly.** A flow authored against the original
+D3 (`revise` → a plain `wait`) keeps registering and running; its approvals stay
+decidable (`approve` / `reject` / `recall` / `reassign` are untouched). What
+changes is that its **send-back is refused** with a message naming the node and
+the one-token fix (`type: 'wait'` → `type: 'approval_revise'`), and re-publishing
+it reports the lint error. A run **already parked** in a legacy revise window
+before the upgrade stays raw-resumable: `SuspendedRun.nodeType` is recorded at
+pause time and read recorded-first on purpose, so a republish cannot re-type a
+node under a live run — such a run is drained by `resubmit` or `recall` as usual.
+
+An ADR-0087 D2 conversion (silently rewriting a `revise`-target `wait` to the new
+type at load) was considered and **rejected**: unlike the conversions in that
+layer it would not be a lossless re-spelling but a topology-conditional semantic
+rewrite, and it would silently drop a timer-flavoured wait's timer or make a wait
+shared by another in-edge service-only for that path too — breakage a conversion
+cannot see. The measured population argues the same way: the Studio designer
+cannot author revise edges yet (this ADR's own follow-up), the `cloud` repo has no
+revise flow, and this repo's single one is the showcase, migrated in the same PR.
+A loud refusal with a one-token fix beats a tolerance layer that would have to be
+retired later — and beats it most for AI authors, who read the diagnostic.
+
+**Narrowing worth knowing:** the `revise` edge's **immediate** target must be the
+window. A graph that wanted `revise → notify → window` is refused rather than
+analysed for "every pause reachable on this branch is service-owned", which is
+unbounded. Send-back already notifies the submitter itself, so the pattern has no
+lost capability behind it.

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

@@ -232,13 +232,18 @@ export const BudgetApprovalFlow = defineFlow({
     {
       // ADR-0044 revise window: the run parks here while the submitter reworks
       // the (now unlocked) record; their resubmit resumes it over the back-edge.
+      //
+      // `approval_revise`, not `wait` — the shape D3 originally prescribed and
+      // its 2026-07-28 amendment reversed (#3823). This pause is service-owned:
+      // `POST /api/v1/approvals/requests/:id/resubmit` is the only thing that
+      // may end it (submitter-only, audited, refusing a colliding pending
+      // request), and the descriptor says so with `resumeAuthority: 'service'`
+      // so the generic run-resume route refuses it. A `wait` here was
+      // raw-resumable by anyone holding the run id — hence no `waitEventConfig`
+      // either: the window has no signal to wait on.
       id: 'wait_revision',
-      type: 'wait',
+      type: 'approval_revise',
       label: 'Awaiting Revision',
-      // `waitEventConfig`, not a loose `config` — the latter is the undeclared
-      // back door retired in #4045. The conversion layer still rewrites it at
-      // load, but the showcase should demonstrate the declared spelling.
-      waitEventConfig: { eventType: 'signal', signalName: 'budget_revision' },
     },
     // A plain exclusive gateway: the predicate is on the out-edges (e4/e5).
     // It also carried `config.condition` — inert on every node but `start`, and
@@ -275,8 +280,8 @@ export const BudgetApprovalFlow = defineFlow({
     { id: 'e6', source: 'exec_review', target: 'approved', label: 'approve' },
     { id: 'e7', source: 'exec_review', target: 'rejected', label: 'reject' },
     // ADR-0044 send-back-for-revision loop on the manager step: revise walks
-    // to the wait node; the resubmit edge is the declared back-edge closing
-    // the cycle (type 'back' — excluded from DAG validation, traversed
+    // to the revise-window node; the resubmit edge is the declared back-edge
+    // closing the cycle (type 'back' — excluded from DAG validation, traversed
     // normally), re-entering the approval node as round 2.
     { id: 'e8', source: 'manager_review', target: 'wait_revision', label: 'revise' },
     { id: 'e9', source: 'wait_revision', target: 'manager_review', label: 'resubmit', type: 'back' },

packages/lint/src/index.ts

@@ -459,6 +459,7 @@ export {
   FLOW_APPROVAL_REVISE_DEAD_END,
   FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE,
   FLOW_APPROVAL_REVISE_DISABLED,
+  FLOW_APPROVAL_REVISE_TARGET_NOT_SERVICE_OWNED,
   FLOW_RUNAS_UNSCOPED,
   FLOW_ERROR_LABEL_NOT_FAULT,
   FLOW_BRANCH_LABEL_UNMATCHED,

packages/lint/src/lint-flow-patterns.test.ts

@@ -12,6 +12,7 @@ import {
   FLOW_APPROVAL_REVISE_DEAD_END,
   FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE,
   FLOW_APPROVAL_REVISE_DISABLED,
+  FLOW_APPROVAL_REVISE_TARGET_NOT_SERVICE_OWNED,
   FLOW_RUNAS_UNSCOPED,
   FLOW_ERROR_LABEL_NOT_FAULT,
   FLOW_BRANCH_LABEL_UNMATCHED,
@@ -216,13 +217,26 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => {
       nodes: [
         { id: 'start', type: 'start', config: { triggerType: 'manual' } },
         { id: 'mgr', type: 'approval', config: approvalConfig },
-        { id: 'wait', type: 'wait', config: { eventType: 'signal' } },
+        // #3823 — the revise window is the service-owned `approval_revise`
+        // node, not the bare `wait` ADR-0044 D3 originally prescribed. The
+        // dedicated case below pins that the old shape is now an error.
+        { id: 'wait', type: 'approval_revise' },
         { id: 'ok', type: 'end' },
         { id: 'no', type: 'end' },
       ],
       edges,
     }],
   });
+  /** The same flow with the revise edge pointed at a plain `wait` (pre-#3823). */
+  const legacyWaitFlow = (
+    edges: Array<{ source: string; target: string; label?: string; type?: string }>,
+  ) => {
+    const stack = approvalFlow(edges) as any;
+    stack.flows[0].nodes = stack.flows[0].nodes.map((n: any) =>
+      n.id === 'wait' ? { id: 'wait', type: 'wait', config: { eventType: 'signal' } } : n,
+    );
+    return stack;
+  };
   const declaredLoop = [
     { source: 'start', target: 'mgr' },
     { source: 'mgr', target: 'ok', label: 'approve' },
@@ -271,6 +285,32 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => {
     ];
     expect(lintFlowPatterns(approvalFlow(edges))).toEqual([]);
   });
+
+  // #3823 — the shape ADR-0044 D3 prescribed until its 2026-07-28 amendment.
+  // `error`, because `ApprovalService.sendBack` refuses this metadata outright:
+  // the revise branch can never run, and before that refusal the pause it
+  // produced was resumable by anyone holding the run id.
+  it('flags a revise edge into a bare wait node as an ERROR', () => {
+    const fnds = lintFlowPatterns(legacyWaitFlow(declaredLoop));
+    expect(fnds).toHaveLength(1);
+    expect(fnds[0]).toMatchObject({
+      rule: FLOW_APPROVAL_REVISE_TARGET_NOT_SERVICE_OWNED,
+      severity: 'error',
+    });
+    expect(fnds[0].where).toContain('mgr');
+    expect(fnds[0].message).toMatch(/node 'wait' of type 'wait'/);
+    expect(fnds[0].hint).toMatch(/type: 'approval_revise'/);
+  });
+
+  it('flags a revise edge into any other node type too (screen, not just wait)', () => {
+    const stack = approvalFlow(declaredLoop) as any;
+    stack.flows[0].nodes = stack.flows[0].nodes.map((n: any) =>
+      n.id === 'wait' ? { id: 'wait', type: 'screen', config: { fields: [{ name: 'note', type: 'text' }] } } : n,
+    );
+    const fnds = lintFlowPatterns(stack);
+    expect(fnds.map((f) => f.rule)).toEqual([FLOW_APPROVAL_REVISE_TARGET_NOT_SERVICE_OWNED]);
+    expect(fnds[0].message).toMatch(/type 'screen'/);
+  });
 });
 
 describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR-0073 D5 / #3760)', () => {