Commit `f3c562dedb`

f3c562dedb9f3c6989727a7fe5dcf6d18e7068d6

parent: c07024b730

Verified · cmc ci/build: success ci/test: failure

cmc <hello@cleberg.net> · 2026-10-19 04:53 UTC

ci: reuse a build only on the ref it ran for

queueJobs let a build of the same commit, or a passed build of the same
tree, on any ref stand for a push. A job sees GITBAY_REF, so a fast-forward
or a squash merge onto the default branch reused a branch build that had
not done what the default-branch build would: krz/orgo's pages job no-ops
off main and was never run after a merge.

Both lookups now take the ref (BuildsForCommitOnRef, SuccessBuildForTree).
A filter skip still leaves alone a status another ref's build reported on
the same commit. New push-shape row for the squash merge; the fast-forward
row now queues.

Closes #335

Layout: unified · split

.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org +1 −1

	@@ -23,7 +23,7 @@ commit instead of failing silently.
23	1. Queue. The post-receive hook calls =queueJobs=	23	1. Queue. The post-receive hook calls =queueJobs=
24	(=internal/control/build.go=). Each job gets a =ci/<job>= status:	24	(=internal/control/build.go=). Each job gets a =ci/<job>= status:
25	=pending= when queued, =skipped= when path filters exclude it, or	25	=pending= when queued, =skipped= when path filters exclude it, or
26	=success= copied from an earlier trusted build of the same tree on the same image (#177, #258).	26	=success= copied from an earlier trusted build of the same tree on the same ref and image (#177, #258).
27	Merge requests from forks are queued against the target repository	27	Merge requests from forks are queued against the target repository
28	with =trusted = false=.	28	with =trusted = false=.
29	2. Claim. A runner calls =runner next= over SSH	29	2. Claim. A runner calls =runner next= over SSH

.gitbay/wiki/CI.org +15 −8

 Three mechanisms decide what a push does to CI, and they interact:
-- *Dedupe.* A job's result is a property of the commit's tree. A commit
+- *Dedupe.* A job's result is a property of the commit's tree and the
-  that already has a passed, queued or running build for a job is not
+  ref it ran for, since a job sees =GITBAY_REF=. A commit that already
-  queued again; a commit whose tree already passed a job gets that
+  has a passed, queued or running build for a job on this ref is not
-  result as its status, naming the build it came from (#177). Only a
+  queued again; a commit whose tree already passed a job on this ref
+  gets that result as its status, naming the build it came from (#177).
+  A merge onto the default branch is built there even when the branch
+  passed. Only a
   trusted build counts, and for tree reuse only one on the image the
   job names: a fork's green build does not stand for the repository's
   own, so its commit is built again when it lands on a branch (#258). A
 | a new branch, no old sha | queued | queued | skipped | — | — | — |
 | rebase onto a moved default branch, old not an ancestor | queued | queued | skipped | — | — | — |
 | rewritten commit, same tree as a passed build | reused | reused | skipped | — | — | — |
-| fast-forward of a commit built on another branch | — | — | — | registered | — | — |
+| fast-forward of a commit built on another branch | queued | queued | — | registered | — | — |
+| squash merge onto the default branch, same tree as a branch build | queued | queued | skipped | registered | — | — |
 | a commit whose earlier build failed, on a new branch | queued | queued | — | — | — | — |
 | merge request head from a fork | queued, untrusted | queued, untrusted | queued, untrusted | — | — | — |
 | tag push | — | — | — | — | queued | — |
   because the tree check comes before the filter. =notapp= never had a
   passed build to reuse, and the push changed nothing, so the filter
   skips it again.
-- A fast-forward of a commit built elsewhere queues nothing: the
+- A build stands for its commit, or its tree, only on the ref it ran
-  builds belong to the commit, not the branch. The default-branch push
+  for: a job sees =GITBAY_REF= and may act on it, as one that publishes
-  still registers the schedule.
+  only from the default branch does. A fast-forward or a squash merge
+  onto the default branch is built there although the branch passed.
+  =notapp= on the fast-forward keeps the skipped status the branch push
+  gave the same commit.
 - A failed build does not stand for its commit. The same commit pushed
   to another branch runs again; =notapp= keeps its skipped status.
 - A merge request head from a fork has no diff base and, unlike a

internal/control/build.go +29 −15

 		st.SetCommitStatus(repo.ID, sha, "ci/config", "failure", err.Error(), "", userID)
 		return
 	}
-	// A build is a fact about a commit, not a ref: a job has no branch
+	// A build is a fact about a commit on a ref: a job sees GITBAY_REF and
-	// filter, so a commit that already passed a job on another branch has
+	// may act on it, as a job that publishes only from the default branch
-	// nothing left to prove when a fast-forward lands it here, and one
+	// does, so a build for another ref does not stand for this one. A
-	// still queued or running there will say soon enough. A failed,
+	// commit already built for this ref, or queued or running for it, has
-	// abandoned or cancelled build does not count; that commit runs again.
+	// nothing left to prove. A failed, abandoned or cancelled build does
-	built, err := st.BuildsForCommit(repo.ID, sha)
+	// not count; that commit runs again.
+	built, err := st.BuildsForCommitOnRef(repo.ID, sha, ref)
 	if err != nil {
 		built = nil
 	}
-	// A job's result is a property of the tree, not the commit: a rebase
+	// The ci/<job> status belongs to the commit, so a build of it for
-	// onto a base that touched nothing the branch did gives every commit
+	// another ref reports there too, and a skip here must not overwrite it.
-	// a new sha and the same tree, and re-running the suite over it
+	elsewhere, err := st.BuildsForCommit(repo.ID, sha)
-	// proves nothing it did not already prove (#177). A success recorded
+	if err != nil {
-	// against the tree stands for the new commit.
+		elsewhere = nil
+	}
+	// A job's result is a property of the tree and the ref, not the
+	// commit: a rebase onto a base that touched nothing the branch did
+	// gives every commit a new sha and the same tree, and re-running the
+	// suite over it proves nothing it did not already prove (#177). A
+	// success recorded against the tree on this ref stands for the new
+	// commit; a squash merge onto another branch is built there.
 	tree, _ := gitutil.ResolveTree(dir, sha)
 	// The changed-file list a job's path filters run against, computed
 	// once and only if some job actually declares one. When the diff
 		// stands for it — unless this queue is trusted and that build was
 		// not: a fork's head that lands on a branch is built again as the
 		// repository's own (#258).
-		if b, ok := built[j.Name]; ok && (b.Trusted || !trusted) &&
+		stands := func(m map[string]store.Build) bool {
-			(b.Status == "success" || b.Status == "pending" || b.Status == "running") {
+			b, ok := m[j.Name]
+			return ok && (b.Trusted || !trusted) &&
+				(b.Status == "success" || b.Status == "pending" || b.Status == "running")
+		}
+		if stands(built) {
 			continue
 		}
-		if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, j.Name, j.Image); ok && prev.SHA != sha {
+		if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, ref, j.Name, j.Image); ok && prev.SHA != sha {
 			url := fmt.Sprintf("%s/%s/builds/%d", siteURL, repo.Path(), prev.Number)
 			st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "success",
 				fmt.Sprintf("passed in build %d as %.10s, same tree", prev.Number, prev.SHA), url, userID)
 		// require_checks with a skipped status instead of leaving the
 		// commit with none at all, which the gate refuses outright (#172).
 		if filtered && !ci.Selected(j, changed) {
-			st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "skipped", skipReason(j, changed), "", userID)
+			if !stands(elsewhere) {
+				st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "skipped", skipReason(j, changed), "", userID)
+			}
 			continue
 		}
 		steps, _ := json.Marshal(j.Steps)

internal/hookd/pushshapes_test.go +14 −1

	@@ -330,7 +330,20 @@ var pushShapes = []pushShape{
330	f.git(f.src, "merge", "-q", "--ff-only", "feat")	330	f.git(f.src, "merge", "-q", "--ff-only", "feat")
331	f.mark(c1)	331	f.mark(c1)
332	f.push("main", f.base, c1)	332	f.push("main", f.base, c1)
333	}, []string{"—", "—", "—", "registered", "—", "—"}},	333	}, []string{"queued", "queued", "—", "registered", "—", "—"}},
		334
		335	{"squash merge onto the default branch, same tree as a branch build", func(f *shapeFixture) {
		336	f.git(f.src, "checkout", "-q", "-b", "feat")
		337	c1 := f.appCommit("more")
		338	f.push("feat", zeroSHA40, c1)
		339	f.finish("success")
		340	f.git(f.src, "checkout", "-q", "main")
		341	f.git(f.src, "merge", "-q", "--squash", "feat")
		342	f.git(f.src, "commit", "-q", "-m", "squashed")
		343	c2 := f.sha("HEAD")
		344	f.mark(c2)
		345	f.push("main", f.base, c2)
		346	}, []string{"queued", "queued", "skipped", "registered", "—", "—"}},
334		347
335	{"a commit whose earlier build failed, on a new branch", func(f *shapeFixture) {	348	{"a commit whose earlier build failed, on a new branch", func(f *shapeFixture) {
336	f.git(f.src, "checkout", "-q", "-b", "feat")	349	f.git(f.src, "checkout", "-q", "-b", "feat")

internal/store/builds.go +18 −5

 // request's checks are ci/<job> statuses; this is where their timing comes
 // from, in one query rather than one per check.
 func (s *Store) BuildsForCommit(repoID int64, sha string) (map[string]Build, error) {
-	rows, err := s.DB.Query(buildSelect+" WHERE repo_id = ? AND sha = ? ORDER BY number ASC", repoID, sha)
+	return s.newestPerJob(" WHERE repo_id = ? AND sha = ? ORDER BY number ASC", repoID, sha)
+}
+// BuildsForCommitOnRef returns the newest build per job for one commit
+// made for one ref. A job sees GITBAY_REF, so a build for another ref
+// says nothing about what the job does on this one.
+func (s *Store) BuildsForCommitOnRef(repoID int64, sha, ref string) (map[string]Build, error) {
+	return s.newestPerJob(" WHERE repo_id = ? AND sha = ? AND ref = ? ORDER BY number ASC", repoID, sha, ref)
+}
+func (s *Store) newestPerJob(where string, args ...any) (map[string]Build, error) {
+	rows, err := s.DB.Query(buildSelect+where, args...)
 	if err != nil {
 		return nil, err
 	}
 // SuccessBuildForTree finds a passed build of the job for a tree rather
 // than a commit: a rebase that changes nothing in the tree has already
-// been built (#177). Only a trusted build on the image the job names
+// been built (#177). Only a build for the same ref counts, since a job
+// sees GITBAY_REF and may do something else on another one. Only a
+// trusted build on the image the job names
 // counts: a fork's result, or one from an image the job has left, does
 // not stand for the repository's own (#258). A job naming no image
 // matches builds that named none, whichever default the runner used;
 // the CI wiki page says so. An empty tree never matches.
-func (s *Store) SuccessBuildForTree(repoID int64, tree, job, image string) (Build, bool, error) {
+func (s *Store) SuccessBuildForTree(repoID int64, tree, ref, job, image string) (Build, bool, error) {
 	if tree == "" {
 		return Build{}, false, nil
 	}
 	b, err := scanBuild(s.DB.QueryRow(buildSelect+
-		" WHERE repo_id = ? AND tree = ? AND job = ? AND image = ? AND trusted = 1 AND status = 'success'"+
+		" WHERE repo_id = ? AND tree = ? AND ref = ? AND job = ? AND image = ? AND trusted = 1 AND status = 'success'"+
-		" ORDER BY number DESC LIMIT 1", repoID, tree, job, image))
+		" ORDER BY number DESC LIMIT 1", repoID, tree, ref, job, image))
 	if errors.Is(err, sql.ErrNoRows) {
 		return Build{}, false, nil
 	}

internal/store/builds_test.go +11 −7

 	}
 }
-// A success is found by tree across commits; an empty tree never matches,
+// A success is found by tree across commits on one ref; an empty tree
-// so builds queued without one (scheduled, tag) are never reused (#177).
+// never matches, so builds queued without one (scheduled, tag) are never
+// reused (#177).
 func TestSuccessBuildForTree(t *testing.T) {
 	s := open(t)
 	if err := s.MigrateUp(); err != nil {
 	if err := s.FinishBuild(b["unit"].ID, "success"); err != nil {
 		t.Fatal(err)
 	}
-	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); !ok || prev.SHA != "aaa" {
+	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", ""); !ok || prev.SHA != "aaa" {
 		t.Fatalf("success not found by tree: ok=%v prev=%+v", ok, prev)
 	}
-	if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "other", ""); ok {
+	if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "other", ""); ok {
 		t.Error("matched a different job")
 	}
-	if _, ok, _ := s.SuccessBuildForTree(repoID, "", "unit", ""); ok {
+	if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "feat", "unit", ""); ok {
+		t.Error("matched a build for another ref")
+	}
+	if _, ok, _ := s.SuccessBuildForTree(repoID, "", "main", "unit", ""); ok {
 		t.Error("an empty tree matched")
 	}
 }
 			t.Fatal(err)
 		}
 	}
-	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); ok {
+	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", ""); ok {
 		t.Fatalf("reused build %d: untrusted, or on another image", prev.Number)
 	}
-	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", "localhost/old:1"); !ok || prev.SHA != "bbb" {
+	if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", "localhost/old:1"); !ok || prev.SHA != "bbb" {
 		t.Fatalf("trusted build on the same image not found: ok=%v prev=%+v", ok, prev)
 	}
 	if _, ok, _ := s.SuccessBuildFor(repoID, "aaa", "unit"); ok {

Commit f3c562dedb

Commit `f3c562dedb`