Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion review-enrichment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ treats any timeout/error as "no brief" and proceeds.
| `GET /ready` | Readiness. |
| `POST /v1/enrich` | `Authorization: Bearer <REES_SHARED_SECRET>` → `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)

Expand Down
83 changes: 4 additions & 79 deletions review-enrichment/src/analyzers/codeowners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -182,79 +174,12 @@ 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. */
async function fetchCodeowners(
owner: string,
repo: string,
headers: Record<string, string>,
fetchFn: typeof fetch,
signal?: AbortSignal,
): Promise<string | null> {
for (const path of CODEOWNERS_PATHS) {
try {
const resp = await fetchFn(
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}`,
{ headers, signal },
);
if (!resp.ok) continue;
return await resp.text();
} catch {
// network error or already-aborted signal → try next location
}
}
return null;
}

// ── 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,
fetchFn: typeof fetch,
opts?: { signal?: AbortSignal },
_fetchFn: typeof fetch,
_opts?: { signal?: AbortSignal },
): Promise<CodeownersFinding[]> {
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<string, string> = {
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 [];
}
115 changes: 115 additions & 0 deletions review-enrichment/src/analyzers/history.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// 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 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<string>();
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;
}

/** 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,
};
}

/** Analyzer entrypoint: use engine prefetch when present; otherwise body-only parsing. */
export async function scanHistory(
req: EnrichRequest,
): Promise<HistoryFinding | null> {
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";
}

export async function fetchAuthorMergedCount(
repoFullName: string,
author: string,
githubToken: string,
fetchFn: typeof fetch,
signal?: AbortSignal,
): Promise<number | null> {
if (!SLUG_RE.test(author.replace(/^@/, ""))) return null;
const q = encodeURIComponent(
`repo:${repoFullName} author:${author.replace(/^@/, "")} is:pr is:merged`,
);
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,
},
);
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;
}
}
2 changes: 2 additions & 0 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>;
Expand All @@ -31,6 +32,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
redos: (req) => scanRedos(req),
codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }),
secretLog: (req, signal) => scanSecretLog(req, signal),
history: (req) => scanHistory(req),
};

function runWithTimeout<T>(
Expand Down
22 changes: 22 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
32 changes: 30 additions & 2 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
Expand Down Expand Up @@ -109,6 +112,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[];
Expand All @@ -120,6 +147,7 @@ export interface BriefFindings {
redos?: RedosFinding[];
codeowners?: CodeownersFinding[];
secretLog?: SecretLogFinding[];
history?: HistoryFinding | null;
}

export type AnalyzerStatus = "ok" | "degraded" | "skipped";
Expand Down
9 changes: 4 additions & 5 deletions review-enrichment/test/enrichment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down
Loading
Loading