From f5b07d012e48027d2386331ee28b16ebba9a847d Mon Sep 17 00:00:00 2001 From: Jory Irving Date: Wed, 2 Sep 2026 16:49:44 -0600 Subject: [PATCH] feat(pr-followup): link a PR to its issue from commit messages when the body has none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extractLinkedIssue read only the PR title and body. A PR whose body carries no reference is never linked, so it has no linked-issue health, is never enqueued for a fix, and a CHANGES_REQUESTED review on it is never acted on — the PR sits blocked indefinitely with nothing watching it. That is not hypothetical: misospace/pr-reviewer-action#535 and misospace/llmkube-images#284 both opened with an empty body while the commit that did the work said 'Fixes #534' and 'Fixes #277'. The link existed; we were not looking where it survived. Fall back to the PR's commit messages, fetched only when title and body yield nothing. Commits are matched more strictly than the body — a closing keyword is required, since a bare '#123' in a commit is as likely to reference a prior PR as to declare what this one fixes, and a wrong link is worse than none. Claude-Session: https://claude.ai/code/session_01YSuDvZq9ncvyX85Uzx3cQh --- src/app/api/pr-followup/sync/route.ts | 17 ++++++- src/lib/github-facades.test.ts | 1 + src/lib/github-prs.ts | 15 +++++++ src/lib/github.ts | 1 + src/lib/pr-followup-ingestion.test.ts | 64 +++++++++++++++++++++++++++ src/lib/pr-followup-ingestion.ts | 27 ++++++++++- 6 files changed, 122 insertions(+), 3 deletions(-) diff --git a/src/app/api/pr-followup/sync/route.ts b/src/app/api/pr-followup/sync/route.ts index f82a8be4..f194f29d 100644 --- a/src/app/api/pr-followup/sync/route.ts +++ b/src/app/api/pr-followup/sync/route.ts @@ -5,6 +5,7 @@ import { reconcileStalePrFixItems } from "@/lib/pr-fix-queue"; import { authorizeRequest } from "@/lib/auth"; import { getTrackedRepos } from "@/lib/config"; import { getGitHubToken, fetchPaginated, fetchPullRequests, fetchPullRequestMergeState, fetchFailedJobLogExcerpt, fetchClosedPullRequests, jobIdFromCheckRunUrl, type GithubPR as GithubPRBase } from "@/lib/github"; +import { fetchPullRequestCommitMessages } from "@/lib/github"; import { processPrFollowupEvents, extractLinkedIssue, isAllowedBotAuthor, ingestMergeConflict, clearResolvedConflictItems } from "@/lib/pr-followup-ingestion"; import { enforceRateLimit } from "@/lib/rate-limit"; import { acquireLock, releaseLock, type AcquiredLock, type LockConflict } from "@/lib/sync-lock"; @@ -135,7 +136,21 @@ export async function POST(request: NextRequest) { prsScanned += botPrs.length; for (const pr of botPrs) { - const linkedIssue = extractLinkedIssue(pr); + // Fall back to the commit messages when the PR carries no reference. + // An unlinked PR never reaches follow-up, so a review requesting + // changes on it is never queued for a fix. + let linkedIssue = extractLinkedIssue(pr); + if (linkedIssue === null) { + try { + const commitMessages = await fetchPullRequestCommitMessages( + `${owner}/${repo}`, + pr.number, + ); + linkedIssue = extractLinkedIssue({ ...pr, commitMessages }); + } catch { + // Best effort: an unlinked PR is the status quo, not a failure. + } + } // Comments, reviews, and check runs are independent — fetch them in // parallel, best effort per source (a failed fetch yields no events). diff --git a/src/lib/github-facades.test.ts b/src/lib/github-facades.test.ts index 63ef6d33..238320d1 100644 --- a/src/lib/github-facades.test.ts +++ b/src/lib/github-facades.test.ts @@ -52,6 +52,7 @@ describe("github domain modules expose expected exports", () => { "fetchClosedPullRequests", "fetchLinkedPrHealthInput", "fetchPullRequestCheckFailures", + "fetchPullRequestCommitMessages", "fetchPullRequestHealthSignals", "fetchPullRequestMergeState", "fetchPullRequestState", diff --git a/src/lib/github-prs.ts b/src/lib/github-prs.ts index 89bc8d24..df9b37f8 100644 --- a/src/lib/github-prs.ts +++ b/src/lib/github-prs.ts @@ -156,3 +156,18 @@ export async function fetchLinkedPrHealthInput(repoFullName: string, pr: GithubP checkFailures, }; } + +/** + * Commit messages on a pull request, newest last. Used only as a fallback when + * a PR's title and body carry no issue reference — the commit that did the work + * often still names the issue. + */ +export async function fetchPullRequestCommitMessages( + repoFullName: string, + prNumber: number, +): Promise { + const commits = await fetchPaginated<{ commit?: { message?: string } }>( + `${GITHUB_API}/repos/${repoFullName}/pulls/${prNumber}/commits`, + ); + return commits.map((c) => c.commit?.message ?? "").filter(Boolean); +} diff --git a/src/lib/github.ts b/src/lib/github.ts index 1a729f43..b750a8f8 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -53,6 +53,7 @@ export { fetchPullRequestMergeState, fetchPullRequestCheckFailures, fetchLinkedPrHealthInput, + fetchPullRequestCommitMessages, } from "./github-prs"; export { diff --git a/src/lib/pr-followup-ingestion.test.ts b/src/lib/pr-followup-ingestion.test.ts index aa5e1be5..b9ecff6f 100644 --- a/src/lib/pr-followup-ingestion.test.ts +++ b/src/lib/pr-followup-ingestion.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, beforeEach, afterEach, vi } from "vitest"; import { + extractLinkedIssue, classifyFeedback, parseAiReviewerFindings, isInformationalComment, @@ -1073,3 +1074,66 @@ describe("comment ingestion requires actionable signal", () => { for (const item of client.items) expect(item.lane).toBe("NORMAL"); }); }); + +describe("extractLinkedIssue commit fallback", () => { + it("still prefers the body when it carries a reference", () => { + expect( + extractLinkedIssue({ + title: "fix: thing", + body: "Fixes #42", + commitMessages: ["chore: x\n\nFixes #99"], + }), + ).toBe(42); + }); + + it("falls back to a commit message when title and body have none", () => { + expect( + extractLinkedIssue({ + title: "fix(carry_forward): let a maintainer dismiss a finding", + body: "## Summary\n- does the thing\n", + commitMessages: ["fix(carry_forward): dismiss\n\nFixes #534\n"], + }), + ).toBe(534); + }); + + it("requires a closing keyword in a commit, ignoring a bare reference", () => { + expect( + extractLinkedIssue({ + title: "t", + body: "b", + commitMessages: ["revert of #101, see discussion"], + }), + ).toBeNull(); + }); + + it("accepts the closing-keyword variants", () => { + for (const [msg, want] of [ + ["Fixes #1", 1], + ["fixed #2", 2], + ["Closes #3", 3], + ["closed #4", 4], + ["Resolves #5", 5], + ["resolve #6", 6], + ] as const) { + expect(extractLinkedIssue({ title: "t", body: "b", commitMessages: [msg] })).toBe(want); + } + }); + + it("takes the first commit that names an issue", () => { + expect( + extractLinkedIssue({ + title: "t", + body: "b", + commitMessages: ["no reference here", "Fixes #77", "Fixes #88"], + }), + ).toBe(77); + }); + + it("tolerates absent, empty and null commit messages", () => { + expect(extractLinkedIssue({ title: "t", body: "b" })).toBeNull(); + expect(extractLinkedIssue({ title: "t", body: "b", commitMessages: [] })).toBeNull(); + expect( + extractLinkedIssue({ title: "t", body: "b", commitMessages: [null, undefined, ""] }), + ).toBeNull(); + }); +}); diff --git a/src/lib/pr-followup-ingestion.ts b/src/lib/pr-followup-ingestion.ts index e3e9579d..17fb57a7 100644 --- a/src/lib/pr-followup-ingestion.ts +++ b/src/lib/pr-followup-ingestion.ts @@ -261,11 +261,34 @@ export function computeEvidenceKey( /** * Extract the linked issue number from a PR's title and body. * Matches the first "#NNN" reference (e.g. "#42", "Fixes #42", "Closes #42"). + * + * `commitMessages` is a fallback for PRs whose title and body carry no + * reference at all. A PR opened by automation can lose the reference in its + * body while the commit that did the work still names the issue, and an + * unlinked PR drops out of follow-up entirely — no linked-issue health, no + * queued fix, so a CHANGES_REQUESTED review is never acted on. + * + * Commits are matched more strictly than the body: only a closing keyword + * counts. A bare "#123" in a commit message is as likely to be a reference to + * a prior PR as a declaration of what this one fixes, and a wrong link is + * worse than none. */ -export function extractLinkedIssue(pr: { title?: string | null; body?: string | null }): number | null { +const CLOSING_KEYWORD = /\b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?)\s+#(\d+)\b/i; + +export function extractLinkedIssue(pr: { + title?: string | null; + body?: string | null; + commitMessages?: readonly (string | null | undefined)[]; +}): number | null { const text = [pr.title, pr.body].filter(Boolean).join("\n"); const match = text.match(/#(\d+)/); - return match ? parseInt(match[1], 10) : null; + if (match) return parseInt(match[1], 10); + + for (const message of pr.commitMessages ?? []) { + const closing = (message ?? "").match(CLOSING_KEYWORD); + if (closing) return parseInt(closing[1], 10); + } + return null; } // ─── Event Ingestion ────────────────────────────────────────────────────────