Skip to content

Commit 142ce37

Browse files
jitokimoh-my-graph
andauthored
feat(graph): a fragment may declare a loop, not only a node (ADR 0027) (#186)
* docs(adr): the reusable unit is a loop, not a node ADR 0027. A fragment may declare `nodes:` plus a required `exit:`, so a QA loop or a review loop is citable with `use:` the way a single node is today. ADR 0013's rule is generalized, not weakened: a fragment may never name an id it does not itself declare, of which "a single-node fragment may not declare depends_on/feedback" is the special case. Measured 2026-08-16 across the 8 shipped graphs and the 18 operator one-shot lanes: 12 of 18 lanes cite `use: pr-publish` and every one of those 12 hand-writes the `review -> apply` pair immediately upstream of the node it cited. Reuse is taken to exactly the depth the mechanism allows and stops there. Corrects the commissioning brief's first figure: `feedback:` is declared by 2 of 8 shipped graphs (not 6) and 0 of 18 lanes. The repeating loop is overwhelmingly a hand-unrolled one, not a declared arc, which is why the unit spliced here is a subgraph rather than a loop primitive. Records three findings the design brief did not carry: - `graph.feedbackTokenPattern` must gain `/` alongside `handoff.placeholderPattern`, or a spliced `{{ feedback.a/b }}` escapes the load-time confinement check and is silently empty forever; - `graph.nodeIDPattern` must admit the joined form, so the cannot-collide property is restored by a loader refusal rather than falling out of the charset; - `internal/serve` and the dry-run seed re-spell the artifact path without `sanitizeNodeID`, and would render 204 for a spliced node that has a result. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * docs(adr): a namespace that collides on disk is not a namespace ADR 0027, revised under design review before any code exists. Two critical corrections, both of which would have shipped a silently wrong run, plus seven smaller ones. `handoff.sanitizeNodeID` is NOT injective — it was merely the identity, because no valid id could contain `/`. Minting `/` ids makes `a` + `b_c` and `a_b` + `c` both persist to `a_b_c.out`, and a hand-written `qa-a_impl` share a file with a spliced `qa-a/impl`. The ids differ, so uniqueness validation passes; the FILE collides, the later node overwrites, and `{{ artifacts.x | inline }}` inlines someone else's output into a paid prompt. This is the exact class `feedback/` and `failed/` exist to dodge, with the reason already in the code. The separator's on-disk spelling therefore becomes `~` — outside the id charset, injective over every id the loader admits, and the identity on every file that exists today. Keeping `_` plus a duplicate-sanitized-id load error was rejected: it makes a fragment's validity depend on what else the host graph contains, which is verbatim why flat ids are rejected. `{{ artifacts.<using-id> }}` now rewrites to the exit, symmetrically with `depends_on`. Without it the promised corpus conversion is impossible and fails at spend rather than at load, since `run` never invokes the handoff lint sweeps. The consequence is stated: a loop exposes exactly one value outward. Also settled: - rewrite applies to fragment-body tokens BEFORE `with:` substitution; a bound value is never rewritten, and one naming no declared id is a load error (self-dev binds `{{ artifacts.e2e }}` today); - the planner needs its own `*PlanError` refusal — `Plan` goes through `graph.Parse`, not `LoadFile`, and `validatePlannedNodes` never checks id shape, so the guarantee is three-way: loader, coordinator, Validate as backstop; - `rerun: qa-a/impl` is a load error (the non-goals blessed it while the failure modes refused it), and `rerun: qa-a` is a load error too rather than a silent rewrite to the exit — a request half-granted without a word; - `exit:` may not sit strictly inside one of the fragment's own feedback bodies, so downstream wiring cannot manufacture a side exit the fragment author cannot see; the remaining host-triggerable body checks are enumerated; - the conversion proof moves to `backlog-batch.yaml`. ADR 0013's frozen equivalence gate keys its mask by node id and covers ids and edges, so converting self-dev/dev-review-pr would retire that one-time evidence; this ADR declines to. Those two cannot be converted anyway — `pr` consumes two internal artifacts; - an internal node may declare `type: gate`, with its three consequences named; - author-supplied ids (`prefix:` or an id map) are weighed for the first time and rejected: cheaper by three refusals and a sanitizer change, but it makes a loop's internals ordinary addressable ids, which is the unit this ADR exists to create. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * feat(graph): a namespaced id is spellable by the splicer alone First slice of ADR 0027 (commit granularity: one commit per feature slice — this is the charset move and the three compensations that must land with it; the multi-node loader follows). A multi-node fragment splices `<using-id>/<internal-id>` ids, so four things move together, and one of them fails silently if forgotten: - nodeIDPattern admits at most one '/', each side an otherwise-valid segment. Validate accepts the joined form as the BACKSTOP it is: it cannot tell a spliced graph from a hand-written one, and a resumed leg re-parses a snapshot already full of joined ids. - handoff.placeholderPattern gains '/' so a spliced {{ artifacts.qa-a/impl }} resolves instead of shipping verbatim into a paid prompt. - graph.feedbackTokenPattern gains '/' in the SAME change, per the invariant its own comment states. Alone, the previous change would have let {{ feedback.qa-a/review }} interpolate at run time while staying invisible to validateFeedbackPlaceholders — and an unconfined feedback token does not fail, it is empty forever. - the id refusal moves to the two places an id is WRITTEN rather than read: the coordinator refuses a '/' in any id the planner produced (the loader's half lands with the multi-node splice). sanitizeNodeID becomes the exported, INJECTIVE SanitizeNodeID: '/' maps to '~', not '_'. With '_', `a` + `b_c`, `a_b` + `c` and a hand-written `a_b_c` all land on a_b_c.out — distinct ids, one file, whichever node finishes last overwriting, and `| inline` reading another node's reply into a paid prompt with nothing failing. '~' is outside the id charset, so the map is injective over every id the loader admits, and it is still the identity on every id that could exist before this change: no artifact file moves and no resume is disturbed. serve's /api/result and `run --dry-run`'s seeded paths stop re-spelling `<node-id>.out` and route through the sanitizer, or serve would answer 204 "no result yet" for a node that has a result. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * feat(graph): a fragment may declare a loop, not only a node Second slice of ADR 0027: the loader learns the multi-node form. A fragment file declares EITHER `node:` — unchanged in every respect, with every one of its tests kept as the regression proof — or `nodes:` plus a required `exit:`, and one sentence covers both: a fragment may never name an id it does not itself declare. A single-node fragment declares none, so depends_on/feedback stay load errors for it; a multi-node one declares its own, so edges among THOSE are legal and nothing else is. The old rule is now the special case, arithmetically. Resolution, in the order it happens: - a '/' in any id a FILE spells is refused BEFORE any splice, since that is the only moment an authored id and a minted one are distinguishable; - each internal node is namespaced <using-id>/<internal-id> — its id, its depends_on, its feedback.rerun, and every artifact/feedback token in its text — and only THEN substituted, so a value bound at the using site is never rewritten. Substitute-then-rewrite would silently re-point a binding whenever the citing graph's id happened to match one the fragment declares, which is a working reference aimed at someone else's node; - entry nodes (no internal parent) inherit the using node's depends_on; cwd/worktree stay on the using node and propagate to every spliced node, which is what backlog-batch writes by hand; - from outside, the loop is one thing whose value is its exit's: both `depends_on: [qa-a]` and `{{ artifacts.qa-a }}` resolve to `qa-a/<exit>`. `rerun: qa-a` does NOT — rewritten to the exit, an author asking to re-run a loop would silently get one node re-run. `exit:` is required and never inferred from the unique sink: inference is right only while there is exactly one, and when it is wrong it is wrong silently. It may not lie strictly inside one of the fragment's own feedback bodies, so no citing graph's downstream edge can manufacture a side exit in a fragment whose author wrote nothing wrong. A multi-node use declares wiring only — a behavior key on it is a load error naming the key, because there is no coherent way to overlay one node's success_check onto five. Tests: the two-lane happy path (ids, inheritance, propagation, both rewrites, no collision between two uses of one fragment, distinct artifact files), the binding-namespace order trap, and 21 refusals — every one asserted through both LoadFile and LintFile so the two views cannot disagree about which problem comes first. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * refactor(graphs): adr-driven-dev cites the round it wrote out twice Third slice of ADR 0027: the corpus proof. `adr-driven-dev` unrolled review → apply twice (round1 → apply1 → round2 → apply2); those four nodes are now two `use: repair-round`, and the graph file loses 119 lines for 53. What stopped being copied is the part that matters more than the line count: the one-direction discipline (a review with no write tool under permission_mode: plan, an apply with no review duty), both verdict contracts, the apply's tool grant, its evidence-grounded gate and its retry — identical in all four nodes, and the next correction to any of them was a hand sweep with a miss rate. The bindings carry only what genuinely differs per round: the focus, the reviewer, the review's bound, the apply's extra duty, the evidence command. `cwd:` is declared once per use and propagates to both spliced nodes. Two prompt convergences are reviewed changes rather than equivalence claims, and are why this graph sits in goldenTemplates and not in migratedTemplates: round2's CLEAN/FINDINGS wording converges on round1's, and the two apply prompts converge except for their one differing sentence. The resolved fields are otherwise identical, empty `review_agent`/`review_timeout` bindings included — a standard round still runs as a plain claude session under the runner's default bound. Three tests move with it, each for a stated reason: - the golden set gains adr-driven-dev, so an edit to repair-round shows up as four moved nodes in a PR diff; - the feedback-reach sweep skips MULTI-NODE fragment files, because a fragment only becomes a graph when it is spliced — its `nodes:` carry {{ with.x }} where a graph carries durations and regexes. It is swept through the templates that cite it; - the gating-review sweep skips multi-node resolutions instead of looking up a using id that is no longer a node, and its documented limits now name that second blind spot. The qualifier-clause sweep DESIGN.md publishes moves 26 → 24 declarations over the same 33 nodes: four fewer places to correct one sentence, which is the gap that section is making a point about. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * docs(adr): what implementation found, and the two docs the code moved Fourth slice of ADR 0027, per the standing rule that code and DESIGN.md never drift apart. DESIGN.md's "Fragments" section states both forms and the one invariant covering them, plus the resolution rules a graph author needs: the namespace and why it cannot collide, rewrite-before-substitution and what that means for a bound value, entry inheritance, cwd/worktree propagation, exit-resolution for downstream edges and artifact tokens, and the refusal of `rerun:` over a loop. The node-schema section points at it from the `use:` example, and the golden list gains adr-driven-dev with the blast radius spelled out. docs/RUN-FEED.md states the artifact filename as a rule a consumer can apply — the node id with '/' (and this platform's separator) replaced by '~' — and says why '~' rather than '_': the map has to be injective, or `a` + `b_c`, `a_b` + `c` and a hand-written `a_b_c` share one file and a consumer is handed another node's result with nothing failing. The rule is a no-op for every id anyone can write. The ADR gains a "What implementation found" section recording three things it got wrong before any code existed. The first matters most: the conversion proof it worked out at length — backlog-batch's lane A — is blocked by this ADR's OWN non-goal, because two of that lane's three nodes are themselves `use:` citations and folding them in would need `use:` inside a fragment. Converting anyway would fork e2e-verify and review-style into a third copy each, which is the drift ADR 0013 exists to kill. So: a lane that already uses fragments well is the hardest lane to convert, and the ones that convert cleanly are the ones that never reached for a fragment — which is also exactly where the measured copy-paste is. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * test(graph): a spliced node faces the graph's own validations Fifth slice of ADR 0027, pinning the half of the design that is deliberately not new machinery. A spliced node is an ordinary node, so the graph-level rules judge it exactly as they judge a hand-written one, and the error names the spliced id — which locates the using site, because the spliced id begins with it. Two shapes ADR 0027 lists as inherent to splicing into someone else's graph rather than as new checks, now asserted instead of asserted-about: an ENTRY node with `handoff: session` inherits parents whose arity the fragment cannot know, and a gate inside a spliced feedback body is refused post-splice by ADR 0010's rule 4. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * fix(graph): three small divergences a fragment could hide behind An empty `depends_on: []` inside a fragment node was neither an entry node nor an internal child. Entry-hood is decided by the key's PRESENCE, so the node inherited nothing, became a root of the citing graph, and started in parallel with the work the using node said it came after. Say it, don't infer it: an empty sequence is now a load error naming the node. `resolveLoopReferences` looked up a loop trimmed and wrote it back untrimmed, so a quoted `depends_on: [" qa"]` minted " qa/review" and died on nodeIDPattern complaining about a shape its author never wrote. Its sibling `namespaceNode` already had this right; the two must agree. `fragmentFeedbackBodies` duplicates Graph.FeedbackBody, and dropped its ancestor guard — so an `exit:` at a non-ancestor rerun target was refused for "lying inside the feedback body" that arc does not have, at the FILE level, which hid the true "not a proper ancestor" error entirely by never letting the splice happen. The duplication's price is paid on the promise that the two computations agree; this is the guard that keeps it. cwd propagation gets the direct assertion worktree already had, rather than only the reading through adr-driven-dev's golden. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * fix(graph): the two spellings that reached in and were let through Encapsulation was enforced on three of the four ways to reach into a loop. The fourth is the one that actually leaks: an artifact token. `prompt: "ship {{ artifacts.qa/review | inline }}"` names a node that really exists after the splice and really is an ancestor of the node quoting it, so LintPlaceholders is satisfied too and nothing anywhere objected — at load, at lint or at run. The loop's internal output was simply read from outside, which is the encapsulation this ADR pays a required exit: and a widened nodeIDPattern for. It also left the host graph laxer than the fragment, which is backwards: a fragment naming an undeclared id in a token was already a load error. refuseAuthoredNamespaces now walks every scalar of every authored node, not a field list — a binding is authored text as surely as a prompt is, and a walk cannot be outrun by a field this schema grows later. Pre- splice, where every '/' in the document is provably one a human typed. The second: a multi-node use: REPLACES its node with the spliced ones, so a hand-written `qa` beside a `use:` node also called `qa` resolved to qa, qa/impl, qa/review — all distinct, all unique, loads clean. Every downstream depends_on: [qa] and {{ artifacts.qa }} was then rewritten to the LOOP's exit, past a node literally named qa. Before ADR 0027 that file was a loud duplicate-id error; after it, a working graph wired to the wrong producer. Refused in document order, so LoadFile and LintFile still agree on which problem comes first. And the residue of mapping loop -> exit before the existence test: a using node binding its OWN id passed, because its exit exists. It is now refused on its own terms — a loop cannot be given its own output as an input. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * test(graph): a structural exclusion decided structurally The multi-node fragment skip in the shipped-graph sweep classified with strings.Contains(data, "\nnodes:"). A SINGLE-node fragment whose prompt happened to contain a line beginning `nodes:` would have been dropped silently — not far-fetched in a repo whose prompts routinely quote graph YAML — from the very sweep that exists to catch what nobody noticed. The exclusion is structural, so it is decided structurally: unmarshal and test the top-level key. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * fix(graph): two orders a map walk was deciding for us fragmentFeedbackBodies ranged over its arcs map, so a fragment whose exit lies inside two feedback bodies reported the two "lies inside the feedback body" errors in whichever order that run drew. The package promises the opposite one line away — refuseLoopIDCollisions walks document order for "the deterministic first error LoadFile and LintFile must agree on", and the multi-node table asserts issues[0] equals the LoadFile error. Those two views load the fragment file separately, so the shuffle would not even have been the same shuffle. Walked over ids now, deduped against the repeat a duplicate-id fragment leaves there. The second order is the one recordBoundReferences was called in: before the isMulti branch, so ADR 0013's single-node form inherited a rule minted for namespacing. A single-node use: mints no namespace and rewrites no token — its body lands on the using node's own id — so a bound {{ artifacts.x }} is the same thing as one typed into a plain node's prompt, which this package leaves to the advisory sweep. It had become a hard load error for that one spelling, undocumented in DESIGN.md (which states the rule inside the multi-node bullet, correctly); and a node binding its OWN id was told a loop cannot be given its own output as an input, naming an exit and a descendant that a single-node splice has neither of. Recorded from the multi-node branch alone now, which is where the justification lives. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * docs(graph): three comments that named more than the code does nodeIDSegmentPattern's docstring claimed three enforcement points — the entry graph's nodes:, a fragment file's nodes:, and the coordinator — and has exactly one caller, judgeMultiNodeIDs. The other two test for a '/' and leave the rest of the shape to nodeIDPattern; same net effect, so the comment now says which check each is rather than implying a shared symbol a reader would go looking for. TestParse_NamespacedNodeIDAcceptedAsBackstop pointed at TestLoadFile_AuthoredNamespaceInIDRejected, which exists nowhere but in that line. The case is a subtest of TestLoadFile_MultiNodeLoadErrors. And the multi-node skip in TestAGatingReviewCarriesItsRecoveryArc is unreachable as graphs/fragments/ stands — the only multi-node fragment is repair-round, which the review-* prefix test already skipped. Kept, since it is what makes that test's second stated limit true rather than merely stated, but the comment now says so instead of reading as a branch that fires today. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * fix(graph): the fifth spelling, in the one body no invariant judges refuseAuthoredNamespaces reads the entry document, and a fragment file's own ids are held to the declared-ids invariant — which runs only when the file declares ids. A single-node fragment declares none: its tokens are deliberately allowed to name the citing graph's nodes (ADR 0013), and nothing narrowed that to non-namespaced ones. So a fragments/foo.yaml containing {{ artifacts.round1/review | inline }} loaded, resolved, and read a loop's internals from outside, with LintPlaceholders silent too because round1/review really exists and really is an ancestor — the exact property ADR 0027's finding 4 identified for the token spelling, arriving one file over. Three artifacts stated the refusal as total, so the gap is closed rather than the claim narrowed: the single-node branch of loadFragmentFile refuses a namespaced token beside the check that judges the multi-node one. {{ feedback.qa-a }} downstream of a loop was already refused, but by validateFeedbackPlaceholders' generic "declares no feedback edge" — about a node the author wrote as `- id: qa-a` and is looking straight at, since the splice replaced it. resolveLoopReferences now says what is true of it, symmetrically with the feedback.rerun refusal beside it. Two comments corrected rather than trusted. resolveFragments claimed errs[0] is "the first error in document order"; resolution is three sequential passes over one slice, so it is the first error of the earliest pass that has one — the property LoadFile and LintFile actually share. And spliceLoop now says that cwd+worktree on one using node reports the contradiction once per spliced node: each spliced node genuinely carries it, and pre-checking here would restate a rule validateWorktrees owns and cascade a worse report onto a node stripped of its use:. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * docs(adr): a node id is a key, and the grammar is the decision The ADR described `/` and `~` as characters the design picked. That framing invites the wrong question — "what if an author writes one anyway?" — and the answer then has to be re-derived at every call site. Stated as a grammar instead, in the sense a Redis key is domain-scoped: atoms joined by a delimiter the atoms may not contain. Two properties then fall out rather than being argued case by case. Decomposition is unique at any depth, because no atom can hold the delimiter, so `a/b/c` is `[a b c]` and there is no reading under which `a/b` was an atom. And the on-disk form is injective for the same reason and not by luck, `~` being as unwritable in an atom as `/`. An authored delimiter is a CONVENTION VIOLATION, refused where authorship happens. That refusal is what makes the properties load-bearing rather than hopeful, and it is why the collapse this design once seemed to risk — two ids on one artifact file, `| inline` feeding one node's reply into another's paid prompt — cannot happen quietly. The id never loads. Also records that the depth cap is enforcement of this ADR's own non-goal, not a limit of the scheme: opening nesting is `?` -> `*` and nothing else. Written because the operator was about to add the opposite warning — that closure would force the delimiter to be reconsidered — which is false and would have sent a later reader looking for a problem the grammar already solved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: jitokim <pigberger70@gmail.com> * test(graph): prompt2 is load-bearing, and now says so Review finding. The key looks like a typo and is not: this case's subject is refuseNestedUse, and prompt2 only has to keep {{ with.task }} referenced so the substitution checks stay quiet and the nesting refusal is what fires. "Correcting" it to prompt would leave the case passing for a different reason than the one it is named for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: jitokim <pigberger70@gmail.com> --------- Signed-off-by: jitokim <pigberger70@gmail.com> Co-authored-by: oh-my-graph <graphs@oh-my-graph.dev>
1 parent 989f828 commit 142ce37

25 files changed

Lines changed: 3759 additions & 211 deletions

CHANGELOG.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,30 @@ oh-my-graph is **alpha software**. The graph YAML schema, the CLI, and the
1010

1111
## [Unreleased]
1212

13+
### Added
14+
15+
- **A fragment may declare a LOOP, not only a node
16+
([ADR 0027](docs/adr/0027-the-reusable-unit-is-a-loop-not-a-node.md)).** A
17+
fragment file may now declare `nodes:` (several, with the edges among them)
18+
plus a required `exit:`, and be cited with the same `use:`/`with:` a
19+
single-node fragment is. Spliced ids are `<using-id>/<internal-id>`, which no
20+
author and no planner may write, so a spliced node can never collide with an
21+
authored one; entry nodes inherit the citing node's `depends_on`, `cwd:` and
22+
`worktree:` propagate from it, and `depends_on: [<loop>]` /
23+
`{{ artifacts.<loop> }}` from downstream both resolve to the loop's exit.
24+
`exit:` is never inferred from the unique sink — inference is right only
25+
while there is exactly one, and when it is wrong it is wrong silently.
26+
ADR 0013's rule is generalized, not weakened: **a fragment may never name an
27+
id it does not itself declare**, of which "a single-node fragment may declare
28+
no wiring at all" is now the special case, with every one of its tests kept.
29+
Measured on the shipped corpus: `adr-driven-dev`'s two hand-unrolled
30+
review/apply rounds became two `use:` of one fragment, **119 lines removed
31+
for 53**, and the one-direction discipline, both verdict contracts, the
32+
apply's tool grant, its evidence gate and its retry stopped being written out
33+
four times. Scheduler, snapshot, event feed and ledger are untouched — a
34+
spliced node is an ordinary node, and a consumer that wants the loop view
35+
groups by the `<using-id>/` prefix.
36+
1337
### Changed
1438

1539
- **A node's `budget_usd` no longer refuses a Codex graph

DESIGN.md

Lines changed: 107 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ resolved away at load time — see "Fragments" below):
220220
checks: run `make local` (build + test + vet).
221221
```
222222

223+
The cited fragment may declare **several** nodes and the edges among them — a
224+
review/repair round, a QA loop — and the citing site is unchanged. It splices
225+
as `<this id>/<the fragment's own id>`, so the node above would become
226+
`e2e/review`, `e2e/apply`, and `depends_on: [e2e]` downstream still means
227+
"after it" (ADR 0027, "Fragments" below).
228+
223229
Graph file has `name`, `version`, `inputs: [..]`, `concurrency: N`,
224230
`on_fail: halt | continue` (default halt — the graph's own failure policy;
225231
see "Execution engine" step 4), `nodes: [..]`.
@@ -233,11 +239,13 @@ other bodies, and `max` required ≥ 1 — an unbounded loop on a paid runtime i
233239
unrepresentable). Iteration is a *runtime* phenomenon; full semantics in
234240
ADR 0010 and under "Execution engine" below.
235241

236-
### Fragments — `use:`/`with:`, resolved by the file loader (ADR 0013)
242+
### Fragments — `use:`/`with:`, resolved by the file loader (ADR 0013, 0027)
237243

238-
A fragment is a **single-node definition file** with declared substitution
239-
points — a proven node shape (the e2e gate, the security review) written
240-
once, upstream, instead of copy-varied across graphs:
244+
A fragment is a **definition file** with declared substitution points — a
245+
proven shape written once, upstream, instead of copy-varied across graphs. It
246+
declares **either** one node's behavior (`node:`, below) **or** a whole
247+
subgraph (`nodes:` + `exit:`, further below — the loop). The citing site is the
248+
same for both: a node with `use:` and `with:`.
241249

242250
```yaml
243251
# graphs/fragments/e2e-verify.yaml
@@ -298,8 +306,8 @@ substitution points or it is a different shape. The behavior fields
298306
(`allowed_tools`, `permission_mode`, `budget_usd`, `timeout`, `handoff`,
299307
`success_check`, `retry`, `agent`, `type`) default from the fragment; a key
300308
written in the using node overrides the **whole** top-level subtree (never a
301-
deep merge). A fragment may not declare wiring — `id`, `depends_on`, `cwd`,
302-
`worktree`, `feedback` (load error) — nor `use:` itself (no nesting in v1) —
309+
deep merge). A **single-node** fragment may not declare wiring — `id`, `depends_on`, `cwd`,
310+
`worktree`, `feedback` (load error) — nor `use:` itself (no nesting) —
303311
nor a **YAML alias or `<<:` merge key inside the `node:` block** (load error):
304312
a spliced body has to be walkable in full, or a `{{ with.x }}` hiding behind
305313
an alias would be neither declaration-checked nor substituted and would reach
@@ -318,9 +326,90 @@ would survive resolution into a paid prompt verbatim. A
318326
declared-but-unreferenced point and a stray `{{ with.x }}` in a plain node are
319327
advisories.
320328

329+
**The multi-node form — a fragment that declares a LOOP (ADR 0027).** A
330+
fragment file may declare `nodes:` (a list, each with its own `id`) plus a
331+
required `exit:`, instead of `node:`. Never both, never neither.
332+
333+
```yaml
334+
# graphs/fragments/repair-round.yaml
335+
fragment: repair-round
336+
description: one review/repair round — a review that never edits, then a gated apply
337+
substitutions: [review_focus, review_agent, review_timeout, apply_scope, verify_command]
338+
exit: apply
339+
nodes:
340+
- id: review
341+
agent: "{{ with.review_agent }}"
342+
prompt: "{{ with.review_focus }} …"
343+
permission_mode: plan
344+
- id: apply
345+
depends_on: [review]
346+
prompt: "Apply this: {{ artifacts.review | inline }} …"
347+
success_check: { verify: { command: "{{ with.verify_command }}", timeout: 5m } }
348+
```
349+
350+
The invariant is one sentence covering both forms: **a fragment may never name
351+
an id it does not itself declare.** A single-node fragment declares none, so
352+
`id`/`depends_on`/`feedback` stay load errors for it; a multi-node one declares
353+
its own, so edges among *those* are legal and naming anything else — in
354+
`depends_on`, in `feedback.rerun`, or in an `{{ artifacts.<id> }}` token — is a
355+
load error charged to the fragment file. `cwd`/`worktree` stay refused in both:
356+
they are the using node's location, not wiring among declared ids.
357+
358+
Resolution, in the order it happens:
359+
360+
- Spliced ids are **`<using-id>/<internal-id>`** — `round1/review`,
361+
`round1/apply`. `/` cannot appear in anything anyone writes: in an entry
362+
graph the loader refuses it in a node `id`, a `depends_on`, a
363+
`feedback.rerun` **and in any `{{ artifacts.<id> }}` / `{{ feedback.<id> }}`
364+
token in any scalar** (a binding included — the token is the spelling that
365+
would otherwise read a loop's internal output from outside, with every other
366+
check satisfied); a multi-node fragment file is held to the same rule by its
367+
declared-ids invariant, and a **single-node** one — whose tokens name the
368+
citing graph and so are checked against no such set — by a namespaced-token
369+
refusal of its own; `coordinator.validatePlannedNodeID` refuses it in a
370+
planner reply. So a spliced id can never collide with an authored one.
371+
`Validate` accepts the joined form as a backstop — it cannot tell a spliced
372+
graph from a resumed snapshot, and must not learn.
373+
- A multi-node `use:` id may not **collide** with another node's id. The splice
374+
replaces the using node, so post-splice uniqueness would see only distinct
375+
ids while every downstream `depends_on: [qa]` and `{{ artifacts.qa }}` was
376+
rewritten to the loop's exit — past a node literally named `qa`.
377+
- Each internal node is **namespaced before substitution**: its id, its
378+
`depends_on`, its `feedback.rerun` and every `{{ artifacts.<id> }}` /
379+
`{{ feedback.<id> }}` token it wrote. A value bound at the using site is
380+
inserted afterwards and is **never** rewritten — it belongs to the citing
381+
graph's namespace, and a bound artifact id that names no node there is a load
382+
error rather than a run-time surprise.
383+
- **Entry nodes** (no internal parent) inherit the using node's `depends_on`;
384+
`cwd`/`worktree` on the using node **propagate to every** spliced node.
385+
Entry-hood is decided by the key's *presence*, so an empty `depends_on: []`
386+
inside a fragment node is a load error rather than a silent opt-out that
387+
would start the node at the top of the citing graph.
388+
- From outside, the loop is one thing whose value is its exit's: both
389+
`depends_on: [round1]` and `{{ artifacts.round1 }}` resolve to
390+
`round1/<exit>`. `feedback: { rerun: round1 }` does **not** — it is a load
391+
error, because rewriting it to the exit would silently re-run one node for an
392+
author who asked to re-run a loop.
393+
- `exit:` is **required and never inferred** from the unique sink: inference is
394+
right only while there is exactly one sink, and when it is wrong it is wrong
395+
silently. It may not lie strictly inside one of the fragment's own feedback
396+
bodies, so no citing graph's downstream edge can manufacture a side exit in a
397+
fragment whose author wrote nothing wrong.
398+
- A multi-node `use:` may declare **wiring only** (`id`, `depends_on`, `cwd`,
399+
`worktree`, `with`): a behavior key on it is a load error naming the key,
400+
since there is no coherent way to overlay one node's `success_check` onto
401+
five. A loop needing different behavior needs a substitution point or a
402+
different fragment.
403+
404+
Non-goals, refused rather than deferred quietly: `use:` inside a fragment
405+
(nesting needs cycle detection over fragment *resolution*), `rerun:` over a
406+
whole loop, loop-until-dry convergence (`max: N` stays the only one), and
407+
dynamic fan-out.
408+
321409
Downstream of the loader **no fragment concept exists**: `run` and `lint` both
322410
print one disclosure line per resolved fragment (source file + the fragment's
323-
own description + every overridden key) plus the same fragment advisories on
411+
own description + every overridden key, or — for a multi-node splice, which
412+
overrides nothing — the ids it spliced) plus the same fragment advisories on
324413
the warning channel (`run` discloses what it spliced, so it discloses the
325414
drift smell too; the four *handoff* sweeps stay lint-only), the snapshot stores the re-encoded
326415
**resolved** graph whenever any node resolved a fragment (so resume never
@@ -333,8 +422,11 @@ alongside the templates (`//go:embed *.yaml fragments/*.yaml`), so
333422
it tops that tree up with payload files it does not have yet (a fragment added
334423
by a later release), keeping every file already there;
335424
`internal/graph/testdata/golden/` holds the resolved goldens — one per
336-
fragment-citing template (`self-dev`, `dev-review-pr`, `backlog-batch`) — that
337-
turn any fragment edit into a reviewed multi-template diff.
425+
fragment-citing template (`self-dev`, `dev-review-pr`, `backlog-batch`,
426+
`adr-driven-dev`) — that turn any fragment edit into a reviewed multi-template
427+
diff. A multi-node fragment multiplies that blast radius by its node count, on
428+
purpose: one edit to `repair-round` moves four nodes in `adr-driven-dev`'s
429+
golden, and the reviewer sees all four.
338430

339431
## Handoff — artifact default, session opt-in (committed)
340432
- **artifact (default):** engine persists each node's `.result` to
@@ -889,10 +981,13 @@ So a verdict pattern is written in two halves, and both are load-bearing:
889981
shipped prefix verdict carries the offer: *anything you need to qualify
890982
goes AFTER the verdict, never before it* — as one unbroken line, so
891983
`grep -c "Anything you need to qualify" graphs/*.yaml graphs/fragments/*.yaml`
892-
is a sweep that cannot silently miss a node. That sweep counts **26
984+
is a sweep that cannot silently miss a node. That sweep counts **24
893985
declarations, covering 33 runtime nodes** — a fragment states the clause
894-
once and every node citing it gets it, which is the point: three of the 26
895-
live in `graphs/fragments/` and carry ten of the nodes between them. The
986+
once and every node citing it gets it, which is the point: five of the 24
987+
live in `graphs/fragments/` and carry fourteen of the nodes between them.
988+
The gap widened by two when `adr-driven-dev`'s two repair rounds became two
989+
`use:` of one multi-node fragment (ADR 0027): the same 33 nodes, four fewer
990+
places to correct the sentence in. The
896991
four whole-reply pins
897992
(`haiku-smoke`'s `write`, the `e2e-verify` fragment, `apply-flags`'s
898993
`verify`, and `coordinator.plannedVerdictPattern`) say the opposite and must —

cmd/oh-my-graph/dryrun.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,15 @@ func printResolvedPlan(w io.Writer, g *graph.Graph) {
7878
// uses. Every node id is seeded with a placeholder artifact path first, and
7979
// any error still on the artifact side is skipped: artifacts materialize while
8080
// the run executes, so they are the one thing a static pass must not judge.
81+
//
82+
// The seeded placeholder path is the real writer's computation
83+
// (handoff.SanitizeNodeID), not a second spelling of it: a spliced node's id
84+
// carries a '/' (ADR 0027), and `node.ID+".out"` would print a path no run
85+
// ever writes.
8186
func inputIssues(g *graph.Graph, inputs map[string]string) []error {
8287
h := handoff.New("", inputs)
8388
for _, node := range g.Nodes {
84-
h.Seed(node.ID, node.ID+".out", "")
89+
h.Seed(node.ID, handoff.SanitizeNodeID(node.ID)+".out", "")
8590
}
8691

8792
var issues []error

cmd/oh-my-graph/fragment_cli_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,17 @@ func TestPrintFragmentResolutions_NamesEveryOverriddenKey(t *testing.T) {
173173
if want := ` — node overrides: success_check, retry`; !strings.HasSuffix(lines[1], want) {
174174
t.Errorf("line 1 = %q, want suffix %q", lines[1], want)
175175
}
176+
177+
// A multi-node resolution overrides nothing — the using node may declare
178+
// only wiring — so the same duty is discharged by naming what it spliced.
179+
var loop strings.Builder
180+
printFragmentResolutions(&loop, []graph.FragmentResolution{
181+
{NodeID: "qa-a", Fragment: "qa-loop", Description: "a repair loop", Source: "graphs/fragments/qa-loop.yaml",
182+
Spliced: []string{"qa-a/impl", "qa-a/review"}},
183+
})
184+
if want := ` — nodes: qa-a/impl, qa-a/review` + "\n"; !strings.HasSuffix(loop.String(), want) {
185+
t.Errorf("multi-node line = %q, want suffix %q", loop.String(), want)
186+
}
176187
}
177188

178189
// TestLintGraph_SurfacesTheFragmentDescription pins why description: is a

cmd/oh-my-graph/main.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,9 +1577,18 @@ func inputKeys(inputs inputFlag) []string {
15771577
// visible to whoever reads the file. The description is what makes the line
15781578
// readable without opening the fragment, which is why an empty one is a load
15791579
// error rather than an empty tail here. Silent for a fragment-free graph.
1580+
//
1581+
// A MULTI-NODE resolution (ADR 0027) overrides nothing — the using node may
1582+
// declare only wiring — so its line names the ids it spliced instead. That is
1583+
// the same disclosure duty in the shape the multi-node form takes: the reader
1584+
// of a run log learns that one `use:` became five nodes, and which five,
1585+
// without opening the fragment file.
15801586
func printFragmentResolutions(w io.Writer, resolutions []graph.FragmentResolution) {
15811587
for _, r := range resolutions {
15821588
line := fmt.Sprintf("fragment: node %q spliced from %q (%s) — %s", r.NodeID, r.Fragment, r.Source, r.Description)
1589+
if len(r.Spliced) > 0 {
1590+
line += " — nodes: " + strings.Join(r.Spliced, ", ")
1591+
}
15831592
if len(r.Overridden) > 0 {
15841593
line += " — node overrides: " + strings.Join(r.Overridden, ", ")
15851594
}

docs/RUN-FEED.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ the very same files, and an external consumer reads exactly what they read:
1111
state.json versioned atomic SNAPSHOT — whole-run state, overwritten after every node
1212
events.jsonl versioned append-only STREAM — one line per lifecycle transition
1313
<node-id>.out per-node artifact — EVERY non-gate node that passes, whatever its handoff
14+
(the filename is the SANITIZED id — see below; identical to the
15+
id for every id a human can write)
1416
graph.json the planned spec (auto runs only)
1517
rejected.json a REFUSED planner reply, kept because the call was paid for (ADR 0023 §3.1)
1618
assess.json the goal-cycle assessment verdict (iterated auto runs only — ADR 0011)
@@ -26,6 +28,19 @@ the one passing path, with no handoff branch). A consumer must not skip the
2628
`.out` beside a `handoff: session` node — it is there, and it holds that
2729
node's real result. A gate node spawns nothing and so has no `.out`.
2830

31+
**The filename is the node id with `/` (and this platform's own path
32+
separator) replaced by `~`** — one file per node, flat, in one directory. That
33+
rule is a no-op for every id an author or a planner can write: both are held to
34+
one path element, and `~` is outside the id character set. It exists for the
35+
ids a MULTI-NODE fragment splice mints, `<using-id>/<internal-id>` (ADR 0027),
36+
which cannot be a filename as written. The replacement is `~` rather than `_`
37+
precisely so the map is injective: with `_`, the distinct nodes `a` + `b_c`,
38+
`a_b` + `c` and a hand-written `a_b_c` would share one file, and a consumer
39+
reading it would be handed another node's result with nothing failing. A
40+
consumer applies the rule in one line, or reads
41+
`state.json`'s per-node `artifact_path`, which records the path the run
42+
actually wrote.
43+
2944
**A run directory may legitimately hold NEITHER `graph.json` NOR `state.json`,
3045
and this is not damage** (ADR 0023). Two shapes reach it. While an `auto` run is
3146
inside its planner call, its directory holds only `resume.lock` and a one-line

0 commit comments

Comments
 (0)