mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 22:24:16 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Its CI must catch regressions without running unrelated work on every pull request. > - The existing task planner selects package lanes, but each selected lane still runs all its test files. > - Vitest can find related imports, but that graph misses tests that read files or launch processes. > - This pull request combines the real Vitest graph with conservative dependency checks before CI allocates test runners. > - The result runs fewer test files and can allocate fewer jobs while retaining tests with unknown dependencies. ## Linked Issues or Issue Description Refs #18214. The repository and PR search found no duplicate file-selection implementation. **What existing behavior does this improve?** The general and serialized Vitest lanes in pull request CI. **Current behavior** An affected package schedules its complete test lane and its fixed shard count. **Proposed behavior** For source-only diffs, select related tests plus conservative filesystem, process, network and opaque consumers. Repartition those files within their existing lane owners. Keep the complete original plan when selection cannot be trusted. **Reason and benefit** Avoid unrelated test execution and allocate fewer runners when the selected workload fits fewer existing job budgets. Report file counts and job counts separately. **Breaking changes** No product or API changes. The set of PR test files and jobs changes for eligible diffs. Full CI and the kill switch retain the original coverage. ## What Changed - Add a separate selection job on the gate-approved test fleet. The metadata gate does not load PR code. - Inspect real Vitest/Vite dependency edges. Retain opaque consumers and unchanged downstream projects. Respect only complete local mock factories. Reject unknown setup dependencies and source mutations during dependency setup. - Declare `oxc-parser` directly for AST inspection. This exact version is already a transitive dependency in the checked-in lockfile; the direct declaration avoids relying on pnpm's internal dependency layout. - Produce a merge-SHA-bound manifest with exact lane ownership and duration-aware partitions. Preserve serialized execution, chat shards and native Runner ownership. - Execute exact Vitest test specifications. Positional substring filters cannot accidentally run a similarly named file from another shard. - Use the original matrix if selection fails or times out. Fail selected jobs if their published artifact is missing or invalid. - Save the uploaded artifact ID in the executable plan. Failed-job retries reuse that artifact even when GitHub increments the attempt number. - Hash selected assignments into Turbo cache inputs. A subset cannot satisfy a full run or another subset. - Add graph, workflow, real-runner and real-Turbo qualification. The regression fixture proves a filesystem contract fails even when raw Vitest selection misses it. - Document selection limits, audit artifacts and `CI_VITEST_SELECTION=off`. ## Verification - All 138 affected routing, sharding, workflow, manifest, real Vitest and real Turbo checks passed under Node 24 on the final implementation. Additional release workflow checks passed. Cases cover opaque setup, remocking, generated inputs, Node's builtin loader, artifact retries, and exact execution of similarly named files. - Repository-wide `pnpm -r typecheck` and `pnpm build` passed before the clean rebase onto current master. The full remote typecheck and build are required on the current head. - `actionlint` passed for both changed workflows. `git diff --check` passed. - The Linux qualification run passed and selected 699 of 911 workspace test files for an Inbox change. UI graph construction took 16.1 seconds. This still uses two jobs. Synthetic partition coverage proves a 15-of-120 selection uses two of twelve jobs without dropping or duplicating files. - Both selected workspace shards passed locally. After tightening execution to exact specifications, the real colliding UI test ran as exactly one file with two passing assertions; the real server auth middleware test ran as exactly one file with 28 passing assertions. - The server probe selected 1,158 of 1,260 general files and 161 of 163 serialized files for an auth middleware change. Those conservative selections retain 12 general and nine serialized jobs. The server's temporary-home setup exception is pinned to reviewed content hashes; changed setup fails closed. - Full `pnpm test:run` has not been run locally. The full PR test matrix is required because this change modifies global CI inputs. Final-head CI is pending at runner allocation, and both qualification jobs are queued. Other recent repository workflows are also queued. Superseded runs of this PR were cancelled; this PR remains in draft until the required checks execute. - Greptile reviewed final commit `31b484fc5b412da245cffb52a8ac8977228c5651` at 5/5 with no unresolved findings. The earlier qualification run passed; it does not replace checks on the final commit. ## Risks - Dependency analysis adds setup and graph time. Fewer files do not guarantee a shorter critical path. CI summaries expose the actual file and job counts. - Static dependency graphs cannot describe all runtime inputs. Unknown inputs retain tests or use the original plan. The pure-package allowlist requires conservative review. - The trusted workflow runs from master. This PR exercises the new selector in the qualification workflow; ordinary PR selection starts after merge. Set `CI_VITEST_SELECTION=off`, use `full-ci`, or select full/shadow mode to disable narrowing. - The lockfile remains managed by the existing repository actions and is not changed here. ## Model Used OpenAI Codex, based on GPT-6, with repository tools and code execution. The exact serving model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
265 lines
15 KiB
JavaScript
265 lines
15 KiB
JavaScript
import { readFileSync, realpathSync } from "node:fs";
|
|
import { execFileSync } from "node:child_process";
|
|
import { createHash } from "node:crypto";
|
|
import path from "node:path";
|
|
import { isBuiltin } from "node:module";
|
|
import { parseSync } from "oxc-parser";
|
|
import { createVitest } from "vitest/node";
|
|
|
|
// Everything else is opaque by default. Do not add IO clients, module loaders,
|
|
// filesystem helpers or process launchers here to improve selection counts.
|
|
const purePackages = new Set([
|
|
"vitest", "@vitest/expect", "@testing-library/react", "@testing-library/dom",
|
|
"@testing-library/user-event", "@testing-library/jest-dom", "react", "react-dom",
|
|
"react-router", "react-router-dom", "@tanstack/react-query", "@tanstack/query-core",
|
|
"zod", "lodash", "lodash-es", "date-fns", "clsx", "tailwind-merge", "class-variance-authority",
|
|
"lucide-react", "immer", "nanoid", "uuid", "semver",
|
|
"radix-ui", "motion", "@dnd-kit/core", "@dnd-kit/sortable", "@dnd-kit/utilities",
|
|
"i18next", "react-i18next",
|
|
]);
|
|
const pureBuiltins = new Set(["assert", "assert/strict", "path", "path/posix", "path/win32",
|
|
"url", "util", "util/types", "buffer", "events", "string_decoder", "crypto"]);
|
|
// Reviewed setup performs only temporary-home IO and Supertest socket setup,
|
|
// not reads of arbitrary repository inputs. Pin contents, not just paths: any
|
|
// edit disables this exception until its effects are reviewed again. Continue
|
|
// following local imports even for these files; helper changes affect all tests.
|
|
const reviewedSetup = new Map([
|
|
["server/src/__tests__/setup-supertest.ts", {
|
|
hash: "99d7193927468e98b41255ea9a5e37b1a681f02b259c304f3af1a52304eed2bb",
|
|
external: ["supertest/lib/test.js"],
|
|
}],
|
|
["server/src/__tests__/helpers/isolated-test-home.ts", {
|
|
hash: "bd5645dc590087f26b473beab06d5c3ddd7956a79949f0b3e9713f11897a0c1b",
|
|
external: [],
|
|
}],
|
|
]);
|
|
const packageName = (specifier) => specifier.startsWith("@")
|
|
? specifier.split("/").slice(0, 2).join("/") : specifier.split("/")[0];
|
|
const inside = (root, file) => file.startsWith(`${root}${path.sep}`) && !file.includes(`${path.sep}node_modules${path.sep}`);
|
|
|
|
// Parse source, including imports erased by mocks/transforms. Unknown loaders
|
|
// retain their consumers. This is intentionally conservative, not a regex-based
|
|
// promise that arbitrary JavaScript has no effects.
|
|
export function sourceDependencies(file, source) {
|
|
const ast = parseSync(file, source);
|
|
const imports = new Set();
|
|
const reasons = new Set();
|
|
const mockBindings = new Set(ast.program.body.filter((node) => node.type === "ImportDeclaration" && node.source.value === "vitest")
|
|
.flatMap((node) => node.specifiers.filter((s) => s.type === "ImportSpecifier" && ["vi", "vitest"].includes(s.imported.name)).map((s) => s.local.name)));
|
|
const hoistedBindings = new Set(ast.program.body.filter((node) => node.type === "ImportDeclaration" && node.source.value === "vitest")
|
|
.flatMap((node) => node.specifiers.filter((s) => s.type === "ImportSpecifier" && ["vi", "vitest"].includes(s.imported.name)
|
|
&& s.local.name === s.imported.name).map((s) => s.local.name)));
|
|
const mocks = [];
|
|
if (ast.errors.length) reasons.add("unparsed source");
|
|
const literal = (node) => node?.type === "Literal" && typeof node.value === "string";
|
|
const importSource = (node) => {
|
|
if (literal(node)) imports.add(node.value);
|
|
else reasons.add("dynamic module name");
|
|
};
|
|
for (const statement of ast.program.body) {
|
|
const call = statement.type === "ExpressionStatement" ? statement.expression : null;
|
|
const callee = call?.type === "CallExpression" ? call.callee : null;
|
|
const factory = call?.arguments?.[1];
|
|
if (callee?.type === "MemberExpression" && !callee.computed && callee.property.name === "mock"
|
|
&& hoistedBindings.has(callee.object.name) && literal(call.arguments[0])
|
|
&& ["ArrowFunctionExpression", "FunctionExpression"].includes(factory?.type) && factory.params.length === 0) {
|
|
mocks.push(call.arguments[0].value);
|
|
}
|
|
}
|
|
function visit(node, parent = null) {
|
|
if (!node || typeof node !== "object") return;
|
|
if ((node.type === "ImportDeclaration" && node.importKind !== "type")
|
|
|| (["ExportNamedDeclaration", "ExportAllDeclaration"].includes(node.type) && node.exportKind !== "type")) {
|
|
if (node.source) importSource(node.source);
|
|
}
|
|
if (node.type === "ImportExpression") importSource(node.source);
|
|
if (node.type === "TSImportEqualsDeclaration") reasons.add("CommonJS import");
|
|
if (node.type === "Identifier" && /^(require|fetch|eval|Function|WebSocket|XMLHttpRequest|Worker)$/.test(node.name)) {
|
|
reasons.add("external input or dynamic code");
|
|
}
|
|
if ((node.type === "Identifier" && node.name === "getBuiltinModule")
|
|
|| (literal(node) && node.value === "getBuiltinModule")) reasons.add("dynamic builtin loader");
|
|
if ((node.type === "Identifier" && /^(importActual|unmock|doUnmock|doMock|arguments)$/.test(node.name))
|
|
|| (literal(node) && /^(importActual|unmock|doUnmock|doMock)$/.test(node.value))) reasons.add("mock bypass");
|
|
if (node.type === "Identifier" && mockBindings.has(node.name) && parent
|
|
&& parent.type !== "ImportSpecifier"
|
|
&& !(parent.type === "MemberExpression" && parent.object === node && !parent.computed)) {
|
|
reasons.add("escaped or dynamic mocking API");
|
|
}
|
|
if (node.type === "CallExpression") {
|
|
const name = source.slice(node.callee.start, node.callee.end);
|
|
if (/^(?:vi|vitest)\.(?:mock|doMock|importActual|importMock)$/.test(name)) {
|
|
importSource(node.arguments[0]);
|
|
}
|
|
if (/\brequire\b/.test(name)) {
|
|
reasons.add("CommonJS loader");
|
|
importSource(node.arguments[0]);
|
|
}
|
|
const viteGlob = name === "import.meta.glob" && (literal(node.arguments[0])
|
|
|| (node.arguments[0]?.type === "ArrayExpression" && node.arguments[0].elements.every(literal)));
|
|
// Vite expands literal globs into dependency edges. New files appear in
|
|
// that expansion; deleted/renamed and non-source inputs force full lanes.
|
|
if (!viteGlob && /\b(?:fetch|eval|Function|glob|globSync)\b/.test(name)) reasons.add("external input or dynamic code");
|
|
}
|
|
for (const value of Object.values(node)) {
|
|
if (Array.isArray(value)) value.forEach((child) => visit(child, node));
|
|
else if (value && typeof value === "object" && value.type) visit(value, node);
|
|
}
|
|
}
|
|
visit(ast.program);
|
|
return { imports: [...imports], reasons: [...reasons], mocks };
|
|
}
|
|
|
|
export async function selectProjectTests({ root = process.cwd(), project, changed, onlyOwnChanges = false }) {
|
|
root = realpathSync(root);
|
|
const started = performance.now();
|
|
const absoluteChanges = changed.map((file) => path.resolve(root, file));
|
|
const ctx = await createVitest({ root, watch: false, project: [project],
|
|
exclude: ["**/dist/**"], reporters: [], related: absoluteChanges });
|
|
try {
|
|
if (ctx.projects.length !== 1 || ctx.projects[0].name !== project) throw new Error(`Unknown project: ${project}`);
|
|
const owner = ctx.projects[0];
|
|
const environment = owner.vite.environments.ssr;
|
|
const inventory = await ctx.globTestSpecifications();
|
|
if (!inventory.length) throw new Error(`Empty inventory: ${project}`);
|
|
const relative = (spec) => path.relative(root, spec.moduleId).split(path.sep).join("/");
|
|
if (onlyOwnChanges && !absoluteChanges.some((file) => file.startsWith(`${owner.config.root}${path.sep}`))) {
|
|
// Keep coarse dependency coverage, but avoid spending a graph traversal
|
|
// on an unchanged consumer project merely to retain most of its IO tests.
|
|
const files = inventory.map(relative).sort();
|
|
return { project, inventory: files, selected: files, related: [], alwaysRun: files, coveredChanges: [],
|
|
opaqueModules: {}, retainedReason: "Unchanged project retains complete lane coverage", seconds: (performance.now() - started) / 1000 };
|
|
}
|
|
const tracked = new Set(execFileSync("git", ["ls-files", "--cached", "-z"], {
|
|
cwd: root, encoding: "utf8", maxBuffer: 16 * 1024 * 1024,
|
|
}).split("\0").filter(Boolean));
|
|
const audit = new Map();
|
|
const parsed = new Map();
|
|
const parse = (file) => {
|
|
if (!parsed.has(file)) parsed.set(file, sourceDependencies(file, readFileSync(file, "utf8")));
|
|
return parsed.get(file);
|
|
};
|
|
const resolve = async (specifier, importer) => {
|
|
try {
|
|
const resolved = await environment.pluginContainer.resolveId(specifier, importer);
|
|
if (!resolved || resolved.external || !path.isAbsolute(resolved.id) || resolved.id.includes("?")) return null;
|
|
return realpathSync(resolved.id);
|
|
} catch { return null; } // A conditional/require-only export is opaque.
|
|
};
|
|
// Setup dependencies are not test-import edges. A change anywhere in their
|
|
// local import closure invalidates the entire project, including helpers.
|
|
const setup = new Set();
|
|
async function setupClosure(file) {
|
|
if (setup.has(file)) return;
|
|
setup.add(file);
|
|
const source = parse(file);
|
|
const review = reviewedSetup.get(path.relative(root, file).split(path.sep).join("/"));
|
|
const reviewed = review?.hash === createHash("sha256").update(readFileSync(file)).digest("hex");
|
|
if (!reviewed && source.reasons.length) throw new Error(`Opaque setup dependency: ${file}`);
|
|
for (const specifier of source.imports) {
|
|
if (isBuiltin(specifier)) {
|
|
if (!reviewed && !pureBuiltins.has(specifier.replace(/^node:/, ""))) throw new Error(`Opaque setup builtin: ${specifier}`);
|
|
continue;
|
|
}
|
|
if (reviewed && review.external.includes(specifier)) continue;
|
|
const dependency = await resolve(specifier, file);
|
|
if (dependency && inside(root, dependency)) await setupClosure(dependency);
|
|
else if (!dependency) throw new Error(`Unresolved setup dependency: ${specifier}`);
|
|
else if (!purePackages.has(packageName(specifier))) throw new Error(`Opaque setup package: ${specifier}`);
|
|
}
|
|
}
|
|
for (const file of [...(owner.config.setupFiles ?? []), ...(owner.config.globalSetup ?? []),
|
|
...(owner.config.snapshotSerializers ?? [])]) {
|
|
const resolved = path.isAbsolute(file) ? file : await resolve(file, owner.vite.config.configFile);
|
|
if (resolved && inside(root, resolved)) await setupClosure(resolved);
|
|
else if (!resolved) throw new Error(`Unresolved setup module: ${file}`);
|
|
}
|
|
for (const file of owner.vite.config.configFileDependencies ?? []) setup.add(realpathSync(file));
|
|
const setupChanged = absoluteChanges.some((file) => setup.has(file));
|
|
const raw = await ctx.getRelevantTestSpecifications();
|
|
const modules = environment.moduleGraph.idToModuleMap;
|
|
const resolvedDependencies = new Map();
|
|
const localImports = new Map();
|
|
const dependencyPath = (dep) => {
|
|
if (!resolvedDependencies.has(dep)) {
|
|
const file = dep.startsWith("/@fs/") ? dep.slice(process.platform === "win32" ? 5 : 4)
|
|
: path.join(owner.config.root, dep);
|
|
try { resolvedDependencies.set(dep, realpathSync(file)); }
|
|
catch { resolvedDependencies.set(dep, null); }
|
|
}
|
|
return resolvedDependencies.get(dep);
|
|
};
|
|
for (const [id, module] of modules) {
|
|
if (!inside(root, id) || !/\.[cm]?[jt]sx?$/.test(id)) continue;
|
|
const source = parse(id);
|
|
const reasons = [...source.reasons];
|
|
if (!tracked.has(path.relative(root, id).split(path.sep).join("/"))) reasons.push("generated or untracked source");
|
|
const local = [];
|
|
localImports.set(id, local);
|
|
// Use the same resolved runtime edges as Vitest, including literal dynamic
|
|
// imports. Resolving the original syntax again uses the wrong conditions
|
|
// for require-only exports and invents edges for erased type imports.
|
|
const transformed = module.transformResult;
|
|
if (!Array.isArray(transformed?.deps)) reasons.push("unknown transform shape");
|
|
for (const dependencyId of [...(transformed?.deps ?? []), ...(transformed?.dynamicDeps ?? [])]) {
|
|
if (isBuiltin(dependencyId)) {
|
|
if (!pureBuiltins.has(dependencyId.replace(/^node:/, ""))) reasons.push(`builtin ${dependencyId}`);
|
|
continue;
|
|
}
|
|
const dependency = dependencyPath(dependencyId);
|
|
if (dependency && inside(root, dependency)) {
|
|
local.push(dependency);
|
|
// Vitest intentionally skips some edges (e.g. CJS, externalized
|
|
// workspaces). Never regard an untracked local edge as independent.
|
|
if (!modules.get(dependency)?.transformResult) reasons.push(`untracked dependency ${path.relative(root, dependency)}`);
|
|
} else {
|
|
const external = dependency?.split(`${path.sep}node_modules${path.sep}`).at(-1).split(path.sep).join("/");
|
|
if (!dependency || !dependency.includes(`${path.sep}node_modules${path.sep}`) || !purePackages.has(packageName(external))) {
|
|
reasons.push(`opaque dependency ${external ? packageName(external) : dependencyId}`);
|
|
}
|
|
}
|
|
}
|
|
if (!module.transformResult) reasons.push("untransformed module");
|
|
if (reasons.length) audit.set(id, [...new Set(reasons)]);
|
|
}
|
|
const retained = [];
|
|
for (const spec of inventory) {
|
|
const mocked = new Set();
|
|
// Honor only unconditional, explicitly supplied mock factories without an
|
|
// importOriginal parameter. Automocks, spies, unmock/importActual, dynamic
|
|
// mocking and escaped vi bindings stay conservative. The related-file
|
|
// graph remains uncut: edits to a mocked module can still select its tests.
|
|
if (owner.config.isolate !== false) {
|
|
for (const name of parse(spec.moduleId).mocks) {
|
|
const target = await resolve(name, spec.moduleId);
|
|
if (target && inside(root, target)) mocked.add(target);
|
|
}
|
|
}
|
|
const queue = [spec.moduleId], visited = new Set();
|
|
let opaque = setupChanged || owner.config.isolate === false || !localImports.has(spec.moduleId);
|
|
while (queue.length && !opaque) {
|
|
const file = queue.pop();
|
|
if (visited.has(file)) continue;
|
|
visited.add(file);
|
|
if (audit.has(file)) { opaque = true; break; }
|
|
for (const dependency of localImports.get(file) ?? []) if (!mocked.has(dependency)) queue.push(dependency);
|
|
}
|
|
if (opaque) retained.push(spec);
|
|
}
|
|
const selected = [...new Set([...raw, ...retained].map(relative))].sort();
|
|
return {
|
|
project,
|
|
inventory: inventory.map(relative).sort(),
|
|
selected,
|
|
related: raw.map(relative).sort(),
|
|
alwaysRun: retained.map(relative).sort(),
|
|
coveredChanges: changed.filter((file, index) => modules.has(absoluteChanges[index]) || setup.has(absoluteChanges[index])),
|
|
opaqueModules: Object.fromEntries([...audit].map(([file, reasons]) => [path.relative(root, file), reasons])),
|
|
setupChanged,
|
|
seconds: (performance.now() - started) / 1000,
|
|
};
|
|
} finally {
|
|
await ctx.close();
|
|
}
|
|
}
|