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 suites and test files to reduce feedback time. > - A selected test inherited the parent selection when it launched an independent runner fixture. > - Ordinary source filenames containing config or setup also forced broad coverage. > - AWS runner access must depend on current CI CODEOWNERS, not only a stored numeric roster. > - This pull request fixes both selection problems and verifies owners before AWS allocation. > - Contributors retain GitHub-hosted CI. Authorized maintainers retain AWS test and final verification jobs. ## Linked Issues or Issue Description Refs #18362 and #18348. The selected server run for #18369 failed in `vitest-chat-shards.test.ts` because its temporary fixture inherited `PAPERCLIP_CI_TEST_SELECTION` and attempted `git rev-parse HEAD` outside a Git repository. #18366 could not use graph selection because `session-config-metadata.test.ts` matched the broad filename rule. The AWS setting was already enabled. The reviewed outsider PRs correctly used hosted runners; a maintainer PR used AWS, including final verification. This change strengthens that boundary rather than enabling AWS for outsiders. ## What Changed - Remove selection settings from Vitest child environments after the wrapper validates the selected shard. - Match conventional Vite/Vitest configuration filenames rather than any occurrence of config or setup. Keep graph tracking for actual setup and configuration dependencies. - Bind rerun actor lookup to the exact run attempt. Require the author, event sender and rerun actor to be current CI CODEOWNERS and members of the existing numeric allowlist. Read the resolver and CODEOWNERS from one protected master revision. - Run metadata authorization on GitHub-hosted capacity before any AWS allocation. Keep authorized test and final verification placement on the AWS fleet. - Add a real selected-server regression that launches nested chat fixtures, negative authorization cases, and qualification for a real config-named test. - Update the developer and profiling guides. - Retire eight legacy PR workflow SHA authorizations and one deleted qualification-branch reference from the live `paperclip-public-pr` runner group. The remaining three references are the trusted PR workflow and the existing full-stack/protocol evaluation workflows on master. No active PR run referenced the retired versions. ## Verification - Selection regression tests: 13 passed, including the real nested chat runner. - Routing, planner and profiling tests: 100 passed, including 69 routing cases. - `actionlint` passed for both changed workflows. `git diff --check` passed. - Confirmed protected master still requires CODEOWNER review. A live lookup found four CI owners, with three of six numeric roster entries eligible and three denied by the new owner check. - `pnpm -r typecheck` and `pnpm build` passed. The full local Vitest suite was stopped after repeated embedded PostgreSQL initialization failures; it did not pass. On final revision `0b40ae15b7996603e405edd4157a5c337e0405c7`, both Linux qualification jobs and all required CI gates passed. The main PR workflow completed with 48 successful jobs and one intentional skip, including final verification. [CI run](https://github.com/paperclipai/paperclip/actions/runs/41126881716). Greptile reviewed the final revision at 5/5 with no inline findings. - Shard and workflow contracts: 44 passed. CI first caught an outdated assertion that required the routing job to use AWS; it now requires hosted authorization, and the policy job passes. - A read-only live gate exercise preserved the existing merge-freshness denial when master advanced beyond the available merge ref. It allocated no runner and was not a successful admission proof. ## Risks - The short metadata authorization job now depends on GitHub-hosted queue capacity. This prevents a stale roster from allocating AWS before current CODEOWNERS are checked. Authorized test and final verification jobs still use AWS. - Identity/API failures fall back to hosted CI. No caller can override the trusted owner list with PR contents. - Configuration dependencies still require conservative graph handling. Existing setup dependency, opaque I/O, stale manifest and wrong-lane tests remain in place. - The runner group is restricted to selected repositories and selected workflows. Its workflow allowlist was verified and narrowed through the GitHub settings UI. Legacy pinned callers must update to the current trusted workflow before using AWS; active PR runs used master references. The current PR workflow authorization and evaluation workflow authorizations remain in place. ## Model Used - OpenAI Codex, based on GPT-6. The exact serving variant and context limit are not exposed in this session. Used reasoning, repository inspection, code execution and GitHub tools. ## 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 suites passed; the full local suite hit the PostgreSQL startup limitation documented above, and full Linux CI passed - [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>
218 lines
15 KiB
JavaScript
218 lines
15 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { execFileSync, spawnSync } from "node:child_process";
|
|
import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import test from "node:test";
|
|
import { selectProjectTests } from "./vitest-selection-graph.mjs";
|
|
import { partitionSelection, refineTestPlan } from "./test-selection-plan.mjs";
|
|
import { selectionHash, selectedShard, validateSelection } from "./test-selection-manifest.mjs";
|
|
import { selectionEligibility } from "./select-tests.mjs";
|
|
import { mergeDiff } from "../../.github/scripts/ci-diff.mjs";
|
|
|
|
const root = process.cwd();
|
|
const head = execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).trim();
|
|
|
|
test("real graph retains direct and indirect filesystem, process and opaque dependencies", async (t) => {
|
|
const directory = realpathSync(mkdtempSync(path.join(tmpdir(), "ci-vg-")));
|
|
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
|
mkdirSync(path.join(directory, "ui/src"), { recursive: true });
|
|
mkdirSync(path.join(directory, "ui/src/glob"));
|
|
symlinkSync(path.join(root, "node_modules"), path.join(directory, "node_modules"), "dir");
|
|
const write = (file, text) => writeFileSync(path.join(directory, file), text);
|
|
write("vitest.config.mjs", 'export default {test:{projects:["ui"]}}');
|
|
write("ui/vitest.config.mjs", `export default {resolve:{alias:{"@leaf":${JSON.stringify(path.join(directory, "ui/src/leaf.ts"))}}},test:{name:"fixture",include:["src/**/*.test.ts"],setupFiles:["./setup.ts"]}}`);
|
|
write("ui/setup.ts", 'import "./src/environment.ts";');
|
|
write("ui/src/environment.ts", "export const environment = true;");
|
|
write("ui/src/leaf.ts", "export const value = 42;");
|
|
write("ui/src/bridge.ts", 'export {value} from "@leaf";');
|
|
write("ui/src/helper.ts", 'import {readFileSync} from "node:fs"; export const read = (file: string) => readFileSync(file, "utf8");');
|
|
write("ui/src/import.test.ts", 'import {value} from "./bridge"; console.log(value); throw new Error("selection must not execute tests");');
|
|
write("ui/src/reader.test.ts", 'import {readFileSync} from "node:fs"; readFileSync(new URL("./leaf.ts", import.meta.url));');
|
|
write("ui/src/indirect.test.ts", 'import {read} from "./helper"; read("input"); throw new Error("selection must not execute tests");');
|
|
write("ui/src/process.test.ts", 'import {execFileSync} from "node:child_process"; execFileSync("must-not-execute");');
|
|
write("ui/src/dynamic.test.ts", 'const load = (file: string) => import(file);');
|
|
write("ui/src/cjs.test.ts", 'const load = require;');
|
|
write("ui/src/builtin-loader.test.ts", 'process.getBuiltinModule("fs").readFileSync("input");');
|
|
write("ui/src/opaque.test.ts", 'import {parse} from "yaml"; console.log(parse("value: true"));');
|
|
write("ui/src/independent.test.ts", 'export const independent = true;');
|
|
write("ui/src/mocked.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",()=>({read:()=>"mock"})); import {read} from "./helper"; read("input");');
|
|
write("ui/src/partial.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",async (original)=>await original()); import {read} from "./helper"; read("input");');
|
|
write("ui/src/automock.test.ts", 'import {vi} from "vitest"; vi.mock("./helper"); import {read} from "./helper"; read("input");');
|
|
write("ui/src/bypass.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",()=>({read:()=>"mock"})); await vi.importActual("./helper");');
|
|
write("ui/src/spy.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",{spy:true}); import {read} from "./helper"; read("input");');
|
|
write("ui/src/remock.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",()=>({read:()=>"mock"})); vi.doMock("./helper",async original=>await original());');
|
|
write("ui/src/arguments.test.ts", 'import {vi} from "vitest"; vi.mock("./helper",function(){return arguments[0]();});');
|
|
write("ui/src/glob/one.ts", "export const value = 1;");
|
|
write("ui/src/globber.ts", 'export const modules = import.meta.glob("./glob/*.ts", {eager:true});');
|
|
write("ui/src/glob.test.ts", 'import {modules} from "./globber"; console.log(modules);');
|
|
execFileSync("git", ["init", "-q"], { cwd: directory });
|
|
execFileSync("git", ["add", "."], { cwd: directory });
|
|
const select = (changed) => selectProjectTests({ root: directory, project: "fixture", changed });
|
|
const result = await select(["ui/src/leaf.ts"]);
|
|
assert.deepEqual(result.related, ["ui/src/import.test.ts"]);
|
|
for (const file of ["reader", "indirect", "process", "dynamic", "cjs", "builtin-loader", "opaque", "partial", "automock", "bypass", "spy", "remock", "arguments"]) {
|
|
assert.ok(result.alwaysRun.includes(`ui/src/${file}.test.ts`), `${file} must never be dropped`);
|
|
}
|
|
assert.ok(!result.selected.includes("ui/src/independent.test.ts"));
|
|
assert.ok(!result.alwaysRun.includes("ui/src/mocked.test.ts"), "a completely replaced local dependency performs no real IO");
|
|
assert.ok(!result.alwaysRun.includes("ui/src/glob.test.ts"), "literal Vite globs use the real dependency graph");
|
|
assert.ok((await select(["ui/src/glob/one.ts"])).related.includes("ui/src/glob.test.ts"));
|
|
write("ui/src/glob/two.ts", "export const value = 2;");
|
|
execFileSync("git", ["add", "."], { cwd: directory });
|
|
assert.ok((await select(["ui/src/glob/two.ts"])).related.includes("ui/src/glob.test.ts"), "new glob members cannot be omitted");
|
|
const setup = await select(["ui/src/environment.ts"]);
|
|
assert.equal(setup.setupChanged, true);
|
|
assert.deepEqual(setup.selected, setup.inventory, "transitive setup edits affect every test");
|
|
write("ui/src/broken.test.ts", 'import "./missing";');
|
|
const broken = await select(["ui/src/leaf.ts"]);
|
|
assert.ok(broken.selected.includes("ui/src/broken.test.ts"), "an unresolved dependency must retain its test");
|
|
write("ui/src/generated.ts", "export const generated = 42;");
|
|
write("ui/src/generated.test.ts", 'import {generated} from "./generated"; console.log(generated);');
|
|
execFileSync("git", ["add", "ui/src/generated.test.ts"], { cwd: directory });
|
|
assert.ok((await select(["ui/src/leaf.ts"])).alwaysRun.includes("ui/src/generated.test.ts"),
|
|
"generated inputs have no trustworthy source-to-output graph and retain their consumers");
|
|
write("ui/src/environment.ts", 'import {readFileSync} from "node:fs"; readFileSync(new URL("./leaf.ts", import.meta.url));');
|
|
await assert.rejects(select(["ui/src/leaf.ts"]), /Opaque setup builtin/, "setup filesystem reads can affect every test outside the import graph");
|
|
mkdirSync(path.join(directory, "server/src/__tests__/helpers"), { recursive: true });
|
|
for (const file of ["server/src/__tests__/setup-supertest.ts", "server/src/__tests__/helpers/isolated-test-home.ts"]) {
|
|
write(file, readFileSync(path.join(root, file), "utf8"));
|
|
}
|
|
write("ui/setup.ts", 'import "../server/src/__tests__/setup-supertest.ts";');
|
|
const reviewed = await select(["ui/src/leaf.ts"]);
|
|
assert.ok(!reviewed.selected.includes("ui/src/independent.test.ts"), "reviewed isolated setup permits file selection");
|
|
const helper = "server/src/__tests__/helpers/isolated-test-home.ts";
|
|
const changedSetup = await select([helper]);
|
|
assert.deepEqual(changedSetup.selected, changedSetup.inventory, "reviewed helpers still invalidate all tests when changed");
|
|
write(helper, `${readFileSync(path.join(directory, helper), "utf8")}\n// setup edited since review\n`);
|
|
await assert.rejects(select(["ui/src/leaf.ts"]), /Opaque setup/, "a stale review cannot authorize new setup effects");
|
|
});
|
|
|
|
test("partitioning reduces runners and preserves an exact, non-overlapping file cover", () => {
|
|
const all = Array.from({ length: 120 }, (_, index) => `server/src/${index}.test.ts`);
|
|
const selected = all.slice(0, 15);
|
|
const shards = partitionSelection(all, selected, 12);
|
|
assert.equal(shards.length, 2);
|
|
assert.deepEqual(shards.flat().sort(), selected.sort());
|
|
assert.equal(new Set(shards.flat()).size, selected.length);
|
|
assert.deepEqual(partitionSelection(all, [], 12), []);
|
|
assert.equal(partitionSelection(all, all, 12).length, 12);
|
|
const durations = Object.fromEntries(all.map((file, index) => [file, index === 0 ? 1000 : 1]));
|
|
assert.equal(partitionSelection(all, [all[0]], 12, durations).length, 1);
|
|
assert.equal(partitionSelection(all, [all[0]], 12, Object.fromEntries(all.map((file) => [file, 0]))).length, 1);
|
|
});
|
|
|
|
function fixturePlan() {
|
|
const files = ["ui/src/pages/Inbox.test.tsx", "ui/src/pages/apps/chat/chat-ui-contract.test.ts"];
|
|
const entries = [0, 1].map((shard_index) => ({ group: "general-workspaces-a", group_label: `workspaces-a (${shard_index + 1}/2)`, shard_index, shard_count: 2, task: "ci:workspaces-a" }));
|
|
const plan = { version: 1, mode: "enabled", forceReason: null, changed: ["ui/src/pages/Inbox.tsx"], jobs: {
|
|
general_tests: { run: true, matrix: { include: entries }, selected: 2, total: 2 },
|
|
verify_serialized_server: { run: false, matrix: { include: [{ task: "ci:serialized" }] } },
|
|
} };
|
|
const owners = { "general-workspaces-a": { "@paperclipai/ui": files } };
|
|
const graphs = [{ project: "@paperclipai/ui", inventory: files, selected: [files[1]] }];
|
|
return { files, ...refineTestPlan({ plan, head, owners, graphs }) };
|
|
}
|
|
|
|
test("runner consumes the exact manifest, and rejects stale, missing, or mismatched manifests", (t) => {
|
|
const directory = realpathSync(mkdtempSync(path.join(tmpdir(), "ci-vm-")));
|
|
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
|
const { manifest, plan } = fixturePlan();
|
|
assert.equal(plan.jobs.general_tests.selected, 1);
|
|
const file = path.join(directory, "selection.json");
|
|
const run = (selection = manifest, hash = selectionHash(manifest)) => {
|
|
writeFileSync(file, JSON.stringify(selection));
|
|
return spawnSync(process.execPath, ["scripts/run-vitest-stable.mjs", "--mode", "general", "--group", "general-workspaces-a", "--shard-index", "0", "--shard-count", "1", "--dry-run"], {
|
|
cwd: root, encoding: "utf8", env: { ...process.env, PAPERCLIP_CI_TEST_SELECTION: file, PAPERCLIP_CI_SELECTION_HASH: hash },
|
|
});
|
|
};
|
|
const result = run();
|
|
assert.equal(result.status, 0, result.stderr);
|
|
assert.deepEqual(JSON.parse(result.stdout).selection, manifest.lanes["general-workspaces-a"].shards[0]);
|
|
assert.notEqual(run({ ...manifest, head: "0".repeat(40) }).status, 0);
|
|
assert.notEqual(run(manifest, "bad-hash").status, 0);
|
|
assert.throws(() => selectedShard(manifest, { mode: "general", group: "general-workspaces-a", shardCount: 2 }, ["@paperclipai/ui"]));
|
|
assert.throws(() => selectedShard(manifest, { mode: "general", group: "general-workspaces-a", shardCount: 1 }, ["@paperclipai/server"]));
|
|
const duplicate = structuredClone(manifest);
|
|
duplicate.lanes["general-workspaces-a"].shards.push(duplicate.lanes["general-workspaces-a"].shards[0]);
|
|
assert.throws(() => validateSelection(duplicate), /assigned twice/);
|
|
const wrongLane = { version: 1, head, lanes: { "general-server-without-chat": {
|
|
shards: [{ "@paperclipai/server": ["server/src/__tests__/issues-service.test.ts"] }],
|
|
} } };
|
|
writeFileSync(file, JSON.stringify(wrongLane));
|
|
const misplaced = spawnSync(process.execPath, ["scripts/run-vitest-stable.mjs", "--mode", "general", "--group", "general-server-without-chat", "--shard-index", "0", "--shard-count", "1", "--dry-run"], {
|
|
cwd: root, encoding: "utf8", env: { ...process.env, PAPERCLIP_CI_TEST_SELECTION: file, PAPERCLIP_CI_SELECTION_HASH: selectionHash(wrongLane) },
|
|
});
|
|
assert.notEqual(misplaced.status, 0);
|
|
assert.match(misplaced.stderr, /belongs to another server lane/);
|
|
});
|
|
|
|
test("forced modes, config, fixtures, deletions and unknown inputs cannot narrow CI", () => {
|
|
const plan = fixturePlan().plan;
|
|
assert.equal(selectionEligibility(plan), null);
|
|
for (const changed of [["ui/vitest.setup.ts"], ["ui/vite.config.ts"], ["package.json"], ["pnpm-lock.yaml"],
|
|
["docs/api/issues.md"], ["ui/src/missing-deleted.ts"], ["server/fixtures/input.json"], []]) {
|
|
assert.ok(selectionEligibility({ ...plan, changed }), JSON.stringify(changed));
|
|
}
|
|
for (const mode of ["full", "shadow"]) assert.ok(selectionEligibility({ ...plan, mode }));
|
|
assert.ok(selectionEligibility({ ...plan, forceReason: "full-ci" }));
|
|
});
|
|
|
|
test("ordinary source names containing config or setup remain graph eligible", (t) => {
|
|
const directory = mkdtempSync(path.join(tmpdir(), "ci-names-"));
|
|
t.after(() => { process.chdir(root); rmSync(directory, { recursive: true, force: true }); });
|
|
process.chdir(directory);
|
|
const plan = fixturePlan().plan;
|
|
for (const file of [
|
|
"server/src/services/heartbeat/session-config-metadata.test.ts",
|
|
"server/src/storage/config.ts",
|
|
"ui/src/pages/apps/chat/slack/setup-state.test.ts",
|
|
]) {
|
|
mkdirSync(path.dirname(file), { recursive: true });
|
|
writeFileSync(file, "export {};\n");
|
|
assert.equal(selectionEligibility({ ...plan, changed: [file] }), null, file);
|
|
}
|
|
for (const file of ["ui/src/vitest.setup.ts", "server/src/vitest.config.mts"]) {
|
|
assert.equal(selectionEligibility({ ...plan, changed: [file] }), `Broad input: ${file}`);
|
|
}
|
|
});
|
|
|
|
test("the CLI publishes the complete coarse plan if setup mutates tracked inputs", (t) => {
|
|
const directory = realpathSync(mkdtempSync(path.join(tmpdir(), "ci-vd-")));
|
|
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
|
mkdirSync(path.join(directory, "ui/src"), { recursive: true });
|
|
const file = path.join(directory, "ui/src/input.ts");
|
|
const git = (...args) => execFileSync("git", args, { cwd: directory, encoding: "utf8" }).trim();
|
|
const commit = () => git("-c", "user.name=CI", "-c", "user.email=ci@example.invalid", "commit", "-qam", "fixture");
|
|
git("init", "-q"); writeFileSync(file, "export const value = 1;"); git("add", "."); commit();
|
|
const base = git("rev-parse", "HEAD");
|
|
git("checkout", "-qb", "pr");
|
|
writeFileSync(file, "export const value = 2;"); commit();
|
|
const prHead = git("rev-parse", "HEAD");
|
|
git("checkout", "--detach", base); git("-c", "user.name=CI", "-c", "user.email=ci@example.invalid", "merge", "--no-ff", "pr", "-m", "merge");
|
|
const merge = git("rev-parse", "HEAD");
|
|
writeFileSync(file, "export const value = 3; // generated during setup");
|
|
const plan = { version: 1, mode: "enabled", forceReason: null, ...mergeDiff(merge, prHead, directory),
|
|
jobs: { general_tests: { run: true, selected: 2, matrix: { include: [{ shard_index: 0 }, { shard_index: 1 }] } } } };
|
|
const result = spawnSync(process.execPath, [path.join(root, "scripts/ci/select-tests.mjs")], {
|
|
cwd: directory, encoding: "utf8", env: { ...process.env, CI_PLAN: JSON.stringify(plan), BASE_SHA: base,
|
|
GITHUB_SHA: merge, GITHUB_OUTPUT: "", GITHUB_STEP_SUMMARY: "" },
|
|
});
|
|
assert.equal(result.status, 0, result.stderr);
|
|
const fallback = JSON.parse(readFileSync(path.join(directory, ".ci-test-selection/plan.json"), "utf8"));
|
|
assert.match(fallback.testSelectionFallback, /Source inputs changed/);
|
|
assert.deepEqual(fallback.jobs, plan.jobs);
|
|
assert.equal(fallback.testSelection, undefined);
|
|
});
|
|
|
|
|
|
test("mixed audited documentation stays graph eligible and large complete source diffs are allowed", () => {
|
|
const plan = fixturePlan().plan;
|
|
for (const doc of ["doc/DEVELOPING.md", "doc/DATABASE.md", "doc/plans/2026-10-30-example.md"]) {
|
|
assert.equal(selectionEligibility({ ...plan, changed: [...plan.changed, doc] }), null);
|
|
}
|
|
assert.equal(selectionEligibility({ ...plan, changed: Array(350).fill(plan.changed[0]) }), null);
|
|
assert.ok(selectionEligibility({ ...plan, changed: ["doc/DEVELOPING.md"] }));
|
|
assert.ok(selectionEligibility({ ...plan, changed: [...plan.changed, "docs/api/issues.md"] }));
|
|
});
|