From c81a9df4adaacaf8dab7d28f5e563f7da7bf7919 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:47:59 -0700 Subject: [PATCH 1/2] fix(signals): recognize the qualified owner/repo#N closing-issue syntax The linked-issue detector that populates the stored pr.linkedIssues field (what the actual gate-close disposition reads) only matched GitHub's bare `Closes #123` closing-keyword form, not the equally valid, fully-qualified `Closes owner/repo#123` form. A PR whose only closing reference used the qualified form was scored "missing linked issue" and closed under the linked-issue-required policy, even though it correctly referenced a real, open issue. A separate, already-correct implementation of the same qualified-form matching existed in signals/engine.ts (added for #1988), but was only used for pre-open preflight planning, not the post-open gate-evaluation path that actually decides to close a PR -- the two implementations had drifted apart. Consolidated to one: db/repositories.ts now owns the canonical extractLinkedIssueNumbers/extractLinkedIssueNumbersWithOverflow, extended to accept a repoFullName and match owner/repo#N only when owner/repo case-insensitively equals the PR's own repo (a reference to a different repo closes an issue there, not here). engine.ts's local duplicate is removed in favor of importing the canonical version. --- src/db/repositories.ts | 18 ++++++++---- src/github/backfill.ts | 4 +-- src/queue/processors.ts | 1 + src/review/enrichment-wire.ts | 3 +- src/review/linked-issue-hard-rules.ts | 2 +- src/signals/engine.ts | 13 +-------- test/unit/db-parsers.test.ts | 40 +++++++++++++++++++++------ test/unit/enrichment-wire.test.ts | 10 +++---- 8 files changed, 56 insertions(+), 35 deletions(-) diff --git a/src/db/repositories.ts b/src/db/repositories.ts index cb893af65f..3d9319a035 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -5406,7 +5406,7 @@ function toPullRequestRecord(repoFullName: string, pr: GitHubPullRequestPayload) // subsequent DB round-trip) sees the same value instead of `undefined`. createdAt: pr.created_at, labels: (pr.labels ?? []).flatMap((label) => (label.name ? [label.name] : [])), - linkedIssues: extractLinkedIssueNumbers(pr.body ?? ""), + linkedIssues: extractLinkedIssueNumbers(pr.body ?? "", repoFullName), }; /* v8 ignore stop */ } @@ -7001,13 +7001,19 @@ export type LinkedIssueExtractionResult = { overflow: boolean; }; -export function extractLinkedIssueNumbersWithOverflow(text: string, limit = MAX_LINKED_ISSUE_NUMBERS): LinkedIssueExtractionResult { +export function extractLinkedIssueNumbersWithOverflow(text: string, repoFullName: string, limit = MAX_LINKED_ISSUE_NUMBERS): LinkedIssueExtractionResult { const normalizedLimit = Math.max(0, Math.floor(limit)); + const target = repoFullName.toLowerCase(); const linkedIssues: number[] = []; const seen = new Set(); - for (const match of text.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)\b/gi)) { - const value = Number(match[1]); + // Matches both GitHub's bare `KEYWORD #N` and fully-qualified `KEYWORD owner/repo#N` closing syntax (#3862) -- + // the qualified form only counts when owner/repo case-insensitively matches THIS repo; a reference to a + // different repo closes an issue there, not here, and must not spoof a same-repo linked-issue match. + for (const match of text.matchAll(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:([\w.-]+\/[\w.-]+)#|#)(\d+)\b/gi)) { + const owner = match[1]; + if (owner && owner.toLowerCase() !== target) continue; + const value = Number(match[2]); if (!Number.isInteger(value) || value <= 0 || seen.has(value)) continue; seen.add(value); if (linkedIssues.length >= normalizedLimit) return { numbers: linkedIssues, overflow: true }; @@ -7016,8 +7022,8 @@ export function extractLinkedIssueNumbersWithOverflow(text: string, limit = MAX_ return { numbers: linkedIssues, overflow: false }; } -export function extractLinkedIssueNumbers(text: string, limit = MAX_LINKED_ISSUE_NUMBERS): number[] { - return extractLinkedIssueNumbersWithOverflow(text, limit).numbers; +export function extractLinkedIssueNumbers(text: string, repoFullName: string, limit = MAX_LINKED_ISSUE_NUMBERS): number[] { + return extractLinkedIssueNumbersWithOverflow(text, repoFullName, limit).numbers; } function extractLinkedPrNumbers(text: string): number[] { diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 0d8d5b92f2..4eecd4ffa3 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -803,7 +803,7 @@ export async function refreshContributorActivity( openPullRequests: openPullRequestCount, issues: issueCount, stalePullRequests: openNodes.filter((node) => node.updatedAt && daysSince(node.updatedAt) >= 14).length, - unlinkedPullRequests: openNodes.filter((node) => extractLinkedIssueNumbers(node.body ?? "").length === 0).length, + unlinkedPullRequests: openNodes.filter((node) => extractLinkedIssueNumbers(node.body ?? "", repo.fullName).length === 0).length, dominantLabels: topItems(labelNames, 8), lastActivityAt: latestDate([ ...compactNodes(allPullRequests).map((node) => node.updatedAt ?? node.mergedAt), @@ -3915,7 +3915,7 @@ function toRecentMergedPullRequest(repoFullName: string, pr: GitHubPullRequestPa htmlUrl: pr.html_url, mergedAt: pr.merged_at, labels: (pr.labels ?? []).flatMap((label) => (label.name ? [label.name] : [])), - linkedIssues: extractLinkedIssueNumbers(pr.body ?? ""), + linkedIssues: extractLinkedIssueNumbers(pr.body ?? "", repoFullName), changedFiles: files.map((file) => file.filename), payload: pr as unknown as Record, }; diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 9c45b21dfa..138b562334 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7013,6 +7013,7 @@ export async function runAiReviewForAdvisory( resolveEnrichmentLinkedIssueNumbers( args.pr.linkedIssues, args.pr.body, + args.repoFullName, ), ), githubToken: isReesGithubTokenForwardingEnabled(env) diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index ba67d81b7c..79ba8ddd8d 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -260,10 +260,11 @@ export function resolveEnrichmentAnalyzerSelection( export function resolveEnrichmentLinkedIssueNumbers( linkedIssues: number[] | undefined, body: string | null | undefined, + repoFullName: string, ): number[] { const explicit = (linkedIssues ?? []).filter((candidate) => Number.isInteger(candidate) && candidate > 0); if (explicit.length > 0) return explicit; - return extractLinkedIssueNumbers(body ?? ""); + return extractLinkedIssueNumbers(body ?? "", repoFullName); } /** Resolve the PR's primary linked issue into the compact REES envelope (#1478). */ diff --git a/src/review/linked-issue-hard-rules.ts b/src/review/linked-issue-hard-rules.ts index 71745f0b54..b746744e33 100644 --- a/src/review/linked-issue-hard-rules.ts +++ b/src/review/linked-issue-hard-rules.ts @@ -158,7 +158,7 @@ export async function resolveLinkedIssueHardRule(args: { args.config.missingPointLabelClose === "block" || args.config.maintainerOnlyLabelClose === "block"; if (!anyRuleOn) return undefined; - if (extractLinkedIssueNumbersWithOverflow(args.body ?? "").overflow) { + if (extractLinkedIssueNumbersWithOverflow(args.body ?? "", args.repoFullName).overflow) { return { violated: true, reason: "PR body links more issues than Gittensory can safely verify automatically; please reduce linked closing references or request maintainer review.", diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 60599c4fce..eba8212912 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -24,6 +24,7 @@ import { gittensoryFooter, gittensorRepoEarnUrl } from "../github/footer"; import type { FocusManifestReviewConfig, ReviewFieldKey } from "./focus-manifest"; import type { GittensorContributorSnapshot } from "../gittensor/api"; import { nowIso } from "../utils/json"; +import { extractLinkedIssueNumbers } from "../db/repositories"; import { sanitizePublicComment } from "../queue-intelligence"; import { labelMatchesPattern, projectLinkedIssueMultiplierForPlannedSolve, type LinkedIssueMultiplierStatus } from "../scoring/preview"; import { hasLocalTestEvidence, hasValidationNote, isTestPath } from "./test-evidence"; @@ -5312,18 +5313,6 @@ export function tokenize(value: string): string[] { .filter((term) => term.length > 2 && !STOPWORDS.has(term)); } -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[] { const patterns: OutcomePattern[] = []; for (const outcome of history.repoOutcomes) { diff --git a/test/unit/db-parsers.test.ts b/test/unit/db-parsers.test.ts index 136a830a49..b87fedefc0 100644 --- a/test/unit/db-parsers.test.ts +++ b/test/unit/db-parsers.test.ts @@ -38,8 +38,8 @@ describe("database row parser hardening", () => { it("caps linked issues extracted from attacker-controlled PR bodies and reports overflow", () => { const body = Array.from({ length: MAX_LINKED_ISSUE_NUMBERS + 25 }, (_, index) => `Fixes #${index + 1}`).join("\n"); - expect(extractLinkedIssueNumbers(body)).toEqual(Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => index + 1)); - expect(extractLinkedIssueNumbersWithOverflow(body)).toEqual({ + expect(extractLinkedIssueNumbers(body, "owner/repo")).toEqual(Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => index + 1)); + expect(extractLinkedIssueNumbersWithOverflow(body, "owner/repo")).toEqual({ numbers: Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => index + 1), overflow: true, }); @@ -48,13 +48,37 @@ describe("database row parser hardening", () => { it("deduplicates linked issues before applying the extraction cap", () => { const body = [`Fixes #1`, ...Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => `Resolves #${index + 1}`)].join("\n"); - expect(extractLinkedIssueNumbers(body)).toEqual(Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => index + 1)); - expect(extractLinkedIssueNumbersWithOverflow(body).overflow).toBe(false); + expect(extractLinkedIssueNumbers(body, "owner/repo")).toEqual(Array.from({ length: MAX_LINKED_ISSUE_NUMBERS }, (_, index) => index + 1)); + expect(extractLinkedIssueNumbersWithOverflow(body, "owner/repo").overflow).toBe(false); }); it("returns no linked issues when the cap is zero or negative", () => { - expect(extractLinkedIssueNumbers("Fixes #1\nCloses #2", 0)).toEqual([]); - expect(extractLinkedIssueNumbers("Fixes #1", -5)).toEqual([]); + expect(extractLinkedIssueNumbers("Fixes #1\nCloses #2", "owner/repo", 0)).toEqual([]); + expect(extractLinkedIssueNumbers("Fixes #1", "owner/repo", -5)).toEqual([]); + }); + + it("recognizes the fully-qualified `Fixes owner/repo#N` closing syntax when owner/repo matches this repo (#3862)", () => { + expect(extractLinkedIssueNumbers("Closes owner/repo#42", "owner/repo")).toEqual([42]); + // Case-insensitive, matching GitHub's own repo-name matching. + expect(extractLinkedIssueNumbers("Fixes Owner/Repo#7", "owner/repo")).toEqual([7]); + // A DIFFERENT repo's qualified reference must not spoof a same-repo linked issue. + expect(extractLinkedIssueNumbers("Resolves other/repo#99", "owner/repo")).toEqual([]); + // Bare and qualified forms mix freely and dedupe together. + expect(extractLinkedIssueNumbers("Fixes #1\nCloses owner/repo#1\nResolves owner/repo#2", "owner/repo")).toEqual([1, 2]); + }); + + it("REGRESSION (#3862): a stored PR using ONLY the qualified `Closes owner/repo#N` form is not flagged as unlinked", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 5, + title: "Qualified-form closing reference", + state: "open", + user: { login: "contributor1" }, + labels: [], + body: "Closes owner/repo#42", + }); + const stored = await getPullRequest(env, "owner/repo", 5); + expect(stored?.linkedIssues).toEqual([42]); }); it("returns empty arrays from D1 raw() when a select has no rows", async () => { @@ -393,7 +417,7 @@ describe("database row parser hardening", () => { await upsertPullRequestFromGitHub(env, "owner/repo", { number: 10, title: "Too many claims", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [], body }); const claimed = await getPullRequest(env, "owner/repo", 10); expect(claimed?.linkedIssues).toHaveLength(MAX_LINKED_ISSUE_NUMBERS); - expect(extractLinkedIssueNumbersWithOverflow(claimed?.body ?? "").overflow).toBe(true); + expect(extractLinkedIssueNumbersWithOverflow(claimed?.body ?? "", "owner/repo").overflow).toBe(true); const resynced = await upsertPullRequestFromGitHub(env, "owner/repo", { number: 10, title: "Too many claims", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [] }); expect(resynced.body).toBe(body); @@ -401,7 +425,7 @@ describe("database row parser hardening", () => { const stored = await getPullRequest(env, "owner/repo", 10); expect(stored?.body).toBe(body); - expect(extractLinkedIssueNumbersWithOverflow(stored?.body ?? "").overflow).toBe(true); + expect(extractLinkedIssueNumbersWithOverflow(stored?.body ?? "", "owner/repo").overflow).toBe(true); expect(stored?.linkedIssues).toHaveLength(MAX_LINKED_ISSUE_NUMBERS); }); diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 3c7752b71b..e38dcad35f 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -717,17 +717,17 @@ describe("resolveReesAnalyzers", () => { describe("resolveEnrichmentLinkedIssueNumbers", () => { it("prefers explicit linkedIssues over body parsing", () => { - expect(resolveEnrichmentLinkedIssueNumbers([7], "Fixes #42")).toEqual([7]); + expect(resolveEnrichmentLinkedIssueNumbers([7], "Fixes #42", "owner/repo")).toEqual([7]); }); it("parses Fixes #N from the PR body when linkedIssues is empty", () => { - expect(resolveEnrichmentLinkedIssueNumbers([], "Fixes #42\nCloses #99")).toEqual([42, 99]); - expect(resolveEnrichmentLinkedIssueNumbers(undefined, "Resolves #3")).toEqual([3]); + expect(resolveEnrichmentLinkedIssueNumbers([], "Fixes #42\nCloses #99", "owner/repo")).toEqual([42, 99]); + expect(resolveEnrichmentLinkedIssueNumbers(undefined, "Resolves #3", "owner/repo")).toEqual([3]); }); it("returns an empty list when neither source yields issue numbers", () => { - expect(resolveEnrichmentLinkedIssueNumbers([], "no issue refs")).toEqual([]); - expect(resolveEnrichmentLinkedIssueNumbers(undefined, undefined)).toEqual([]); + expect(resolveEnrichmentLinkedIssueNumbers([], "no issue refs", "owner/repo")).toEqual([]); + expect(resolveEnrichmentLinkedIssueNumbers(undefined, undefined, "owner/repo")).toEqual([]); }); }); From 16a346176d9d6196dc6bb1ae063499de35d8b84f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:51:40 -0700 Subject: [PATCH 2/2] test(github): cover the nullish body fallback in unlinkedPullRequests counting --- test/unit/backfill.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index f2807d0727..4f5cbb68ea 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -254,8 +254,14 @@ describe("GitHub backfill", () => { nodes: [{ __typename: "PullRequest", mergedAt: "2026-05-24T00:00:00Z", labels: { nodes: [{ name: "bug" }] }, body: "Fixes #1" }], }, r_JSONbored_gittensory_open: { - issueCount: 2, - nodes: [{ __typename: "PullRequest", updatedAt: "2026-04-01T00:00:00Z", labels: { nodes: [{ name: "ci" }] }, body: "" }], + issueCount: 3, + nodes: [ + { __typename: "PullRequest", updatedAt: "2026-04-01T00:00:00Z", labels: { nodes: [{ name: "ci" }] }, body: "" }, + { __typename: "PullRequest", updatedAt: "2026-04-02T00:00:00Z", labels: { nodes: [{ name: "ci" }] }, body: "Fixes #2" }, + // REGRESSION: no `body` field at all (GitHub omits it, not just an empty string) -- exercises + // the `node.body ?? ""` nullish fallback the unlinkedPullRequests count feeds into. + { __typename: "PullRequest", updatedAt: "2026-04-03T00:00:00Z", labels: { nodes: [{ name: "ci" }] } }, + ], }, r_JSONbored_gittensory_issues: { issueCount: 12, @@ -270,7 +276,7 @@ describe("GitHub backfill", () => { expect(result).toMatchObject({ repoCount: 1, updatedRepoStats: 1, warnings: [] }); expect(authHeaders).toContain("Bearer public-token"); expect(await listContributorRepoStats(env, "JSONbored")).toMatchObject([ - { repoFullName: "JSONbored/gittensory", pullRequests: 50, mergedPullRequests: 47, openPullRequests: 2, issues: 12, unlinkedPullRequests: 1 }, + { repoFullName: "JSONbored/gittensory", pullRequests: 50, mergedPullRequests: 47, openPullRequests: 3, issues: 12, unlinkedPullRequests: 2 }, ]); });