From 4a390b0b8b1f26dec811332a5107e94f46bdf426 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:11:25 -0700 Subject: [PATCH] fix(review): trust a linked issue closed by this PR's own merge for label propagation (#4528) Merging a PR with "Closes #N" auto-closes issue #N as an immediate side effect of the merge, which defeated the propagation lookup's open-issue-only check seconds later and stripped the just-applied gittensor:feature/priority labels back down to a title-guessed gittensor:bug. Extends the trust condition to accept a closed issue when it was closed at or after this PR's own merge, while still rejecting an issue closed before the PR merged (the anti-gaming case the open-only check originally existed to block). --- src/github/backfill.ts | 6 ++ src/queue/processors.ts | 4 ++ .../linked-issue-label-propagation-fetch.ts | 32 +++++++-- test/unit/backfill.test.ts | 19 ++++- test/unit/linked-issue-hard-rules.test.ts | 2 +- ...nked-issue-label-propagation-fetch.test.ts | 71 +++++++++++++++++++ test/unit/queue.test.ts | 71 +++++++++++++++++++ 7 files changed, 197 insertions(+), 8 deletions(-) diff --git a/src/github/backfill.ts b/src/github/backfill.ts index aaa3333993..7ad27d7d62 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -3839,6 +3839,10 @@ export type LinkedIssueFactsResult = { authorLogin: string | null; title?: string | null; body?: string | null; + /** GitHub's `closed_at` for this issue, or `null` while open (#4528: label-propagation callers use this + * to trust an issue closed by THIS PR's own merge, without granting authority to one closed earlier + * for an unrelated reason). Same REST payload as every other field here -- no extra call. */ + closedAt: string | null; }; /** Tri-state outcome of fetching one linked issue's facts (#2136). `not_found` is a CONFIRMED 404 seen with a @@ -3884,6 +3888,7 @@ export async function fetchLinkedIssueFacts( user?: { login?: string | null } | null; title?: string | null; body?: string | null; + closed_at?: string | null; }>(env, repoFullName, `/issues/${issueNumber}`, token, githubRateLimitOptions(admissionKey)); } catch (error) { if (!(error instanceof GitHubApiError) || error.statusCode !== 404) return { status: "fetch_error" }; @@ -3906,6 +3911,7 @@ export async function fetchLinkedIssueFacts( authorLogin: data.user?.login ?? null, title: typeof data.title === "string" && data.title.length > 0 ? data.title : null, body: typeof data.body === "string" && data.body.length > 0 ? data.body : null, + closedAt: typeof data.closed_at === "string" ? data.closed_at : null, }, }; } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ba6a0bedd6..d2ee69c232 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -8688,6 +8688,10 @@ async function maybePublishPrPublicSurface( installationId, prAuthorLogin: pr.authorLogin, mappings: propagation.mappings, + // #4528: lets a closed linked issue still count when THIS PR's own merge is what closed it + // (the standard "Closes #N" auto-close), instead of losing propagation authority the instant + // the merge that's supposed to earn the label also closes its evidence. + prMergedAt: pr.mergedAt ?? null, }) : []; const decisionResult = resolvePrTypeLabel({ diff --git a/src/review/linked-issue-label-propagation-fetch.ts b/src/review/linked-issue-label-propagation-fetch.ts index 1c17dbc90d..1db7e1864b 100644 --- a/src/review/linked-issue-label-propagation-fetch.ts +++ b/src/review/linked-issue-label-propagation-fetch.ts @@ -1,4 +1,4 @@ -import { fetchLinkedIssueFacts, type LinkedIssueFactsFetch } from "../github/backfill"; +import { fetchLinkedIssueFacts, type LinkedIssueFactsFetch, type LinkedIssueFactsResult } from "../github/backfill"; import { createInstallationToken, getRepositoryCollaboratorPermission } from "../github/app"; import { githubRateLimitAdmissionKeyForToken } from "../github/client"; import { parseGitHubLoginList } from "../auth/security"; @@ -40,6 +40,19 @@ async function isRepoMaintainerLogin(env: Env, installationId: number, repoFullN return permission != null && new Set(["admin", "maintain", "write"]).has(permission); } +/** 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 { + if (facts.state === "open") return true; + return prMergedAt !== null && facts.closedAt !== null && facts.closedAt >= prMergedAt; +} + /** Per-issue label resolution for {@link fetchLinkedIssueLabelsForPropagation}: a direct PR-author-is- * issue-author-or-assignee match unlocks EVERY label the issue carries (today's original behavior, * unchanged). Failing that, a mapping explicitly opted into `trustMaintainerAuthoredIssue` OR @@ -61,8 +74,9 @@ async function resolveIssueLabelsForPropagation( result: LinkedIssueFactsFetch, prAuthorLogin: string | undefined, relaxableLabels: ReadonlySet, + prMergedAt: string | null, ): Promise { - if (result.status !== "found" || result.facts.state !== "open" || !prAuthorLogin) return []; + if (result.status !== "found" || !isLinkedIssueTrustworthy(result.facts, prMergedAt) || !prAuthorLogin) return []; const allLabels = result.facts.labels; const issueAuthorLogin = result.facts.authorLogin?.toLowerCase(); const assignees = result.facts.assignees.map((login) => login.toLowerCase()); @@ -89,9 +103,9 @@ 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 verified OPEN issues - * can contribute labels; closing-keyword text in a PR body is author-controlled and is not authority by - * itself. Mirrors + * `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 + * 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, the result is * `[]` — which can never match a mapping, meaning a sensitive label like `gittensor:priority` never applies @@ -108,7 +122,10 @@ async function resolveIssueLabelsForPropagation( * `mappings` (optional, #priority-linked-issue-gate-ownership) is the propagation config's own mapping * list, used ONLY to know which `issueLabel`s are allowed to unlock via `resolveIssueLabelsForPropagation`'s * relaxed maintainer-authored-issue path (either trust flag) -- omitting it (or a mapping never setting - * either flag) reproduces today's strict author-or-assignee-only behavior exactly. */ + * either flag) reproduces today's strict author-or-assignee-only behavior exactly. + * + * `prMergedAt` (#4528) is this PR's own `merged_at`, or `null` while unmerged -- the caller's `pr.mergedAt` + * straight from the DB row, no extra fetch. */ export async function fetchLinkedIssueLabelsForPropagation(args: { env: Env; repoFullName: string; @@ -116,6 +133,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: { installationId: number; prAuthorLogin: string | null | undefined; mappings?: readonly LinkedIssueLabelPropagationMapping[] | undefined; + prMergedAt?: string | null | undefined; }): Promise { if (args.linkedIssues.length === 0) return []; const linkedIssues = args.linkedIssues.slice(0, MAX_LINKED_ISSUES_TO_FETCH); @@ -129,6 +147,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: { args.installationId, ); const prAuthorLogin = args.prAuthorLogin?.toLowerCase(); + const prMergedAt = args.prMergedAt ?? null; const relaxableLabels = new Set( (args.mappings ?? []) .filter((mapping) => mapping.trustMaintainerAuthoredIssue === true || mapping.trustMaintainerAuthoredIssueForReward === true) @@ -152,6 +171,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: { result, prAuthorLogin, relaxableLabels, + prMergedAt, ), ), ); diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 27726a11c0..58ff0f5470 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -6089,7 +6089,7 @@ describe("GitHub backfill", () => { 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", title: null, body: null }, + facts: { number: 42, labels: ["bug", "manual-string-label"], assignees: ["maintainer"], state: "open", authorLogin: "reporter", title: null, body: null, closedAt: null }, }); }); @@ -6117,10 +6117,27 @@ describe("GitHub backfill", () => { authorLogin: "reporter", title: "Enrich SN74 Gittensor — add SSE stream", body: "We need a live SSE stream surface for SN74 Gittensor.", + closedAt: null, }, }); }); + it("extracts closedAt (#4528) from the same REST payload when the issue is closed", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async () => + Response.json({ number: 4279, state: "closed", closed_at: "2026-07-09T22:15:14Z" }), + ); + const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 4279, "tok"); + expect(result.status === "found" && result.facts.closedAt).toBe("2026-07-09T22:15:14Z"); + }); + + it("falls back to null for closedAt (#4528) when the payload omits it or it isn't a string", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async () => Response.json({ number: 4279, state: "open", closed_at: null })); + const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 4279, "tok"); + expect(result.status === "found" && result.facts.closedAt).toBeNull(); + }); + it("falls back to null for title/body when the payload omits them or they are empty strings", async () => { const env = createTestEnv({}); vi.stubGlobal("fetch", async () => Response.json({ number: 7, state: "open", title: "", body: "" })); diff --git a/test/unit/linked-issue-hard-rules.test.ts b/test/unit/linked-issue-hard-rules.test.ts index 12fe41d6ca..bad1e59cfe 100644 --- a/test/unit/linked-issue-hard-rules.test.ts +++ b/test/unit/linked-issue-hard-rules.test.ts @@ -563,7 +563,7 @@ describe("mergeLinkedIssueHardRuleWithPersistedViolation (#linked-issue-hard-rul }); describe("hasVerifiableOpenLinkedIssueReference (#unlinked-issue-guardrail-followup — pure evaluator)", () => { - const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null } }); + const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null, closedAt: null } }); const notFound: LinkedIssueFactsFetch = { status: "not_found" }; const fetchError: LinkedIssueFactsFetch = { status: "fetch_error" }; diff --git a/test/unit/linked-issue-label-propagation-fetch.test.ts b/test/unit/linked-issue-label-propagation-fetch.test.ts index 2f181d2b4f..7dbbe8c94f 100644 --- a/test/unit/linked-issue-label-propagation-fetch.test.ts +++ b/test/unit/linked-issue-label-propagation-fetch.test.ts @@ -231,6 +231,77 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( expect(result).toEqual([]); }); + describe("closed-by-own-merge trust (#4528 — merging a PR auto-closes its linked issue)", () => { + it("REGRESSION (PR #4494 shape): still propagates when the linked issue was closed at or after THIS PR's own merge", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/4279")) + return Response.json({ + number: 4279, + state: "closed", + closed_at: "2026-07-09T22:15:14Z", + user: { login: "contrib" }, + labels: ["gittensor:feature", "gittensor:priority"], + }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [4279], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + }); + expect(result).toEqual(["gittensor:feature", "gittensor:priority"]); + }); + + 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" }); + if (url.endsWith("/issues/777")) + return Response.json({ + number: 777, + state: "closed", + closed_at: "2026-07-01T00:00:00Z", + user: { login: "contrib" }, + labels: ["gittensor:priority"], + }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [777], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + }); + expect(result).toEqual([]); + }); + + it("does not propagate a closed issue missing closed_at even when prMergedAt is present (defensive: no provable closing-time relationship)", async () => { + stubFetch((url) => { + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/issues/778")) + return Response.json({ number: 778, state: "closed", user: { login: "contrib" }, labels: ["gittensor:priority"] }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({}); + const result = await fetchLinkedIssueLabelsForPropagation({ + env, + repoFullName: "owner/repo", + linkedIssues: [778], + installationId: 123, + prAuthorLogin: "contrib", + prMergedAt: "2026-07-09T22:15:13Z", + }); + expect(result).toEqual([]); + }); + }); + it("does not propagate labels when the PR author is missing", async () => { stubFetch((url) => { if (url.includes("/access_tokens")) diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index db088ce57d..8f3b7d1956 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -26979,6 +26979,77 @@ describe("queue processors", () => { expect(seen.removed).toEqual(["gittensor:feature"]); }); + it("REGRESSION (#4528, PR #4494 shape): keeps the propagated labels on the PR's own merge-closed webhook, instead of falling back to the title guess", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositoryFromGitHub(env, { name: "widget", full_name: "acme/widget", private: false, owner: { login: "acme" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "acme/widget", + commentMode: "off", + publicSurface: "label_only", + autoLabelEnabled: true, + createMissingLabel: false, + checkRunMode: "off", + gateCheckMode: "off", + reviewCheckMode: "disabled", + linkedIssueGateMode: "off", + aiReviewMode: "off", + // Real-world shape: the type-label decision runs regardless of the check-run/gate publish mode, but + // the SURROUNDING function only reaches that far for an already-closed PR when the agent layer is + // configured (autonomyNeedsGateEvaluation) -- an unconfigured repo's closed-PR pass has nothing else + // to do and bails before the label block. `label: "auto"` is the minimal opt-in that reproduces this + // without pulling in merge/close autonomy's own CI-wait/rebase machinery. + autonomy: { label: "auto" }, + linkedIssueLabelPropagation: { + enabled: true, + mode: "exclusive_type_label", + mappings: [ + { issueLabel: "gittensor:feature", prLabel: "gittensor:feature", removeOtherTypeLabels: true }, + { issueLabel: "gittensor:priority", prLabel: "gittensor:priority", removeOtherTypeLabels: false }, + ], + }, + }); + const seen = { posted: [] as string[], removed: [] as string[], issueFetches: 0 }; + // The linked issue is CLOSED, at a timestamp at/after this PR's own merge -- GitHub's standard "Closes #N" + // auto-close, fired by this very merge. Title deliberately uses a verb ("fold") absent from the + // feature-action-verb whitelist, so a title-only fallback would misclassify this as gittensor:bug -- + // this only stays gittensor:feature/gittensor:priority if the merge-closed issue is still trusted. + stubPropagationFetch(4494, 4279, seen, () => + Response.json({ + number: 4279, + state: "closed", + closed_at: "2026-07-09T22:15:14Z", + user: { login: "contributor" }, + labels: ["gittensor:feature", "gittensor:priority"], + }), + ); + + await processJob(env, { + type: "github-webhook", + deliveryId: "merge-close-race-4528", + eventName: "pull_request", + payload: { + action: "closed", + installation: { id: 123, account: { login: "acme", id: 1, type: "User" } }, + repository: { name: "widget", full_name: "acme/widget", private: false, owner: { login: "acme" } }, + pull_request: { + number: 4494, + title: "feat(x): fold run-state into the status panel", + state: "closed", + merged_at: "2026-07-09T22:15:13Z", + user: { login: "contributor" }, + author_association: "NONE", + head: { sha: "sha4494" }, + labels: [], + body: "Closes #4279", + }, + }, + }); + + expect(seen.issueFetches).toBe(1); + expect(seen.posted.sort()).toEqual(["gittensor:feature", "gittensor:priority"]); + expect(seen.removed).toEqual(["gittensor:bug"]); + }); + it("fails open to the normal title-based label when the linked issue's fetch fails (#priority-linked-issue-gate)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);