Merge pull request 'Issue 309: two order rules ordered one pair both ways; ADR 0249 gives them a precedence' (#194) from issues/309-two-order-rules-ordered-one-pair-both-ways into main

2026-10-08 09:11:45 +00:00
parent c6e76015b7 a3bedeb227
commit b11219e3de
5 changed files with 224 additions and 1 deletions
@@ -11,6 +11,14 @@ extends: 02-DECISIONS/0238-a-commit-is-the-build-at-hand-one-commit-one-change-p

 # 239. A delivery is owned by the mesh-delivery module and runs from commit to delivered

+> **Superseded in part — 2026-10-08, by [ADR 0249](0249-a-delivery-groups-order-rules-have-a-precedence-and-a-declared-order-outranks-them-all.md).**
+> Decision 4's sentence "a declared order that contradicts an inferred one is a cycle" no longer holds.
+> Two rules ordering one pair both ways were always a cycle, and a group of the controller, the node-engine
+> and the lab was refused for it (issue 309). The order rules now have a precedence: declared, then built
+> by and version skew, then engine before controller. The higher rule decides the pair and says what it
+> won over. Only rules of one rank in opposite directions are refused, naming both. Everything else here
+> stands.
+
 ## Context

 [ADR 0238](0238-a-commit-is-the-build-at-hand-one-commit-one-change-plan-checked-off-the-trunk-and-published-only-on-it.md)
@@ -0,0 +1,99 @@
+---
+topic: the mesh
+status: accepted
+date: 2026-10-08
+deciders: jochen
+reconstructed: false
+supersedes-in-part:
+  - 0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md
+extends: 02-DECISIONS/0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md
+---
+
+# 249. A delivery group's order rules have a precedence, and a declared order outranks them all
+
+## Context
+
+[ADR 0239](0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md)
+decision 4 orders a delivery group's members by a line `after: <repository>` and by three inferred rules:
+*built by* (a member that moves a module goes before a member whose modules are built by it or stand on
+it), *version skew* (the controller before a manifest change) and *engine before controller* (the
+node-engine before the controller, from ADR 0236's order for its walks). It says nothing about two rules
+ordering one pair in opposite directions. The planner treated every such pair as a cycle, and a declared
+order against an inferred one as a cycle too.
+
+On 2026-10-08 the group `feat/a-machine-joins-through-the-tunnel` held three pull requests, each `ready`
+on its own: the controller's, the node-engine's and the lab's
+([issue 309](../04-ISSUES/309-two-order-rules-ordered-one-pair-both-ways/00-report.md)). The controller's
+repository also holds the build agent, and its pull request moved it. Every source-built module is built
+by the build agent, so *built by* put the controller's member first. *Engine before controller* put the
+node-engine's member first. The group was rejected: "its order contradicts itself", naming the two
+members and neither rule. The person merged it by hand, the controller first, since that member only added
+a verb the new node-engine calls. That is the order *built by* gave, and the order
+[ADR 0246](0246-a-seats-new-verb-is-promised-before-it-is-required.md) asks for: a verb is promised before
+it is required.
+
+The rules are not of one kind. *Built by* and *version skew* are read from the graph and from what the
+change does: which module builds which, and whether a manifest changes. *Engine before controller* is
+read from two module names alone. It holds when the controller starts sending something the node-engine
+must first accept. It is wrong when the node-engine starts asking for something the controller must first
+serve, as here. The rule cannot tell the two apart.
+
+## Considered Options
+
+1. **A precedence among the rules, and a declared order above them all.** Chosen.
+2. **Narrow *built by* so it never applies through the build agent to the node-engine.** It would end
+   this case. But a builder that changes how the node-engine is built does have to go first, and the next
+   pair of rules that disagree would be refused the same way. Rejected.
+3. **Drop *engine before controller*.** ADR 0236's reason still holds when the node-engine is what must
+   accept something new. Rejected.
+4. **Keep refusing, and name the two rules.** Then every group that moves the controller's repository and
+   the node-engine together is refused, and the person must still merge by hand. A declared order could not
+   help, because a declared order against an inferred one was also a cycle. Rejected.
+
+## Decision
+
+1. **When rules order one pair of members in opposite directions, the rule of higher precedence decides.**
+   From highest to lowest:
+   - *declared*: an `after:` line a person wrote;
+   - *built by* and *version skew*: what the graph and the change say;
+   - *engine before controller*: the default for walks, read from module names alone.
+2. **A declared order outranks every inferred rule.** This replaces ADR 0239 decision 4's sentence "a
+   declared order that contradicts an inferred one is a cycle". A person who knows the order the rules
+   cannot see writes one line, and the group follows it.
+3. **The order says which rule won.** Each pair carries its rule and the rules it won over. The delivery's
+   `show`, `groups` and plan note print them, for example "the controller's member before the node-engine's:
+   built by, over engine before controller".
+4. **Rules of one rank in opposite directions are still refused.** Two `after:` lines naming each other,
+   or *built by* both ways, have no precedence between them. The group is rejected. The reason names both
+   members, both rules and the direction each gives, and says how to declare the order. A cycle through
+   three members or more is refused naming the pairs in it.
+
+## Consequences
+
+- The group of issue 309 is ordered without a person: the controller's member first.
+- A group can no longer be refused because a person's `after:` disagrees with an inferred rule. The person
+  can now overrule *version skew*, and a manifest change merged before the controller that parses it fails
+  at the gate, not silently. The rule the person overruled is printed on the pair.
+- *Engine before controller* now decides only where nothing in the graph or the pull requests does.
+- The ranks are a list in the controller's planner. A new inferred rule must be given a rank when it is
+  added.
+
+**How it is checked.** The controller's tests `TestAnOrderRuleYieldsToAStrongerOne` and
+`TestAGroupIsOrderedByTheGraphAndWhatItsPullRequestsSay` check that *built by* wins over *engine before
+controller*, that a declared line wins over both and over *version skew*, that each pair names what it won
+over, and that equal-rank rules both ways are refused with both rules named and the `after:` lines to
+write. mesh-delivery's `TestAGroupSaysWhichOrderRuleWonOrWhyNoneDid` and
+`TestACycleWithoutAContradictionNamesItsPairs` check that the refusal and the won-over rules reach
+`groups` and the group's state. The replay R309 in mesh-lab's register fails before the fix and passes on it.
+
+## References
+
+- [Issue 309](../04-ISSUES/309-two-order-rules-ordered-one-pair-both-ways/00-report.md), its report and
+  diagnosis.
+- [ADR 0239](0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md)
+  decision 4, the sentence this replaces.
+- [ADR 0236](0236-a-build-is-judged-on-its-first-machine-and-put-back-by-something-other-than-itself-and-so-it-rolls-out-unattended.md),
+  *Rollout order*, where *engine before controller* comes from.
+- [ADR 0246](0246-a-seats-new-verb-is-promised-before-it-is-required.md), a verb promised before it is
+  required.
+- [Design 47](../03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md), *A delivery group*.
@@ -2,8 +2,9 @@
 layer: to-be
 status: designed
 code: []
-updated: 2026-10-07
+updated: 2026-10-08
 decisions:
+  - 02-DECISIONS/0249-a-delivery-groups-order-rules-have-a-precedence-and-a-declared-order-outranks-them-all.md
  - 02-DECISIONS/0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md
  - 02-DECISIONS/0238-a-commit-is-the-build-at-hand-one-commit-one-change-plan-checked-off-the-trunk-and-published-only-on-it.md
  - 02-DECISIONS/0236-a-build-is-judged-on-its-first-machine-and-put-back-by-something-other-than-itself-and-so-it-rolls-out-unattended.md
@@ -93,6 +94,10 @@ controller's `plan-moved` event, never inferred from time.
 - **Order**: `after: <repository>` lines in a member's description, and the controller's `delivery-order`
  over the graph (ADR 0239 decision 4). The plan shows the order and why each pair is ordered: *declared*,
  *built by*, *version skew*, *engine before controller*, *by name*.
+- **Precedence** (ADR 0249): when rules order one pair both ways, the higher one decides. *Declared* comes
+  first, then *built by* and *version skew*, then *engine before controller*. The pair shows the rule that
+  won and the rules it won over. Rules of one rank both ways are a contradiction. The group is `rejected`,
+  and the reason names both members, both rules and the `after:` line that would settle it.
 - **Check**: the members' heads composed together, one gate over every machine with all their definitions. It
  is asked by mesh-delivery through `delivery-check` when the group forms or a member's head moves, and
  posted on every member's head as `mesh/delivery-group`.
@@ -0,0 +1,74 @@
+---
+status: resolved
+opened: 2026-10-08
+located-in: [mesh-controller cmd/mesh-controller (delivery.go, orderOf), mesh-catalog modules/mesh-delivery (holder.go, the group's state)]
+fixed-by: novox/mesh-controller PR #134, novox/mesh-catalog PR #119
+replay: R309
+amended-design: 03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md
+---
+
+# 309. Two order rules ordered one pair both ways
+
+## Symptom
+
+On 2026-10-08 the delivery group `feat/a-machine-joins-through-the-tunnel` held three pull requests: the
+controller's mesh-controller #132, the node-engine's `mesh-host` #52 and the lab's mesh-lab #61. Each was
+`ready` on its own. The delivery's `show` said of the group:
+
+- `"state": "rejected"`;
+- `"why": "its order contradicts itself: novox/mesh-controller@…, novox/mesh-host@…"`.
+
+The group's `pairs` held two orderings of the same two members:
+
+- the controller's member before the node-engine's, `why: "built by"`;
+- the node-engine's member before the controller's, `why: "engine before controller"`.
+
+Nothing would ever deliver the group. The person merged it by hand, the controller first, since that
+member only added a verb the new node-engine calls.
+
+## Why it is a design issue
+
+[ADR 0239](../../02-DECISIONS/0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md)
+decision 4 states four order rules and says what to do when a declared order contradicts an inferred
+one. It does not say what to do when two inferred rules contradict each other. The planner treated that
+as a cycle, so a group that moved the controller's repository and the node-engine together could never be
+delivered. The refusal named two members and no rule, so the person could not tell what had clashed or
+what to write. A declared `after:` line could not help, because a declared order against an inferred one
+was a cycle too.
+
+## Cause
+
+The controller's pull request moved the build agent, which lives in the controller's repository. Every
+source-built module is built by the build agent, including the node-engine. *Built by* therefore put the
+controller's member first. *Engine before controller* put the node-engine's member first. Both were
+added as pairs, and the planner's sort found no member with nothing before it.
+
+## Fix
+
+[ADR 0249](../../02-DECISIONS/0249-a-delivery-groups-order-rules-have-a-precedence-and-a-declared-order-outranks-them-all.md)
+gives the rules a precedence: *declared*, then *built by* and *version skew*, then *engine before
+controller*.
+
+- **The controller (novox/mesh-controller PR #134).** `orderOf` collects each rule's claim, and for each
+  two members the claims of the highest rank decide. The pair carries the rules it won over. Claims of one
+  rank in opposite directions are a contradiction. Both pairs are kept, the members are the cycle, and the
+  contradiction names both rules, the member each puts first and the `after:` line that settles it.
+- **mesh-delivery (novox/mesh-catalog PR #119).** It keeps the rules each pair won over and the
+  contradictions, lists them in `groups` and the plan note, and refuses a group in the contradiction's
+  words. A cycle through three members or more names the pairs in it. An older controller's answer reads
+  as before.
+
+Replayed, the group is ordered: the controller's member, then the node-engine's, then the lab's. The pair
+reads "built by, over engine before controller". That is the order the person chose by hand.
+
+## How it is checked
+
+**The check:** the replay R309 in mesh-lab's register (novox/mesh-lab PR #64), `TestReplay309` in the
+controller. It replays this group's members, reach and graph using only what `orderOf` had before the
+fix. Proved with the register's `cmd/prove`: it fails on the commit before the fix (the group refused as
+a cycle) and passes on it. The controller's `TestAnOrderRuleYieldsToAStrongerOne` checks the precedence
+and the refusal's words. mesh-delivery's `TestAGroupSaysWhichOrderRuleWonOrWhyNoneDid` checks that they
+reach `groups` and the group's state.
+
+**Not checked:** that a new inferred rule is given a rank. One added without a rank sorts below every other
+rule, and nothing refuses that.
@@ -0,0 +1,37 @@
+# 309 — diagnosis
+
+## 2026-10-08
+
+- The delivery's `show` for the group gave both pairs and `cycle` with the controller's and the
+  node-engine's members. The lab's member was ordered alone, since it moves no module.
+- The pairs come from the controller's `delivery-order` verb, `orderOf` in
+  `cmd/mesh-controller/delivery.go`. mesh-delivery only stores them and says "its order contradicts
+  itself" with the cycle's ids (`GroupState` in `holder.go`). So the order is the controller's to fix, and
+  the words are mesh-delivery's.
+- *Built by*: `orderOf` adds a pair for any edge of the dependency relation from a module the second
+  member moves to one the first moves. The controller's plan for #132 moved `build-agent`,
+  `mesh-controller` and `route-proxy`. The relation (`inventory.dependenciesOf`) gives every source-built
+  module an edge `built-by` to the build seat's holder, so `mesh-host → build-agent` ordered the
+  controller's member first. The pair was labelled *built by* although the edge was the build agent's, not
+  the controller's.
+- *Engine before controller*: the node-engine's member moved `mesh-host`, the controller's moved
+  `mesh-controller`, and the node-engine's did not move the controller. The rule put the node-engine's
+  member first.
+- `add` dropped a pair only when the same direction was already there, so both directions stood and
+  the planner's sort stalled. Every two-way pair was a cycle, whatever the rules.
+- The person's order, the controller first, was right for this change. The new node-engine calls a verb
+  the new controller adds, which is ADR 0246's case: a verb is promised before it is required.
+  *Engine before controller* comes from ADR 0236's order for its walks, where the node-engine had to
+  accept something new first. The rule reads only module names, so it cannot tell the two cases apart. *Built
+  by* is read from the graph.
+
+**Ruled out.**
+
+- That the relation was wrong. The node-engine is source-built and is built by the build agent.
+- Narrowing *built by* so it never applies through the build agent. A builder change can matter to what
+  it builds, and the next pair of disagreeing rules would be refused the same way.
+- Leaving it to a declared `after:` line alone. ADR 0239 made a declared order against an inferred one a
+  cycle too, so no line could settle it.
+
+**Decided.** Precedence, with a declared order above all inferred rules (ADR 0249). That supersedes the one
+sentence of ADR 0239 decision 4 that made a declared order against an inferred one a cycle.