mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +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>
171 lines
9.9 KiB
JavaScript
171 lines
9.9 KiB
JavaScript
import test from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import { execFileSync, spawnSync } from "node:child_process";
|
|
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import { LOCKFILES } from "../lockfile-trust.mjs";
|
|
|
|
const workflow = readFileSync(new URL("../../workflows/refresh-lockfile.yml", import.meta.url), "utf8");
|
|
const stepNames = [...workflow.matchAll(/^ - name: (.+)$/gm)].map((match) => match[1]);
|
|
const step = (name) => {
|
|
const body = workflow.split(` - name: ${name}\n`)[1]?.split(" - name:")[0];
|
|
assert.ok(body, `the refresh workflow must have a "${name}" step`);
|
|
return body;
|
|
};
|
|
const condition = (name) => step(name).match(/^ if: (.+)$/m)?.[1].trim();
|
|
|
|
test("the bot key is only reachable from the master-only lockfile-refresh environment", () => {
|
|
assert.match(workflow, /^ environment: lockfile-refresh$/m);
|
|
const keyUses = workflow.match(/secrets\.LOCKFILE_BOT_PRIVATE_KEY/g) ?? [];
|
|
assert.equal(keyUses.length, 1, "only the token step may read the bot key");
|
|
assert.match(step("Create lockfile bot token"), /private-key: \$\{\{ secrets\.LOCKFILE_BOT_PRIVATE_KEY \}\}/);
|
|
});
|
|
|
|
test("the bot token is minted only after resolution and the release-age gate", () => {
|
|
const order = (name) => {
|
|
const index = stepNames.indexOf(name);
|
|
assert.notEqual(index, -1, `missing step "${name}"`);
|
|
return index;
|
|
};
|
|
assert.ok(order("Refresh pnpm lockfile") < order("Check release age of added package versions"));
|
|
assert.ok(order("Check release age of added package versions") < order("Create lockfile bot token"));
|
|
assert.ok(order("Create lockfile bot token") < order("Merge lockfile PR"));
|
|
});
|
|
|
|
test("the gate runs only for an open PR, and the bot acts only when the gate passes", () => {
|
|
assert.equal(condition("Check release age of added package versions"), "steps.upsert-pr.outputs.pr_url != ''");
|
|
assert.equal(condition("Hold lockfile PR for review"), "steps.release-age.outputs.eligible == 'false'");
|
|
assert.equal(condition("Create lockfile bot token"), "steps.release-age.outputs.eligible == 'true'");
|
|
assert.equal(condition("Merge lockfile PR"), "steps.release-age.outputs.eligible == 'true'");
|
|
});
|
|
|
|
test("a held refresh keeps its versions through the next master run's artifact", () => {
|
|
assert.ok(stepNames.indexOf("Reuse versions from a held refresh") < stepNames.indexOf("Refresh pnpm lockfile"));
|
|
const reuse = step("Reuse versions from a held refresh");
|
|
assert.match(reuse, /gh run list --workflow refresh-lockfile\.yml --branch master --status success/);
|
|
assert.match(reuse, /gh run download "\$run_id" --name held-pnpm-lock/);
|
|
assert.match(
|
|
reuse,
|
|
/if ! sha256sum --check --status "\$held\/source-locks\.sha256"; then[\s\S]*?exit 0\n fi\n for lock in \$LOCKFILES; do\n cp "\$held\/\$lock" "\$lock"/,
|
|
"held locks must only be reused while master still has every lock they were resolved from",
|
|
);
|
|
assert.match(reuse, /if \[ ! -f "\$held\/source-locks\.sha256" \]; then[\s\S]*?exit 0/, "an older held artifact must not be reused");
|
|
assert.doesNotMatch(reuse, /chore\/refresh-lockfile/, "the PR branch is writable by others and must not seed resolution");
|
|
|
|
for (const name of ["Stage held lockfile", "Save held lockfile for the next refresh"]) {
|
|
assert.equal(condition(name), "steps.release-age.outputs.eligible == 'false'");
|
|
}
|
|
const stage = step("Stage held lockfile");
|
|
assert.match(stage, /for lock in \$LOCKFILES; do[\s\S]*?cp "\$lock" "\$held\/\$lock"/);
|
|
assert.match(stage, /\(cd "\$RUNNER_TEMP\/base-locks" && sha256sum \$LOCKFILES\) > "\$held\/source-locks\.sha256"/);
|
|
const save = step("Save held lockfile for the next refresh");
|
|
assert.match(save, /name: held-pnpm-lock/);
|
|
assert.match(save, /path: \$\{\{ runner\.temp \}\}\/held-upload\//);
|
|
});
|
|
|
|
test("the bot merges only the commit this job pushed", () => {
|
|
assert.match(step("Create or update pull request"), /echo "head_sha=\$\(git rev-parse HEAD\)" >> "\$GITHUB_OUTPUT"/);
|
|
const merge = step("Merge lockfile PR");
|
|
assert.match(merge, /HEAD_SHA: \$\{\{ steps\.upsert-pr\.outputs\.head_sha \}\}/);
|
|
assert.match(merge, /GH_TOKEN: \$\{\{ steps\.bot-token\.outputs\.token \}\}/);
|
|
assert.match(merge, /gh pr merge "\$PR_URL" .*--match-head-commit "\$HEAD_SHA"/);
|
|
});
|
|
|
|
test("a refresh that renews only the reporting-lock proof still opens a pull request", () => {
|
|
const upsert = step("Create or update pull request");
|
|
const check = upsert.match(/^ if (git diff --quiet -- [^;]+); then$/m)?.[1];
|
|
assert.ok(check, "the PR step must check for changes with git diff --quiet");
|
|
const added = upsert.match(/^ git add (.+)$/m)?.[1].split(" ");
|
|
assert.deepEqual(check.replace("git diff --quiet -- ", "").split(" "), added, "the change check must cover every file the PR step commits");
|
|
|
|
const root = mkdtempSync(path.join(tmpdir(), "refresh-proof-only-"));
|
|
try {
|
|
const git = (...args) => execFileSync("git", args, { cwd: root });
|
|
execFileSync("mkdir", ["-p", path.join(root, "tests/runner-e2e")]);
|
|
const files = { "pnpm-lock.yaml": "lock\n", "tests/runner-e2e/report-lock.json": "{}\n", "tests/runner-e2e/report-lock.patch": "old\n" };
|
|
for (const [file, contents] of Object.entries(files)) writeFileSync(path.join(root, file), contents);
|
|
git("init", "-q");
|
|
git("add", ".");
|
|
git("-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid", "commit", "-qm", "fixture");
|
|
const unchanged = () => spawnSync("bash", ["-c", check], { cwd: root }).status === 0;
|
|
assert.equal(unchanged(), true);
|
|
writeFileSync(path.join(root, "tests/runner-e2e/report-lock.json"), '{"outputSha256":"new"}\n');
|
|
writeFileSync(path.join(root, "tests/runner-e2e/report-lock.patch"), "");
|
|
assert.equal(unchanged(), false, "a proof-only change must not exit as unchanged");
|
|
} finally {
|
|
rmSync(root, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
test("the workflow owns the same lockfiles that the trust policy guards", () => {
|
|
const env = workflow.match(/^ LOCKFILES: (.+)$/m)?.[1];
|
|
assert.ok(env, "the refresh job must define LOCKFILES");
|
|
assert.deepEqual(env.trim().split(/\s+/), LOCKFILES);
|
|
assert.match(step("Refresh sandbox provider lockfiles"),
|
|
/pnpm --dir scripts\/typecheck-sandbox-providers install --resolution-only --ignore-scripts --no-frozen-lockfile/);
|
|
assert.match(step("Refresh sandbox provider lockfiles"),
|
|
/pnpm --dir packages\/plugins\/sandbox-providers\/daytona install --ignore-workspace --resolution-only --ignore-scripts --no-frozen-lockfile/);
|
|
assert.ok(stepNames.indexOf("Refresh sandbox provider lockfiles") < stepNames.indexOf("Fail on unexpected file changes"));
|
|
assert.match(step("Check release age of added package versions"), /for lock in \$LOCKFILES; do[\s\S]*?pairs\+=\(--base "\$base\/\$lock" --head "\$lock"\)/);
|
|
});
|
|
|
|
// Run a workflow step's script with bash, as GitHub would.
|
|
const runStep = (name, cwd, env) => {
|
|
const lines = step(name).split("\n");
|
|
const start = lines.findIndex((line) => line === " run: |") + 1;
|
|
const script = lines.slice(start).map((line) => line.slice(10)).join("\n");
|
|
return spawnSync("bash", ["-e", "-c", script], { cwd, encoding: "utf8", env: { ...process.env, LOCKFILES: LOCKFILES.join(" "), ...env } });
|
|
};
|
|
|
|
test("unexpected-change check admits every managed lockfile and nothing else", () => {
|
|
const root = mkdtempSync(path.join(tmpdir(), "refresh-unexpected-"));
|
|
try {
|
|
const git = (...args) => execFileSync("git", args, { cwd: root });
|
|
for (const file of [...LOCKFILES, "notes.md"]) {
|
|
mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
|
writeFileSync(path.join(root, file), "old\n");
|
|
}
|
|
git("init", "-q");
|
|
git("add", ".");
|
|
git("-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid", "commit", "-qm", "fixture");
|
|
for (const file of LOCKFILES) writeFileSync(path.join(root, file), "new\n");
|
|
const allowed = runStep("Fail on unexpected file changes", root, {});
|
|
assert.equal(allowed.status, 0, allowed.stdout + allowed.stderr);
|
|
writeFileSync(path.join(root, "notes.md"), "new\n");
|
|
const blocked = runStep("Fail on unexpected file changes", root, {});
|
|
assert.equal(blocked.status, 1, blocked.stdout + blocked.stderr);
|
|
assert.match(blocked.stdout, /notes\.md/);
|
|
} finally {
|
|
rmSync(root, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
test("a held refresh round-trips every managed lockfile", () => {
|
|
const root = mkdtempSync(path.join(tmpdir(), "refresh-held-"));
|
|
const temp = mkdtempSync(path.join(tmpdir(), "refresh-held-temp-"));
|
|
try {
|
|
for (const file of LOCKFILES) {
|
|
for (const dir of [root, path.join(temp, "base-locks")]) {
|
|
mkdirSync(path.dirname(path.join(dir, file)), { recursive: true });
|
|
writeFileSync(path.join(dir, file), `master ${file}\n`);
|
|
}
|
|
writeFileSync(path.join(root, file), `held ${file}\n`);
|
|
}
|
|
const staged = runStep("Stage held lockfile", root, { RUNNER_TEMP: temp });
|
|
assert.equal(staged.status, 0, staged.stderr);
|
|
const held = path.join(temp, "held-upload");
|
|
for (const file of LOCKFILES) assert.equal(readFileSync(path.join(held, file), "utf8"), `held ${file}\n`);
|
|
|
|
// The next run checks the hashes against master's lockfiles, then copies the held ones in.
|
|
for (const file of LOCKFILES) writeFileSync(path.join(root, file), `master ${file}\n`);
|
|
const check = `sha256sum --check --status "${held}/source-locks.sha256"`;
|
|
assert.equal(spawnSync("bash", ["-c", check], { cwd: root }).status, 0, "unchanged master locks must match the held source hashes");
|
|
writeFileSync(path.join(root, LOCKFILES[1]), "changed on master\n");
|
|
assert.notEqual(spawnSync("bash", ["-c", check], { cwd: root }).status, 0, "a changed provider lock must invalidate the held refresh");
|
|
} finally {
|
|
rmSync(root, { recursive: true, force: true });
|
|
rmSync(temp, { recursive: true, force: true });
|
|
}
|
|
});
|