diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index c49fc8cb..3faccf9a 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -256,7 +256,13 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri v, err = builder.Check(building, builder.Command, checkSpecOf(request), workspace, registry, forgeFrom(), say) if err == nil { result.Check = &link.CheckOutcome{Verdict: v.Verdict, Summary: v.Summary, Report: v.Report, - Took: v.Took.Round(time.Second).String(), Gate: layerOf(v.Gate), RepoCheck: layerOf(v.Repo)} + Took: v.Took.Round(time.Second).String(), Gate: layerOf(v.Gate), RepoCheck: layerOf(v.Repo), + Head: v.Head, Trunk: v.Trunk, Beside: v.Beside} + // What each of a group's heads was judged as (novox/hq ADR 0323 §6). + for _, m := range v.Members { + result.Check.Members = append(result.Check.Members, link.CheckedMember{Owner: m.Owner, Repo: m.Repo, + Commit: m.Head, Trunk: m.Trunk}) + } } } else if err == nil { // The package-registry credential is a build input, so it is resolved before the clone: a @@ -354,7 +360,7 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { } for _, m := range c.Members { spec.Group = append(spec.Group, builder.GroupHead{Owner: m.Owner, Repo: m.Repo, Repository: m.Repository, - Ref: m.Ref, Paths: m.Paths}) + Ref: m.Ref, Paths: m.Paths, Base: m.Base}) } return spec } diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index bb78c718..1ff6a275 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -161,9 +161,7 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return u, err == nil }) fmt.Print(planText(plan)) - direct := link.Checked{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit, - ID: link.NewBuildID(time.Now()), Verdict: "pass", Summary: noModuleTouched, - Gate: &link.CheckLayer{Verdict: "pass", Summary: noModuleTouched}, Plan: &plan} + direct := directVerdict(p, plan) switch { case !scope.gated() && !scope.Mesh: fmt.Printf("%s/%s#%d (%.8s): %s, and the repository is not the mesh's: said, nothing run\n", @@ -201,6 +199,24 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return nil } +// directVerdict is the verdict of a change no build judges — it touches no module, and either the repository is not +// the mesh's or the head holds no merge-check.sh. **It names the trunk commit it holds for** (novox/hq ADR 0323 §3): +// the base branch's head the forge announced with the head, so the merge tool can weigh it against the trunk's head +// and does not refuse it for ever (a verdict naming no trunk is never the trunk's head's). Nothing was built to +// judge: the trunk commit is the one the forge said the change was announced against. +func directVerdict(p link.PullUpdated, plan link.ChangePlan) link.Checked { + trunk := p.BaseSHA + if len(trunk) > 12 { + trunk = trunk[:12] + } + if trunk != "" { + plan.Trunk = trunk + } + return link.Checked{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit, + ID: link.NewBuildID(time.Now()), Verdict: "pass", Summary: noModuleTouched, + Gate: &link.CheckLayer{Verdict: "pass", Summary: noModuleTouched}, Plan: &plan, Trunk: trunk} +} + // checkRequestFor is the ask for one pull request's head: the repository as the mesh clones it, the head, // and what is read beside it. func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scope checkScope, @@ -363,6 +379,9 @@ func checkedOf(result link.BuildResult) link.Checked { case result.Check != nil: c.Verdict, c.Summary, c.Report = result.Check.Verdict, result.Check.Summary, result.Check.Report c.Gate, c.RepoCheck = result.Check.Gate, result.Check.RepoCheck + // What was judged (novox/hq ADR 0323 §3): the head merged into the trunk commit, and the commits of the + // repositories read beside it. + c.Trunk, c.Beside, c.Took = result.Check.Trunk, result.Check.Beside, result.Check.Took if c.Gate == nil { // A build seat from before the layers: its verdict is the gate's. c.Gate = &link.CheckLayer{Verdict: c.Verdict, Summary: c.Summary} @@ -388,14 +407,20 @@ func checkedOf(result link.BuildResult) link.Checked { } if result.Checked != nil { c.Plan = result.Checked.Plan + if c.Plan != nil && c.Trunk != "" { + // The kept delivery plan records the trunk commit with its other inputs (ADR 0323 §3). + plan := *c.Plan + plan.Trunk = c.Trunk + c.Plan = &plan + } // A delivery group's composed check (novox/hq ADR 0239): every head it judged, this one first. if g := result.Checked; g.Group != "" { c.Group = g.Group c.Members = append(c.Members, link.CheckedMember{Owner: g.Owner, Repo: g.Repo, Number: g.Number, - Commit: result.Ref}) + Commit: result.Ref, Trunk: c.Trunk}) for _, m := range g.Members { c.Members = append(c.Members, link.CheckedMember{Owner: m.Owner, Repo: m.Repo, Number: m.Number, - Commit: m.Ref}) + Commit: m.Ref, Trunk: memberTrunk(result.Check, m)}) } } } @@ -410,6 +435,20 @@ func checkedOf(result link.BuildResult) link.Checked { return c } +// memberTrunk is the trunk commit the build seat merged one of a group's heads into (novox/hq ADR 0323 §6), empty +// when it said none. +func memberTrunk(outcome *link.CheckOutcome, m link.GroupMember) string { + if outcome == nil { + return "" + } + for _, j := range outcome.Members { + if strings.EqualFold(j.Owner, m.Owner) && strings.EqualFold(j.Repo, m.Repo) && strings.HasPrefix(m.Ref, j.Commit) { + return j.Trunk + } + } + return "" +} + func prefixedAll(prefix string, items []string) []string { out := make([]string, 0, len(items)) for _, i := range items { @@ -424,8 +463,12 @@ func sayChecked(ctx context.Context, c link.Checked) { if c.RepoCheck != nil { repo = strings.ToUpper(c.RepoCheck.Verdict) + " — " + c.RepoCheck.Summary } - fmt.Printf("%s: %s/%s#%d at %.8s checked on %s: gate %s — %s; repository %s\n", c.ID, c.Owner, c.Repo, c.Number, - c.Commit, orSomewhere(c.On), strings.ToUpper(c.Verdict), c.Summary, repo) + judged := "" + if c.Trunk != "" { + judged = " merged into trunk " + c.Trunk + } + fmt.Printf("%s: %s/%s#%d at %.8s%s checked on %s: gate %s — %s; repository %s\n", c.ID, c.Owner, c.Repo, c.Number, + c.Commit, judged, orSomewhere(c.On), strings.ToUpper(c.Verdict), c.Summary, repo) if checkEvents == nil { return } diff --git a/cmd/mesh-controller/checks_test.go b/cmd/mesh-controller/checks_test.go index 7e0bc946..8ea4242a 100644 --- a/cmd/mesh-controller/checks_test.go +++ b/cmd/mesh-controller/checks_test.go @@ -210,3 +210,81 @@ func TestABuildOffTheTrunkIsNeverPublished(t *testing.T) { t.Errorf("a build seat that said nothing was refused: %v", err) } } + +// **A verdict names the trunk commit it judged** (novox/hq ADR 0323 §3, issue 341): the controller's `checked` +// carries the head and the trunk commit the build seat merged it into, the commits of the repositories read beside +// it, how long it took, and the kept delivery plan records the trunk commit with its other inputs. A group's +// composed check names, per member, the trunk commit that head was merged into (§6). +func TestAVerdictNamesTheTrunkCommitItJudged(t *testing.T) { + plan := &link.ChangePlan{Repository: "novox/mesh-host", Base: "main", Head: "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Summary: "builds mesh-host"} + asked := &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Number: 66, Base: "main", Plan: plan} + c := checkedOf(link.BuildResult{ID: "b", Ref: "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Checked: asked, + Check: &link.CheckOutcome{Verdict: "pass", Took: "4m12s", Head: "dfb1415aaaaa", Trunk: "d28ca95bbbbb", + Gate: &link.CheckLayer{Verdict: "pass"}, RepoCheck: &link.CheckLayer{Verdict: "pass"}, + Beside: map[string]string{"mesh-catalog": "0123456789ab", "mesh-lab": "ba9876543210"}}}) + if c.Trunk != "d28ca95bbbbb" || c.Commit != "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" { + t.Fatalf("the verdict names head %s and trunk %q", c.Commit, c.Trunk) + } + if c.Beside["mesh-catalog"] != "0123456789ab" || c.Beside["mesh-lab"] != "ba9876543210" { + t.Fatalf("the verdict names the repositories read beside it as %v", c.Beside) + } + if c.Took != "4m12s" { + t.Fatalf("the verdict says it took %q", c.Took) + } + if c.Plan == nil || c.Plan.Trunk != "d28ca95bbbbb" || c.Plan.Head != plan.Head { + t.Fatalf("the kept plan records %+v", c.Plan) + } + if plan.Trunk != "" { + t.Fatal("the plan the controller asked with was changed: the verdict's is a copy") + } + + group := &link.CheckRequest{Owner: "novox", Repo: "mesh-controller", Number: 7, Base: "main", Group: "feat/x", + Members: []link.GroupMember{{Owner: "novox", Repo: "mesh-host", Number: 8, Ref: "cccccccccccc0000", Base: "main"}}} + c = checkedOf(link.BuildResult{ID: "g", Ref: "aaaaaaaaaaaa0000", Checked: group, Check: &link.CheckOutcome{ + Verdict: "pass", Head: "aaaaaaaaaaaa", Trunk: "111111111111", Gate: &link.CheckLayer{Verdict: "pass"}, + Members: []link.CheckedMember{{Owner: "novox", Repo: "mesh-host", Commit: "cccccccccccc", Trunk: "222222222222"}}}}) + if len(c.Members) != 2 || c.Members[0].Trunk != "111111111111" || c.Members[1].Trunk != "222222222222" || + c.Members[1].Commit != "cccccccccccc0000" { + t.Fatalf("the composed check names its members %+v", c.Members) + } +} + +// **The merge made for a check is never registered** (novox/hq ADR 0323 §2): a build of it — a commit on no branch +// of the forge — is refused, naming the trunk. The rule of ADR 0238 §1 stands unchanged; this holds it for the +// merge commit a check makes. +func TestABuildOfACheckMergeIsRefusedNamingTheTrunk(t *testing.T) { + merge := link.BuildResult{ID: "b", Commit: "4b5c6d7e8f90aabbccddeeff0011223344556677", Trunk: "main"} + err := publishable(merge, "") + if !errors.Is(err, errOffTheTrunk) || !strings.Contains(err.Error(), "not on main") { + t.Fatalf("a build of a check's merge was answered %v", err) + } +} + +// **A verdict no build judges names its trunk commit too** (novox/hq ADR 0323 §3): a change touching no module of a +// repository with no merge-check.sh (novox/mesh-tools-go) is answered by the controller itself. Naming no trunk, the +// merge tool refused it, asked again, got the same verdict, and refused for ever. +func TestADirectVerdictNamesTheTrunkTheForgeAnnounced(t *testing.T) { + p := link.PullUpdated{Owner: "novox", Repo: "mesh-tools-go", Number: 3, Base: "main", Commit: "1a2b3c4d5e6f7a8b9c0d", + BaseSHA: "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"} + c := directVerdict(p, link.ChangePlan{Repository: "novox/mesh-tools-go", Base: "main", Head: p.Commit}) + if c.Trunk != "7a8b9c0d1e2f" || c.Plan == nil || c.Plan.Trunk != "7a8b9c0d1e2f" { + t.Fatalf("the direct verdict names trunk %q, its plan %+v", c.Trunk, c.Plan) + } + if c.Verdict != "pass" || c.Commit != p.Commit { + t.Fatalf("the direct verdict is %+v", c) + } + if c := directVerdict(link.PullUpdated{Owner: "novox", Repo: "x", Commit: "abc"}, link.ChangePlan{}); c.Trunk != "" { + t.Fatalf("a forge that said no base head gave the trunk %q", c.Trunk) + } +} + +// A head checked again through mesh-delivery (`delivery check --members`, a recheck) reaches the same direct verdict: +// it names the base head the forge last announced, kept on the member, or the merge tool would refuse it for ever. +func TestARecheckedHeadsDirectVerdictNamesItsTrunk(t *testing.T) { + m := orderMember{ID: "novox/mesh-tools-go@1a2b3c4d5e6f", Repository: "novox/mesh-tools-go", Base: "main", + Head: "1a2b3c4d5e6f7a8b9c0d", Number: 3, BaseSHA: "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"} + if c := directVerdict(m.pull(), link.ChangePlan{}); c.Trunk != "7a8b9c0d1e2f" { + t.Fatalf("a rechecked head's direct verdict names trunk %q", c.Trunk) + } +} diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 7f95d589..615386c0 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -580,6 +580,9 @@ type orderMember struct { CloneURL string `json:"clone_url,omitempty"` // After are the repositories its pull request says it goes after (`after: `). After []string `json:"after,omitempty"` + // BaseSHA is the base branch's head the forge last announced the head with (novox/hq ADR 0323): the trunk commit + // a verdict no build judges names when the head is checked again through mesh-delivery. + BaseSHA string `json:"base_sha,omitempty"` } // pull is the member as the forge announced it. @@ -589,7 +592,7 @@ func (m orderMember) pull() link.PullUpdated { if base == "" { base = "main" } - return link.PullUpdated{Owner: owner, Repo: repo, Number: m.Number, Base: base, Commit: m.Head, + return link.PullUpdated{Owner: owner, Repo: repo, Number: m.Number, Base: base, Commit: m.Head, BaseSHA: m.BaseSHA, CloneURL: m.CloneURL, Paths: m.Paths, PathsTruncated: m.PathsTruncated, Removed: m.Removed, ModuleDirs: m.ModuleDirs, ModuleDirsSaid: m.ModuleDirsSaid} } @@ -1185,7 +1188,7 @@ func askGroupCheck(ctx context.Context, open *stores, group string, members []or return "", fmt.Errorf("%s cannot be cloned for the group's check: %w", m.ID, err) } request.Check.Members = append(request.Check.Members, link.GroupMember{Owner: mp.Owner, Repo: mp.Repo, - Number: mp.Number, Repository: url, Ref: mp.Commit, Paths: mp.Paths}) + Number: mp.Number, Repository: url, Ref: mp.Commit, Paths: mp.Paths, Base: mp.Base}) } seat := buildSeatHeld(ctx) ask, err := askOverOn(seat) diff --git a/cmd/mesh-controller/plan_ids.go b/cmd/mesh-controller/plan_ids.go new file mode 100644 index 00000000..d9dca230 --- /dev/null +++ b/cmd/mesh-controller/plan_ids.go @@ -0,0 +1,26 @@ +package main + +import ( + "fmt" + "regexp" +) + +// planID is every id the mesh gives a plan: a merge's walk or batch (plan-) or a release +// (release-). +var planID = regexp.MustCompile(`^(plan|release)-[0-9]+$`) + +// planIDArguments are the arguments of plans that name a plan. wait is among them for when it is declared +// (novox/hq issue 229); an argument not declared is refused before this is asked. +var planIDArguments = []string{"id", "wait", "stop", "close", "retry", "go"} + +// plainPlanIDs refuses a plans call naming a plan by anything but a plan's id (novox/hq issue 561): checked +// once, where the call becomes a command line, so nothing else a caller sent reaches a command or the store. +func plainPlanIDs(given map[string]string) error { + for _, arg := range planIDArguments { + if id, named := given[arg]; named && !planID.MatchString(id) { + return fmt.Errorf("plans %s: %q is not a plan's id (plan- or release-, as `plans` "+ + "lists them); nothing was done", arg, id) + } + } + return nil +} diff --git a/cmd/mesh-controller/plan_ids_test.go b/cmd/mesh-controller/plan_ids_test.go new file mode 100644 index 00000000..f49039f4 --- /dev/null +++ b/cmd/mesh-controller/plan_ids_test.go @@ -0,0 +1,37 @@ +package main + +import ( + "strings" + "testing" +) + +// **A plan's id is checked once, where a call becomes a command line** (novox/hq issue 561): every argument of +// plans that names a plan is refused unless it is an id the mesh gives a plan, so a quote never reaches a +// command, nor the store behind it. +func TestAPlanIdThatIsNoPlansIdIsRefusedBeforeACommandStarts(t *testing.T) { + for _, arg := range []string{"id", "stop", "close", "retry", "go"} { + for _, bad := range []string{"x' or '1'='1", "plan-1'", "plan-", "plan-1 2", "batch-1", "PLAN-1"} { + args := map[string]any{arg: bad} + if arg == "stop" || arg == "close" || arg == "go" { + args["why"] = "a test" + } + argv, err := argvFor("plans", args) + if err == nil { + t.Errorf("plans %s %q was composed: %v", arg, bad, argv) + continue + } + if !strings.Contains(err.Error(), "is not a plan's id") { + t.Errorf("plans %s %q was refused for another reason: %v", arg, bad, err) + } + } + for _, good := range []string{"plan-1791813626034291775", "release-1791811938267404408"} { + args := map[string]any{arg: good} + if arg == "stop" || arg == "close" || arg == "go" { + args["why"] = "a test" + } + if _, err := argvFor("plans", args); err != nil { + t.Errorf("plans %s %q was refused: %v", arg, good, err) + } + } + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3e4d1d18..017d9fea 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -336,6 +336,9 @@ func (a *verbArguments) commandLine() ([]string, error) { } return argv, nil case "plans": + if err := plainPlanIDs(a.given); err != nil { + return nil, err + } if r := str("repository"); r != "" { argv := []string{"plans", "--what-if", r} if p := str("paths"); p != "" { diff --git a/internal/builder/check.go b/internal/builder/check.go index 50b3a476..0a08d40c 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -49,8 +49,10 @@ import ( // // One check: // -// 1. clones the repository at the pull request's head, and beside it the repositories its check -// reads — the controller the mesh runs, the catalogue, the host, the lab — each at the ref asked; +// 1. clones the repository at the pull request's head and **makes the merge of that head into the trunk's +// head**, the check's trunk commit, in its own clone (novox/hq ADR 0323, issue 341; check_merge.go): what +// is judged is what merging makes, never pushed. Beside it the repositories its check reads — the +// controller the mesh runs, the catalogue, the host, the lab — each at the ref asked; // 2. reads the facts snapshot the controller keeps, and with it **the versions the mesh runs**: the // store a check's tests stand on is the store's own release, and the bus the bus's; // 3. raises a throwaway PostgreSQL and a throwaway bus of those versions, labelled with the ask so a @@ -79,7 +81,8 @@ type CheckSpec struct { Modules []string New []string Manifests []string - // Base is the branch the pull request merges into: what the gate compares the change against. + // Base is the branch the pull request merges into: what the gate compares the change against, and the trunk + // whose head the change is merged into before it is judged (novox/hq ADR 0323). Empty judges the head alone. Base string // Judge is who judges the gate: "" the controller the mesh runs, "self" the change's own, "validator" // the running one with the change's validator. @@ -104,6 +107,9 @@ type GroupHead struct { Repository string Ref string Paths []string + // Base is the branch it merges into: the head is merged into that trunk's head before it is composed + // (novox/hq ADR 0323 §6). Empty composes the head alone, as a controller from before it asks. + Base string } // Gated is whether the change touches the mesh's graph, and so whether the gate runs. @@ -139,6 +145,16 @@ type CheckVerdict struct { Took time.Duration Gate *Layer Repo *Layer + // Head and Trunk are what was judged (novox/hq ADR 0323 §3): the pull request's head merged into the trunk + // commit, each as twelve characters. Trunk is empty for a check asked with no trunk, which judged the head + // alone. Merge is the merge commit made for the check, whole: only checked, never pushed or registered. + Head, Trunk, Merge string + // Beside are the commits of the repositories the check read beside its own, by the directory each was + // cloned under, as twelve characters: named, not judged against (ADR 0323 §3, issue 568). + Beside map[string]string + // Members are a delivery group's other heads as the composed check judged them, each with the trunk + // commit it was merged into (ADR 0323 §6). + Members []JudgedHead } // Layer is one layer of a check, judged. @@ -281,6 +297,34 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry return CheckVerdict{}, err } tree := filepath.Join(root, name) + // **What merging makes is judged, not the head** (novox/hq ADR 0323, issue 341): the head merged into the + // trunk's head as this clone found it, in this clone alone. A head that does not merge cleanly fails here. + judged := CheckVerdict{Head: twelve(spec.Ref)} + paths := spec.Paths + if spec.Base != "" { + m, err := mergeIntoTrunk(ctx, run, tree, spec.Base, spec.Ref) + if err != nil { + return CheckVerdict{}, err + } + judged.Head, judged.Trunk = twelve(m.Head), twelve(m.Trunk) + if len(m.Conflicts) > 0 { + said := conflictWords(m) + say("check", "%s", said) + return CheckVerdict{Verdict: "fail", Summary: said, Report: said, Took: time.Since(began), + Gate: &Layer{Verdict: "fail", Summary: said}, Repo: &Layer{Verdict: "fail", Summary: said}, + Head: judged.Head, Trunk: judged.Trunk}, nil + } + judged.Merge = m.Merge + if changed := changedByMerge(ctx, run, tree, m); len(changed) > 0 { + paths = changed + } + say("check", "head %s merged into trunk %s (%s): the merge is judged, and never pushed", judged.Head, + judged.Trunk, spec.Base) + } + named := func(v CheckVerdict) CheckVerdict { + v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members + return v + } // **The base branch's check judges the change** (novox/hq issue 310), never the one the change brings. judging, err := TheCheckThatJudges(ctx, tree, spec.Base) if err != nil { @@ -297,7 +341,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry v := CheckVerdict{Verdict: "pass", Summary: noModule, Took: time.Since(began), Gate: &Layer{Verdict: "pass", Summary: noModule}, Repo: &Layer{Verdict: "warning", Summary: noScript}} say("check", "%s; %s", noModule, noScript) - return v, nil + return named(v), nil } for dir, b := range spec.Beside { if dir == name || !safeName.MatchString(dir) { @@ -306,6 +350,13 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if err := clone(b.Repository, b.Ref, dir); err != nil { return CheckVerdict{}, fmt.Errorf("beside it, %w", err) } + // The commit each one read is named in the verdict (ADR 0323 §3): its move is issue 568's question. + if at := headAt(ctx, run, filepath.Join(root, dir)); at != "" { + if judged.Beside == nil { + judged.Beside = map[string]string{} + } + judged.Beside[dir] = at + } say("check", "beside it %s at %s", dir, short(b.Ref)) } @@ -322,6 +373,26 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry return CheckVerdict{}, fmt.Errorf("a head of the group, %w", err) } say("check", "with it, of its group, %s/%s at %s", g.Owner, g.Repo, short(g.Ref)) + // Each member is composed as its merge into its own trunk's head (ADR 0323 §6). + member := JudgedHead{Owner: g.Owner, Repo: g.Repo, Head: twelve(g.Ref)} + if g.Base != "" { + m, err := mergeIntoTrunk(ctx, run, filepath.Join(root, dir), g.Base, g.Ref) + if err != nil { + return CheckVerdict{}, fmt.Errorf("a head of the group, %s/%s: %w", g.Owner, g.Repo, err) + } + member.Head, member.Trunk = twelve(m.Head), twelve(m.Trunk) + if len(m.Conflicts) > 0 { + said := fmt.Sprintf("%s/%s: %s", g.Owner, g.Repo, conflictWords(m)) + say("check", "%s", said) + judged.Members = append(judged.Members, member) + return named(CheckVerdict{Verdict: "fail", Summary: said, Report: said, Took: time.Since(began), + Gate: &Layer{Verdict: "fail", Summary: said}, Repo: &Layer{Verdict: "fail", Summary: said}}), nil + } + if changed := changedByMerge(ctx, run, filepath.Join(root, dir), m); len(changed) > 0 { + g.Paths = changed + } + } + judged.Members = append(judged.Members, member) heads = append(heads, map[string]any{"repository": g.Owner + "/" + g.Repo, "tree": filepath.Join(root, dir), "changed": g.Paths}) } @@ -433,7 +504,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry verdictFile := filepath.Join(root, "verdict.json") env := []string{EnvFacts + "=" + factsFile, EnvGate + "=" + gate, EnvGateStore + "=" + storeURL, EnvTestStore + "=" + storeURL, EnvTestBus + "=nats://" + bus, EnvRepository + "=" + spec.Owner + "/" + spec.Repo, - EnvChanged + "=" + strings.Join(spec.Paths, ","), EnvBeside + "=" + root, + EnvChanged + "=" + strings.Join(paths, ","), EnvBeside + "=" + root, EnvModules + "=" + strings.Join(append(append([]string{}, spec.Modules...), spec.New...), ","), EnvGroup + "=" + groupFile, "GOCACHE=" + filepath.Join(workspace, "go-cache"), "GOMODCACHE=" + filepath.Join(workspace, "go-modules")} @@ -501,7 +572,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry v.Report, v.Took = redact.redact(withWhatFailed(out.String(), v.Repo)), time.Since(began) say("check", "gate %s — %s; repository %s — %s (%s)", strings.ToUpper(v.Gate.Verdict), v.Gate.Summary, strings.ToUpper(v.Repo.Verdict), v.Repo.Summary, v.Took.Round(time.Second)) - return v, nil + return named(v), nil } // ownCheck runs the repository's own check, part by part, each in its toolchain's own container: the diff --git a/internal/builder/check_merge.go b/internal/builder/check_merge.go new file mode 100644 index 00000000..e656d4e9 --- /dev/null +++ b/internal/builder/check_merge.go @@ -0,0 +1,115 @@ +package builder + +import ( + "context" + "fmt" + "strings" +) + +// **A merge check judges the merge, not the head** (novox/hq ADR 0323, issue 341). +// +// On 2026-10-09 a node-engine pull request was checked green at its head and broke the trunk when it merged: +// a merge an hour earlier had added a field under the same JSON key on other lines, git merged the two edits +// without a conflict, and the result did not compile. The check had judged the head alone. So a check now +// takes the trunk the pull request merges into, its head at the moment the check clones it as the check's +// **trunk commit**, and makes in its own clone the merge of the head into that commit, as the forge's +// default merge would. The gate and the repository's own check judge that merge. +// +// **The merge is only checked** (ADR 0323 §2, ADR 0238 §1): it is a commit off the trunk, made in the +// check's own clone, never pushed to the forge and never registered; the clone is removed when the check +// ends. Nothing here writes to a remote. +// +// The forge keeps no merge reference for a pull request, only its head, so the builder makes it itself. + +// TrunkMerge is the merge a check made: the head and the trunk commit it was merged into, whole, and the merge +// commit; or the files in conflict when the head does not merge cleanly. +type TrunkMerge struct { + Head, Trunk, Merge string + Conflicts []string +} + +// JudgedHead is one head a check judged, with the trunk commit it merged that head into, each as twelve +// characters (ADR 0323 §3, §6). +type JudgedHead struct { + Owner, Repo string + Head, Trunk string +} + +// mergeIdentity is who a check's merge is made as: it is never pushed, so it names nobody's account. No hook of the +// clone's or the build machine's git configuration runs, and no setting there changes the merge. +var mergeIdentity = []string{"-c", "user.name=mesh merge check", "-c", "user.email=merge-check@mesh.invalid", + "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null", "-c", "merge.verifySignatures=false"} + +// mergeIntoTrunk makes, in the clone at tree, the merge of head into the head of origin/ as it stands in +// that fresh clone: the trunk commit. A head that does not merge cleanly answers the files in conflict and leaves +// the tree at the trunk commit; an error is that the merge could not be tried at all. +func mergeIntoTrunk(ctx context.Context, run Runner, tree, base, head string) (TrunkMerge, error) { + var m TrunkMerge + revParse := func(ref string) (string, error) { + out, err := run(ctx, tree, "git", "rev-parse", "--verify", "--quiet", ref+"^{commit}") + return strings.TrimSpace(out), err + } + var err error + if m.Head, err = revParse(head); err != nil || m.Head == "" { + return m, fmt.Errorf("the head %s cannot be read in the check's clone: %v", head, err) + } + if m.Trunk, err = revParse("refs/remotes/origin/" + base); err != nil || m.Trunk == "" { + return m, fmt.Errorf("the trunk %s cannot be read, and a check judges the head merged into it "+ + "(novox/hq ADR 0323): %v", base, err) + } + if _, err := run(ctx, tree, "git", "checkout", "--quiet", "--detach", m.Trunk); err != nil { + return m, fmt.Errorf("the trunk commit %s cannot be checked out: %w", twelve(m.Trunk), err) + } + message := fmt.Sprintf("Check the merge of %s into %s at %s (made for a merge check, never pushed)", + twelve(m.Head), base, twelve(m.Trunk)) + args := append(append([]string{}, mergeIdentity...), "merge", "--no-ff", "--no-edit", "--no-verify", "--quiet", "-m", message, m.Head) + if _, err := run(ctx, tree, "git", args...); err != nil { + out, _ := run(ctx, tree, "git", "diff", "--name-only", "--diff-filter=U") + m.Conflicts = strings.Fields(out) + _, _ = run(ctx, tree, "git", "merge", "--abort") + if len(m.Conflicts) == 0 { + return m, fmt.Errorf("the head %s could not be merged into trunk %s: %w", twelve(m.Head), twelve(m.Trunk), err) + } + return m, nil + } + if m.Merge, err = revParse("HEAD"); err != nil { + return m, fmt.Errorf("the merge made for the check cannot be read: %w", err) + } + return m, nil +} + +// changedByMerge is every path the merge changes against the trunk commit: what the gate's delivery plan is +// computed from (ADR 0323 §1). Nil when it cannot be read; the caller keeps the pull request's own list. +func changedByMerge(ctx context.Context, run Runner, tree string, m TrunkMerge) []string { + if m.Merge == "" || m.Trunk == "" { + return nil + } + out, err := run(ctx, tree, "git", "diff", "--name-only", "--no-renames", m.Trunk, m.Merge) + if err != nil { + return nil // empty-on-error: the caller keeps the pull request's own list of changed files, the safe one + } + return strings.Fields(out) +} + +// conflictWords is a head that does not merge cleanly, said as its check's failure. +func conflictWords(m TrunkMerge) string { + return fmt.Sprintf("head %s does not merge cleanly into trunk %s: in conflict %s", twelve(m.Head), twelve(m.Trunk), + strings.Join(m.Conflicts, ", ")) +} + +// headAt is the commit a clone is checked out at, as twelve characters; empty when it cannot be read. +func headAt(ctx context.Context, run Runner, tree string) string { + out, err := run(ctx, tree, "git", "rev-parse", "--verify", "--quiet", "HEAD^{commit}") + if err != nil { + return "" + } + return twelve(strings.TrimSpace(out)) +} + +// twelve is a commit as a verdict names it: its first twelve characters. +func twelve(commit string) string { + if len(commit) > 12 { + return commit[:12] + } + return commit +} diff --git a/internal/builder/check_merge_test.go b/internal/builder/check_merge_test.go new file mode 100644 index 00000000..a35fe888 --- /dev/null +++ b/internal/builder/check_merge_test.go @@ -0,0 +1,212 @@ +package builder + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// **A merge check judges the merge, not the head** (novox/hq ADR 0323, issue 341): the head merged into the trunk +// commit in the check's own clone, never pushed; a head that does not merge cleanly fails, naming the files; the +// verdict names the head, the trunk commit and the commits of the repositories read beside it. + +// aTrunkAndABranch is a repository whose main moved after the branch was cut: main holds trunkFiles over the +// common files, the branch headFiles. It answers the repository, the branch's head and main's head. +func aTrunkAndABranch(t *testing.T, common, trunkFiles, headFiles map[string]string) (string, string, string) { + t.Helper() + dir := t.TempDir() + git := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.org", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.org") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(files map[string]string) { + for name, body := range files { + if err := os.MkdirAll(filepath.Dir(filepath.Join(dir, name)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + git("add", "-A") + } + git("init", "--quiet", "-b", "main") + write(common) + git("commit", "--quiet", "-m", "common") + git("checkout", "--quiet", "-b", "change") + write(headFiles) + git("commit", "--quiet", "-m", "the change") + head := git("rev-parse", "HEAD") + git("checkout", "--quiet", "main") + write(trunkFiles) + git("commit", "--quiet", "-m", "the trunk moved") + return dir, head, git("rev-parse", "HEAD") +} + +func TestACheckJudgesTheHeadMergedIntoTheTrunk(t *testing.T) { + // main's script passes only in a tree holding both the trunk's file and the head's: the merge. + repo, head, trunk := aTrunkAndABranch(t, + map[string]string{CheckScript: "test -f trunk-only && test -f head-only || { echo 'FAIL: not the merge'; exit 1; }\n"}, + map[string]string{"trunk-only": "t"}, map[string]string{"head-only": "h"}) + beside, besideHead := aCheckedRepository(t, map[string]string{"README": "the catalogue"}) + registry := aFactsRegistry(t) + aRuntimeOnThisMachine(t) + + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-341", Repository: repo, Ref: head, Owner: "novox", + Repo: "mesh-lab", Number: 1, Base: "main", Toolchain: "golang", + Beside: map[string]Beside{"mesh-catalog": {Repository: beside, Ref: besideHead}}}, t.TempDir(), registry, + GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Repo == nil || v.Repo.Verdict != "pass" { + t.Fatalf("the check did not judge the head merged into the trunk: %+v\n%s", v.Repo, v.Report) + } + if v.Head != head[:12] || v.Trunk != trunk[:12] { + t.Fatalf("the verdict names head %q and trunk %q, not %s and %s", v.Head, v.Trunk, head[:12], trunk[:12]) + } + if v.Beside["mesh-catalog"] != besideHead[:12] { + t.Fatalf("the verdict names the catalogue read beside it as %q, not %s: %v", v.Beside["mesh-catalog"], + besideHead[:12], v.Beside) + } + if v.Merge == "" || v.Merge == head || v.Merge == trunk { + t.Fatalf("no merge of the head into the trunk was made for the check: %q", v.Merge) + } +} + +func TestAHeadThatDoesNotMergeCleanlyFailsNamingTheFiles(t *testing.T) { + repo, head, trunk := aTrunkAndABranch(t, + map[string]string{"store.go": "package store\n\nconst Declared = 1\n", "other.go": "package store\n"}, + map[string]string{"store.go": "package store\n\nconst Declared = 2\n"}, + map[string]string{"store.go": "package store\n\nconst Declared = 3\n"}) + + // No artifact store and no container runtime: a head that does not merge is judged before either is needed. + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-341-conflict", Repository: repo, Ref: head, + Owner: "novox", Repo: "mesh-lab", Number: 2, Base: "main", Toolchain: "golang"}, t.TempDir(), "", + GitCredential{}, nil) + if err != nil { + t.Fatalf("a head that does not merge cleanly was not judged: %v", err) + } + want := "does not merge cleanly into trunk " + trunk[:12] + for name, l := range map[string]*Layer{"gate": v.Gate, "repository": v.Repo} { + if l == nil || l.Verdict != "fail" || !strings.Contains(l.Summary, want) || !strings.Contains(l.Summary, "store.go") { + t.Fatalf("its %s layer said %+v, not a failure saying %q and naming store.go", name, l, want) + } + } + if strings.Contains(v.Summary, "other.go") { + t.Fatalf("a file not in conflict is named: %s", v.Summary) + } + if v.Head != head[:12] || v.Trunk != trunk[:12] { + t.Fatalf("the failure names head %q and trunk %q", v.Head, v.Trunk) + } +} + +// **The merge made for a check is only checked** (ADR 0323 §2): the forge never receives it. +func TestTheMergeMadeForACheckIsNeverPushed(t *testing.T) { + repo, head, trunk := aTrunkAndABranch(t, map[string]string{CheckScript: "true\n"}, + map[string]string{"trunk-only": "t"}, map[string]string{"head-only": "h"}) + registry := aFactsRegistry(t) + aRuntimeOnThisMachine(t) + refsBefore := gitOut(t, repo, "for-each-ref") + + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-341-push", Repository: repo, Ref: head, Owner: "novox", + Repo: "mesh-lab", Number: 3, Base: "main", Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Merge == "" { + t.Fatal("the check made no merge of the head into the trunk") + } + if err := exec.Command("git", "-C", repo, "cat-file", "-e", v.Merge+"^{commit}").Run(); err == nil { + t.Fatalf("the merge %s made for the check reached the forge's repository", v.Merge) + } + if after := gitOut(t, repo, "for-each-ref"); after != refsBefore { + t.Fatalf("the check moved a ref of the forge's repository:\nbefore %s\nafter %s", refsBefore, after) + } + if got := gitOut(t, repo, "rev-parse", "main"); got != trunk { + t.Fatalf("the trunk moved to %s", got) + } +} + +// A delivery group's composed check merges each member's head into its own trunk's head (ADR 0323 §6), and names +// both for each; a member that does not merge cleanly fails the composed check, naming it. +func TestAGroupsComposedCheckMergesEachMemberIntoItsTrunk(t *testing.T) { + repo, head, trunk := aTrunkAndABranch(t, map[string]string{CheckScript: "true\n"}, + map[string]string{"trunk-only": "t"}, map[string]string{"head-only": "h"}) + member, memberHead, memberTrunk := aTrunkAndABranch(t, map[string]string{"a": "1"}, + map[string]string{"b": "trunk"}, map[string]string{"c": "head"}) + registry := aFactsRegistry(t) + aRuntimeOnThisMachine(t) + + v, err := Check(t.Context(), Command, CheckSpec{ID: "check-341-group", Repository: repo, Ref: head, Owner: "novox", + Repo: "mesh-controller", Number: 4, Base: "main", Toolchain: "golang", Group: []GroupHead{{Owner: "novox", + Repo: "mesh-host", Repository: member, Ref: memberHead, Base: "main"}}}, t.TempDir(), registry, + GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Head != head[:12] || v.Trunk != trunk[:12] { + t.Fatalf("the composed check names its first head %q into %q", v.Head, v.Trunk) + } + if len(v.Members) != 1 || v.Members[0].Head != memberHead[:12] || v.Members[0].Trunk != memberTrunk[:12] { + t.Fatalf("the composed check names its members %+v, not mesh-host %s into %s", v.Members, memberHead[:12], + memberTrunk[:12]) + } + + conflicting, conflictHead, _ := aTrunkAndABranch(t, map[string]string{"a": "1"}, map[string]string{"a": "2"}, + map[string]string{"a": "3"}) + v, err = Check(t.Context(), Command, CheckSpec{ID: "check-341-group-conflict", Repository: repo, Ref: head, + Owner: "novox", Repo: "mesh-controller", Number: 4, Base: "main", Toolchain: "golang", Group: []GroupHead{{ + Owner: "novox", Repo: "mesh-host", Repository: conflicting, Ref: conflictHead, Base: "main"}}}, t.TempDir(), + registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Verdict != "fail" || !strings.Contains(v.Summary, "novox/mesh-host") || + !strings.Contains(v.Summary, "does not merge cleanly") { + t.Fatalf("a member that does not merge cleanly was answered %s: %s", v.Verdict, v.Summary) + } +} + +func gitOut(t *testing.T, dir string, args ...string) string { + t.Helper() + out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// No hook runs when a check makes its merge: not one in the clone, whatever the build machine's git configuration says. +func TestACheckMergeRunsNoHook(t *testing.T) { + repo, head, _ := aTrunkAndABranch(t, map[string]string{"a": "1"}, map[string]string{"b": "trunk"}, + map[string]string{"c": "head"}) + clone := filepath.Join(t.TempDir(), "clone") + if out, err := exec.Command("git", "clone", "--quiet", repo, clone).CombinedOutput(); err != nil { + t.Fatalf("%v\n%s", err, out) + } + ran := filepath.Join(t.TempDir(), "ran") + for _, hook := range []string{"pre-merge-commit", "commit-msg", "prepare-commit-msg", "post-merge"} { + if err := os.WriteFile(filepath.Join(clone, ".git", "hooks", hook), + []byte("#!/bin/sh\necho "+hook+" >> "+ran+"\n"), 0o755); err != nil { + t.Fatal(err) + } + } + m, err := mergeIntoTrunk(t.Context(), Command, clone, "main", head) + if err != nil || m.Merge == "" { + t.Fatalf("the merge was not made: %+v, %v", m, err) + } + if said, err := os.ReadFile(ran); err == nil { + t.Fatalf("a hook ran when the check made its merge: %s", said) + } +} diff --git a/internal/builder/replay341_test.go b/internal/builder/replay341_test.go new file mode 100644 index 00000000..031a903d --- /dev/null +++ b/internal/builder/replay341_test.go @@ -0,0 +1,139 @@ +package builder + +import ( + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestReplay341 is the replay of novox/hq issue 341, written with only what the build agent had before its fix, +// so mesh-lab's prover can lay it over the commit before. On 2026-10-09 node-engine #64/#65 merged at 00:15 and +// added Applied.Declared, under the JSON key `declared`, to the store record. #66, checked at 00:49 at its head, +// had added its own Applied.Declared under the same key on other lines. Its check passed; git merged the two +// edits without a conflict, and the trunk no longer compiled: "Declared redeclared". Before the fix the check +// judged the head alone and passed. After it (novox/hq ADR 0323), the check judges the head merged into the +// trunk commit that held the other field, and fails saying the trunk does not compile. +// +// It runs the check whole, with a container runtime that runs each container's command on this machine +// (aRuntimeOnThisMachine): the repository's own check is `go build ./...`. +func TestReplay341(t *testing.T) { + origin, head := theNinthOfOctober(t) + registry := aFactsRegistry(t) + aRuntimeOnThisMachine(t) + + v, err := Check(t.Context(), Command, CheckSpec{ID: "replay-341", Repository: origin, Ref: head, Owner: "novox", + Repo: "mesh-host", Number: 66, Base: "main", Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatalf("the check could not run: %v", err) + } + if v.Repo == nil || v.Repo.Verdict != "fail" { + t.Fatalf("node-engine #66 at its head was answered %+v: the merge it makes into the trunk that holds #65's "+ + "field does not compile, and the check let it merge", v.Repo) + } + if said := v.Repo.Summary + "\n" + v.Repo.Failed + "\n" + v.Report; !strings.Contains(said, "Declared redeclared") { + t.Fatalf("the check failed without saying the trunk does not compile:\n%s", said) + } +} + +// theNinthOfOctober is the node-engine's repository as it stood at 00:49 on 2026-10-09, in miniature: a trunk +// holding #65's Applied.Declared (a string, key `declared`), and #66's head, branched before it, holding its own +// Applied.Declared (a *FoundUnit, the same key) on other lines. Each compiles alone; their merge does not. +func theNinthOfOctober(t *testing.T) (string, string) { + t.Helper() + origin := t.TempDir() + git := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = origin + cmd.Env = append(os.Environ(), "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.org", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.org") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(name, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(filepath.Join(origin, name)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(origin, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + store := func(declared65, declared66 string) string { + return "package store\n\n// FoundUnit is a unit found on the machine.\ntype FoundUnit struct{ Name string }\n\n" + + "// Applied is what the node-engine applied.\ntype Applied struct {\n\tGeneration int `json:\"generation\"`\n" + + declared66 + "\tModules []string `json:\"modules\"`\n\tFiles []string `json:\"files\"`\n" + + "\tUnits []string `json:\"units\"`\n\tAt string `json:\"at\"`\n" + declared65 + "}\n" + } + git("init", "--quiet", "-b", "main") + write("go.mod", "module example.org/mesh-host\n\ngo 1.21\n") + write("internal/store/store.go", store("", "")) + write(CheckScript, "# mesh-check-toolchain: go\nset -e\nGOFLAGS= GOWORK=off GOTOOLCHAIN=local GOPROXY=off go build ./...\n") + git("add", "-A") + git("commit", "--quiet", "-m", "#63: the store record") + git("checkout", "--quiet", "-b", "adr-0269") + write("internal/store/store.go", store("", "\tDeclared *FoundUnit `json:\"declared\"`\n")) + git("commit", "--quiet", "-am", "#66: the unit a declaration names") + head := git("rev-parse", "HEAD") + git("checkout", "--quiet", "main") + write("internal/store/store.go", store("\tDeclared string `json:\"declared\"`\n", "")) + git("commit", "--quiet", "-am", "#65: the declaration applied") + return origin, head +} + +// aRuntimeOnThisMachine puts a container runtime on PATH that runs a container's command on this machine, in the +// directory and with the environment the check gives it: a store and a bus that answer, a judge that is never +// built, and the repository's own check run as written. Only for a check whose code is the test's own. +func aRuntimeOnThisMachine(t *testing.T) { + t.Helper() + bus, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { bus.Close() }) + go func() { + for { + c, err := bus.Accept() + if err != nil { + return + } + c.Close() + } + }() + dir := t.TempDir() + runtime := `#!/bin/sh +cmd="$1"; shift +case "$cmd" in +ps|rm|exec) exit 0 ;; +port) echo "` + bus.Addr().String() + `"; exit 0 ;; +run) + workdir=""; detached="" + while [ $# -gt 0 ]; do + case "$1" in + -d) detached=1; shift ;; + --rm) shift ;; + --workdir) workdir="$2"; shift 2 ;; + --env) export "$2"; shift 2 ;; + --network|--volume|--user|--label|--name|-p|-e) shift 2 ;; + *) break ;; + esac + done + if [ -n "$detached" ]; then echo replay-container; exit 0; fi + shift + if [ "$1" = "sh" ] && [ -n "$workdir" ]; then cd "$workdir" && exec "$@"; fi + echo "no such image on this machine" >&2 + exit 1 ;; +esac +exit 1 +` + if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(runtime), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} diff --git a/internal/inventory/plan_id_test.go b/internal/inventory/plan_id_test.go new file mode 100644 index 00000000..76323226 --- /dev/null +++ b/internal/inventory/plan_id_test.go @@ -0,0 +1,45 @@ +package inventory + +import ( + "strings" + "testing" + "time" +) + +// **A plan's id is a value, never SQL** (novox/hq issue 561): the id reaches PlanByID from a caller of the +// controller's plans verb, so an id holding a quote is read as a plain id — no error from the store, and no +// other plan answered for it. +func TestAPlanIdHoldingAQuoteIsReadAsAPlainId(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + p := Plan{ID: "plan-561", Repository: "novox/mesh-controller", Commit: "abc", Created: time.Now().UTC(), + State: PlanBuilding, Tiers: [][]string{{"a"}}, Modules: map[string]*PlanModule{"a": {}}} + if err := inv.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } + for _, id := range []string{ + "x' or '1'='1", // every plan, when pasted into the query + "plan-561' or id like '%", // the plan above, by a pattern + "it's", // a quote alone: a syntax error, when pasted + } { + got, err := inv.PlanByID(ctx, id) + if err == nil { + t.Fatalf("PlanByID(%q) answered %s, a plan of another id", id, got.ID) + } + if want := "no plan " + id; err.Error() != want { + t.Fatalf("PlanByID(%q) failed with %q, not as a plain id no plan has (%q)", id, err, want) + } + } + // A plan whose id holds a quote is found by it, and by nothing else. + quoted := p + quoted.ID, quoted.Revision = "plan-'561'", 0 + if err := inv.SavePlan(ctx, "ed); err != nil { + t.Fatal(err) + } + if got, err := inv.PlanByID(ctx, quoted.ID); err != nil || got.ID != quoted.ID { + t.Fatalf("the plan with a quote in its id was not read back: %v %+v", err, got.ID) + } + if recent, err := inv.RecentPlans(ctx, 1); err != nil || len(recent) != 1 || !strings.HasPrefix(recent[0].ID, "plan-") { + t.Fatalf("RecentPlans with a parameter: %v %+v", err, recent) + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index b6530854..d6933d04 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -623,7 +623,7 @@ func (i *Inventory) PlansSince(ctx context.Context, since time.Time) ([]Plan, er // RecentPlans is the last few plans, newest first, open or not — what the overview shows. func (i *Inventory) RecentPlans(ctx context.Context, limit int) ([]Plan, error) { - return i.plans(ctx, fmt.Sprintf(`order by created desc limit %d`, limit)) + return i.plans(ctx, `order by created desc limit $1`, limit) } // NewestMergeOf is the plan, in any state, of the newest merge into a repository's branch that the mesh @@ -638,9 +638,10 @@ func (i *Inventory) NewestMergeOf(ctx context.Context, repository, branch string return plans[0], true, nil } -// PlanByID is one plan. +// PlanByID is one plan. The id is a parameter, never part of the query (novox/hq issue 561): it comes from a +// caller of the plans verb. func (i *Inventory) PlanByID(ctx context.Context, id string) (Plan, error) { - plans, err := i.plans(ctx, `where id = '`+id+`'`) + plans, err := i.plans(ctx, `where id = $1`, id) if err != nil { return Plan{}, err } diff --git a/internal/link/build.go b/internal/link/build.go index aa90165e..33f4c372 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -138,6 +138,9 @@ type GroupMember struct { Repository string `json:"repository"` Ref string `json:"ref"` Paths []string `json:"paths,omitempty"` + // Base is the branch it merges into: the build seat merges the head into that trunk's head before the + // group is composed (novox/hq ADR 0323 §6). A build seat from before it composes the head alone. + Base string `json:"base,omitempty"` } // Who judges a merge check's gate. @@ -164,6 +167,14 @@ type CheckOutcome struct { // Gate and RepoCheck are its two layers; Verdict and Summary above are the gate's. Gate *CheckLayer `json:"gate,omitempty"` RepoCheck *CheckLayer `json:"repo-check,omitempty"` + // Head and Trunk are what was judged (novox/hq ADR 0323 §3): the pull request's head merged into the trunk + // commit, each as twelve characters. Trunk is empty from a build seat from before it, which judged the head + // alone. Beside are the commits of the repositories read beside it, by directory, named and not judged + // against (issue 568); Members a delivery group's other heads, each with the trunk commit it merged into. + Head string `json:"head,omitempty"` + Trunk string `json:"trunk,omitempty"` + Beside map[string]string `json:"beside,omitempty"` + Members []CheckedMember `json:"members,omitempty"` } // SourceOnSeat names a repository by the seat whose holder serves it and its path there. diff --git a/internal/link/events.go b/internal/link/events.go index eab227d9..ab2a511e 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -221,13 +221,17 @@ type SourceMoved struct { // PullUpdated is what the forge announces when an open pull request's head moves — opened, or pushed // to (novox/hq to-be 45 §9): what the controller asks the build seat to check before it merges. type PullUpdated struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Number int `json:"number"` - Title string `json:"title,omitempty"` - Base string `json:"base"` - Head string `json:"head"` - Commit string `json:"head_sha"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Title string `json:"title,omitempty"` + Base string `json:"base"` + Head string `json:"head"` + Commit string `json:"head_sha"` + // BaseSHA is the base branch's head when the forge announced the head (novox/hq ADR 0323): the trunk commit a + // verdict the controller gives without the build seat — a change that runs no build — is judged against. Empty + // from a forge module from before it. + BaseSHA string `json:"base_sha,omitempty"` CloneURL string `json:"clone_url"` HTMLURL string `json:"html_url,omitempty"` // Paths are the files the pull request changes; PathsTruncated says there were more. @@ -278,14 +282,24 @@ type Checked struct { // status from it — the group's verdict is its owner's, mesh-delivery, to say on each member's head. Group string `json:"group,omitempty"` Members []CheckedMember `json:"members,omitempty"` + // Trunk is the trunk commit the head was merged into and judged as, twelve characters (novox/hq ADR 0323 + // §3): the verdict holds for that merge only, and goes out of date when the trunk moves. Empty from a build + // seat that judged the head alone. Beside are the commits of the other repositories the check read, by + // directory, named and not judged against (issue 568). Took is how long the check ran, for the merge tool + // to say how long the next one may take (ADR 0323 §5). + Trunk string `json:"trunk,omitempty"` + Beside map[string]string `json:"beside,omitempty"` + Took string `json:"took,omitempty"` } -// CheckedMember is one head a group's composed check judged. +// CheckedMember is one head a group's composed check judged, and the trunk commit it was merged into (novox/hq +// ADR 0323 §6), twelve characters; empty from a build seat that composed the head alone. type CheckedMember struct { Owner string `json:"owner"` Repo string `json:"repo"` Number int `json:"number,omitempty"` Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` } // ChangePlan is what a change does to the mesh, computed from its diffset — a repository, the branch it @@ -311,6 +325,9 @@ type ChangePlan struct { Steps []string `json:"steps,omitempty"` // Summary is the plan in one line, for a commit status. Summary string `json:"summary"` + // Trunk is the trunk commit the head was merged into when it was judged (novox/hq ADR 0323 §3), twelve + // characters: set on the plan a verdict carries, which mesh-delivery keeps with its other inputs. + Trunk string `json:"trunk,omitempty"` } // MachinePlan is one machine's part of a change plan. diff --git a/testdata/store-tests b/testdata/store-tests index c510f6d9..e3ec633f 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -13,6 +13,7 @@ ./internal/inventory TestAnIssuedValueIsSealedToEveryHolderOnceAndNotAgain ./internal/inventory TestAnIssuedValueIsSealedAgainAfterAKeyChangeOrAnAcceptance ./internal/inventory TestAConditionsLooksAreKeptBesideTheBucket +./internal/inventory TestAPlanIdHoldingAQuoteIsReadAsAPlainId ./cmd/mesh-controller TestTheConditionsVerbReadsTheLooksTheBucketNoLongerHolds ./internal/inventory TestAWalksStartIsKeptAndARefusedSaveLeavesItUnsaid ./cmd/mesh-controller TestAWalkLetGoAndStoppedByVerbsIsSaidStartedAndEnded