Merge branch 'main' into claude/reopen-reclose-audit-fidelity

Author: JSONbored

src/github/backfill.ts

@@ -2885,40 +2885,68 @@ export function isOwnReviewThreadAuthor(login: string | null | undefined): boole
 /** The deterministic linked-issue facts the hard-rule evaluator needs (labels / assignees / open-state). */
 export type LinkedIssueFactsResult = { number: number; labels: string[]; assignees: string[]; state: string; authorLogin: string | null };
 
+/** Tri-state outcome of fetching one linked issue's facts (#2136). `not_found` is a CONFIRMED 404 seen with a
+ *  genuine, repo-scoped token — GitHub told an authenticated caller this issue number does not exist. `fetch_error`
+ *  is everything else that prevented a read (network, 5xx, rate-limit, malformed body, or a 404 seen with only
+ *  the public/anonymous token, which GitHub also returns for a real-but-inaccessible private issue) — a genuine
+ *  outage or an unproven access gap, not confirmed evidence about the issue itself. Callers that treat an
+ *  ALL-not_found result as significant (the linked-issue hard rule) must never extend that same treatment to
+ *  fetch_error, or a GitHub outage would spuriously look like a fabricated reference. */
+export type LinkedIssueFactsFetch =
+  | { status: "found"; facts: LinkedIssueFactsResult }
+  | { status: "not_found" }
+  | { status: "fetch_error" };
+
 /**
- * FETCH the facts for one linked issue via the REST issues endpoint. FAIL-OPEN: any fetch/parse error returns
- * undefined so the caller skips that issue — a deterministic auto-close must NEVER fire (or be blocked) on a
- * transient fetch failure. Uses the same authenticated REST client + public-token 404-fallback as the other
- * live fetches. (Note: GitHub's issues endpoint also returns pull requests, which carry a `pull_request` field;
- * a PR number passed here would simply fail the rules — we only treat real issues' labels/assignees.)
+ * FETCH the facts for one linked issue via the REST issues endpoint. Distinguishes a CONFIRMED-nonexistent
+ * issue (404) from a transient fetch failure (#2136) — a deterministic auto-close must never fire on a
+ * transient failure, but a fabricated issue number is real, verifiable information the hard-rule evaluator
+ * needs. Uses the same authenticated REST client + public-token 404-fallback as the other live fetches. (Note:
+ * GitHub's issues endpoint also returns pull requests, which carry a `pull_request` field; a PR number passed
+ * here would simply fail the rules — we only treat real issues' labels/assignees.)
+ *
+ * GitHub returns 404 for BOTH a genuinely nonexistent issue and a real-but-inaccessible one (private repo, no
+ * grant) — it deliberately doesn't distinguish the two, to avoid leaking a private repo's existence to a caller
+ * without access. So a 404 is only trustworthy as CONFIRMED absence when `token` is a genuine, repo-scoped
+ * credential; the public/anonymous fallback token proves nothing about access. Without that, treat the 404 as
+ * `fetch_error` (fails open) rather than risk closing a PR over a real linked issue our token just can't see.
  */
 export async function fetchLinkedIssueFacts(
   env: Env,
   repoFullName: string,
   issueNumber: number,
   token: string | undefined,
   admissionKey?: GitHubRateLimitAdmissionKey,
-): Promise<LinkedIssueFactsResult | undefined> {
-  const result = await githubJsonWithHeaders<{
-    number?: number;
-    state?: string | null;
-    labels?: Array<{ name?: string | null } | string | null> | null;
-    assignees?: Array<{ login?: string | null } | null> | null;
-    user?: { login?: string | null } | null;
-  }>(env, repoFullName, `/issues/${issueNumber}`, token, githubRateLimitOptions(admissionKey)).catch(() => undefined);
-  if (!result) return undefined;
+): Promise<LinkedIssueFactsFetch> {
+  let result;
+  try {
+    result = await githubJsonWithHeaders<{
+      number?: number;
+      state?: string | null;
+      labels?: Array<{ name?: string | null } | string | null> | null;
+      assignees?: Array<{ login?: string | null } | null> | null;
+      user?: { login?: string | null } | null;
+    }>(env, repoFullName, `/issues/${issueNumber}`, token, githubRateLimitOptions(admissionKey));
+  } catch (error) {
+    if (!(error instanceof GitHubApiError) || error.statusCode !== 404) return { status: "fetch_error" };
+    const hasProvenAccess = Boolean(token) && token !== env.GITHUB_PUBLIC_TOKEN;
+    return { status: hasProvenAccess ? "not_found" : "fetch_error" };
+  }
   const data = result.data;
   const labels = (data.labels ?? []).flatMap((label) => {
     if (typeof label === "string") return label.length > 0 ? [label] : [];
     return label?.name ? [label.name] : [];
   });
   const assignees = (data.assignees ?? []).flatMap((assignee) => (assignee?.login ? [assignee.login] : []));
   return {
-    number: data.number ?? issueNumber,
-    labels,
-    assignees,
-    state: String(data.state ?? "open").toLowerCase(),
-    authorLogin: data.user?.login ?? null,
+    status: "found",
+    facts: {
+      number: data.number ?? issueNumber,
+      labels,
+      assignees,
+      state: String(data.state ?? "open").toLowerCase(),
+      authorLogin: data.user?.login ?? null,
+    },
   };
 }
 

src/queue/processors.ts

@@ -4334,8 +4334,7 @@ export async function resolveLinkedIssueAuthorLogins(
       login != null
         ? Promise.resolve(login)
         : fetchLinkedIssueFacts(env, repoFullName, linkedIssues[index]!, token, admissionKey)
-            .then((facts) => facts?.authorLogin ?? null)
-            .catch(() => null),
+            .then((result) => (result.status === "found" ? result.facts.authorLogin : null)),
     ),
   );
 }

src/review/linked-issue-hard-rules.ts

@@ -173,7 +173,21 @@ export async function resolveLinkedIssueHardRule(args: {
   if (args.linkedIssues.length === 0) return undefined;
   const token = args.ciToken ?? args.env.GITHUB_PUBLIC_TOKEN;
   const admissionKey = githubRateLimitAdmissionKeyForToken(args.env, token, args.installationId);
-  const issueFacts = (await Promise.all(args.linkedIssues.map((issueNumber) => fetchLinkedIssueFacts(args.env, args.repoFullName, issueNumber, token, admissionKey)))).flatMap((facts) => (facts ? [facts] : []));
-  if (issueFacts.length === 0) return undefined;
+  const fetchResults = await Promise.all(args.linkedIssues.map((issueNumber) => fetchLinkedIssueFacts(args.env, args.repoFullName, issueNumber, token, admissionKey)));
+  const issueFacts = fetchResults.flatMap((result) => (result.status === "found" ? [result.facts] : []));
+  if (issueFacts.length === 0) {
+    // Every reference resolved to a CONFIRMED 404 — never a transient fetch_error (#2136). Mirrors the overflow
+    // treatment above: a contributor citing a fabricated issue number must not silently satisfy the hard rule
+    // the same way a genuinely-linked-but-unfetchable issue fails open. A single fetch_error in the mix still
+    // fails open (we cannot rule out a real, rule-violating issue behind that failure).
+    const allConfirmedNotFound = fetchResults.every((result) => result.status === "not_found");
+    if (allConfirmedNotFound) {
+      return {
+        violated: true,
+        reason: "The linked issue reference could not be found — please link a real, open issue or request maintainer review.",
+      };
+    }
+    return undefined;
+  }
   return evaluateLinkedIssueHardRules({ issues: issueFacts, config: args.config, repoOwner: args.repoOwner });
 }

test/unit/backfill.test.ts

@@ -34,6 +34,7 @@ import {
   enqueueRepositoryOpenDataBackfill,
   enrichInstallationHealth,
   fetchAndStorePullRequestFilesForReview,
+  fetchLinkedIssueFacts,
   fetchLiveCiAggregate,
   fetchLiveReviewThreadBlockers,
   fetchRequiredStatusContexts,
@@ -5007,6 +5008,47 @@ describe("GitHub backfill", () => {
     });
   });
 
+  describe("fetchLinkedIssueFacts (#2136)", () => {
+    it("returns a found result with the extracted facts, falling back to the requested number and open state when the payload omits them", async () => {
+      const env = createTestEnv({});
+      // Sparse payload: no `number`, no `state` — exercises the `data.number ?? issueNumber` and
+      // `data.state ?? "open"` defensive fallbacks.
+      vi.stubGlobal("fetch", async () => Response.json({ labels: [{ name: "bug" }, "manual-string-label"], assignees: [{ login: "maintainer" }], user: { login: "reporter" } }));
+      const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 42, "tok");
+      expect(result).toEqual({
+        status: "found",
+        facts: { number: 42, labels: ["bug", "manual-string-label"], assignees: ["maintainer"], state: "open", authorLogin: "reporter" },
+      });
+    });
+
+    it("returns not_found on a confirmed 404, distinct from a transient fetch error", async () => {
+      const env = createTestEnv({});
+      vi.stubGlobal("fetch", async () => new Response("missing", { status: 404 }));
+      expect(await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 999999, "tok")).toEqual({ status: "not_found" });
+    });
+
+    it("REGRESSION: treats a 404 seen with the public/anonymous token as fetch_error, not not_found — GitHub also returns 404 for a real but inaccessible private issue", async () => {
+      const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-tok" });
+      vi.stubGlobal("fetch", async () => new Response("missing", { status: 404 }));
+      // The public token proves nothing about repo access, so a 404 here could just as easily mean "this issue
+      // is real but private and this token can't see it" -- treating it as CONFIRMED absence risks closing a PR
+      // over a genuinely-linked issue.
+      expect(await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 42, env.GITHUB_PUBLIC_TOKEN)).toEqual({ status: "fetch_error" });
+    });
+
+    it("REGRESSION: treats a 404 seen with no token at all as fetch_error, not not_found", async () => {
+      const env = createTestEnv({});
+      vi.stubGlobal("fetch", async () => new Response("missing", { status: 404 }));
+      expect(await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 42, undefined)).toEqual({ status: "fetch_error" });
+    });
+
+    it("returns fetch_error on a transient failure (5xx), never conflating it with not_found", async () => {
+      const env = createTestEnv({});
+      vi.stubGlobal("fetch", async () => new Response("server error", { status: 500 }));
+      expect(await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 42, "tok")).toEqual({ status: "fetch_error" });
+    });
+  });
+
   describe("isRateLimitedGitHubFailure", () => {
     it("does not treat a bare permission 403 (remaining > 0, no Retry-After, no secondary body) as a rate limit", () => {
       expect(

test/unit/linked-issue-hard-rules.test.ts

@@ -288,9 +288,36 @@ describe("resolveLinkedIssueHardRule (#1144 — overflow + orchestration)", () =
     expect(await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), body: null, linkedIssues: [] }))).toBeUndefined();
   });
 
-  it("is fail-open: undefined when every fetch fails (404), with no CI token → public-token fallback", async () => {
+  it("treats a confirmed-nonexistent linked issue as a violation, not a silent pass (#2136)", async () => {
+    // Every reference 404s with a GENUINE installation token (proven repo access) — CONFIRMED not-found, not a
+    // transient error — a contributor citing a fabricated issue number must not silently satisfy the hard rule
+    // the same way a genuine fetch outage fails open.
     vi.stubGlobal("fetch", async () => new Response("missing", { status: 404 }));
-    expect(await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), ciToken: undefined, linkedIssues: [1, 2] }))).toBeUndefined();
+    const r = await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), ciToken: "installation-token", linkedIssues: [1, 2] }));
+    expect(r?.violated).toBe(true);
+    expect(r?.reason).toMatch(/could not be found/i);
+  });
+
+  it("REGRESSION: does NOT violate when every reference 404s but ciToken is unavailable (falls back to the public token) — a 404 without proven repo access is not confirmed absence", async () => {
+    // GitHub also returns 404 for a real-but-inaccessible private issue, not just a genuinely nonexistent one.
+    // Without a genuine ciToken, this call falls back to env.GITHUB_PUBLIC_TOKEN, which proves nothing about
+    // repo access — closing the PR here would risk punishing a contributor for a real linked issue our token
+    // just can't see.
+    vi.stubGlobal("fetch", async () => new Response("missing", { status: 404 }));
+    const r = await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), ciToken: undefined, linkedIssues: [1, 2] }));
+    expect(r).toBeUndefined();
+  });
+
+  it("still fails open (undefined) when a linked-issue fetch fails transiently (5xx), not confirmed-nonexistent", async () => {
+    vi.stubGlobal("fetch", async () => new Response("server error", { status: 500 }));
+    expect(await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), ciToken: "tok", linkedIssues: [1, 2] }))).toBeUndefined();
+  });
+
+  it("fails open when the linked issues are a MIX of confirmed-not-found and a transient fetch error", async () => {
+    // Cannot rule out a real, rule-violating issue behind the transient failure — must not treat this the same
+    // as an all-confirmed-not-found set.
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL) => (input.toString().endsWith("/issues/1") ? new Response("missing", { status: 404 }) : new Response("server error", { status: 500 })));
+    expect(await resolveLinkedIssueHardRule(args({ config: config({ ownerAssignedClose: "block" }), ciToken: "tok", linkedIssues: [1, 2] }))).toBeUndefined();
   });
 
   it("fetches with the CI token and runs the deterministic evaluator over the facts", async () => {
@@ -305,7 +332,7 @@ describe("resolveLinkedIssueHardRule (#1144 — overflow + orchestration)", () =
   });
 
   it("derives the installation admission key from the ci token + installation id so installation reads attribute to the installation bucket, not 'unknown' (#1951 blocker)", async () => {
-    const spy = vi.spyOn(backfillModule, "fetchLinkedIssueFacts").mockResolvedValue(undefined);
+    const spy = vi.spyOn(backfillModule, "fetchLinkedIssueFacts").mockResolvedValue({ status: "fetch_error" });
     await resolveLinkedIssueHardRule(
       args({ config: config({ ownerAssignedClose: "block" }), ciToken: "installation-token", installationId: 143010787, linkedIssues: [7] }),
     );