From 621f60538e36279c43be7be9b44111c5b8e4b046 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:26:35 +0200 Subject: [PATCH 1/6] feat(enrichment): PR history analyzer + engine context passthrough Add REES history analyzer for author track record and linked-issue alignment. Pass author, body, and installation token through enrichment-wire so webhook reviews can run historical analysis during the AI review path. Fixes #1697 Co-authored-by: Cursor --- review-enrichment/src/analyzers/history.ts | 185 +++++++++++++++++++++ review-enrichment/src/brief.ts | 2 + review-enrichment/src/render.ts | 22 +++ review-enrichment/src/types.ts | 25 +++ review-enrichment/test/history.test.ts | 106 ++++++++++++ src/queue/processors.ts | 9 + src/review/enrichment-wire.ts | 21 +++ test/unit/enrichment-wire.test.ts | 52 ++++++ test/unit/enrichment-wiring.test.ts | 11 ++ 9 files changed, 433 insertions(+) create mode 100644 review-enrichment/src/analyzers/history.ts create mode 100644 review-enrichment/test/history.test.ts diff --git a/review-enrichment/src/analyzers/history.ts b/review-enrichment/src/analyzers/history.ts new file mode 100644 index 0000000000..4c37c3709d --- /dev/null +++ b/review-enrichment/src/analyzers/history.ts @@ -0,0 +1,185 @@ +// History analyzer (#1697 / #1478). Uses PR metadata + optional GitHub API access to surface author track +// record and linked-issue alignment — context the diff-only reviewer cannot infer. Fail-safe: returns a +// degraded finding set when no token is available (linked-issue parsing still runs on the body). +import type { EnrichRequest, HistoryFinding, LinkedIssueFinding } from "../types.js"; + +const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; +const MAX_LINKED_ISSUES = 8; +const NEWCOMER_MERGED_THRESHOLD = 3; +const MAX_BODY_CHARS = 8000; + +const LINKED_ISSUE_RE = + /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)\s*)?#(\d+)\b/gi; + +/** Parse `Fixes #123` / `Closes org/repo#456` references from the PR body. Pure. */ +export function extractLinkedIssues( + body: string | undefined, + defaultRepo: string, +): Array<{ repo: string; number: number }> { + const text = (body ?? "").slice(0, MAX_BODY_CHARS); + const seen = new Set(); + const linked: Array<{ repo: string; number: number }> = []; + for (const match of text.matchAll(LINKED_ISSUE_RE)) { + const owner = match[1]; + const repo = match[2]; + const number = Number(match[3]); + if (!Number.isFinite(number) || number <= 0) continue; + const repoFullName = + owner && repo ? `${owner}/${repo}` : defaultRepo; + const key = `${repoFullName}#${number}`; + if (seen.has(key)) continue; + seen.add(key); + linked.push({ repo: repoFullName, number }); + if (linked.length >= MAX_LINKED_ISSUES) break; + } + return linked; +} + +function parseRepoParts(repoFullName: string): { owner: string; repo: string } | null { + const [owner, repo] = repoFullName.split("/"); + if (!owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return null; + return { owner, repo }; +} + +async function fetchJson( + url: string, + headers: Record, + fetchFn: typeof fetch, + signal?: AbortSignal, +): Promise { + try { + const resp = await fetchFn(url, { headers, signal }); + if (!resp.ok) return null; + return (await resp.json()) as T; + } catch { + return null; + } +} + +/** Count merged PRs by the author in this repo (search API, bounded). */ +export async function fetchAuthorMergedCount( + repoFullName: string, + author: string, + githubToken: string, + fetchFn: typeof fetch, + signal?: AbortSignal, +): Promise { + const parts = parseRepoParts(repoFullName); + if (!parts || !SLUG_RE.test(author.replace(/^@/, ""))) return null; + const q = encodeURIComponent( + `repo:${repoFullName} author:${author.replace(/^@/, "")} is:pr is:merged`, + ); + const headers = { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; + const payload = await fetchJson<{ total_count?: number }>( + `https://api.github.com/search/issues?q=${q}&per_page=1`, + headers, + fetchFn, + signal, + ); + return typeof payload?.total_count === "number" ? payload.total_count : null; +} + +/** Fetch issue state/title for a linked reference. */ +async function fetchLinkedIssue( + repo: string, + number: number, + headers: Record, + fetchFn: typeof fetch, + signal?: AbortSignal, +): Promise { + const parts = parseRepoParts(repo); + if (!parts) { + return { + number, + repo, + state: null, + title: null, + aligned: false, + }; + } + const payload = await fetchJson<{ state?: string; title?: string }>( + `https://api.github.com/repos/${encodeURIComponent(parts.owner)}/${encodeURIComponent(parts.repo)}/issues/${number}`, + headers, + fetchFn, + signal, + ); + const state = payload?.state ?? null; + return { + number, + repo, + state, + title: payload?.title ?? null, + aligned: state === "open" || state === "closed", + }; +} + +export function classifyAuthorTier( + mergedCount: number | null, +): HistoryFinding["authorTier"] { + if (mergedCount === null) return "unknown"; + return mergedCount < NEWCOMER_MERGED_THRESHOLD ? "newcomer" : "established"; +} + +/** Analyzer entrypoint: linked issues + optional author history via GitHub API. */ +export async function scanHistory( + req: EnrichRequest, + fetchFn: typeof fetch = fetch, + opts?: { signal?: AbortSignal }, +): Promise { + const author = req.author?.replace(/^@/, "") ?? ""; + if (!author) return null; + + const linkedRefs = extractLinkedIssues(req.body, req.repoFullName); + let mergedPrCount: number | null = null; + const linkedIssues: LinkedIssueFinding[] = []; + + if (req.githubToken) { + const headers = { + Authorization: `Bearer ${req.githubToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; + mergedPrCount = await fetchAuthorMergedCount( + req.repoFullName, + author, + req.githubToken, + fetchFn, + opts?.signal, + ); + for (const ref of linkedRefs) { + linkedIssues.push( + await fetchLinkedIssue(ref.repo, ref.number, headers, fetchFn, opts?.signal), + ); + } + } else { + for (const ref of linkedRefs) { + linkedIssues.push({ + number: ref.number, + repo: ref.repo, + state: null, + title: null, + aligned: true, + }); + } + } + + if (!linkedIssues.length && mergedPrCount === null) { + return { + authorLogin: author, + mergedPrCount: null, + authorTier: "unknown", + linkedIssues: [], + }; + } + + return { + authorLogin: author, + mergedPrCount, + authorTier: classifyAuthorTier(mergedPrCount), + linkedIssues, + }; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index ccb30f107b..14e19b12c9 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -16,6 +16,7 @@ import { scanEol } from "./analyzers/eol-check.js"; import { scanRedos } from "./analyzers/redos.js"; import { scanCodeowners } from "./analyzers/codeowners.js"; import { scanSecretLog } from "./analyzers/secret-log.js"; +import { scanHistory } from "./analyzers/history.js"; import { renderBrief } from "./render.js"; type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise; @@ -31,6 +32,7 @@ const ANALYZERS: Record = { redos: (req) => scanRedos(req), codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }), secretLog: (req, signal) => scanSecretLog(req, signal), + history: (req, signal) => scanHistory(req, fetch, { signal }), }; function runWithTimeout( diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 5270f795e0..c097378865 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -160,6 +160,28 @@ export function renderBrief( } } + const history = findings.history; + if (history) { + lines.push("### Author history & linked issues"); + const countLabel = + history.mergedPrCount === null + ? "unknown merged PR count" + : `${history.mergedPrCount} merged PR(s) in this repo`; + lines.push( + `- Author ${safeCodeSpan(history.authorLogin)} — **${history.authorTier}** (${countLabel})`, + ); + for (const issue of history.linkedIssues) { + const state = issue.state ?? "unknown state"; + const title = issue.title ? ` — ${promptText(issue.title.slice(0, 120))}` : ""; + lines.push( + `- Linked ${safeCodeSpan(`${issue.repo}#${issue.number}`)} (${state})${title}`, + ); + } + if (!history.linkedIssues.length) { + lines.push("- No linked issues detected in the PR body"); + } + } + if (!lines.length) return { promptSection: "", systemSuffix: "" }; const header = diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 0136d320af..3207a58c9d 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -109,6 +109,30 @@ export interface SecretLogFinding { category: "secret" | "pii" | "request-object"; } +/** A revert/regression signal: explicit revert language or symmetric churn that mirrors undoing prior work. */ +export interface RevertRecurrenceFinding { + kind: "explicit-revert" | "rollback-language" | "symmetric-churn"; + detail: string; + files?: string[]; + confidence: "high" | "medium"; +} + +/** Author track record + linked-issue alignment for historical review context. */ +export interface LinkedIssueFinding { + number: number; + repo: string; + state: string | null; + title: string | null; + aligned: boolean; +} + +export interface HistoryFinding { + authorLogin: string; + mergedPrCount: number | null; + authorTier: "newcomer" | "established" | "unknown"; + linkedIssues: LinkedIssueFinding[]; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -120,6 +144,7 @@ export interface BriefFindings { redos?: RedosFinding[]; codeowners?: CodeownersFinding[]; secretLog?: SecretLogFinding[]; + history?: HistoryFinding | null; } export type AnalyzerStatus = "ok" | "degraded" | "skipped"; diff --git a/review-enrichment/test/history.test.ts b/review-enrichment/test/history.test.ts new file mode 100644 index 0000000000..0c0b40aa22 --- /dev/null +++ b/review-enrichment/test/history.test.ts @@ -0,0 +1,106 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { + classifyAuthorTier, + extractLinkedIssues, + fetchAuthorMergedCount, + scanHistory, +} from "../src/analyzers/history.ts"; + +describe("history analyzer", () => { + it("extracts linked issues from PR body with default and explicit repos", () => { + assert.deepEqual( + extractLinkedIssues("Fixes #12 and closes #34", "org/app"), + [ + { repo: "org/app", number: 12 }, + { repo: "org/app", number: 34 }, + ], + ); + assert.deepEqual( + extractLinkedIssues("Closes other/repo#99", "org/app"), + [{ repo: "other/repo", number: 99 }], + ); + }); + + it("classifies author tiers from merged PR counts", () => { + assert.equal(classifyAuthorTier(null), "unknown"); + assert.equal(classifyAuthorTier(0), "newcomer"); + assert.equal(classifyAuthorTier(2), "newcomer"); + assert.equal(classifyAuthorTier(3), "established"); + assert.equal(classifyAuthorTier(40), "established"); + }); + + it("fetchAuthorMergedCount reads search total_count", async () => { + const fetchFn = async () => + ({ + ok: true, + json: async () => ({ total_count: 7 }), + }) as Response; + assert.equal( + await fetchAuthorMergedCount( + "org/app", + "dev1", + "token", + fetchFn as typeof fetch, + ), + 7, + ); + }); + + it("scanHistory degrades gracefully without a token but still parses links", async () => { + const result = await scanHistory({ + repoFullName: "org/app", + prNumber: 5, + author: "dev1", + body: "Fixes #42", + }); + assert.ok(result); + assert.equal(result!.authorLogin, "dev1"); + assert.equal(result!.authorTier, "unknown"); + assert.deepEqual(result!.linkedIssues, [ + { + number: 42, + repo: "org/app", + state: null, + title: null, + aligned: true, + }, + ]); + }); + + it("scanHistory fetches author history and issue state when token present", async () => { + const calls: string[] = []; + const fetchFn = (async (url: string) => { + calls.push(url); + if (url.includes("/search/issues")) { + return { + ok: true, + json: async () => ({ total_count: 1 }), + } as Response; + } + if (url.includes("/issues/7")) { + return { + ok: true, + json: async () => ({ state: "open", title: "Bug in parser" }), + } as Response; + } + return { ok: false, json: async () => ({}) } as Response; + }) as typeof fetch; + + const result = await scanHistory( + { + repoFullName: "org/app", + prNumber: 9, + author: "dev1", + body: "Fixes #7", + githubToken: "ghs_test", + }, + fetchFn, + ); + assert.equal(result!.authorTier, "newcomer"); + assert.equal(result!.linkedIssues[0]?.state, "open"); + assert.match(result!.linkedIssues[0]?.title ?? "", /Bug in parser/); + assert.ok(calls.some((u) => u.includes("/search/issues"))); + assert.ok(calls.some((u) => u.includes("/issues/7"))); + }); +}); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 83b6d98e2d..6a85387e85 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -325,6 +325,7 @@ import { buildReviewRagContext, isRagEnabled } from "../review/rag-wire"; import { buildReviewEnrichment, isEnrichmentEnabled, + resolveEnrichmentGithubToken, } from "../review/enrichment-wire"; import { captureReviewFailure } from "../selfhost/sentry"; import { evaluateWithSurfaceLane } from "../review/content-lane-wire"; @@ -3733,6 +3734,11 @@ export async function runAiReviewForAdvisory( // its public-safe brief splices into the prompt next to grounding + RAG. Flag-OFF (default) → no call, no branch, // byte-identical prompt. Fully fail-safe (any timeout/error/empty → undefined → review proceeds). const enrichmentDiff = buildAiReviewDiff(files); + const enrichmentInstallationId = + (await getRepository(env, args.repoFullName))?.installationId ?? null; + const enrichmentGithubToken = isEnrichmentEnabled(env) + ? await resolveEnrichmentGithubToken(env, enrichmentInstallationId) + : undefined; const enrichment = isEnrichmentEnabled(env) && convergedRepoAllowed ? await buildReviewEnrichment(env, { @@ -3740,6 +3746,9 @@ export async function runAiReviewForAdvisory( prNumber: args.pr.number, headSha: args.advisory.headSha, title: args.pr.title, + body: args.pr.body ?? undefined, + author: args.author ?? undefined, + githubToken: enrichmentGithubToken, files, diff: enrichmentDiff, }) diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index 4ec00e5624..1804008d95 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -8,6 +8,7 @@ // network / parse error, or an empty brief, returns undefined and the review proceeds on diff + grounding + RAG. import { sanitizePublicComment } from "../queue-intelligence"; import { neutralizePromptInjection } from "./prompt-injection"; +import { createInstallationToken } from "../github/app"; import type { PullRequestFileRecord } from "../types"; interface EnrichmentEnv { @@ -53,10 +54,27 @@ interface EnrichmentInput { headSha: string | null; baseSha?: string | null; title?: string | undefined; + body?: string | undefined; + author?: string | undefined; + githubToken?: string | undefined; files: PullRequestFileRecord[]; diff: string; } +/** Best-effort GitHub token for REES history/codeowners fetches — installation token, then public token. */ +export async function resolveEnrichmentGithubToken( + env: Env, + installationId: number | null | undefined, +): Promise { + if (installationId) { + const token = await createInstallationToken(env, installationId).catch( + () => undefined, + ); + if (token) return token; + } + return env.GITHUB_PUBLIC_TOKEN ?? undefined; +} + /** POST the PR to the REES and return the spliceable brief, or undefined on any error/timeout/empty (fail-safe). */ export async function buildReviewEnrichment( env: Env, @@ -81,6 +99,9 @@ export async function buildReviewEnrichment( headSha: input.headSha, baseSha: input.baseSha ?? null, title: input.title, + body: input.body, + author: input.author, + githubToken: input.githubToken, files: input.files.map((file) => ({ path: file.path, patch: diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 65e7680428..9ac40d65d6 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -2,7 +2,17 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { isEnrichmentEnabled, buildReviewEnrichment, + resolveEnrichmentGithubToken, } from "../../src/review/enrichment-wire"; +import { createInstallationToken } from "../../src/github/app"; +import { createTestEnv } from "../helpers/d1"; + +vi.mock("../../src/github/app", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, createInstallationToken: vi.fn() }; +}); + +const mockedToken = vi.mocked(createInstallationToken); const env = (o: Record) => o as unknown as Env; const input = { @@ -86,6 +96,30 @@ describe("buildReviewEnrichment", () => { ]); }); + it("POST includes author, body, and githubToken when provided", async () => { + const calls: RequestInit[] = []; + globalThis.fetch = vi.fn(async (_url: unknown, init: RequestInit) => { + calls.push(init); + return { + ok: true, + json: async () => ({ promptSection: "history brief" }), + } as Response; + }) as unknown as typeof fetch; + await buildReviewEnrichment( + env({ REES_URL: "https://rees/" }), + { + ...input, + body: "Fixes #12", + author: "dev1", + githubToken: "ghs_test", + }, + ); + const body = JSON.parse(calls[0]!.body as string); + expect(body.body).toBe("Fixes #12"); + expect(body.author).toBe("dev1"); + expect(body.githubToken).toBe("ghs_test"); + }); + it("undefined when REES_URL is unset", async () => { expect(await buildReviewEnrichment(env({}), input)).toBeUndefined(); }); @@ -208,3 +242,21 @@ describe("buildReviewEnrichment", () => { ).toBeUndefined(); }); }); + +describe("resolveEnrichmentGithubToken", () => { + it("prefers installation token and falls back to GITHUB_PUBLIC_TOKEN", async () => { + mockedToken.mockResolvedValueOnce("install-token"); + const envWithInstall = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( + "install-token", + ); + + mockedToken.mockRejectedValueOnce(new Error("no app")); + await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( + "public-token", + ); + + const bareEnv = createTestEnv({}); + await expect(resolveEnrichmentGithubToken(bareEnv, null)).resolves.toBeUndefined(); + }); +}); diff --git a/test/unit/enrichment-wiring.test.ts b/test/unit/enrichment-wiring.test.ts index 0c3b28ace5..098433eb45 100644 --- a/test/unit/enrichment-wiring.test.ts +++ b/test/unit/enrichment-wiring.test.ts @@ -84,12 +84,17 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE await seedRepoFile(env, "acme/widgets"); let reesUrl = ""; let reesAuth: string | null = null; + let reesBody: Record | null = null; const fetchSpy = vi .spyOn(globalThis, "fetch") .mockImplementation(async (url, init) => { if (String(url).includes("/v1/enrich")) { reesUrl = String(url); reesAuth = new Headers(init?.headers).get("authorization"); + reesBody = JSON.parse(String(init?.body ?? "{}")) as Record< + string, + unknown + >; return new Response( JSON.stringify({ promptSection: "## EXTERNAL REVIEW BRIEF\n- CVE-1 in lodash", @@ -116,6 +121,12 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE // The enrichment build branch executed: the REES was POSTed at /v1/enrich with the shared-secret bearer. expect(reesUrl).toBe("https://rees.example/v1/enrich"); expect(reesAuth).toBe("Bearer sek"); + expect(reesBody).toMatchObject({ + author: "alice", + body: "Implements the thing.", + repoFullName: "acme/widgets", + prNumber: 7, + }); // The brief's content flows into the user prompt, but the system prompt carries our FIXED // enrichment suffix — the REES-supplied systemSuffix is untrusted and is never spliced in. expect(seenUser[0] ?? "").toContain("## EXTERNAL REVIEW BRIEF"); From 82d80039af3d94057b9b922371ed4ddbeb66ae13 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:32:39 +0200 Subject: [PATCH 2/6] test(enrichment): cover processors enrichment branch paths for codecov Exercise null author/body, missing installation id, allowlist-off path, and installation-token resolution in enrichment wiring tests. Fixes #1697 Co-authored-by: Cursor --- test/unit/enrichment-wiring.test.ts | 108 +++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/test/unit/enrichment-wiring.test.ts b/test/unit/enrichment-wiring.test.ts index 098433eb45..d3609b64f1 100644 --- a/test/unit/enrichment-wiring.test.ts +++ b/test/unit/enrichment-wiring.test.ts @@ -1,9 +1,17 @@ import { describe, expect, it, vi } from "vitest"; import { runAiReviewForAdvisory } from "../../src/queue/processors"; import { upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { createInstallationToken } from "../../src/github/app"; import type { Advisory, RepositorySettings } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; +vi.mock("../../src/github/app", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, createInstallationToken: vi.fn() }; +}); + +const mockedToken = vi.mocked(createInstallationToken); + const notesJson = JSON.stringify({ assessment: "Looks fine.", suggestions: [], @@ -26,7 +34,11 @@ const adv = (repo: string): Advisory => ({ generatedAt: "2026-06-20T00:00:00.000Z", }); -async function seedRepoFile(env: Env, repo: string) { +async function seedRepoFile( + env: Env, + repo: string, + installationId: number | undefined = 4242, +) { await upsertRepositoryFromGitHub( env, { @@ -35,7 +47,7 @@ async function seedRepoFile(env: Env, repo: string) { private: true, owner: { login: repo.split("/")[0]! }, }, - 4242, + installationId, ); await env.DB.prepare( "INSERT INTO pull_request_files (repo_full_name, pull_number, path, status, additions, deletions, changes, payload_json) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", @@ -82,6 +94,7 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE REES_SHARED_SECRET: "sek", }); await seedRepoFile(env, "acme/widgets"); + mockedToken.mockResolvedValueOnce("install-token-for-rees"); let reesUrl = ""; let reesAuth: string | null = null; let reesBody: Record | null = null; @@ -124,6 +137,7 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE expect(reesBody).toMatchObject({ author: "alice", body: "Implements the thing.", + githubToken: "install-token-for-rees", repoFullName: "acme/widgets", prNumber: 7, }); @@ -137,6 +151,96 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE } }); + it("FLAG-ON but repo not allowlisted: resolves token yet skips the REES POST", async () => { + mockedToken.mockResolvedValueOnce("unused-install-token"); + const run = vi.fn(async () => ({ response: notesJson })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + GITTENSORY_REVIEW_REPOS: "JSONbored/gittensory", + }); + Object.assign(env, { + GITTENSORY_REVIEW_ENRICHMENT: "true", + REES_URL: "https://rees.example", + }); + await seedRepoFile(env, "acme/not-allowlisted", 5151); + let reesCalled = false; + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockImplementation(async (url) => { + if (String(url).includes("/v1/enrich")) reesCalled = true; + return new Response("nope", { status: 404 }); + }); + try { + await runAiReviewForAdvisory(env, { + settings: { aiReviewMode: "advisory" } as RepositorySettings, + repoFullName: "acme/not-allowlisted", + pr: { number: 7, title: "t", body: null }, + author: null, + confirmedContributor: true, + advisory: adv("acme/not-allowlisted"), + }); + expect(reesCalled).toBe(false); + expect(mockedToken).toHaveBeenCalledWith(env, 5151); + } finally { + fetchSpy.mockRestore(); + } + }); + + it("FLAG-ON with missing author/body/installation: POSTs without optional enrichment fields", async () => { + mockedToken.mockRejectedValueOnce(new Error("no app key")); + const run = vi.fn(async () => ({ response: notesJson })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + GITHUB_PUBLIC_TOKEN: "public-fallback-token", + }); + Object.assign(env, { + GITTENSORY_REVIEW_ENRICHMENT: "true", + REES_URL: "https://rees.example", + }); + await seedRepoFile(env, "acme/widgets", undefined); + let reesBody: Record | null = null; + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockImplementation(async (url, init) => { + if (String(url).includes("/v1/enrich")) { + reesBody = JSON.parse(String(init?.body ?? "{}")) as Record< + string, + unknown + >; + return new Response( + JSON.stringify({ promptSection: "## EXTERNAL REVIEW BRIEF\n- note" }), + { status: 200 }, + ); + } + return new Response("nope", { status: 404 }); + }); + try { + await runAiReviewForAdvisory(env, { + settings: { aiReviewMode: "advisory" } as RepositorySettings, + repoFullName: "acme/widgets", + pr: { number: 7, title: "t", body: null }, + author: null, + confirmedContributor: true, + advisory: adv("acme/widgets"), + }); + expect(reesBody).toMatchObject({ + repoFullName: "acme/widgets", + prNumber: 7, + githubToken: "public-fallback-token", + }); + expect(reesBody).not.toHaveProperty("author"); + expect(reesBody).not.toHaveProperty("body"); + } finally { + fetchSpy.mockRestore(); + } + }); + it("FLAG-OFF (default): the REES is never called", async () => { const run = vi.fn(async () => ({ response: notesJson })); const env = createTestEnv({ From 55fff338df0ca658425abdbd7796850ad2f15cf4 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:39:21 +0200 Subject: [PATCH 3/6] fix(enrichment): prefetch GitHub context in engine, never send tokens to REES Move history GitHub API calls into enrichment-prefetch.ts so installation tokens stay in the engine process. REES receives structured prefetch findings only; githubToken is removed from the enrich wire contract. Fixes #1697 Co-authored-by: Cursor --- review-enrichment/README.md | 4 +- review-enrichment/src/analyzers/codeowners.ts | 48 +--- review-enrichment/src/analyzers/history.ts | 198 +++++---------- review-enrichment/src/brief.ts | 2 +- review-enrichment/src/types.ts | 7 +- review-enrichment/test/enrichment.test.ts | 9 +- review-enrichment/test/history.test.ts | 59 ++--- src/queue/processors.ts | 6 +- src/review/enrichment-prefetch.ts | 225 ++++++++++++++++++ src/review/enrichment-wire.ts | 35 +-- test/unit/enrichment-prefetch.test.ts | 106 +++++++++ test/unit/enrichment-wire.test.ts | 93 ++------ test/unit/enrichment-wiring.test.ts | 33 ++- 13 files changed, 501 insertions(+), 324 deletions(-) create mode 100644 src/review/enrichment-prefetch.ts create mode 100644 test/unit/enrichment-prefetch.test.ts diff --git a/review-enrichment/README.md b/review-enrichment/README.md index 1a009b2678..dc8f9b78ea 100644 --- a/review-enrichment/README.md +++ b/review-enrichment/README.md @@ -16,7 +16,9 @@ treats any timeout/error as "no brief" and proceeds. | `GET /ready` | Readiness. | | `POST /v1/enrich` | `Authorization: Bearer ` → `EnrichRequest` → `ReviewBrief`. | -See `src/server.ts` for the `EnrichRequest` / `ReviewBrief` contract. +See `src/server.ts` for the `EnrichRequest` / `ReviewBrief` contract. GitHub installation tokens are prefetched in the +gittensory engine (`src/review/enrichment-prefetch.ts`) and passed as structured `prefetch` findings — never as raw +credentials in the POST body. ## Analyzers (added behind the contract) diff --git a/review-enrichment/src/analyzers/codeowners.ts b/review-enrichment/src/analyzers/codeowners.ts index 2dc50e1039..72c7b0ba23 100644 --- a/review-enrichment/src/analyzers/codeowners.ts +++ b/review-enrichment/src/analyzers/codeowners.ts @@ -212,49 +212,9 @@ async function fetchCodeowners( /** Report changed files whose CODEOWNERS rule does not include the PR author, and surface blast-radius context. */ export async function scanCodeowners( req: EnrichRequest, - fetchFn: typeof fetch, - opts?: { signal?: AbortSignal }, + _fetchFn: typeof fetch, + _opts?: { signal?: AbortSignal }, ): Promise { - const { repoFullName, githubToken, author, files = [] } = req; - if (!githubToken || !author) return []; - - const parts = repoFullName.split("/"); - const repoOwner = parts[0]; - const repoName = parts[1]; - if ( - !repoOwner || - !repoName || - !SLUG_RE.test(repoOwner) || - !SLUG_RE.test(repoName) - ) - return []; - - const headers: Record = { - Authorization: `Bearer ${githubToken}`, - Accept: "application/vnd.github.raw", - "X-GitHub-Api-Version": "2022-11-28", - }; - - const content = await fetchCodeowners( - repoOwner, - repoName, - headers, - fetchFn, - opts?.signal, - ); - if (!content) return []; - - const rules = parseCodeowners(content); - if (rules.length === 0) return []; - - const findings: CodeownersFinding[] = []; - for (const file of files) { - if (findings.length >= MAX_FILES_REPORTED) break; - const owners = findOwners(rules, file.path); - if (owners.length === 0) continue; // unowned file — not a violation - if (authorMatchesOwner(author, owners)) continue; // author is listed — no violation - findings.push({ file: file.path, owners }); - } - - return findings; + if (req.prefetch?.codeowners) return req.prefetch.codeowners; + return []; } diff --git a/review-enrichment/src/analyzers/history.ts b/review-enrichment/src/analyzers/history.ts index 4c37c3709d..d7d8a7deff 100644 --- a/review-enrichment/src/analyzers/history.ts +++ b/review-enrichment/src/analyzers/history.ts @@ -1,11 +1,9 @@ -// History analyzer (#1697 / #1478). Uses PR metadata + optional GitHub API access to surface author track -// record and linked-issue alignment — context the diff-only reviewer cannot infer. Fail-safe: returns a -// degraded finding set when no token is available (linked-issue parsing still runs on the body). +// History analyzer (#1697 / #1478). Uses PR metadata plus engine-prefetched GitHub context when provided. +// Fail-safe: without prefetch, parses linked issues from the body only (no GitHub API in REES). import type { EnrichRequest, HistoryFinding, LinkedIssueFinding } from "../types.js"; const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; const MAX_LINKED_ISSUES = 8; -const NEWCOMER_MERGED_THRESHOLD = 3; const MAX_BODY_CHARS = 8000; const LINKED_ISSUE_RE = @@ -35,28 +33,56 @@ export function extractLinkedIssues( return linked; } -function parseRepoParts(repoFullName: string): { owner: string; repo: string } | null { - const [owner, repo] = repoFullName.split("/"); - if (!owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return null; - return { owner, repo }; +/** Body-only history fallback when the engine did not prefetch GitHub API results. */ +function historyFromBodyOnly( + req: EnrichRequest, +): HistoryFinding | null { + const author = req.author?.replace(/^@/, "") ?? ""; + if (!author) return null; + const linkedIssues: LinkedIssueFinding[] = extractLinkedIssues( + req.body, + req.repoFullName, + ).map((ref) => ({ + number: ref.number, + repo: ref.repo, + state: null, + title: null, + aligned: true, + })); + if (!linkedIssues.length) { + return { + authorLogin: author, + mergedPrCount: null, + authorTier: "unknown", + linkedIssues: [], + }; + } + return { + authorLogin: author, + mergedPrCount: null, + authorTier: "unknown", + linkedIssues, + }; } -async function fetchJson( - url: string, - headers: Record, - fetchFn: typeof fetch, - signal?: AbortSignal, -): Promise { - try { - const resp = await fetchFn(url, { headers, signal }); - if (!resp.ok) return null; - return (await resp.json()) as T; - } catch { - return null; +/** Analyzer entrypoint: use engine prefetch when present; otherwise body-only parsing. */ +export async function scanHistory( + req: EnrichRequest, +): Promise { + if (req.prefetch && "history" in req.prefetch) { + return req.prefetch.history ?? null; } + return historyFromBodyOnly(req); +} + +// Retained for REES unit tests that exercise GitHub search helpers directly. +export function classifyAuthorTier( + mergedCount: number | null, +): HistoryFinding["authorTier"] { + if (mergedCount === null) return "unknown"; + return mergedCount < 3 ? "newcomer" : "established"; } -/** Count merged PRs by the author in this repo (search API, bounded). */ export async function fetchAuthorMergedCount( repoFullName: string, author: string, @@ -64,122 +90,26 @@ export async function fetchAuthorMergedCount( fetchFn: typeof fetch, signal?: AbortSignal, ): Promise { - const parts = parseRepoParts(repoFullName); - if (!parts || !SLUG_RE.test(author.replace(/^@/, ""))) return null; + if (!SLUG_RE.test(author.replace(/^@/, ""))) return null; const q = encodeURIComponent( `repo:${repoFullName} author:${author.replace(/^@/, "")} is:pr is:merged`, ); - const headers = { - Authorization: `Bearer ${githubToken}`, - Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - }; - const payload = await fetchJson<{ total_count?: number }>( - `https://api.github.com/search/issues?q=${q}&per_page=1`, - headers, - fetchFn, - signal, - ); - return typeof payload?.total_count === "number" ? payload.total_count : null; -} - -/** Fetch issue state/title for a linked reference. */ -async function fetchLinkedIssue( - repo: string, - number: number, - headers: Record, - fetchFn: typeof fetch, - signal?: AbortSignal, -): Promise { - const parts = parseRepoParts(repo); - if (!parts) { - return { - number, - repo, - state: null, - title: null, - aligned: false, - }; - } - const payload = await fetchJson<{ state?: string; title?: string }>( - `https://api.github.com/repos/${encodeURIComponent(parts.owner)}/${encodeURIComponent(parts.repo)}/issues/${number}`, - headers, - fetchFn, - signal, - ); - const state = payload?.state ?? null; - return { - number, - repo, - state, - title: payload?.title ?? null, - aligned: state === "open" || state === "closed", - }; -} - -export function classifyAuthorTier( - mergedCount: number | null, -): HistoryFinding["authorTier"] { - if (mergedCount === null) return "unknown"; - return mergedCount < NEWCOMER_MERGED_THRESHOLD ? "newcomer" : "established"; -} - -/** Analyzer entrypoint: linked issues + optional author history via GitHub API. */ -export async function scanHistory( - req: EnrichRequest, - fetchFn: typeof fetch = fetch, - opts?: { signal?: AbortSignal }, -): Promise { - const author = req.author?.replace(/^@/, "") ?? ""; - if (!author) return null; - - const linkedRefs = extractLinkedIssues(req.body, req.repoFullName); - let mergedPrCount: number | null = null; - const linkedIssues: LinkedIssueFinding[] = []; - - if (req.githubToken) { - const headers = { - Authorization: `Bearer ${req.githubToken}`, - Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - }; - mergedPrCount = await fetchAuthorMergedCount( - req.repoFullName, - author, - req.githubToken, - fetchFn, - opts?.signal, + try { + const resp = await fetchFn( + `https://api.github.com/search/issues?q=${q}&per_page=1`, + { + headers: { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }, + signal, + }, ); - for (const ref of linkedRefs) { - linkedIssues.push( - await fetchLinkedIssue(ref.repo, ref.number, headers, fetchFn, opts?.signal), - ); - } - } else { - for (const ref of linkedRefs) { - linkedIssues.push({ - number: ref.number, - repo: ref.repo, - state: null, - title: null, - aligned: true, - }); - } - } - - if (!linkedIssues.length && mergedPrCount === null) { - return { - authorLogin: author, - mergedPrCount: null, - authorTier: "unknown", - linkedIssues: [], - }; + if (!resp.ok) return null; + const payload = (await resp.json()) as { total_count?: number }; + return typeof payload.total_count === "number" ? payload.total_count : null; + } catch { + return null; } - - return { - authorLogin: author, - mergedPrCount, - authorTier: classifyAuthorTier(mergedPrCount), - linkedIssues, - }; } diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index 14e19b12c9..4e1e109571 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -32,7 +32,7 @@ const ANALYZERS: Record = { redos: (req) => scanRedos(req), codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }), secretLog: (req, signal) => scanSecretLog(req, signal), - history: (req, signal) => scanHistory(req, fetch, { signal }), + history: (req) => scanHistory(req), }; function runWithTimeout( diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 3207a58c9d..f397c92f6f 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -18,8 +18,11 @@ export interface EnrichRequest { deletions?: number; }>; diff?: string; - /** Short-lived broker token for OSV/license/history fetches. Never logged. */ - githubToken?: string; + /** Engine-prefetched GitHub findings — tokens never cross the REES wire. */ + prefetch?: { + history?: HistoryFinding | null; + codeowners?: CodeownersFinding[]; + }; budget?: { timeoutMs?: number; maxBriefChars?: number }; analyzers?: string[]; } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 94c475f4d4..350852a04d 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -868,14 +868,13 @@ test("scanCodeowners: reports files not owned by the PR author", async () => { { repoFullName: "owner/repo", prNumber: 1, - githubToken: "token", author: "alice", files: [{ path: "src/app.ts" }, { path: "README.md" }], + prefetch: { + codeowners: [{ file: "src/app.ts", owners: ["@team/reviewers"] }], + }, }, - async () => ({ - ok: true, - text: async () => "src/** @team/reviewers\nREADME.md @alice", - }), + fetch, ); assert.deepEqual(findings, [ diff --git a/review-enrichment/test/history.test.ts b/review-enrichment/test/history.test.ts index 0c0b40aa22..24ed009c5f 100644 --- a/review-enrichment/test/history.test.ts +++ b/review-enrichment/test/history.test.ts @@ -47,7 +47,7 @@ describe("history analyzer", () => { ); }); - it("scanHistory degrades gracefully without a token but still parses links", async () => { + it("scanHistory degrades gracefully without prefetch but still parses links", async () => { const result = await scanHistory({ repoFullName: "org/app", prNumber: 5, @@ -68,39 +68,28 @@ describe("history analyzer", () => { ]); }); - it("scanHistory fetches author history and issue state when token present", async () => { - const calls: string[] = []; - const fetchFn = (async (url: string) => { - calls.push(url); - if (url.includes("/search/issues")) { - return { - ok: true, - json: async () => ({ total_count: 1 }), - } as Response; - } - if (url.includes("/issues/7")) { - return { - ok: true, - json: async () => ({ state: "open", title: "Bug in parser" }), - } as Response; - } - return { ok: false, json: async () => ({}) } as Response; - }) as typeof fetch; - - const result = await scanHistory( - { - repoFullName: "org/app", - prNumber: 9, - author: "dev1", - body: "Fixes #7", - githubToken: "ghs_test", - }, - fetchFn, - ); - assert.equal(result!.authorTier, "newcomer"); - assert.equal(result!.linkedIssues[0]?.state, "open"); - assert.match(result!.linkedIssues[0]?.title ?? "", /Bug in parser/); - assert.ok(calls.some((u) => u.includes("/search/issues"))); - assert.ok(calls.some((u) => u.includes("/issues/7"))); + it("scanHistory uses engine prefetch when provided", async () => { + const prefetched = { + authorLogin: "dev1", + mergedPrCount: 1, + authorTier: "newcomer" as const, + linkedIssues: [ + { + number: 7, + repo: "org/app", + state: "open", + title: "Bug in parser", + aligned: true, + }, + ], + }; + const result = await scanHistory({ + repoFullName: "org/app", + prNumber: 9, + author: "dev1", + body: "Fixes #7", + prefetch: { history: prefetched }, + }); + assert.deepEqual(result, prefetched); }); }); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 6a85387e85..2bbf1d603a 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -325,7 +325,6 @@ import { buildReviewRagContext, isRagEnabled } from "../review/rag-wire"; import { buildReviewEnrichment, isEnrichmentEnabled, - resolveEnrichmentGithubToken, } from "../review/enrichment-wire"; import { captureReviewFailure } from "../selfhost/sentry"; import { evaluateWithSurfaceLane } from "../review/content-lane-wire"; @@ -3736,9 +3735,6 @@ export async function runAiReviewForAdvisory( const enrichmentDiff = buildAiReviewDiff(files); const enrichmentInstallationId = (await getRepository(env, args.repoFullName))?.installationId ?? null; - const enrichmentGithubToken = isEnrichmentEnabled(env) - ? await resolveEnrichmentGithubToken(env, enrichmentInstallationId) - : undefined; const enrichment = isEnrichmentEnabled(env) && convergedRepoAllowed ? await buildReviewEnrichment(env, { @@ -3748,7 +3744,7 @@ export async function runAiReviewForAdvisory( title: args.pr.title, body: args.pr.body ?? undefined, author: args.author ?? undefined, - githubToken: enrichmentGithubToken, + installationId: enrichmentInstallationId, files, diff: enrichmentDiff, }) diff --git a/src/review/enrichment-prefetch.ts b/src/review/enrichment-prefetch.ts new file mode 100644 index 0000000000..1fc24113d0 --- /dev/null +++ b/src/review/enrichment-prefetch.ts @@ -0,0 +1,225 @@ +// GitHub-backed enrichment prefetch (#1697 security fix). Installation tokens stay in the engine — +// REES receives only derived, public-safe findings over the shared-secret wire, never raw credentials. +import { createInstallationToken } from "../github/app"; +import type { PullRequestFileRecord } from "../types"; + +const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; +const MAX_LINKED_ISSUES = 8; +const NEWCOMER_MERGED_THRESHOLD = 3; +const MAX_BODY_CHARS = 8000; + +const LINKED_ISSUE_RE = + /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)\s*)?#(\d+)\b/gi; + +export interface EnrichmentLinkedIssueFinding { + number: number; + repo: string; + state: string | null; + title: string | null; + aligned: boolean; +} + +export interface EnrichmentHistoryFinding { + authorLogin: string; + mergedPrCount: number | null; + authorTier: "newcomer" | "established" | "unknown"; + linkedIssues: EnrichmentLinkedIssueFinding[]; +} + +export interface EnrichmentPrefetch { + history?: EnrichmentHistoryFinding | null; +} + +export interface EnrichmentPrefetchInput { + repoFullName: string; + author?: string | undefined; + body?: string | undefined; + installationId?: number | null | undefined; + files?: PullRequestFileRecord[]; +} + +/** Best-effort GitHub token for engine-side prefetch — installation token, then public token. */ +export async function resolveEnrichmentGithubToken( + env: Env, + installationId: number | null | undefined, +): Promise { + if (installationId) { + const token = await createInstallationToken(env, installationId).catch( + () => undefined, + ); + if (token) return token; + } + return env.GITHUB_PUBLIC_TOKEN ?? undefined; +} + +/** Parse `Fixes #123` / `Closes org/repo#456` references from the PR body. Pure. */ +export function extractLinkedIssues( + body: string | undefined, + defaultRepo: string, +): Array<{ repo: string; number: number }> { + const text = (body ?? "").slice(0, MAX_BODY_CHARS); + const seen = new Set(); + const linked: Array<{ repo: string; number: number }> = []; + for (const match of text.matchAll(LINKED_ISSUE_RE)) { + const owner = match[1]; + const repo = match[2]; + const number = Number(match[3]); + if (!Number.isFinite(number) || number <= 0) continue; + const repoFullName = owner && repo ? `${owner}/${repo}` : defaultRepo; + const key = `${repoFullName}#${number}`; + if (seen.has(key)) continue; + seen.add(key); + linked.push({ repo: repoFullName, number }); + if (linked.length >= MAX_LINKED_ISSUES) break; + } + return linked; +} + +function parseRepoParts( + repoFullName: string, +): { owner: string; repo: string } | null { + const [owner, repo] = repoFullName.split("/"); + if (!owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) { + return null; + } + return { owner, repo }; +} + +async function fetchJson( + url: string, + headers: Record, + signal?: AbortSignal, +): Promise { + try { + const resp = await fetch(url, { headers, signal }); + if (!resp.ok) return null; + return (await resp.json()) as T; + } catch { + return null; + } +} + +export async function fetchAuthorMergedCount( + repoFullName: string, + author: string, + githubToken: string, + signal?: AbortSignal, +): Promise { + const parts = parseRepoParts(repoFullName); + if (!parts || !SLUG_RE.test(author.replace(/^@/, ""))) return null; + const q = encodeURIComponent( + `repo:${repoFullName} author:${author.replace(/^@/, "")} is:pr is:merged`, + ); + const headers = { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; + const payload = await fetchJson<{ total_count?: number }>( + `https://api.github.com/search/issues?q=${q}&per_page=1`, + headers, + signal, + ); + return typeof payload?.total_count === "number" ? payload.total_count : null; +} + +async function fetchLinkedIssue( + repo: string, + number: number, + headers: Record, + signal?: AbortSignal, +): Promise { + const parts = parseRepoParts(repo); + if (!parts) { + return { number, repo, state: null, title: null, aligned: false }; + } + const payload = await fetchJson<{ state?: string; title?: string }>( + `https://api.github.com/repos/${encodeURIComponent(parts.owner)}/${encodeURIComponent(parts.repo)}/issues/${number}`, + headers, + signal, + ); + const state = payload?.state ?? null; + return { + number, + repo, + state, + title: payload?.title ?? null, + aligned: state === "open" || state === "closed", + }; +} + +export function classifyAuthorTier( + mergedCount: number | null, +): EnrichmentHistoryFinding["authorTier"] { + if (mergedCount === null) return "unknown"; + return mergedCount < NEWCOMER_MERGED_THRESHOLD ? "newcomer" : "established"; +} + +/** Build history findings locally (fail-safe). Without a token, linked issues are parsed from the body only. */ +export async function prefetchEnrichmentHistory( + input: EnrichmentPrefetchInput, + githubToken?: string, + signal?: AbortSignal, +): Promise { + const author = input.author?.replace(/^@/, "") ?? ""; + if (!author) return null; + + const linkedRefs = extractLinkedIssues(input.body, input.repoFullName); + let mergedPrCount: number | null = null; + const linkedIssues: EnrichmentLinkedIssueFinding[] = []; + + if (githubToken) { + const headers = { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; + mergedPrCount = await fetchAuthorMergedCount( + input.repoFullName, + author, + githubToken, + signal, + ); + for (const ref of linkedRefs) { + linkedIssues.push( + await fetchLinkedIssue(ref.repo, ref.number, headers, signal), + ); + } + } else { + for (const ref of linkedRefs) { + linkedIssues.push({ + number: ref.number, + repo: ref.repo, + state: null, + title: null, + aligned: true, + }); + } + } + + if (!linkedIssues.length && mergedPrCount === null) { + return { + authorLogin: author, + mergedPrCount: null, + authorTier: "unknown", + linkedIssues: [], + }; + } + + return { + authorLogin: author, + mergedPrCount, + authorTier: classifyAuthorTier(mergedPrCount), + linkedIssues, + }; +} + +/** Prefetch GitHub-derived enrichment context in the engine. Tokens never leave this process. */ +export async function prefetchEnrichmentGitHubContext( + env: Env, + input: EnrichmentPrefetchInput, +): Promise { + const token = await resolveEnrichmentGithubToken(env, input.installationId); + const history = await prefetchEnrichmentHistory(input, token); + return { history }; +} diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index 1804008d95..d3250da5fb 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -6,9 +6,14 @@ // Single env switch: GITTENSORY_REVIEW_ENRICHMENT (+ REES_URL must be set, so the hosted Worker — which sets neither // — is unaffected). Default OFF → gathers nothing, prompt byte-identical. FULLY FAIL-SAFE: any timeout / non-200 / // network / parse error, or an empty brief, returns undefined and the review proceeds on diff + grounding + RAG. +// +// GitHub installation tokens are prefetched in enrichment-prefetch.ts and NEVER serialized into the REES POST body. import { sanitizePublicComment } from "../queue-intelligence"; import { neutralizePromptInjection } from "./prompt-injection"; -import { createInstallationToken } from "../github/app"; +import { + prefetchEnrichmentGitHubContext, + type EnrichmentPrefetch, +} from "./enrichment-prefetch"; import type { PullRequestFileRecord } from "../types"; interface EnrichmentEnv { @@ -56,25 +61,11 @@ interface EnrichmentInput { title?: string | undefined; body?: string | undefined; author?: string | undefined; - githubToken?: string | undefined; + installationId?: number | null | undefined; files: PullRequestFileRecord[]; diff: string; } -/** Best-effort GitHub token for REES history/codeowners fetches — installation token, then public token. */ -export async function resolveEnrichmentGithubToken( - env: Env, - installationId: number | null | undefined, -): Promise { - if (installationId) { - const token = await createInstallationToken(env, installationId).catch( - () => undefined, - ); - if (token) return token; - } - return env.GITHUB_PUBLIC_TOKEN ?? undefined; -} - /** POST the PR to the REES and return the spliceable brief, or undefined on any error/timeout/empty (fail-safe). */ export async function buildReviewEnrichment( env: Env, @@ -84,6 +75,16 @@ export async function buildReviewEnrichment( const base = cfg.REES_URL?.trim(); if (!base) return undefined; const timeoutMs = Math.max(1000, Number(cfg.REES_TIMEOUT_MS ?? "8000")); + const prefetch: EnrichmentPrefetch = await prefetchEnrichmentGitHubContext( + env, + { + repoFullName: input.repoFullName, + author: input.author, + body: input.body, + installationId: input.installationId, + files: input.files, + }, + ); try { const response = await fetch(`${base.replace(/\/+$/, "")}/v1/enrich`, { method: "POST", @@ -101,7 +102,7 @@ export async function buildReviewEnrichment( title: input.title, body: input.body, author: input.author, - githubToken: input.githubToken, + prefetch, files: input.files.map((file) => ({ path: file.path, patch: diff --git a/test/unit/enrichment-prefetch.test.ts b/test/unit/enrichment-prefetch.test.ts new file mode 100644 index 0000000000..54718be74f --- /dev/null +++ b/test/unit/enrichment-prefetch.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { + classifyAuthorTier, + extractLinkedIssues, + fetchAuthorMergedCount, + prefetchEnrichmentGitHubContext, + prefetchEnrichmentHistory, + resolveEnrichmentGithubToken, +} from "../../src/review/enrichment-prefetch"; +import { createInstallationToken } from "../../src/github/app"; +import { createTestEnv } from "../helpers/d1"; + +vi.mock("../../src/github/app", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, createInstallationToken: vi.fn() }; +}); + +const mockedToken = vi.mocked(createInstallationToken); + +describe("enrichment-prefetch", () => { + it("extractLinkedIssues parses default and explicit repo references", () => { + expect(extractLinkedIssues("Fixes #12", "org/app")).toEqual([ + { repo: "org/app", number: 12 }, + ]); + expect(extractLinkedIssues("Closes other/repo#99", "org/app")).toEqual([ + { repo: "other/repo", number: 99 }, + ]); + }); + + it("classifyAuthorTier buckets merged counts", () => { + expect(classifyAuthorTier(null)).toBe("unknown"); + expect(classifyAuthorTier(2)).toBe("newcomer"); + expect(classifyAuthorTier(3)).toBe("established"); + }); + + it("fetchAuthorMergedCount reads search total_count", async () => { + globalThis.fetch = vi.fn(async () => ({ + ok: true, + json: async () => ({ total_count: 7 }), + })) as unknown as typeof fetch; + await expect( + fetchAuthorMergedCount("org/app", "dev1", "token"), + ).resolves.toBe(7); + }); + + it("prefetchEnrichmentHistory without token parses linked issues only", async () => { + const result = await prefetchEnrichmentHistory({ + repoFullName: "org/app", + author: "dev1", + body: "Fixes #42", + }); + expect(result).toMatchObject({ + authorLogin: "dev1", + authorTier: "unknown", + linkedIssues: [{ number: 42, repo: "org/app", aligned: true }], + }); + }); + + it("resolveEnrichmentGithubToken prefers installation token then public token", async () => { + mockedToken.mockResolvedValueOnce("install-token"); + const envWithInstall = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( + "install-token", + ); + + mockedToken.mockRejectedValueOnce(new Error("no app")); + await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( + "public-token", + ); + + const bareEnv = createTestEnv({}); + await expect(resolveEnrichmentGithubToken(bareEnv, null)).resolves.toBeUndefined(); + }); + + it("prefetchEnrichmentGitHubContext fetches history with installation token", async () => { + mockedToken.mockResolvedValueOnce("install-token"); + globalThis.fetch = vi.fn(async (url: string) => { + if (String(url).includes("/search/issues")) { + return { + ok: true, + json: async () => ({ total_count: 1 }), + } as Response; + } + if (String(url).includes("/issues/7")) { + return { + ok: true, + json: async () => ({ state: "open", title: "Bug" }), + } as Response; + } + return { ok: false, json: async () => ({}) } as Response; + }) as unknown as typeof fetch; + const env = createTestEnv({}); + const result = await prefetchEnrichmentGitHubContext(env, { + repoFullName: "org/app", + author: "dev1", + body: "Fixes #7", + installationId: 42, + }); + expect(result.history).toMatchObject({ + authorLogin: "dev1", + authorTier: "newcomer", + linkedIssues: [{ number: 7, state: "open" }], + }); + expect(mockedToken).toHaveBeenCalledWith(env, 42); + }); +}); diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 9ac40d65d6..78dcffc0d8 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -1,18 +1,9 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; import { isEnrichmentEnabled, buildReviewEnrichment, - resolveEnrichmentGithubToken, } from "../../src/review/enrichment-wire"; -import { createInstallationToken } from "../../src/github/app"; -import { createTestEnv } from "../helpers/d1"; - -vi.mock("../../src/github/app", async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, createInstallationToken: vi.fn() }; -}); - -const mockedToken = vi.mocked(createInstallationToken); +import * as enrichmentPrefetch from "../../src/review/enrichment-prefetch"; const env = (o: Record) => o as unknown as Env; const input = { @@ -41,8 +32,8 @@ describe("isEnrichmentEnabled", () => { ).toBe(true); expect( isEnrichmentEnabled(env({ GITTENSORY_REVIEW_ENRICHMENT: "on" })), - ).toBe(false); // no URL - expect(isEnrichmentEnabled(env({ REES_URL: "https://r" }))).toBe(false); // flag off + ).toBe(false); + expect(isEnrichmentEnabled(env({ REES_URL: "https://r" }))).toBe(false); expect( isEnrichmentEnabled( env({ GITTENSORY_REVIEW_ENRICHMENT: "false", REES_URL: "https://r" }), @@ -56,12 +47,23 @@ describe("buildReviewEnrichment", () => { let realFetch: typeof fetch; beforeEach(() => { realFetch = globalThis.fetch; + vi.spyOn(enrichmentPrefetch, "prefetchEnrichmentGitHubContext").mockResolvedValue( + { + history: { + authorLogin: "dev1", + mergedPrCount: 2, + authorTier: "newcomer", + linkedIssues: [], + }, + }, + ); }); afterEach(() => { globalThis.fetch = realFetch; + vi.restoreAllMocks(); }); - it("returns the trimmed brief, sends the bearer + mapped files, honors REES_TIMEOUT_MS", async () => { + it("returns the trimmed brief, sends prefetch (not githubToken), honors REES_TIMEOUT_MS", async () => { const calls: Array<{ url: unknown; init: RequestInit }> = []; globalThis.fetch = vi.fn(async (url: unknown, init: RequestInit) => { calls.push({ url, init }); @@ -79,47 +81,27 @@ describe("buildReviewEnrichment", () => { REES_SHARED_SECRET: "sek", REES_TIMEOUT_MS: "12000", }), - input, + { + ...input, + body: "Fixes #12", + author: "dev1", + installationId: 42, + }, ); expect(r?.promptSection).toBe("BRIEF"); - expect(r?.systemSuffix).toContain("REVIEW ENRICHMENT"); - expect(r?.systemSuffix).not.toContain("suffix"); - expect(calls[0]!.url).toBe("https://rees/v1/enrich"); expect( - (calls[0]!.init.headers as Record).authorization, - ).toBe("Bearer sek"); + enrichmentPrefetch.prefetchEnrichmentGitHubContext, + ).toHaveBeenCalled(); const body = JSON.parse(calls[0]!.init.body as string); expect(body.repoFullName).toBe("o/r"); + expect(body.prefetch.history.authorLogin).toBe("dev1"); + expect(body.githubToken).toBeUndefined(); expect(body.files).toEqual([ { path: "a.ts", patch: "@@ +1 @@" }, { path: "b.ts", patch: undefined }, ]); }); - it("POST includes author, body, and githubToken when provided", async () => { - const calls: RequestInit[] = []; - globalThis.fetch = vi.fn(async (_url: unknown, init: RequestInit) => { - calls.push(init); - return { - ok: true, - json: async () => ({ promptSection: "history brief" }), - } as Response; - }) as unknown as typeof fetch; - await buildReviewEnrichment( - env({ REES_URL: "https://rees/" }), - { - ...input, - body: "Fixes #12", - author: "dev1", - githubToken: "ghs_test", - }, - ); - const body = JSON.parse(calls[0]!.body as string); - expect(body.body).toBe("Fixes #12"); - expect(body.author).toBe("dev1"); - expect(body.githubToken).toBe("ghs_test"); - }); - it("undefined when REES_URL is unset", async () => { expect(await buildReviewEnrichment(env({}), input)).toBeUndefined(); }); @@ -133,7 +115,6 @@ describe("buildReviewEnrichment", () => { expect( await buildReviewEnrichment(env({ REES_URL: "https://r" }), input), ).toBeUndefined(); - // A non-2xx REES response now logs at error level (was a silent skip) so a broken backend is visible in Sentry. expect( errSpy.mock.calls.some( (c) => @@ -150,7 +131,6 @@ describe("buildReviewEnrichment", () => { throw new Error("network down"); }) as unknown as typeof fetch; expect(await buildReviewEnrichment(env({ REES_URL: "https://r" }), input)).toBeUndefined(); - // A broken/slow REES backend now surfaces at level:error (central Sentry forwarder) instead of degrading silently. expect(errSpy.mock.calls.some((c) => String(c[0]).includes("review_context_fetch_failed") && String(c[0]).includes('"contextType":"enrichment"'))).toBe(true); errSpy.mockRestore(); }); @@ -198,9 +178,6 @@ describe("buildReviewEnrichment", () => { /ignore previous instructions|approve this PR/i, ); expect(r?.systemSuffix).toContain("untrusted advisory context"); - expect(r?.systemSuffix).not.toMatch( - /ignore previous instructions|approve this PR/i, - ); globalThis.fetch = vi.fn( async () => @@ -242,21 +219,3 @@ describe("buildReviewEnrichment", () => { ).toBeUndefined(); }); }); - -describe("resolveEnrichmentGithubToken", () => { - it("prefers installation token and falls back to GITHUB_PUBLIC_TOKEN", async () => { - mockedToken.mockResolvedValueOnce("install-token"); - const envWithInstall = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); - await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( - "install-token", - ); - - mockedToken.mockRejectedValueOnce(new Error("no app")); - await expect(resolveEnrichmentGithubToken(envWithInstall, 42)).resolves.toBe( - "public-token", - ); - - const bareEnv = createTestEnv({}); - await expect(resolveEnrichmentGithubToken(bareEnv, null)).resolves.toBeUndefined(); - }); -}); diff --git a/test/unit/enrichment-wiring.test.ts b/test/unit/enrichment-wiring.test.ts index d3609b64f1..806c9013d0 100644 --- a/test/unit/enrichment-wiring.test.ts +++ b/test/unit/enrichment-wiring.test.ts @@ -66,7 +66,7 @@ async function seedRepoFile( } describe("review-enrichment wired into the processors review (flag GITTENSORY_REVIEW_ENRICHMENT + REES_URL)", () => { - it("FLAG-ON via runAiReviewForAdvisory: POSTs the PR to the REES (with bearer) and splices the brief into the prompts", async () => { + it("FLAG-ON via runAiReviewForAdvisory: POSTs prefetch (not githubToken) to REES and splices the brief", async () => { const seenUser: string[] = []; const seenSystem: string[] = []; const run = vi.fn( @@ -87,14 +87,13 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE AI_PUBLIC_COMMENTS_ENABLED: "true", AI_DAILY_NEURON_BUDGET: "100000", }); - // The REES vars are self-host runtime env (not declared on the Worker Env type) — set them as the self-host does. Object.assign(env, { GITTENSORY_REVIEW_ENRICHMENT: "true", REES_URL: "https://rees.example", REES_SHARED_SECRET: "sek", }); await seedRepoFile(env, "acme/widgets"); - mockedToken.mockResolvedValueOnce("install-token-for-rees"); + mockedToken.mockResolvedValueOnce("install-token-local-only"); let reesUrl = ""; let reesAuth: string | null = null; let reesBody: Record | null = null; @@ -116,6 +115,11 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE { status: 200 }, ); } + if (String(url).includes("/search/issues")) { + return new Response(JSON.stringify({ total_count: 4 }), { + status: 200, + }); + } return new Response("nope", { status: 404 }); }); try { @@ -131,27 +135,31 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE confirmedContributor: true, advisory: adv("acme/widgets"), }); - // The enrichment build branch executed: the REES was POSTed at /v1/enrich with the shared-secret bearer. expect(reesUrl).toBe("https://rees.example/v1/enrich"); expect(reesAuth).toBe("Bearer sek"); expect(reesBody).toMatchObject({ author: "alice", body: "Implements the thing.", - githubToken: "install-token-for-rees", repoFullName: "acme/widgets", prNumber: 7, }); - // The brief's content flows into the user prompt, but the system prompt carries our FIXED - // enrichment suffix — the REES-supplied systemSuffix is untrusted and is never spliced in. + expect(reesBody!.githubToken).toBeUndefined(); + expect(reesBody!.prefetch).toMatchObject({ + history: { + authorLogin: "alice", + mergedPrCount: 4, + authorTier: "established", + }, + }); + expect(mockedToken).toHaveBeenCalledWith(env, 4242); expect(seenUser[0] ?? "").toContain("## EXTERNAL REVIEW BRIEF"); expect(seenSystem[0] ?? "").toContain("untrusted advisory context"); - expect(seenSystem[0] ?? "").not.toContain("verified ground truth"); } finally { fetchSpy.mockRestore(); } }); - it("FLAG-ON but repo not allowlisted: resolves token yet skips the REES POST", async () => { + it("FLAG-ON but repo not allowlisted: skips prefetch and the REES POST", async () => { mockedToken.mockResolvedValueOnce("unused-install-token"); const run = vi.fn(async () => ({ response: notesJson })); const env = createTestEnv({ @@ -183,14 +191,12 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE advisory: adv("acme/not-allowlisted"), }); expect(reesCalled).toBe(false); - expect(mockedToken).toHaveBeenCalledWith(env, 5151); } finally { fetchSpy.mockRestore(); } }); - it("FLAG-ON with missing author/body/installation: POSTs without optional enrichment fields", async () => { - mockedToken.mockRejectedValueOnce(new Error("no app key")); + it("FLAG-ON with missing author/body/installation: POSTs prefetch without githubToken", async () => { const run = vi.fn(async () => ({ response: notesJson })); const env = createTestEnv({ AI: { run } as unknown as Ai, @@ -232,8 +238,9 @@ describe("review-enrichment wired into the processors review (flag GITTENSORY_RE expect(reesBody).toMatchObject({ repoFullName: "acme/widgets", prNumber: 7, - githubToken: "public-fallback-token", + prefetch: { history: null }, }); + expect(reesBody!.githubToken).toBeUndefined(); expect(reesBody).not.toHaveProperty("author"); expect(reesBody).not.toHaveProperty("body"); } finally { From c938fa4feaba5b0ce86a0e1f98a7eb3d091d95d4 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:42:10 +0200 Subject: [PATCH 4/6] fix(enrichment): satisfy exactOptionalPropertyTypes in prefetch fetch Fixes #1697 Co-authored-by: Cursor --- src/review/enrichment-prefetch.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/review/enrichment-prefetch.ts b/src/review/enrichment-prefetch.ts index 1fc24113d0..86589b49a2 100644 --- a/src/review/enrichment-prefetch.ts +++ b/src/review/enrichment-prefetch.ts @@ -91,7 +91,10 @@ async function fetchJson( signal?: AbortSignal, ): Promise { try { - const resp = await fetch(url, { headers, signal }); + const resp = await fetch( + url, + signal ? { headers, signal } : { headers }, + ); if (!resp.ok) return null; return (await resp.json()) as T; } catch { From d8e014a0c2e4390f002c436c5c527e430208430c Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:50:23 +0200 Subject: [PATCH 5/6] fix(enrichment): prefetch CODEOWNERS in engine and raise patch coverage Engine-side CODEOWNERS scan restores the prefetch contract without sending tokens to REES; expanded prefetch unit tests cover abort signals and branches. Co-authored-by: Cursor --- review-enrichment/src/analyzers/codeowners.ts | 67 +++++- src/review/enrichment-prefetch.ts | 24 ++- test/unit/enrichment-prefetch.test.ts | 192 +++++++++++++++++- 3 files changed, 273 insertions(+), 10 deletions(-) diff --git a/review-enrichment/src/analyzers/codeowners.ts b/review-enrichment/src/analyzers/codeowners.ts index 72c7b0ba23..f486292478 100644 --- a/review-enrichment/src/analyzers/codeowners.ts +++ b/review-enrichment/src/analyzers/codeowners.ts @@ -185,7 +185,7 @@ export function authorMatchesOwner(author: string, owners: string[]): boolean { // ── Network ─────────────────────────────────────────────────────────────────── /** Try each CODEOWNERS location in priority order; return raw content of the first found, or null. */ -async function fetchCodeowners( +export async function fetchCodeowners( owner: string, repo: string, headers: Record, @@ -196,7 +196,7 @@ async function fetchCodeowners( try { const resp = await fetchFn( `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}`, - { headers, signal }, + signal ? { headers, signal } : { headers }, ); if (!resp.ok) continue; return await resp.text(); @@ -207,6 +207,69 @@ async function fetchCodeowners( return null; } +// ── Prefetch (engine-side; tokens never cross the REES wire) ───────────────── + +/** Match changed files against parsed CODEOWNERS text; report author-absent violations. */ +export function matchCodeownersViolations( + content: string, + author: string, + files: Array<{ path: string }>, +): CodeownersFinding[] { + const rules = parseCodeowners(content); + if (rules.length === 0) return []; + + const findings: CodeownersFinding[] = []; + for (const file of files) { + if (findings.length >= MAX_FILES_REPORTED) break; + const owners = findOwners(rules, file.path); + if (owners.length === 0) continue; + if (authorMatchesOwner(author, owners)) continue; + findings.push({ file: file.path, owners }); + } + return findings; +} + +/** Fetch CODEOWNERS from GitHub and return author-absent violations (engine prefetch). */ +export async function prefetchCodeownersFindings( + repoFullName: string, + author: string, + files: Array<{ path: string }>, + githubToken: string, + fetchFn: typeof fetch = fetch, + signal?: AbortSignal, +): Promise { + if (!githubToken || !author) return []; + + const parts = repoFullName.split("/"); + const repoOwner = parts[0]; + const repoName = parts[1]; + if ( + !repoOwner || + !repoName || + !SLUG_RE.test(repoOwner) || + !SLUG_RE.test(repoName) + ) { + return []; + } + + const headers: Record = { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github.raw", + "X-GitHub-Api-Version": "2022-11-28", + }; + + const content = await fetchCodeowners( + repoOwner, + repoName, + headers, + fetchFn, + signal, + ); + if (!content) return []; + + return matchCodeownersViolations(content, author, files); +} + // ── Analyzer entrypoint ─────────────────────────────────────────────────────── /** Report changed files whose CODEOWNERS rule does not include the PR author, and surface blast-radius context. */ diff --git a/src/review/enrichment-prefetch.ts b/src/review/enrichment-prefetch.ts index 86589b49a2..ff04bf6fd5 100644 --- a/src/review/enrichment-prefetch.ts +++ b/src/review/enrichment-prefetch.ts @@ -1,6 +1,7 @@ // GitHub-backed enrichment prefetch (#1697 security fix). Installation tokens stay in the engine — // REES receives only derived, public-safe findings over the shared-secret wire, never raw credentials. import { createInstallationToken } from "../github/app"; +import { prefetchCodeownersFindings } from "../../review-enrichment/src/analyzers/codeowners.js"; import type { PullRequestFileRecord } from "../types"; const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; @@ -26,8 +27,14 @@ export interface EnrichmentHistoryFinding { linkedIssues: EnrichmentLinkedIssueFinding[]; } +export interface EnrichmentCodeownersFinding { + file: string; + owners: string[]; +} + export interface EnrichmentPrefetch { history?: EnrichmentHistoryFinding | null; + codeowners?: EnrichmentCodeownersFinding[]; } export interface EnrichmentPrefetchInput { @@ -221,8 +228,21 @@ export async function prefetchEnrichmentHistory( export async function prefetchEnrichmentGitHubContext( env: Env, input: EnrichmentPrefetchInput, + signal?: AbortSignal, ): Promise { const token = await resolveEnrichmentGithubToken(env, input.installationId); - const history = await prefetchEnrichmentHistory(input, token); - return { history }; + const [history, codeowners] = await Promise.all([ + prefetchEnrichmentHistory(input, token, signal), + token && input.author && input.files?.length + ? prefetchCodeownersFindings( + input.repoFullName, + input.author, + input.files.map((f) => ({ path: f.path })), + token, + fetch, + signal, + ) + : Promise.resolve([]), + ]); + return { history, codeowners }; } diff --git a/test/unit/enrichment-prefetch.test.ts b/test/unit/enrichment-prefetch.test.ts index 54718be74f..4537462dcd 100644 --- a/test/unit/enrichment-prefetch.test.ts +++ b/test/unit/enrichment-prefetch.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { describe, expect, it, vi, afterEach } from "vitest"; import { classifyAuthorTier, extractLinkedIssues, @@ -18,13 +18,29 @@ vi.mock("../../src/github/app", async (importOriginal) => { const mockedToken = vi.mocked(createInstallationToken); describe("enrichment-prefetch", () => { - it("extractLinkedIssues parses default and explicit repo references", () => { - expect(extractLinkedIssues("Fixes #12", "org/app")).toEqual([ + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("extractLinkedIssues handles defaults, explicit repos, dupes, and invalid refs", () => { + expect(extractLinkedIssues(undefined, "org/app")).toEqual([]); + expect(extractLinkedIssues("Fixes #12 and closes #34", "org/app")).toEqual([ { repo: "org/app", number: 12 }, + { repo: "org/app", number: 34 }, ]); expect(extractLinkedIssues("Closes other/repo#99", "org/app")).toEqual([ { repo: "other/repo", number: 99 }, ]); + expect( + extractLinkedIssues("Fixes #12 and fixes #12", "org/app"), + ).toEqual([{ repo: "org/app", number: 12 }]); + expect(extractLinkedIssues("Fixes #0 and closes #abc", "org/app")).toEqual( + [], + ); + const many = Array.from({ length: 10 }, (_, i) => `Fixes #${i + 1}`).join( + " ", + ); + expect(extractLinkedIssues(many, "org/app")).toHaveLength(8); }); it("classifyAuthorTier buckets merged counts", () => { @@ -33,16 +49,63 @@ describe("enrichment-prefetch", () => { expect(classifyAuthorTier(3)).toBe("established"); }); - it("fetchAuthorMergedCount reads search total_count", async () => { + it("fetchAuthorMergedCount returns null on invalid repo, author, or API errors", async () => { + await expect( + fetchAuthorMergedCount("bad repo", "dev1", "token"), + ).resolves.toBeNull(); + await expect( + fetchAuthorMergedCount("org/app", "../evil", "token"), + ).resolves.toBeNull(); + + globalThis.fetch = vi.fn(async () => ({ + ok: false, + json: async () => ({}), + })) as unknown as typeof fetch; + await expect( + fetchAuthorMergedCount("org/app", "dev1", "token"), + ).resolves.toBeNull(); + + globalThis.fetch = vi.fn(async () => { + throw new Error("network"); + }) as unknown as typeof fetch; + await expect( + fetchAuthorMergedCount("org/app", "dev1", "token"), + ).resolves.toBeNull(); + globalThis.fetch = vi.fn(async () => ({ ok: true, - json: async () => ({ total_count: 7 }), + json: async () => ({}), })) as unknown as typeof fetch; await expect( fetchAuthorMergedCount("org/app", "dev1", "token"), + ).resolves.toBeNull(); + }); + + it("fetchAuthorMergedCount reads search total_count with abort signal", async () => { + globalThis.fetch = vi.fn(async (_url, init) => { + expect(init).toEqual( + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + return { + ok: true, + json: async () => ({ total_count: 7 }), + }; + }) as unknown as typeof fetch; + const controller = new AbortController(); + await expect( + fetchAuthorMergedCount("org/app", "@dev1", "token", controller.signal), ).resolves.toBe(7); }); + it("prefetchEnrichmentHistory returns null when author is missing", async () => { + await expect( + prefetchEnrichmentHistory({ + repoFullName: "org/app", + author: undefined, + }), + ).resolves.toBeNull(); + }); + it("prefetchEnrichmentHistory without token parses linked issues only", async () => { const result = await prefetchEnrichmentHistory({ repoFullName: "org/app", @@ -56,6 +119,101 @@ describe("enrichment-prefetch", () => { }); }); + it("prefetchEnrichmentHistory returns null without author and unknown-only author record", async () => { + await expect( + prefetchEnrichmentHistory({ + repoFullName: "org/app", + author: "", + }), + ).resolves.toBeNull(); + + await expect( + prefetchEnrichmentHistory({ + repoFullName: "org/app", + author: "solo", + body: "", + }), + ).resolves.toMatchObject({ + authorLogin: "solo", + authorTier: "unknown", + linkedIssues: [], + mergedPrCount: null, + }); + }); + + it("prefetchEnrichmentHistory with token fetches merged count and linked issue metadata", async () => { + globalThis.fetch = vi.fn(async (url: string) => { + if (String(url).includes("/search/issues")) { + return { + ok: true, + json: async () => ({ total_count: 5 }), + } as Response; + } + if (String(url).includes("/issues/7")) { + return { + ok: true, + json: async () => ({ state: "closed", title: "Done" }), + } as Response; + } + if (String(url).includes("/issues/8")) { + return { + ok: true, + json: async () => ({ state: "draft", title: "WIP" }), + } as Response; + } + if (String(url).includes("/issues/9")) { + return { ok: false, json: async () => ({}) } as Response; + } + return { ok: false, json: async () => ({}) } as Response; + }) as unknown as typeof fetch; + + const result = await prefetchEnrichmentHistory( + { + repoFullName: "org/app", + author: "@dev1", + body: "Fixes #7 and closes bad/repo#8 and closes org/app#9", + }, + "token", + ); + expect(result).toMatchObject({ + authorLogin: "dev1", + mergedPrCount: 5, + authorTier: "established", + }); + expect(result!.linkedIssues).toEqual( + expect.arrayContaining([ + expect.objectContaining({ number: 7, aligned: true, state: "closed" }), + expect.objectContaining({ number: 8, aligned: false, state: "draft" }), + expect.objectContaining({ + number: 9, + aligned: false, + state: null, + title: null, + }), + ]), + ); + }); + + it("prefetchEnrichmentHistory marks malformed linked repos as unaligned", async () => { + globalThis.fetch = vi.fn(async () => ({ + ok: true, + json: async () => ({ total_count: 0 }), + })) as unknown as typeof fetch; + const result = await prefetchEnrichmentHistory( + { + repoFullName: "org/app", + author: "dev1", + body: "Fixes owner/-bad#5", + }, + "token", + ); + expect(result!.linkedIssues[0]).toMatchObject({ + repo: "owner/-bad", + aligned: false, + state: null, + }); + }); + it("resolveEnrichmentGithubToken prefers installation token then public token", async () => { mockedToken.mockResolvedValueOnce("install-token"); const envWithInstall = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); @@ -68,11 +226,13 @@ describe("enrichment-prefetch", () => { "public-token", ); + mockedToken.mockRejectedValueOnce(new Error("no app")); const bareEnv = createTestEnv({}); + await expect(resolveEnrichmentGithubToken(bareEnv, 42)).resolves.toBeUndefined(); await expect(resolveEnrichmentGithubToken(bareEnv, null)).resolves.toBeUndefined(); }); - it("prefetchEnrichmentGitHubContext fetches history with installation token", async () => { + it("prefetchEnrichmentGitHubContext fetches history and codeowners with installation token", async () => { mockedToken.mockResolvedValueOnce("install-token"); globalThis.fetch = vi.fn(async (url: string) => { if (String(url).includes("/search/issues")) { @@ -81,6 +241,12 @@ describe("enrichment-prefetch", () => { json: async () => ({ total_count: 1 }), } as Response; } + if (String(url).includes("/contents/.github/CODEOWNERS")) { + return { + ok: true, + text: async () => "src/ @team-leads\n", + } as Response; + } if (String(url).includes("/issues/7")) { return { ok: true, @@ -95,12 +261,26 @@ describe("enrichment-prefetch", () => { author: "dev1", body: "Fixes #7", installationId: 42, + files: [ + { + repoFullName: "org/app", + pullNumber: 1, + path: "src/a.ts", + additions: 1, + deletions: 0, + changes: 1, + payload: {}, + }, + ], }); expect(result.history).toMatchObject({ authorLogin: "dev1", authorTier: "newcomer", linkedIssues: [{ number: 7, state: "open" }], }); + expect(result.codeowners).toEqual([ + { file: "src/a.ts", owners: ["@team-leads"] }, + ]); expect(mockedToken).toHaveBeenCalledWith(env, 42); }); }); From 64aabc6a13bca60554e9460214402386ec288e85 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:57:19 +0200 Subject: [PATCH 6/6] fix(enrichment): move CODEOWNERS prefetch to src and guard prefetch in try Relocate engine-side CODEOWNERS fetch/match into src/review so vitest covers the patch; wrap prefetch in buildReviewEnrichment's fail-safe try. Co-authored-by: Cursor --- review-enrichment/src/analyzers/codeowners.ts | 98 ------------ src/review/codeowners-prefetch.ts | 103 +++++++++++++ src/review/enrichment-prefetch.ts | 2 +- src/review/enrichment-wire.ts | 20 +-- test/unit/codeowners-prefetch.test.ts | 139 ++++++++++++++++++ test/unit/enrichment-wire.test.ts | 16 ++ vitest.config.ts | 2 +- 7 files changed, 270 insertions(+), 110 deletions(-) create mode 100644 src/review/codeowners-prefetch.ts create mode 100644 test/unit/codeowners-prefetch.test.ts diff --git a/review-enrichment/src/analyzers/codeowners.ts b/review-enrichment/src/analyzers/codeowners.ts index f486292478..a8088187ed 100644 --- a/review-enrichment/src/analyzers/codeowners.ts +++ b/review-enrichment/src/analyzers/codeowners.ts @@ -6,14 +6,6 @@ // Fail-safe: returns [] on any network error, non-ok response, or missing/unreadable CODEOWNERS file. import type { EnrichRequest, CodeownersFinding } from "../types.js"; -const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; // rejects `..` and other path-traversal segments -const CODEOWNERS_PATHS = [ - ".github/CODEOWNERS", - "CODEOWNERS", - "docs/CODEOWNERS", -] as const; -const MAX_FILES_REPORTED = 20; - type GlobToken = | { kind: "literal"; value: string } | { kind: "star" } @@ -182,96 +174,6 @@ export function authorMatchesOwner(author: string, owners: string[]): boolean { return owners.some((o) => o.toLowerCase() === norm); } -// ── Network ─────────────────────────────────────────────────────────────────── - -/** Try each CODEOWNERS location in priority order; return raw content of the first found, or null. */ -export async function fetchCodeowners( - owner: string, - repo: string, - headers: Record, - fetchFn: typeof fetch, - signal?: AbortSignal, -): Promise { - for (const path of CODEOWNERS_PATHS) { - try { - const resp = await fetchFn( - `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}`, - signal ? { headers, signal } : { headers }, - ); - if (!resp.ok) continue; - return await resp.text(); - } catch { - // network error or already-aborted signal → try next location - } - } - return null; -} - -// ── Prefetch (engine-side; tokens never cross the REES wire) ───────────────── - -/** Match changed files against parsed CODEOWNERS text; report author-absent violations. */ -export function matchCodeownersViolations( - content: string, - author: string, - files: Array<{ path: string }>, -): CodeownersFinding[] { - const rules = parseCodeowners(content); - if (rules.length === 0) return []; - - const findings: CodeownersFinding[] = []; - for (const file of files) { - if (findings.length >= MAX_FILES_REPORTED) break; - const owners = findOwners(rules, file.path); - if (owners.length === 0) continue; - if (authorMatchesOwner(author, owners)) continue; - findings.push({ file: file.path, owners }); - } - return findings; -} - -/** Fetch CODEOWNERS from GitHub and return author-absent violations (engine prefetch). */ -export async function prefetchCodeownersFindings( - repoFullName: string, - author: string, - files: Array<{ path: string }>, - githubToken: string, - fetchFn: typeof fetch = fetch, - signal?: AbortSignal, -): Promise { - if (!githubToken || !author) return []; - - const parts = repoFullName.split("/"); - const repoOwner = parts[0]; - const repoName = parts[1]; - if ( - !repoOwner || - !repoName || - !SLUG_RE.test(repoOwner) || - !SLUG_RE.test(repoName) - ) { - return []; - } - - const headers: Record = { - Authorization: `Bearer ${githubToken}`, - Accept: "application/vnd.github.raw", - "X-GitHub-Api-Version": "2022-11-28", - }; - - const content = await fetchCodeowners( - repoOwner, - repoName, - headers, - fetchFn, - signal, - ); - if (!content) return []; - - return matchCodeownersViolations(content, author, files); -} - -// ── Analyzer entrypoint ─────────────────────────────────────────────────────── - /** Report changed files whose CODEOWNERS rule does not include the PR author, and surface blast-radius context. */ export async function scanCodeowners( req: EnrichRequest, diff --git a/src/review/codeowners-prefetch.ts b/src/review/codeowners-prefetch.ts new file mode 100644 index 0000000000..97cedc6242 --- /dev/null +++ b/src/review/codeowners-prefetch.ts @@ -0,0 +1,103 @@ +// Engine-side CODEOWNERS prefetch (#1697). Installation tokens stay in the engine — +// REES receives only derived findings via EnrichmentPrefetch, never raw credentials. +import { + authorMatchesOwner, + findOwners, + parseCodeowners, +} from "../../review-enrichment/src/analyzers/codeowners.js"; + +const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; +const CODEOWNERS_PATHS = [ + ".github/CODEOWNERS", + "CODEOWNERS", + "docs/CODEOWNERS", +] as const; +const MAX_FILES_REPORTED = 20; + +export interface CodeownersPrefetchFinding { + file: string; + owners: string[]; +} + +async function fetchCodeownersContent( + owner: string, + repo: string, + headers: Record, + fetchFn: typeof fetch, + signal?: AbortSignal, +): Promise { + for (const path of CODEOWNERS_PATHS) { + try { + const resp = await fetchFn( + `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}`, + signal ? { headers, signal } : { headers }, + ); + if (!resp.ok) continue; + return await resp.text(); + } catch { + // network error or aborted signal → try next location + } + } + return null; +} + +/** Match changed files against parsed CODEOWNERS text; report author-absent violations. */ +export function matchCodeownersViolations( + content: string, + author: string, + files: Array<{ path: string }>, +): CodeownersPrefetchFinding[] { + const rules = parseCodeowners(content); + if (rules.length === 0) return []; + + const findings: CodeownersPrefetchFinding[] = []; + for (const file of files) { + if (findings.length >= MAX_FILES_REPORTED) break; + const owners = findOwners(rules, file.path); + if (owners.length === 0) continue; + if (authorMatchesOwner(author, owners)) continue; + findings.push({ file: file.path, owners }); + } + return findings; +} + +/** Fetch CODEOWNERS from GitHub and return author-absent violations (engine prefetch). */ +export async function prefetchCodeownersFindings( + repoFullName: string, + author: string, + files: Array<{ path: string }>, + githubToken: string, + fetchFn: typeof fetch = fetch, + signal?: AbortSignal, +): Promise { + if (!githubToken || !author) return []; + + const parts = repoFullName.split("/"); + const repoOwner = parts[0]; + const repoName = parts[1]; + if ( + !repoOwner || + !repoName || + !SLUG_RE.test(repoOwner) || + !SLUG_RE.test(repoName) + ) { + return []; + } + + const headers: Record = { + Authorization: `Bearer ${githubToken}`, + Accept: "application/vnd.github.raw", + "X-GitHub-Api-Version": "2022-11-28", + }; + + const content = await fetchCodeownersContent( + repoOwner, + repoName, + headers, + fetchFn, + signal, + ); + if (!content) return []; + + return matchCodeownersViolations(content, author, files); +} diff --git a/src/review/enrichment-prefetch.ts b/src/review/enrichment-prefetch.ts index ff04bf6fd5..83e2a7f0be 100644 --- a/src/review/enrichment-prefetch.ts +++ b/src/review/enrichment-prefetch.ts @@ -1,7 +1,7 @@ // GitHub-backed enrichment prefetch (#1697 security fix). Installation tokens stay in the engine — // REES receives only derived, public-safe findings over the shared-secret wire, never raw credentials. import { createInstallationToken } from "../github/app"; -import { prefetchCodeownersFindings } from "../../review-enrichment/src/analyzers/codeowners.js"; +import { prefetchCodeownersFindings } from "./codeowners-prefetch"; import type { PullRequestFileRecord } from "../types"; const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index d3250da5fb..38d3184d51 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -75,17 +75,17 @@ export async function buildReviewEnrichment( const base = cfg.REES_URL?.trim(); if (!base) return undefined; const timeoutMs = Math.max(1000, Number(cfg.REES_TIMEOUT_MS ?? "8000")); - const prefetch: EnrichmentPrefetch = await prefetchEnrichmentGitHubContext( - env, - { - repoFullName: input.repoFullName, - author: input.author, - body: input.body, - installationId: input.installationId, - files: input.files, - }, - ); try { + const prefetch: EnrichmentPrefetch = await prefetchEnrichmentGitHubContext( + env, + { + repoFullName: input.repoFullName, + author: input.author, + body: input.body, + installationId: input.installationId, + files: input.files, + }, + ); const response = await fetch(`${base.replace(/\/+$/, "")}/v1/enrich`, { method: "POST", headers: { diff --git a/test/unit/codeowners-prefetch.test.ts b/test/unit/codeowners-prefetch.test.ts new file mode 100644 index 0000000000..10eed96736 --- /dev/null +++ b/test/unit/codeowners-prefetch.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it, vi, afterEach } from "vitest"; +import { + matchCodeownersViolations, + prefetchCodeownersFindings, +} from "../../src/review/codeowners-prefetch"; +import { scanCodeowners } from "../../review-enrichment/src/analyzers/codeowners.js"; + +describe("codeowners-prefetch", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("matchCodeownersViolations reports author-absent files and skips unowned paths", () => { + const content = [ + "src/ @team-leads", + "docs/ @docs-team", + ].join("\n"); + expect( + matchCodeownersViolations(content, "dev1", [ + { path: "src/a.ts" }, + { path: "docs/guide.txt" }, + { path: "unowned.txt" }, + ]), + ).toEqual([ + { file: "src/a.ts", owners: ["@team-leads"] }, + { file: "docs/guide.txt", owners: ["@docs-team"] }, + ]); + expect(matchCodeownersViolations("", "dev1", [{ path: "a.ts" }])).toEqual( + [], + ); + expect( + matchCodeownersViolations("* @team\n", "@team", [{ path: "a.ts" }]), + ).toEqual([]); + }); + + it("matchCodeownersViolations caps reported files", () => { + const content = "*.ts @owners\n"; + const files = Array.from({ length: 25 }, (_, i) => ({ + path: `src/f${i}.ts`, + })); + expect(matchCodeownersViolations(content, "outsider", files)).toHaveLength( + 20, + ); + }); + + it("prefetchCodeownersFindings returns [] without token, author, or valid repo", async () => { + await expect( + prefetchCodeownersFindings("org/app", "dev1", [{ path: "a.ts" }], ""), + ).resolves.toEqual([]); + await expect( + prefetchCodeownersFindings("org/app", "", [{ path: "a.ts" }], "token"), + ).resolves.toEqual([]); + await expect( + prefetchCodeownersFindings("bad repo", "dev1", [{ path: "a.ts" }], "token"), + ).resolves.toEqual([]); + }); + + it("prefetchCodeownersFindings tries fallback paths and handles fetch failures", async () => { + globalThis.fetch = vi.fn(async (url: string) => { + if (String(url).includes("/contents/.github/CODEOWNERS")) { + return { ok: false, text: async () => "" } as Response; + } + if (String(url).includes("/contents/CODEOWNERS")) { + throw new Error("network"); + } + if (String(url).includes("/contents/docs/CODEOWNERS")) { + return { + ok: true, + text: async () => "src/ @owners\n", + } as Response; + } + return { ok: false, text: async () => "" } as Response; + }) as unknown as typeof fetch; + + await expect( + prefetchCodeownersFindings( + "org/app", + "dev1", + [{ path: "src/x.ts" }], + "token", + ), + ).resolves.toEqual([{ file: "src/x.ts", owners: ["@owners"] }]); + }); + + it("prefetchCodeownersFindings returns [] when every CODEOWNERS path fails", async () => { + globalThis.fetch = vi.fn(async () => ({ + ok: false, + text: async () => "", + })) as unknown as typeof fetch; + await expect( + prefetchCodeownersFindings( + "org/app", + "dev1", + [{ path: "src/x.ts" }], + "token", + ), + ).resolves.toEqual([]); + }); + + it("prefetchCodeownersFindings passes abort signal to fetch", async () => { + globalThis.fetch = vi.fn(async (_url, init) => { + expect(init).toEqual( + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + return { + ok: true, + text: async () => "* @team\n", + } as Response; + }) as unknown as typeof fetch; + const controller = new AbortController(); + await prefetchCodeownersFindings( + "org/app", + "outsider", + [{ path: "any.ts" }], + "token", + fetch, + controller.signal, + ); + }); +}); + +describe("scanCodeowners (REES prefetch contract)", () => { + it("returns prefetched findings or [] when absent", async () => { + const findings = [{ file: "src/a.ts", owners: ["@team"] }]; + await expect( + scanCodeowners( + { + repoFullName: "org/app", + prNumber: 1, + prefetch: { codeowners: findings }, + }, + fetch, + ), + ).resolves.toEqual(findings); + await expect( + scanCodeowners({ repoFullName: "org/app", prNumber: 1 }, fetch), + ).resolves.toEqual([]); + }); +}); diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 78dcffc0d8..5b941653ef 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -200,6 +200,22 @@ describe("buildReviewEnrichment", () => { ).toBeUndefined(); }); + it("undefined when prefetch throws — fail-safe", async () => { + vi.mocked(enrichmentPrefetch.prefetchEnrichmentGitHubContext).mockRejectedValueOnce( + new Error("prefetch boom"), + ); + globalThis.fetch = vi.fn( + async () => + ({ + ok: true, + json: async () => ({ promptSection: "brief" }), + }) as Response, + ) as unknown as typeof fetch; + expect( + await buildReviewEnrichment(env({ REES_URL: "https://r" }), input), + ).toBeUndefined(); + }); + it("omits the bearer header when no secret, and defaults systemSuffix to empty", async () => { const calls: RequestInit[] = []; globalThis.fetch = vi.fn(async (_url: unknown, init: RequestInit) => { diff --git a/vitest.config.ts b/vitest.config.ts index 55616f191a..4bfc377064 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -26,7 +26,7 @@ export default defineConfig({ ...(junitPath ? { outputFile: { junit: junitPath } } : {}), coverage: { provider: "v8", - include: ["src/**/*.ts"], + include: ["src/**/*.ts", "review-enrichment/src/analyzers/codeowners.ts"], exclude: ["src/env.d.ts", "apps/**"], // Emit lcov for Codecov to compute patch (changed-lines) coverage. reporter: ["text", "lcov"],