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
30 changes: 27 additions & 3 deletions review-enrichment/src/analyzers/native-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { boundedFetchJson } from "../external-fetch.js";
const MAX_QUERIES = 25;
const MAX_NPM_VERSION_JSON_BYTES = 256 * 1024;
const MAX_PYPI_VERSION_JSON_BYTES = 2 * 1024 * 1024;
const MAX_CONCURRENT_REGISTRY_QUERIES = 4;
const INSTALL_HOOKS = ["preinstall", "install", "postinstall"];
// Tokens in an install-lifecycle script that indicate a native toolchain runs on install.
const NATIVE_TOOL_RE =
Expand Down Expand Up @@ -175,6 +176,27 @@ async function fetchJson(
return response.ok ? response.data : null;
}

async function mapWithConcurrency<T, U>(
items: T[],
concurrency: number,
fn: (item: T) => Promise<U>,
): Promise<U[]> {
const results = new Array<U>(items.length);
let nextIndex = 0;

async function worker(): Promise<void> {
while (nextIndex < items.length) {
const index = nextIndex;
nextIndex += 1;
results[index] = await fn(items[index]!);
}
}

const workerCount = Math.min(concurrency, items.length);
await Promise.all(Array.from({ length: workerCount }, () => worker()));
return results;
}

/** Analyzer entrypoint: added/changed deps → registry metadata → only the versions with a native-build install cost. */
export async function scanNativeBuild(
req: EnrichRequest,
Expand All @@ -186,8 +208,10 @@ export async function scanNativeBuild(
const changes = extractDependencyChanges(req.files ?? [])
.filter(isQueryable)
.slice(0, options.limits?.maxQueries ?? MAX_QUERIES);
const results = await Promise.all(
changes.map(async (change): Promise<NativeBuildFinding | null> => {
const results = await mapWithConcurrency(
changes,
MAX_CONCURRENT_REGISTRY_QUERIES,
async (change): Promise<NativeBuildFinding | null> => {
if (options.signal?.aborted) return null;

if (change.ecosystem === "npm") {
Expand Down Expand Up @@ -233,7 +257,7 @@ export async function scanNativeBuild(
}
}
return null;
}),
},
);
const findings = results.filter((f): f is NativeBuildFinding => f !== null);
return findings;
Expand Down
35 changes: 35 additions & 0 deletions review-enrichment/test/native-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ const pypiAdd = (name, version = "1.0.0") => ({
},
],
});
const pypiAdds = (count) => ({
repoFullName: "o/r",
prNumber: 1,
files: [
{
path: "requirements.txt",
patch: `@@ -1,0 +1,${count} @@\n${Array.from(
{ length: count },
(_, i) => `+native${i}==1.0.0`,
).join("\n")}`,
},
],
});
const jsonResponse = (body, init) => new Response(JSON.stringify(body), init);
const npmFetch = (meta) => async () =>
jsonResponse({
Expand Down Expand Up @@ -227,6 +240,28 @@ test("scanNativeBuild: the query cap counts only queryable changes (skips don't
assert.equal(findings[0].package, "bcrypt");
});

test("scanNativeBuild bounds concurrent registry fetches below the total query cap", async () => {
let active = 0;
let maxActive = 0;
let started = 0;
const findings = await scanNativeBuild(
pypiAdds(10),
async () => {
started += 1;
active += 1;
maxActive = Math.max(maxActive, active);
await Promise.resolve();
active -= 1;
return jsonResponse({ urls: [{ packagetype: "sdist" }] });
},
{ limits: { maxQueries: 10 } },
);

assert.equal(started, 10);
assert.equal(maxActive, 4);
assert.equal(findings.length, 10);
});

test("scanNativeBuild: a PyPI PEP 440 (non-semver) sdist-only version is flagged", async () => {
const findings = await scanNativeBuild(
pypiAdd("ujson", "24.1"),
Expand Down