diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 9b06b742..98088518 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "github.com/novox/mesh-controller/internal/broker" + "io" "slices" "sort" "strings" @@ -54,6 +55,9 @@ func assignWith(ctx context.Context, open *stores, node string, opts assignOptio if err := refusedMovingTheController(modules); err != nil { return "", err } + // Every line of the answer comes from the one resolution after the write (novox/hq issue 237): + // what the reads before it would say about a machine left out is not the answer's. + ctx = leftOutSaidBy(ctx, io.Discard) // Held while it is recorded, so it cannot land between a converge's preview and its flip and // be taken without ever having been previewed (novox/hq ADR 0100). ctx, release, err := holdNodes(ctx, open, []string{node}) @@ -125,22 +129,28 @@ func assignWith(ctx context.Context, open *stores, node string, opts assignOptio for _, line := range settled { answer += "\n " + line } - // What this act changed about this node's unmet seat dependencies, and nothing else (novox/hq - // ADR 0207): a dependency of a module just assigned, or one this assignment met. The rest of the - // node's list, and every other node's, is `status`'s. - for _, line := range unheldChange(shelf, node, before, append(append([]string(nil), before...), added...)) { - answer += "\n " + line - } // Its bus credential, in the same act (novox/hq issue 203): an assignment pushed before its // credential exists delivers a process that cannot authenticate and crash-loops until somebody // runs a second verb and a second push. Issued here when the module speaks on the bus and has // no credential yet; kept when it has one, so re-assigning rotates nothing. + var issued []string for _, module := range added { if line := issueOnAssign(ctx, open, node, module); line != "" { - answer += "\n " + line + issued = append(issued, line) } } - plan, _, err := planFor(ctx, open, node) + // **The one resolution the rest of the answer is said from** (novox/hq issue 237), taken after + // the write: what this act changed about this node's unmet seat dependencies, and nothing else + // (novox/hq ADR 0207) — a dependency of a module just assigned, or one this assignment met; the + // rest of the node's list, and every other node's, is `status`'s — and whether the node resolves. + after := resolvedAfter(ctx, open, shelf, node) + for _, line := range unheldChangeTo(shelf, node, before, after.modules, after.unheld) { + answer += "\n " + line + } + for _, line := range issued { + answer += "\n " + line + } + plan, err := after.plan, after.err if err != nil { // Kept, and still refused. Both halves are the answer, and the rest of the mesh is still // worth reporting: this machine's refusal is rarely the only consequence. @@ -218,6 +228,8 @@ func unassign(ctx context.Context, open *stores, node string, modules ...string) if len(modules) == 0 { return "", fmt.Errorf("unassign %s names no module", node) } + // One resolution after the write, as assign's answer (novox/hq issue 237). + ctx = leftOutSaidBy(ctx, io.Discard) ctx, release, err := holdNodes(ctx, open, []string{node}) if err != nil { return "", err @@ -262,15 +274,22 @@ func unassign(ctx context.Context, open *stores, node string, modules ...string) } answer := fmt.Sprintf("%s no longer runs %s — run `push %s` to make it so", node, strings.Join(modules, ", "), node) - var left []string - for _, a := range assigned { - if !slices.Contains(modules, a) { - left = append(left, a) - } - } - for _, line := range unheldChange(shelf, node, assigned, left) { + after := resolvedAfter(ctx, open, shelf, node) + for _, line := range unheldChangeTo(shelf, node, assigned, after.modules, after.unheld) { answer += "\n " + line } + // Whether the node itself still resolves, from the same resolution: before, only the rest of the + // mesh's reads said it, on standard error and once per other machine. + if after.err != nil { + if unresolvable(after.err) { + answer += fmt.Sprintf("\n but %s does not resolve now, so nothing will be sent to it:", node) + } else { + answer += fmt.Sprintf("\n but whether %s still resolves could not be read:", node) + } + for _, line := range strings.Split(strings.TrimRight(after.err.Error(), "\n"), "\n") { + answer += "\n " + strings.TrimSpace(line) + } + } // What it leaves behind that is irreplaceable is kept and retired, never removed (novox/hq ADR 0233). for _, line := range keptOnUnassign(ctx, open.inventory, node, modules) { answer += "\n " + line @@ -278,6 +297,39 @@ func unassign(ctx context.Context, open *stores, node string, modules ...string) return answer + blockedElsewhere(ctx, open, node), nil } +// afterAct is the one resolution of a node an act's answer is said from, taken after its write +// (novox/hq issue 237): the plan or why there is none, the modules it has on the node, and its unmet +// seat dependencies. +type afterAct struct { + plan catalogue.Resolution + err error + modules []string + unheld []catalogue.Unheld +} + +// resolvedAfter resolves the node as `plan` does, once, after the act's write. When the node resolves, +// its modules and unmet seat dependencies are the resolution's — the closure, so a holder pulled in by +// a requirement counts as `plan` counts it. When it does not, they are judged over the assignments read +// after the write, which can only find more unmet than the closure would: a seat said held there is +// held in the closure too. +func resolvedAfter(ctx context.Context, open *stores, shelf map[string]catalogue.Manifest, node string) afterAct { + plan, _, err := planFor(ctx, open, node) + if err == nil { + out := afterAct{plan: plan, unheld: plan.Unheld} + for _, m := range plan.Modules { + out.modules = append(out.modules, m.Module) + } + return out + } + out := afterAct{err: err} + assigned, readErr := open.inventory.Assigned(ctx, node) + if readErr != nil { + return out + } + out.modules, out.unheld = assigned, unheldOver(shelf, node, assigned) + return out +} + // splitModules is a surface's one `module` field as the modules it names: several, comma-separated, // are one act (novox/hq ADR 0207), so the holders that depend on each other go on together from the // command API and the controller seat's verbs as they do from the command line. diff --git a/cmd/mesh-controller/one_resolution_test.go b/cmd/mesh-controller/one_resolution_test.go new file mode 100644 index 00000000..c49b91c6 --- /dev/null +++ b/cmd/mesh-controller/one_resolution_test.go @@ -0,0 +1,112 @@ +package main + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// novox/hq issue 237: every line of an act's answer comes from one resolution of the node taken after +// its write, so the answer agrees with `plan` for the node right after. + +func aProviderAndItsConsumer(t *testing.T, open *stores) { + t.Helper() + register(t, open, catalogue.Manifest{Module: "db", Version: "1", + Provides: []catalogue.Offer{{Name: "rows", Scope: catalogue.ScopeNode}}}) + register(t, open, catalogue.Manifest{Module: "app", Version: "1", Requires: []string{"rows"}}) +} + +// twoRivalsAndANote is a node that does not resolve — two modules claim one node seat there — and a +// module beside them that changes nothing about it. +func twoRivalsAndANote(t *testing.T, open *stores) { + t.Helper() + one, two := rivals() + register(t, open, one) + register(t, open, two) + register(t, open, catalogue.Manifest{Module: "notes", Version: "1"}) +} + +func TestAnUnassignThatLeavesItsNodeUnresolvedSaysSoOnceInItsAnswer(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + twoRivalsAndANote(t, open) + // Straight into the store, as a node that came to this before the act. + for _, m := range []string{"rival-one", "rival-two", "notes"} { + if _, err := open.inventory.Assign(ctx, "laptop", m); err != nil { + t.Fatal(err) + } + } + + said := answered(t, func() error { return assignCommand(ctx, "unassign", []string{"laptop", "notes"}) }) + + if _, _, err := planFor(ctx, open, "laptop"); err == nil || !unresolvable(err) { + t.Fatalf("laptop resolves with two claimants of one seat: %v", err) + } + if n := strings.Count(said, "laptop does not resolve now"); n != 1 { + t.Errorf("the answer says laptop does not resolve %d times, want once:\n%s", n, said) + } + if strings.Contains(said, "is left out of the rest of the mesh") { + t.Errorf("the answer carries a line from a read that is not its resolution:\n%s", said) + } +} + +func TestAnUnassignThatLeavesItsNodeResolvedSaysNothingOfIt(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + aProviderAndItsConsumer(t, open) + if _, err := assign(ctx, open, "laptop", "db", "app"); err != nil { + t.Fatal(err) + } + said, err := unassign(ctx, open, "laptop", "app") + if err != nil { + t.Fatal(err) + } + if _, _, err := planFor(ctx, open, "laptop"); err != nil { + t.Fatal(err) + } + if strings.Contains(said, "does not resolve") || strings.Contains(said, "could not be read") { + t.Errorf("a node that resolves is said not to:\n%s", said) + } +} + +func TestAnAssignThatLeavesItsNodeUnresolvedSaysOnlyThePlansRefusal(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + twoRivalsAndANote(t, open) + if _, err := assign(ctx, open, "laptop", "rival-one"); err != nil { + t.Fatal(err) + } + // Kept and not resolvable: a second claimant of the seat, which assignment keeps. + said, err := assign(ctx, open, "laptop", "rival-two") + if err == nil { + t.Fatalf("two claimants of one node seat resolved:\n%s", said) + } + _, _, planErr := planFor(ctx, open, "laptop") + if planErr == nil || planErr.Error() != err.Error() { + t.Errorf("the act's refusal is not the plan's right after:\nact: %v\nplan: %v", err, planErr) + } + if strings.Contains(said, "now has") { + t.Errorf("the answer says a seat is now held on a node that does not resolve:\n%s", said) + } +} + +// The other readers of the whole mesh still say a machine they leave out, on standard error (novox/hq +// issue 188); an act sends it elsewhere. +func TestAMachineLeftOutIsSaidWhereTheContextSays(t *testing.T) { + var buf bytes.Buffer + sayLeftOut(leftOutSaidBy(context.Background(), &buf), "laptop", errors.New("why")) + if buf.String() != "laptop is left out of the rest of the mesh: it does not resolve: why\n" { + t.Errorf("said %q", buf.String()) + } + got := answered(t, func() error { + sayLeftOut(context.Background(), "laptop", errors.New("why")) + return nil + }) + if !strings.Contains(got, "laptop is left out") { + t.Errorf("without an act's writer the line did not reach standard error: %q", got) + } +} diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index b4811f4e..3fd360c7 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -292,7 +292,7 @@ func theRestOfTheMesh(ctx context.Context, inv *inventory.Inventory, if err != nil { // Said, not skipped: a machine dropped here offers nothing and holds nothing as far // as every other machine's plan can tell (novox/hq issue 188). - fmt.Fprintf(os.Stderr, "%s is left out of the rest of the mesh: it does not resolve: %v\n", o.node.Name, err) + sayLeftOut(ctx, o.node.Name, err) continue } firstHeld = append(firstHeld, got.Claims...) @@ -337,7 +337,7 @@ func theRestOfTheMesh(ctx context.Context, inv *inventory.Inventory, // resolved without its offers, and one that consumes them cannot resolve here by // design — that is not the machine being dropped, it is the view being partial. if exclude == "" { - fmt.Fprintf(os.Stderr, "%s is left out of the rest of the mesh: it does not resolve: %v\n", o.node.Name, err) + sayLeftOut(ctx, o.node.Name, err) } continue } @@ -347,6 +347,35 @@ func theRestOfTheMesh(ctx context.Context, inv *inventory.Inventory, return world, nil } +// leftOutKey carries where theRestOfTheMesh says a machine it leaves out, for an act that answers with +// its own resolution instead (novox/hq issue 237). +type leftOutKey struct{} + +// leftOutSaidBy is ctx with theRestOfTheMesh's "left out" lines written to w rather than to standard +// error. +// +// **An act's answer is one resolution taken after its write** (novox/hq issue 237, decided 2026-10-11). +// `assign` read the whole mesh once before its write — to record the seats held by derivation — and that +// read said, on standard error, that the node did not resolve for lack of the seat the act was about to +// give it. The controller seat's verb answers with both streams, so the stale line came last, after the +// act's own "now has it held", and named as its remedy the assignment just made. An act gives io.Discard +// here: what the node resolves to after the write is said by its own planFor, and every other machine +// that does not resolve by blockedElsewhere, both read after the write. +func leftOutSaidBy(ctx context.Context, w io.Writer) context.Context { + return context.WithValue(ctx, leftOutKey{}, w) +} + +// sayLeftOut says that a machine is left out of the rest of the mesh, where ctx says to: standard +// error unless an act said otherwise. Said, not skipped: a machine dropped there offers nothing and +// holds nothing as far as every other machine's plan can tell (novox/hq issue 188). +func sayLeftOut(ctx context.Context, node string, err error) { + w, ok := ctx.Value(leftOutKey{}).(io.Writer) + if !ok { + w = os.Stderr + } + fmt.Fprintf(w, "%s is left out of the rest of the mesh: it does not resolve: %v\n", node, err) +} + // declarationFor is everything a node would be sent. // // One place, because there were three and one of them was written before credentials existed and @@ -1751,28 +1780,43 @@ func (l *unheldLogger) log(node string, unheld []catalogue.Unheld) { // which includes every one of a module just assigned — and each now met that was not. Nothing about // any other node, and nothing that was already true before the act. func unheldChange(shelf map[string]catalogue.Manifest, node string, before, after []string) []string { - judge := func(names []string) map[string]catalogue.Unheld { - var set []catalogue.Manifest - for _, n := range names { - if m, known := shelf[n]; known { - set = append(set, m) - } + return unheldChangeTo(shelf, node, before, after, unheldOver(shelf, node, after)) +} + +// unheldOver is the node's unmet seat dependencies judged over a set of module names alone. +func unheldOver(shelf map[string]catalogue.Manifest, node string, names []string) []catalogue.Unheld { + var set []catalogue.Manifest + for _, n := range names { + if m, known := shelf[n]; known { + set = append(set, m) } - out := map[string]catalogue.Unheld{} - for _, u := range catalogue.UnheldDependencies(shelf, node, set, nil) { - out[u.Module+" "+u.Seat] = u - } - return out } - was, now := judge(before), judge(after) + return catalogue.UnheldDependencies(shelf, node, set, nil) +} + +// unheldChangeTo is unheldChange with the node's dependencies as they now stand given: `now` is what +// the act's one resolution after its write says is unmet, and `after` the modules that resolution has +// on the node (novox/hq issue 237). A line about how things stand after the act is never judged from +// anything else, so it cannot disagree with `plan` for the node. +func unheldChangeTo(shelf map[string]catalogue.Manifest, node string, before, after []string, + now []catalogue.Unheld) []string { + key := func(u catalogue.Unheld) string { return u.Module + " " + u.Seat } + was := map[string]catalogue.Unheld{} + for _, u := range unheldOver(shelf, node, before) { + was[key(u)] = u + } + is := map[string]catalogue.Unheld{} + for _, u := range now { + is[key(u)] = u + } var lines []string - for _, k := range sortedNames(now) { + for _, k := range sortedNames(is) { if _, already := was[k]; !already { - lines = append(lines, "but "+now[k].String()) + lines = append(lines, "but "+is[k].String()) } } for _, k := range sortedNames(was) { - if _, still := now[k]; still { + if _, still := is[k]; still { continue } u := was[k] diff --git a/cmd/mesh-controller/replay237_test.go b/cmd/mesh-controller/replay237_test.go new file mode 100644 index 00000000..85f7c071 --- /dev/null +++ b/cmd/mesh-controller/replay237_test.go @@ -0,0 +1,85 @@ +package main + +import ( + "bytes" + "io" + "os" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// TestReplay237 replays novox/hq issue 237 (2026-10-04): a workstation already ran a module that +// depends on a node seat nothing there held, and the module holding that seat was assigned to it. The +// answer said the seat was now held, and then that the workstation was left out of the rest of the mesh +// for lack of it, naming as the remedy the assignment just made. `plan` for the node, right after, +// resolved. The outcome asserted: every line of the answer, as the controller seat's verb gives it +// (standard output, then standard error), agrees with `plan` for the node right after the act. Written +// with only what the controller had before its fix — register, assign's command, planFor — so it is laid +// over the commit before. +func TestReplay237(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, aContainer("web")) + register(t, open, catalogue.Manifest{Module: "docker", Version: "1", + Claims: []catalogue.Claim{{Name: catalogue.ContainerRuntimeSeat, Scope: catalogue.ScopeNode}}}) + // Already on the node, from before the rule refused it: the node does not resolve until the + // holder comes. + if _, err := open.inventory.Assign(ctx, "laptop", "web"); err != nil { + t.Fatal(err) + } + + said := answered(t, func() error { return assignCommand(ctx, "assign", []string{"laptop", "docker"}) }) + + if _, _, err := planFor(ctx, open, "laptop"); err != nil { + t.Fatalf("laptop does not resolve right after the act: %v\nthe answer was:\n%s", err, said) + } + if !strings.Contains(said, "web on laptop now has "+catalogue.ContainerRuntimeSeat+" held") { + t.Errorf("the answer does not say the seat is now held:\n%s", said) + } + for _, not := range []string{"laptop is left out", "does not resolve", "which nothing on laptop holds"} { + if strings.Contains(said, not) { + t.Errorf("the answer says %q, which `plan` right after contradicts:\n%s", not, said) + } + } +} + +// answered is what a command says as the controller seat's verb answers it: standard output, then +// standard error (answerOf). +func answered(t *testing.T, f func() error) string { + t.Helper() + read := func(w **os.File) (func() string, error) { + old := *w + r, pw, err := os.Pipe() + if err != nil { + return nil, err + } + *w = pw + got := make(chan string) + go func() { + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + got <- buf.String() + }() + return func() string { + _ = pw.Close() + *w = old + return <-got + }, nil + } + stdout, err := read(&os.Stdout) + if err != nil { + t.Fatal(err) + } + stderr, err := read(&os.Stderr) + if err != nil { + t.Fatal(err) + } + runErr := f() + said := stdout() + stderr() + if runErr != nil { + t.Fatalf("%v\n%s", runErr, said) + } + return said +} diff --git a/testdata/store-tests b/testdata/store-tests index d60bca8d..67d064c8 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -22,3 +22,7 @@ ./cmd/mesh-controller TestAWalkEndedByCancelOrClearIsSaidEnded ./cmd/mesh-controller TestAWalkRestartedByARebuildIsSaidStarted ./cmd/mesh-controller TestAWalkStoppedByHandAndRetriedEndsFailedWhenItFailsAgain +./cmd/mesh-controller TestReplay237 +./cmd/mesh-controller TestAnUnassignThatLeavesItsNodeUnresolvedSaysSoOnceInItsAnswer +./cmd/mesh-controller TestAnUnassignThatLeavesItsNodeResolvedSaysNothingOfIt +./cmd/mesh-controller TestAnAssignThatLeavesItsNodeUnresolvedSaysOnlyThePlansRefusal