From d6b0e7efe247b3a2d9fa5864a619221f9cd18331 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:27 -0400 Subject: [PATCH 1/6] pulls: add ListOpenPRsForHeadSHA query (S64) --- internal/pulls/queries/pulls.sql | 15 ++++++++++++ internal/pulls/sqlc/pulls.sql.go | 40 ++++++++++++++++++++++++++++++++ internal/pulls/sqlc/querier.go | 6 +++++ 3 files changed, 61 insertions(+) diff --git a/internal/pulls/queries/pulls.sql b/internal/pulls/queries/pulls.sql index aa47d9a7..7c0eaad6 100644 --- a/internal/pulls/queries/pulls.sql +++ b/internal/pulls/queries/pulls.sql @@ -155,3 +155,18 @@ WHERE pr.head_repo_id = $1 AND pr.head_ref = $2 AND i.state = 'open' AND pr.merged_at IS NULL; + + +-- name: ListOpenPRsForHeadSHA :many +-- Returns the issue_ids of every still-open PR whose head_repo_id + +-- head_oid match a given SHA. Used by the check-completion trigger +-- (S64) to fan-out pr:mergeability jobs once CI for a head SHA +-- finishes — the required-checks gate inside Mergeability needs a +-- recompute to flip blocked → clean. +SELECT pr.issue_id +FROM pull_requests pr +JOIN issues i ON i.id = pr.issue_id +WHERE pr.head_repo_id = $1 + AND pr.head_oid = $2 + AND i.state = 'open' + AND pr.merged_at IS NULL; diff --git a/internal/pulls/sqlc/pulls.sql.go b/internal/pulls/sqlc/pulls.sql.go index 618b25ad..d558ef35 100644 --- a/internal/pulls/sqlc/pulls.sql.go +++ b/internal/pulls/sqlc/pulls.sql.go @@ -347,6 +347,46 @@ func (q *Queries) ListOpenPRsForHeadRef(ctx context.Context, db DBTX, arg ListOp return items, nil } +const listOpenPRsForHeadSHA = `-- name: ListOpenPRsForHeadSHA :many +SELECT pr.issue_id +FROM pull_requests pr +JOIN issues i ON i.id = pr.issue_id +WHERE pr.head_repo_id = $1 + AND pr.head_oid = $2 + AND i.state = 'open' + AND pr.merged_at IS NULL +` + +type ListOpenPRsForHeadSHAParams struct { + HeadRepoID int64 + HeadOid string +} + +// Returns the issue_ids of every still-open PR whose head_repo_id + +// head_oid match a given SHA. Used by the check-completion trigger +// (S64) to fan-out pr:mergeability jobs once CI for a head SHA +// finishes — the required-checks gate inside Mergeability needs a +// recompute to flip blocked → clean. +func (q *Queries) ListOpenPRsForHeadSHA(ctx context.Context, db DBTX, arg ListOpenPRsForHeadSHAParams) ([]int64, error) { + rows, err := db.Query(ctx, listOpenPRsForHeadSHA, arg.HeadRepoID, arg.HeadOid) + if err != nil { + return nil, err + } + defer rows.Close() + items := []int64{} + for rows.Next() { + var issue_id int64 + if err := rows.Scan(&issue_id); err != nil { + return nil, err + } + items = append(items, issue_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listPullRequestCommits = `-- name: ListPullRequestCommits :many SELECT pr_id, sha, position, author_name, author_email, committer_name, committer_email, subject, body, authored_at, committed_at FROM pull_request_commits WHERE pr_id = $1 diff --git a/internal/pulls/sqlc/querier.go b/internal/pulls/sqlc/querier.go index e0500a6e..98b1f4e3 100644 --- a/internal/pulls/sqlc/querier.go +++ b/internal/pulls/sqlc/querier.go @@ -58,6 +58,12 @@ type Querier interface { // head_ref match the pushed ref. push:process uses this to fan-out // pr:synchronize jobs after a head-side push. ListOpenPRsForHeadRef(ctx context.Context, db DBTX, arg ListOpenPRsForHeadRefParams) ([]int64, error) + // Returns the issue_ids of every still-open PR whose head_repo_id + + // head_oid match a given SHA. Used by the check-completion trigger + // (S64) to fan-out pr:mergeability jobs once CI for a head SHA + // finishes — the required-checks gate inside Mergeability needs a + // recompute to flip blocked → clean. + ListOpenPRsForHeadSHA(ctx context.Context, db DBTX, arg ListOpenPRsForHeadSHAParams) ([]int64, error) ListPRReviewComments(ctx context.Context, db DBTX, prIssueID int64) ([]PrReviewComment, error) // Files-tab fetch: comments anchored to a single file path, oldest first. ListPRReviewCommentsForFile(ctx context.Context, db DBTX, arg ListPRReviewCommentsForFileParams) ([]PrReviewComment, error) From 272a4d880542d3bafcddf7686c6585b36b941178 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:34 -0400 Subject: [PATCH 2/6] pulls/mergeenqueue: extract pr:mergeability enqueue helpers (S64) --- internal/pulls/mergeenqueue/mergeenqueue.go | 63 +++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 internal/pulls/mergeenqueue/mergeenqueue.go diff --git a/internal/pulls/mergeenqueue/mergeenqueue.go b/internal/pulls/mergeenqueue/mergeenqueue.go new file mode 100644 index 00000000..7177af7f --- /dev/null +++ b/internal/pulls/mergeenqueue/mergeenqueue.go @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package mergeenqueue centralizes pr:mergeability job enqueueing so +// triggers across the codebase (PR create, head sync, check completion, +// review submit) share a single, side-effect-only helper. Lives outside +// the pulls orchestrator package to break the +// pulls → actions/trigger → actions/checksync import cycle. +// +// Best-effort: every helper here logs on failure and returns nothing. +// pr:mergeability is idempotent — a missed enqueue gets picked up by +// the next trigger. +package mergeenqueue + +import ( + "context" + "log/slog" + + "github.com/jackc/pgx/v5/pgxpool" + + pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc" + "github.com/tenseleyFlow/shithub/internal/worker" +) + +// ForPR enqueues a pr:mergeability job for prID. trigger is a short +// label used for logs / future metrics ("pr_create", "head_sync", +// "check_complete", "review_submit"). +func ForPR(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, prID int64, trigger string) { + if pool == nil || prID == 0 { + return + } + if _, err := worker.Enqueue(ctx, pool, worker.KindPRMergeability, + map[string]any{"pr_id": prID}, worker.EnqueueOptions{}); err != nil { + if logger != nil { + logger.WarnContext(ctx, "mergeenqueue: enqueue", + "error", err, "pr_id", prID, "trigger", trigger) + } + return + } + _ = worker.Notify(ctx, pool) +} + +// ForHeadSHA fans out ForPR to every open PR whose head_oid matches +// headSHA in the given repo. Use after a check run for that SHA +// transitions to "completed". +func ForHeadSHA(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, repoID int64, headSHA string) { + if pool == nil || headSHA == "" { + return + } + prIDs, err := pullsdb.New().ListOpenPRsForHeadSHA(ctx, pool, pullsdb.ListOpenPRsForHeadSHAParams{ + HeadRepoID: repoID, + HeadOid: headSHA, + }) + if err != nil { + if logger != nil { + logger.WarnContext(ctx, "mergeenqueue: list open PRs", + "error", err, "repo_id", repoID, "head_sha", headSHA) + } + return + } + for _, prID := range prIDs { + ForPR(ctx, pool, logger, prID, "check_complete") + } +} From 4e62eadf9ad504d9cd31748a517f6b7d8fe8c4b4 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:34 -0400 Subject: [PATCH 3/6] pulls/mergeenqueue: cover ForHeadSHA fan-out + empty-SHA guard (S64) --- .../pulls/mergeenqueue/mergeenqueue_test.go | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 internal/pulls/mergeenqueue/mergeenqueue_test.go diff --git a/internal/pulls/mergeenqueue/mergeenqueue_test.go b/internal/pulls/mergeenqueue/mergeenqueue_test.go new file mode 100644 index 00000000..ad3fbd71 --- /dev/null +++ b/internal/pulls/mergeenqueue/mergeenqueue_test.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package mergeenqueue_test + +import ( + "context" + "io" + "log/slog" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc" + "github.com/tenseleyFlow/shithub/internal/pulls/mergeenqueue" + pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc" + reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" + "github.com/tenseleyFlow/shithub/internal/testing/dbtest" + usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" +) + +const fixtureHash = "$argon2id$v=19$m=16384,t=1,p=1$" + + "AAAAAAAAAAAAAAAA$" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + +// S64: ForHeadSHA fans out one pr:mergeability job per open PR whose +// head_oid matches the given SHA. Used by the check-completion trigger. +// Merged or closed PRs are filtered out. +func TestForHeadSHA_EnqueuesPerOpenPR(t *testing.T) { + ctx := context.Background() + pool := dbtest.NewTestDB(t) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + uq := usersdb.New() + user, err := uq.CreateUser(ctx, pool, usersdb.CreateUserParams{ + Username: "alice", DisplayName: "Alice", PasswordHash: fixtureHash, + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + repo, err := reposdb.New().CreateRepo(ctx, pool, reposdb.CreateRepoParams{ + OwnerUserID: pgtype.Int8{Int64: user.ID, Valid: true}, + Name: "demo", + DefaultBranch: "trunk", + Visibility: reposdb.RepoVisibilityPublic, + }) + if err != nil { + t.Fatalf("CreateRepo: %v", err) + } + iq := issuesdb.New() + if err := iq.EnsureRepoIssueCounter(ctx, pool, repo.ID); err != nil { + t.Fatalf("EnsureRepoIssueCounter: %v", err) + } + + const sharedSHA = "deadbeefcafef00ddeadbeefcafef00ddeadbeef" + const otherSHA = "0000000000000000000000000000000000000000" + + // Two PRs at sharedSHA, one PR at otherSHA — only the first two + // should be enqueued. + var nextNum int64 + openPR := func(headRef, headOID string) int64 { + nextNum++ + issue, err := iq.CreateIssue(ctx, pool, issuesdb.CreateIssueParams{ + RepoID: repo.ID, + Number: nextNum, + Kind: issuesdb.IssueKindPr, + Title: "x", + Body: "", + AuthorUserID: pgtype.Int8{Int64: user.ID, Valid: true}, + }) + if err != nil { + t.Fatalf("CreateIssue: %v", err) + } + if _, err := pullsdb.New().CreatePullRequest(ctx, pool, pullsdb.CreatePullRequestParams{ + IssueID: issue.ID, BaseRef: "trunk", HeadRef: headRef, HeadRepoID: repo.ID, + BaseOid: sharedSHA, HeadOid: headOID, Draft: false, + }); err != nil { + t.Fatalf("CreatePullRequest: %v", err) + } + return issue.ID + } + pr1 := openPR("feature-a", sharedSHA) + pr2 := openPR("feature-b", sharedSHA) + _ = openPR("feature-c", otherSHA) + + mergeenqueue.ForHeadSHA(ctx, pool, logger, repo.ID, sharedSHA) + + var n1, n2 int + if err := pool.QueryRow(ctx, + `SELECT count(*) FROM jobs WHERE kind = 'pr:mergeability' + AND (payload->>'pr_id')::bigint = $1`, pr1).Scan(&n1); err != nil { + t.Fatalf("count pr1: %v", err) + } + if err := pool.QueryRow(ctx, + `SELECT count(*) FROM jobs WHERE kind = 'pr:mergeability' + AND (payload->>'pr_id')::bigint = $1`, pr2).Scan(&n2); err != nil { + t.Fatalf("count pr2: %v", err) + } + if n1 == 0 || n2 == 0 { + t.Errorf("expected jobs for pr1=%d and pr2=%d; got %d and %d", pr1, pr2, n1, n2) + } + + // Verify the other-SHA PR was NOT enqueued. + var total int + if err := pool.QueryRow(ctx, + `SELECT count(*) FROM jobs WHERE kind = 'pr:mergeability'`, + ).Scan(&total); err != nil { + t.Fatalf("count total: %v", err) + } + if total > n1+n2 { + t.Errorf("expected only PRs at sharedSHA to enqueue; total=%d (pr1+pr2=%d)", total, n1+n2) + } +} + +// Empty SHA is a no-op — guards against accidental fan-out on rows that +// haven't been snapshotted yet. +func TestForHeadSHA_EmptyHeadSHA(t *testing.T) { + pool := dbtest.NewTestDB(t) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + // No PRs needed: just confirms the helper doesn't panic + writes + // nothing. + mergeenqueue.ForHeadSHA(context.Background(), pool, logger, 1, "") + var n int + if err := pool.QueryRow(context.Background(), + `SELECT count(*) FROM jobs WHERE kind = 'pr:mergeability'`, + ).Scan(&n); err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Errorf("empty SHA should not enqueue; got %d jobs", n) + } +} From 1197b4ebe4f20ad4b60d6b86069136c2c4de8a10 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:41 -0400 Subject: [PATCH 4/6] pulls: kick pr:mergeability after Create (S64 trigger 1) --- internal/pulls/pulls.go | 6 ++++++ internal/pulls/pulls_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/pulls/pulls.go b/internal/pulls/pulls.go index 2a8f7f2e..89f5dc65 100644 --- a/internal/pulls/pulls.go +++ b/internal/pulls/pulls.go @@ -34,6 +34,7 @@ import ( "github.com/tenseleyFlow/shithub/internal/issues" issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc" mdrender "github.com/tenseleyFlow/shithub/internal/markdown" + "github.com/tenseleyFlow/shithub/internal/pulls/mergeenqueue" "github.com/tenseleyFlow/shithub/internal/pulls/review" pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc" repogit "github.com/tenseleyFlow/shithub/internal/repos/git" @@ -163,6 +164,11 @@ func Create(ctx context.Context, deps Deps, p CreateParams) (CreateResult, error } } + // S64: kick off the initial mergeability tick so the PR's + // mergeable_state moves off `unknown` without waiting for a human to + // open the HTML review screen (the only previous trigger). + mergeenqueue.ForPR(ctx, deps.Pool, deps.Logger, prRow.IssueID, "pr_create") + return CreateResult{Issue: issueRow, PullRequest: prRow}, nil } diff --git a/internal/pulls/pulls_test.go b/internal/pulls/pulls_test.go index b98156f0..83169d1a 100644 --- a/internal/pulls/pulls_test.go +++ b/internal/pulls/pulls_test.go @@ -212,6 +212,41 @@ func TestCreate_OpensPRWithIssueRow(t *testing.T) { } } +// S64: Create enqueues a pr:mergeability job so the new PR's +// mergeable_state moves off `unknown` without waiting for a human to +// open the HTML review screen. The earlier "review-handler-only" wiring +// left CLI-driven PRs stuck on `unknown` forever — A17/A20 audit. +func TestCreate_EnqueuesMergeabilityJob(t *testing.T) { + f := setup(t) + commitOnBranch(t, f.gitDir, "trunk", "init", "README.md", "hi\n") + commitOnBranch(t, f.gitDir, "feature", "add foo", "foo.txt", "foo\n") + + res, err := pulls.Create(context.Background(), f.deps, pulls.CreateParams{ + RepoID: f.repoID, + AuthorUserID: f.userID, + Title: "Add foo", + BaseRef: "trunk", + HeadRef: "feature", + GitDir: f.gitDir, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + var n int + if err := f.pool.QueryRow(context.Background(), + `SELECT count(*) FROM jobs + WHERE kind = 'pr:mergeability' + AND (payload->>'pr_id')::bigint = $1`, + res.PullRequest.IssueID, + ).Scan(&n); err != nil { + t.Fatalf("count jobs: %v", err) + } + if n < 1 { + t.Errorf("expected at least one pr:mergeability job for PR %d, got %d", res.PullRequest.IssueID, n) + } +} + func TestCreate_RequestsCodeOwners(t *testing.T) { f := setup(t) ctx := context.Background() From 34ac53145be37e676df5473f600f478c51cb14e1 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:41 -0400 Subject: [PATCH 5/6] api/checks: fan out mergeability on completed status (S64 trigger 5) --- internal/web/handlers/api/checks.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/web/handlers/api/checks.go b/internal/web/handlers/api/checks.go index c1c9801c..dc7531ee 100644 --- a/internal/web/handlers/api/checks.go +++ b/internal/web/handlers/api/checks.go @@ -16,6 +16,7 @@ import ( "github.com/tenseleyFlow/shithub/internal/auth/policy" "github.com/tenseleyFlow/shithub/internal/checks" checksdb "github.com/tenseleyFlow/shithub/internal/checks/sqlc" + "github.com/tenseleyFlow/shithub/internal/pulls/mergeenqueue" reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" "github.com/tenseleyFlow/shithub/internal/web/middleware" ) @@ -198,6 +199,12 @@ func (h *Handlers) checkRunUpdate(w http.ResponseWriter, r *http.Request) { writeChecksError(w, err) return } + // S64: if this update flipped the run into "completed", fan out + // pr:mergeability ticks to every open PR sharing this head SHA so + // blocked-on-required-checks states recompute promptly. + if updated.Status == checksdb.CheckStatusCompleted { + mergeenqueue.ForHeadSHA(r.Context(), h.d.Pool, h.d.Logger, updated.RepoID, updated.HeadSha) + } writeJSON(w, http.StatusOK, presentRun(updated)) } From f4b1f32b174c9be5578042566f2d2751a4666ea4 Mon Sep 17 00:00:00 2001 From: espadonne Date: Mon, 18 May 2026 00:52:41 -0400 Subject: [PATCH 6/6] actions/checksync: fan out mergeability on completed job (S64 trigger 5) --- internal/actions/checksync/checksync.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/actions/checksync/checksync.go b/internal/actions/checksync/checksync.go index ccb9b1f3..0e2aa1d9 100644 --- a/internal/actions/checksync/checksync.go +++ b/internal/actions/checksync/checksync.go @@ -18,6 +18,7 @@ import ( actionsdb "github.com/tenseleyFlow/shithub/internal/actions/sqlc" "github.com/tenseleyFlow/shithub/internal/checks" checksdb "github.com/tenseleyFlow/shithub/internal/checks/sqlc" + "github.com/tenseleyFlow/shithub/internal/pulls/mergeenqueue" ) // Deps wires check synchronization to postgres and logging. @@ -91,7 +92,16 @@ func Job(ctx context.Context, deps Deps, job actionsdb.WorkflowJob) error { return nil } _, err = checks.Update(ctx, checks.Deps{Pool: deps.Pool, Logger: deps.Logger}, params) - return err + if err != nil { + return err + } + // S64: when the Actions runner reports a terminal job state, fan out + // pr:mergeability ticks to every open PR sharing the head SHA so a + // required-checks gate can flip blocked → clean. + if params.Status == "completed" { + mergeenqueue.ForHeadSHA(ctx, deps.Pool, deps.Logger, run.RepoID, run.HeadSha) + } + return nil } func timeFromPg(ts pgtype.Timestamptz) time.Time {