diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 5d3293a740..a7bf74d2f0 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -3230,6 +3230,33 @@ export async function fetchLivePullRequestMergedAt( return result === undefined ? undefined : (result.data.merged_at ?? null); } +export type LinkedIssueClosureByPullRequestResult = "closed_by_pull_request" | "not_closed_by_pull_request" | "fetch_error"; + +function timelineEventClosesIssueFromPullRequest( + event: { event?: string | null; source?: { issue?: { number?: number | null; pull_request?: unknown } | null } | null }, + prNumber: number, +): boolean { + return event.event === "closed" && event.source?.issue?.number === prNumber && event.source.issue.pull_request !== undefined; +} + +/** Verifies whether GitHub's issue timeline attributes this issue close to the specific PR. Timestamp ordering + * alone only proves the issue closed after the PR merged; the timeline's closing-reference source binds the + * closure to THIS PR and prevents borrowing labels from an unrelated issue that happened to close later. */ +export async function fetchLinkedIssueClosedByPullRequest( + env: Env, + repoFullName: string, + issueNumber: number, + prNumber: number, + token: string | undefined, + admissionKey?: GitHubRateLimitAdmissionKey, +): Promise { + const result = await githubJsonWithHeaders< + Array<{ event?: string | null; source?: { issue?: { number?: number | null; pull_request?: unknown } | null } | null }> + >(env, repoFullName, `/issues/${issueNumber}/timeline?per_page=100`, token, githubRateLimitOptions(admissionKey)).catch(() => undefined); + if (result === undefined) return "fetch_error"; + return result.data.some((event) => timelineEventClosesIssueFromPullRequest(event, prNumber)) ? "closed_by_pull_request" : "not_closed_by_pull_request"; +} + /** The issue's LIVE state ("open" / "closed") via REST `GET /issues/{n}`. Mirrors {@link fetchLivePullRequestState} * for issues: the stored open-issue cache lags GitHub, so a sibling closed on GitHub (or elsewhere) can still * read `open` locally. The per-contributor open-issue cap (#2479 gate finding) confirms each counted sibling's diff --git a/src/review/linked-issue-label-propagation-fetch.ts b/src/review/linked-issue-label-propagation-fetch.ts index f88c00e282..d6b21bb58e 100644 --- a/src/review/linked-issue-label-propagation-fetch.ts +++ b/src/review/linked-issue-label-propagation-fetch.ts @@ -1,4 +1,10 @@ -import { fetchLinkedIssueFacts, fetchLivePullRequestMergedAt, type LinkedIssueFactsFetch, type LinkedIssueFactsResult } from "../github/backfill"; +import { + fetchLinkedIssueClosedByPullRequest, + fetchLinkedIssueFacts, + fetchLivePullRequestMergedAt, + type LinkedIssueFactsFetch, + type LinkedIssueFactsResult, +} from "../github/backfill"; import { createInstallationToken, getRepositoryCollaboratorPermission } from "../github/app"; import { githubRateLimitAdmissionKeyForToken, type GitHubRateLimitAdmissionKey } from "../github/client"; import { parseGitHubLoginList } from "../auth/security"; @@ -68,17 +74,13 @@ async function isRepoMaintainerLogin(env: Env, installationId: number, repoFullN return permission != null && new Set(["admin", "maintain", "write"]).has(permission) ? "maintainer" : "not_maintainer"; } -/** True when the linked issue's authority for propagation can be trusted (#4528): it's still OPEN, or it - * was closed no earlier than THIS PR's own merge. Merging a PR whose body says "Closes #N" auto-closes - * issue #N as an immediate side effect of that same merge -- so `closedAt >= prMergedAt` is exactly the - * signature of "this merge is what closed it," the single most authoritative moment for propagation to - * fire, not a weaker one. An issue closed BEFORE this PR ever merged (`closedAt < prMergedAt`) is the - * gaming case the OPEN-only check originally existed to block -- a PR opportunistically referencing some - * unrelated, already-resolved issue to borrow its label -- and stays blocked, unchanged. `prMergedAt` - * absent (PR not yet merged) never trusts a closed issue, also unchanged. */ -function isLinkedIssueTrustworthy(facts: LinkedIssueFactsResult, prMergedAt: string | null): boolean { +function linkedIssueNeedsClosureVerification(facts: LinkedIssueFactsResult, prMergedAt: string | null): boolean { + return facts.state !== "open" && prMergedAt !== null && facts.closedAt !== null && facts.closedAt >= prMergedAt; +} + +function isLinkedIssueTrustworthy(facts: LinkedIssueFactsResult, prMergedAt: string | null, closedByThisPr: boolean): boolean { if (facts.state === "open") return true; - return prMergedAt !== null && facts.closedAt !== null && facts.closedAt >= prMergedAt; + return linkedIssueNeedsClosureVerification(facts, prMergedAt) && closedByThisPr; } /** {@link resolveIssueLabelsForPropagation}'s and {@link fetchLinkedIssueLabelsForPropagation}'s return shape @@ -172,7 +174,31 @@ async function resolveIssueLabelsForPropagation( } trustedMergedAt = liveMergedAt; } - if (!isLinkedIssueTrustworthy(result.facts, trustedMergedAt)) return { labels: [], inconclusive: false }; + let closedByThisPr = false; + if (linkedIssueNeedsClosureVerification(result.facts, trustedMergedAt)) { + if (args.prNumber === undefined) return { labels: [], inconclusive: false }; + const closure = await fetchLinkedIssueClosedByPullRequest( + args.env, + args.repoFullName, + result.facts.number, + args.prNumber, + args.token, + args.admissionKey, + ); + if (closure === "fetch_error") { + console.log( + JSON.stringify({ + event: "linked_issue_label_propagation_inconclusive", + repoFullName: args.repoFullName, + issueNumber: result.facts.number, + reason: "issue_closure_timeline_check_failed", + }), + ); + return { labels: [], inconclusive: true }; + } + closedByThisPr = closure === "closed_by_pull_request"; + } + if (!isLinkedIssueTrustworthy(result.facts, trustedMergedAt, closedByThisPr)) return { labels: [], inconclusive: false }; const allLabels = result.facts.labels; const issueAuthorLogin = result.facts.authorLogin?.toLowerCase(); const assignees = result.facts.assignees.map((login) => login.toLowerCase()); @@ -212,7 +238,7 @@ async function resolveIssueLabelsForPropagation( /** FETCH every linked issue's labels (fail-open) and flatten into one label list for * `resolvePrTypeLabel` (`src/settings/pr-type-label.ts`) to match against. Only an OPEN issue, or one - * closed no earlier than THIS PR's own merge (#4528, {@link isLinkedIssueTrustworthy}), can contribute + * closed by THIS PR as verified from GitHub's timeline (#4528, {@link isLinkedIssueTrustworthy}), can contribute * labels; closing-keyword text in a PR body is author-controlled and is not authority by itself. Mirrors * `resolveLinkedIssueHardRule`'s own fetch idiom (`src/review/linked-issue-hard-rules.ts`): a per-issue * fetch failure contributes no labels rather than throwing, so if EVERY linked issue fails, `labels` is diff --git a/test/unit/linked-issue-label-propagation-fetch.test.ts b/test/unit/linked-issue-label-propagation-fetch.test.ts index eb2b0f6044..50b4ed503f 100644 --- a/test/unit/linked-issue-label-propagation-fetch.test.ts +++ b/test/unit/linked-issue-label-propagation-fetch.test.ts @@ -300,6 +300,7 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( user: { login: "contrib" }, labels: ["gittensor:feature", "gittensor:priority"], }); + if (url.includes("/issues/4279/timeline")) return Response.json([{ event: "closed", source: { issue: { number: 4494, pull_request: {} } } }]); return new Response("not found", { status: 404 }); }); const env = createTestEnv({}); @@ -310,10 +311,81 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( installationId: 123, prAuthorLogin: "contrib", prMergedAt: "2026-07-09T22:15:13Z", + prNumber: 4494, }); expectPropagation(result, ["gittensor:feature", "gittensor:priority"]); }); + it("REGRESSION (#closed-issue-timestamp-spoof): does NOT propagate when an unrelated issue closed after this PR merged", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/9001")) + return Response.json({ + number: 9001, + state: "closed", + closed_at: "2026-07-09T22:15:14Z", + user: { login: "contrib" }, + labels: ["gittensor:feature", "gittensor:priority"], + }); + if (url.includes("/issues/9001/timeline")) return Response.json([{ event: "closed", source: { issue: { number: 123, pull_request: {} } } }]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [9001], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + prNumber: 4494, + }); + expectPropagation(result, []); + }); + + it("does not propagate a timestamp-eligible closed issue when the caller cannot identify this PR number", async () => { + const fetchSpy = vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/9003")) + return Response.json({ number: 9003, state: "closed", closed_at: "2026-07-09T22:15:14Z", user: { login: "contrib" }, labels: ["gittensor:priority"] }); + return new Response("not found", { status: 404 }); + }); + vi.stubGlobal("fetch", fetchSpy); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [9003], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + }); + expectPropagation(result, []); + expect(fetchSpy.mock.calls.some(([input]) => input.toString().includes("/timeline"))).toBe(false); + }); + + it("flags closed issue propagation inconclusive when the timeline closure check fails", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/9002")) + return Response.json({ number: 9002, state: "closed", closed_at: "2026-07-09T22:15:14Z", user: { login: "contrib" }, labels: ["gittensor:priority"] }); + if (url.includes("/issues/9002/timeline")) return new Response("server error", { status: 500 }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [9002], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + prNumber: 4494, + }); + expectPropagation(result, [], true); + }); + it("does NOT propagate when the linked issue was already closed BEFORE this PR merged (anti-gaming: an unrelated, already-resolved issue can't be borrowed)", async () => { stubFetch((url) => { if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); @@ -372,6 +444,7 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( labels: ["gittensor:feature"], }); if (url.endsWith("/pulls/4818")) return Response.json({ merged_at: "2026-07-11T02:26:24Z" }); + if (url.includes("/issues/2192/timeline")) return Response.json([{ event: "closed", source: { issue: { number: 4818, pull_request: {} } } }]); return new Response("not found", { status: 404 }); }); const env = createTestEnv({}); diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index f465cbaf0b..cc2eff6834 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -6222,6 +6222,12 @@ describe("queue processors", () => { seen.issueFetches += 1; return linkedIssueResponse(); } + // #4528 timeline attribution (#closed-issue-timestamp-spoof): every existing caller here exercises the + // legitimate "this PR's own merge closed the linked issue" trust path, never the spoofing case (which + // gets its own dedicated stub) -- so the shared helper can safely attribute every closure to this PR. + if (url.includes(`/issues/${linkedIssueNumber}/timeline`)) { + return Response.json([{ event: "closed", source: { issue: { number: prNumber, pull_request: {} } } }]); + } if (url.includes(`/issues/${prNumber}/labels`) && method === "GET") return Response.json([]); if (url.includes(`/issues/${prNumber}/labels`) && method === "POST") { seen.posted.push(...((JSON.parse(String(init?.body ?? "{}")).labels ?? []) as string[]));