Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<LinkedIssueClosureByPullRequestResult> {
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
Expand Down
52 changes: 39 additions & 13 deletions src/review/linked-issue-label-propagation-fetch.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down
73 changes: 73 additions & 0 deletions test/unit/linked-issue-label-propagation-fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
Expand All @@ -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" });
Expand Down Expand Up @@ -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({});
Expand Down
6 changes: 6 additions & 0 deletions test/unit/queue-5.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]));
Expand Down