Skip to content

Commit 86eeea0

Browse files
authored
fix(review): trust a linked issue closed by this PR's own merge for label propagation (#4528) (#4534)
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).
1 parent 23693c3 commit 86eeea0

7 files changed

Lines changed: 197 additions & 8 deletions

src/github/backfill.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3839,6 +3839,10 @@ export type LinkedIssueFactsResult = {
38393839
authorLogin: string | null;
38403840
title?: string | null;
38413841
body?: string | null;
3842+
/** GitHub's `closed_at` for this issue, or `null` while open (#4528: label-propagation callers use this
3843+
* to trust an issue closed by THIS PR's own merge, without granting authority to one closed earlier
3844+
* for an unrelated reason). Same REST payload as every other field here -- no extra call. */
3845+
closedAt: string | null;
38423846
};
38433847

38443848
/** 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(
38843888
user?: { login?: string | null } | null;
38853889
title?: string | null;
38863890
body?: string | null;
3891+
closed_at?: string | null;
38873892
}>(env, repoFullName, `/issues/${issueNumber}`, token, githubRateLimitOptions(admissionKey));
38883893
} catch (error) {
38893894
if (!(error instanceof GitHubApiError) || error.statusCode !== 404) return { status: "fetch_error" };
@@ -3906,6 +3911,7 @@ export async function fetchLinkedIssueFacts(
39063911
authorLogin: data.user?.login ?? null,
39073912
title: typeof data.title === "string" && data.title.length > 0 ? data.title : null,
39083913
body: typeof data.body === "string" && data.body.length > 0 ? data.body : null,
3914+
closedAt: typeof data.closed_at === "string" ? data.closed_at : null,
39093915
},
39103916
};
39113917
}

src/queue/processors.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8688,6 +8688,10 @@ async function maybePublishPrPublicSurface(
86888688
installationId,
86898689
prAuthorLogin: pr.authorLogin,
86908690
mappings: propagation.mappings,
8691+
// #4528: lets a closed linked issue still count when THIS PR's own merge is what closed it
8692+
// (the standard "Closes #N" auto-close), instead of losing propagation authority the instant
8693+
// the merge that's supposed to earn the label also closes its evidence.
8694+
prMergedAt: pr.mergedAt ?? null,
86918695
})
86928696
: [];
86938697
const decisionResult = resolvePrTypeLabel({

src/review/linked-issue-label-propagation-fetch.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fetchLinkedIssueFacts, type LinkedIssueFactsFetch } from "../github/backfill";
1+
import { fetchLinkedIssueFacts, type LinkedIssueFactsFetch, type LinkedIssueFactsResult } from "../github/backfill";
22
import { createInstallationToken, getRepositoryCollaboratorPermission } from "../github/app";
33
import { githubRateLimitAdmissionKeyForToken } from "../github/client";
44
import { parseGitHubLoginList } from "../auth/security";
@@ -40,6 +40,19 @@ async function isRepoMaintainerLogin(env: Env, installationId: number, repoFullN
4040
return permission != null && new Set(["admin", "maintain", "write"]).has(permission);
4141
}
4242

43+
/** True when the linked issue's authority for propagation can be trusted (#4528): it's still OPEN, or it
44+
* was closed no earlier than THIS PR's own merge. Merging a PR whose body says "Closes #N" auto-closes
45+
* issue #N as an immediate side effect of that same merge -- so `closedAt >= prMergedAt` is exactly the
46+
* signature of "this merge is what closed it," the single most authoritative moment for propagation to
47+
* fire, not a weaker one. An issue closed BEFORE this PR ever merged (`closedAt < prMergedAt`) is the
48+
* gaming case the OPEN-only check originally existed to block -- a PR opportunistically referencing some
49+
* unrelated, already-resolved issue to borrow its label -- and stays blocked, unchanged. `prMergedAt`
50+
* absent (PR not yet merged) never trusts a closed issue, also unchanged. */
51+
function isLinkedIssueTrustworthy(facts: LinkedIssueFactsResult, prMergedAt: string | null): boolean {
52+
if (facts.state === "open") return true;
53+
return prMergedAt !== null && facts.closedAt !== null && facts.closedAt >= prMergedAt;
54+
}
55+
4356
/** Per-issue label resolution for {@link fetchLinkedIssueLabelsForPropagation}: a direct PR-author-is-
4457
* issue-author-or-assignee match unlocks EVERY label the issue carries (today's original behavior,
4558
* unchanged). Failing that, a mapping explicitly opted into `trustMaintainerAuthoredIssue` OR
@@ -61,8 +74,9 @@ async function resolveIssueLabelsForPropagation(
6174
result: LinkedIssueFactsFetch,
6275
prAuthorLogin: string | undefined,
6376
relaxableLabels: ReadonlySet<string>,
77+
prMergedAt: string | null,
6478
): Promise<string[]> {
65-
if (result.status !== "found" || result.facts.state !== "open" || !prAuthorLogin) return [];
79+
if (result.status !== "found" || !isLinkedIssueTrustworthy(result.facts, prMergedAt) || !prAuthorLogin) return [];
6680
const allLabels = result.facts.labels;
6781
const issueAuthorLogin = result.facts.authorLogin?.toLowerCase();
6882
const assignees = result.facts.assignees.map((login) => login.toLowerCase());
@@ -89,9 +103,9 @@ async function resolveIssueLabelsForPropagation(
89103
}
90104

91105
/** FETCH every linked issue's labels (fail-open) and flatten into one label list for
92-
* `resolvePrTypeLabel` (`src/settings/pr-type-label.ts`) to match against. Only verified OPEN issues
93-
* can contribute labels; closing-keyword text in a PR body is author-controlled and is not authority by
94-
* itself. Mirrors
106+
* `resolvePrTypeLabel` (`src/settings/pr-type-label.ts`) to match against. Only an OPEN issue, or one
107+
* closed no earlier than THIS PR's own merge (#4528, {@link isLinkedIssueTrustworthy}), can contribute
108+
* labels; closing-keyword text in a PR body is author-controlled and is not authority by itself. Mirrors
95109
* `resolveLinkedIssueHardRule`'s own fetch idiom (`src/review/linked-issue-hard-rules.ts`): a per-issue
96110
* fetch failure contributes no labels rather than throwing, so if EVERY linked issue fails, the result is
97111
* `[]` — which can never match a mapping, meaning a sensitive label like `gittensor:priority` never applies
@@ -108,14 +122,18 @@ async function resolveIssueLabelsForPropagation(
108122
* `mappings` (optional, #priority-linked-issue-gate-ownership) is the propagation config's own mapping
109123
* list, used ONLY to know which `issueLabel`s are allowed to unlock via `resolveIssueLabelsForPropagation`'s
110124
* relaxed maintainer-authored-issue path (either trust flag) -- omitting it (or a mapping never setting
111-
* either flag) reproduces today's strict author-or-assignee-only behavior exactly. */
125+
* either flag) reproduces today's strict author-or-assignee-only behavior exactly.
126+
*
127+
* `prMergedAt` (#4528) is this PR's own `merged_at`, or `null` while unmerged -- the caller's `pr.mergedAt`
128+
* straight from the DB row, no extra fetch. */
112129
export async function fetchLinkedIssueLabelsForPropagation(args: {
113130
env: Env;
114131
repoFullName: string;
115132
linkedIssues: number[];
116133
installationId: number;
117134
prAuthorLogin: string | null | undefined;
118135
mappings?: readonly LinkedIssueLabelPropagationMapping[] | undefined;
136+
prMergedAt?: string | null | undefined;
119137
}): Promise<string[]> {
120138
if (args.linkedIssues.length === 0) return [];
121139
const linkedIssues = args.linkedIssues.slice(0, MAX_LINKED_ISSUES_TO_FETCH);
@@ -129,6 +147,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: {
129147
args.installationId,
130148
);
131149
const prAuthorLogin = args.prAuthorLogin?.toLowerCase();
150+
const prMergedAt = args.prMergedAt ?? null;
132151
const relaxableLabels = new Set(
133152
(args.mappings ?? [])
134153
.filter((mapping) => mapping.trustMaintainerAuthoredIssue === true || mapping.trustMaintainerAuthoredIssueForReward === true)
@@ -152,6 +171,7 @@ export async function fetchLinkedIssueLabelsForPropagation(args: {
152171
result,
153172
prAuthorLogin,
154173
relaxableLabels,
174+
prMergedAt,
155175
),
156176
),
157177
);

test/unit/backfill.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6089,7 +6089,7 @@ describe("GitHub backfill", () => {
60896089
const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 42, "tok");
60906090
expect(result).toEqual({
60916091
status: "found",
6092-
facts: { number: 42, labels: ["bug", "manual-string-label"], assignees: ["maintainer"], state: "open", authorLogin: "reporter", title: null, body: null },
6092+
facts: { number: 42, labels: ["bug", "manual-string-label"], assignees: ["maintainer"], state: "open", authorLogin: "reporter", title: null, body: null, closedAt: null },
60936093
});
60946094
});
60956095

@@ -6117,10 +6117,27 @@ describe("GitHub backfill", () => {
61176117
authorLogin: "reporter",
61186118
title: "Enrich SN74 Gittensor — add SSE stream",
61196119
body: "We need a live SSE stream surface for SN74 Gittensor.",
6120+
closedAt: null,
61206121
},
61216122
});
61226123
});
61236124

6125+
it("extracts closedAt (#4528) from the same REST payload when the issue is closed", async () => {
6126+
const env = createTestEnv({});
6127+
vi.stubGlobal("fetch", async () =>
6128+
Response.json({ number: 4279, state: "closed", closed_at: "2026-07-09T22:15:14Z" }),
6129+
);
6130+
const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 4279, "tok");
6131+
expect(result.status === "found" && result.facts.closedAt).toBe("2026-07-09T22:15:14Z");
6132+
});
6133+
6134+
it("falls back to null for closedAt (#4528) when the payload omits it or it isn't a string", async () => {
6135+
const env = createTestEnv({});
6136+
vi.stubGlobal("fetch", async () => Response.json({ number: 4279, state: "open", closed_at: null }));
6137+
const result = await fetchLinkedIssueFacts(env, "JSONbored/gittensory", 4279, "tok");
6138+
expect(result.status === "found" && result.facts.closedAt).toBeNull();
6139+
});
6140+
61246141
it("falls back to null for title/body when the payload omits them or they are empty strings", async () => {
61256142
const env = createTestEnv({});
61266143
vi.stubGlobal("fetch", async () => Response.json({ number: 7, state: "open", title: "", body: "" }));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ describe("mergeLinkedIssueHardRuleWithPersistedViolation (#linked-issue-hard-rul
563563
});
564564

565565
describe("hasVerifiableOpenLinkedIssueReference (#unlinked-issue-guardrail-followup — pure evaluator)", () => {
566-
const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null } });
566+
const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null, closedAt: null } });
567567
const notFound: LinkedIssueFactsFetch = { status: "not_found" };
568568
const fetchError: LinkedIssueFactsFetch = { status: "fetch_error" };
569569

test/unit/linked-issue-label-propagation-fetch.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,77 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", (
231231
expect(result).toEqual([]);
232232
});
233233

234+
describe("closed-by-own-merge trust (#4528 — merging a PR auto-closes its linked issue)", () => {
235+
it("REGRESSION (PR #4494 shape): still propagates when the linked issue was closed at or after THIS PR's own merge", async () => {
236+
stubFetch((url) => {
237+
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
238+
if (url.endsWith("/issues/4279"))
239+
return Response.json({
240+
number: 4279,
241+
state: "closed",
242+
closed_at: "2026-07-09T22:15:14Z",
243+
user: { login: "contrib" },
244+
labels: ["gittensor:feature", "gittensor:priority"],
245+
});
246+
return new Response("not found", { status: 404 });
247+
});
248+
const env = createTestEnv({});
249+
const result = await fetchLinkedIssueLabelsForPropagation({
250+
env,
251+
repoFullName: "owner/repo",
252+
linkedIssues: [4279],
253+
installationId: 123,
254+
prAuthorLogin: "contrib",
255+
prMergedAt: "2026-07-09T22:15:13Z",
256+
});
257+
expect(result).toEqual(["gittensor:feature", "gittensor:priority"]);
258+
});
259+
260+
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 () => {
261+
stubFetch((url) => {
262+
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
263+
if (url.endsWith("/issues/777"))
264+
return Response.json({
265+
number: 777,
266+
state: "closed",
267+
closed_at: "2026-07-01T00:00:00Z",
268+
user: { login: "contrib" },
269+
labels: ["gittensor:priority"],
270+
});
271+
return new Response("not found", { status: 404 });
272+
});
273+
const env = createTestEnv({});
274+
const result = await fetchLinkedIssueLabelsForPropagation({
275+
env,
276+
repoFullName: "owner/repo",
277+
linkedIssues: [777],
278+
installationId: 123,
279+
prAuthorLogin: "contrib",
280+
prMergedAt: "2026-07-09T22:15:13Z",
281+
});
282+
expect(result).toEqual([]);
283+
});
284+
285+
it("does not propagate a closed issue missing closed_at even when prMergedAt is present (defensive: no provable closing-time relationship)", async () => {
286+
stubFetch((url) => {
287+
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
288+
if (url.endsWith("/issues/778"))
289+
return Response.json({ number: 778, state: "closed", user: { login: "contrib" }, labels: ["gittensor:priority"] });
290+
return new Response("not found", { status: 404 });
291+
});
292+
const env = createTestEnv({});
293+
const result = await fetchLinkedIssueLabelsForPropagation({
294+
env,
295+
repoFullName: "owner/repo",
296+
linkedIssues: [778],
297+
installationId: 123,
298+
prAuthorLogin: "contrib",
299+
prMergedAt: "2026-07-09T22:15:13Z",
300+
});
301+
expect(result).toEqual([]);
302+
});
303+
});
304+
234305
it("does not propagate labels when the PR author is missing", async () => {
235306
stubFetch((url) => {
236307
if (url.includes("/access_tokens"))

test/unit/queue.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26979,6 +26979,77 @@ describe("queue processors", () => {
2697926979
expect(seen.removed).toEqual(["gittensor:feature"]);
2698026980
});
2698126981

26982+
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 () => {
26983+
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
26984+
await upsertRepositoryFromGitHub(env, { name: "widget", full_name: "acme/widget", private: false, owner: { login: "acme" } }, 123);
26985+
await upsertRepositorySettings(env, {
26986+
repoFullName: "acme/widget",
26987+
commentMode: "off",
26988+
publicSurface: "label_only",
26989+
autoLabelEnabled: true,
26990+
createMissingLabel: false,
26991+
checkRunMode: "off",
26992+
gateCheckMode: "off",
26993+
reviewCheckMode: "disabled",
26994+
linkedIssueGateMode: "off",
26995+
aiReviewMode: "off",
26996+
// Real-world shape: the type-label decision runs regardless of the check-run/gate publish mode, but
26997+
// the SURROUNDING function only reaches that far for an already-closed PR when the agent layer is
26998+
// configured (autonomyNeedsGateEvaluation) -- an unconfigured repo's closed-PR pass has nothing else
26999+
// to do and bails before the label block. `label: "auto"` is the minimal opt-in that reproduces this
27000+
// without pulling in merge/close autonomy's own CI-wait/rebase machinery.
27001+
autonomy: { label: "auto" },
27002+
linkedIssueLabelPropagation: {
27003+
enabled: true,
27004+
mode: "exclusive_type_label",
27005+
mappings: [
27006+
{ issueLabel: "gittensor:feature", prLabel: "gittensor:feature", removeOtherTypeLabels: true },
27007+
{ issueLabel: "gittensor:priority", prLabel: "gittensor:priority", removeOtherTypeLabels: false },
27008+
],
27009+
},
27010+
});
27011+
const seen = { posted: [] as string[], removed: [] as string[], issueFetches: 0 };
27012+
// The linked issue is CLOSED, at a timestamp at/after this PR's own merge -- GitHub's standard "Closes #N"
27013+
// auto-close, fired by this very merge. Title deliberately uses a verb ("fold") absent from the
27014+
// feature-action-verb whitelist, so a title-only fallback would misclassify this as gittensor:bug --
27015+
// this only stays gittensor:feature/gittensor:priority if the merge-closed issue is still trusted.
27016+
stubPropagationFetch(4494, 4279, seen, () =>
27017+
Response.json({
27018+
number: 4279,
27019+
state: "closed",
27020+
closed_at: "2026-07-09T22:15:14Z",
27021+
user: { login: "contributor" },
27022+
labels: ["gittensor:feature", "gittensor:priority"],
27023+
}),
27024+
);
27025+
27026+
await processJob(env, {
27027+
type: "github-webhook",
27028+
deliveryId: "merge-close-race-4528",
27029+
eventName: "pull_request",
27030+
payload: {
27031+
action: "closed",
27032+
installation: { id: 123, account: { login: "acme", id: 1, type: "User" } },
27033+
repository: { name: "widget", full_name: "acme/widget", private: false, owner: { login: "acme" } },
27034+
pull_request: {
27035+
number: 4494,
27036+
title: "feat(x): fold run-state into the status panel",
27037+
state: "closed",
27038+
merged_at: "2026-07-09T22:15:13Z",
27039+
user: { login: "contributor" },
27040+
author_association: "NONE",
27041+
head: { sha: "sha4494" },
27042+
labels: [],
27043+
body: "Closes #4279",
27044+
},
27045+
},
27046+
});
27047+
27048+
expect(seen.issueFetches).toBe(1);
27049+
expect(seen.posted.sort()).toEqual(["gittensor:feature", "gittensor:priority"]);
27050+
expect(seen.removed).toEqual(["gittensor:bug"]);
27051+
});
27052+
2698227053
it("fails open to the normal title-based label when the linked issue's fetch fails (#priority-linked-issue-gate)", async () => {
2698327054
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
2698427055
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);

0 commit comments

Comments
 (0)