Keep mesh-lab at its trunk and build the release judge with a cache of its own
mesh/merge-gate out of date: judged against trunk 535c7641ac41, the trunk is now c975dbf7b053; judged again on a recheck or when it is merged
mesh/repo-check out of date: judged against trunk 535c7641ac41, the trunk is now c975dbf7b053; judged again on a recheck or when it is merged
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate out of date: judged against trunk 535c7641ac41, the trunk is now c975dbf7b053; judged again on a recheck or when it is merged
mesh/repo-check out of date: judged against trunk 535c7641ac41, the trunk is now c975dbf7b053; judged again on a recheck or when it is merged
mesh/delivery superseded: a newer head of the same pull request
Review of #329: mesh-lab's replays run with the container runtime's socket, so a same-named branch must never be what runs there; the shared Go cache and HOME are writable by a pull request's own script, so the judge kept for a head must not be built from them (issue 704, issue 482). A forge answer naming no branch is an error, and a trunk is the branch a repository follows, not a name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -276,7 +276,8 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
|
||||
// besideOf is what a check reads beside its own repository, by the directory it is found under: each core repository
|
||||
// by the module the mesh builds from it, at besideRefs — or, for a pull request into a release branch (novox/hq ADR
|
||||
// 0358 §3), at its branch of that name where it has one and at its trunk otherwise, the controller's main kept for
|
||||
// its judge. running is the commit the mesh runs of a module.
|
||||
// its judge, and mesh-lab at its trunk whatever the base: its replays run with the container runtime's socket. running
|
||||
// is the commit the mesh runs of a module.
|
||||
func besideOf(entries []inventory.Entry, running func(module string) string,
|
||||
clone func(inventory.Source) (string, error), release string) (map[string]link.CheckedOut, error) {
|
||||
beside := map[string]link.CheckedOut{}
|
||||
@@ -312,7 +313,9 @@ func besideOf(entries []inventory.Entry, running func(module string) string,
|
||||
if url, err := clone(inventory.Source{Seat: e.Source.Seat, Repository: siblingOf(e.Source.Repository,
|
||||
sibling)}); err == nil {
|
||||
b := link.CheckedOut{Repository: url, Ref: refs[sibling]}
|
||||
if release != "" {
|
||||
// **mesh-lab is read at its trunk, always** (ADR 0358): its replays run with the container
|
||||
// runtime's socket, so they are only ever reviewed code — never a branch anyone may create.
|
||||
if release != "" && sibling != "mesh-lab" {
|
||||
b = link.CheckedOut{Repository: url, Ref: release, Else: "main"}
|
||||
}
|
||||
beside[sibling] = b
|
||||
@@ -326,15 +329,15 @@ func besideOf(entries []inventory.Entry, running func(module string) string,
|
||||
|
||||
// releaseBranchOf is the release branch a pull request merges into (novox/hq ADR 0358 §1): its base, when that is not
|
||||
// its repository's trunk and mesh-controller has a branch of that name; empty otherwise, and the pull request is
|
||||
// checked as before it. **A base that is a trunk asks nothing**: main, the branch a module built from the repository
|
||||
// follows, or mesh-controller's own trunk. Any other base is asked of the forge once, and an answer it cannot give is
|
||||
// an error, never read as "no release branch".
|
||||
// checked as before it. **A base that is a trunk asks nothing**: the branch a module built from the repository follows
|
||||
// (main when it records none), or mesh-controller's own trunk. Any other base is asked of the forge once, and an
|
||||
// answer it cannot give is an error, never read as "no release branch".
|
||||
func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope, entries []inventory.Entry) (string, error) {
|
||||
base := p.Base
|
||||
if base == "" || base == "main" {
|
||||
if base == "" {
|
||||
return "", nil
|
||||
}
|
||||
if scope.From != nil && base == followedBranch(scope.From.Source.Ref) {
|
||||
if scope.From != nil && base == trunkOf(scope.From.Source) {
|
||||
return "", nil
|
||||
}
|
||||
var controller *inventory.Entry
|
||||
@@ -344,7 +347,7 @@ func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope,
|
||||
break
|
||||
}
|
||||
}
|
||||
if controller == nil || base == followedBranch(controller.Source.Ref) {
|
||||
if controller == nil || base == trunkOf(controller.Source) {
|
||||
return "", nil
|
||||
}
|
||||
if !tryRef.MatchString(base) || strings.Contains(base, "..") {
|
||||
@@ -365,6 +368,14 @@ func releaseBranchOf(ctx context.Context, p link.PullUpdated, scope checkScope,
|
||||
return base, nil
|
||||
}
|
||||
|
||||
// trunkOf is the branch a module's source follows: main when it records none, or records a commit.
|
||||
func trunkOf(s inventory.Source) string {
|
||||
if t := followedBranch(s.Ref); t != "" {
|
||||
return t
|
||||
}
|
||||
return "main"
|
||||
}
|
||||
|
||||
// sourceName is the name of a recorded repository, a path on the git seat or a URL: novox/mesh-host → mesh-host.
|
||||
func sourceName(repository string) string {
|
||||
parts := strings.Split(strings.Trim(strings.TrimSuffix(repository, ".git"), "/"), "/")
|
||||
@@ -387,6 +398,13 @@ var forgeHasBranch = func(ctx context.Context, owner, repo, branch string) (bool
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return branchAnswered(answer, branch)
|
||||
}
|
||||
|
||||
// branchAnswered reads the forge's answer about a branch: a 404 is "no such branch"; an answer naming the branch is
|
||||
// "it has it"; anything else — another error, an unreadable answer, one naming no branch or another — is an error,
|
||||
// never read as "no release branch".
|
||||
func branchAnswered(answer link.Answer, branch string) (bool, error) {
|
||||
if answer.Error != "" {
|
||||
if strings.Contains(answer.Error, " 404 ") {
|
||||
return false, nil
|
||||
@@ -399,9 +417,12 @@ var forgeHasBranch = func(ctx context.Context, owner, repo, branch string) (bool
|
||||
} `json:"result"`
|
||||
}
|
||||
if err := json.Unmarshal(answer.Result, &found); err != nil {
|
||||
return false, err
|
||||
return false, fmt.Errorf("the forge's answer about branch %s cannot be read: %w", branch, err)
|
||||
}
|
||||
return found.Result.Name == branch, nil
|
||||
if found.Result.Name != branch {
|
||||
return false, fmt.Errorf("the forge answered about branch %s naming %q", branch, found.Result.Name)
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// namedForTheCheck are the builds earlier checks of the same pull request at the same head named, read once for the
|
||||
|
||||
@@ -125,8 +125,9 @@ func TestBesideAReleaseBranchCheckEachRepositoryIsReadAtThatBranchElseItsTrunk(t
|
||||
"mesh-host": at("mesh-host", aRelease, "main"),
|
||||
"mesh-tools": at("mesh-tools", aRelease, "main"),
|
||||
"mesh-catalog": at("mesh-catalog", aRelease, "main"),
|
||||
"mesh-lab": at("mesh-lab", aRelease, "main"),
|
||||
"mesh-sdk": at("mesh-sdk", aRelease, "main"),
|
||||
// mesh-lab at its trunk, always: its replays run with the container runtime's socket.
|
||||
"mesh-lab": at("mesh-lab", "main", ""),
|
||||
"mesh-sdk": at("mesh-sdk", aRelease, "main"),
|
||||
}
|
||||
if !reflect.DeepEqual(beside, want) {
|
||||
t.Fatalf("beside a release-branch check:\n got %v\nwant %v", beside, want)
|
||||
@@ -193,3 +194,70 @@ func TestAReleaseBranchsPullRequestIntoItsTrunkIsCheckedAsAnyTrunkPullRequest(t
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// **A forge answer that names no branch is an error**, never "no release branch"; a 404 is "no such branch".
|
||||
func TestTheForgesAnswerAboutABranchIsReadStrictly(t *testing.T) {
|
||||
for _, c := range []struct {
|
||||
what string
|
||||
answer link.Answer
|
||||
has bool
|
||||
fails bool
|
||||
}{
|
||||
{"the branch named", link.Answer{Result: []byte(`{"result":{"name":"` + aRelease + `"}}`)}, true, false},
|
||||
{"not found", link.Answer{Error: "Gitea API /repos/novox/mesh-controller/branches/x: 404 {\"message\":\"not found\"}"},
|
||||
false, false},
|
||||
{"an empty branch name", link.Answer{Result: []byte(`{"result":{"name":""}}`)}, false, true},
|
||||
{"no result", link.Answer{Result: []byte(`{}`)}, false, true},
|
||||
{"another branch named", link.Answer{Result: []byte(`{"result":{"name":"main"}}`)}, false, true},
|
||||
{"unreadable", link.Answer{Result: []byte(`not json`)}, false, true},
|
||||
{"another error", link.Answer{Error: "Gitea API: 500 internal"}, false, true},
|
||||
} {
|
||||
has, err := branchAnswered(c.answer, aRelease)
|
||||
if has != c.has || (err != nil) != c.fails {
|
||||
t.Errorf("%s: has %v, error %v", c.what, has, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// **The trunk is the repository's own, not a name**: a repository whose trunk is not main, one that follows main, and
|
||||
// a controller whose trunk is not main.
|
||||
func TestATrunkIsTheBranchTheRepositoryFollows(t *testing.T) {
|
||||
asked := aForgeWithBranches(t, map[string][]string{"novox/mesh-controller": {"main", aRelease, "master"}}, nil)
|
||||
legacy := fromRepo("legacy", "novox/legacy", "")
|
||||
legacy.Source.Ref = "master"
|
||||
onMain := fromRepo("photos", "novox/photos", "")
|
||||
onMain.Source.Ref = "main"
|
||||
entries := append(theCore(), legacy, onMain)
|
||||
for _, c := range []struct {
|
||||
what, repo, base, release string
|
||||
asks int
|
||||
}{
|
||||
{"a repository whose trunk is master, into master", "legacy", "master", "", 0},
|
||||
{"a repository whose trunk is master, into the release branch", "legacy", aRelease, aRelease, 1},
|
||||
{"a repository whose module follows main, into main", "photos", "main", "", 0},
|
||||
{"a repository whose module follows main, into the release branch", "photos", aRelease, aRelease, 1},
|
||||
} {
|
||||
*asked = 0
|
||||
p := link.PullUpdated{Owner: "novox", Repo: c.repo, Base: c.base, Commit: "abc", Paths: []string{"x"}}
|
||||
got, err := releaseBranchOf(t.Context(), p, pullScope(p, entries, nil, nil), entries)
|
||||
if err != nil || got != c.release || *asked != c.asks {
|
||||
t.Errorf("%s: release %q (%v) after %d question(s), not %q after %d", c.what, got, err, *asked, c.release, c.asks)
|
||||
}
|
||||
}
|
||||
|
||||
// A controller whose trunk is not main: main may be a release branch there, and its own trunk never is.
|
||||
core := theCore()
|
||||
core[0].Source.Ref = "trunk"
|
||||
for _, c := range []struct {
|
||||
base, release string
|
||||
asks int
|
||||
}{{"trunk", "", 0}, {"main", "main", 1}} {
|
||||
*asked = 0
|
||||
p := link.PullUpdated{Owner: "novox", Repo: "hq", Base: c.base, Commit: "abc", Paths: []string{"x"}}
|
||||
got, err := releaseBranchOf(t.Context(), p, pullScope(p, core, nil, nil), core)
|
||||
if err != nil || got != c.release || *asked != c.asks {
|
||||
t.Errorf("hq into %s, the controller following trunk: release %q (%v) after %d question(s)", c.base, got, err,
|
||||
*asked)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -429,6 +429,11 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
if dir == name || !safeName.MatchString(dir) {
|
||||
continue
|
||||
}
|
||||
if dir == "mesh-lab" && b.Else != "" {
|
||||
// **mesh-lab is read at its trunk, always** (novox/hq ADR 0358): its replays run with the container
|
||||
// runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there.
|
||||
b.Ref, b.Else = b.Else, ""
|
||||
}
|
||||
if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil {
|
||||
return CheckVerdict{}, fmt.Errorf("beside it, %w", err)
|
||||
}
|
||||
@@ -1130,8 +1135,12 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s
|
||||
}
|
||||
}
|
||||
}
|
||||
// **Built with a cache of its own, never the workspace's** (novox/hq issue 482): the workspace's Go cache and HOME are
|
||||
// written by every pull request's own script, so a poisoned cache entry or Go environment file would otherwise be
|
||||
// built into the judge kept for this head. A fresh cache under this check's root, made before anything of the
|
||||
// change runs, and no Go environment file.
|
||||
if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir),
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")},
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOENV=off", "GOCACHE=" + judgeGoCache(root)},
|
||||
"go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil {
|
||||
return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err)
|
||||
}
|
||||
@@ -1141,6 +1150,10 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s
|
||||
return bin, dir, nil
|
||||
}
|
||||
|
||||
// judgeGoCache is the Go cache a release branch's judge is built with: fresh for each check, under its root, which is
|
||||
// emptied before the check clones anything — never the workspace's shared cache.
|
||||
func judgeGoCache(root string) string { return filepath.Join(root, "judge-go-cache") }
|
||||
|
||||
// judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again.
|
||||
const judgesKept = 4
|
||||
|
||||
|
||||
@@ -143,7 +143,7 @@ run)
|
||||
done
|
||||
if [ -n "$detached" ]; then echo a-container; exit 0; fi
|
||||
shift
|
||||
if [ "$1" = go ]; then echo "$*" >> "` + built + `"; fi
|
||||
if [ "$1" = go ]; then echo "GOCACHE=$GOCACHE GOENV=$GOENV $*" >> "` + built + `"; fi
|
||||
cd "$workdir" && exec "$@" ;;
|
||||
esac
|
||||
exit 1
|
||||
@@ -255,6 +255,13 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController(
|
||||
if n := builds(t, built); n != 1 {
|
||||
t.Fatalf("the judge was built %d times for one check", n)
|
||||
}
|
||||
// **The judge is built with a cache no pull request's script can write** (issue 482): a fresh one under the
|
||||
// check's own root, never the workspace's shared go-cache, and no Go environment file from the workspace's HOME.
|
||||
said, _ := os.ReadFile(built)
|
||||
if !strings.Contains(string(said), "/check/judge-go-cache GOENV=off go build") ||
|
||||
strings.Contains(string(said), "/go-cache GOENV") {
|
||||
t.Fatalf("the release judge was built with the workspace's cache or Go environment:\n%s", said)
|
||||
}
|
||||
|
||||
// **Built once per release-branch head**: a second check while that head stands takes the judge kept.
|
||||
v = check("check-704-catalogue-again")
|
||||
@@ -381,3 +388,27 @@ func TestAReleaseBranchsPullRequestIntoItsTrunkIsJudgedByTheControllerTheMeshRun
|
||||
t.Fatalf("beside it were read %v, not the trunk's %s and %s", v.Beside, controllerMain[:12], hostMain[:12])
|
||||
}
|
||||
}
|
||||
|
||||
// **mesh-lab is read at its trunk, always** (ADR 0358): its replays run with the container runtime's socket, so a
|
||||
// branch of the release's name in mesh-lab — which anyone may create — is never what a check reads.
|
||||
func TestAReleaseBranchCheckReadsMeshLabAtItsTrunkEvenWhenItHasThatBranch(t *testing.T) {
|
||||
aRuntimeThatBuildsTheJudge(t)
|
||||
registry := aFactsRegistry(t)
|
||||
beside, _, _, _ := theReleasedMesh(t)
|
||||
lab, labMain, labRelease := aRepositoryWithARelease(t, map[string]string{"README": "the lab"},
|
||||
map[string]string{"README": "a branch anyone could push"})
|
||||
beside["mesh-lab"] = Beside{Repository: lab, Ref: releaseBranch, Else: "main"}
|
||||
catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`,
|
||||
CheckScript: "true\n"}, releaseBranch, map[string]string{"modules/gitea/index.ts": "x"})
|
||||
v, err := Check(t.Context(), Command, CheckSpec{ID: "check-704-lab", Repository: catalogue, Ref: head,
|
||||
Owner: "novox", Repo: "mesh-catalog", Number: 286, Base: releaseBranch, Release: releaseBranch,
|
||||
JudgeCache: t.TempDir(), Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Beside: beside,
|
||||
Toolchain: "golang"}, t.TempDir(), registry, GitCredential{}, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if v.BesideAt["mesh-lab"] != "main" || v.Beside["mesh-lab"] != labMain[:12] || v.Beside["mesh-lab"] == labRelease[:12] {
|
||||
t.Fatalf("mesh-lab was read at %q (%s), not its trunk main at %s", v.BesideAt["mesh-lab"], v.Beside["mesh-lab"],
|
||||
labMain[:12])
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user