From 17bb4a8d171b4863105be4a072b3df2c94fffd59 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 09:58:44 -0700 Subject: [PATCH] fix(review): stop the deterministic type-label mislabel from #5233's broken closure check fetchLinkedIssueClosedByPullRequest read GitHub's REST /issues/{n}/timeline looking for a source.issue field on "closed" events, but that field never appears there -- only on cross-referenced events. Confirmed against three live production issues (commit_id null, no source key at all). This made the check always fail, turning a rare race into a 100%-reproducing failure on every "Closes #N" merge. Replaced with GraphQL's Issue.timelineItems -> ClosedEvent.closer, verified empirically against live issues. Also fixes two related gaps found during the same investigation: - resolvePrTypeLabel picked the exclusive bug/feature label by config array order (bug always won when both matched an issue's labels) instead of declared precedence. Now the LAST-configured exclusive match wins; operators declare exclusive mappings in ascending precedence order. Updated the two bundled example configs, which still described the old first-wins rule. - maybeReReviewOnLinkedIssueChange only checked isConvergenceRepoAllowed, unlike the periodic sweep, which also falls back to isAgentConfigured(settings.autonomy). Aligned the two gates, short-circuited so the common allowlisted case never pays for the extra settings fetch. Closes #5385 --- .gittensory.yml.example | 6 +- config/examples/gittensory.full.yml | 6 +- .../src/settings/pr-type-label.ts | 28 +++-- src/github/backfill.ts | 45 ++++--- src/queue/processors.ts | 9 +- src/settings/pr-type-label.ts | 28 +++-- test/unit/backfill.test.ts | 115 ++++++++++++++++++ ...nked-issue-label-propagation-fetch.test.ts | 44 +++++-- test/unit/pr-type-label-engine.test.ts | 4 +- test/unit/pr-type-label.test.ts | 11 +- test/unit/queue-5.test.ts | 12 +- test/unit/queue.test.ts | 66 +++++++++- 12 files changed, 308 insertions(+), 66 deletions(-) diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 4d2aa12725..2806a2035c 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -619,8 +619,10 @@ settings: # output, or existing PR labels, only ever copied from a linked/closing issue ("Fixes #123") that # ALREADY carries the configured issue label. Generic beyond the priority use case: any issue label # can map to any PR label. `removeOtherTypeLabels: true` marks the mapping EXCLUSIVE -- it REPLACES - # the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; only - # the FIRST-configured exclusive match wins when more than one applies. `false` (e.g. `gittensor:priority`, + # the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; when + # more than one applies, the LAST-configured exclusive match wins -- declare exclusive mappings in + # ASCENDING precedence order (lowest-value category first, e.g. bug before feature) so the highest- + # precedence one you actually want is declared last. `false` (e.g. `gittensor:priority`, # which is a reward tag that coexists WITH whichever type already applies, not a type of its own) applies # the mapped label ADDITIVELY alongside whichever exclusive match (or the normal title-based bug/feature # label) already won -- every additive match composes together rather than competing for a single slot. diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 589742ab63..39fe571eeb 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -633,8 +633,10 @@ settings: # output, or existing PR labels, only ever copied from a linked/closing issue ("Fixes #123") that # ALREADY carries the configured issue label. Generic beyond the priority use case: any issue label # can map to any PR label. `removeOtherTypeLabels: true` marks the mapping EXCLUSIVE -- it REPLACES - # the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; only - # the FIRST-configured exclusive match wins when more than one applies. `false` (e.g. `gittensor:priority`, + # the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; when + # more than one applies, the LAST-configured exclusive match wins -- declare exclusive mappings in + # ASCENDING precedence order (lowest-value category first, e.g. bug before feature) so the highest- + # precedence one you actually want is declared last. `false` (e.g. `gittensor:priority`, # which is a reward tag that coexists WITH whichever type already applies, not a type of its own) applies # the mapped label ADDITIVELY alongside whichever exclusive match (or the normal title-based bug/feature # label) already won -- every additive match composes together rather than competing for a single slot. diff --git a/packages/gittensory-engine/src/settings/pr-type-label.ts b/packages/gittensory-engine/src/settings/pr-type-label.ts index bbcc5ffdc1..1e494668c3 100644 --- a/packages/gittensory-engine/src/settings/pr-type-label.ts +++ b/packages/gittensory-engine/src/settings/pr-type-label.ts @@ -114,8 +114,9 @@ export type PrTypeLabelDecision = { /** * Resolve the TYPE label decision for a PR. * 1. Linked-issue label PROPAGATION (config-driven, #priority-linked-issue-gate): when enabled, the - * FIRST configured mapping whose `issueLabel` appears (case-insensitively) among the - * ALREADY-FETCHED `linkedIssueLabels` wins. This is the ONLY way a label like `gittensor:priority` + * LAST configured EXCLUSIVE mapping whose `issueLabel` appears (case-insensitively) among the + * ALREADY-FETCHED `linkedIssueLabels` wins (#5385 -- declare exclusive mappings in ascending + * precedence order). This is the ONLY way a label like `gittensor:priority` * can ever be chosen — this function does no I/O and never infers it from title, changed files, * AI output, or PR labels; the caller must fetch `linkedIssueLabels` itself (see * `fetchLinkedIssueLabelsForPropagation` in `review/linked-issue-label-propagation-fetch.ts`). @@ -152,19 +153,24 @@ export function resolvePrTypeLabel(input: { if (input.propagation?.enabled) { const wanted = new Set((input.linkedIssueLabels ?? []).map((label) => label.toLowerCase())); // Collect EVERY mapping the linked issue's labels satisfy, not just the first. An exclusive mapping - // (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) still only - // ever lets the FIRST-configured match win, same precedence as before. But an additive mapping (e.g. - // priority -- a maintainer-hand-picked reward tag that coexists WITH whichever type already applies, not a - // type of its own) must compose with that winner instead of being skipped just because an earlier mapping - // in the array already matched and returned. Before this, an additive match was unreachable whenever the - // SAME linked issue also carried a label an earlier (exclusive) mapping matched -- the overwhelmingly common - // case for gittensor:priority, which is applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never - // instead of it (#priority-linked-issue-gate). + // (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) lets the + // LAST-configured match win, not the first (#5385 fix -- was first-match-wins, which meant a linked issue + // carrying BOTH gittensor:bug and gittensor:feature always resolved to bug, the lower-value label, purely + // because bug is declared before feature in `.gittensory.yml`). Operators must declare exclusive mappings + // in ASCENDING precedence order (lowest-value category first, e.g. bug then feature) so the last match + // encountered while iterating is the highest-precedence one that actually applies -- this mirrors the + // repo's own default mapping order, which is already bug/feature/priority (ascending multiplier value). + // An additive mapping (e.g. priority -- a maintainer-hand-picked reward tag that coexists WITH whichever + // type already applies, not a type of its own) must compose with that winner instead of being skipped just + // because an earlier mapping in the array already matched. Before the original #priority-linked-issue-gate + // composition fix, an additive match was unreachable whenever the SAME linked issue also carried a label an + // earlier (exclusive) mapping matched -- the overwhelmingly common case for gittensor:priority, which is + // applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never instead of it. let exclusiveMatch: LinkedIssueLabelPropagationMapping | undefined; const additiveMatches: LinkedIssueLabelPropagationMapping[] = []; for (const mapping of input.propagation.mappings) { if (!wanted.has(mapping.issueLabel.toLowerCase())) continue; - if (mapping.removeOtherTypeLabels) exclusiveMatch ??= mapping; + if (mapping.removeOtherTypeLabels) exclusiveMatch = mapping; else additiveMatches.push(mapping); } if (exclusiveMatch || additiveMatches.length > 0) { diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 203db9565b..407e12ad20 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -3239,16 +3239,20 @@ export async function fetchLivePullRequestMergedAt( 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. */ +/** Verifies whether GitHub attributes this issue's closure to the specific PR, via GraphQL's `ClosedEvent.closer` + * field -- the one place GitHub's API actually records "what closed this issue" (a `PullRequest` or a `Commit`). + * Timestamp ordering alone only proves the issue closed after the PR merged; `closer` binds the closure to THIS + * PR and prevents borrowing labels from an unrelated issue that happened to close later (#4528). + * + * #5385 (production incident): a prior version of this check read REST's `GET /issues/{n}/timeline` and looked + * for a `closed`-type event carrying a `source.issue` field matching this PR. That field NEVER appears on a + * `closed` event -- confirmed against three live production examples -- it only exists on `cross-referenced` + * events, a structurally different type. The REST check therefore always returned `not_closed_by_pull_request`, + * even for a completely legitimate same-PR close, silently converting every "Closes #N" merge into a title- + * heuristic mislabel. `closer` is GraphQL-only; there is no equivalent REST field to fall back to. + * + * `last: 1` deliberately reads only the MOST RECENT closing event, not history: an issue that was closed, + * reopened, and closed again by something else must be judged on its current closer, not a superseded one. */ export async function fetchLinkedIssueClosedByPullRequest( env: Env, repoFullName: string, @@ -3257,11 +3261,24 @@ export async function fetchLinkedIssueClosedByPullRequest( token: string | undefined, admissionKey?: GitHubRateLimitAdmissionKey, ): Promise { - 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 (!token) return "fetch_error"; + const { owner, name } = repoParts(repoFullName); + if (!owner || !name) return "fetch_error"; + const query = `query GittensoryIssueCloser { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { issue(number: ${issueNumber}) { timelineItems(last: 1, itemTypes: [CLOSED_EVENT]) { nodes { __typename ... on ClosedEvent { closer { __typename ... on PullRequest { number } } } } } } } }`; + const result = await githubGraphQl<{ + data?: { + repository?: { + issue?: { timelineItems?: { nodes?: Array<{ closer?: { __typename?: string | null; number?: number | null } | null } | null> | null } | null } | null; + } | null; + }; + errors?: unknown[]; + }>(env, query, token, 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"; + if (Array.isArray(result.errors) && result.errors.length > 0) return "fetch_error"; + const nodes = result.data?.repository?.issue?.timelineItems?.nodes; + if (!Array.isArray(nodes)) return "fetch_error"; + const closer = nodes[0]?.closer; + return closer?.__typename === "PullRequest" && closer.number === 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} diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 58de1a6252..07b136551b 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -4086,7 +4086,14 @@ async function maybeReReviewOnLinkedIssueChange( const installationId = getInstallationId(payload); const issueNumber = payload.issue?.number; if (!repoFullName || !installationId || !issueNumber) return false; - if (isConvergenceRepoAllowed(env, repoFullName)) { + // #5385: mirrors sweepRepoRegate's own gate exactly -- a repo with acting autonomy configured but NOT in the + // GITTENSORY_REVIEW_REPOS allowlist (e.g. removed during a rollback, or a self-hoster who configured autonomy + // without also updating the env allowlist) used to silently never wake affected PRs here, leaving a stale + // type label (or any other issue-driven verdict) until the sweep eventually reached it, cycles later. + // Short-circuited deliberately: resolveRepositorySettings does a live manifest fetch, so the allowlisted + // common case (this repo is already in GITTENSORY_REVIEW_REPOS) must never pay for it -- this handler's own + // doc comment promises "never doing the expensive live re-review inline", and that includes this gate check. + if (isConvergenceRepoAllowed(env, repoFullName) || isAgentConfigured((await resolveRepositorySettings(env, repoFullName)).autonomy)) { const openPullRequests = await listOpenPullRequests(env, repoFullName); // Issue-side label/assignment changes can flip linked-issue hard-rule verdicts from mergeable to close. // Wake affected PRs promptly: the issue-side signal can invalidate public gate state, so dropping every diff --git a/src/settings/pr-type-label.ts b/src/settings/pr-type-label.ts index 454ca208ec..7e5d538b5f 100644 --- a/src/settings/pr-type-label.ts +++ b/src/settings/pr-type-label.ts @@ -114,8 +114,9 @@ export type PrTypeLabelDecision = { /** * Resolve the TYPE label decision for a PR. * 1. Linked-issue label PROPAGATION (config-driven, #priority-linked-issue-gate): when enabled, the - * FIRST configured mapping whose `issueLabel` appears (case-insensitively) among the - * ALREADY-FETCHED `linkedIssueLabels` wins. This is the ONLY way a label like `gittensor:priority` + * LAST configured EXCLUSIVE mapping whose `issueLabel` appears (case-insensitively) among the + * ALREADY-FETCHED `linkedIssueLabels` wins (#5385 -- declare exclusive mappings in ascending + * precedence order). This is the ONLY way a label like `gittensor:priority` * can ever be chosen — this function does no I/O and never infers it from title, changed files, * AI output, or PR labels; the caller must fetch `linkedIssueLabels` itself (see * `fetchLinkedIssueLabelsForPropagation` in `review/linked-issue-label-propagation-fetch.ts`). @@ -152,19 +153,24 @@ export function resolvePrTypeLabel(input: { if (input.propagation?.enabled) { const wanted = new Set((input.linkedIssueLabels ?? []).map((label) => label.toLowerCase())); // Collect EVERY mapping the linked issue's labels satisfy, not just the first. An exclusive mapping - // (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) still only - // ever lets the FIRST-configured match win, same precedence as before. But an additive mapping (e.g. - // priority -- a maintainer-hand-picked reward tag that coexists WITH whichever type already applies, not a - // type of its own) must compose with that winner instead of being skipped just because an earlier mapping - // in the array already matched and returned. Before this, an additive match was unreachable whenever the - // SAME linked issue also carried a label an earlier (exclusive) mapping matched -- the overwhelmingly common - // case for gittensor:priority, which is applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never - // instead of it (#priority-linked-issue-gate). + // (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) lets the + // LAST-configured match win, not the first (#5385 fix -- was first-match-wins, which meant a linked issue + // carrying BOTH gittensor:bug and gittensor:feature always resolved to bug, the lower-value label, purely + // because bug is declared before feature in `.gittensory.yml`). Operators must declare exclusive mappings + // in ASCENDING precedence order (lowest-value category first, e.g. bug then feature) so the last match + // encountered while iterating is the highest-precedence one that actually applies -- this mirrors the + // repo's own default mapping order, which is already bug/feature/priority (ascending multiplier value). + // An additive mapping (e.g. priority -- a maintainer-hand-picked reward tag that coexists WITH whichever + // type already applies, not a type of its own) must compose with that winner instead of being skipped just + // because an earlier mapping in the array already matched. Before the original #priority-linked-issue-gate + // composition fix, an additive match was unreachable whenever the SAME linked issue also carried a label an + // earlier (exclusive) mapping matched -- the overwhelmingly common case for gittensor:priority, which is + // applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never instead of it. let exclusiveMatch: LinkedIssueLabelPropagationMapping | undefined; const additiveMatches: LinkedIssueLabelPropagationMapping[] = []; for (const mapping of input.propagation.mappings) { if (!wanted.has(mapping.issueLabel.toLowerCase())) continue; - if (mapping.removeOtherTypeLabels) exclusiveMatch ??= mapping; + if (mapping.removeOtherTypeLabels) exclusiveMatch = mapping; else additiveMatches.push(mapping); } if (exclusiveMatch || additiveMatches.length > 0) { diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index e408f432dc..eafdd77dfb 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -37,6 +37,7 @@ import { enqueueRepositoryOpenDataBackfill, enrichInstallationHealth, fetchAndStorePullRequestFilesForReview, + fetchLinkedIssueClosedByPullRequest, fetchLinkedIssueFacts, fetchLiveBaseBranchAdvancedAt, fetchLiveCiAggregate, @@ -4515,3 +4516,117 @@ describe("GitHub backfill", () => { // empty (the PR-opened webhook beat the async detail-sync), so the FIRST AI review/grounding/comment sees // the real diff instead of "0 files". }); + +// #5385: fetchLinkedIssueClosedByPullRequest was rewritten from a REST /issues/{n}/timeline read (which could +// never actually succeed -- GitHub's real "closed" timeline event carries no source.issue field, only +// cross-referenced events do) to GraphQL's Issue.timelineItems -> ClosedEvent.closer. These tests pin every +// branch of the new implementation, including the guard clauses and malformed-response shapes that didn't +// exist in the old REST version at all. +describe("fetchLinkedIssueClosedByPullRequest (#5385)", () => { + afterEach(() => vi.unstubAllGlobals()); + + function closerBody(closer: { typename: string; number?: number } | null): unknown { + return { + data: { + repository: { + issue: { + timelineItems: { + nodes: closer === null ? [] : [{ __typename: "ClosedEvent", closer: { __typename: closer.typename, ...(closer.number !== undefined ? { number: closer.number } : {}) } }], + }, + }, + }, + }, + }; + } + + it("returns fetch_error without ever calling fetch when no token is available", async () => { + const env = createTestEnv({}); + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + const result = await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, undefined); + expect(result).toBe("fetch_error"); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("returns fetch_error without ever calling fetch for a malformed repoFullName (no owner/name split)", async () => { + const env = createTestEnv({}); + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + expect(await fetchLinkedIssueClosedByPullRequest(env, "", 100, 200, "test-token")).toBe("fetch_error"); + expect(await fetchLinkedIssueClosedByPullRequest(env, "no-slash-repo", 100, 200, "test-token")).toBe("fetch_error"); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("returns fetch_error when GraphQL responds 200 OK with a top-level errors array (GitHub's REAL response shape for an unresolvable issue number, confirmed via gh api graphql)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" + ? Response.json({ data: { repository: { issue: null } }, errors: [{ type: "NOT_FOUND", message: "Could not resolve to an issue." }] }) + : new Response("not found", { status: 404 }), + ); + const result = await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 999999, 200, "test-token"); + expect(result).toBe("fetch_error"); + }); + + it("returns fetch_error when timelineItems.nodes is missing/non-array (malformed response, no top-level errors)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" + ? Response.json({ data: { repository: { issue: { timelineItems: {} } } } }) + : new Response("not found", { status: 404 }), + ); + const result = await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token"); + expect(result).toBe("fetch_error"); + }); + + it("returns fetch_error when the GraphQL request itself throws (network failure)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async () => { + throw new Error("network down"); + }); + const result = await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token"); + expect(result).toBe("fetch_error"); + }); + + it("returns closed_by_pull_request when the closer is a matching PullRequest", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" ? Response.json(closerBody({ typename: "PullRequest", number: 200 })) : new Response("not found", { status: 404 }), + ); + expect(await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token")).toBe("closed_by_pull_request"); + }); + + it("returns not_closed_by_pull_request when the closer is a DIFFERENT PullRequest (anti-spoofing)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" ? Response.json(closerBody({ typename: "PullRequest", number: 999 })) : new Response("not found", { status: 404 }), + ); + expect(await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token")).toBe("not_closed_by_pull_request"); + }); + + it("returns not_closed_by_pull_request when the issue was closed manually with no closer at all (confirmed live shape via gh api graphql against JSONbored/gittensory#5130: closer: null)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" + ? Response.json({ data: { repository: { issue: { timelineItems: { nodes: [{ __typename: "ClosedEvent", closer: null }] } } } } }) + : new Response("not found", { status: 404 }), + ); + expect(await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token")).toBe("not_closed_by_pull_request"); + }); + + it("returns not_closed_by_pull_request when the closer is a Commit, not a PullRequest (issue closed via a commit message reference)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" ? Response.json(closerBody({ typename: "Commit" })) : new Response("not found", { status: 404 }), + ); + expect(await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token")).toBe("not_closed_by_pull_request"); + }); + + it("returns not_closed_by_pull_request when there is no CLOSED_EVENT at all (nodes is an empty array)", async () => { + const env = createTestEnv({}); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => + input.toString() === "https://api.github.com/graphql" ? Response.json(closerBody(null)) : new Response("not found", { status: 404 }), + ); + expect(await fetchLinkedIssueClosedByPullRequest(env, "owner/repo", 100, 200, "test-token")).toBe("not_closed_by_pull_request"); + }); +}); diff --git a/test/unit/linked-issue-label-propagation-fetch.test.ts b/test/unit/linked-issue-label-propagation-fetch.test.ts index 50b4ed503f..6511c917a6 100644 --- a/test/unit/linked-issue-label-propagation-fetch.test.ts +++ b/test/unit/linked-issue-label-propagation-fetch.test.ts @@ -38,6 +38,29 @@ function expectPropagation(result: LinkedIssuePropagationLabels, labels: string[ expect(result).toEqual({ labels, inconclusive }); } +const GRAPHQL_URL = "https://api.github.com/graphql"; + +/** GraphQL `Issue.timelineItems -> ClosedEvent.closer` response shape, matching exactly what + * `fetchLinkedIssueClosedByPullRequest` (src/github/backfill.ts) parses -- built from a REAL `gh api graphql` + * response captured against a live PR-merge-closed issue (#5385), not an invented shape. A prior REST- + * `/issues/{n}/timeline`-based version of this check assumed a `source.issue` field that GitHub's REST + * Timeline API never actually populates on a `closed` event -- confirmed empirically -- which made every + * legitimate same-PR close silently fail closure verification in production. `closerPrNumber: null` mirrors + * an issue with no CLOSED_EVENT in its last-item window (still open, or closed by neither a PR nor a commit). */ +function closerGraphQlBody(closerPrNumber: number | null): unknown { + return { + data: { + repository: { + issue: { + timelineItems: { + nodes: closerPrNumber === null ? [] : [{ __typename: "ClosedEvent", closer: { __typename: "PullRequest", number: closerPrNumber } }], + }, + }, + }, + }, + }; +} + describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", () => { afterEach(() => { vi.unstubAllGlobals(); @@ -300,10 +323,13 @@ 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: {} } } }]); + if (url === GRAPHQL_URL) return Response.json(closerGraphQlBody(4494)); return new Response("not found", { status: 404 }); }); - const env = createTestEnv({}); + // GITHUB_PUBLIC_TOKEN: the closure-verification GraphQL call (#5385) requires a real token to + // authenticate at all (unlike REST, which can fall back to an unauthenticated read) -- createTestEnv({}) + // has no signable GITHUB_APP_PRIVATE_KEY, so createInstallationToken fails and this is the fallback. + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); const result = await fetchLinkedIssueLabelsForPropagation({ env, repoFullName: "owner/repo", @@ -327,10 +353,12 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( 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: {} } } }]); + // Closer is PR #123, not the PR calling in (#4494) -- proves the spoof protection still rejects an + // unrelated issue that happened to close (by something else) after this PR merged. + if (url === GRAPHQL_URL) return Response.json(closerGraphQlBody(123)); return new Response("not found", { status: 404 }); }); - const env = createTestEnv({}); + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); const result = await fetchLinkedIssueLabelsForPropagation({ env, repoFullName: "owner/repo", @@ -370,10 +398,10 @@ describe("fetchLinkedIssueLabelsForPropagation (#priority-linked-issue-gate)", ( 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 }); + if (url === GRAPHQL_URL) return new Response("server error", { status: 500 }); return new Response("not found", { status: 404 }); }); - const env = createTestEnv({}); + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); const result = await fetchLinkedIssueLabelsForPropagation({ env, repoFullName: "owner/repo", @@ -444,10 +472,10 @@ 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: {} } } }]); + if (url === GRAPHQL_URL) return Response.json(closerGraphQlBody(4818)); return new Response("not found", { status: 404 }); }); - const env = createTestEnv({}); + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); const result = await fetchLinkedIssueLabelsForPropagation({ env, repoFullName: "owner/repo", diff --git a/test/unit/pr-type-label-engine.test.ts b/test/unit/pr-type-label-engine.test.ts index 56a0f12e4e..3552a46433 100644 --- a/test/unit/pr-type-label-engine.test.ts +++ b/test/unit/pr-type-label-engine.test.ts @@ -115,7 +115,7 @@ describe("resolvePrTypeLabel (#priority-linked-issue-gate)", () => { expect(result.source).toBe("title"); }); - it("resolves the FIRST matching mapping when multiple linked-issue labels are present", () => { + it("resolves the LAST matching exclusive mapping (highest declared precedence) when multiple linked-issue labels are present (#5385)", () => { const result = resolvePrTypeLabel({ title: "fix: y", linkedIssueLabels: ["customer:vip", "gittensor:priority"], @@ -126,7 +126,7 @@ describe("resolvePrTypeLabel (#priority-linked-issue-gate)", () => { ], }), }); - expect(result.applyLabels).toEqual(["triage:vip"]); + expect(result.applyLabels).toEqual(["gittensor:priority"]); expect(result.source).toBe("propagation_exclusive"); }); diff --git a/test/unit/pr-type-label.test.ts b/test/unit/pr-type-label.test.ts index 5f27b6223e..fc305dd416 100644 --- a/test/unit/pr-type-label.test.ts +++ b/test/unit/pr-type-label.test.ts @@ -108,7 +108,7 @@ describe("resolvePrTypeLabel (#priority-linked-issue-gate)", () => { expect(result.source).toBe("title"); }); - it("resolves the FIRST matching mapping when multiple linked-issue labels are present", () => { + it("resolves the LAST matching exclusive mapping (highest declared precedence) when multiple linked-issue labels are present (#5385)", () => { const result = resolvePrTypeLabel({ title: "fix: y", linkedIssueLabels: ["customer:vip", "gittensor:priority"], @@ -119,7 +119,7 @@ describe("resolvePrTypeLabel (#priority-linked-issue-gate)", () => { ], }), }); - expect(result.applyLabels).toEqual(["triage:vip"]); + expect(result.applyLabels).toEqual(["gittensor:priority"]); expect(result.source).toBe("propagation_exclusive"); }); @@ -172,14 +172,15 @@ describe("resolvePrTypeLabel (#priority-linked-issue-gate)", () => { expect(result.source).toBe("propagation_exclusive"); }); - it("two exclusive candidates + an additive one: only the FIRST-configured exclusive mapping wins, additive still composes", () => { + it("two exclusive candidates + an additive one: only the LAST-configured (highest-precedence) exclusive mapping wins, additive still composes (#5385)", () => { const result = resolvePrTypeLabel({ title: "fix: y", linkedIssueLabels: ["gittensor:bug", "gittensor:feature", "gittensor:priority"], propagation: propagation({ mappings: bugFeaturePriorityMappings }), }); - // bug is configured before feature, so bug wins the exclusive slot even though both matched. - expect(result).toEqual({ applyLabels: ["gittensor:bug", "gittensor:priority"], removeLabels: ["gittensor:feature"], source: "propagation_exclusive" }); + // feature is configured after bug (ascending precedence), so feature wins the exclusive slot even + // though both matched -- matching feature's higher scoring weight than bug. + expect(result).toEqual({ applyLabels: ["gittensor:feature", "gittensor:priority"], removeLabels: ["gittensor:bug"], source: "propagation_exclusive" }); }); }); diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index cc2eff6834..4065ede074 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -6222,11 +6222,13 @@ 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: {} } } }]); + // #4528/#5385 closure 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. Verified via GraphQL's `ClosedEvent.closer`, matching fetchLinkedIssueClosedByPullRequest's + // real query shape (src/github/backfill.ts) -- REST's Timeline API has no equivalent field. + if (url === "https://api.github.com/graphql") { + return Response.json({ data: { repository: { issue: { timelineItems: { nodes: [{ __typename: "ClosedEvent", closer: { __typename: "PullRequest", number: prNumber } }] } } } } }); } if (url.includes(`/issues/${prNumber}/labels`) && method === "GET") return Response.json([]); if (url.includes(`/issues/${prNumber}/labels`) && method === "POST") { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index ca43d24b73..8df32f3fa5 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3275,17 +3275,73 @@ describe("queue processors", () => { expect(checkRunsFetched).toBe(false); // PR #7 links #99, not #1 — never re-reviewed }); - it("issue label change is dormant on a repo outside the GITTENSORY_REVIEW_REPOS convergence allowlist", async () => { - const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_REPOS: "" }); + it("issue label change wakes the linked PR via the acting-autonomy fallback even OUTSIDE the GITTENSORY_REVIEW_REPOS convergence allowlist, mirroring sweepRepoRegate's own gate (#5385)", async () => { + // Before #5385, this exact shape (real acting autonomy, but not in the env allowlist) silently stayed + // dormant here even though the periodic sweep (sweepRepoRegate) would already treat this repo as eligible + // via `isConvergenceRepoAllowed || isAgentConfigured(settings.autonomy)` — a self-hoster who configures + // autonomy without also updating the allowlist (or a repo removed from the allowlist during a rollback) + // would see a stale type label / hard-rule verdict until the sweep eventually reached it, cycles later. + const sent: Array<{ message: import("../../src/types").JobMessage; options?: QueueSendOptions }> = []; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_REPOS: "", + JOBS: { + async send(message: import("../../src/types").JobMessage, options?: QueueSendOptions) { + sent.push(options ? { message, options } : { message }); + }, + } as unknown as Queue, + }); await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", reviewCheckMode: "required", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1", created_at: "2026-07-03T10:00:00.000Z" }); - let checkRunsFetched = false; vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); - checkRunsFetched ||= url.includes("/commits/a7/check-runs"); if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + // The acting-autonomy fallback resolves settings via resolveRepositorySettings, which reads the repo's + // manifest file (unlike the allowlisted common case, which never fetches at all) — a 404 here is the + // realistic "no .gittensory.yml override" response, not a stub gap. + if (url.includes("/contents/")) return new Response("not found", { status: 404 }); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "issue-label-fallback-wake", + eventName: "issues", + payload: { + action: "labeled", + repository: { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, + installation: { id: 9001 }, + issue: { number: 1, title: "Issue", state: "open", labels: [{ name: "maintainer-only" }] }, + label: { name: "maintainer-only" }, + } as never, + }); + + expect(sent).toEqual([ + { message: expect.objectContaining({ type: "agent-regate-pr", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001, prCreatedAt: "2026-07-03T10:00:00.000Z" }) }, + ]); + const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("issue-label-fallback-wake").first<{ status: string }>(); + expect(webhookRow?.status).toBe("processed"); + }); + + it("issue label change stays dormant when the repo is neither in the convergence allowlist NOR has any acting autonomy configured (#5385 — genuinely inert repos are still untouched)", async () => { + const sent: import("../../src/types").JobMessage[] = []; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_REPOS: "", + JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue, + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + // Every autonomy class left at the deny-by-default "observe" floor — isAgentConfigured is false, same as + // isConvergenceRepoAllowed, so this repo has genuinely opted into nothing. + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { review: "observe" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", reviewCheckMode: "required", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Linking PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/contents/")) return new Response("not found", { status: 404 }); return Response.json({}); }); @@ -3302,7 +3358,7 @@ describe("queue processors", () => { } as never, }); - expect(checkRunsFetched).toBe(false); // dormant default: not in the convergence allowlist + expect(sent).toEqual([]); // dormant: neither allowlisted nor acting-autonomy-configured const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("issue-label-not-converged").first<{ status: string }>(); expect(webhookRow?.status).toBe("processed"); // still marked handled — only the re-review work is skipped });