Skip to content
Merged
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
139 changes: 139 additions & 0 deletions review-enrichment/src/analyzers/native-build.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// Native-build / install-cost analyzer (#1512). For each dependency a PR newly adds or upgrades, flags the ones
// whose install does real work the manifest diff never shows: an npm package that compiles a native addon
// (node-gyp / gypfile) on install, or a PyPI release that ships no prebuilt wheel (sdist-only) so pip compiles from
// source. Both are a hidden CI cold-start cost and a frequent cross-platform breakage source. Factual signals from
// the registry metadata only — the no-checkout reviewer can fetch neither. Reports package@version + the property.
import type { EnrichRequest, NativeBuildFinding } from "../types.js";
import { extractDependencyChanges } from "./dependency-scan.js";

const MAX_QUERIES = 25;
const INSTALL_HOOKS = ["preinstall", "install", "postinstall"];
// Tokens in an install-lifecycle script that indicate a native toolchain runs on install.
const NATIVE_TOOL_RE = /\b(node-gyp|node-pre-gyp|prebuild|prebuild-install|cmake-js|node-addon-api|nan)\b/;
// Tokens that mean prebuilt binaries are DOWNLOADED (compile only as a fallback for an unmatched platform/ABI).
const PREBUILT_TOOL_RE = /\b(node-pre-gyp|prebuild-install)\b/;

const NPM_PACKAGE_RE = /^(?:@[a-z0-9][a-z0-9._-]*\/[a-z0-9][a-z0-9._-]*|[a-z0-9][a-z0-9._-]*)$/;
const PYPI_PACKAGE_RE = /^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$/;
const SEMVER_RE = /^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
// PyPI versions are PEP 440, not semver: `1.0`, `24.1`, `1.0rc1`, `1.0.post1`, `1!2.0`. Validate only that the
// string is non-empty and URL-path-safe (it goes into the version JSON URL) rather than imposing semver.
const PYPI_VERSION_RE = /^[A-Za-z0-9][A-Za-z0-9._+!-]{0,63}$/;

/** Is this dependency change one we can query a registry for (supported ecosystem + URL-safe name/version)? */
function isQueryable(change: { ecosystem: string; package: string; to: string }): boolean {
if (change.ecosystem === "npm") return NPM_PACKAGE_RE.test(change.package) && SEMVER_RE.test(change.to);
if (change.ecosystem === "PyPI") return PYPI_PACKAGE_RE.test(change.package) && PYPI_VERSION_RE.test(change.to);
return false;
}

interface ScanLimits {
maxQueries?: number;
}

interface ScanOptions {
signal?: AbortSignal;
limits?: ScanLimits;
}

/** npm packument version metadata, the subset that signals a native build. */
export interface NpmVersionMeta {
gypfile?: boolean;
binary?: unknown;
scripts?: Record<string, string>;
}

/** Pure: does this npm version compile a native addon on install? Returns a reason (+ whether a prebuilt fallback
* exists), or null. Signals: `gypfile: true`, or an install/preinstall/postinstall script that runs a native tool. */
export function npmNativeBuild(meta: NpmVersionMeta): { reason: string; prebuiltFallback: boolean } | null {
const installScript = INSTALL_HOOKS.map((hook) => meta.scripts?.[hook] ?? "").join(" ");
const isNative = meta.gypfile === true || NATIVE_TOOL_RE.test(installScript);
if (!isNative) return null;
const prebuiltFallback = Boolean(meta.binary) || PREBUILT_TOOL_RE.test(installScript);
const reason = prebuiltFallback
? "ships a native addon with prebuilt binaries — compiles from source only when no prebuilt matches the platform/Node ABI"
: "compiles a native addon (node-gyp) on install — cold-CI build cost and a cross-platform breakage source";
return { reason, prebuiltFallback };
}

/** A PyPI release file entry (from the `urls` array of the version JSON). */
export interface PypiUrl {
packagetype?: string;
}

/** Pure: is this PyPI version sdist-only (a source dist is published but no prebuilt wheel)? Requires an actual
* `sdist` so an empty or wheel-less-but-also-sdist-less file set is not mistaken for "compiles from source". */
export function pypiSdistOnly(urls: PypiUrl[]): boolean {
const hasSdist = urls.some((url) => url.packagetype === "sdist");
const hasWheel = urls.some((url) => url.packagetype === "bdist_wheel");
return hasSdist && !hasWheel;
}

async function fetchJson(
fetchImpl: typeof fetch,
url: string,
signal?: AbortSignal,
): Promise<unknown | null> {
if (signal?.aborted) return null;
try {
const response = await fetchImpl(url, { signal });
if (!response.ok) return null;
return await response.json();
} catch {
return null;
}
}

/** Analyzer entrypoint: added/changed deps → registry metadata → only the versions with a native-build install cost. */
export async function scanNativeBuild(
req: EnrichRequest,
fetchImpl: typeof fetch = fetch,
options: ScanOptions = {},
): Promise<NativeBuildFinding[]> {
// Filter to queryable (supported, URL-safe) changes BEFORE applying the cap, so unsupported/invalid entries can't
// consume the budget and starve a later native dependency.
const changes = extractDependencyChanges(req.files ?? [])
.filter(isQueryable)
.slice(0, options.limits?.maxQueries ?? MAX_QUERIES);
const findings: NativeBuildFinding[] = [];
for (const change of changes) {
if (options.signal?.aborted) break;

if (change.ecosystem === "npm") {
const data = (await fetchJson(
fetchImpl,
`https://registry.npmjs.org/${encodeURIComponent(change.package)}`,
options.signal,
)) as { versions?: Record<string, NpmVersionMeta> } | null;
const meta = data?.versions?.[change.to];
const native = meta && npmNativeBuild(meta);
if (native) {
findings.push({
ecosystem: change.ecosystem,
package: change.package,
version: change.to,
kind: "native-addon",
prebuiltFallback: native.prebuiltFallback,
reason: native.reason,
});
}
} else {
// PyPI — the only other ecosystem isQueryable admits.
const data = (await fetchJson(
fetchImpl,
`https://pypi.org/pypi/${encodeURIComponent(change.package)}/${encodeURIComponent(change.to)}/json`,
options.signal,
)) as { urls?: PypiUrl[] } | null;
if (data && pypiSdistOnly(data.urls ?? [])) {
findings.push({
ecosystem: change.ecosystem,
package: change.package,
version: change.to,
kind: "sdist-only",
reason: "no prebuilt wheel for this version — pip compiles from source (sdist) on install",
});
}
}
}
return findings;
}
2 changes: 2 additions & 0 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { scanCodeowners } from "./analyzers/codeowners.js";
import { scanSecretLog } from "./analyzers/secret-log.js";
import { scanAssetWeight } from "./analyzers/asset-weight.js";
import { scanTyposquat } from "./analyzers/typosquat.js";
import { scanNativeBuild } from "./analyzers/native-build.js";
import { renderBrief } from "./render.js";
import { captureAnalyzerDegradation } from "./sentry.js";

Expand All @@ -41,6 +42,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
secretLog: (req, signal) => scanSecretLog(req, signal),
assetWeight: (req, signal) => scanAssetWeight(req, fetch, { signal }),
typosquat: (req, signal) => scanTyposquat(req, fetch, { signal }),
nativeBuild: (req, signal) => scanNativeBuild(req, fetch, { signal }),
};

function runWithTimeout<T>(
Expand Down
12 changes: 12 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,18 @@ export function renderBrief(
}
}

const nativeBuilds = findings.nativeBuild ?? [];
if (nativeBuilds.length) {
lines.push(
"### Native-build / install-cost dependencies (CI cold-start + cross-platform build cost)",
);
for (const item of nativeBuilds) {
lines.push(
`- ${safeCodeSpan(`${item.package}@${item.version}`)} (${item.ecosystem}): ${item.reason}`,
);
}
}

if (!lines.length) return { promptSection: "", systemSuffix: "" };

const header =
Expand Down
16 changes: 16 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,21 @@ export interface TyposquatFinding {
reason: string;
}

/** A newly-added dependency whose install compiles native code (npm node-gyp addon) or has no prebuilt wheel
* (PyPI sdist-only) — a hidden CI cold-start/install cost and a frequent cross-platform breakage source. Reports
* package@version + the factual build property only. (#1512) */
export interface NativeBuildFinding {
ecosystem: string;
package: string;
version: string;
kind: "native-addon" | "sdist-only";
/** npm only: a prebuilt-binary path exists (node-pre-gyp/prebuild or a `binary` field), so a compile is the
* fallback when no prebuilt matches the platform/ABI rather than guaranteed. */
prebuiltFallback?: boolean;
/** Short, public-safe explanation of the build cost. */
reason: string;
}

/** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */
export interface BriefFindings {
dependency?: DependencyFinding[];
Expand All @@ -177,6 +192,7 @@ export interface BriefFindings {
secretLog?: SecretLogFinding[];
assetWeight?: AssetWeightFinding[];
typosquat?: TyposquatFinding[];
nativeBuild?: NativeBuildFinding[];
}

export type AnalyzerStatus = "ok" | "degraded" | "skipped";
Expand Down
150 changes: 150 additions & 0 deletions review-enrichment/test/native-build.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// Units for the native-build / install-cost analyzer (#1512). Own file (not enrichment.test.ts) so concurrent
// analyzer PRs don't collide. Runs against the compiled dist/.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
npmNativeBuild,
pypiSdistOnly,
scanNativeBuild,
} from "../dist/analyzers/native-build.js";
import { renderBrief } from "../dist/render.js";

const npmAdd = (name, version = "1.0.0") => ({
repoFullName: "o/r",
prNumber: 1,
files: [{ path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "${name}": "^${version}"` }],
});
const pypiAdd = (name, version = "1.0.0") => ({
repoFullName: "o/r",
prNumber: 1,
files: [{ path: "requirements.txt", patch: `@@ -1,0 +1,1 @@\n+${name}==${version}` }],
});
const npmFetch = (meta) => async () => ({ ok: true, json: async () => ({ versions: { "1.0.0": meta } }) });
const pypiFetch = (urls) => async () => ({ ok: true, json: async () => ({ urls }) });
const status = (code) => async () => ({ ok: code >= 200 && code < 300, status: code, json: async () => ({}) });
const throwingFetch = async () => {
throw new Error("network down");
};

test("npmNativeBuild: gypfile compiles without a prebuilt fallback", () => {
const hit = npmNativeBuild({ gypfile: true });
assert.equal(hit?.prebuiltFallback, false);
assert.match(hit.reason, /compiles a native addon/);
});

test("npmNativeBuild: install script running node-gyp flags a compile", () => {
assert.ok(npmNativeBuild({ scripts: { install: "node-gyp rebuild" } }));
assert.ok(npmNativeBuild({ scripts: { postinstall: "cmake-js compile" } }));
});

test("npmNativeBuild: a prebuilt-binary path is reported as a fallback compile", () => {
const viaBinary = npmNativeBuild({ gypfile: true, binary: { module_name: "x" } });
assert.equal(viaBinary?.prebuiltFallback, true);
assert.match(viaBinary.reason, /prebuilt/);
const viaScript = npmNativeBuild({ scripts: { install: "node-pre-gyp install --fallback-to-build" } });
assert.equal(viaScript?.prebuiltFallback, true);
});

test("npmNativeBuild: a pure-JS package is not flagged", () => {
assert.equal(npmNativeBuild({ scripts: { build: "tsc", postinstall: "echo hi" } }), null);
assert.equal(npmNativeBuild({}), null);
});

test("pypiSdistOnly: true only when an sdist exists and no wheel does", () => {
assert.equal(pypiSdistOnly([{ packagetype: "sdist" }]), true);
assert.equal(pypiSdistOnly([{ packagetype: "sdist" }, { packagetype: "bdist_wheel" }]), false);
assert.equal(pypiSdistOnly([{ packagetype: "bdist_wheel" }]), false); // wheel present
assert.equal(pypiSdistOnly([{ packagetype: "bdist_egg" }]), false); // no sdist → not "sdist-only"
assert.equal(pypiSdistOnly([]), false); // undeterminable → no finding
});

test("scanNativeBuild: npm gypfile dependency is flagged native-addon", async () => {
const findings = await scanNativeBuild(npmAdd("bcrypt"), npmFetch({ gypfile: true }));
assert.equal(findings.length, 1);
assert.equal(findings[0].kind, "native-addon");
assert.equal(findings[0].package, "bcrypt");
assert.equal(findings[0].prebuiltFallback, false);
});

test("scanNativeBuild: a pure-JS npm dependency is not flagged", async () => {
assert.deepEqual(await scanNativeBuild(npmAdd("lodash"), npmFetch({ scripts: { build: "tsc" } })), []);
});

test("scanNativeBuild: PyPI sdist-only release is flagged", async () => {
const findings = await scanNativeBuild(pypiAdd("ujson"), pypiFetch([{ packagetype: "sdist" }]));
assert.equal(findings.length, 1);
assert.equal(findings[0].kind, "sdist-only");
assert.match(findings[0].reason, /compiles from source/);
});

test("scanNativeBuild: a PyPI release with a wheel is not flagged", async () => {
assert.deepEqual(
await scanNativeBuild(pypiAdd("requests"), pypiFetch([{ packagetype: "bdist_wheel" }])),
[],
);
});

test("scanNativeBuild: unsupported ecosystems and invalid names/versions are never queried", async () => {
const req = {
repoFullName: "o/r",
prNumber: 1,
files: [
{ path: "go.mod", patch: `@@ -1,0 +1,1 @@\n+require example.com/x v1.0.0` }, // Go — unsupported
{ path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "BadCaps": "^1.0.0"` }, // invalid npm name
],
};
let called = false;
const out = await scanNativeBuild(req, async () => {
called = true;
return status(200)();
});
assert.deepEqual(out, []);
assert.equal(called, false); // nothing queryable → no registry call
});

test("scanNativeBuild: the query cap counts only queryable changes (skips don't starve a later native dep)", async () => {
// 25 unsupported Go changes precede one native npm dep; with filter-before-cap the npm dep is still queried.
const goLines = Array.from({ length: 25 }, (_, i) => `+require example.com/m${i} v1.0.0`).join("\n");
const req = {
repoFullName: "o/r",
prNumber: 1,
files: [
{ path: "go.mod", patch: `@@ -1,0 +1,25 @@\n${goLines}` },
{ path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "bcrypt": "^1.0.0"` },
],
};
const findings = await scanNativeBuild(req, npmFetch({ gypfile: true }), { limits: { maxQueries: 25 } });
assert.equal(findings.length, 1);
assert.equal(findings[0].package, "bcrypt");
});

test("scanNativeBuild: a PyPI PEP 440 (non-semver) sdist-only version is flagged", async () => {
const findings = await scanNativeBuild(pypiAdd("ujson", "24.1"), pypiFetch([{ packagetype: "sdist" }]));
assert.equal(findings.length, 1);
assert.equal(findings[0].kind, "sdist-only");
assert.equal(findings[0].version, "24.1");
});

test("scanNativeBuild fails safe on a non-ok or throwing fetch", async () => {
assert.deepEqual(await scanNativeBuild(npmAdd("bcrypt"), status(404)), []);
assert.deepEqual(await scanNativeBuild(npmAdd("bcrypt"), throwingFetch), []);
});

test("scanNativeBuild stops on an already-aborted signal", async () => {
const findings = await scanNativeBuild(npmAdd("bcrypt"), npmFetch({ gypfile: true }), {
signal: AbortSignal.abort(),
});
assert.deepEqual(findings, []);
});

test("renderBrief emits a public-safe native-build block", () => {
const { promptSection } = renderBrief({
nativeBuild: [
{ ecosystem: "npm", package: "bcrypt", version: "5.1.0", kind: "native-addon", prebuiltFallback: false, reason: "compiles a native addon (node-gyp) on install — cold-CI build cost and a cross-platform breakage source" },
{ ecosystem: "PyPI", package: "ujson", version: "5.0.0", kind: "sdist-only", reason: "no prebuilt wheel for this version — pip compiles from source (sdist) on install" },
],
});
assert.match(promptSection, /Native-build \/ install-cost dependencies/);
assert.match(promptSection, /bcrypt@5\.1\.0/);
assert.match(promptSection, /ujson@5\.0\.0/);
});