diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 7bf565be..be469d09 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -332,6 +332,12 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri result.Check.Members = append(result.Check.Members, link.CheckedMember{Owner: m.Owner, Repo: m.Repo, Commit: m.Head, Trunk: m.Trunk}) } + // A branch tried before its pull request reads its outcome from this log, whole (novox/hq issue 686). + if request.Check.Try { + if line, err := json.Marshal(triedOf(v)); err == nil { + say(link.StepTried, string(line)) + } + } } } else if err == nil { // The package-registry credential is a build input, so it is resolved before the clone: a @@ -457,7 +463,8 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { spec := builder.CheckSpec{ID: request.ID, Repository: request.Repository, Ref: request.Ref, Owner: c.Owner, Repo: c.Repo, Number: c.Number, Paths: c.Paths, Beside: map[string]builder.Beside{}, Modules: c.Modules, New: c.New, Manifests: c.Manifests, Judge: c.Judge, Base: c.Base, - Toolchain: builder.ToolchainOf(request.Held), Toolchains: builder.ToolchainsOf(request.Held)} + Toolchain: builder.ToolchainOf(request.Held), Toolchains: builder.ToolchainsOf(request.Held), + Tests: c.Tests, StoreOnly: c.StoreOnly} for dir, b := range c.Beside { spec.Beside[dir] = builder.Beside{Repository: b.Repository, Ref: b.Ref} } @@ -472,6 +479,23 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { return spec } +// triedOf is a try's outcome as its log line says it (novox/hq issue 686): the repository's own check's verdict, +// or the check's where that did not run — a head that does not merge — and the lines naming what failed. +func triedOf(v builder.CheckVerdict) link.Tried { + t := link.Tried{Verdict: v.Verdict, Summary: v.Summary, Took: v.Took.Round(time.Second).String(), + Head: v.Head, Trunk: v.Trunk} + if v.Repo != nil && (v.Gate == nil || v.Gate.Verdict == "pass") { + t.Verdict, t.Summary = v.Repo.Verdict, v.Repo.Summary + } + if v.Repo != nil && v.Repo.Failed != "" { + t.Failed = strings.Split(v.Repo.Failed, "\n") + } + if t.Verdict == "" { + t.Verdict = "error" + } + return t +} + // checkBuiltOf is a check's builds for its merge as the outcome carries them (novox/hq ADR 0350 §1). func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt { var out []link.CheckBuilt diff --git a/cmd/mesh-builder/tried_test.go b/cmd/mesh-builder/tried_test.go new file mode 100644 index 00000000..06aaf706 --- /dev/null +++ b/cmd/mesh-builder/tried_test.go @@ -0,0 +1,31 @@ +package main + +import ( + "testing" + "time" + + "github.com/novox/mesh-controller/internal/builder" +) + +// A try's outcome, as the build seat says it in the build's log (novox/hq issue 686): the repository's own +// check's verdict, how long it took, and each line naming what failed. +func TestATrysOutcomeNamesWhatFailed(t *testing.T) { + tried := triedOf(builder.CheckVerdict{Verdict: "pass", Summary: "the change touches no module", Took: 303 * time.Second, + Head: "c0ffee", Trunk: "ca3f43c", Gate: &builder.Layer{Verdict: "pass"}, + Repo: &builder.Layer{Verdict: "fail", Summary: "its merge-check.sh failed: TestX", + Failed: "--- FAIL: TestX (0.01s)\n x_test.go:12: got 1, want 2"}}) + if tried.Verdict != "fail" || tried.Took != "5m3s" || len(tried.Failed) != 2 || tried.Trunk != "ca3f43c" || + tried.Summary != "its merge-check.sh failed: TestX" { + t.Fatalf("a failed try: %+v", tried) + } + // A head that does not merge fails both layers: the check's own word. + conflict := triedOf(builder.CheckVerdict{Verdict: "fail", Summary: "does not merge", Gate: &builder.Layer{Verdict: "fail"}, + Repo: &builder.Layer{Verdict: "fail", Summary: "does not merge"}}) + if conflict.Verdict != "fail" || conflict.Summary != "does not merge" { + t.Fatalf("a try that does not merge: %+v", conflict) + } + // No verdict is an error, never a pass. + if none := triedOf(builder.CheckVerdict{}); none.Verdict != "error" { + t.Fatalf("a try with no verdict: %+v", none) + } +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 1e656622..41d7319d 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -106,6 +106,8 @@ func run() error { return killCommand(ctx, args[1:]) case "forget-checks": return forgetChecksCommand(ctx, args[1:]) + case "try": + return tryCommand(ctx, args[1:]) case "pause", "resume": return pauseCommand(ctx, args[0], args[1:]) case "collection": @@ -381,6 +383,13 @@ func (b builds) Built(ctx context.Context, result link.BuildResult) error { // **A merge check builds nothing** (novox/hq to-be 45 §9): its verdict is said, and nothing of it is // recorded or registered. if result.Check != nil || result.Checked != nil { + if isTry(result) { + // A branch tried before its pull request (novox/hq issue 686): its asker reads the outcome from the + // build's log. Never said as `checked`, so the forge's holder sets no status. + fmt.Printf("%s: a try of %s at %.8s, read with `try --id`: said to nobody, nothing taken in\n", result.ID, + result.Repository, result.Ref) + return nil + } checked(ctx, result) return nil } diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3cc62d1a..c0c222e3 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -457,6 +457,28 @@ func (a *verbArguments) commandLine() ([]string, error) { return nil, err } return []string{"forget-checks", str("owner"), str("repo"), str("number")}, nil + case "try": + // A branch tried before its pull request (novox/hq issue 686): asked, or read by the id it answered. + if id := str("id"); id != "" { + return []string{"try", "--id", id}, nil + } + if a.given["repository"] == "" && a.given["ref"] == "" { + // Neither shape named: the command says its usage, which names both, as `build` does. + return []string{"try"}, nil + } + if err := need("repository", "ref"); err != nil { + return nil, err + } + argv := []string{"try", str("repository"), str("ref")} + for _, flag := range []string{"base", "tests"} { + if v := str(flag); v != "" { + argv = append(argv, "--"+flag, v) + } + } + if on("store-only") { + argv = append(argv, "--store-only") + } + return argv, nil case "cancel", "kill": if err := need("id"); err != nil { return nil, err @@ -1018,6 +1040,8 @@ var jsonVerbs = map[string]bool{"status": true, "seats": true, "plan": true, "co "delivery": true, // The build queue, each ask with its position among the waiting (novox/hq issue 455). "queue": true, + // A branch tried before its pull request (novox/hq issue 686). + "try": true, // A plan followed (novox/hq issue 229): `plans wait` answers JSON; its other forms say text, and so answer no data. "plans": true} @@ -1308,6 +1332,10 @@ func seatToolHandlers() (map[string]link.ToolHandler, []string, error) { link.AnswersWithin(ctx, bound+planWaitStart) } } + if verb == "try" && len(argv) > 1 && argv[1] == "--id" { + // Reading a try holds its answer for the verdict, up to its bound (novox/hq issue 686, 229). + link.AnswersWithin(ctx, tryWait+planWaitStart) + } if answer, read := readHere(ctx, argv); read { return answer, nil } diff --git a/cmd/mesh-controller/try.go b/cmd/mesh-controller/try.go new file mode 100644 index 00000000..a120385c --- /dev/null +++ b/cmd/mesh-controller/try.go @@ -0,0 +1,353 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "flag" + "fmt" + "net/url" + "os" + "regexp" + "strings" + "time" + + "github.com/nats-io/nats.go" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/link" +) + +// A branch is tried before its pull request (novox/hq issue 686). +// +// A repository's store tests skip without a store, and no agent may start a database on its machine. So a +// builder pushed, a reviewer read, and only the pull request's merge check ran those tests for the first time: +// each failure there cost a round of fifteen to thirty minutes (mesh-controller #283, #310, #312). +// +// `try / ` asks the build seat for the repository's own check of a pushed branch or +// commit, as the merge check runs it (internal/builder/check.go): the ref merged into the trunk's head, the +// trunk's merge-check.sh, each part in its toolchain's container, against a throwaway store and bus. It is a +// check and nothing more: +// +// - **no pull request, so no status**: its outcome is never said as `checked`, which is what the forge's +// holder sets statuses from (isTry, in Built); +// - **nothing registered, nothing built**: it names no module of the graph, so the merge gate does not run and +// no build is made for a merge (novox/hq ADR 0350's scratch builds are a pull request's check's alone); +// - **a check in the build queue**: after deliveries (link.KindOf, novox/hq issue 558), and bounded by the +// check's own bound (checkTimeout); +// - **the code a pull request's check would run anyway**: only a repository of the mesh, whose every pull +// request the build seat already checks, in containers that hold no runtime socket. +// +// A check outlasts a seat verb's one answer, so the ask answers its id at once and `try --id ` reads what +// came of it from the build's log, where the build seat says it whole (link.StepTried): holding its answer up +// to tryWait for a verdict, like `plans wait` (novox/hq issue 229). Nothing is kept beside the log. + +// tryWait is how long reading a try holds its answer for the verdict. A variable so a test need not wait. +var tryWait = 20 * time.Second + +// A ref as the forge is asked for it — a branch or a commit, never an option — and a `go test -run` pattern. +var ( + tryRef = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]*$`) + tryTests = regexp.MustCompile(`^[A-Za-z0-9_^$|().*+?/\[\]-]+$`) +) + +// tryAnswer is what `try` answers, asking or reading. +type tryAnswer struct { + ID string `json:"id"` + // State is asked (just now), waiting (nothing said of it yet: in the build queue), running, or the verdict: + // pass, warning (the repository declares no merge-check.sh), fail, or error (it could not run). + State string `json:"state"` + Said string `json:"said"` + Repository string `json:"repository,omitempty"` + Ref string `json:"ref,omitempty"` + Commit string `json:"commit,omitempty"` + Base string `json:"base,omitempty"` + Trunk string `json:"trunk,omitempty"` + On string `json:"on,omitempty"` + Took string `json:"took,omitempty"` + Failed []string `json:"failed,omitempty"` + // Last is the last thing the build seat said of one still running. + Last string `json:"last,omitempty"` +} + +// isTry says a check's outcome is a try's: marked so, or — echoed by a build seat from before the mark — a +// check of no pull request and no group, which is nobody's status either. +func isTry(result link.BuildResult) bool { + c := result.Checked + return c != nil && (c.Try || (c.Number == 0 && c.Group == "")) +} + +// asATry makes a check's ask a try's: the repository's own script alone, for no pull request. +func asATry(request link.BuildRequest, tests string, storeOnly bool) link.BuildRequest { + c := *request.Check + c.Number, c.Try, c.Tests, c.StoreOnly = 0, true, tests, storeOnly + c.Paths, c.Modules, c.New, c.Dependents, c.Manifests, c.Builds = nil, nil, nil, nil, nil, nil + c.Judge, c.Plan, c.Group, c.Members = "", nil, "", nil + request.Check = &c + return request +} + +// maxTries is how many tries may wait or run at once, mesh-wide: each holds a build machine for minutes. +const maxTries = 4 + +// triesIn reads the build queue for a new try of one commit: the id of a try of that commit already waiting or +// running — the answer to a second ask — and whether there is room for another. +func triesIn(q link.Queue, owner, repo, commit string) (same string, room bool) { + n := 0 + for _, a := range q.Asks { + if !link.IsTry(a.Request) || (a.State != link.AskWaiting && a.State != link.AskInFlight) { + continue + } + n++ + c := a.Request.Check + if same == "" && strings.EqualFold(c.Owner, owner) && strings.EqualFold(c.Repo, repo) && a.Request.Ref == commit { + same = a.ID + } + } + return same, n < maxTries +} + +// forgeCommit asks the forge, through its module's tools on the bus, which commit a branch or commit of a +// repository is, and the repository's default branch. A variable so a test needs no forge. +var forgeCommit = func(ctx context.Context, owner, repo, ref string) (commit, trunk string, err error) { + js, err := aBus() + if err != nil { + return "", "", err + } + defer js.Close() + bus := link.OverNATS{Conn: js.Conn()} + ask := func(path string, into any) error { + raw, _ := json.Marshal(map[string]any{"path": path}) + answer, err := link.Ask(ctx, bus, "gitea", "gitea_api", raw, 30*time.Second) + if err != nil { + return err + } + if answer.Error != "" { + return errors.New(answer.Error) + } + return json.Unmarshal(answer.Result, into) + } + var commits struct { + Result []struct { + SHA string `json:"sha"` + } `json:"result"` + } + if err := ask("/repos/"+owner+"/"+repo+"/commits?limit=1&stat=false&verification=false&files=false&sha="+ + url.QueryEscape(ref), &commits); err != nil { + return "", "", err + } + if len(commits.Result) == 0 || commits.Result[0].SHA == "" { + return "", "", errors.New("the forge named no commit for it") + } + var found struct { + Result struct { + DefaultBranch string `json:"default_branch"` + } `json:"result"` + } + if err := ask("/repos/"+owner+"/"+repo, &found); err != nil { + return "", "", err + } + return commits.Result[0].SHA, found.Result.DefaultBranch, nil +} + +// tryCommand is `try / [--base ] [--tests ] [--store-only]`, and +// `try --id ` to read what came of one. +func tryCommand(ctx context.Context, args []string) error { + set := flag.NewFlagSet("try", flag.ContinueOnError) + base := set.String("base", "", "the branch it is merged into (the repository's default branch when none)") + tests := set.String("tests", "", "only the tests this `go test -run` pattern names") + storeOnly := set.Bool("store-only", false, "only the repository's store tests") + id := set.String("id", "", "a try's id: what came of it") + positionals, err := parseAround(set, args) + if err != nil { + return err + } + usage := errors.New("try / [--base ] [--tests ] " + + "[--store-only], or try --id ") + if *id != "" { + if len(positionals) != 0 { + return usage + } + // Before it is a token of a bus subject: an id, never a pattern. + if !link.SafeBuildID(*id) { + return fmt.Errorf("%q is not a try's id", *id) + } + return tryRead(ctx, *id) + } + if len(positionals) != 2 { + return usage + } + owner, repo, isPath := strings.Cut(positionals[0], "/") + ref := positionals[1] + switch { + case !isPath || !tryRef.MatchString(owner) || !tryRef.MatchString(repo) || strings.Contains(repo, "/"): + return fmt.Errorf("%q is not a repository on the forge, owner/name; nothing was asked", positionals[0]) + case !tryRef.MatchString(ref): + return fmt.Errorf("%q is not a branch or a commit; nothing was asked", ref) + case *base != "" && !tryRef.MatchString(*base): + return fmt.Errorf("%q is not a branch; nothing was asked", *base) + case *tests != "" && !tryTests.MatchString(*tests): + return fmt.Errorf("%q is not a `go test -run` pattern of test names; nothing was asked", *tests) + } + // **An unknown ref is refused here, loudly** — not sent to a machine to fail at its clone minutes later. + commit, trunk, err := forgeCommit(ctx, owner, repo, ref) + if err != nil { + return fmt.Errorf("the forge holds no branch or commit %s in %s/%s — is it pushed? — so nothing was asked: %w", + ref, owner, repo, err) + } + if *base != "" { + trunk = *base + } + if trunk == "" { + return fmt.Errorf("the forge named no default branch for %s/%s, and no --base says what %s is merged into; "+ + "nothing was asked", owner, repo, ref) + } + + open, err := openStores(ctx) + if err != nil { + return err + } + defer open.Close() + entries, read, edges, err := theGraph(ctx, open.inventory) + if err != nil { + return err + } + p := link.PullUpdated{Owner: owner, Repo: repo, Base: trunk, Commit: commit} + // Whose repository it is, as a pull request's check reckons it: only the mesh's is tried, the code the + // build seat runs for every pull request already. + mesh := pullScope(p, entries, read, edges) + if !mesh.Mesh { + return fmt.Errorf("%s/%s is no repository of the mesh — no module is built from it and its owner is not "+ + "the core's — and only the mesh's are tried on the build seat; nothing was asked", owner, repo) + } + request, err := checkRequestFor(ctx, open, p, checkScope{Mesh: true, From: mesh.From}, entries) + if err != nil { + return err + } + request = asATry(request, *tests, *storeOnly) + seat := buildSeatHeld(ctx) + // **Bounded in number** (novox/hq issue 686): one try of a commit at a time, and maxTries in all. + js, err := dialTheBus() + if err != nil { + return err + } + queue, err := link.ReadQueue(ctx, js, seat) + js.Close() + if err != nil { + return err + } + same, room := triesIn(queue, owner, repo, commit) + if same != "" { + return printTry(tryAnswer{ID: same, State: "asked", Repository: owner + "/" + repo, Ref: ref, Commit: commit, + On: seat, Said: fmt.Sprintf("a try of %s/%s at %.12s already waits or runs, as %s: nothing new was asked. "+ + "Read it with try id=%s", owner, repo, commit, same, same)}) + } + if !room { + return fmt.Errorf("%d tries already wait or run on the build seat, the most at once (`queue` lists them); "+ + "nothing was asked — read one of yours, or ask again when one has ended", maxTries) + } + ask, err := askOverOn(seat) + if err != nil { + return err + } + defer ask.Close() + if err := ask.Ask(ctx, request); err != nil { + return err + } + return printTry(tryAnswer{ID: request.ID, State: "asked", Repository: owner + "/" + repo, Ref: ref, Commit: commit, + Base: trunk, On: seat, Said: fmt.Sprintf("asked %s to try %s/%s at %.12s (%s) merged into %s: its own "+ + "merge-check.sh, as a check in the build queue, bounded at %s; no status is set and nothing is registered. "+ + "Read it with try id=%s — a full run of this kind takes minutes", seat, owner, repo, commit, ref, trunk, + checkTimeout, request.ID)}) +} + +// tryRead answers what came of a try, holding the answer up to tryWait for its verdict. +func tryRead(ctx context.Context, id string) error { + until := time.Now().Add(tryWait) + for { + lines, err := buildLines(ctx, id) + if err != nil { + return err + } + a := tryAnswerOf(id, lines) + if (a.State != "waiting" && a.State != "running") || !time.Now().Add(2*time.Second).Before(until) { + return printTry(a) + } + if err := sleepOrDone(ctx, 2*time.Second); err != nil { + return err + } + } +} + +// tryAnswerOf reads a try's answer from what the build seat said of it: its outcome under StepTried; `failed` +// for one that could not run — an error, never a pass; else how far it is. +func tryAnswerOf(id string, lines []link.BuildLine) tryAnswer { + a := tryAnswer{ID: id, State: "waiting", Said: "nothing is said of " + id + " yet: it waits in the build queue " + + "(`queue` says where), or no try has that id; ask again"} + for _, l := range lines { + switch l.Step { + case link.StepTried: + var t link.Tried + if err := json.Unmarshal([]byte(l.Message), &t); err != nil || t.Verdict == "" { + return tryAnswer{ID: id, State: "error", Said: "the build seat's word on it cannot be read: " + l.Message} + } + return tryAnswer{ID: id, State: t.Verdict, Said: t.Summary, Took: t.Took, Commit: t.Head, Trunk: t.Trunk, + Failed: t.Failed} + case "failed": + return tryAnswer{ID: id, State: "error", Said: "it could not run: " + l.Message} + case "checked": + // A check that is no try, or a build seat from before the verb: its verdict is not a try's. + return tryAnswer{ID: id, State: "error", Said: "the build seat checked it and said no try's outcome — " + + "it predates the verb (novox/hq issue 686), or " + id + " is no try: " + l.Message + "; `builds` with " + + "the id says every line"} + } + a.State, a.Last = "running", l.Message + a.Said = "still running; ask again. `builds` with the id says every line so far" + } + return a +} + +func printTry(a tryAnswer) error { + fmt.Fprintln(os.Stderr, a.Said) + for _, l := range a.Failed { + fmt.Fprintln(os.Stderr, " "+l) + } + out, err := json.MarshalIndent(a, "", " ") + if err != nil { + return err + } + fmt.Println(string(out)) + return nil +} + +// buildLines is everything the build seat said of one build, as the bus keeps it, in order. +func buildLines(ctx context.Context, id string) ([]link.BuildLine, error) { + js, err := aBus() + if err != nil { + return nil, fmt.Errorf("cannot reach the bus to read a build's log: %w", err) + } + defer js.Close() + subject := link.BuildLogOf("*", id) + sub, err := js.Context().PullSubscribe(subject, "", + nats.BindStream(broker.EventsStream), nats.DeliverAll(), nats.AckNone()) + if err != nil { + return nil, fmt.Errorf("cannot read %s from the bus: %w", subject, err) + } + defer func() { _ = sub.Unsubscribe() }() + var lines []link.BuildLine + for { + batch, err := sub.Fetch(200, nats.MaxWait(2*time.Second)) + if err != nil && !errors.Is(err, nats.ErrTimeout) && !errors.Is(err, context.DeadlineExceeded) { + return nil, fmt.Errorf("reading a build's log: %w", err) + } + for _, msg := range batch { + var line link.BuildLine + if json.Unmarshal(msg.Data, &line) == nil { + lines = append(lines, line) + } + } + if len(batch) < 200 { + return lines, nil + } + } +} diff --git a/cmd/mesh-controller/try_test.go b/cmd/mesh-controller/try_test.go new file mode 100644 index 00000000..5f72bcda --- /dev/null +++ b/cmd/mesh-controller/try_test.go @@ -0,0 +1,194 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "reflect" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/link" +) + +// A branch is tried before its pull request (novox/hq issue 686): the repository's own check asked for a pushed +// branch or commit, with no pull request. + +// What a try asks is a check — after deliveries in the build queue, bounded as one — of the repository's own +// script alone: no module of the graph, so no gate; nothing built for a merge, so no reusable image. +func TestATryAsksACheckOfTheRepositorysOwnScriptAlone(t *testing.T) { + request := link.BuildRequest{ID: "build-1", Ref: "c0ffee", Check: &link.CheckRequest{Owner: "novox", Repo: "a", + Number: 7, Base: "main", Modules: []string{"a"}, New: []string{"b"}, Manifests: []string{"module.json"}, + Dependents: []string{"c"}, Builds: []link.CheckBuild{{Module: "a"}}, Judge: link.JudgeSelf, + Plan: &link.ChangePlan{}}} + tried := asATry(request, "^TestStore", true) + c := tried.Check + if !c.Try || c.Number != 0 || c.Tests != "^TestStore" || !c.StoreOnly { + t.Fatalf("not marked a try, or it kept a pull request's number: %+v", c) + } + if len(c.Modules)+len(c.New)+len(c.Manifests)+len(c.Dependents)+len(c.Builds) != 0 || c.Judge != "" || c.Plan != nil { + t.Fatalf("a try runs the gate or builds for a merge: %+v", c) + } + if c.Base != "main" || c.Owner != "novox" || tried.Ref != "c0ffee" { + t.Fatalf("a try lost what it merges into what: %+v", tried) + } + if link.KindOf(tried) != link.KindCheck { + t.Fatal("a try does not count as a check in the build queue") + } +} + +// A try's outcome is heard and left there: no `checked` is said, so the forge's holder sets no status, and +// nothing is recorded or registered (the handler holds no store here, and would fail reaching for one). +func TestATrysOutcomeSetsNoStatusAndRegistersNothing(t *testing.T) { + said := &saidOnTheBus{} + checkEvents = said + defer func() { checkEvents = nil }() + for name, result := range map[string]link.BuildResult{ + "a try": {ID: "build-1", Ref: "c0ffee", Checked: &link.CheckRequest{Owner: "novox", Repo: "a", Try: true}, + Check: &link.CheckOutcome{Verdict: "pass", RepoCheck: &link.CheckLayer{Verdict: "fail", Summary: "failed"}}}, + "a try that could not run": {ID: "build-2", Checked: &link.CheckRequest{Owner: "novox", Repo: "a", Try: true}, + Failed: "cannot clone"}, + // A build seat from before the mark echoes the ask without it: a check of no pull request and no + // group is still nobody's status. + "a try echoed by an older build seat": {ID: "build-3", Checked: &link.CheckRequest{Owner: "novox", Repo: "a"}, + Check: &link.CheckOutcome{Verdict: "pass"}}, + } { + if err := (builds{}).Built(context.Background(), result); err != nil { + t.Fatalf("%s: %v", name, err) + } + } + if len(said.events) != 0 { + t.Fatalf("a try's outcome was said for the forge: %v", said.events) + } + // A pull request's check is still said. + _ = (builds{}).Built(context.Background(), link.BuildResult{ID: "build-4", Checked: &link.CheckRequest{ + Owner: "novox", Repo: "a", Number: 7}, Check: &link.CheckOutcome{Verdict: "pass"}}) + if len(said.events) != 1 { + t.Fatalf("a pull request's check was said %d time(s)", len(said.events)) + } +} + +// saidOnTheBus keeps the seat events said, and nothing else. +type saidOnTheBus struct { + link.Bus + events []string +} + +func (s *saidOnTheBus) PublishSeatEvent(_ context.Context, seat, event string, _ []byte) error { + s.events = append(s.events, seat+" "+event) + return nil +} + +// A ref the forge does not hold is refused before anything is asked, naming it. +func TestATryOfAnUnknownRefIsRefusedLoudly(t *testing.T) { + was := forgeCommit + defer func() { forgeCommit = was }() + forgeCommit = func(_ context.Context, owner, repo, ref string) (string, string, error) { + return "", "", errors.New("404 object does not exist [id: " + ref + "]") + } + err := tryCommand(context.Background(), []string{"novox/mesh-controller", "fix/no-such-branch"}) + if err == nil { + t.Fatal("a try of a ref the forge does not hold was asked") + } + for _, want := range []string{"fix/no-such-branch", "novox/mesh-controller", "nothing was asked", "404"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not say %q: %v", want, err) + } + } + for _, args := range [][]string{{"mesh-controller", "main"}, {"novox/mesh-controller"}, {"novox/a", "--upload-pack=x"}, + {"novox/a", "main", "--tests", "x; rm -rf /"}, {"novox/a", "main", "--id", "build-1"}} { + if err := tryCommand(context.Background(), args); err == nil { + t.Errorf("try %v was taken", args) + } + } +} + +// The answer of a try carries the lines that name what failed (novox/hq issue 460's shape) and how long it took. +func TestATrysAnswerCarriesTheFailingLines(t *testing.T) { + tried, _ := json.Marshal(link.Tried{Verdict: "fail", Summary: "its merge-check.sh failed: TestX", Took: "5m3s", + Head: "c0ffee", Trunk: "ca3f43c", Failed: []string{"--- FAIL: TestX (0.01s)", " x_test.go:12: got 1, want 2", + "FAIL\tgithub.com/novox/a/cmd\t3.1s"}}) + lines := []link.BuildLine{{Seq: 1, Step: "check", Message: "novox/a#0 at c0ffee"}, + {Seq: 2, Step: "output", Message: "ok github.com/novox/a/internal 1.2s"}, + {Seq: 3, Step: link.StepTried, Message: string(tried)}, {Seq: 4, Step: "checked", Message: "pass: x"}} + a := tryAnswerOf("build-1", lines) + if a.State != "fail" || a.Took != "5m3s" || len(a.Failed) != 3 || a.Failed[0] != "--- FAIL: TestX (0.01s)" || + a.Trunk != "ca3f43c" || a.ID != "build-1" { + t.Fatalf("the answer lost the verdict, the time or the failing lines: %+v", a) + } + // Not answered yet: said as running, with how far it is — never as a pass. + if a := tryAnswerOf("build-1", lines[:2]); a.State != "running" || a.Last == "" { + t.Fatalf("a try still running: %+v", a) + } + if a := tryAnswerOf("build-1", nil); a.State != "waiting" { + t.Fatalf("a try nothing was said of: %+v", a) + } + // One that could not run is an error with the build seat's word, never a pass. + if a := tryAnswerOf("build-1", append(lines[:1:1], link.BuildLine{Seq: 2, Step: "failed", + Message: "cannot clone"})); a.State != "error" || !strings.Contains(a.Said, "cannot clone") { + t.Fatalf("a try that could not run: %+v", a) + } +} + +// One try of a commit at a time, and four in all (novox/hq issue 686): a second ask is answered the first's id, +// a fifth is refused; a try that ended, and a pull request's check, count for nothing. +func TestTriesAreBoundedInNumber(t *testing.T) { + ask := func(id, repo, ref, state string, try bool) link.QueuedAsk { + return link.QueuedAsk{ID: id, State: state, Request: link.BuildRequest{ID: id, Ref: ref, + Check: &link.CheckRequest{Owner: "novox", Repo: repo, Try: try}}} + } + q := link.Queue{Asks: []link.QueuedAsk{ + ask("t1", "a", "c1", link.AskInFlight, true), ask("t2", "a", "c2", link.AskWaiting, true), + ask("t3", "b", "c1", link.AskDead, true), ask("p1", "a", "c9", link.AskWaiting, false)}} + if same, room := triesIn(q, "novox", "a", "c2"); same != "t2" || !room { + t.Fatalf("a second try of a commit: %q %v", same, room) + } + if same, room := triesIn(q, "novox", "b", "c1"); same != "" || !room { + t.Fatalf("a dead try, or another repository's, was taken for this one: %q %v", same, room) + } + if same, _ := triesIn(q, "novox", "a", "c9"); same != "" { + t.Fatalf("a pull request's check was taken for a try: %q", same) + } + q.Asks = append(q.Asks, ask("t4", "c", "c3", link.AskWaiting, true), ask("t5", "d", "c4", link.AskWaiting, true)) + if _, room := triesIn(q, "novox", "e", "c5"); room { + t.Fatal("a fifth try was let in") + } + // An id is checked before it is a token of a bus subject. + for _, id := range []string{"x.>", "*", "a b"} { + if err := tryCommand(context.Background(), []string{"--id", id}); err == nil || + !strings.Contains(err.Error(), "not a try's id") { + t.Errorf("try --id %q: %v", id, err) + } + } +} + +// The verb runs its command, and is declared with the shell habit it replaces (novox/hq ADR 0245). +func TestTheTryVerbRunsItsCommand(t *testing.T) { + for _, c := range []struct { + args map[string]any + want []string + }{ + {map[string]any{"repository": "novox/a", "ref": "fix/x"}, []string{"try", "novox/a", "fix/x"}}, + {map[string]any{"repository": "novox/a", "ref": "fix/x", "base": "main", "tests": "^TestX$", "store-only": "true"}, + []string{"try", "novox/a", "fix/x", "--base", "main", "--tests", "^TestX$", "--store-only"}}, + {map[string]any{"id": "build-1"}, []string{"try", "--id", "build-1"}}, + } { + got, err := argvFor("try", c.args) + if err != nil || !reflect.DeepEqual(got, c.want) { + t.Errorf("try %v: %v %v, want %v", c.args, got, err, c.want) + } + } + if _, err := argvFor("try", map[string]any{"repository": "novox/a"}); err == nil { + t.Error("a try of no ref was taken") + } + for _, v := range catalogue.ControllerVerbs { + if v.Name == "try" { + if len(v.Replaces) == 0 { + t.Error("try names no shell habit it replaces") + } + return + } + } + t.Error("try is not a verb of the controller seat") +} diff --git a/internal/builder/check.go b/internal/builder/check.go index bf0ce5bc..a2107499 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -103,6 +103,23 @@ type CheckSpec struct { // for the walk to take. None for a group's composed check, whose members' own checks build. Builds []CheckBuildSpec BuildWith *CheckBuilding + // Tests and StoreOnly narrow what the repository's own script runs, for a branch tried before its pull + // request (novox/hq issue 686): handed to the script as EnvTests and EnvStoreOnly, which it may read. + Tests string + StoreOnly bool +} + +// selection is what a check hands the repository's script to narrow what it runs (novox/hq issue 686): nothing +// for a pull request's check, which always runs whole. +func selection(spec CheckSpec) []string { + var env []string + if spec.Tests != "" { + env = append(env, EnvTests+"="+spec.Tests) + } + if spec.StoreOnly { + env = append(env, EnvStoreOnly+"=1") + } + return env } // GroupHead is one other head of a delivery group's composed check. @@ -193,6 +210,11 @@ const ( // EnvGroup is a delivery group's other heads, as JSON, for the gate (novox/hq ADR 0239); empty for a // pull request checked alone. EnvGroup = "MESH_CHECK_GROUP" + // EnvTests is a `go test -run` pattern and EnvStoreOnly is 1 when a branch tried before its pull request + // asks for only those tests, or only the store tests (novox/hq issue 686). Never set for a pull request's + // check. A script that reads neither runs whole. + EnvTests = "MESH_CHECK_TESTS" + EnvStoreOnly = "MESH_CHECK_STORE_ONLY" ) // CheckTimeout bounds one check; a check that runs past it is an error, not a pass. @@ -515,6 +537,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry EnvModules + "=" + strings.Join(append(append([]string{}, spec.Modules...), spec.New...), ","), EnvGroup + "=" + groupFile, "GOCACHE=" + filepath.Join(workspace, "go-cache"), "GOMODCACHE=" + filepath.Join(workspace, "go-modules")} + env = append(env, selection(spec)...) v := CheckVerdict{} modules := append(append([]string{}, spec.Modules...), prefixed("new:", spec.New)...) diff --git a/internal/builder/check_test.go b/internal/builder/check_test.go index 92583f5a..1eb5890a 100644 --- a/internal/builder/check_test.go +++ b/internal/builder/check_test.go @@ -590,3 +590,15 @@ func TestARepositoryInTwoLanguagesIsCheckedInBoth(t *testing.T) { t.Errorf("a part in a toolchain the mesh does not hold answered %+v — never a pass", layer) } } + +// A branch tried before its pull request may narrow what the repository's script runs (novox/hq issue 686); a +// pull request's check is handed nothing, and runs whole. +func TestOnlyATrysSelectionReachesTheScript(t *testing.T) { + if env := selection(CheckSpec{Number: 7}); len(env) != 0 { + t.Fatalf("a pull request's check was handed a selection: %v", env) + } + env := selection(CheckSpec{Tests: "^TestX$", StoreOnly: true}) + if len(env) != 2 || env[0] != "MESH_CHECK_TESTS=^TestX$" || env[1] != "MESH_CHECK_STORE_ONLY=1" { + t.Fatalf("a try's selection: %v", env) + } +} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index 673f62f4..a186e558 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -362,6 +362,28 @@ var ControllerVerbs = []Verb{ "register": "\"true\" to register what it builds", "older": "\"true\", with register: even though a newer build of the module is registered", }, []string{"id"}, "register", "older")}, + // novox/hq issue 686: a builder ran a branch's store tests for the first time in its pull request's merge check. + {Name: "try", Description: "Try a pushed branch or commit before its pull request (novox/hq issue 686): the build " + + "seat merges it into the repository's trunk and runs the repository's own merge-check.sh on the merge, in the " + + "toolchain containers and against the throwaway store and bus a pull request's merge check gets. Answers at " + + "once with an id; asked again with that id it waits up to 20 s and answers pass, fail or error, how long it " + + "took and the lines naming what failed — or that it is still waiting or running, to ask again. Sets no " + + "status on the forge, registers and builds nothing, runs no merge gate, counts as a check in the build queue " + + "(after deliveries and after every pull request's check, however long it waits) and is bounded as one. One " + + "try of a commit at a time — a second ask answers the first's id — and at most four waiting or running in " + + "all: a fifth is refused. Only for a repository of the mesh; the ref must be on the forge.", + Input: schema(map[string]string{ + "repository": "the repository on the forge, owner/name; with ref", + "ref": "the branch or commit to try, as pushed to the forge", + "base": "the branch it is merged into (the repository's default branch when none is named). The " + + "merge-check.sh that runs is that branch's, never the ref's own: the same trust as a pull request's base", + "tests": "only the tests this `go test -run` pattern names, where the repository's merge-check.sh reads " + + "MESH_CHECK_TESTS; a script that does not runs whole", + "store-only": "\"true\": only the repository's store tests, where its merge-check.sh reads " + + "MESH_CHECK_STORE_ONLY", + "id": "instead of a repository: the id a try answered, to read what came of it", + }, nil, "store-only"), + Replaces: []string{"sh merge-check.sh", "docker run postgres"}}, // novox/hq ADR 0350 §5: asked by the delivery's owner when the forge says a pull request closed unmerged. {Name: "forget-checks", Description: "Let go of every build a pull request's merge checks made for its merge in " + "the artifact store's scratch namespace (novox/hq ADR 0350 §5): asked when it closed unmerged; a merged one's go " + diff --git a/internal/link/build.go b/internal/link/build.go index 6eb16588..73384e9f 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -164,6 +164,30 @@ type CheckRequest struct { // each, from the merge it judged, into the scratch namespace once it has judged and not failed, for the walk to // take. A build seat from before it builds nothing, and the walk builds as before. Builds []CheckBuild `json:"builds,omitempty"` + // Try marks a branch tried before its pull request (novox/hq issue 686): the repository's own check for a + // pushed branch or commit, asked by a builder through the controller's `try` verb. Its outcome is read from + // the build's log (StepTried) and said to nobody: no status on the forge, nothing recorded or registered, + // nothing built for a merge. Tests narrows what the repository's script runs to the tests a pattern names, + // and StoreOnly to its store tests; a script that reads neither runs whole. + Try bool `json:"try,omitempty"` + Tests string `json:"tests,omitempty"` + StoreOnly bool `json:"store-only,omitempty"` +} + +// StepTried is the step a build seat says a try's outcome under, in the build's log: one line, a Tried as JSON +// (novox/hq issue 686). +const StepTried = "tried" + +// Tried is what came of a branch tried before its pull request (novox/hq issue 686): the verdict of the +// repository's own check for the ref merged into the trunk, how long it took, and the lines that name what +// failed, picked from the whole of its output (issue 460). +type Tried struct { + Verdict string `json:"verdict"` + Summary string `json:"summary"` + Took string `json:"took"` + Head string `json:"head,omitempty"` + Trunk string `json:"trunk,omitempty"` + Failed []string `json:"failed,omitempty"` } // CheckBuild is one module a check builds for its merge: from the merge it made when Merged — a module built from diff --git a/internal/link/builds.go b/internal/link/builds.go index 7fe20541..f372d056 100644 --- a/internal/link/builds.go +++ b/internal/link/builds.go @@ -96,8 +96,27 @@ type BuildStart struct { // an earlier taker left unfinished — most often a build agent replaced mid-build — and started // again from the beginning (novox/hq issue 594). 0 from a machine that predates it. Delivery int `json:"delivery,omitempty"` + // Try marks the start of a branch tried before its pull request (novox/hq issue 686), which names no + // repository and no ref (startOf). + Try bool `json:"try,omitempty"` } +// startOf is what a build machine says of the ask it took. **A try's start names no repository and no commit** +// (novox/hq issue 686): the forge's holder writes mesh/merge-gate pending on an open pull request whose head is +// the commit a start names, and a try of that head is no check of the pull request — nothing on the forge moves +// for a try. Its id, machine and time are all the queue reads of a start. +func startOf(r BuildRequest, on string, delivery int, at time.Time) BuildStart { + s := BuildStart{ID: r.ID, Repository: r.Repository, Path: r.Path, Ref: r.Ref, On: on, + At: at.UTC().Format(time.RFC3339Nano), Delivery: delivery} + if IsTry(r) { + s.Repository, s.Path, s.Ref, s.Try = "", "", "", true + } + return s +} + +// IsTry says an ask is a branch tried before its pull request (novox/hq issue 686). +func IsTry(r BuildRequest) bool { return r.Check != nil && r.Check.Try } + // Restarted is how a build handed out again says so (novox/hq issue 594), in its log and in its check's // report, so a second run never reads as the first. Neutral on why: a holder replaced mid-build, a paused // holder handing an ask back as it arrived, and a bus that held its acknowledgements past the wait all diff --git a/internal/link/builds_nats.go b/internal/link/builds_nats.go index c7d0c67e..5ec3b9c1 100644 --- a/internal/link/builds_nats.go +++ b/internal/link/builds_nats.go @@ -401,10 +401,7 @@ func (b *natsBuild) Done() error { return b.msg.Ack() } // Began publishes that this machine has taken the build, into the stream like the outcome, so a // reader that asks afterwards sees when it started as well as how it ended. func (b *natsBuild) Began(ctx context.Context) error { - body, err := json.Marshal(BuildStart{ - ID: b.request.ID, Repository: b.request.Repository, Path: b.request.Path, - Ref: b.request.Ref, On: b.on, At: time.Now().UTC().Format(time.RFC3339Nano), Delivery: b.delivery, - }) + body, err := json.Marshal(startOf(b.request, b.on, b.delivery, time.Now())) if err != nil { return err } diff --git a/internal/link/queue.go b/internal/link/queue.go index 50865ef8..3bbbb8a2 100644 --- a/internal/link/queue.go +++ b/internal/link/queue.go @@ -427,6 +427,9 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul // safeKey is an id that can be a key, and a subject token, as it is: a build id, never a pattern. var safeKey = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) +// SafeBuildID says an id can be a subject token as it is: a build's id, never a pattern. +func SafeBuildID(id string) bool { return safeKey.MatchString(id) } + // Cancelled is what the controller writes for an ask it cancelled. type Cancelled struct { At string `json:"at"` diff --git a/internal/link/queue_order.go b/internal/link/queue_order.go index 449618f9..90e68f50 100644 --- a/internal/link/queue_order.go +++ b/internal/link/queue_order.go @@ -26,6 +26,8 @@ import ( // - a check that has waited CheckWaitsAtMost goes before every delivery asked after it, so no check // waits for ever behind a stream of deliveries; // - within a kind, first come, first served, by when it was asked; +// - a branch tried before its pull request goes after everything else, however long it has waited: it is +// never promoted ahead of a delivery, and never ahead of a pull request's check (novox/hq issue 686); // - an ask whose kind cannot be told counts as a check: a delivery taken late breaks a budget, a check // taken late breaks nothing. // @@ -64,8 +66,12 @@ func (a QueuedAsk) askedOrStored() time.Time { // QueueOrder is the order the waiting asks are to be handed out in, now: the deliveries and every check // that has waited CheckWaitsAtMost, by when each was asked, then the other checks, by when each was asked. func QueueOrder(waiting []QueuedAsk, now time.Time) []QueuedAsk { - var first, then []QueuedAsk + var first, then, tries []QueuedAsk for _, a := range waiting { + if IsTry(a.Request) { + tries = append(tries, a) + continue + } if KindOf(a.Request) == KindCheck && now.Sub(a.askedOrStored()) < CheckWaitsAtMost { then = append(then, a) continue @@ -82,7 +88,8 @@ func QueueOrder(waiting []QueuedAsk, now time.Time) []QueuedAsk { } byAsking(first) byAsking(then) - return append(first, then...) + byAsking(tries) + return append(append(first, then...), tries...) } // toMove is the asks to move to the tail, in this order, for the queue to stand as order says: everything diff --git a/internal/link/try_test.go b/internal/link/try_test.go new file mode 100644 index 00000000..f434dc18 --- /dev/null +++ b/internal/link/try_test.go @@ -0,0 +1,55 @@ +package link + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + "time" +) + +// The start of a try names no repository and no commit (novox/hq issue 686): the forge's holder writes +// mesh/merge-gate pending on the open pull request whose head a start names, and nothing on the forge moves +// for a try. A pull request's check still names both. +func TestATrysStartNamesNoCommitTheForgeCouldMatch(t *testing.T) { + sha := strings.Repeat("c0ffee", 7)[:40] + at := time.Date(2026, 10, 14, 0, 0, 0, 0, time.UTC) + try := BuildRequest{ID: "build-1", Repository: "https://forge/novox/a.git", Ref: sha, + Check: &CheckRequest{Owner: "novox", Repo: "a", Try: true}} + body, _ := json.Marshal(startOf(try, "ace", 1, at)) + if strings.Contains(string(body), sha) || strings.Contains(string(body), "novox/a") { + t.Fatalf("a try's start names what the forge matches a pull request by: %s", body) + } + if s := startOf(try, "ace", 2, at); s.ID != "build-1" || s.On != "ace" || !s.Try || s.Delivery != 2 || s.At == "" { + t.Fatalf("a try's start lost what the queue reads: %+v", s) + } + pull := try + pull.Check = &CheckRequest{Owner: "novox", Repo: "a", Number: 7} + if s := startOf(pull, "ace", 1, at); s.Ref != sha || s.Repository == "" || s.Try { + t.Fatalf("a pull request's check's start: %+v", s) + } +} + +// A try stays behind every delivery and every pull request's check, however long it has waited: the ten +// minutes that put a check ahead of later deliveries never promote a try (novox/hq issue 686). +func TestATryIsNeverHandedOutBeforeADeliveryOrAPullRequestsCheck(t *testing.T) { + now := time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC) + try := func(seq uint64, id string, asked time.Time) QueuedAsk { + a := anAsk(seq, id, asked, true) + a.Request.Check.Try = true + return a + } + waiting := []QueuedAsk{ + try(1, "try-old", now.Add(-40*time.Minute)), + try(2, "try-young", now.Add(-time.Minute)), + anAsk(3, "check-young", now.Add(-2*time.Minute), true), + anAsk(4, "delivery", now.Add(-time.Minute), false), + } + want := []string{"delivery", "check-young", "try-old", "try-young"} + if got := idsOf(QueueOrder(waiting, now)); !reflect.DeepEqual(got, want) { + t.Fatalf("handed out as %v, want %v", got, want) + } + if !SafeBuildID("build-20261014T000000-1a2b") || SafeBuildID("x.>") || SafeBuildID("*") || SafeBuildID("") { + t.Fatal("an id that is a pattern passes for a build's id") + } +} diff --git a/merge-check.sh b/merge-check.sh index e1e85687..36de3711 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -54,11 +54,26 @@ if [ -n "$unformatted" ]; then fi CGO_ENABLED=0 go vet ./... -if command -v gcc >/dev/null 2>&1; then - CGO_ENABLED=1 go test -race -count=1 -timeout 30m ./... +suite() { + if command -v gcc >/dev/null 2>&1; then + CGO_ENABLED=1 go test -race -count=1 -timeout 30m "$@" + else + echo "NOT RACE-CHECKED: the toolchain holds no C compiler; the suite runs without the race detector" + CGO_ENABLED=0 go test -count=1 -timeout 30m "$@" + fi +} +# **A branch tried before its pull request may ask for a part** (novox/hq issue 686): the controller's `try` verb +# hands MESH_CHECK_TESTS, a `go test -run` pattern, or MESH_CHECK_STORE_ONLY=1. A pull request's merge check is +# handed neither and always runs whole; a part that passes is said as a part, never as the check. +if [ -n "${MESH_CHECK_TESTS:-}" ]; then + echo "TRIED IN PART: only the tests matching $MESH_CHECK_TESTS, in every package" + suite -run "$MESH_CHECK_TESTS" ./... + exit 0 +fi +if [ "${MESH_CHECK_STORE_ONLY:-}" = 1 ]; then + echo "TRIED IN PART: only the store tests listed in testdata/store-tests" else - echo "NOT RACE-CHECKED: the toolchain holds no C compiler; the suite runs without the race detector" - CGO_ENABLED=0 go test -count=1 -timeout 30m ./... + suite ./... fi # **The store tests listed ran** (novox/hq issue 459): a test that needs the store skips without one, and a skip diff --git a/module.json b/module.json index 1b84a8c4..538f5cca 100644 --- a/module.json +++ b/module.json @@ -77,6 +77,7 @@ "replay", "kill", "forget-checks", + "try", "pause", "resume", "hand-act",