fix(ci): refuse a merge when the checks of the head are too many to read

CommitChecks returned the first 1,000 checks without saying it cut the list,
so the merge rule could pass on a part of them. It now fails, and the merge is
refused with a precondition error.

commit: 8dbcdb5076c8a0cb7c67557a942f3494dc22f5f3
author: brice · 9d ago · 2026-10-12 19:13 UTC
parent: b7f6252
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +41 -5

M internal/ports/ci.go +3 -2


@@ -103,8 +103,9 @@ type CIRunCleaner interface {
103	103		// CIChecksReader is the read side of the checks, for the merge rule and the
104	104		// pages that show them. *usecase.CI implements it.
105	105		type CIChecksReader interface {
106		-	// CommitChecks returns the checks of a commit, at most 1,000, without
107		-	// access control (the caller has checked it).
	106	+	// CommitChecks returns the checks of a commit, without access control (the
	107	+	// caller has checked it). More than 1,000 is domain.ErrResourceExhausted,
	108	+	// not a part of them: a rule must not decide on some of the checks.
108	109		CommitChecks(ctx context.Context, repo domain.RepoName, sha string) ([]domain.CICheck, error)
109	110		}
110	111

 // CIChecksReader is the read side of the checks, for the merge rule and the
 // pages that show them. *usecase.CI implements it.
 type CIChecksReader interface {
-	// CommitChecks returns the checks of a commit, at most 1,000, without
+	// CommitChecks returns the checks of a commit, without access control (the
-	// access control (the caller has checked it).
+	// caller has checked it). More than 1,000 is domain.ErrResourceExhausted,
+	// not a part of them: a rule must not decide on some of the checks.
 	CommitChecks(ctx context.Context, repo domain.RepoName, sha string) ([]domain.CICheck, error)
 }

M internal/usecase/ci_runner_unit_test.go +16 -0


@@ -269,3 +269,19 @@ func TestGetJobLogRevalidatesABusyStream(t *testing.T) {
269	269		assert.ErrorIs(t, err, domain.ErrUnauthenticated, "a revoked token stops a busy stream")
270	270		assert.GreaterOrEqual(t, sent, 2)
271	271		}
	272	+
	273	+	// S9: more than 1,000 checks on a commit is an error for the merge rule, not a
	274	+	// list cut at 1,000.
	275	+	func TestCommitChecksRefusesATruncatedList(t *testing.T) {
	276	+	t.Parallel()
	277	+	u := newCIUnit(t, nil)
	278	+	repo := name(t, "proj")
	279	+	u.store.EXPECT().ListChecks(mock.Anything, repo, "abc", usecase.MaxCIChecks).Return(make([]domain.CICheck, usecase.MaxCIChecks), true, nil).Once()
	280	+	_, err := u.ci.CommitChecks(context.Background(), repo, "abc")
	281	+	assert.ErrorIs(t, err, domain.ErrResourceExhausted)
	282	+
	283	+	u.store.EXPECT().ListChecks(mock.Anything, repo, "def", usecase.MaxCIChecks).Return([]domain.CICheck{{Name: "a"}}, false, nil).Once()
	284	+	checks, err := u.ci.CommitChecks(context.Background(), repo, "def")
	285	+	require.NoError(t, err)
	286	+	assert.Len(t, checks, 1)
	287	+	}


@@ -269,3 +269,19 @@ func TestGetJobLogRevalidatesABusyStream(t *testing.T) {
269	assert.ErrorIs(t, err, domain.ErrUnauthenticated, "a revoked token stops a busy stream")	269		assert.ErrorIs(t, err, domain.ErrUnauthenticated, "a revoked token stops a busy stream")
270	assert.GreaterOrEqual(t, sent, 2)	270		assert.GreaterOrEqual(t, sent, 2)
271	}	271		}
		272	+
		273	+	// S9: more than 1,000 checks on a commit is an error for the merge rule, not a
		274	+	// list cut at 1,000.
		275	+	func TestCommitChecksRefusesATruncatedList(t *testing.T) {
		276	+	t.Parallel()
		277	+	u := newCIUnit(t, nil)
		278	+	repo := name(t, "proj")
		279	+	u.store.EXPECT().ListChecks(mock.Anything, repo, "abc", usecase.MaxCIChecks).Return(make([]domain.CICheck, usecase.MaxCIChecks), true, nil).Once()
		280	+	_, err := u.ci.CommitChecks(context.Background(), repo, "abc")
		281	+	assert.ErrorIs(t, err, domain.ErrResourceExhausted)
		282	+
		283	+	u.store.EXPECT().ListChecks(mock.Anything, repo, "def", usecase.MaxCIChecks).Return([]domain.CICheck{{Name: "a"}}, false, nil).Once()
		284	+	checks, err := u.ci.CommitChecks(context.Background(), repo, "def")
		285	+	require.NoError(t, err)
		286	+	assert.Len(t, checks, 1)
		287	+	}

M internal/usecase/ci_runs.go +11 -3


@@ -466,10 +466,18 @@ func (w *wfCache) drop(runID int64) {
466	466
467	467		// --- checks ---
468	468
469		-	// CommitChecks returns the checks of a commit, without access control.
	469	+	// CommitChecks returns the checks of a commit, without access control. A
	470	+	// commit with more than MaxCIChecks checks is an error (domain.ErrResourceExhausted):
	471	+	// the merge rule cannot answer from a part of them.
470	472		func (c *CI) CommitChecks(ctx context.Context, repo domain.RepoName, sha string) ([]domain.CICheck, error) {
471		-	checks, _, err := c.cfg.Store.ListChecks(ctx, repo, sha, MaxCIChecks)
472		-	return checks, err
	473	+	checks, more, err := c.cfg.Store.ListChecks(ctx, repo, sha, MaxCIChecks)
	474	+	if err != nil {
	475	+	return nil, err
	476	+	}
	477	+	if more {
	478	+	return nil, fmt.Errorf("%w: the commit has more than %d checks", domain.ErrResourceExhausted, MaxCIChecks)
	479	+	}
	480	+	return checks, nil
473	481		}
474	482
475	483		// ListChecks returns the checks of a commit: the one of each job of the latest

 // --- checks ---
-// CommitChecks returns the checks of a commit, without access control.
+// CommitChecks returns the checks of a commit, without access control. A
+// commit with more than MaxCIChecks checks is an error (domain.ErrResourceExhausted):
+// the merge rule cannot answer from a part of them.
 func (c *CI) CommitChecks(ctx context.Context, repo domain.RepoName, sha string) ([]domain.CICheck, error) {
-	checks, _, err := c.cfg.Store.ListChecks(ctx, repo, sha, MaxCIChecks)
+	checks, more, err := c.cfg.Store.ListChecks(ctx, repo, sha, MaxCIChecks)
-	return checks, err
+	if err != nil {
+		return nil, err
+	}
+	if more {
+		return nil, fmt.Errorf("%w: the commit has more than %d checks", domain.ErrResourceExhausted, MaxCIChecks)
+	}
+	return checks, nil
 }
 // ListChecks returns the checks of a commit: the one of each job of the latest

M internal/usecase/ci_unit_test.go +8 -0


@@ -91,6 +91,14 @@ func TestMergeRuleOfFailingChecksBlock(t *testing.T) {
91	91		assert.Contains(t, err.Error(), tt.blocked)
92	92		})
93	93		}
	94	+	t.Run("too many checks to tell", func(t *testing.T) {
	95	+	t.Parallel()
	96	+	m := newMergeUnit(t, true)
	97	+	m.checks.EXPECT().CommitChecks(mock.Anything, m.repo.Name, m.head.String()).Return(nil, domain.ErrResourceExhausted)
	98	+	err := m.merge()
	99	+	require.ErrorIs(t, err, domain.ErrPreconditionFailed)
	100	+	assert.Contains(t, err.Error(), "too many checks")
	101	+	})
94	102		// The cases where the merge goes on (no check, all passing, rule off) need
95	103		// the whole merge: see TestCIMergeIsBlockedByChecks, which merges for real.
96	104		}


@@ -91,6 +91,14 @@ func TestMergeRuleOfFailingChecksBlock(t *testing.T) {
91	assert.Contains(t, err.Error(), tt.blocked)	91		assert.Contains(t, err.Error(), tt.blocked)
92	})	92		})
93	}	93		}
		94	+	t.Run("too many checks to tell", func(t *testing.T) {
		95	+	t.Parallel()
		96	+	m := newMergeUnit(t, true)
		97	+	m.checks.EXPECT().CommitChecks(mock.Anything, m.repo.Name, m.head.String()).Return(nil, domain.ErrResourceExhausted)
		98	+	err := m.merge()
		99	+	require.ErrorIs(t, err, domain.ErrPreconditionFailed)
		100	+	assert.Contains(t, err.Error(), "too many checks")
		101	+	})
94	// The cases where the merge goes on (no check, all passing, rule off) need	102		// The cases where the merge goes on (no check, all passing, rule off) need
95	// the whole merge: see TestCIMergeIsBlockedByChecks, which merges for real.	103		// the whole merge: see TestCIMergeIsBlockedByChecks, which merges for real.
96	}	104		}

M internal/usecase/mergerequests.go +3 -0


@@ -821,6 +821,9 @@ func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, h
821	821		return nil
822	822		}
823	823		checks, err := m.checks.CommitChecks(ctx, name, head.String())
	824	+	if errors.Is(err, domain.ErrResourceExhausted) {
	825	+	return fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)
	826	+	}
824	827		if err != nil {
825	828		return fmt.Errorf("read the checks of %s: %w", head, err)
826	829		}


@@ -821,6 +821,9 @@ func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, h
821	return nil	821		return nil
822	}	822		}
823	checks, err := m.checks.CommitChecks(ctx, name, head.String())	823		checks, err := m.checks.CommitChecks(ctx, name, head.String())
		824	+	if errors.Is(err, domain.ErrResourceExhausted) {
		825	+	return fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)
		826	+	}
824	if err != nil {	827		if err != nil {
825	return fmt.Errorf("read the checks of %s: %w", head, err)	828		return fmt.Errorf("read the checks of %s: %w", head, err)
826	}	829		}