fix(pr-fix): keep the item URL as the PR URL when CI evidence re-enqueues it

prisma/migrations/20261004000000_repair_pr_fix_item_urls/migration.sql

@@ -0,0 +1,6 @@
+-- Repair PR-fix item URLs that the pre-#1098 ingestion overwrote with a CI job
+-- URL (#1118). The item URL is identity and is now write-once, so these rows
+-- would otherwise keep the job URL. repo + pr already identify the PR.
+UPDATE "PrFixQueueItem"
+SET "url" = 'https://api.github.com/repos/' || "repo" || '/pulls/' || "pr"
+WHERE "url" ~ '^https://github\.com/[^/]+/[^/]+/actions/runs/';

src/app/api/pr-followup/sync/route.ts

@@ -287,7 +287,8 @@ export async function POST(request: NextRequest) {
               repoFullName,
               prNumber: pr.number,
               branch: pr.head?.ref ?? null,
-              url: checkRun.html_url,
+              url: pr.url,
+              checkRunUrl: checkRun.html_url,
               title: checkRun.name,
               author: pr.user.login,
               body: excerpt || checkRun.output?.summary || "",

src/app/api/pr-followup/webhook/route.test.ts

@@ -680,7 +680,8 @@ describe("POST /api/pr-followup/webhook — event dispatch", () => {
         repoFullName: "org/repo",
         prNumber: 42,
         branch: "fix/issue-7",
-        url: "https://github.com/org/repo/runs/321",
+        url: "https://api.github.com/repos/org/repo/pulls/42",
+        checkRunUrl: "https://github.com/org/repo/runs/321",
         title: "lint",
         author: null,
         body: "2 errors",
@@ -693,6 +694,59 @@ describe("POST /api/pr-followup/webhook — event dispatch", () => {
     ]);
   });
 
+  // #1098: when the associated PR object has no `url`, the event's `url` must
+  // be the empty string (falsy → the next sync enqueue backfills the real PR
+  // URL), never the CI job URL, which the write-once queue guard would freeze
+  // as the item identity. The job URL belongs in `checkRunUrl`.
+  it("sets url to the empty string when the associated PR has no url (#1098)", async () => {
+    const res = await signedRequest("check_run", {
+      action: "completed",
+      check_run: {
+        id: 324,
+        name: "lint",
+        head_sha: "abc123",
+        status: "completed",
+        conclusion: "failure",
+        url: "https://api.github.com/repos/org/repo/check-runs/324",
+        html_url: "https://github.com/org/repo/runs/324",
+        details_url: "https://github.com/org/repo/actions/runs/1/job/324",
+        output: { title: "Lint failed", summary: "2 errors", text: null, annotations_count: 2 },
+        check_suite: { id: 8, head_branch: "fix/issue-7", head_sha: "abc123" },
+        app: { slug: "github-actions" },
+        pull_requests: [
+          {
+            id: 9001,
+            number: 42,
+            head: { ref: "fix/issue-7", sha: "abc123", repo: { id: 1296269, url: "https://api.github.com/repos/org/repo", name: "repo" } },
+            base: { ref: "main", sha: "def456", repo: { id: 1296269, url: "https://api.github.com/repos/org/repo", name: "repo" } },
+          },
+        ],
+      },
+      repository,
+      sender: { login: "github-actions[bot]", type: "Bot" },
+    });
+
+    expect(res.status).toBe(200);
+    expect(ingestedEvents()).toEqual([
+      {
+        eventType: "check_run",
+        repoFullName: "org/repo",
+        prNumber: 42,
+        branch: "fix/issue-7",
+        url: "",
+        checkRunUrl: "https://github.com/org/repo/runs/324",
+        title: "lint",
+        author: null,
+        body: "2 errors",
+        id: "324",
+        conclusion: "failure",
+        checkName: "lint",
+        linkedIssue: null,
+        headSha: "abc123",
+      },
+    ]);
+  });
+
   it("falls back to check_suite.head_branch and check_run.head_sha when the PR head is absent", async () => {
     const res = await signedRequest("check_run", {
       action: "completed",

src/app/api/pr-followup/webhook/route.ts

@@ -120,7 +120,11 @@ function parseWebhookEvent(githubEvent: string, body: Record<string, unknown>):
         repoFullName: checkRun.repository?.full_name ?? null,
         prNumber,
         branch: firstPr?.head?.ref ?? check.check_suite?.head_branch ?? null,
-        url: check.html_url,
+        // #1098: `url` is the item's identity and the queue guard is
+        // write-once — a job-URL fallback would freeze it in place. The empty
+        // string stays falsy, so the next sync enqueue backfills the real PR URL.
+        url: typeof firstPr?.url === "string" ? firstPr.url : "",
+        checkRunUrl: check.html_url,
         title: check.name,
         author: null,
         body: check.output?.summary ?? "",

src/lib/pr-fix-queue.test.ts

@@ -274,6 +274,127 @@ describe("PR review-fix queue", () => {
   });
 });
 
+describe("item URL is identity, write-once (#1098)", () => {
+  let client: ReturnType<typeof makeClient>;
+
+  beforeEach(() => {
+    client = makeClient();
+    surfacingMocks.surfacePrFixBlocked.mockReset();
+    surfacingMocks.surfacePrFixBlocked.mockResolvedValue({ labelApplied: true, commentPosted: true, errors: [] });
+  });
+
+  it("keeps the PR URL when a CI-failure re-enqueue carries a job URL", async () => {
+    await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 7, lane: "NORMAL", type: "REVIEW_FEEDBACK",
+      reason: "PR review: CHANGES_REQUESTED", feedback: "changes requested",
+      evidenceKey: "rev-1",
+      url: "https://api.github.com/repos/o/repo/pulls/7",
+    });
+
+    const after = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 7, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "checks failed", feedback: "build failed",
+      evidenceKey: "cr-1",
+      url: "https://github.com/o/repo/actions/runs/9/job/1",
+    });
+
+    // The item URL is identity: it must stay the PR URL, never flip to the
+    // CI job URL, so next-task keeps handing out pullRequest.url.
+    expect(after.url).toBe("https://api.github.com/repos/o/repo/pulls/7");
+    expect(client.items[0].url).toBe("https://api.github.com/repos/o/repo/pulls/7");
+    // Evidence is still appended even when the URL is not.
+    expect(client.items[0].feedback).toEqual(["changes requested", "build failed"]);
+    expect(client.items[0].evidenceKeys).toEqual(["rev-1", "cr-1"]);
+  });
+
+  it("repairs a stored CI job URL on the next enqueue that carries the PR URL (#1118)", async () => {
+    await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 9, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f1", evidenceKey: "cr-1",
+    });
+    // A row poisoned by the pre-#1098 ingestion.
+    client.items[0].url = "https://github.com/o/repo/actions/runs/9/job/1";
+
+    const after = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 9, lane: "NORMAL", type: "REVIEW_FEEDBACK",
+      reason: "r", feedback: "f2", evidenceKey: "rev-1",
+      url: "https://api.github.com/repos/o/repo/pulls/9",
+    });
+
+    expect(after.url).toBe("https://api.github.com/repos/o/repo/pulls/9");
+  });
+
+  it("leaves a legitimate stored URL alone even when a different one arrives", async () => {
+    await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 10, lane: "NORMAL", type: "REVIEW_FEEDBACK",
+      reason: "r", feedback: "f1", evidenceKey: "rev-1",
+      url: "https://github.com/o/repo/pull/10",
+    });
+
+    const after = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 10, lane: "NORMAL", type: "REVIEW_FEEDBACK",
+      reason: "r", feedback: "f2", evidenceKey: "rev-2",
+      url: "https://api.github.com/repos/o/repo/pulls/10",
+    });
+
+    expect(after.url).toBe("https://github.com/o/repo/pull/10");
+  });
+
+  it("never stores a CI job URL as the item URL, on create or on backfill (#1118)", async () => {
+    const created = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 11, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f1", evidenceKey: "cr-1",
+      url: "https://github.com/o/repo/actions/runs/9/job/1",
+    });
+    expect(created.url).toBeUndefined();
+
+    const reenqueued = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 11, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f2", evidenceKey: "cr-2",
+      url: "https://github.com/o/repo/actions/runs/10/job/2",
+    });
+    expect(reenqueued.url).toBeUndefined();
+  });
+
+  it("backfills the URL only when the item has none", async () => {
+    await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 8, lane: "NORMAL", type: "REVIEW_FEEDBACK",
+      reason: "r", feedback: "f1", evidenceKey: "rev-1",
+    });
+    expect(client.items[0].url).toBeUndefined();
+
+    const after = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 8, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f2", evidenceKey: "cr-1",
+      url: "https://api.github.com/repos/o/repo/pulls/8",
+    });
+
+    expect(after.url).toBe("https://api.github.com/repos/o/repo/pulls/8");
+    expect(client.items[0].url).toBe("https://api.github.com/repos/o/repo/pulls/8");
+  });
+
+  it("backfills the URL when the stored value is the empty string (String? column shape)", async () => {
+    await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 8, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f1", evidenceKey: "cr-1",
+      url: "",
+    });
+    // The webhook enqueues with url: "" when the PR object has no url (#1098);
+    // a String? column can carry the empty string, which the write-once guard
+    // must treat as empty.
+    client.items[0].url = "";
+
+    const after = await enqueuePrFixItem(client, {
+      repo: "o/repo", pr: 8, lane: "NORMAL", type: "CI_FAILURE",
+      reason: "r", feedback: "f2", evidenceKey: "cr-2",
+      url: "https://api.github.com/repos/o/repo/pulls/8",
+    });
+
+    expect(after.url).toBe("https://api.github.com/repos/o/repo/pulls/8");
+    expect(client.items[0].url).toBe("https://api.github.com/repos/o/repo/pulls/8");
+  });
+});
+
 describe("work generation identity (#1044)", () => {
   let client: ReturnType<typeof makeClient>;
 

src/lib/pr-fix-queue.ts

@@ -32,6 +32,7 @@ export interface EnqueuePrFixInput {
   evidenceKey: string;
   issue?: number | null;
   branch?: string | null;
+  /** Item identity — should be the PR URL. Set at first enqueue, only backfilled while empty (#1098). */
   url?: string | null;
   title?: string | null;
   headSha?: string | null;
@@ -206,12 +207,30 @@ function extractUrlsFromTextSafe(text: string): string[] {
   }
 }
 
+/**
+ * A GitHub Actions run/job URL: what the pre-#1098 ingestion stored as an
+ * item URL for CI failures. Never valid item identity (#1118).
+ */
+const ACTIONS_RUN_URL = /^https:\/\/github\.com\/[^/]+\/[^/]+\/actions\/runs\//;
+
+function isActionsRunUrl(url: string | null | undefined): boolean {
+  return typeof url === "string" && ACTIONS_RUN_URL.test(url);
+}
+
+/** The item URL an enqueue may write: never a CI job URL (#1098, #1118). */
+function itemUrlFromInput(input: EnqueuePrFixInput): string | undefined {
+  return input.url && !isActionsRunUrl(input.url) ? input.url : undefined;
+}
+
+// `url` is not part of this patch (#1098): the item URL is identity (the PR
+// URL) — set at first enqueue, backfilled only while empty. A CI-failure
+// re-enqueue's job URL must not flip it; job links belong in feedback/
+// evidence (pr-followup-ingestion), not the item URL.
 function metadataPatch(input: EnqueuePrFixInput): Record<string, string | number> {
   const patch: Record<string, string | number> = {};
   for (const [key, value] of Object.entries({
     issue: input.issue ?? undefined,
     branch: input.branch ?? undefined,
-    url: input.url ?? undefined,
     title: input.title ?? undefined,
     headSha: input.headSha ?? undefined,
     author: input.author ?? undefined,
@@ -324,6 +343,14 @@ export async function enqueuePrFixItem(client: PrFixQueueClient, input: EnqueueP
           reason: input.reason,
           feedback: uniqueAppend(existing.feedback ?? [], input.feedback, 12),
           evidenceKeys: nextEvidenceKeys,
+          // #1098: the item URL is identity — write-once. Backfill it only
+          // while empty; never overwrite an existing URL with a
+          // re-enqueue's value (e.g. a CI job URL). A stored CI job URL left
+          // by the pre-#1098 ingestion counts as empty, so it heals on the
+          // next enqueue instead of being frozen by the guard (#1118).
+          ...((!existing.url || isActionsRunUrl(existing.url)) && itemUrlFromInput(input)
+            ? { url: itemUrlFromInput(input) }
+            : {}),
           // A fresh attempt gets a fresh per-attempt head baseline (#1074):
           // the head the sync observed in THIS enqueue, else the last one
           // observed (#1104). metadataPatch below keeps refreshing the mutable
@@ -364,6 +391,9 @@ export async function enqueuePrFixItem(client: PrFixQueueClient, input: EnqueueP
         reason: input.reason,
         feedback: [input.feedback],
         evidenceKeys: [input.evidenceKey],
+        // #1098: the item URL is identity (the PR URL) — set it at first
+        // enqueue, explicitly, so it is never left to the update patch.
+        ...(itemUrlFromInput(input) ? { url: itemUrlFromInput(input) } : {}),
         // A brand-new item is a fresh attempt: capture the head the sync
         // observed now as its immutable per-attempt baseline (#1074).
         attemptHeadSha: input.headSha ?? null,

src/lib/pr-followup-ingestion.test.ts

@@ -1011,6 +1011,63 @@ describe("processPrFollowupEvents", () => {
     error.mockRestore();
   });
 
+  it("enqueues a check_run with the PR URL as the item URL and the job URL in feedback (#1098)", async () => {
+    process.env.PR_FOLLOWUP_BOT_IDENTITIES = "itsmiso-ai";
+    const client = makeClient();
+
+    const result = await processPrFollowupEvents(client, [
+      {
+        eventType: "check_run",
+        repoFullName: "org/repo",
+        prNumber: 7,
+        branch: "fix/c",
+        url: "https://api.github.com/repos/org/repo/pulls/7",
+        checkRunUrl: "https://github.com/org/repo/actions/runs/9/job/1",
+        title: "Fix C",
+        author: "itsmiso-ai",
+        body: "Error: Cannot read property 'x' of undefined",
+        id: "cr1098",
+        conclusion: "failure",
+        checkName: "lint",
+      },
+    ]);
+
+    expect(result.enqueued).toBe(1);
+    expect(client.items).toHaveLength(1);
+    // The item URL is the PR URL, not the CI job URL.
+    expect(client.items[0].url).toBe("https://api.github.com/repos/org/repo/pulls/7");
+    // The job URL is evidence in the feedback text ("Full log:" line).
+    const feedback = String(client.items[0].feedback);
+    expect(feedback).toContain("Full log: https://github.com/org/repo/actions/runs/9/job/1");
+    expect(feedback).not.toContain("pulls/7");
+  });
+
+  it("keeps the job URL in feedback when checkRunUrl is absent (legacy shape)", async () => {
+    process.env.PR_FOLLOWUP_BOT_IDENTITIES = "itsmiso-ai";
+    const client = makeClient();
+
+    const result = await processPrFollowupEvents(client, [
+      {
+        eventType: "check_run",
+        repoFullName: "org/repo",
+        prNumber: 7,
+        branch: "fix/c",
+        url: "https://github.com/org/repo/actions/runs/9/job/1",
+        title: "Fix C",
+        author: "itsmiso-ai",
+        body: "Error: Cannot read property 'x' of undefined",
+        id: "cr1098-legacy",
+        conclusion: "failure",
+        checkName: "lint",
+      },
+    ]);
+
+    expect(result.enqueued).toBe(1);
+    expect(client.items).toHaveLength(1);
+    const feedback = String(client.items[0].feedback);
+    expect(feedback).toContain("Full log: https://github.com/org/repo/actions/runs/9/job/1");
+  });
+
   afterEach(() => {
     delete process.env.PR_FOLLOWUP_BOT_IDENTITIES;
   });