From d840309e8d3f345bd47e4d922a6be5f78f0768dc Mon Sep 17 00:00:00 2001 From: ultrahighsuper Date: Sat, 4 Jul 2026 10:00:46 +0900 Subject: [PATCH] fix(signals): recognize the fully-qualified owner/repo#N closing reference extractLinkedIssueNumbers only matched the bare `KEYWORD #N` closing form, so GitHub`s other documented auto-close syntax -- the fully-qualified `KEYWORD owner/repo#N` (routinely emitted by Renovate/Dependabot and by contributors pasting a qualified reference) -- was silently dropped. A PR whose body says `Fixes myorg/myrepo#42` was therefore scored as having NO linked issue: it wrongly tripped the missing_linked_issue preflight finding and fed slop a false hasLinkedIssue=false, penalizing a properly-linked PR. Match the qualified form too, repo-scoped: count `owner/repo#N` only when owner/repo case-insensitively equals this repo, so a cross-repo reference (which closes an issue elsewhere) never spoofs a same-repo link. The bare form and the #1988 word-boundary invariant are unchanged. --- src/signals/engine.ts | 17 ++++++++++++----- test/unit/signals-coverage.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 16768a0877..de7668b2a0 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -2506,7 +2506,7 @@ export function buildPreflightResult( issueQuality?: IssueQualityReport | null | undefined, ): PreflightResult { const lane = buildLaneAdvice(repo, input.repoFullName); - const linkedIssues = [...new Set([...(input.linkedIssues ?? []), ...extractLinkedIssueNumbers(truncateText(input.body ?? "", PREFLIGHT_LIMITS.bodyChars))])].sort( + const linkedIssues = [...new Set([...(input.linkedIssues ?? []), ...extractLinkedIssueNumbers(truncateText(input.body ?? "", PREFLIGHT_LIMITS.bodyChars), input.repoFullName)])].sort( (left, right) => left - right, ); // Flag an existing open-work cluster as a possible duplicate when it shares a @@ -2621,7 +2621,7 @@ export function buildLocalDiffPreflightResult( ): LocalDiffPreflightResult { /* v8 ignore next -- Undefined metadata arrays are normalized at API/MCP boundaries; local analysis tests cover empty metadata behavior. */ const changedFiles = [...new Set([...(input.changedFiles ?? []), ...(input.testFiles ?? [])])]; - const linkedFromCommit = extractLinkedIssueNumbers([input.commitMessage, input.body, input.title].filter(Boolean).join("\n")); + const linkedFromCommit = extractLinkedIssueNumbers([input.commitMessage, input.body, input.title].filter(Boolean).join("\n"), input.repoFullName); const base = buildPreflightResult( { ...input, @@ -5294,9 +5294,16 @@ function tokenize(value: string): string[] { .filter((term) => term.length > 2 && !STOPWORDS.has(term)); } -function extractLinkedIssueNumbers(text: string): number[] { - const matches = [...text.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)\b/gi)]; - return [...new Set(matches.map((match) => Number(match[1])).filter((value) => Number.isInteger(value) && value > 0))]; +function extractLinkedIssueNumbers(text: string, repoFullName: string): number[] { + const numbers = [...text.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)\b/gi)].map((match) => Number(match[1])); + // GitHub also auto-closes via the fully-qualified `KEYWORD owner/repo#N` form (e.g. Renovate/Dependabot bodies). + // Count it only when owner/repo case-insensitively equals THIS repo — a reference to a different repo closes an + // issue elsewhere, not here, so it must not spoof a same-repo link. Same `\b`-anchored keywords as above (#1988). + const target = repoFullName.toLowerCase(); + for (const match of text.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+([\w.-]+\/[\w.-]+)#(\d+)\b/gi)) { + if (match[1]!.toLowerCase() === target) numbers.push(Number(match[2])); + } + return [...new Set(numbers.filter((value) => Number.isInteger(value) && value > 0))]; } function outcomeSuccessPatterns(history: ContributorOutcomeHistory): OutcomePattern[] { diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 91ddc99cde..03199442b1 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -282,6 +282,29 @@ describe("signal coverage edge cases", () => { }); }); + it("recognizes GitHub's fully-qualified owner/repo#N closing reference, repo-scoped", () => { + const directRepo = repo("owner/direct"); + const linkedIssuesFor = (body: string) => + buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix cache invalidation", body }, directRepo, [issue(directRepo.fullName, 42, "Cache invalidation")], []); + + // Same-repo fully-qualified closing ref (GitHub's documented `KEYWORD owner/repo#N` form) links issue 42. + const qualified = linkedIssuesFor("Fixes owner/direct#42"); + expect(qualified.linkedIssues).toContain(42); + expect(qualified.findings.map((f) => f.code)).not.toContain("missing_linked_issue"); + + // …case-insensitively on owner/repo. + expect(linkedIssuesFor("Resolves Owner/Direct#42").linkedIssues).toContain(42); + + // A cross-repo reference closes an issue elsewhere and must NOT spoof a same-repo link. + const crossRepo = linkedIssuesFor("Fixes other-org/other#42"); + expect(crossRepo.linkedIssues).not.toContain(42); + expect(crossRepo.findings.map((f) => f.code)).toContain("missing_linked_issue"); + + // The bare `#N` form and word-boundary guard (#1988) are unchanged: `unfixes` is not a keyword. + expect(linkedIssuesFor("Closes #42").linkedIssues).toContain(42); + expect(linkedIssuesFor("unfixes owner/direct#42").linkedIssues).not.toContain(42); + }); + it("covers issue quality, burden, bounties, noise, and reviewability edge decisions", () => { const directRepo = repo("owner/direct"); const issueRepo = repo("owner/issues", { issueDiscoveryShare: 1 });