tenseleyflow/shithub / 752c4ed

Browse files

S24: compose checks gate with reviews in Mergeability + Merge (row-lock recheck)

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
752c4eddf7648b9976660ef45f4bbac45f7261d6
Parents
f0bdb52
Tree
2a62a7a

2 changed files

StatusFile+-
M internal/pulls/merge.go 23 6
M internal/pulls/pulls.go 46 3
internal/pulls/merge.gomodified
@@ -12,6 +12,7 @@ import (
1212
1313
 	"github.com/jackc/pgx/v5/pgtype"
1414
15
+	"github.com/tenseleyFlow/shithub/internal/checks"
1516
 	"github.com/tenseleyFlow/shithub/internal/issues"
1617
 	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
1718
 	"github.com/tenseleyFlow/shithub/internal/pulls/review"
@@ -85,11 +86,12 @@ func Merge(ctx context.Context, deps Deps, p MergeParams) error {
8586
 		return ErrMergeBlocked
8687
 	}
8788
88
-	// Belt-and-braces: re-evaluate the review gate inside the row
89
-	// lock so a `request_changes` submitted between the last
90
-	// Mergeability tick and this merge attempt is still honored.
91
-	// Spec pitfall: required-review bypass via direct API.
92
-	gate, err := review.Evaluate(ctx, deps.Pool, review.GateInputs{
89
+	// Belt-and-braces: re-evaluate both gates (review + required
90
+	// checks) inside the row lock so a `request_changes` submitted or
91
+	// a check failing between the last Mergeability tick and this
92
+	// merge attempt is still honored. Spec pitfall: required-review/
93
+	// required-check bypass via direct API.
94
+	reviewGate, err := review.Evaluate(ctx, deps.Pool, review.GateInputs{
9395
 		RepoID:    issue.RepoID,
9496
 		BaseRef:   pr.BaseRef,
9597
 		PRIssueID: p.PRID,
@@ -97,7 +99,22 @@ func Merge(ctx context.Context, deps Deps, p MergeParams) error {
9799
 	if err != nil {
98100
 		return fmt.Errorf("review gate: %w", err)
99101
 	}
100
-	if !gate.Satisfied {
102
+	if !reviewGate.Satisfied {
103
+		return ErrMergeBlocked
104
+	}
105
+	requiredCheckNames, err := loadRequiredCheckNames(ctx, deps.Pool, issue.RepoID, pr.BaseRef)
106
+	if err != nil {
107
+		return fmt.Errorf("required-check rule lookup: %w", err)
108
+	}
109
+	checksGate, err := checks.EvaluateRequiredChecks(ctx, deps.Pool, checks.GateInputs{
110
+		RepoID:        issue.RepoID,
111
+		HeadSHA:       pr.HeadOid,
112
+		RequiredNames: requiredCheckNames,
113
+	})
114
+	if err != nil {
115
+		return fmt.Errorf("checks gate: %w", err)
116
+	}
117
+	if !checksGate.Satisfied {
101118
 		return ErrMergeBlocked
102119
 	}
103120
internal/pulls/pulls.gomodified
@@ -21,12 +21,14 @@ import (
2121
 	"errors"
2222
 	"fmt"
2323
 	"log/slog"
24
+	"path/filepath"
2425
 	"strings"
2526
2627
 	"github.com/jackc/pgx/v5"
2728
 	"github.com/jackc/pgx/v5/pgtype"
2829
 	"github.com/jackc/pgx/v5/pgxpool"
2930
31
+	"github.com/tenseleyFlow/shithub/internal/checks"
3032
 	"github.com/tenseleyFlow/shithub/internal/issues"
3133
 	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
3234
 	"github.com/tenseleyFlow/shithub/internal/pulls/review"
@@ -322,12 +324,14 @@ func Mergeability(ctx context.Context, deps Deps, gitDir string, prID int64) err
322324
 			MergeableState: pullsdb.PrMergeableStateDirty,
323325
 		})
324326
 	}
325
-	// Review gate. Need the issue's repo + author for the eval.
327
+	// Composed gate: review (S23) + required-checks (S24). Either one
328
+	// failing produces `blocked`. The two evaluators are independent;
329
+	// each loads its own slice of the protection rule.
326330
 	issue, err := issuesdb.New().GetIssueByID(ctx, deps.Pool, prID)
327331
 	if err != nil {
328332
 		return fmt.Errorf("load issue: %w", err)
329333
 	}
330
-	gate, err := review.Evaluate(ctx, deps.Pool, review.GateInputs{
334
+	reviewGate, err := review.Evaluate(ctx, deps.Pool, review.GateInputs{
331335
 		RepoID:    issue.RepoID,
332336
 		BaseRef:   pr.BaseRef,
333337
 		PRIssueID: prID,
@@ -335,9 +339,21 @@ func Mergeability(ctx context.Context, deps Deps, gitDir string, prID int64) err
335339
 	if err != nil {
336340
 		return fmt.Errorf("review gate: %w", err)
337341
 	}
342
+	requiredCheckNames, err := loadRequiredCheckNames(ctx, deps.Pool, issue.RepoID, pr.BaseRef)
343
+	if err != nil {
344
+		return fmt.Errorf("required-check rule lookup: %w", err)
345
+	}
346
+	checksGate, err := checks.EvaluateRequiredChecks(ctx, deps.Pool, checks.GateInputs{
347
+		RepoID:        issue.RepoID,
348
+		HeadSHA:       pr.HeadOid,
349
+		RequiredNames: requiredCheckNames,
350
+	})
351
+	if err != nil {
352
+		return fmt.Errorf("checks gate: %w", err)
353
+	}
338354
 	state := pullsdb.PrMergeableStateClean
339355
 	mergeable := true
340
-	if !gate.Satisfied {
356
+	if !reviewGate.Satisfied || !checksGate.Satisfied {
341357
 		state = pullsdb.PrMergeableStateBlocked
342358
 		mergeable = false
343359
 	}
@@ -348,6 +364,33 @@ func Mergeability(ctx context.Context, deps Deps, gitDir string, prID int64) err
348364
 	})
349365
 }
350366
367
+// loadRequiredCheckNames returns the `status_checks_required` list
368
+// from the longest-pattern-matching protection rule for `baseRef`.
369
+// Empty slice means no rule, no required checks.
370
+func loadRequiredCheckNames(ctx context.Context, pool *pgxpool.Pool, repoID int64, baseRef string) ([]string, error) {
371
+	rules, err := reposdb.New().ListBranchProtectionRules(ctx, pool, repoID)
372
+	if err != nil {
373
+		return nil, err
374
+	}
375
+	var best reposdb.BranchProtectionRule
376
+	bestLen := -1
377
+	for _, r := range rules {
378
+		ok, _ := filepath.Match(r.Pattern, baseRef)
379
+		if !ok {
380
+			continue
381
+		}
382
+		if len(r.Pattern) > bestLen ||
383
+			(len(r.Pattern) == bestLen && r.Pattern < best.Pattern) {
384
+			best = r
385
+			bestLen = len(r.Pattern)
386
+		}
387
+	}
388
+	if bestLen < 0 {
389
+		return []string{}, nil
390
+	}
391
+	return best.StatusChecksRequired, nil
392
+}
393
+
351394
 // int64FromPg unwraps a pgtype.Int8; returns 0 when invalid.
352395
 func int64FromPg(p pgtype.Int8) int64 {
353396
 	if !p.Valid {