|
| 1 | +# ADR-0044: Flow-level send-back-for-revision — `revise` branch + typed back-edge re-entry |
| 2 | + |
| 3 | +**Status**: Proposed (2026-06-12) |
| 4 | +**Deciders**: ObjectStack Protocol Architects |
| 5 | +**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) |
| 6 | +**Closes**: [#1744](https://github.com/objectstack-ai/framework/issues/1744) |
| 7 | +**Consumers**: `@objectstack/spec` (flow edge type, branch labels, contracts), `@objectstack/service-automation` (back-edge traversal), `@objectstack/plugin-approvals` (send-back / resubmit runtime), REST, Console approvals inbox |
| 8 | + |
| 9 | +--- |
| 10 | + |
| 11 | +## TL;DR |
| 12 | + |
| 13 | +`requestInfo()` (#1740) is a conversation: the request stays pending, the |
| 14 | +record stays locked, the approver keeps the slot. Mainstream approval |
| 15 | +centers also model 退回修改 / *send back for revision* — a **flow |
| 16 | +movement**: the current approval request terminates, the flow walks a |
| 17 | +`revise` out-edge to a wait point where the record unlocks and the |
| 18 | +submitter edits it, and a *resubmit* walks a **back-edge** into the |
| 19 | +approval node, opening a fresh request (round 2) with a clean approver |
| 20 | +slate. A `maxRevisions` guard auto-rejects instances that would orbit |
| 21 | +forever. |
| 22 | + |
| 23 | +``` |
| 24 | +approval (suspended, round N) |
| 25 | + ├─ approve ──▶ … |
| 26 | + ├─ reject ───▶ … |
| 27 | + └─ revise ──▶ wait (suspended; record unlocked; submitter edits) |
| 28 | + └─ resubmit ──[back-edge]──▶ approval (round N+1) |
| 29 | +``` |
| 30 | + |
| 31 | +## Decisions |
| 32 | + |
| 33 | +### D1 — the sent-back request's terminal state is a new `ApprovalStatus: 'returned'` |
| 34 | + |
| 35 | +A third terminal state alongside `approved` / `rejected` / `recalled` |
| 36 | +(do **not** reuse `recalled`: recall is submitter-initiated withdrawal, |
| 37 | +returned is approver-initiated rework — inbox filters, SLA reporting and |
| 38 | +the status mirror must distinguish them). Because the record lock and the |
| 39 | +`openNodeRequest` per-(object, record) pending-dedupe are both keyed on |
| 40 | +`status: 'pending'`, finalizing round N as `returned` *automatically* |
| 41 | +unlocks the record and clears the way for round N+1 — no lock-machinery |
| 42 | +change at all. |
| 43 | + |
| 44 | +Sync points (dual-source enums, all updated together): |
| 45 | +`ApprovalStatus` (spec contracts), `sys_approval_request.object.ts` |
| 46 | +status select, and the Console status filters/badges. |
| 47 | + |
| 48 | +### D2 — `revise` joins `APPROVAL_BRANCH_LABELS`; `maxRevisions` joins the node config |
| 49 | + |
| 50 | +- `APPROVAL_BRANCH_LABELS = { approve, reject, revise }`. The decision |
| 51 | + surface stays `approve | reject` (`ApprovalDecision` unchanged); |
| 52 | + send-back is a **separate service verb** (`sendBack`), mirroring how |
| 53 | + `recall` is not a "decision". |
| 54 | +- `ApprovalNodeConfigSchema` gains |
| 55 | + `maxRevisions: int ≥ 0, default 3` — the maximum number of send-backs |
| 56 | + per (run, node). A send-back that would *exceed* the budget instead |
| 57 | + **auto-rejects**: the request finalizes `rejected` (audit carries the |
| 58 | + revise intent + an auto-reject marker comment), and the run resumes |
| 59 | + down the `reject` edge with `output.decision = 'reject'`, |
| 60 | + `output.autoRejected = true`. `maxRevisions: 0` ⇒ send-back always |
| 61 | + auto-rejects (effectively disabled, loudly). |
| 62 | +- A flow whose approval node has **no `revise` out-edge** rejects |
| 63 | + `sendBack` with `VALIDATION_FAILED` (checked against |
| 64 | + `automation.getFlow()` before any mutation). This guards the engine's |
| 65 | + label-fallback behavior — resuming with an unmatched `branchLabel` |
| 66 | + falls back to *all* out-edges, which must never happen by a user |
| 67 | + clicking a button. |
| 68 | + |
| 69 | +### D3 — wait-node + REST resubmit (not record-change triggers) |
| 70 | + |
| 71 | +The revise edge targets an ordinary **`wait` node** (signal flavor) — the |
| 72 | +durable pause already shipped for timers/signals. The revise window is |
| 73 | +therefore *visible flow state* (designer canvas, run logs, suspended-run |
| 74 | +stores all already understand it), not an invisible service limbo. |
| 75 | + |
| 76 | +Resubmit is an explicit REST verb by the submitter: |
| 77 | + |
| 78 | +``` |
| 79 | +POST /api/v1/approvals/requests/:id/revise (approver; audited 'revise') |
| 80 | +POST /api/v1/approvals/requests/:id/resubmit (submitter; audited 'resubmit') |
| 81 | +``` |
| 82 | + |
| 83 | +`resubmit` validates: actor is the submitter, the request is `returned`, |
| 84 | +and it is the **latest** request for its (run, node) — then resumes the |
| 85 | +run (branch label `resubmit`, informational). Traversal walks the |
| 86 | +back-edge into the approval node, whose executor re-runs `openNodeRequest` |
| 87 | +→ round N+1 pending request → re-lock → suspend. A record-change trigger |
| 88 | +was rejected: saving a draft mid-edit must not resubmit; an explicit |
| 89 | +"I'm done" verb matches every mainstream approval center and gives the |
| 90 | +UI an unambiguous button. |
| 91 | + |
| 92 | +New audit kinds `'revise'` / `'resubmit'` join `ApprovalActionKind` AND |
| 93 | +the `sys_approval_action` select enum (dual-source, missed sync = insert |
| 94 | +500). Both rows land on the *round-N* request: round N's trail ends |
| 95 | +`… revise → resubmit`, round N+1 opens with its own `submit`. |
| 96 | + |
| 97 | +### D4 — round numbering rides the config snapshot (`__round`), no migration |
| 98 | + |
| 99 | +`openNodeRequest` counts existing requests for (`flow_run_id`, |
| 100 | +`flow_node_id`) and stamps `__round: N+1` into `node_config_json` |
| 101 | +(precedent: `__flowLabel` / `__nodeLabel`). Surfaced as `round?: number` |
| 102 | +on `ApprovalRequestRow` (absent/1 ⇒ first round). `current_step_index` |
| 103 | +keeps its existing meaning; no schema change, old rows read as round 1. |
| 104 | + |
| 105 | +### D5 — engine: typed back-edges, re-entry semantics, runaway guard |
| 106 | + |
| 107 | +The flow spec docs already promise back-edges (*"back-to-previous |
| 108 | +rejection → a back-edge to an earlier node"*); the executor now honours |
| 109 | +them, under explicit constraints: |
| 110 | + |
| 111 | +- **Authoring**: `FlowEdgeSchema.type` gains `'back'`. A back-edge is an |
| 112 | + ordinary traversal edge at run time; its *only* special property is |
| 113 | + that **cycle validation ignores it**. `registerFlow` validation becomes: |
| 114 | + the graph **minus `back`-typed edges must be a DAG** (the existing |
| 115 | + `detectCycles` runs on the reduced graph). An unmarked cycle is still |
| 116 | + rejected — authors must opt in, edge by edge. |
| 117 | +- **Re-entry semantics** (same node, second visit): node outputs are |
| 118 | + written under `${nodeId}.${key}` — a re-entry **overwrites** (latest |
| 119 | + round wins), which is exactly what `decision`-style outputs want; |
| 120 | + the step log appends (every visit is a separate step entry, so run |
| 121 | + observability shows round 1 and round 2); a re-suspend at the same |
| 122 | + node persists a fresh continuation under the same `runId` (the resume |
| 123 | + path already rebuilds `SuspendedRun` from live state — no keyed-by-node |
| 124 | + assumption exists). |
| 125 | +- **ADR-0039 compatibility**: the single-program-counter invariant is |
| 126 | + untouched — a back-edge moves the *one* position backwards; it never |
| 127 | + creates a second concurrent position. Back-edges remain **banned inside |
| 128 | + structured regions** (regions stay acyclic per ADR-0031 validation, and |
| 129 | + durable pause inside a region is already rejected). ADR-0039's D7 |
| 130 | + "no back-edges" applied to *Track B's runtime tokens*; this ADR amends |
| 131 | + the authoring surface deliberately and narrowly. |
| 132 | +- **Runaway guard**: `executeNode` counts top-level visits per node |
| 133 | + (step-log entries without a `parentNodeId`, so loop-region iterations |
| 134 | + don't count); exceeding `MAX_NODE_REENTRIES = 100` fails the run with |
| 135 | + a loud error. This is the engine's backstop; the *product* guard is |
| 136 | + `maxRevisions` (D2), which terminates well before. |
| 137 | + |
| 138 | +### D6 — lock lifecycle and the interaction matrix |
| 139 | + |
| 140 | +| moment | request status | lock | |
| 141 | +|---|---|---| |
| 142 | +| round N pending | `pending` | locked | |
| 143 | +| revise window (run at wait node) | `returned` | **unlocked** (hook keys on pending) | |
| 144 | +| after resubmit (round N+1) | new row `pending` | re-locked | |
| 145 | + |
| 146 | +- **unanimous × revise**: one approver's send-back finalizes the request |
| 147 | + immediately (like reject under unanimous). Round N+1 reopens with the |
| 148 | + **full approver set**; prior approvals do not carry over — the data |
| 149 | + changed, so every sign-off is stale by definition. |
| 150 | +- **recall × revise window**: the submitter may abandon a revision — |
| 151 | + `recall` on the *latest `returned`* request (the one normal recall |
| 152 | + precondition `pending` doesn't cover) flips it `returned → recalled` |
| 153 | + (the one sanctioned terminal→terminal transition) and audits `recall`. |
| 154 | + The run is paused at the *wait node*, which has no `reject` out-edge to |
| 155 | + resume down — so this lands the engine's first **run-cancel primitive**: |
| 156 | + `cancelRun(runId, reason)` consumes the continuation and records a |
| 157 | + terminal `cancelled` log (`ExecutionStatus` already reserves the value). |
| 158 | + Recall of a *pending* request keeps its existing reject-edge resume. |
| 159 | + SLA escalation, reminders and action links all key on `pending` and are |
| 160 | + naturally inert during the window. |
| 161 | +- **escalation × revise**: `returned` requests are invisible to the |
| 162 | + escalation sweep (it scans `pending`); round N+1 starts a fresh SLA |
| 163 | + clock from its own `created_at`. Deliberate: the clock measures *this |
| 164 | + approver's* latency, not the submitter's rework time. |
| 165 | + |
| 166 | +## Why not the alternatives |
| 167 | + |
| 168 | +- **Reuse `recalled` for sent-back** — collapses two different actors and |
| 169 | + intents into one state; the inbox can no longer say "waiting on you to |
| 170 | + fix and resubmit" vs "you withdrew this". |
| 171 | +- **Approval node re-suspends itself in a "revise mode"** (no wait node, |
| 172 | + no back-edge) — hides a whole state machine inside one node, invisible |
| 173 | + to the canvas/run log, and still needs re-entry semantics the moment a |
| 174 | + second round opens a new request. |
| 175 | +- **Record-change-triggered resubmit** — every draft save becomes a |
| 176 | + resubmission; no explicit user intent; collides with the lock hook's |
| 177 | + system-write exemptions. |
| 178 | +- **Generic engine `goto`/jump API** — strictly more power than needed; |
| 179 | + typed back-edges keep the authored graph the single source of truth |
| 180 | + and keep validation decidable. |
| 181 | + |
| 182 | +## Consequences |
| 183 | + |
| 184 | +- **Revise window × record-change triggers**: an edit made inside the |
| 185 | + window can re-fire the very record-change trigger that opened the flow |
| 186 | + (the showcase budget flow gates on `budget != previous.budget`), opening |
| 187 | + a *parallel* run's pending request on the same record. `resubmit` |
| 188 | + refuses with `DUPLICATE_REQUEST` while any pending request collides on |
| 189 | + the record — refusing *before* the suspension is consumed, so the |
| 190 | + parked run stays resumable once the collision is recalled. Flow authors |
| 191 | + should gate such start conditions (e.g. on the mirrored approval-status |
| 192 | + field) when the trigger field is one the submitter is expected to edit |
| 193 | + during revision. |
| 194 | + |
| 195 | +- The DAG invariant softens to "DAG modulo declared back-edges" — cycle |
| 196 | + detection, designer validation and AI flow authoring all need the same |
| 197 | + reduced-graph rule (Studio designer support for *drawing* revise edges |
| 198 | + is a follow-up issue; the model/engine land first). |
| 199 | +- Two enum dual-sources gain values (`returned`; `revise`/`resubmit`) — |
| 200 | + the known 500-on-insert trap if either side is missed. |
| 201 | +- `IApprovalService` grows `sendBack()` / `resubmit()`; REST grows the |
| 202 | + two verbs; Console inbox grows the approver button, the submitter |
| 203 | + resubmit entry, timeline rendering and ten-locale strings. |
| 204 | +- Round-aware inbox: `round` on the row enables "Round 2" chips with no |
| 205 | + migration. |
| 206 | + |
| 207 | +## Test matrix (the real cost, ADR-0039 style) |
| 208 | + |
| 209 | +multi-round (1→2→3) × `unanimous` (send-back mid-round clears partial |
| 210 | +approvals) × lock states (locked → unlocked → re-locked) × recall crossing |
| 211 | +the revise window × `maxRevisions` overflow auto-reject × flows with no |
| 212 | +revise edge (sendBack rejected) × engine: back-edge registration passes / |
| 213 | +unmarked cycle still rejected / re-entry overwrites outputs / runaway |
| 214 | +guard trips. |
0 commit comments