From db9ce75392954d04c8cc2543036b050caad2865d Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 4 Sep 2026 20:04:14 -0500 Subject: [PATCH 1/2] engine: honor branch protection at admission Read Forgejo branch-protection rules and wait for required external statuses before staging a merge queue batch. --- internal/engine/engine.go | 46 +++++++++++++++++++++------------- internal/engine/engine_test.go | 26 +++++++++++++++++++ internal/forge/client.go | 2 +- internal/forge/client_test.go | 6 ++--- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 2082af9..6bcdc1f 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -313,7 +313,7 @@ func (e *Engine) readyNumbers(ctx context.Context) ([]int, error) { if err != nil { return nil, err } - gate := admissionGate{protection: protection} + gate := admissionGate{protection: protection, statusCtx: e.cfg.StatusCtx} var nums []int for _, p := range prs { // Quick check: if the PR is not scheduled, it was likely a bounced @@ -343,10 +343,10 @@ func (e *Engine) readyNumbers(ctx context.Context) ([]int, error) { continue } // For an already-scheduled PR the 409 path skips Forgejo's 422 - // requirement validation, so re-check the review policy ourselves: - // a PR blocked by approvals or a live changes-requested review would - // never merge and must not consume a staging/CI run. - if blocked, reason, err := gate.blocks(ctx, e.fc, e.cfg.Owner, e.cfg.Repo, p.Number); err != nil { + // requirement validation, so re-check branch protection ourselves: + // a PR blocked by a required status or review policy would never merge + // and must not consume a staging/CI run. + if blocked, reason, err := gate.blocks(ctx, e.fc, e.cfg.Owner, e.cfg.Repo, p.Number, p.Head.Sha); err != nil { return nil, err } else if blocked { e.logger.Info("PR blocked from merge queue by review policy", "pr", p.Number, "reason", reason) @@ -368,23 +368,35 @@ func (e *Engine) readyNumbers(ctx context.Context) ([]int, error) { return nums, nil } -// admissionGate replicates the branch-protection review policy that Forgejo's -// merge gate applies, so shunt can refuse to queue a PR that can never merge. -// The rule only blocks when it actually requires something; a repo with no -// protection rule (or one without approval/review requirements) admits freely. +// admissionGate replicates the branch-protection status and review policy +// that Forgejo's merge gate applies, so shunt can refuse to queue a PR that +// can never merge. shunt's own status is excluded because it is only written +// after the staged batch passes. type admissionGate struct { protection forge.BranchProtection + statusCtx string } -// blocks reports whether the PR is barred from the queue by the base-branch -// review policy, and why. It mirrors HasEnoughApprovals, -// MergeBlockedByRejectedReview, and MergeBlockedByOfficialReviewRequests: -// approvals must be official, non-dismissed, and (when IgnoreStaleApprovals) -// non-stale; a live REQUEST_CHANGES blocks when BlockOnRejectedReviews; an -// outstanding official review request blocks when -// BlockOnOfficialReviewRequests. -func (g admissionGate) blocks(ctx context.Context, fc ForgeAPI, owner, repo string, num int) (bool, string, error) { +// blocks reports whether the PR is barred from the queue by base-branch +// protection. Required statuses other than shunt's own must be successful. +// Review semantics mirror HasEnoughApprovals, MergeBlockedByRejectedReview, +// and MergeBlockedByOfficialReviewRequests. +func (g admissionGate) blocks(ctx context.Context, fc ForgeAPI, owner, repo string, num int, sha string) (bool, string, error) { p := g.protection + if p.EnableStatusCheck { + for _, context := range p.StatusCheckContexts { + if context == g.statusCtx { + continue + } + status, ok, err := fc.LatestCommitStatus(ctx, owner, repo, sha, context) + if err != nil { + return false, "", err + } + if !ok || status.Status != "success" { + return true, fmt.Sprintf("required status check %q is not successful", context), nil + } + } + } if p.RequiredApprovals == 0 && !p.BlockOnRejectedReviews && !p.BlockOnOfficialReviewRequests { return false, "", nil } diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 1d1121e..ccaeeb4 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -362,6 +362,32 @@ func TestAdmissionAllowsPRWithEnoughApprovals(t *testing.T) { } } +func TestAdmissionBlocksUnsatisfiedRequiredStatusBeforeStaging(t *testing.T) { + for _, tc := range []struct { + name string + status forge.CommitStatus + wantStaged bool + }{ + {name: "missing", wantStaged: false}, + {name: "failure", status: forge.CommitStatus{Context: "ci", Status: "failure"}, wantStaged: false}, + {name: "success", status: forge.CommitStatus{Context: "ci", Status: "success"}, wantStaged: true}, + } { + t.Run(tc.name, func(t *testing.T) { + m := newMock(-1, 1) + m.protection = forge.BranchProtection{EnableStatusCheck: true, StatusCheckContexts: []string{"merge-queue", "ci"}} + m.latestStatus[1] = tc.status + e := New(Config{Owner: "o", Repo: "r", Base: "main", StatusCtx: "merge-queue", StagingBranch: "mq/main/staging"}, m, m) + + if err := e.Reconcile(context.Background()); err != nil { + t.Fatalf("reconcile: %v", err) + } + if got := fmt.Sprint(m.staged); (got == "[[1]]") != tc.wantStaged { + t.Fatalf("staged = %s, want staged = %v", got, tc.wantStaged) + } + }) + } +} + // TestAdmissionGateHonestState is the honest-state sibling: the gate blocks // only when the branch-protection rule actually requires something. A PR with // a dismissed reject, a non-official reject, or no protection rule at all must diff --git a/internal/forge/client.go b/internal/forge/client.go index e6d4be7..3af96fa 100644 --- a/internal/forge/client.go +++ b/internal/forge/client.go @@ -537,7 +537,7 @@ func (c *Client) ListReviews(ctx context.Context, owner, repo string, index int) // ProtectedBranch returns the branch-protection rule for branch, or // (zero-value, nil) when the branch has no protection rule (not blocked). func (c *Client) ProtectedBranch(ctx context.Context, owner, repo, branch string) (BranchProtection, error) { - data, err := c.doRaw(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/branches/%s/protection", repoPath(owner, repo), url.PathEscape(branch)), nil) + data, err := c.doRaw(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/branch_protections/%s", repoPath(owner, repo), url.PathEscape(branch)), nil) if errors.Is(err, ErrNotFound) { return BranchProtection{}, nil } diff --git a/internal/forge/client_test.go b/internal/forge/client_test.go index 9b6f6d0..7fee8c2 100644 --- a/internal/forge/client_test.go +++ b/internal/forge/client_test.go @@ -115,7 +115,7 @@ func TestProtectedBranchParsesRequirementsAnd404MeansNoRule(t *testing.T) { var path string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path = r.URL.Path - if r.URL.Path == "/api/v1/repos/o/r/branches/main/protection" { + if r.URL.Path == "/api/v1/repos/o/r/branch_protections/main" { w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"required_approvals":2,"block_on_rejected_reviews":true,"block_on_official_review_requests":true,"ignore_stale_approvals":true}`) return @@ -132,8 +132,8 @@ func TestProtectedBranchParsesRequirementsAnd404MeansNoRule(t *testing.T) { if p.RequiredApprovals != 2 || !p.BlockOnRejectedReviews || !p.BlockOnOfficialReviewRequests || !p.IgnoreStaleApprovals { t.Fatalf("protection = %+v", p) } - if !strings.Contains(path, "branches/main/protection") { - t.Fatalf("path = %s, want protection endpoint", path) + if !strings.Contains(path, "branch_protections/main") { + t.Fatalf("path = %s, want Forgejo branch-protection endpoint", path) } // A branch with no rule returns a zero-value rule, not an error. From 645b0df51621477a7b1844d0ba006d8a64b8dbaa Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 4 Sep 2026 20:11:43 -0500 Subject: [PATCH 2/2] ci: use patched Go toolchain --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eaa4e6a..2f89499 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff with: - go-version: "1.25.12" + go-version: "1.25.13" - name: gofmt run: test -z "$(gofmt -l .)" || { echo 'gofmt needed:'; gofmt -l .; exit 1; } - name: vet