|
1 | 1 | # ADR-0044: Flow-level send-back-for-revision — `revise` branch + typed back-edge re-entry |
2 | 2 |
|
3 | | -**Status**: Accepted — engine + model implemented; designer pending (objectui) (proposed 2026-06-12 · calibrated 2026-06-12) |
| 3 | +**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**) |
4 | 4 | **Deciders**: ObjectStack Protocol Architects |
5 | 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 | 6 | **Closes**: [#1744](https://github.com/objectstack-ai/objectstack/issues/1744) |
@@ -73,6 +73,13 @@ durable pause already shipped for timers/signals. The revise window is |
73 | 73 | therefore *visible flow state* (designer canvas, run logs, suspended-run |
74 | 74 | stores all already understand it), not an invisible service limbo. |
75 | 75 |
|
| 76 | +> **Superseded 2026-07-28 (#3823).** Reusing the generic `wait` put an |
| 77 | +> author-placed, raw-resumable node in a service-owned position. Once #3801 |
| 78 | +> made `resume` authorization-bearing, that became exploitable (unauthorized |
| 79 | +> resubmit with no audit row; a colliding request can permanently destroy the |
| 80 | +> run). The revise pause moves to a **dedicated service-owned node** — still |
| 81 | +> visible on the canvas, no longer raw-resumable. See the amendment below. |
| 82 | +
|
76 | 83 | Resubmit is an explicit REST verb by the submitter: |
77 | 84 |
|
78 | 85 | ``` |
@@ -171,7 +178,11 @@ them, under explicit constraints: |
171 | 178 | - **Approval node re-suspends itself in a "revise mode"** (no wait node, |
172 | 179 | no back-edge) — hides a whole state machine inside one node, invisible |
173 | 180 | to the canvas/run log, and still needs re-entry semantics the moment a |
174 | | - second round opens a new request. |
| 181 | + second round opens a new request. *(Partially reversed 2026-07-28, #3823: |
| 182 | + the objection is to hiding the state inside the approval node, and it does |
| 183 | + **not** apply to a **dedicated** revise-pause node — visible on the canvas |
| 184 | + like `wait`, but typed as service-owned. The real axis was reuse-vs-a-new-type, |
| 185 | + not visibility-vs-enforcement. See the amendment.)* |
175 | 186 | - **Record-change-triggered resubmit** — every draft save becomes a |
176 | 187 | resubmission; no explicit user intent; collides with the lock hook's |
177 | 188 | system-write exemptions. |
@@ -212,3 +223,102 @@ the revise window × `maxRevisions` overflow auto-reject × flows with no |
212 | 223 | revise edge (sendBack rejected) × engine: back-edge registration passes / |
213 | 224 | unmarked cycle still rejected / re-entry overwrites outputs / runaway |
214 | 225 | guard trips. |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | +## Amendment (2026-07-28, #3823) — the revise pause must be service-owned; the generic `wait` was the wrong reuse |
| 230 | + |
| 231 | +**What reverses.** D3's "the revise edge targets an ordinary `wait` node" and the |
| 232 | +second *Why not* bullet's rejection of a service-owned revise pause. Both stand on |
| 233 | +information this ADR did not have: at authoring time `resume(runId)` had **no |
| 234 | +authorization model**, so *which node the run is parked on* carried no security |
| 235 | +weight — a `wait` was as safe as anything, and reuse was pure upside. [#3801](https://github.com/objectstack-ai/objectstack/issues/3801) |
| 236 | +then made the resume seam authorization-bearing (a node's descriptor declares |
| 237 | +`resumeAuthority`, and an `approval` pause is refused to anyone but its owning |
| 238 | +service). That reframes the choice: **a generic node sitting in a service-owned |
| 239 | +position is now precisely what a type-keyed gate cannot see.** This is new |
| 240 | +information reversing an earlier trade-off, not a defect in the original call. |
| 241 | + |
| 242 | +**The hole it opened** (demonstrated in #3823 — a repro on the real engine + |
| 243 | +`ApprovalService`, not reasoned). The revise window parks the run on an |
| 244 | +author-placed `wait`, which is `resumeAuthority: 'any'`. A raw |
| 245 | +`POST …/runs/:runId/resume` therefore: |
| 246 | + |
| 247 | +1. walks the `resubmit` back-edge into the approval node with **no submitter |
| 248 | + check and no `resubmit` audit row** — an unauthorized resubmit the trail |
| 249 | + never records (an empty body suffices; no `branchLabel` is needed); and |
| 250 | +2. worse — when a colliding pending request exists on the record, the exact case |
| 251 | + `resubmit` refuses with `DUPLICATE_REQUEST` *specifically to keep the run |
| 252 | + alive* (see Consequences) — the raw resume goes around that guard, the |
| 253 | + approval node's re-entry fails **after** the engine has consumed the |
| 254 | + suspension, and the run is **permanently destroyed**: the round-N request is |
| 255 | + stuck `returned`, the run that owned it is gone, and no resubmit can ever |
| 256 | + reach it. |
| 257 | + |
| 258 | +The first is unconditional; the second is opportunistic but is remote run |
| 259 | +destruction, not merely a missing audit row. |
| 260 | + |
| 261 | +**Why the original objection no longer decides it.** This ADR rejected the |
| 262 | +service-owned revise pause because it "hides a whole state machine inside one |
| 263 | +node, invisible to the canvas/run log." That objection is against re-suspending |
| 264 | +*inside the approval node*. It does **not** apply to a **dedicated revise-pause |
| 265 | +node type**: that node is still a first-class box on the canvas and in the run |
| 266 | +log — visible exactly as the generic `wait` is today — while its descriptor |
| 267 | +declares `resumeAuthority: 'service'`, so the existing #3801 gate covers it with |
| 268 | +no new machinery. The real axis was never visibility-vs-enforcement; it was |
| 269 | +**reuse-vs-a-new-type.** ADR-0044 chose reuse (no new node type), and reuse is |
| 270 | +what seated a generic node in a privileged position. |
| 271 | + |
| 272 | +**Decision of record (the short-term fix).** |
| 273 | + |
| 274 | +- The `revise` edge targets a **dedicated service-owned pause** — a distinct node |
| 275 | + type whose descriptor carries `resumeAuthority: 'service'` (equivalently, the |
| 276 | + approval node re-suspends in a revise mode that surfaces as its own step). It |
| 277 | + stays visible on the canvas and in run logs; it is no longer raw-resumable. |
| 278 | +- `resubmit` remains the only door into the window, so the submitter check, the |
| 279 | + latest-request check and the `resubmit` audit row are back on the only path |
| 280 | + that can advance it — and the `DUPLICATE_REQUEST` run-preservation guard can no |
| 281 | + longer be bypassed. |
| 282 | +- **Publish-time graph-lint rejects a `revise` edge wired into a bare `wait`.** |
| 283 | + The previously-recommended shape becomes *un-authorable*, surfaced at authoring |
| 284 | + rather than runtime — the cloud#688 pattern (fix the producer + reject the |
| 285 | + wrong shape; never tolerate it at the consumer). This is the decisive property |
| 286 | + for a metadata-driven platform: an AI author following the *original* D3 sketch |
| 287 | + would generate the vulnerable graph verbatim, because nothing in the metadata |
| 288 | + expressed that the wait sat in a privileged position. Making the wrong shape |
| 289 | + unrepresentable is the only fix that survives AI authoring. |
| 290 | + |
| 291 | +**Directions recorded but deliberately not built here** (ADR-0049 posture — no |
| 292 | +speculative machinery ahead of a consumer): |
| 293 | + |
| 294 | +- *Fail-closed descriptor default.* `resumeAuthority` defaults to `'any'`, so |
| 295 | + every future pausing node ships fail-open unless the author remembers the flag |
| 296 | + — the "declared ≠ enforced" trap (AGENTS.md Prime Directive #10) one node away. |
| 297 | + The long-term-correct default is that a pause is **not** raw-resumable unless it |
| 298 | + opts into `'any'` (screen / wait-signal declaring it explicitly). A breaking |
| 299 | + change to the descriptor default; needs a migration; tracked separately. |
| 300 | +- *Per-suspension owner claim.* The general answer for pauses whose authority |
| 301 | + depends on runtime **position** rather than declared **type**: the pause mints a |
| 302 | + capability the resumer must present (the Step Functions task-token shape — our |
| 303 | + `RESUME_AUTHORITY_SERVICE` symbol, but bound to the suspension *instance* rather |
| 304 | + than the node type). Not needed for the two cases we have (approval, revise), |
| 305 | + both of which type + a fail-closed default resolve. It also carries a |
| 306 | + non-obvious hazard: a claim must **not** be inherited by a downstream pause — an |
| 307 | + approval whose `approve` branch reaches a `screen` must leave that screen |
| 308 | + caller-continuable, so blanket inheritance would break screen flows. A reason |
| 309 | + not to reach for it prematurely. |
| 310 | + |
| 311 | +**How mainstream approval/workflow engines inform this.** None exposes a |
| 312 | +generic, node-type-agnostic "resume run X at its current pause" to untrusted |
| 313 | +callers. Salesforce / ServiceNow make each pause a distinct authorized operation |
| 314 | +(approve / reject / recall). Camunda separates user-task `complete` from message |
| 315 | +`correlate`, with manual token moves gated as an audited admin escape hatch. |
| 316 | +Temporal routes through named signals the workflow itself gates. Step Functions |
| 317 | +mints a task token at suspend that the resumer must hold. ObjectStack's one |
| 318 | +generic resume door is the outlier; keying its authority on node type is a |
| 319 | +reasonable stopgap, but the revise `wait` is the model showing the key doesn't |
| 320 | +always match the trust boundary. The short-term fix realigns them for the one |
| 321 | +case that matters; the two deferred directions are how the platform would |
| 322 | +generalise if a third case appears. |
| 323 | + |
| 324 | +Refs #3801, #3853, #3879; security lineage in ADR-0019's #3801 / #3879 addenda. |
0 commit comments