mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The repository has a supply-chain rule: contributors do not commit lockfiles. A bot and verified Dependabot commits write them. > - The rule checks only the root `pnpm-lock.yaml`. > - The sandbox providers are outside the root workspace, so they have two more pnpm lockfiles. Pull requests edited them by hand, for example #12791, #18839 and #19489. > - This pull request makes the same rule and the same bot cover those two lockfiles. > - The benefit is one lockfile rule for every managed pnpm lockfile, and no lockfile edits in contributor pull requests. ## Linked Issues or Issue Description Refs #19489 **What existing behavior does this improve?** The lockfile policy and the lockfile refresh bot. **Subsystem affected** CI: `.github/scripts/lockfile-trust.mjs`, `.github/workflows/refresh-lockfile.yml`, and the sandbox provider installs. **Current behavior** `lockfile-trust.mjs` blocks contributor edits to `pnpm-lock.yaml` only. A contributor can edit `scripts/typecheck-sandbox-providers/pnpm-lock.yaml` or `packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml`, and CI passes. The refresh bot does not update these files. `typecheck:test-programs` fails on a stale provider lockfile and tells the contributor to commit a new one. **Proposed behavior** The policy blocks contributor edits to all three lockfiles. The refresh bot regenerates all three after merge. CI resolves a stale provider lockfile in place for the run, as it does for the root lockfile. **Reason and benefit** Lockfile content comes only from the bot or from verified Dependabot commits. Contributors do not need to run pnpm in special folders. **Breaking changes** None. A pull request that edits a provider lockfile now fails the policy check. Remove the lockfile change from the pull request. The bot commits it after merge. ## What Changed - `lockfile-trust.mjs` now exports `LOCKFILES`: the root lockfile, the shared sandbox provider lockfile, and the Daytona lockfile. The policy blocks contributor edits to each of them. A Dependabot pull request must match a verified Dependabot commit for each lockfile that it changes. The `merged` command and the reporting-lock proof still use only the root lockfile. - `check-pr-lockfile.mjs` (advisory check) uses the same list. - `refresh-lockfile.yml`: - A job-level `LOCKFILES` env lists the same three files. A test checks that it equals the policy list. - A new step regenerates the two provider lockfiles with `--resolution-only --ignore-scripts --no-frozen-lockfile`. - The unexpected-changes check, the change check, and `git add` cover all three lockfiles. - The release-age gate checks the added versions in every lockfile. - A held refresh saves all three lockfiles at their repository paths, with a `source-locks.sha256` file. The next run reuses them only if every master lockfile is unchanged. An artifact in the old format is not reused. - The PR title is now `chore(lockfile): refresh pnpm lockfiles`. - `check-lockfile-release-age.mjs` accepts more than one `--base`/`--head` pair. - `scripts/typecheck-test-programs.mjs`: if the frozen install of the provider workspace fails, the script resolves the lockfile in place and installs frozen again. - `scripts/build-standalone-public-packages.mjs`: the same fallback applies to a package with a committed lockfile (Daytona). `release.sh` already restores tracked files when it exits. - `doc/DEVELOPING.md` and `doc/RELEASE-AUTOMATION-SETUP.md` describe the managed lockfiles. They also say that the Pi and Kimi distribution lockfiles stay hand-managed, because they hold attested provider pins. ## Verification - `node --test .github/scripts/tests/*.test.mjs scripts/__tests__/build-standalone-concurrency.test.mjs`: 731 tests pass. New tests cover: - the policy for provider lockfiles, including a real git history with a Dependabot commit and a later hand edit - the workflow and policy lists match - the unexpected-changes check, run with bash against a fixture - a held refresh round trip, and its invalidation when a provider lockfile changes on master - the release-age gate across lockfile pairs - the inline fallback in both install scripts - `node --test scripts/typecheck-test-programs.test.mjs`: the install tests pass. Two tests fail locally only because this worktree has no root `node_modules` (no `tsc`). They also fail without this change. - I ran the new bot commands on master with pnpm 9.15.4. No package version changed. pnpm 9.15.4 removes `libc:` fields and adds `deprecated:` notes, so the first refresh is a metadata-only change. The release-age gate passes it because it adds no versions. - I ran the release-age and held-lock steps locally with bash against this commit. The gate reported `eligible=true`, and `sha256sum --check` accepted the saved hashes. - The workflow YAML parses, and `bash -n` accepts every `run` block. - Not testable before merge: the policy job loads `lockfile-trust.mjs` from the base branch, so the new rule applies only to pull requests opened after this merges. The refresh workflow runs only on master. ## Risks - The release build now resolves a stale Daytona lockfile in place, from the registry. The root release install already resolves without a frozen lockfile, so this is the same level of trust. - The paid runner e2e job still installs Daytona with `--frozen-lockfile`. After a Daytona manifest change merges, its Daytona cases fail until the bot's refresh merges. This matches the rule that paid jobs refuse a new root lockfile until its refresh merges. - Dependabot pull requests for the provider entries in #19489 change a provider lockfile. They pass only if each lockfile matches a verified Dependabot commit, the same as for the root lockfile. - Low risk otherwise: the bot keeps its existing token, environment, and merge rules. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, and git). ## 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 - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
192 lines
7.2 KiB
JavaScript
192 lines
7.2 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import { execFileSync, spawnSync } from "node:child_process";
|
|
import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import { test } from "node:test";
|
|
import { fileURLToPath } from "node:url";
|
|
import {
|
|
checkCoverage,
|
|
findUncheckedTests,
|
|
PROGRAMS,
|
|
PROVIDER_WORKSPACE,
|
|
RESOLVE_ARGS,
|
|
SANDBOX_PROVIDERS,
|
|
typecheckTestPrograms,
|
|
} from "./typecheck-test-programs.mjs";
|
|
|
|
const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
|
|
const tsc = path.join(root, "node_modules", "typescript", "bin", "tsc");
|
|
|
|
const harness = (overrides = {}) => {
|
|
const calls = { pnpm: [], tsc: [], linked: 0, errors: [] };
|
|
return {
|
|
calls,
|
|
options: {
|
|
root: "/repo",
|
|
programs: ["a/tsconfig.test.json", "b/tsconfig.test.json"],
|
|
pnpm: (args, { cwd }) => {
|
|
calls.pnpm.push({ args, cwd });
|
|
return 0;
|
|
},
|
|
linkPlugins: () => {
|
|
calls.linked += 1;
|
|
},
|
|
run: (args) => {
|
|
calls.tsc.push(args);
|
|
return { status: 0 };
|
|
},
|
|
checkFiles: () => [],
|
|
log: () => {},
|
|
logError: (message) => calls.errors.push(message),
|
|
...overrides,
|
|
},
|
|
};
|
|
};
|
|
|
|
test("findUncheckedTests reports only test files outside programs and other checks", () => {
|
|
assert.deepEqual(
|
|
findUncheckedTests({
|
|
trackedFiles: ["a/x.test.ts", "a/y.spec.tsx", "a/z.test.tsx", "a/helper.ts", "a/n.test.mjs", "server/src/s.test.ts"],
|
|
checkedFiles: ["a/x.test.ts", "a/y.spec.tsx"],
|
|
checkedElsewhere: ["server/src/"],
|
|
}),
|
|
["a/z.test.tsx"],
|
|
);
|
|
});
|
|
|
|
test("every program exists, and the provider list matches the tracked provider packages", () => {
|
|
for (const program of PROGRAMS) assert.ok(existsSync(path.join(root, program)), program);
|
|
assert.ok(existsSync(path.join(root, PROVIDER_WORKSPACE, "pnpm-lock.yaml")));
|
|
const tracked = execFileSync("git", ["ls-files", "packages/plugins/sandbox-providers"], { cwd: root, encoding: "utf8" });
|
|
const manifests = tracked.split("\n").filter((file) => file.endsWith("/package.json"));
|
|
assert.deepEqual(manifests.map((manifest) => path.dirname(manifest)).sort(), [...SANDBOX_PROVIDERS].sort());
|
|
});
|
|
|
|
test("no provider commits a lockfile, except the one Daytona already has", () => {
|
|
const tracked = execFileSync("git", ["ls-files", "packages/plugins/sandbox-providers"], { cwd: root, encoding: "utf8" });
|
|
assert.deepEqual(
|
|
tracked.split("\n").filter((file) => file.endsWith("/pnpm-lock.yaml")),
|
|
["packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml"],
|
|
);
|
|
});
|
|
|
|
test("a type error in one program still checks the rest and fails the run", () => {
|
|
const { options, calls } = harness({
|
|
run: (args) => {
|
|
calls.tsc.push(args);
|
|
return { status: args[1].startsWith("a/") ? 2 : 0 };
|
|
},
|
|
});
|
|
assert.equal(typecheckTestPrograms(options), 2);
|
|
assert.deepEqual(calls.tsc, [
|
|
["-p", "a/tsconfig.test.json"],
|
|
["-p", "b/tsconfig.test.json"],
|
|
]);
|
|
});
|
|
|
|
test("a killed compiler stops the run", () => {
|
|
const { options, calls } = harness({
|
|
run: (args) => {
|
|
calls.tsc.push(args);
|
|
return { status: null, signal: "SIGKILL" };
|
|
},
|
|
});
|
|
assert.equal(typecheckTestPrograms(options), 1);
|
|
assert.equal(calls.tsc.length, 1);
|
|
});
|
|
|
|
test("providers install frozen without scripts, then relink the SDK", () => {
|
|
const { options, calls } = harness();
|
|
assert.equal(typecheckTestPrograms(options), 0);
|
|
const install = calls.pnpm.find(({ cwd }) => cwd === path.join("/repo", PROVIDER_WORKSPACE));
|
|
assert.ok(install.args.includes("--frozen-lockfile"));
|
|
assert.ok(install.args.includes("--ignore-scripts"));
|
|
assert.equal(calls.linked, 1);
|
|
});
|
|
|
|
test("a stale provider lockfile is resolved in place, then installed frozen", () => {
|
|
const workspace = path.join("/repo", PROVIDER_WORKSPACE);
|
|
let frozenAttempts = 0;
|
|
const { options, calls } = harness({
|
|
pnpm: (args, { cwd }) => {
|
|
calls.pnpm.push({ args, cwd });
|
|
if (cwd !== workspace || !args.includes("--frozen-lockfile")) return 0;
|
|
frozenAttempts += 1;
|
|
return frozenAttempts === 1 ? 1 : 0;
|
|
},
|
|
});
|
|
assert.equal(typecheckTestPrograms(options), 0);
|
|
assert.deepEqual(
|
|
calls.pnpm.filter(({ cwd }) => cwd === workspace).map(({ args }) => args.includes("--frozen-lockfile") ? "frozen" : args),
|
|
["frozen", RESOLVE_ARGS, "frozen"],
|
|
);
|
|
assert.ok(RESOLVE_ARGS.includes("--resolution-only") && RESOLVE_ARGS.includes("--ignore-scripts"));
|
|
assert.equal(calls.tsc.length, 2);
|
|
});
|
|
|
|
test("a provider install that still fails stops before any program runs", () => {
|
|
const { options, calls } = harness({
|
|
pnpm: (args, { cwd }) => (cwd === path.join("/repo", PROVIDER_WORKSPACE) ? 1 : 0),
|
|
});
|
|
assert.equal(typecheckTestPrograms(options), 1);
|
|
assert.equal(calls.tsc.length, 0);
|
|
assert.match(calls.errors[0], /could not install the sandbox providers/);
|
|
});
|
|
|
|
test("an uncovered test file fails the run and is named", () => {
|
|
const { options, calls } = harness({ checkFiles: () => ["a/z.test.tsx"] });
|
|
assert.equal(typecheckTestPrograms(options), 1);
|
|
assert.match(calls.errors[0], /a\/z\.test\.tsx/);
|
|
});
|
|
|
|
test("the coverage check fails closed when a program cannot be listed", () => {
|
|
const { options } = harness({ checkFiles: () => null });
|
|
assert.equal(typecheckTestPrograms(options), 1);
|
|
});
|
|
|
|
test("naming programs skips the coverage check", () => {
|
|
let checked = false;
|
|
const { options } = harness({
|
|
coverage: false,
|
|
checkFiles: () => {
|
|
checked = true;
|
|
return [];
|
|
},
|
|
});
|
|
assert.equal(typecheckTestPrograms(options), 0);
|
|
assert.equal(checked, false);
|
|
});
|
|
|
|
test("checkCoverage uses the compiler's file list, so a dropped sibling is caught", () => {
|
|
const directory = mkdtempSync(path.join(root, "node_modules", ".typecheck-test-programs-"));
|
|
try {
|
|
const relative = path.relative(root, directory).split(path.sep).join("/");
|
|
writeFileSync(path.join(directory, "foo.test.ts"), "export {};\n");
|
|
writeFileSync(path.join(directory, "foo.test.tsx"), "export {};\n");
|
|
writeFileSync(path.join(directory, "tsconfig.json"), JSON.stringify({ compilerOptions: { noEmit: true, jsx: "preserve" }, include: ["*"] }));
|
|
const unchecked = checkCoverage({
|
|
root,
|
|
programs: [`${relative}/tsconfig.json`],
|
|
workspaceConfigs: () => [],
|
|
trackedFiles: () => [`${relative}/foo.test.ts`, `${relative}/foo.test.tsx`],
|
|
});
|
|
assert.deepEqual(unchecked, [`${relative}/foo.test.tsx`]);
|
|
} finally {
|
|
rmSync(directory, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
test("a real type error in a test-style program makes tsc exit non-zero", () => {
|
|
const directory = mkdtempSync(path.join(tmpdir(), "typecheck-test-programs-"));
|
|
try {
|
|
writeFileSync(path.join(directory, "bad.test.ts"), "export const value: number = 'text';\n");
|
|
writeFileSync(path.join(directory, "tsconfig.json"), JSON.stringify({ compilerOptions: { noEmit: true, types: [] }, include: ["*.ts"] }));
|
|
const { status, stdout } = spawnSync(process.execPath, [tsc, "-p", directory, "--pretty", "false"], { encoding: "utf8" });
|
|
assert.notEqual(status, 0);
|
|
assert.match(stdout, /bad\.test\.ts.*TS2322/);
|
|
} finally {
|
|
rmSync(directory, { recursive: true, force: true });
|
|
}
|
|
});
|