diff --git a/review-enrichment/src/analyzers/a11y-regression.ts b/review-enrichment/src/analyzers/a11y-regression.ts index 156a5fdf2e..0d4b2beeaf 100644 --- a/review-enrichment/src/analyzers/a11y-regression.ts +++ b/review-enrichment/src/analyzers/a11y-regression.ts @@ -3,9 +3,10 @@ // positive tabindex values. Pure compute over added lines in .jsx/.tsx/.html/.vue files, no network. import type { A11yFinding, EnrichRequest } from "../types.js"; import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; const MARKUP_PATH_RE = /\.(?:tsx|jsx|html|vue)$/i; diff --git a/review-enrichment/src/analyzers/api-break.ts b/review-enrichment/src/analyzers/api-break.ts index 676a32d304..0d8330641b 100644 --- a/review-enrichment/src/analyzers/api-break.ts +++ b/review-enrichment/src/analyzers/api-break.ts @@ -8,9 +8,10 @@ // exact whole-name loss is; a non-entrypoint file is out of scope (that is the caller-impact analyzer's job). // Deterministic, no network, no token. Reports file, old-file line, and symbol only — never surrounding code. import type { ApiBreakFinding, EnrichRequest } from "../types.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const MAX_ENTRYPOINTS = 25; // cap changed entrypoint files scanned per PR -const MAX_FINDINGS = 25; // keep the brief bounded +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; // Files whose top-level exports form a package's PUBLIC surface: barrel/entry modules only. Restricting to these // entrypoint basenames keeps the signal conservative — a removed export in an internal module is not a downstream diff --git a/review-enrichment/src/analyzers/caller-impact.ts b/review-enrichment/src/analyzers/caller-impact.ts index 65630e7bb7..64f1da9400 100644 --- a/review-enrichment/src/analyzers/caller-impact.ts +++ b/review-enrichment/src/analyzers/caller-impact.ts @@ -29,6 +29,7 @@ import type { AnalysisContext } from "../analysis-context.js"; import { boundedFetchJson } from "../external-fetch.js"; import { exportedNames, isPublicEntrypoint } from "./api-break.js"; import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const GITHUB_API = "https://api.github.com"; const GITHUB_API_VERSION = "2022-11-28"; @@ -37,7 +38,7 @@ const MAX_SYMBOLS = 6; // removed symbols searched per PR (Code Search rate budg const MAX_SEARCHES = 6; // bounded Code Search queries per PR const MAX_FILE_FETCHES = 12; // bounded candidate-caller content fetches per PR const MAX_CALLERS_PER_FINDING = 5; // caller paths listed per finding (keeps the brief bounded) -const MAX_FINDINGS = 25; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; const MIN_SYMBOL_LEN = 3; // skip 1-2 char names — too generic to search reliably const MAX_FETCH_BYTES = 1_000_000; const MAX_SEARCH_JSON_BYTES = 256 * 1024; diff --git a/review-enrichment/src/analyzers/commit-hygiene.ts b/review-enrichment/src/analyzers/commit-hygiene.ts index 264defe2a3..3aae789b25 100644 --- a/review-enrichment/src/analyzers/commit-hygiene.ts +++ b/review-enrichment/src/analyzers/commit-hygiene.ts @@ -16,11 +16,12 @@ import type { } from "../types.js"; import type { AnalysisContext } from "../analysis-context.js"; import { boundedFetchJson } from "../external-fetch.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const GITHUB_API = "https://api.github.com"; const SLUG_RE = /^[A-Za-z0-9._-]+$/; const MAX_COMMITS = 100; -const MAX_FINDINGS = 25; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; const SHA_PREFIX_LEN = 12; // Git's own autosquash markers (`git commit --fixup`/`--squash`, `git rebase -i --autosquash`) — a documented, diff --git a/review-enrichment/src/analyzers/commit-lint.ts b/review-enrichment/src/analyzers/commit-lint.ts index 39938fb71e..51c7f55cd8 100644 --- a/review-enrichment/src/analyzers/commit-lint.ts +++ b/review-enrichment/src/analyzers/commit-lint.ts @@ -12,11 +12,12 @@ import type { } from "../types.js"; import type { AnalysisContext } from "../analysis-context.js"; import { boundedFetchJson } from "../external-fetch.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const GITHUB_API = "https://api.github.com"; const SLUG_RE = /^[A-Za-z0-9._-]+$/; const MAX_COMMITS = 100; -const MAX_FINDINGS = 25; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; const SHA_PREFIX_LEN = 12; const MAX_SUBJECT_LEN = 72; // Conventional Commits / git convention soft cap for the subject line. diff --git a/review-enrichment/src/analyzers/complexity.ts b/review-enrichment/src/analyzers/complexity.ts index 6645bd68e0..9e8d248e97 100644 --- a/review-enrichment/src/analyzers/complexity.ts +++ b/review-enrichment/src/analyzers/complexity.ts @@ -27,10 +27,11 @@ import type { ComplexityFinding, EnrichRequest } from "../types.js"; import { codeOnly } from "./secret-log.js"; import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; export const DEFAULT_MAX_COMPLEXITY = 10; -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i; @@ -90,6 +91,34 @@ function braceDepthDelta(code: string): number { return depth; } +/** Given the current pending function (or none) and one added code line, return the updated pending + * state -- mutated in place and returned when tracking continues, freshly started when the line opens + * a new function, or null when neither applies. Extracted out of scanPatchForComplexity's own loop + * (rather than left as an inline if/else) to keep that loop's own control-flow nesting under this + * analyzer's sibling deep-nesting.ts threshold. Pure; deciding whether to flush (pending.depth <= 0) + * stays the caller's job so this has no dependency on flushFunction's closure. */ +function advancePendingFunction( + pending: PendingFunction | null, + body: string, + commented: boolean, + code: string, + newLine: number, +): PendingFunction | null { + if (pending) { + if (!commented) pending.complexity += countDecisionPoints(code); + pending.depth += braceDepthDelta(code); + return pending; + } + const name = functionNameFromLine(body); + if (!name) return null; + return { + name, + startLine: newLine, + complexity: 1 + (commented ? 0 : countDecisionPoints(code)), + depth: braceDepthDelta(code), + }; +} + type ScanLimits = { maxComplexity?: number; maxFindings?: number; @@ -150,22 +179,8 @@ export function scanPatchForComplexity( if (body.length <= MAX_LINE_CHARS) { const commented = isCommentLine(body); const code = codeOnly(body); - if (pending) { - if (!commented) pending.complexity += countDecisionPoints(code); - pending.depth += braceDepthDelta(code); - if (pending.depth <= 0) flushFunction(); - } else { - const name = functionNameFromLine(body); - if (name) { - pending = { - name, - startLine: newLine, - complexity: 1 + (commented ? 0 : countDecisionPoints(code)), - depth: braceDepthDelta(code), - }; - if (pending.depth <= 0) flushFunction(); - } - } + pending = advancePendingFunction(pending, body, commented, code, newLine); + if (pending && pending.depth <= 0) flushFunction(); } newLine++; } else { diff --git a/review-enrichment/src/analyzers/conflict-marker.ts b/review-enrichment/src/analyzers/conflict-marker.ts index 9080330c19..d924be1d90 100644 --- a/review-enrichment/src/analyzers/conflict-marker.ts +++ b/review-enrichment/src/analyzers/conflict-marker.ts @@ -4,8 +4,9 @@ // Detection is purely structural (a fixed run of seven identical characters at column 0), so there is no // comment/string state to track. Line-cited via hunk headers, mirroring the sibling local analyzers. import type { EnrichRequest, ConflictMarkerFinding } from "../types.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; -const MAX_FINDINGS = 25; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; // Git writes each conflict marker as EXACTLY seven identical characters at the start of the line. The ours/base/ // theirs markers may carry a trailing space + label (a branch or commit); the separator is a bare seven `=`. diff --git a/review-enrichment/src/analyzers/debug-leftover.ts b/review-enrichment/src/analyzers/debug-leftover.ts index b8c8154ca7..427cdb227e 100644 --- a/review-enrichment/src/analyzers/debug-leftover.ts +++ b/review-enrichment/src/analyzers/debug-leftover.ts @@ -1,90 +1,91 @@ -// Debug-leftover analyzer (#2015). Flags debugging leftovers introduced in the diff — `debugger;` statements -// and bare `console.*` / `print()` calls added to non-test source files. Distinct from the secret-log analyzer -// (which only fires on sensitive-value sinks); this catches plain debug noise regardless of payload. Pure compute, -// no network. String-literal content is stripped before matching so a `"console.log('hi')"` inside a string is -// not flagged. Line-cited via hunk headers, mirroring the sibling local analyzers. -import type { DebugLeftoverFinding, EnrichRequest } from "../types.js"; -import { codeOnly } from "./secret-log.js"; -import { isTestPath } from "./test-ratio.js"; - -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; - -const DEBUGGER_RE = /\bdebugger\s*;/; -const CONSOLE_RE = /\bconsole\s*\.\s*(?:log|debug|info|warn|error|trace|dir|table)\s*\(/; -const PRINT_RE = /(?= maxFindings) return findings; - } - } - newLine++; - } else if (!line.startsWith("-") && !line.startsWith("\\")) { - newLine++; - } - } - return findings; -} - -/** Analyzer entrypoint: scan every changed non-test file's added lines for debug leftovers. */ -export async function scanDebugLeftover( - req: EnrichRequest, - signal?: AbortSignal, -): Promise { - const findings: DebugLeftoverFinding[] = []; - for (const file of req.files ?? []) { - if (signal?.aborted) throw new Error("analyzer_aborted"); - if (!file.patch) continue; - for (const finding of scanPatchForDebugLeftover(file.path, file.patch, { - maxFindings: MAX_FINDINGS - findings.length, - signal, - })) { - findings.push(finding); - if (findings.length >= MAX_FINDINGS) return findings; - } - } - return findings; -} +// Debug-leftover analyzer (#2015). Flags debugging leftovers introduced in the diff — `debugger;` statements +// and bare `console.*` / `print()` calls added to non-test source files. Distinct from the secret-log analyzer +// (which only fires on sensitive-value sinks); this catches plain debug noise regardless of payload. Pure compute, +// no network. String-literal content is stripped before matching so a `"console.log('hi')"` inside a string is +// not flagged. Line-cited via hunk headers, mirroring the sibling local analyzers. +import type { DebugLeftoverFinding, EnrichRequest } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; + +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; + +const DEBUGGER_RE = /\bdebugger\s*;/; +const CONSOLE_RE = /\bconsole\s*\.\s*(?:log|debug|info|warn|error|trace|dir|table)\s*\(/; +const PRINT_RE = /(?= maxFindings) return findings; + } + } + newLine++; + } else if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed non-test file's added lines for debug leftovers. */ +export async function scanDebugLeftover( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: DebugLeftoverFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForDebugLeftover(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/deep-nesting.ts b/review-enrichment/src/analyzers/deep-nesting.ts index 84c1faf4b8..10ac49df7b 100644 --- a/review-enrichment/src/analyzers/deep-nesting.ts +++ b/review-enrichment/src/analyzers/deep-nesting.ts @@ -5,10 +5,11 @@ import type { DeepNestingFinding, EnrichRequest } from "../types.js"; import { codeOnly } from "./secret-log.js"; import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; export const DEFAULT_MAX_DEPTH = 4; -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; type BraceKind = "control" | "other"; diff --git a/review-enrichment/src/analyzers/dependency-diff.ts b/review-enrichment/src/analyzers/dependency-diff.ts index 32edb7023a..0281606f99 100644 --- a/review-enrichment/src/analyzers/dependency-diff.ts +++ b/review-enrichment/src/analyzers/dependency-diff.ts @@ -1,28 +1,29 @@ -// Dependency-diff inventory analyzer (#2020). Emits a neutral summary of direct dependency manifest -// changes — added, removed, or version-changed packages across package.json, requirements.txt, and go.mod. -// Distinct from the CVE-scanning dependency analyzer: no registry calls, pure compute over manifest patches. -import type { DependencyDiffFinding, EnrichRequest } from "../types.js"; -import { - extractDependencyInventoryChanges, - type ScanLimits, -} from "./dependency-scan.js"; - -const MAX_FINDINGS = 25; -const LIMITS: ScanLimits = { - maxManifestFiles: 20, - maxPatchLinesPerFile: 500, -}; - -/** Scan changed manifest patches for direct dependency inventory deltas. Pure. */ -export function scanDependencyDiff(req: EnrichRequest): DependencyDiffFinding[] { - return extractDependencyInventoryChanges(req.files ?? [], LIMITS, MAX_FINDINGS); -} - -/** Analyzer entrypoint: summarize direct dependency add/remove/change deltas from manifest patches. */ -export async function scanDependencyDiffInventory( - req: EnrichRequest, - signal?: AbortSignal, -): Promise { - if (signal?.aborted) throw new Error("analyzer_aborted"); - return scanDependencyDiff(req); -} +// Dependency-diff inventory analyzer (#2020). Emits a neutral summary of direct dependency manifest +// changes — added, removed, or version-changed packages across package.json, requirements.txt, and go.mod. +// Distinct from the CVE-scanning dependency analyzer: no registry calls, pure compute over manifest patches. +import type { DependencyDiffFinding, EnrichRequest } from "../types.js"; +import { + extractDependencyInventoryChanges, + type ScanLimits, +} from "./dependency-scan.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; + +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const LIMITS: ScanLimits = { + maxManifestFiles: 20, + maxPatchLinesPerFile: 500, +}; + +/** Scan changed manifest patches for direct dependency inventory deltas. Pure. */ +export function scanDependencyDiff(req: EnrichRequest): DependencyDiffFinding[] { + return extractDependencyInventoryChanges(req.files ?? [], LIMITS, MAX_FINDINGS); +} + +/** Analyzer entrypoint: summarize direct dependency add/remove/change deltas from manifest patches. */ +export async function scanDependencyDiffInventory( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) throw new Error("analyzer_aborted"); + return scanDependencyDiff(req); +} diff --git a/review-enrichment/src/analyzers/deprecated-dep.ts b/review-enrichment/src/analyzers/deprecated-dep.ts index aec64eb244..57874d2e11 100644 --- a/review-enrichment/src/analyzers/deprecated-dep.ts +++ b/review-enrichment/src/analyzers/deprecated-dep.ts @@ -9,10 +9,11 @@ // version, the change direction, the documented reason, and the recommended replacement — never manifest contents. import type { DeprecatedDependencyFinding, EnrichRequest } from "../types.js"; import { extractDependencyChanges } from "./dependency-scan.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const MAX_MANIFEST_FILES = 20; // bound manifest files parsed per PR const MAX_PATCH_LINES_PER_FILE = 500; // bound patch lines parsed per manifest -const MAX_FINDINGS = 25; // keep the brief bounded +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; interface DeprecationNote { reason: string; diff --git a/review-enrichment/src/analyzers/duplication-scan.ts b/review-enrichment/src/analyzers/duplication-scan.ts index 62785fb2c2..ff2d07ebbc 100644 --- a/review-enrichment/src/analyzers/duplication-scan.ts +++ b/review-enrichment/src/analyzers/duplication-scan.ts @@ -15,6 +15,7 @@ import type { } from "../types.js"; import type { AnalysisContext } from "../analysis-context.js"; import { boundedFetchJson } from "../external-fetch.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; const GITHUB_API = "https://api.github.com"; const GITHUB_API_VERSION = "2022-11-28"; @@ -22,7 +23,7 @@ const GITHUB_API_VERSION = "2022-11-28"; const MIN_RUN = 8; // a contiguous run of >= this many significant normalized lines is required to flag a duplicate const MAX_CANDIDATES = 40; // cap candidate files (closest-by-path first) we consider per scan const MAX_FETCHES = 30; // global cap on candidate blob fetches per scan -const MAX_FINDINGS = 25; // keep the brief bounded +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; const MIN_SIGNIFICANT_LEN = 12; // lines shorter than this (after trim) are treated as trivial and dropped const MAX_FILE_BYTES = 500_000; // skip an oversized candidate blob so one huge (likely generated) file can't eat the budget const MAX_TREE_JSON_BYTES = 4 * 1024 * 1024; // recursive git tree can be large; bound it like asset-weight does diff --git a/review-enrichment/src/analyzers/error-swallow.ts b/review-enrichment/src/analyzers/error-swallow.ts index b42cc57698..7cf1311e95 100644 --- a/review-enrichment/src/analyzers/error-swallow.ts +++ b/review-enrichment/src/analyzers/error-swallow.ts @@ -5,9 +5,10 @@ // of silent failures. Pure compute over added diff lines, no network. Scoped to JS/TS/Python/Go source files. import type { EnrichRequest, ErrorSwallowFinding } from "../types.js"; import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; const SOURCE_EXTS = new Set(["ts", "tsx", "js", "jsx", "mjs", "cjs", "mts", "cts", "py", "go"]); @@ -139,6 +140,43 @@ function flushPending(pending: PendingCatch): ErrorSwallowFinding["kind"] | null return bodySwallowsError(body, pending.binding); } +type PendingCatchResult = { + pending: PendingCatch | null; + finding: ErrorSwallowFinding["kind"] | null; + findingLine: number | null; +}; + +/** Continue tracking an already-open catch/error block for one more added line: buffer it and update its + * brace depth, then flush (classify + clear) once the depth balances back to zero. Pure. Extracted out + * of scanPatchForErrorSwallow's own loop to keep that loop's control-flow nesting under this analyzer's + * sibling deep-nesting.ts threshold. */ +function advancePendingCatch(pending: PendingCatch, body: string): PendingCatchResult { + const updated = updatePending(pending, body); + if (updated.depth > 0) return { pending: updated, finding: null, findingLine: null }; + return { pending: null, finding: flushPending(updated), findingLine: updated.startLine }; +} + +/** Look for a NEW error-handling block opening on one added line when nothing is currently pending: + * either an immediate single-line finding, or the start of a multi-line block whose closing brace + * hasn't appeared yet. Pure. Same extraction rationale as advancePendingCatch. */ +function tryStartPendingCatch(body: string, newLine: number): PendingCatchResult { + const kind = detectErrorSwallow(body); + if (kind) return { pending: null, finding: kind, findingLine: newLine }; + + const open = matchErrorOpen(body); + if (!open) return { pending: null, finding: null, findingLine: null }; + const braceIndex = body.indexOf("{", open.index ?? 0); + if (braceIndex < 0) return { pending: null, finding: null, findingLine: null }; + const depth = braceBalanceFrom(body, braceIndex); + if (depth <= 0) return { pending: null, finding: null, findingLine: null }; + + return { + pending: { startLine: newLine, binding: open[1] ?? null, body: body.slice(braceIndex), depth }, + finding: null, + findingLine: null, + }; +} + type ScanLimits = { maxFindings?: number; signal?: AbortSignal; @@ -175,38 +213,11 @@ export function scanPatchForErrorSwallow( if (line.startsWith("+")) { const body = line.slice(1); if (body.length <= MAX_LINE_CHARS) { - if (pending) { - pending = updatePending(pending, body); - if (pending.depth <= 0) { - const kind = flushPending(pending); - if (kind) { - pushFinding(pending.startLine, kind); - if (findings.length >= maxFindings) return findings; - } - pending = null; - } - } else { - const kind = detectErrorSwallow(body); - if (kind) { - pushFinding(newLine, kind); - if (findings.length >= maxFindings) return findings; - } else { - const open = matchErrorOpen(body); - if (open) { - const braceIndex = body.indexOf("{", open.index ?? 0); - if (braceIndex >= 0) { - const depth = braceBalanceFrom(body, braceIndex); - if (depth > 0) { - pending = { - startLine: newLine, - binding: open[1] ?? null, - body: body.slice(braceIndex), - depth, - }; - } - } - } - } + const result: PendingCatchResult = pending ? advancePendingCatch(pending, body) : tryStartPendingCatch(body, newLine); + pending = result.pending; + if (result.finding) { + pushFinding(result.findingLine ?? newLine, result.finding); + if (findings.length >= maxFindings) return findings; } } newLine++; diff --git a/review-enrichment/src/analyzers/exhaustiveness-drift.ts b/review-enrichment/src/analyzers/exhaustiveness-drift.ts index 7cabe56b61..30195a9f3b 100644 --- a/review-enrichment/src/analyzers/exhaustiveness-drift.ts +++ b/review-enrichment/src/analyzers/exhaustiveness-drift.ts @@ -1,357 +1,358 @@ -// Enum / literal-union exhaustiveness-drift analyzer (#2028). Flags when a PR adds a new enum member or string-literal -// union variant but a switch that previously covered every old member still omits the new one. Fetches changed type -// files and other changed consumer files at headSha (injected fetch), reverse-applies the patch to recover the -// pre-PR member set, and only reports high-confidence misses (explicit enum/union cases, no default branch). Bounded -// file-fetch caps; fail-safe on missing token/headSha, bad slug, or fetch errors. -import type { EnrichRequest, ExhaustivenessFinding } from "../types.js"; -import { reconstructOldContent } from "./doc-comment-drift.js"; -import { isDiffFileHeaderLine } from "./diff-lines.js"; -import { isTestPath } from "./test-ratio.js"; - -const GITHUB_API = "https://api.github.com"; -const SLUG_RE = /^[A-Za-z0-9._-]+$/; -const MAX_FILES = 10; +// Enum / literal-union exhaustiveness-drift analyzer (#2028). Flags when a PR adds a new enum member or string-literal +// union variant but a switch that previously covered every old member still omits the new one. Fetches changed type +// files and other changed consumer files at headSha (injected fetch), reverse-applies the patch to recover the +// pre-PR member set, and only reports high-confidence misses (explicit enum/union cases, no default branch). Bounded +// file-fetch caps; fail-safe on missing token/headSha, bad slug, or fetch errors. +import type { EnrichRequest, ExhaustivenessFinding } from "../types.js"; +import { reconstructOldContent } from "./doc-comment-drift.js"; +import { isDiffFileHeaderLine } from "./diff-lines.js"; +import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; + +const GITHUB_API = "https://api.github.com"; +const SLUG_RE = /^[A-Za-z0-9._-]+$/; +const MAX_FILES = 10; const MAX_FETCHES = 10; -const MAX_FINDINGS = 25; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; const MAX_FETCH_BYTES = 1_000_000; const MAX_SWITCH_HEADER_LINES = 25; const SOURCE_RE = /\.(?:ts|tsx|mts|cts|js|jsx|mjs|cjs)$/; -const SKIP_RE = /(?:\.d\.ts$|\.min\.|(?:^|\/)(?:dist|build|vendor)\/)/; - -const ENUM_DECL_RE = /^\s*(?:export\s+)?(?:declare\s+)?(?:const\s+)?enum\s+([A-Za-z_$][\w$]*)\s*\{/; -const ENUM_MEMBER_RE = /^\s*([A-Za-z_$][\w$]*)\s*(?:=\s*[^,{]+)?,?\s*(?:\/\/.*)?$/; -const UNION_DECL_RE = /^\s*(?:export\s+)?type\s+([A-Za-z_$][\w$]*)\s*=\s*/; -const UNION_MEMBER_RE = /^\s*\|\s*["']([^"']+)["']\s*/; -const DEFAULT_CASE_RE = /^\s*default\s*:/; - -interface ScanOptions { - signal?: AbortSignal; -} - -interface AddedMemberCandidate { - file: string; - unionName: string; - addedMember: string; - line: number; - kind: "enum" | "union"; -} - -function escapeRegExp(value: string): string { - return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&"); -} - -function isScannablePath(path: string): boolean { - return SOURCE_RE.test(path) && !SKIP_RE.test(path) && !isTestPath(path); -} - -function githubHeaders(token: string): Record { - return { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github.raw", - "X-GitHub-Api-Version": "2022-11-28", - }; -} - -async function readBoundedText(resp: Response, signal?: AbortSignal): Promise { - const length = Number(resp.headers.get("content-length")); - if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null; - if (!resp.body) return null; - const reader = resp.body.getReader(); - const decoder = new TextDecoder(); - let size = 0; - let text = ""; - try { - while (true) { - if (signal?.aborted) return null; - const { done, value } = await reader.read(); - if (done) break; - size += value.byteLength; - if (size > MAX_FETCH_BYTES) { - await reader.cancel(); - return null; - } - text += decoder.decode(value, { stream: true }); - } - text += decoder.decode(); - return text; - } finally { - reader.releaseLock(); - } -} - -async function fetchFileAtHead( - owner: string, - repo: string, - path: string, - headSha: string, - token: string, - fetchFn: typeof fetch, - signal: AbortSignal | undefined, -): Promise { - try { - const encoded = path.split("/").map(encodeURIComponent).join("/"); - const resp = await fetchFn( - `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`, - { headers: githubHeaders(token), signal }, - ); - if (!resp.ok) return null; - return await readBoundedText(resp, signal); - } catch { - return null; - } -} - -/** Walk a unified diff and collect newly added enum/union members with their declaring type name and new-file line. */ -export function parseAddedTypeMembers( - patch: string, -): Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> { - const out: Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> = []; - let newLine = 0; - let enumName: string | null = null; - let unionName: string | null = null; - let enumDepth = 0; - - for (const raw of patch.split("\n")) { - const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw); - if (header) { - newLine = Number(header[1]); - enumName = null; - unionName = null; - enumDepth = 0; - continue; - } - - const isAdd = raw.startsWith("+") && !isDiffFileHeaderLine(raw); - const isContext = !raw.startsWith("-") && !raw.startsWith("\\") && !isDiffFileHeaderLine(raw); - if (!isAdd && !isContext) continue; - - const line = isAdd ? raw.slice(1) : raw.startsWith(" ") ? raw.slice(1) : raw; - const enumDecl = ENUM_DECL_RE.exec(line); - if (enumDecl) { - enumName = enumDecl[1]!; - unionName = null; - enumDepth = (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; - } - const unionDecl = UNION_DECL_RE.exec(line); - if (unionDecl) { - unionName = unionDecl[1]!; - enumName = null; - enumDepth = 0; - } - - if (isAdd) { - if (enumName && enumDepth >= 0) { - const member = ENUM_MEMBER_RE.exec(line); - if (member && member[1] !== "const") { - out.push({ unionName: enumName, addedMember: member[1]!, line: newLine, kind: "enum" }); - } - } - const unionMember = UNION_MEMBER_RE.exec(line); - if (unionName && unionMember) { - out.push({ unionName, addedMember: unionMember[1]!, line: newLine, kind: "union" }); - } - newLine += 1; - } else { - if (enumName) { - enumDepth += (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; - if (enumDepth <= 0 && line.includes("}")) enumName = null; - } - newLine += 1; - } - } - return out; -} - -/** Extract the member names of a TS enum declaration from file content. Returns null when the enum is not found. */ -export function extractEnumMembers(content: string, enumName: string): Set | null { - const decl = new RegExp(`(?:export\\s+)?(?:declare\\s+)?(?:const\\s+)?enum\\s+${escapeRegExp(enumName)}\\s*\\{`).exec( - content, - ); - if (!decl) return null; - const start = decl.index + decl[0].length; - let depth = 1; - let i = start; - const members = new Set(); - let chunk = ""; - while (i < content.length && depth > 0) { - const ch = content[i]!; - if (ch === "{") depth += 1; - else if (ch === "}") depth -= 1; - if (depth === 1) chunk += ch; - i += 1; - } - for (const part of chunk.split(",")) { - const trimmed = part.trim(); - if (!trimmed || trimmed.startsWith("//")) continue; - const name = /^([A-Za-z_$][\w$]*)/.exec(trimmed); - if (name) members.add(name[1]!); - } - return members.size ? members : null; -} - -/** Extract string-literal members from a `type Name = ...` alias. Returns null when not found or ambiguous. */ -export function extractUnionMembers(content: string, unionName: string): Set | null { - const decl = new RegExp( - `(?:export\\s+)?type\\s+${escapeRegExp(unionName)}\\s*=\\s*([^;]+);`, - "s", - ).exec(content); - if (!decl) return null; - const literals = [...decl[1]!.matchAll(/["']([^"']+)["']/g)].map((m) => m[1]!); - return literals.length ? new Set(literals) : null; -} - -interface SwitchGap { - line: number; -} - -/** Find a switch that covered all `oldMembers` but omits `addedMember`. Skips switches with a default branch. */ -export function findExhaustivenessGap( - content: string, - kind: "enum" | "union", - typeName: string, - oldMembers: Set, - addedMember: string, -): SwitchGap | null { - const lines = content.split("\n"); - for (let i = 0; i < lines.length; i++) { - if (!/^\s*switch\s*\(/.test(lines[i]!)) continue; - const block = extractSwitchBlock(lines, i); - if (!block) continue; - if (block.some((l) => DEFAULT_CASE_RE.test(l))) continue; - const cases = kind === "enum" ? collectEnumCases(block, typeName) : collectUnionCases(block); - if (!oldMembers.size || ![...oldMembers].every((m) => cases.has(m))) continue; - if (cases.has(addedMember)) continue; - return { line: i + 1 }; - } - return null; -} - -function extractSwitchBlock(lines: string[], switchLine: number): string[] | null { - let depth = 0; +const SKIP_RE = /(?:\.d\.ts$|\.min\.|(?:^|\/)(?:dist|build|vendor)\/)/; + +const ENUM_DECL_RE = /^\s*(?:export\s+)?(?:declare\s+)?(?:const\s+)?enum\s+([A-Za-z_$][\w$]*)\s*\{/; +const ENUM_MEMBER_RE = /^\s*([A-Za-z_$][\w$]*)\s*(?:=\s*[^,{]+)?,?\s*(?:\/\/.*)?$/; +const UNION_DECL_RE = /^\s*(?:export\s+)?type\s+([A-Za-z_$][\w$]*)\s*=\s*/; +const UNION_MEMBER_RE = /^\s*\|\s*["']([^"']+)["']\s*/; +const DEFAULT_CASE_RE = /^\s*default\s*:/; + +interface ScanOptions { + signal?: AbortSignal; +} + +interface AddedMemberCandidate { + file: string; + unionName: string; + addedMember: string; + line: number; + kind: "enum" | "union"; +} + +function escapeRegExp(value: string): string { + return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&"); +} + +function isScannablePath(path: string): boolean { + return SOURCE_RE.test(path) && !SKIP_RE.test(path) && !isTestPath(path); +} + +function githubHeaders(token: string): Record { + return { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github.raw", + "X-GitHub-Api-Version": "2022-11-28", + }; +} + +async function readBoundedText(resp: Response, signal?: AbortSignal): Promise { + const length = Number(resp.headers.get("content-length")); + if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null; + if (!resp.body) return null; + const reader = resp.body.getReader(); + const decoder = new TextDecoder(); + let size = 0; + let text = ""; + try { + while (true) { + if (signal?.aborted) return null; + const { done, value } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > MAX_FETCH_BYTES) { + await reader.cancel(); + return null; + } + text += decoder.decode(value, { stream: true }); + } + text += decoder.decode(); + return text; + } finally { + reader.releaseLock(); + } +} + +async function fetchFileAtHead( + owner: string, + repo: string, + path: string, + headSha: string, + token: string, + fetchFn: typeof fetch, + signal: AbortSignal | undefined, +): Promise { + try { + const encoded = path.split("/").map(encodeURIComponent).join("/"); + const resp = await fetchFn( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`, + { headers: githubHeaders(token), signal }, + ); + if (!resp.ok) return null; + return await readBoundedText(resp, signal); + } catch { + return null; + } +} + +/** Walk a unified diff and collect newly added enum/union members with their declaring type name and new-file line. */ +export function parseAddedTypeMembers( + patch: string, +): Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> { + const out: Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> = []; + let newLine = 0; + let enumName: string | null = null; + let unionName: string | null = null; + let enumDepth = 0; + + for (const raw of patch.split("\n")) { + const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw); + if (header) { + newLine = Number(header[1]); + enumName = null; + unionName = null; + enumDepth = 0; + continue; + } + + const isAdd = raw.startsWith("+") && !isDiffFileHeaderLine(raw); + const isContext = !raw.startsWith("-") && !raw.startsWith("\\") && !isDiffFileHeaderLine(raw); + if (!isAdd && !isContext) continue; + + const line = isAdd ? raw.slice(1) : raw.startsWith(" ") ? raw.slice(1) : raw; + const enumDecl = ENUM_DECL_RE.exec(line); + if (enumDecl) { + enumName = enumDecl[1]!; + unionName = null; + enumDepth = (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; + } + const unionDecl = UNION_DECL_RE.exec(line); + if (unionDecl) { + unionName = unionDecl[1]!; + enumName = null; + enumDepth = 0; + } + + if (isAdd) { + if (enumName && enumDepth >= 0) { + const member = ENUM_MEMBER_RE.exec(line); + if (member && member[1] !== "const") { + out.push({ unionName: enumName, addedMember: member[1]!, line: newLine, kind: "enum" }); + } + } + const unionMember = UNION_MEMBER_RE.exec(line); + if (unionName && unionMember) { + out.push({ unionName, addedMember: unionMember[1]!, line: newLine, kind: "union" }); + } + newLine += 1; + } else { + if (enumName) { + enumDepth += (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; + if (enumDepth <= 0 && line.includes("}")) enumName = null; + } + newLine += 1; + } + } + return out; +} + +/** Extract the member names of a TS enum declaration from file content. Returns null when the enum is not found. */ +export function extractEnumMembers(content: string, enumName: string): Set | null { + const decl = new RegExp(`(?:export\\s+)?(?:declare\\s+)?(?:const\\s+)?enum\\s+${escapeRegExp(enumName)}\\s*\\{`).exec( + content, + ); + if (!decl) return null; + const start = decl.index + decl[0].length; + let depth = 1; + let i = start; + const members = new Set(); + let chunk = ""; + while (i < content.length && depth > 0) { + const ch = content[i]!; + if (ch === "{") depth += 1; + else if (ch === "}") depth -= 1; + if (depth === 1) chunk += ch; + i += 1; + } + for (const part of chunk.split(",")) { + const trimmed = part.trim(); + if (!trimmed || trimmed.startsWith("//")) continue; + const name = /^([A-Za-z_$][\w$]*)/.exec(trimmed); + if (name) members.add(name[1]!); + } + return members.size ? members : null; +} + +/** Extract string-literal members from a `type Name = ...` alias. Returns null when not found or ambiguous. */ +export function extractUnionMembers(content: string, unionName: string): Set | null { + const decl = new RegExp( + `(?:export\\s+)?type\\s+${escapeRegExp(unionName)}\\s*=\\s*([^;]+);`, + "s", + ).exec(content); + if (!decl) return null; + const literals = [...decl[1]!.matchAll(/["']([^"']+)["']/g)].map((m) => m[1]!); + return literals.length ? new Set(literals) : null; +} + +interface SwitchGap { + line: number; +} + +/** Find a switch that covered all `oldMembers` but omits `addedMember`. Skips switches with a default branch. */ +export function findExhaustivenessGap( + content: string, + kind: "enum" | "union", + typeName: string, + oldMembers: Set, + addedMember: string, +): SwitchGap | null { + const lines = content.split("\n"); + for (let i = 0; i < lines.length; i++) { + if (!/^\s*switch\s*\(/.test(lines[i]!)) continue; + const block = extractSwitchBlock(lines, i); + if (!block) continue; + if (block.some((l) => DEFAULT_CASE_RE.test(l))) continue; + const cases = kind === "enum" ? collectEnumCases(block, typeName) : collectUnionCases(block); + if (!oldMembers.size || ![...oldMembers].every((m) => cases.has(m))) continue; + if (cases.has(addedMember)) continue; + return { line: i + 1 }; + } + return null; +} + +function extractSwitchBlock(lines: string[], switchLine: number): string[] | null { + let depth = 0; let started = false; const block: string[] = []; for (let i = switchLine; i < lines.length; i++) { if (!started && i - switchLine >= MAX_SWITCH_HEADER_LINES) return null; const line = lines[i]!; - block.push(line); - for (const ch of line) { - if (ch === "{") { - depth += 1; - started = true; - } else if (ch === "}") depth -= 1; - } - if (started && depth === 0) return block; - } - return null; -} - -function collectEnumCases(block: string[], enumName: string): Set { - const cases = new Set(); - const qualified = new RegExp(`case\\s+${escapeRegExp(enumName)}\\.([A-Za-z_$][\\w$]*)\\s*:`); - const bare = /case\s+([A-Za-z_$][\w$]*)\s*:/; - for (const line of block) { - const q = qualified.exec(line); - if (q) cases.add(q[1]!); - else { - const b = bare.exec(line); - if (b) cases.add(b[1]!); - } - } - return cases; -} - -function collectUnionCases(block: string[]): Set { - const cases = new Set(); - const re = /case\s+["']([^"']+)["']\s*:/; - for (const line of block) { - const match = re.exec(line); - if (match) cases.add(match[1]!); - } - return cases; -} - -/** Analyzer entrypoint. Fail-safe — returns no finding on missing token/headSha or fetch errors. */ -export async function scanExhaustivenessDrift( - req: EnrichRequest, - fetchFn: typeof fetch = fetch, - options: ScanOptions = {}, -): Promise { - const { repoFullName, githubToken, headSha, files = [] } = req; - if (!githubToken || !headSha) return []; - const parts = repoFullName.split("/"); - const [owner, repo] = parts; - if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; - - const candidates: AddedMemberCandidate[] = []; - for (const file of files) { - if (!file.patch || !isScannablePath(file.path)) continue; - for (const item of parseAddedTypeMembers(file.patch)) { - candidates.push({ file: file.path, ...item }); - } - } - if (!candidates.length) return []; - - const scannableFiles = files.filter((f) => f.patch && isScannablePath(f.path)).slice(0, MAX_FILES); - const contentCache = new Map(); - let fetches = 0; - - const loadFile = async (path: string, patch?: string): Promise => { - if (contentCache.has(path)) return contentCache.get(path) ?? null; - if (fetches >= MAX_FETCHES) { - contentCache.set(path, null); - return null; - } - fetches += 1; - const content = await fetchFileAtHead(owner, repo, path, headSha, githubToken, fetchFn, options.signal); - contentCache.set(path, content); - return content; - }; - - const findings: ExhaustivenessFinding[] = []; - const seen = new Set(); - - for (const candidate of candidates) { - if (options.signal?.aborted) break; - if (findings.length >= MAX_FINDINGS) break; - if (!scannableFiles.some((f) => f.path === candidate.file)) continue; - - const typeFile = files.find((f) => f.path === candidate.file); - if (!typeFile?.patch) continue; - const headContent = await loadFile(candidate.file, typeFile.patch); - if (!headContent) continue; - const oldContent = reconstructOldContent(headContent, typeFile.patch); - if (!oldContent) continue; - - const extract = candidate.kind === "enum" ? extractEnumMembers : extractUnionMembers; - const oldMembers = extract(oldContent, candidate.unionName); - const newMembers = extract(headContent, candidate.unionName); - if (!oldMembers || !newMembers) continue; - if (!newMembers.has(candidate.addedMember) || oldMembers.has(candidate.addedMember)) continue; - - for (const consumer of scannableFiles) { - const consumerContent = - consumer.path === candidate.file ? headContent : await loadFile(consumer.path, consumer.patch); - if (!consumerContent) continue; - const gap = findExhaustivenessGap( - consumerContent, - candidate.kind, - candidate.unionName, - oldMembers, - candidate.addedMember, - ); - if (!gap) continue; - const key = `${consumer.path}:${gap.line}:${candidate.unionName}:${candidate.addedMember}`; - if (seen.has(key)) continue; - seen.add(key); - findings.push({ - file: candidate.file, - line: candidate.line, - unionName: candidate.unionName, - addedMember: candidate.addedMember, - ...(consumer.path !== candidate.file ? { consumerFile: consumer.path } : {}), - }); - break; - } - } - return findings; -} + block.push(line); + for (const ch of line) { + if (ch === "{") { + depth += 1; + started = true; + } else if (ch === "}") depth -= 1; + } + if (started && depth === 0) return block; + } + return null; +} + +function collectEnumCases(block: string[], enumName: string): Set { + const cases = new Set(); + const qualified = new RegExp(`case\\s+${escapeRegExp(enumName)}\\.([A-Za-z_$][\\w$]*)\\s*:`); + const bare = /case\s+([A-Za-z_$][\w$]*)\s*:/; + for (const line of block) { + const q = qualified.exec(line); + if (q) cases.add(q[1]!); + else { + const b = bare.exec(line); + if (b) cases.add(b[1]!); + } + } + return cases; +} + +function collectUnionCases(block: string[]): Set { + const cases = new Set(); + const re = /case\s+["']([^"']+)["']\s*:/; + for (const line of block) { + const match = re.exec(line); + if (match) cases.add(match[1]!); + } + return cases; +} + +/** Analyzer entrypoint. Fail-safe — returns no finding on missing token/headSha or fetch errors. */ +export async function scanExhaustivenessDrift( + req: EnrichRequest, + fetchFn: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const { repoFullName, githubToken, headSha, files = [] } = req; + if (!githubToken || !headSha) return []; + const parts = repoFullName.split("/"); + const [owner, repo] = parts; + if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; + + const candidates: AddedMemberCandidate[] = []; + for (const file of files) { + if (!file.patch || !isScannablePath(file.path)) continue; + for (const item of parseAddedTypeMembers(file.patch)) { + candidates.push({ file: file.path, ...item }); + } + } + if (!candidates.length) return []; + + const scannableFiles = files.filter((f) => f.patch && isScannablePath(f.path)).slice(0, MAX_FILES); + const contentCache = new Map(); + let fetches = 0; + + const loadFile = async (path: string, patch?: string): Promise => { + if (contentCache.has(path)) return contentCache.get(path) ?? null; + if (fetches >= MAX_FETCHES) { + contentCache.set(path, null); + return null; + } + fetches += 1; + const content = await fetchFileAtHead(owner, repo, path, headSha, githubToken, fetchFn, options.signal); + contentCache.set(path, content); + return content; + }; + + const findings: ExhaustivenessFinding[] = []; + const seen = new Set(); + + for (const candidate of candidates) { + if (options.signal?.aborted) break; + if (findings.length >= MAX_FINDINGS) break; + if (!scannableFiles.some((f) => f.path === candidate.file)) continue; + + const typeFile = files.find((f) => f.path === candidate.file); + if (!typeFile?.patch) continue; + const headContent = await loadFile(candidate.file, typeFile.patch); + if (!headContent) continue; + const oldContent = reconstructOldContent(headContent, typeFile.patch); + if (!oldContent) continue; + + const extract = candidate.kind === "enum" ? extractEnumMembers : extractUnionMembers; + const oldMembers = extract(oldContent, candidate.unionName); + const newMembers = extract(headContent, candidate.unionName); + if (!oldMembers || !newMembers) continue; + if (!newMembers.has(candidate.addedMember) || oldMembers.has(candidate.addedMember)) continue; + + for (const consumer of scannableFiles) { + const consumerContent = + consumer.path === candidate.file ? headContent : await loadFile(consumer.path, consumer.patch); + if (!consumerContent) continue; + const gap = findExhaustivenessGap( + consumerContent, + candidate.kind, + candidate.unionName, + oldMembers, + candidate.addedMember, + ); + if (!gap) continue; + const key = `${consumer.path}:${gap.line}:${candidate.unionName}:${candidate.addedMember}`; + if (seen.has(key)) continue; + seen.add(key); + findings.push({ + file: candidate.file, + line: candidate.line, + unionName: candidate.unionName, + addedMember: candidate.addedMember, + ...(consumer.path !== candidate.file ? { consumerFile: consumer.path } : {}), + }); + break; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/flaky-test.ts b/review-enrichment/src/analyzers/flaky-test.ts index 4f83f3e4c2..d1b9abf7a7 100644 --- a/review-enrichment/src/analyzers/flaky-test.ts +++ b/review-enrichment/src/analyzers/flaky-test.ts @@ -1,214 +1,215 @@ -// Flaky-test history annotator (#2033). For test files a PR touches, probes bounded recent default-branch commit + -// check-run history and counts CI test-check failures whose output references that file — a signal the change -// lands on historically flaky coverage. Structured GitHub API fields only in findings (counts + window, never -// logs). Bounded file/commit/check-run fanout; marks partial status when the probe budget is exhausted. Fail-safe -// without a token, bad slug, or fetch errors. -import type { - AnalyzerDiagnostics, - EnrichRequest, - FlakyTestFinding, -} from "../types.js"; -import type { AnalysisContext } from "../analysis-context.js"; -import { boundedFetchJson } from "../external-fetch.js"; -import { isTestPath } from "./test-ratio.js"; - -const GITHUB_API = "https://api.github.com"; -const SLUG_RE = /^[A-Za-z0-9._-]+$/; -const WINDOW_DAYS = 30; -const WINDOW_LABEL = "30d"; -const MAX_FILES_PROBED = 6; -const MAX_COMMITS_PER_FILE = 5; -const MAX_CHECK_RUN_FETCHES = 12; -const MAX_FINDINGS = 25; -const MIN_FAILURE_EVENTS = 2; -const COMMITS_PER_PAGE = 20; - -const FAILURE_CONCLUSIONS = new Set(["failure", "timed_out", "cancelled", "action_required"]); -const TEST_CHECK_RE = - /test|jest|vitest|pytest|mocha|rspec|unittest|validate-code|go test|cargo test|npm test|yarn test|pnpm test/i; - -interface ScanOptions { - signal?: AbortSignal; - analysis?: Pick; - diagnostics?: AnalyzerDiagnostics; -} - -interface CommitItem { - sha?: string; -} - -interface CheckRunItem { - name?: string; - status?: string; - conclusion?: string | null; - output?: { title?: string | null; summary?: string | null; text?: string | null }; -} - -interface CheckRunsResponse { - check_runs?: CheckRunItem[]; -} - -interface RepoInfo { - default_branch?: string; -} - -function githubHeaders(token: string): Record { - return { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - }; -} - -function markPartial(diagnostics: AnalyzerDiagnostics | undefined, reason: string): void { - if (!diagnostics) return; - diagnostics.partialStatus = "partial"; - diagnostics.partialReason ??= reason; -} - -/** True when a check-run name looks like a test/CI test job rather than lint/build-only. Pure. */ -export function isTestCheckName(name: string): boolean { - return TEST_CHECK_RE.test(name); -} - -/** True when structured check-run output references the test file path or its basename. Pure. */ -export function referencesTestFile( - output: { title?: string | null; summary?: string | null; text?: string | null } | undefined, - filePath: string, -): boolean { - if (!output) return false; - const haystack = `${output.title ?? ""}\n${output.summary ?? ""}\n${output.text ?? ""}`; - if (haystack.includes(filePath)) return true; - const base = filePath.split("/").pop() ?? filePath; - if (base && haystack.includes(base)) return true; - const stem = base.replace(/\.[^.]+$/, ""); - return Boolean(stem && stem.length >= 3 && haystack.includes(stem)); -} - -/** Count completed test-check failures on one commit's runs that reference `filePath`. Pure. */ -export function countCommitTestFailures(runs: CheckRunItem[], filePath: string): number { - let failures = 0; - for (const run of runs) { - if (run.status !== "completed" || !run.name || !run.conclusion) continue; - if (!FAILURE_CONCLUSIONS.has(run.conclusion)) continue; - if (!isTestCheckName(run.name)) continue; - if (!referencesTestFile(run.output, filePath)) continue; - failures += 1; - } - return failures; -} - -async function fetchJson( - url: string, - token: string, - fetchFn: typeof fetch, - options: ScanOptions, - endpointCategory: string, - phase: string, - subcall: string, - maxCallsPerCategory?: number, -): Promise { - const fetchOptions = { - endpointCategory, - headers: githubHeaders(token), - signal: options.signal, - fetchImpl: fetchFn, - diagnostics: options.diagnostics, - phase, - subcall, - maxBytes: 512 * 1024, - maxCallsPerCategory, - }; - const response = options.analysis - ? await options.analysis.fetchJson(url, fetchOptions) - : await boundedFetchJson(url, fetchOptions); - return response.ok ? response.data : null; -} - -/** Analyzer entrypoint. Fail-safe — returns no finding without a token or on fetch errors. */ -export async function scanFlakyTest( - req: EnrichRequest, - fetchFn: typeof fetch = fetch, - options: ScanOptions = {}, -): Promise { - const { repoFullName, githubToken, files = [] } = req; - if (!githubToken) return []; - const parts = repoFullName.split("/"); - if (parts.length !== 2) return []; - const [owner, repo] = parts; - if (!owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; - - const testPaths = [...new Set(files.filter((f) => isTestPath(f.path)).map((f) => f.path))].slice( - 0, - MAX_FILES_PROBED, - ); - if (!testPaths.length) return []; - - const repoInfo = await fetchJson( - `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`, - githubToken, - fetchFn, - options, - "github-repo-info", - "flaky-test", - "default-branch", - 1, - ); - const defaultBranch = repoInfo?.default_branch; - if (!defaultBranch) return []; - - const since = new Date(Date.now() - WINDOW_DAYS * 86_400_000).toISOString(); - const findings: FlakyTestFinding[] = []; - let checkRunFetches = 0; - let capped = false; - - for (const path of testPaths) { - if (options.signal?.aborted) break; - if (findings.length >= MAX_FINDINGS) break; - - const commits = await fetchJson( - `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits` + - `?sha=${encodeURIComponent(defaultBranch)}&path=${encodeURIComponent(path)}` + - `&since=${encodeURIComponent(since)}&per_page=${COMMITS_PER_PAGE}`, - githubToken, - fetchFn, - options, - "github-commits", - "flaky-test", - "file-commits", - MAX_FILES_PROBED, - ); - if (!commits?.length) continue; - - let failureEvents = 0; - for (const commit of commits.slice(0, MAX_COMMITS_PER_FILE)) { - if (!commit.sha) continue; - if (checkRunFetches >= MAX_CHECK_RUN_FETCHES) { - capped = true; - break; - } - checkRunFetches += 1; - const checkData = await fetchJson( - `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/` + - `${encodeURIComponent(commit.sha)}/check-runs?per_page=100&filter=all`, - githubToken, - fetchFn, - options, - "github-check-runs", - "flaky-test", - "commit-check-runs", - MAX_CHECK_RUN_FETCHES, - ); - const runs = checkData?.check_runs ?? []; - if (countCommitTestFailures(runs, path) > 0) failureEvents += 1; - } - - if (failureEvents >= MIN_FAILURE_EVENTS) { - findings.push({ file: path, recentFailures: failureEvents, window: WINDOW_LABEL }); - } - if (capped) break; - } - - if (capped) markPartial(options.diagnostics, "flaky_test_probe_cap"); - return findings; -} +// Flaky-test history annotator (#2033). For test files a PR touches, probes bounded recent default-branch commit + +// check-run history and counts CI test-check failures whose output references that file — a signal the change +// lands on historically flaky coverage. Structured GitHub API fields only in findings (counts + window, never +// logs). Bounded file/commit/check-run fanout; marks partial status when the probe budget is exhausted. Fail-safe +// without a token, bad slug, or fetch errors. +import type { + AnalyzerDiagnostics, + EnrichRequest, + FlakyTestFinding, +} from "../types.js"; +import type { AnalysisContext } from "../analysis-context.js"; +import { boundedFetchJson } from "../external-fetch.js"; +import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; + +const GITHUB_API = "https://api.github.com"; +const SLUG_RE = /^[A-Za-z0-9._-]+$/; +const WINDOW_DAYS = 30; +const WINDOW_LABEL = "30d"; +const MAX_FILES_PROBED = 6; +const MAX_COMMITS_PER_FILE = 5; +const MAX_CHECK_RUN_FETCHES = 12; +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MIN_FAILURE_EVENTS = 2; +const COMMITS_PER_PAGE = 20; + +const FAILURE_CONCLUSIONS = new Set(["failure", "timed_out", "cancelled", "action_required"]); +const TEST_CHECK_RE = + /test|jest|vitest|pytest|mocha|rspec|unittest|validate-code|go test|cargo test|npm test|yarn test|pnpm test/i; + +interface ScanOptions { + signal?: AbortSignal; + analysis?: Pick; + diagnostics?: AnalyzerDiagnostics; +} + +interface CommitItem { + sha?: string; +} + +interface CheckRunItem { + name?: string; + status?: string; + conclusion?: string | null; + output?: { title?: string | null; summary?: string | null; text?: string | null }; +} + +interface CheckRunsResponse { + check_runs?: CheckRunItem[]; +} + +interface RepoInfo { + default_branch?: string; +} + +function githubHeaders(token: string): Record { + return { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; +} + +function markPartial(diagnostics: AnalyzerDiagnostics | undefined, reason: string): void { + if (!diagnostics) return; + diagnostics.partialStatus = "partial"; + diagnostics.partialReason ??= reason; +} + +/** True when a check-run name looks like a test/CI test job rather than lint/build-only. Pure. */ +export function isTestCheckName(name: string): boolean { + return TEST_CHECK_RE.test(name); +} + +/** True when structured check-run output references the test file path or its basename. Pure. */ +export function referencesTestFile( + output: { title?: string | null; summary?: string | null; text?: string | null } | undefined, + filePath: string, +): boolean { + if (!output) return false; + const haystack = `${output.title ?? ""}\n${output.summary ?? ""}\n${output.text ?? ""}`; + if (haystack.includes(filePath)) return true; + const base = filePath.split("/").pop() ?? filePath; + if (base && haystack.includes(base)) return true; + const stem = base.replace(/\.[^.]+$/, ""); + return Boolean(stem && stem.length >= 3 && haystack.includes(stem)); +} + +/** Count completed test-check failures on one commit's runs that reference `filePath`. Pure. */ +export function countCommitTestFailures(runs: CheckRunItem[], filePath: string): number { + let failures = 0; + for (const run of runs) { + if (run.status !== "completed" || !run.name || !run.conclusion) continue; + if (!FAILURE_CONCLUSIONS.has(run.conclusion)) continue; + if (!isTestCheckName(run.name)) continue; + if (!referencesTestFile(run.output, filePath)) continue; + failures += 1; + } + return failures; +} + +async function fetchJson( + url: string, + token: string, + fetchFn: typeof fetch, + options: ScanOptions, + endpointCategory: string, + phase: string, + subcall: string, + maxCallsPerCategory?: number, +): Promise { + const fetchOptions = { + endpointCategory, + headers: githubHeaders(token), + signal: options.signal, + fetchImpl: fetchFn, + diagnostics: options.diagnostics, + phase, + subcall, + maxBytes: 512 * 1024, + maxCallsPerCategory, + }; + const response = options.analysis + ? await options.analysis.fetchJson(url, fetchOptions) + : await boundedFetchJson(url, fetchOptions); + return response.ok ? response.data : null; +} + +/** Analyzer entrypoint. Fail-safe — returns no finding without a token or on fetch errors. */ +export async function scanFlakyTest( + req: EnrichRequest, + fetchFn: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const { repoFullName, githubToken, files = [] } = req; + if (!githubToken) return []; + const parts = repoFullName.split("/"); + if (parts.length !== 2) return []; + const [owner, repo] = parts; + if (!owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; + + const testPaths = [...new Set(files.filter((f) => isTestPath(f.path)).map((f) => f.path))].slice( + 0, + MAX_FILES_PROBED, + ); + if (!testPaths.length) return []; + + const repoInfo = await fetchJson( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`, + githubToken, + fetchFn, + options, + "github-repo-info", + "flaky-test", + "default-branch", + 1, + ); + const defaultBranch = repoInfo?.default_branch; + if (!defaultBranch) return []; + + const since = new Date(Date.now() - WINDOW_DAYS * 86_400_000).toISOString(); + const findings: FlakyTestFinding[] = []; + let checkRunFetches = 0; + let capped = false; + + for (const path of testPaths) { + if (options.signal?.aborted) break; + if (findings.length >= MAX_FINDINGS) break; + + const commits = await fetchJson( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits` + + `?sha=${encodeURIComponent(defaultBranch)}&path=${encodeURIComponent(path)}` + + `&since=${encodeURIComponent(since)}&per_page=${COMMITS_PER_PAGE}`, + githubToken, + fetchFn, + options, + "github-commits", + "flaky-test", + "file-commits", + MAX_FILES_PROBED, + ); + if (!commits?.length) continue; + + let failureEvents = 0; + for (const commit of commits.slice(0, MAX_COMMITS_PER_FILE)) { + if (!commit.sha) continue; + if (checkRunFetches >= MAX_CHECK_RUN_FETCHES) { + capped = true; + break; + } + checkRunFetches += 1; + const checkData = await fetchJson( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/` + + `${encodeURIComponent(commit.sha)}/check-runs?per_page=100&filter=all`, + githubToken, + fetchFn, + options, + "github-check-runs", + "flaky-test", + "commit-check-runs", + MAX_CHECK_RUN_FETCHES, + ); + const runs = checkData?.check_runs ?? []; + if (countCommitTestFailures(runs, path) > 0) failureEvents += 1; + } + + if (failureEvents >= MIN_FAILURE_EVENTS) { + findings.push({ file: path, recentFailures: failureEvents, window: WINDOW_LABEL }); + } + if (capped) break; + } + + if (capped) markPartial(options.diagnostics, "flaky_test_probe_cap"); + return findings; +} diff --git a/review-enrichment/src/analyzers/floating-promise.ts b/review-enrichment/src/analyzers/floating-promise.ts index 764302ca5c..f8e2886bc6 100644 --- a/review-enrichment/src/analyzers/floating-promise.ts +++ b/review-enrichment/src/analyzers/floating-promise.ts @@ -1,134 +1,135 @@ -// Floating-promise analyzer (#2023). Flags newly-added async-shaped calls whose returned promise is neither -// awaited, returned, voided, nor .catch()/.then()-chained on the same statement — a common silent-failure bug. -// Precision-first structural heuristic over added TS/JS lines only: promise-shaped callees (`fetch`, `Promise.*`, -// or an `*Async` suffix) on bare expression statements. Pure compute, no network. -import type { EnrichRequest, FloatingPromiseFinding } from "../types.js"; -import { codeOnly } from "./secret-log.js"; -import { isTestPath } from "./test-ratio.js"; - -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; -const MAX_CALL_CHARS = 40; - -const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i; - -const HANDLED_PREFIX = - /^\s*(?:await\b|return\b|void\b|throw\b|if\b|for\b|while\b|switch\b|case\b|else\b|try\b|catch\b|finally\b|import\b|export\b|const\b|let\b|var\b|type\b|interface\b|class\b|function\b|async\s+function\b)/; - -const PROMISE_CHAIN_RE = /\.(?:then|catch)\s*\(/; - -function isJsTsPath(path: string): boolean { - return JS_TS_PATH_RE.test(path) && !isTestPath(path); -} - -function isCommentLine(line: string): boolean { - const trimmed = line.trimStart(); - return /^(?:\/\/|\/\*|\*)/.test(trimmed); -} - -function truncateCall(call: string): string { - if (call.length <= MAX_CALL_CHARS) return call; - return `${call.slice(0, MAX_CALL_CHARS - 3)}...`; -} - -function isPromiseShapedCallee(callee: string): boolean { - if (callee === "fetch" || callee.endsWith(".fetch")) return true; - if (callee === "Promise" || /^Promise\.(?:all(?:Settled)?|race|any|resolve|reject)$/.test(callee)) { - return true; - } - const last = callee.split(".").pop() ?? callee; - return /Async$/.test(last); -} - -function extractLeadingCallCallee(line: string): string | null { - const code = codeOnly(line).trim(); - const semiIdx = code.indexOf(";"); - if (semiIdx >= 0 && semiIdx < code.length - 1) { - const after = code.slice(semiIdx + 1).trim(); - if (after.length > 0) return null; - } - - const newPromise = /^new\s+Promise\s*\(/.exec(code); - if (newPromise) return "Promise"; - - const match = /^((?:[a-zA-Z_$][\w$]*(?:\.[a-zA-Z_$][\w$]*)*))\s*\(/.exec(code); - return match?.[1] ?? null; -} - -/** Classify one added line for a floating promise call, or null. Pure. */ -export function detectFloatingPromise(line: string): string | null { - if (isCommentLine(line) || HANDLED_PREFIX.test(line)) { - return null; - } - - const code = codeOnly(line).replace(/=>/g, " "); - if (PROMISE_CHAIN_RE.test(code)) return null; - if (/(?!])=(?!=)/.test(code)) return null; - - const callee = extractLeadingCallCallee(line); - if (!callee || !isPromiseShapedCallee(callee)) return null; - - return truncateCall(callee); -} - -type ScanLimits = { - maxFindings?: number; - signal?: AbortSignal; -}; - -/** Scan one file patch's added lines for floating promises, line-cited via hunk headers. Pure. */ -export function scanPatchForFloatingPromise( - path: string, - patch: string, - limits: ScanLimits = {}, -): FloatingPromiseFinding[] { - const maxFindings = limits.maxFindings ?? MAX_FINDINGS; - if (maxFindings <= 0 || !isJsTsPath(path)) return []; - const findings: FloatingPromiseFinding[] = []; - let newLine = 0; - let inHunk = false; - for (const line of patch.split("\n")) { - if (limits.signal?.aborted) throw new Error("analyzer_aborted"); - const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); - if (hunk) { - newLine = Number(hunk[1]); - inHunk = true; - continue; - } - if (!inHunk) continue; - if (line.startsWith("+")) { - const body = line.slice(1); - if (body.length <= MAX_LINE_CHARS) { - const call = detectFloatingPromise(body); - if (call) { - findings.push({ file: path, line: newLine, call }); - if (findings.length >= maxFindings) return findings; - } - } - newLine++; - } else if (!line.startsWith("-") && !line.startsWith("\\")) { - newLine++; - } - } - return findings; -} - -/** Analyzer entrypoint: scan every changed TS/JS file's added lines for floating promises. */ -export async function scanFloatingPromise( - req: EnrichRequest, - signal?: AbortSignal, -): Promise { - const findings: FloatingPromiseFinding[] = []; - for (const file of req.files ?? []) { - if (signal?.aborted) throw new Error("analyzer_aborted"); - if (!file.patch) continue; - for (const finding of scanPatchForFloatingPromise(file.path, file.patch, { - maxFindings: MAX_FINDINGS - findings.length, - signal, - })) { - findings.push(finding); - if (findings.length >= MAX_FINDINGS) return findings; - } - } - return findings; -} +// Floating-promise analyzer (#2023). Flags newly-added async-shaped calls whose returned promise is neither +// awaited, returned, voided, nor .catch()/.then()-chained on the same statement — a common silent-failure bug. +// Precision-first structural heuristic over added TS/JS lines only: promise-shaped callees (`fetch`, `Promise.*`, +// or an `*Async` suffix) on bare expression statements. Pure compute, no network. +import type { EnrichRequest, FloatingPromiseFinding } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; +import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js"; + +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; +const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS; +const MAX_CALL_CHARS = 40; + +const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i; + +const HANDLED_PREFIX = + /^\s*(?:await\b|return\b|void\b|throw\b|if\b|for\b|while\b|switch\b|case\b|else\b|try\b|catch\b|finally\b|import\b|export\b|const\b|let\b|var\b|type\b|interface\b|class\b|function\b|async\s+function\b)/; + +const PROMISE_CHAIN_RE = /\.(?:then|catch)\s*\(/; + +function isJsTsPath(path: string): boolean { + return JS_TS_PATH_RE.test(path) && !isTestPath(path); +} + +function isCommentLine(line: string): boolean { + const trimmed = line.trimStart(); + return /^(?:\/\/|\/\*|\*)/.test(trimmed); +} + +function truncateCall(call: string): string { + if (call.length <= MAX_CALL_CHARS) return call; + return `${call.slice(0, MAX_CALL_CHARS - 3)}...`; +} + +function isPromiseShapedCallee(callee: string): boolean { + if (callee === "fetch" || callee.endsWith(".fetch")) return true; + if (callee === "Promise" || /^Promise\.(?:all(?:Settled)?|race|any|resolve|reject)$/.test(callee)) { + return true; + } + const last = callee.split(".").pop() ?? callee; + return /Async$/.test(last); +} + +function extractLeadingCallCallee(line: string): string | null { + const code = codeOnly(line).trim(); + const semiIdx = code.indexOf(";"); + if (semiIdx >= 0 && semiIdx < code.length - 1) { + const after = code.slice(semiIdx + 1).trim(); + if (after.length > 0) return null; + } + + const newPromise = /^new\s+Promise\s*\(/.exec(code); + if (newPromise) return "Promise"; + + const match = /^((?:[a-zA-Z_$][\w$]*(?:\.[a-zA-Z_$][\w$]*)*))\s*\(/.exec(code); + return match?.[1] ?? null; +} + +/** Classify one added line for a floating promise call, or null. Pure. */ +export function detectFloatingPromise(line: string): string | null { + if (isCommentLine(line) || HANDLED_PREFIX.test(line)) { + return null; + } + + const code = codeOnly(line).replace(/=>/g, " "); + if (PROMISE_CHAIN_RE.test(code)) return null; + if (/(?!])=(?!=)/.test(code)) return null; + + const callee = extractLeadingCallCallee(line); + if (!callee || !isPromiseShapedCallee(callee)) return null; + + return truncateCall(callee); +} + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +/** Scan one file patch's added lines for floating promises, line-cited via hunk headers. Pure. */ +export function scanPatchForFloatingPromise( + path: string, + patch: string, + limits: ScanLimits = {}, +): FloatingPromiseFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || !isJsTsPath(path)) return []; + const findings: FloatingPromiseFinding[] = []; + let newLine = 0; + let inHunk = false; + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + inHunk = true; + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + const body = line.slice(1); + if (body.length <= MAX_LINE_CHARS) { + const call = detectFloatingPromise(body); + if (call) { + findings.push({ file: path, line: newLine, call }); + if (findings.length >= maxFindings) return findings; + } + } + newLine++; + } else if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed TS/JS file's added lines for floating promises. */ +export async function scanFloatingPromise( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: FloatingPromiseFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForFloatingPromise(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/hardcoded-url.ts b/review-enrichment/src/analyzers/hardcoded-url.ts index c5790d50ba..56ce031997 100644 --- a/review-enrichment/src/analyzers/hardcoded-url.ts +++ b/review-enrichment/src/analyzers/hardcoded-url.ts @@ -1,143 +1,144 @@ -// Hardcoded-URL / raw-endpoint analyzer (#2027). Flags absolute HTTP(S) URLs and IP:port endpoints newly -// added in non-test, non-config source — often environment leakage or a value that should come from config. -// Distinct from the secret scanner (no credential); this is a portability/config-hygiene signal. Pure compute -// over added lines, no network. Hostnames are redacted/truncated in findings — never full paths or queries. -import type { EnrichRequest, HardcodedUrlFinding } from "../types.js"; -import { isMagicNumberSourcePath } from "./magic-number.js"; - -const MAX_FINDINGS = 25; -const MAX_LINE_CHARS = 2000; -const MAX_HOST_CHARS = 40; - -const CONFIG_PATH_RE = - /(?:^|\/)(?:docker-compose[^/]*\.ya?ml|compose[^/]*\.ya?ml|values(?:\.[^/]+)?\.ya?ml|\.env(?:\.[^/]+)?|.*\.(?:tf|tfvars|hcl|ya?ml|json|toml|ini|conf|env)|Dockerfile(?:\.[^/]+)?|nginx[^/]*\.conf)$/i; - -const HTTP_URL_RE = /https?:\/\/[^\s'"\`<>]+/gi; -const IP_ENDPOINT_RE = /\b(?:\d{1,3}\.){3}\d{1,3}:\d{1,5}\b/g; - -const ALLOWLISTED_HOSTS = new Set(["localhost", "127.0.0.1", "example.com"]); - -function isConfigPath(path: string): boolean { - return CONFIG_PATH_RE.test(path); -} - -function isScannablePath(path: string): boolean { - return isMagicNumberSourcePath(path) && !isConfigPath(path); -} - -function redactHost(host: string): string { - const lower = host.toLowerCase(); - if (lower.length <= MAX_HOST_CHARS) return lower; - return `${lower.slice(0, MAX_HOST_CHARS - 3)}...`; -} - -function isAllowlistedHost(host: string): boolean { - const lower = host.toLowerCase(); - if (ALLOWLISTED_HOSTS.has(lower)) return true; - if (lower.endsWith(".example.com")) return true; - return false; -} - -function hostFromHttpUrl(url: string): string { - const match = /^https?:\/\/([^/?#:]+)(?::\d+)?/i.exec(url); - return match?.[1] ?? url; -} - -function isCommentLine(line: string): boolean { - const trimmed = line.trimStart(); - return /^(?:\/\/|#|\/\*|\*|