From ef41fa2e2796f7f60035677442031886fe53204c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 13:04:55 +0100 Subject: [PATCH] The node-engine's live path holds a module whose claim or credential fails (issue 1966, review round 1) compose.CompositionsHolding returns the modules the declaration leaves out for a claim that cannot be held or a refused credential; frombus composeEach holds each with those words, so the node-engine keeps what it handed of the module (no stop of a running module), the candidate answer and the check's sandbox name it refused, and the rest composes. CredentialsNeeded asks nothing for a held module. Resolve and checkClaims move to a test helper (production uses ResolveHolding and holdClaims); a pass refused anyway names a held module's reason among the others. Quick fix: Declared.Held read in the composer only. Thorough, chosen: the same held words reach every reader of the composition (selfcompose, ComposeSent, the candidate answer) through composeEach's one hold path. Claude-Session: https://claude.ai/code/session_01NcbgfFfQ4Z9UE5jK32PmE5 --- internal/compose/composed.go | 43 +++++++++++-------- internal/compose/credentials_needed.go | 4 +- internal/compose/frombus/answer.go | 15 ++++++- internal/compose/frombus/held_rows_test.go | 32 +++++++++++++++ internal/compose/node.go | 5 ++- internal/compose/resolution/resolve.go | 48 ++++++---------------- internal/compose/resolution/strict_test.go | 35 ++++++++++++++++ 7 files changed, 125 insertions(+), 57 deletions(-) create mode 100644 internal/compose/resolution/strict_test.go diff --git a/internal/compose/composed.go b/internal/compose/composed.go index e976e04..5bdc1c0 100644 --- a/internal/compose/composed.go +++ b/internal/compose/composed.go @@ -28,7 +28,7 @@ import ( // writer folds them (issue 1126). Each module's Data says whether its manifest declares data. The compositions are unsigned // (ADR 0593 §6) and not yet by digest (Composed.ByDigest). func Compositions(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped []string, err error) { - out, skipped, _, err = compositions(in, node, fps, mode, false) + out, skipped, _, _, err = compositions(in, node, fps, mode, false) return out, skipped, err } @@ -45,7 +45,7 @@ const BusUsersStandIn = "the bus's user list" // ErrStoodIn, naming each, so a stand-in is never applied as if it were the value; but for the bus's user list // (BusUsersStandIn), which the bus node holds out of every apply of a composition it made itself. func CompositionsApplied(in Input, node string, fps map[string]string) (map[string]engcomp.Composed, []string, error) { - out, skipped, _, err := compositions(in, node, fps, Reading, true) + out, skipped, _, _, err := compositions(in, node, fps, Reading, true) return out, skipped, err } @@ -56,13 +56,22 @@ func CompositionsApplied(in Input, node string, fps map[string]string) (map[stri // module the candidate adds, or one missing for a module the node runs. A module whose every value is held composes // as CompositionsApplied composes it: the modes differ only in an own secret never issued. func CompositionsShadow(in Input, node string, fps map[string]string) (out map[string]engcomp.Composed, skipped, stood []string, err error) { - return compositions(in, node, fps, Foreseeing, false) + out, skipped, stood, _, err = compositions(in, node, fps, Foreseeing, false) + return out, skipped, stood, err } // CompositionsIn is the node's compositions in mode with what they stand in for answered (stood, sorted, the bus's user // list left out) rather than refused: CompositionsShadow in Foreseeing, and in Reading what CompositionsApplied refuses // before it is refused, so a caller that applies refuses each stand-in to the module it is for alone (issue 1126). func CompositionsIn(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped, stood []string, err error) { + out, skipped, stood, _, err = compositions(in, node, fps, mode, false) + return out, skipped, stood, err +} + +// CompositionsHolding is CompositionsIn with the modules the node leaves out as they run, each with why: one whose claim cannot be +// held or whose credential was refused (issue 1966). Their compositions are not in out, so a caller that applies keeps what it last +// applied of each and says it held, as for any module refused alone. +func CompositionsHolding(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped, stood []string, held map[string]string, err error) { return compositions(in, node, fps, mode, false) } @@ -75,7 +84,7 @@ func Folds(inOwn bool, fp string) bool { return inOwn && !strings.HasPrefix(fp, meshcatalogclient.SourcePrefix2) } -func compositions(in Input, node string, fps map[string]string, mode Mode, applied bool) (out map[string]engcomp.Composed, skipped, stood []string, err error) { +func compositions(in Input, node string, fps map[string]string, mode Mode, applied bool) (out map[string]engcomp.Composed, skipped, stood []string, heldBy map[string]string, err error) { reading := in reading.Nodes = append([]Node(nil), in.Nodes...) found := false @@ -99,7 +108,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli return module != "" && Folds(inOwn[module], fps[module]) } if !found { - return nil, nil, nil, fmt.Errorf("compose: the mesh has no node %q", node) + return nil, nil, nil, nil, fmt.Errorf("compose: the mesh has no node %q", node) } // A folded module built by a build of the old form (Node.RunsOwn) is left to the node: composed from what runs of it // (Carried), so what it provides and requires still shapes the rest and what the others read of it (its published @@ -167,10 +176,10 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli } d, problems, err := ComposeIn(reading, node, mode) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if len(problems) > 0 { - return nil, nil, nil, fmt.Errorf("compose: %s's declaration is refused by the node-engine's validator: %v", node, problems) + return nil, nil, nil, nil, fmt.Errorf("compose: %s's declaration is refused by the node-engine's validator: %v", node, problems) } for _, x := range d.StoodIn { // The bus's user list is the bus node's own to follow, written alone and never from a composition the node @@ -186,11 +195,11 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli sort.Strings(stood) stood = slices.Compact(stood) // an own secret is both stood in for and foreseen if applied && len(stood) > 0 { - return nil, nil, nil, fmt.Errorf("%w on %s: %s", ErrStoodIn, node, strings.Join(stood, ", ")) + return nil, nil, nil, nil, fmt.Errorf("%w on %s: %s", ErrStoodIn, node, strings.Join(stood, ", ")) } body, err := d.Body() if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } var envelope struct { Resources []json.RawMessage `json:"resources"` @@ -199,7 +208,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli Provisioner string `json:"provisioner_account"` } if err := json.Unmarshal(body, &envelope); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } var kept []json.RawMessage keptPaths := map[string]bool{} @@ -213,7 +222,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli Path string `json:"path"` } if err := json.Unmarshal(raw, &head); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if isCarried(head.Module, head.ID) { continue // what the stub composed of it: the node's own carries what runs of it instead @@ -223,11 +232,11 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli // compositionsOf): the node's own composition is then the controller's, byte for byte. var fields map[string]json.RawMessage if err := json.Unmarshal(raw, &fields); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } delete(fields, "module") if raw, err = json.Marshal(fields); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if head.Type == string(declaration.TypeFile) { foldedPaths[head.Path] = true @@ -248,7 +257,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli if len(carried) > 0 { var err error if kept, foldedPaths, err = carry(kept, foldedPaths, runsOwn, isCarried); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } for _, p := range runsOwn.Filled { if foldedPaths[p] && !slices.Contains(envelope.Filled, p) { @@ -270,12 +279,12 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli out, err = engcomp.Split(node, kept, fps, filled, engcomp.NodeOwn{Adoption: envelope.Adoption, Provisioner: envelope.Provisioner}) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if len(ownFilled) > 0 { own, err := engcomp.NodeKey(node) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } c := out[own] c.Filled = append(c.Filled, ownFilled...) @@ -295,7 +304,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli skipped = append(skipped, m) } sort.Strings(skipped) - return out, skipped, stood, nil + return out, skipped, stood, d.Held, nil } // RunsAnyOf says runs (a composition as it runs) holds a resource of module, by its module or the module its id names. diff --git a/internal/compose/credentials_needed.go b/internal/compose/credentials_needed.go index 6937130..cd9306c 100644 --- a/internal/compose/credentials_needed.go +++ b/internal/compose/credentials_needed.go @@ -23,12 +23,12 @@ type CredentialKey struct { // in neither list: the vault mints nothing for it (issue 1126). func CredentialsNeeded(in Input, node string) (asConsumer, asProvider []CredentialKey, err error) { c := newComposer(in) - resolved, _, err := c.resolutionHolding(node) + resolved, held, err := c.resolutionHolding(node) if err != nil { return nil, nil, err } for _, n := range resolved.Needs { - if n.ByRecord || n.From == "" || credentialless(n) { + if _, isHeld := held[n.For]; isHeld || n.ByRecord || n.From == "" || credentialless(n) { continue } asConsumer = append(asConsumer, CredentialKey{Provision: n.Name, Consumer: node, ConsumerModule: n.For, diff --git a/internal/compose/frombus/answer.go b/internal/compose/frombus/answer.go index 702f1cb..9e0ecb9 100644 --- a/internal/compose/frombus/answer.go +++ b/internal/compose/frombus/answer.go @@ -337,7 +337,8 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b hold(m, failed[m], "") } for { - out, skipped, stood, err = compose.CompositionsIn(in, node, fps, mode) + var leftOut map[string]string + out, skipped, stood, leftOut, err = compose.CompositionsHolding(in, node, fps, mode) if err != nil { module, leave := culprit(in, node, fps, mode, failed, err) if module == "" { @@ -346,6 +347,18 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b hold(module, err.Error(), leave) continue } + // A module the node leaves out because its claim cannot be held or its credential was refused (issue 1966) is held with + // those words: its handed composition stays and the check names it refused; the rest composes. + heldNow := false + for _, m := range sortedKeys(leftOut) { + if _, done := failed[m]; !done { + hold(m, leftOut[m], "") + heldNow = true + } + } + if heldNow { + continue + } again := false if applied && len(stood) > 0 { placed := map[string]bool{} diff --git a/internal/compose/frombus/held_rows_test.go b/internal/compose/frombus/held_rows_test.go index e4d5673..5bc01e9 100644 --- a/internal/compose/frombus/held_rows_test.go +++ b/internal/compose/frombus/held_rows_test.go @@ -300,3 +300,35 @@ func aHeldModuleOfItsOwnAccountNeverRefusesTheNode(t *testing.T) { } } } + +// Issue 1966: two modules claiming one seat held once cost the second claimant, not the node: Compose holds it with the claim's +// words, composes the first, and (the node-engine keeping what it handed of a held module) leaves its composition out, whether or +// not it runs. +func TestASecondClaimantOfASeatIsHeldAndTheNodeComposesTheRest(t *testing.T) { + first := `{"module":"alpha","version":"1","claims":[{"name":"the-seat","scope":"node"}]}` + second := `{"module":"beta","version":"1","claims":[{"name":"the-seat","scope":"node"}]}` + for _, running := range []bool{false, true} { + b, fps := aNode(t, first, second) + scope := Scope{Node: "n1"} + if running { + scope.Running = []engcomp.Composed{{Node: "n1", Module: "beta", FP: fps["beta"], Resources: nil}} + } + got, _, held, err := Compose(context.Background(), b, Mesh{}, scope, fps) + if err != nil || len(held) != 1 || !strings.Contains(held["beta"], "alpha and beta both claim") { + t.Fatalf("running %v: err %v, held %v; want beta held with the claim's words", running, err, held) + } + for _, c := range got { + if c.Module == "beta" { + t.Errorf("running %v: the held claimant is composed: %+v", running, c) + } + } + // A check that adds the second claimant is refused for it, in the claim's words, and not judged to compose. + p := b.assignments["n1"] + ans := Answer(context.Background(), b, Mesh{}, Scope{Node: "n1", Assignments: map[string]*meshmoduleassigner.Assignments{"n1": &p}}, + fps, scope.Running, "p-h1", 1) + if ans.Verdict != nodeengine.CandidateAnswerVerdictRefused || len(ans.Refusals) != 1 || ans.Refusals[0].Module != "beta" || + !strings.Contains(ans.Refusals[0].Why, "both claim") { + t.Errorf("running %v: the answer is %+v, want beta refused with the claim's words", running, ans) + } + } +} diff --git a/internal/compose/node.go b/internal/compose/node.go index bfd0408..1d52253 100644 --- a/internal/compose/node.go +++ b/internal/compose/node.go @@ -37,6 +37,9 @@ type Declared struct { // ADR 0163, rule 6), sorted; LeftOutWhy is why each was. LeftOut []string LeftOutWhy map[string]string + // Held is, of those, each module left out because its claim cannot be held or its credential was refused, and why (issue + // 1966): the node keeps what runs of it and the composer says it held. Not on the wire. + Held map[string]string // Not on the wire: Received map[string]map[string][]catalogue.Contribution @@ -199,7 +202,7 @@ func (c *composer) declaration(nodeName string, mode Mode) (Declared, error) { if err != nil { return Declared{}, err } - out := Declared{Resources: composed.Resources, Adoption: adoptionFor(plan, node, composed), + out := Declared{Held: resolved.Held, Resources: composed.Resources, Adoption: adoptionFor(plan, node, composed), Received: composed.Received, Mesh: with.Mesh, BusUsers: with.BusUsers, LeftOut: sortedKeysOf(composed.LeftOut), LeftOutWhy: composed.LeftOut, Withheld: rendered.Withheld, Unbound: rendered.Unbound, diff --git a/internal/compose/resolution/resolve.go b/internal/compose/resolution/resolve.go index 553c538..89b6ffb 100644 --- a/internal/compose/resolution/resolve.go +++ b/internal/compose/resolution/resolve.go @@ -26,26 +26,17 @@ func (r *Refusal) Error() string { return "these assignments cannot be applied:\n - " + strings.Join(r.Problems, "\n - ") } -// Resolve works out everything a node runs, from what was assigned to it. -// -// The catalogue is every module the mesh knows about; assigned is what a person put on this node. -// What comes back is the closure — assigned modules plus everything they require — or a refusal -// naming every reason it could not be closed. -func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, error) { - resolution, _, err := resolve(catalogue, assigned, node, world, true) - return resolution, err -} - -// ResolveHolding is Resolve where a claim problem costs the module that has it, not the machine (issue 1966, ADR 0636 §3: a -// refused claim blocks that claimant only). The modules that cannot hold what they claim come back with why, each still in +// ResolveHolding works out everything a node runs, from what was assigned to it. The catalogue is every module the mesh knows +// about; assigned is what a person put on this node. What comes back is the closure -- assigned modules plus everything they +// require -- or a refusal naming every reason it could not be closed. A claim problem costs the module that has it, not the +// machine (issue 1966, ADR 0636 §3: a refused claim blocks that claimant only). The modules that cannot hold what they claim come back with why, each still in // the resolution's modules and none of them in its claims; the caller leaves them out of what it declares (LeftOut), so the // machine composes everything else. Every other refusal is Resolve's. func ResolveHolding(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, map[string]string, error) { - return resolve(catalogue, assigned, node, world, false) + return resolve(catalogue, assigned, node, world) } -// resolve is Resolve; with strict, a claim problem is a refusal beside every other, and without it a module held. -func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World, strict bool) (mcat.Resolution, map[string]string, error) { +func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, map[string]string, error) { elsewhere := world.Held var problems []string @@ -608,12 +599,6 @@ func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.No problems = append(problems, checkCapabilities(resolution.Modules, node)...) claims, held := holdClaims(resolution.Modules, node, elsewhere, world.Holdings) - if strict { - for _, why := range held { - problems = append(problems, why) - } - held = nil - } problems = append(problems, checkResources(resolution.Modules)...) resolution.Claims = claims @@ -634,6 +619,10 @@ func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.No } if len(problems) > 0 { + // A pass refused anyway says every reason at once, a held module's among them. + for _, why := range held { + problems = append(problems, why) + } sort.Strings(problems) return mcat.Resolution{}, nil, &Refusal{Problems: problems} } @@ -691,21 +680,8 @@ func checkCapabilities(modules []mcat.Manifest, node mcat.Node) []string { return problems } -// checkClaims refuses two modules holding one singular thing. -// -// Within this node's own set, and against what is already held elsewhere for the wider scopes. A -// claim at mesh scope is the same idea as the mesh's one hub, said once instead of hard-coded. -func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, []string) { - held, problems := claimsOf(modules, node, elsewhere, holdings) - var out []string - for _, p := range problems { - out = append(out, p.why) - } - return held, out -} - -// holdClaims is checkClaims with each problem put on the module that has it: the claims that hold, and for each module that -// cannot hold what it claims, why (every reason, joined). +// holdClaims refuses two modules holding one singular thing, within this node's own set and against what is held elsewhere for +// the wider scopes: the claims that hold, and for each module that cannot hold what it claims, why (every reason, joined). func holdClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, map[string]string) { held, problems := claimsOf(modules, node, elsewhere, holdings) var out map[string]string diff --git a/internal/compose/resolution/strict_test.go b/internal/compose/resolution/strict_test.go new file mode 100644 index 0000000..6c5680e --- /dev/null +++ b/internal/compose/resolution/strict_test.go @@ -0,0 +1,35 @@ +package resolution + +import ( + "sort" + + mcat "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// Resolve is ResolveHolding for a test that wants a claim problem as a refusal of the whole set, as before issue 1966. +func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, error) { + got, held, err := ResolveHolding(catalogue, assigned, node, world) + if err != nil { + return mcat.Resolution{}, err + } + if len(held) > 0 { + var problems []string + for _, why := range held { + problems = append(problems, why) + } + sort.Strings(problems) + return mcat.Resolution{}, &Refusal{Problems: problems} + } + return got, nil +} + +// checkClaims is holdClaims with the problems as a list. +func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, []string) { + held, why := holdClaims(modules, node, elsewhere, holdings) + var problems []string + for _, w := range why { + problems = append(problems, w) + } + sort.Strings(problems) + return held, problems +}