mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - PR CI selects affected tests before allocating parallel runners. > - Recent audits retained about 96% of test files despite narrow changes. > - Unchanged consumer packages kept full coverage, and source scans confused data fields and message text with runtime loaders. > - File reads need explicit edges that imports alone cannot provide. > - This change follows consumer graphs and known file inputs while retaining unknown effects. > - The AWS CODEOWNERS gate stays in front of runner allocation. ## Linked Issues or Issue Description Refs #18376 and #18362. Narrow source changes still selected most tests because consumer packages kept full coverage and known file reads were treated as arbitrary input. Expected behavior is to retain affected import/file consumers and unknown inputs, then partition only the selected files. The shared storage failure found during the audit was fixed upstream in #18357. The candidate merge ran all 18 managed-skill storage tests successfully in [Linux qualification](https://github.com/paperclipai/paperclip/actions/runs/41140209697/job/124352365547). This branch includes that upstream commit; no additional storage implementation or permanent diagnostic job is included. ## What Changed - Assess every Vitest project in the coarse plan, including unchanged consumer workspaces. - Add dependency edges for statically resolved module-relative filesystem reads and directory listings. Keep unknown paths, writes, escaped APIs, mutable path objects and untracked inputs conservative. - Distinguish data keys, erased types and message strings from runtime globals, loaders and mock bypasses. Retain destructured globals, aliased object reads and mocked filesystem/path resolution. - Normalize `fs` and `node:fs` before recording coverage so a known read cannot hide an unknown one. - Review the Drizzle SQL/schema entry points by hashes of their complete import closures. CommonJS/dynamic loaders reject the fingerprint; drivers and migration readers remain opaque, and dependency updates revoke the review. - Record file input edges and per-test retention reasons in selection audit artifacts. - Add cross-workspace, mixed-import, symlink retargeting, setup-input and dependency-review regression cases. Object-valued URL constants are explicitly opaque with and without mutation. Update the developer guide. - Preserve the AWS CODEOWNERS authorization gate, runner-group restrictions and existing lane ownership. ## Verification - Final head `eedad362e84d6b68f0f048c92db6032e45622c46` passed [complete Actions CI](https://github.com/paperclipai/paperclip/actions/runs/41144233520) on attempt 1. Overall status: 57 successful checks, three intended skips, and one neutral Paperclip review. Both Greptile and Paperclip rated this head 5/5; zero unresolved review threads. - All 17 local selector tests passed: `node --test scripts/ci/test-selection*.test.mjs`. Final [Linux qualification](https://github.com/paperclipai/paperclip/actions/runs/41144233203/job/124365428832) passed all 18 selector/impact tests without skips, plus real workspace selection and cache/routing checks. - Same-tree local comparison against the previous selector at `d331d8481a`: Inbox changes selected 466 rather than 700 files (33% fewer); auth middleware changes selected 2,141 rather than 2,788 files (23% fewer). The sampled plans retained the same 2 and 24 jobs under existing weighted budgets. Final Linux qualification selected 466 files for Inbox and 1,949 for auth/config-test changes; platform-dependent graphs differ, so these are not a Linux before/after comparison. Chat/native Runner lanes retain their existing ownership and are excluded from these counts. - Local `pnpm -r typecheck`, `pnpm build` and `git diff --check` passed. The complete local `pnpm test:run` was attempted and stopped after repeated embedded PostgreSQL startup failures; initdb reported exhausted macOS semaphore capacity. Full Linux CI passed on the final combined tree. - The storage regression was corrected upstream in #18357. [Focused Linux verification](https://github.com/paperclipai/paperclip/actions/runs/41140209697/job/124352365547) ran all 18 managed-skill tests successfully, with no skips; the final full CI run also passed its server lane. ## Risks - Unknown effects must retain their consumers. Regression cases cover mixed builtin spellings, dynamic paths, escaped/mocked APIs, global aliases, reviewed dependency changes and cross-package file reads. - Graph analysis now visits unchanged consumers. Its added work must be included in rollout latency measurements; fewer test files do not by themselves prove shorter end-to-end PR time. - The weighted shard budget deliberately remains unchanged. These samples reduce file execution without reducing allocated jobs. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository inspection, code execution and GitHub tools. The exact serving variant and context window are not exposed in this session. No subagents were used. ## 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 — targeted selector suite; full local suite limitation documented above - [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 - [x] 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>
135 lines
10 KiB
JavaScript
135 lines
10 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { execFileSync } from "node:child_process";
|
|
import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import test from "node:test";
|
|
import { parseSync } from "oxc-parser";
|
|
import { repositoryFileInputs } from "./repository-file-inputs.mjs";
|
|
import { selectProjectTests, sourceDependencies } from "./vitest-selection-graph.mjs";
|
|
import { dependencyFingerprint, dependencyReviews } from "./reviewed-test-dependencies.mjs";
|
|
|
|
const file = "/repo/ui/src/contract.test.ts";
|
|
const analyze = source => repositoryFileInputs(file, parseSync(file, source).program);
|
|
|
|
test("data properties and erased types are not global loaders or mock bypasses", () => {
|
|
const data = sourceDependencies(file, 'import {vi} from "vitest"; const schema = {arguments: {}, fetch: false, Function: "name"}; type Factory = Function;');
|
|
assert.deepEqual(data.reasons, []);
|
|
const chained = sourceDependencies(file, 'import {z} from "zod"; const schema = z.object({message:z.literal("routes require auth; fetch input")}).refine(value => true);');
|
|
assert.deepEqual(chained.reasons, []);
|
|
for (const source of [
|
|
'fetch("https://example.invalid");',
|
|
'const load = globalThis.fetch; load("https://example.invalid");',
|
|
'const make = globalThis.Function; make("return 1");',
|
|
'const scope = globalThis; const make = scope.Function; make("return 1");',
|
|
'const {Function: make} = globalThis; make("return 1");',
|
|
'const {fetch: request} = globalThis; request("https://example.invalid");',
|
|
'import {vi as mocker} from "vitest"; mocker.mock("node:path", () => ({join: () => "/other.ts"}));',
|
|
'import {vi} from "vitest"; vi.mock("./input", function() { return arguments[0](); });',
|
|
'import {vi} from "vitest"; vi.importActual("./input");',
|
|
'const result = (fetch("https://example.invalid") as unknown);',
|
|
]) assert.ok(sourceDependencies(file, source).reasons.length, source);
|
|
});
|
|
|
|
test("reviewed SQL builders exclude drivers and revoke approval when imported source changes", t => {
|
|
const root = realpathSync("server/node_modules/drizzle-orm");
|
|
const review = dependencyReviews();
|
|
for (const entry of ["index.js", "pg-core/index.js"]) assert.equal(review(path.join(root, entry), `drizzle-orm/${entry}`), true);
|
|
assert.equal(review(path.join(root, "postgres-js/index.js"), "drizzle-orm/postgres-js/index.js"), false);
|
|
const fixture = realpathSync(mkdtempSync(path.join(tmpdir(), "ci-review-")));
|
|
t.after(() => rmSync(fixture, { recursive: true, force: true }));
|
|
const entry = path.join(fixture, "index.js"), leaf = path.join(fixture, "leaf.js");
|
|
writeFileSync(entry, 'export * from "./leaf.js";'); writeFileSync(leaf, 'export const value = 1;');
|
|
const before = dependencyFingerprint(entry, fixture);
|
|
writeFileSync(leaf, 'export const value = 2;');
|
|
assert.notEqual(dependencyFingerprint(entry, fixture), before);
|
|
assert.equal(dependencyReviews()(entry, "drizzle-orm/index.js"), false);
|
|
writeFileSync(leaf, 'import "node:fs";');
|
|
assert.throws(() => dependencyFingerprint(entry, fixture), /Unreviewed external import/);
|
|
for (const source of ['import("./child.js");', 'require("./child.js");', 'const load = require; load("./child.js");']) {
|
|
writeFileSync(leaf, source);
|
|
assert.throws(() => dependencyFingerprint(entry, fixture), /Unreviewed runtime loader/);
|
|
}
|
|
});
|
|
|
|
test("file input edges require module-relative paths and non-escaping read APIs", () => {
|
|
for (const source of [
|
|
'import {readFileSync as read} from "node:fs"; read(new URL("./leaf.ts", import.meta.url));',
|
|
'import fs from "node:fs"; import path from "node:path"; const root = path.dirname(import.meta.filename); const input = path.join(root, "leaf.ts"); fs.readFileSync(input);',
|
|
'import {promises as fs} from "node:fs"; import {fileURLToPath} from "node:url"; fs.readFile(fileURLToPath(new URL("./leaf.ts", import.meta.url)));',
|
|
]) {
|
|
assert.deepEqual(analyze(source), { covered: ["fs"], inputs: ["/repo/ui/src/leaf.ts"] });
|
|
}
|
|
for (const source of [
|
|
'import fs from "node:fs"; fs.readFileSync(process.env.INPUT);',
|
|
'import {readFileSync as known} from "node:fs"; import {readFileSync as unknown} from "fs"; known(new URL("./leaf.ts", import.meta.url)); unknown(process.env.INPUT);',
|
|
'import {readFileSync as known} from "fs"; import {readFileSync as unknown} from "node:fs"; known(new URL("./leaf.ts", import.meta.url)); unknown(process.env.INPUT);',
|
|
'import fs from "node:fs"; fs.readFileSync("src/leaf.ts");',
|
|
'import fs from "node:fs"; const read = fs.readFileSync; read(new URL("./leaf.ts", import.meta.url));',
|
|
'import {readFileSync as read} from "node:fs"; function load(read) { read(new URL("./leaf.ts", import.meta.url)); }',
|
|
'import fs from "node:fs"; const input = new URL("./leaf.ts", import.meta.url); function load(input) { fs.readFileSync(input); }',
|
|
'import fs from "node:fs"; let input = new URL("./leaf.ts", import.meta.url); fs.readFileSync(input);',
|
|
'import fs from "node:fs"; fs.writeFileSync(new URL("./leaf.ts", import.meta.url), "value");',
|
|
'import fs from "node:fs"; consume(fs); fs.readFileSync(new URL("./leaf.ts", import.meta.url));',
|
|
'import fs from "node:fs"; const URL = custom; fs.readFileSync(new URL("./leaf.ts", import.meta.url));',
|
|
'import fs from "node:fs"; fs.promises[method](new URL("./leaf.ts", import.meta.url));',
|
|
'import fs from "node:fs"; fs.readFileSync(new URL("https://example.invalid/source.ts"));',
|
|
'import fs from "node:fs"; const input = new URL("./leaf.ts", import.meta.url); fs.readFileSync(input);',
|
|
'import fs from "node:fs"; const input = new URL("./leaf.ts", import.meta.url); input.pathname = "/other.ts"; fs.readFileSync(input);',
|
|
'import fs from "node:fs"; import path from "node:path"; path.join = custom; fs.readFileSync(path.join(__dirname,"leaf.ts"));',
|
|
'import fs from "node:fs"; import path from "node:path"; mutate(path); fs.readFileSync(path.join(__dirname,"leaf.ts"));',
|
|
'import fs from "node:fs"; import.meta.url = "file:///other/test.ts"; fs.readFileSync(new URL("./leaf.ts", import.meta.url));',
|
|
'import fs from "node:fs"; const meta = import.meta; meta.url = "file:///other/test.ts"; fs.readFileSync(new URL("./leaf.ts", import.meta.url));',
|
|
]) assert.deepEqual(analyze(source).covered, [], source);
|
|
});
|
|
|
|
test("unchanged consumer workspaces follow cross-package imports and file inputs", async t => {
|
|
const root = realpathSync(mkdtempSync(path.join(tmpdir(), "ci-inputs-")));
|
|
t.after(() => rmSync(root, { recursive: true, force: true }));
|
|
symlinkSync(path.join(process.cwd(), "node_modules"), path.join(root, "node_modules"), "dir");
|
|
const write = (file, source) => {
|
|
mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
|
writeFileSync(path.join(root, file), source);
|
|
};
|
|
write("vitest.config.mjs", 'export default {test:{projects:["consumer", "independent"]}}');
|
|
for (const name of ["consumer", "independent"]) {
|
|
write(`${name}/vitest.config.mjs`, `export default {test:{name:"${name}",include:["*.test.ts"]}}`);
|
|
write(`${name}/unrelated.test.ts`, 'export const value = 1;');
|
|
}
|
|
write("producer/leaf.ts", "export const value = 1;");
|
|
write("producer/other.ts", "export const other = 1;");
|
|
write("consumer/import.test.ts", 'import {value} from "../producer/leaf"; console.log(value);');
|
|
write("consumer/read.test.ts", 'import {readFileSync} from "node:fs"; readFileSync(new URL("../producer/leaf.ts",import.meta.url));');
|
|
write("consumer/directory.test.ts", 'import {readdirSync} from "node:fs"; readdirSync(new URL("../producer/",import.meta.url));');
|
|
write("consumer/dynamic.test.ts", 'import {readFileSync} from "node:fs"; readFileSync(process.env.INPUT);');
|
|
write("consumer/mixed.test.ts", 'import {readFileSync as known} from "node:fs"; import {readFileSync as unknown} from "fs"; known(new URL("../producer/leaf.ts",import.meta.url)); unknown(process.env.INPUT);');
|
|
write("consumer/helper.ts", 'import {readFileSync} from "node:fs"; export const source = readFileSync(new URL("../producer/leaf.ts",import.meta.url));');
|
|
write("consumer/helper.test.ts", 'import {source} from "./helper"; console.log(source);');
|
|
write("consumer/outside.test.ts", 'import {readFileSync} from "node:fs"; readFileSync(new URL("/etc/hosts",import.meta.url));');
|
|
const git = (...args) => execFileSync("git", args, { cwd: root, stdio: "ignore" });
|
|
git("init", "-q"); git("add", ".");
|
|
const select = changed => selectProjectTests({ root, project: "consumer", changed });
|
|
const affected = await select(["producer/leaf.ts"]);
|
|
assert.deepEqual(affected.related, ["consumer/import.test.ts"]);
|
|
assert.deepEqual(affected.inputRelated, ["consumer/directory.test.ts", "consumer/helper.test.ts", "consumer/mixed.test.ts", "consumer/read.test.ts"]);
|
|
assert.deepEqual(affected.alwaysRun, ["consumer/dynamic.test.ts", "consumer/mixed.test.ts", "consumer/outside.test.ts"]);
|
|
assert.ok(!affected.selected.includes("consumer/unrelated.test.ts"));
|
|
const other = await select(["producer/other.ts"]);
|
|
assert.deepEqual(other.selected, ["consumer/directory.test.ts", "consumer/dynamic.test.ts", "consumer/mixed.test.ts", "consumer/outside.test.ts"]);
|
|
const independent = await selectProjectTests({ root, project: "independent", changed: ["producer/leaf.ts"] });
|
|
assert.deepEqual(independent.selected, [], "a project without imports or opaque inputs can drop out entirely");
|
|
write("producer/new.ts", "export const added = true;"); git("add", ".");
|
|
assert.ok((await select(["producer/new.ts"])).inputRelated.includes("consumer/directory.test.ts"));
|
|
symlinkSync("leaf.ts", path.join(root, "producer/link.ts"));
|
|
write("consumer/symlink.test.ts", 'import {readFileSync} from "node:fs"; readFileSync(new URL("../producer/link.ts",import.meta.url));');
|
|
git("add", ".");
|
|
assert.ok((await select(["producer/leaf.ts"])).inputRelated.includes("consumer/symlink.test.ts"));
|
|
rmSync(path.join(root, "producer/link.ts"));
|
|
symlinkSync("other.ts", path.join(root, "producer/link.ts"));
|
|
assert.ok((await select(["producer/link.ts"])).inputRelated.includes("consumer/symlink.test.ts"));
|
|
write("generated/unknown.ts", "export const generated = true;");
|
|
write("consumer/generated.test.ts", 'import {readdirSync} from "node:fs"; readdirSync(new URL("../generated/",import.meta.url));');
|
|
git("add", "consumer/generated.test.ts");
|
|
assert.ok((await select(["producer/other.ts"])).alwaysRun.includes("consumer/generated.test.ts"));
|
|
});
|