Skip to content

Commit ae35d1e

Browse files
os-zhuangclaude
andcommitted
docs(adr): correct ADR-0037 — the map node is NOT engine-free (A1 vs A2)
Building A2 surfaced a real error in the ADR: it claimed Track A's `map` / multi-instance node needs "no change to the engine's execution model." Examined against the resume/bubble code, that is false for any map that serves batch approval (each item can pause): - concurrent map needs durable N:1 aggregation + per-parent serialization + completion-ordering handling (part of Track B's hard concurrency, confined to one node); - sequential map needs resume-INTO-the-node (next item) instead of the engine's resume-past-the-node default (the DAG has no back-edge to loop the node); - only a synchronous, non-pausing map is engine-free, and that does not serve batch approval. Splits Track A into A1 (aggregating approval — truly free, shipped #1708) and A2 (map node — a bounded, separately-justified engine task, design-first). A1 covers parallel approvals at zero engine cost; A2 is not a free rider on it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2d3c6bf commit ae35d1e

1 file changed

Lines changed: 60 additions & 33 deletions

File tree

docs/adr/0037-token-scope-tree-execution.md

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@ A code review (below) shows the cost is much larger than the data-structure
2525
change implies.
2626

2727
**Decision: two tracks.** Ship **Track A — multi-instance / aggregating nodes**
28-
first: model the actual demand (parallel approvals, batch approvals) as *single
29-
nodes* that wait for N decisions, the way Camunda multi-instance and AWS Step
30-
Functions `Map` do. This needs **no change to the engine's execution model**
31-
the node owns the fan-out and the aggregation, the run still has one program
32-
counter. Defer **Track B — the general token/scope tree** until demand exceeds
33-
what multi-instance covers; this ADR records its design so Track A is built
34-
toward it, not away from it.
28+
first: model the demand as *single nodes* that wait for N decisions, the way
29+
Camunda multi-instance and AWS Step Functions `Map` do. Track A splits into a
30+
**free** tier and a **bounded** tier — a distinction worth stating up front:
31+
**A1 (parallel approval — one `approval` node aggregating N decisions) needs no
32+
engine change and is shipped (#1708)**; **A2 (a `map` / multi-instance node for
33+
batch approval) is NOT free** — because each item can pause, it needs a bounded
34+
extension of the engine's resume path (N:1 aggregation or node re-entry), so it
35+
is a separately-justified increment, not a free rider on A1. Defer **Track B —
36+
the general token/scope tree** until demand exceeds what multi-instance covers;
37+
this ADR records its design so Track A is built toward it, not away from it.
3538

3639
## Context — current state (verified 2026-06-11, against the code)
3740

@@ -107,24 +110,41 @@ execution model," not "add a tree to `SuspendedRun`."
107110
Model the concrete demand as **single nodes** that internally fan out and
108111
aggregate, leaving the engine's one-program-counter model intact:
109112

110-
- **`approval` gains multi-approver aggregation** (it largely has this already —
111-
`behavior: 'unanimous' | 'first_response'`): one `approval` node opens N
112-
approval requests and **stays suspended at that one node** until the
113-
aggregation rule is met, then resumes down `approve` / `reject`. "Finance AND
114-
legal" is `unanimous` over two approver groups — **one node, one program
115-
counter, already paused once**. No engine-core change.
116-
- **A `map` / multi-instance node** for "do X for each item, then continue":
117-
one node owns the collection, opens a child unit per item (e.g. an approval or
118-
a subflow per row), and suspends at that single node until all units settle.
119-
The aggregation (all / any / threshold) is node config. This is the Step
120-
Functions `Map` shape and Camunda's multi-instance activity.
121-
- These compose with ADR-0019 durable pause exactly as today: the node suspends
122-
once, at one position; resume targets the run, not a token. `sys_automation_run`
123-
is unchanged. The node executor tracks its own per-unit state (it already may,
124-
via the approvals service's `sys_approval_request` rows).
125-
126-
Track A covers parallel approvals and batch approvals — the demand that
127-
motivated this ADR — at a fraction of Track B's cost and risk.
113+
Track A has **two tiers of cost** — a distinction the first revision of this ADR
114+
got wrong by lumping them together. They are not equal.
115+
116+
**A1 — aggregating `approval` node (truly free; shipped #1708).** One `approval`
117+
node with `behavior: 'unanimous'` over N approver groups opens **one**
118+
`sys_approval_request` whose `pending_approvers` lists all groups (notified in
119+
parallel) and stays suspended until every group approves, then resumes down
120+
`approve` / `reject`. "Finance AND legal" is exactly this — **one node, one
121+
program counter, paused once**. This needed **no engine change**: the
122+
unanimous-over-N aggregation already exists in the approvals service and is
123+
unit-tested; A1 added a showcase (`showcase_invoice_signoff`) and docs, browser-
124+
verified. The aggregation state lives in the plugin's own `sys_approval_request`
125+
row, not the engine.
126+
127+
**A2 — `map` / multi-instance node (NOT free — engine-adjacent).** A correction:
128+
a `map` node that serves **batch approval** (each item can pause) **cannot** be
129+
"no engine change," contrary to this ADR's first revision. Examined against the
130+
code, every flavor needs a bounded extension of the engine's resume/bubble path:
131+
- *concurrent* map (N items pause at once) needs **durable N:1 aggregation +
132+
per-parent serialization + completion-ordering handling** — i.e. part of
133+
Track B's hard concurrency, just confined to one node;
134+
- *sequential* map (one item at a time) needs **resume-into-the-node** (process
135+
the next item) instead of the engine's resume-past-the-node default — the DAG
136+
has no back-edge to loop the node;
137+
- only a *synchronous, non-pausing* map is engine-free, and that does not serve
138+
batch approval (which pauses).
139+
The map node reuses ADR-0019's linked-runs (#1693) for the 1:1 bubble but
140+
extends it to N:1 / re-entry. It is a real, bounded engine task — smaller than
141+
the full Track B scheduler, but **not** the zero-cost item A1 was. It should be
142+
built only against concrete batch-approval demand, with the aggregation /
143+
re-entry semantics designed first.
144+
145+
So Track A as shipped (**A1**) covers *parallel* approvals at zero engine cost.
146+
*Batch* approvals (**A2**) are a deliberate, separately-justified increment, not
147+
a free rider on A1.
128148

129149
### Track B (deferred) — the general token / scope tree
130150

@@ -192,15 +212,22 @@ model:
192212

193213
## Sequencing
194214

195-
1. **A1 — specify the aggregating `approval` contract** (formalize
196-
`unanimous` / `first_response` / threshold over approver *groups*; today's
197-
`unanimous` already tallies — make N-group explicit and tested).
198-
2. **A2 — `map` / multi-instance node**: collection in, per-item child unit
199-
(approval or subflow), aggregation policy, single suspend/resume at the node.
200-
Showcase example: per-line-item approval over an invoice's lines.
215+
1. **A1 — aggregating `approval` node. ✅ Shipped (#1708).** The
216+
`unanimous`-over-N-approver-groups aggregation already existed and was
217+
unit-tested; #1708 added the `showcase_invoice_signoff` worked example
218+
(finance AND legal, browser-verified) and docs. No engine change. Threshold /
219+
quorum (M-of-N) stays enterprise-tier per `approval.zod.ts`.
220+
2. **A2 — `map` / multi-instance node (design-first; not started).** Collection
221+
in, per-item child unit, aggregation, single suspend at the node. **Cost
222+
correction**: because items can pause, this needs a bounded engine resume-path
223+
extension (durable N:1 aggregation for concurrent, or resume-into-node for
224+
sequential) — it is *not* the zero-engine-change item A1 was, so it is gated on
225+
concrete batch-approval demand and a design note that nails the aggregation /
226+
re-entry + serialization semantics first.
201227
3. **B-gate** — only if a concrete flow needs arbitrary-position concurrent
202-
pause: open a follow-up ADR to start Track B at §1's scheduler, with the
203-
one-token refactor as the first, behavior-preserving step.
228+
pause that a multi-instance node cannot express: open a follow-up ADR to start
229+
Track B at the scheduler, with the one-token refactor as the first,
230+
behavior-preserving step.
204231

205232
## Non-goals / deferred
206233

0 commit comments

Comments
 (0)