From 7d194e901fd2cd1b3937172de3acf20d81af3dd5 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Thu, 29 Oct 2026 17:55:02 -0700 Subject: [PATCH] feat(ci): merge lockfile refresh PRs through a dedicated bot (#18238) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - CI owns `pnpm-lock.yaml`. Contributors do not commit it, so a poisoned lockfile cannot come in from a community PR. > - `refresh-lockfile.yml` regenerates the lockfile from the manifests on master and opens a PR. > - That PR waits on required checks and code owner review. A `GITHUB_TOKEN` push does not start the checks, so a maintainer must merge each PR by hand. > - The PR content comes only from manifests that maintainers already reviewed on master. A human review adds little. > - This pull request lets a dedicated GitHub App merge the refresh PR at once, without CI or review. > - A release-age gate holds the merge when the refresh adds a package version that is less than 3 days old on npm. > - The benefit is a lockfile loop that closes itself, without a bypass for any identity that other workflows can use. ## Linked Issues or Issue Description Refs #12115 (the same stuck-PR problem, fixed there by starting the checks). Refs #18219 (an example refresh PR that needed a manual approval and merge). **What happened?** Each lockfile refresh PR stays open until a code owner approves it and a maintainer merges it. The required checks `ci / verify` and `ci / e2e` also do not start, because the workflow pushes with `GITHUB_TOKEN`. **Expected behavior** The refresh PR merges with no human action and no CI run, unless it adds a package version that is too new to trust without review. **Steps to reproduce** Merge a manifest change to master. Look at the `chore/refresh-lockfile` PR that the workflow opens. It stays blocked. ## What Changed - `refresh-lockfile.yml` runs in a new `lockfile-refresh` environment. Only `master` can use this environment. - The workflow creates a token for a dedicated lockfile bot app after dependency resolution is complete. The token has only contents and pull request write access. - The workflow merges with `gh pr merge --squash --admin --match-head-commit `, not `--auto`. It merges only the commit that it pushed. - New `.github/scripts/check-lockfile-release-age.mjs` compares the base and refreshed lockfiles. It looks up the npm publish time of each added package version. The bot merges only when every version is at least 3 days old. A lookup error, a missing publish time, a non-registry version, or a resolution that is not a plain registry tarball also holds the merge. - When the gate holds the PR, the workflow posts or updates one report comment on the PR and does not merge. - A daily schedule runs the refresh again, so a held PR merges after its versions are old enough. - A held run uploads its lockfile as a `held-pnpm-lock` artifact. The artifact records the hash of the master lockfile it started from. The next master run resolves from it only while master still has that lockfile, so held versions stay while their manifests do not change, and a maintainer's lockfile correction replaces them. The workflow does not read the PR branch for this, because users with write access can push to it. - The generated PR body now describes the gate. - `tests/runner-e2e/OPERATIONS.md` now says that the bot can merge the report-lock correction without review, under the same gate. - New `lockfile-refresh-merge.test.mjs` pins the merge safeguards: the environment, the step order, the gate conditions, and `--match-head-commit`. - `doc/RELEASE-AUTOMATION-SETUP.md` §8.1 tells an admin how to create the app, the environment, and the ruleset bypass. It also tells why we do not use `github-actions` or `commitperclip` for the bypass. ## Verification - `node --test .github/scripts/tests/check-lockfile-release-age.test.mjs .github/scripts/tests/lockfile-refresh-*.test.mjs` passes (17/17). - I ran the gate against the real lockfile diff of #18219 and the live npm registry. With 3 days, all 7 added versions passed (`eligible=true`). With 60 days, it held all 7 `turbo` 2.11.10 versions (9.1 days old) and wrote the report. - The workflow YAML parses. actionlint did not run, because it is not installed locally and Docker was not available. - No live run yet. After the admin setup in §8.1, run the workflow with `workflow_dispatch` on master, or merge a manifest change. Then make sure that the bot opens and merges the refresh PR. ## Risks - **Setup order.** An org admin must do the steps in §8.1 before this PR merges. If not, the next refresh fails at the token step, and the PR stays open for a manual merge, as it does now. - **No CI on lockfile changes.** A bad resolution can land on master without a check. The next manifest PR or the canary release shows the problem. - **Release-age gate limits.** The gate stops only fast-detected attacks on new versions. A malicious version that stays undetected for more than 3 days still merges without review. Lockfile changes that add very new versions now wait for a maintainer or up to 3 days. - **Bypass scope.** The app bypasses the master ruleset only in pull request mode. It cannot push to master directly. Only the master-only `lockfile-refresh` environment holds its key. If the key leaks, the holder can merge any open PR past review. Rotate the key if you think it leaked. - **More master runs.** A merge by an app token starts push workflows on master. A merge by `GITHUB_TOKEN` does not. After each lockfile merge, the canary release runs, and the refresh workflow runs again with no change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use (shell, file edit, GitHub CLI). ## 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 --- .../scripts/check-lockfile-release-age.mjs | 160 ++++++++++++++++++ .../tests/check-lockfile-release-age.test.mjs | 152 +++++++++++++++++ .../tests/lockfile-refresh-merge.test.mjs | 68 ++++++++ .github/workflows/refresh-lockfile.yml | 117 ++++++++++++- doc/RELEASE-AUTOMATION-SETUP.md | 21 +++ tests/runner-e2e/OPERATIONS.md | 8 +- 6 files changed, 518 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/check-lockfile-release-age.mjs create mode 100644 .github/scripts/tests/check-lockfile-release-age.test.mjs create mode 100644 .github/scripts/tests/lockfile-refresh-merge.test.mjs diff --git a/.github/scripts/check-lockfile-release-age.mjs b/.github/scripts/check-lockfile-release-age.mjs new file mode 100644 index 00000000000..f5338250687 --- /dev/null +++ b/.github/scripts/check-lockfile-release-age.mjs @@ -0,0 +1,160 @@ +#!/usr/bin/env node +/** + * check-lockfile-release-age.mjs + * Decides whether the lockfile bot may merge a refreshed pnpm-lock.yaml without review. + * Every package version that the refresh adds must be published on npm for at least + * the minimum age. A version that is too new, or that cannot be verified, holds the + * PR for a human. + * + * Exports: parseLockfilePackages(text), addedPackageVersions(baseText, headText), + * checkReleaseAge(entries, { fetchPublishTime, now, minAgeMs, concurrency }) + * + * CLI: node check-lockfile-release-age.mjs --base --head --min-age-days + * Writes `eligible=true|false` to GITHUB_OUTPUT and a Markdown report to --report. + */ +import { appendFileSync, readFileSync, writeFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { parseArgs } from 'node:util'; + +const SEMVER = /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/; +// Every 2-space-indented line in `packages:` is an entry key. Keys can contain +// colons (URL and git dependencies), so match up to the final colon. +const PACKAGE_KEY = /^ {2}(\S.*):\s*$/; +const RESOLUTION = /^ {4}resolution: (.+?)\s*$/; +// A registry tarball is locked by integrity alone. A tarball URL, git repo or +// directory resolution names its own source and needs a human. +const REGISTRY_RESOLUTION = /^\{integrity: [^,{}]+\}$/; +const DAY_MS = 24 * 60 * 60 * 1000; + +/** Maps each key of the lockfile's top-level `packages:` section to its resolution. */ +export function parseLockfilePackages(text) { + const packages = new Map(); + let inPackages = false; + let key = null; + for (const line of text.split('\n')) { + if (/^\S/.test(line)) { + inPackages = line.trimEnd() === 'packages:'; + key = null; + continue; + } + if (!inPackages) continue; + const keyMatch = PACKAGE_KEY.exec(line); + if (keyMatch) { + key = keyMatch[1].replace(/^'(.*)'$/, '$1').replace(/\(.*$/, ''); + packages.set(key, ''); + continue; + } + const resolutionMatch = key && RESOLUTION.exec(line); + if (resolutionMatch) packages.set(key, resolutionMatch[1]); + } + return packages; +} + +export function splitPackageKey(key) { + const at = key.lastIndexOf('@'); + if (at <= 0) return { name: key, version: '' }; + return { name: key.slice(0, at), version: key.slice(at + 1) }; +} + +/** Package entries in headText that baseText lacks or resolves differently. */ +export function addedPackageVersions(baseText, headText) { + const base = parseLockfilePackages(baseText); + return [...parseLockfilePackages(headText)] + .filter(([key, resolution]) => base.get(key) !== resolution) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, resolution]) => ({ key, ...splitPackageKey(key), resolution })); +} + +export async function fetchNpmPublishTime(name, version, fetchImpl = fetch) { + const res = await fetchImpl(`https://registry.npmjs.org/${name.replace('/', '%2f')}`, { + headers: { Accept: 'application/json' }, + signal: AbortSignal.timeout(30_000), + }); + if (!res.ok) throw new Error(`npm registry returned ${res.status}`); + const doc = await res.json(); + const published = doc.time?.[version]; + return published ? new Date(published) : null; +} + +/** + * Checks each added version. Fails closed: a lookup error, a missing publish time or a + * non-registry version holds the merge. + */ +export async function checkReleaseAge(entries, { + fetchPublishTime = fetchNpmPublishTime, + now = new Date(), + minAgeMs, + concurrency = 8, +}) { + const held = []; + const queue = [...entries]; + const worker = async () => { + for (let entry = queue.shift(); entry; entry = queue.shift()) { + const reason = await holdReason(entry, fetchPublishTime, now, minAgeMs); + if (reason) held.push({ ...entry, reason }); + } + }; + await Promise.all(Array.from({ length: concurrency }, worker)); + held.sort((a, b) => a.key.localeCompare(b.key)); + return { eligible: held.length === 0, checked: entries.length, held }; +} + +async function holdReason({ name, version, resolution }, fetchPublishTime, now, minAgeMs) { + if (!SEMVER.test(version)) return 'not a registry version'; + if (!REGISTRY_RESOLUTION.test(resolution ?? '')) return `not a registry resolution: ${resolution || 'missing'}`; + let published; + try { + published = await fetchPublishTime(name, version); + } catch (error) { + return `registry lookup failed: ${error.message}`; + } + if (!published || Number.isNaN(published.getTime())) return 'no publish time on npm'; + const ageMs = now.getTime() - published.getTime(); + if (ageMs < minAgeMs) { + return `published ${published.toISOString()} (${(ageMs / DAY_MS).toFixed(1)} days ago)`; + } + return null; +} + +export function formatReport(result, minAgeDays) { + const lines = ['']; + if (result.eligible) { + lines.push(`All ${result.checked} added package versions are at least ${minAgeDays} days old. The lockfile bot merges this PR.`); + } else { + lines.push( + `The lockfile bot did not merge this PR. These added package versions are newer than ${minAgeDays} days or could not be verified:`, + '', + ...result.held.map(({ key, reason }) => `- \`${key}\`: ${reason}`), + '', + 'A maintainer can review and merge this PR. If no one does, the daily refresh tries again and merges it when every version is old enough.', + ); + } + return `${lines.join('\n')}\n`; +} + +async function main() { + const { values } = parseArgs({ + options: { + base: { type: 'string' }, + head: { type: 'string' }, + 'min-age-days': { type: 'string' }, + report: { type: 'string' }, + }, + }); + const minAgeDays = Number(values['min-age-days']); + if (!values.base || !values.head || !Number.isFinite(minAgeDays) || minAgeDays < 0) { + console.error('Usage: check-lockfile-release-age.mjs --base --head --min-age-days [--report ]'); + process.exit(2); + } + + const entries = addedPackageVersions(readFileSync(values.base, 'utf8'), readFileSync(values.head, 'utf8')); + const result = await checkReleaseAge(entries, { minAgeMs: minAgeDays * DAY_MS }); + const report = formatReport(result, minAgeDays); + console.log(report); + if (values.report) writeFileSync(values.report, report); + if (process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, `eligible=${result.eligible}\n`); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main().catch(e => { console.error(e.message); process.exit(1); }); +} diff --git a/.github/scripts/tests/check-lockfile-release-age.test.mjs b/.github/scripts/tests/check-lockfile-release-age.test.mjs new file mode 100644 index 00000000000..a5cdaebc607 --- /dev/null +++ b/.github/scripts/tests/check-lockfile-release-age.test.mjs @@ -0,0 +1,152 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + addedPackageVersions, + checkReleaseAge, + formatReport, + parseLockfilePackages, +} from '../check-lockfile-release-age.mjs'; + +const DAY_MS = 24 * 60 * 60 * 1000; +const now = new Date('2026-10-30T00:00:00Z'); +const daysAgo = days => new Date(now.getTime() - days * DAY_MS); + +const REGISTRY = '{integrity: sha512-x}'; + +// Each entry is a key, or [key, resolution] for a non-registry resolution. +const lockfile = entries => [ + "lockfileVersion: '9.0'", + '', + 'importers:', + '', + ' .:', + ' dependencies:', + ' left-pad:', + ' specifier: ^1.3.0', + ' version: 1.3.0', + '', + 'packages:', + '', + ...entries.flatMap(entry => { + const [key, resolution] = Array.isArray(entry) ? entry : [entry, REGISTRY]; + return [` ${key}:`, ` resolution: ${resolution}`, ' engines: {node: \'>=18\'}', '']; + }), + 'snapshots:', + '', + ' snapshot-only@9.9.9:', + ' dependencies: {}', + '', +].join('\n'); + +const timesFrom = table => async (name, version) => { + const value = table[`${name}@${version}`]; + if (value instanceof Error) throw value; + return value ?? null; +}; + +test('parses plain, quoted and scoped package keys from the packages section only', () => { + const packages = parseLockfilePackages(lockfile([ + "'@scope/pkg@2.0.0'", + 'left-pad@1.3.0', + ['gitdep@https://codeload.github.com/o/r/tar.gz/abc', '{tarball: https://codeload.github.com/o/r/tar.gz/abc}'], + ])); + assert.deepEqual([...packages].sort(), [ + ['@scope/pkg@2.0.0', REGISTRY], + ['gitdep@https://codeload.github.com/o/r/tar.gz/abc', '{tarball: https://codeload.github.com/o/r/tar.gz/abc}'], + ['left-pad@1.3.0', REGISTRY], + ]); +}); + +test('lists only package versions the refresh adds', () => { + const base = lockfile(['left-pad@1.3.0', 'old@1.0.0']); + const head = lockfile(['left-pad@1.3.0', 'old@1.1.0', "'@scope/new@0.1.0'"]); + assert.deepEqual(addedPackageVersions(base, head), [ + { key: '@scope/new@0.1.0', name: '@scope/new', version: '0.1.0', resolution: REGISTRY }, + { key: 'old@1.1.0', name: 'old', version: '1.1.0', resolution: REGISTRY }, + ]); +}); + +test('lists an existing version whose resolution changed', () => { + const swapped = ['left-pad@1.3.0', '{tarball: https://example.com/left-pad-1.3.0.tgz}']; + assert.deepEqual(addedPackageVersions(lockfile(['left-pad@1.3.0']), lockfile([swapped])), [ + { key: 'left-pad@1.3.0', name: 'left-pad', version: '1.3.0', resolution: swapped[1] }, + ]); +}); + +test('holds unquoted URL keys and registry-looking keys with non-registry resolutions', async () => { + const entries = addedPackageVersions(lockfile([]), lockfile([ + ['gitdep@https://codeload.github.com/o/r/tar.gz/abc', '{tarball: https://codeload.github.com/o/r/tar.gz/abc}'], + ['sneaky@1.0.0', '{integrity: sha512-x, tarball: https://example.com/sneaky.tgz}'], + ])); + const result = await checkReleaseAge(entries, { + fetchPublishTime: timesFrom({ 'sneaky@1.0.0': daysAgo(400) }), + now, + minAgeMs: DAY_MS, + }); + assert.equal(result.eligible, false); + assert.equal(result.checked, 2); + assert.deepEqual( + Object.fromEntries(result.held.map(h => [h.name, h.reason])), + { + gitdep: 'not a registry version', + sneaky: 'not a registry resolution: {integrity: sha512-x, tarball: https://example.com/sneaky.tgz}', + }, + ); +}); + +test('is eligible when every added version is old enough', async () => { + const entries = addedPackageVersions(lockfile([]), lockfile(['a@1.0.0', "'@s/b@2.0.0'"])); + const result = await checkReleaseAge(entries, { + fetchPublishTime: timesFrom({ 'a@1.0.0': daysAgo(30), '@s/b@2.0.0': daysAgo(3.5) }), + now, + minAgeMs: 3 * DAY_MS, + }); + assert.deepEqual(result, { eligible: true, checked: 2, held: [] }); +}); + +test('is eligible when the refresh adds no package versions', async () => { + const result = await checkReleaseAge([], { fetchPublishTime: timesFrom({}), now, minAgeMs: DAY_MS }); + assert.equal(result.eligible, true); +}); + +test('holds a version newer than the minimum age', async () => { + const entries = addedPackageVersions(lockfile([]), lockfile(['old@1.0.0', 'fresh@1.0.1'])); + const result = await checkReleaseAge(entries, { + fetchPublishTime: timesFrom({ 'old@1.0.0': daysAgo(30), 'fresh@1.0.1': daysAgo(0.5) }), + now, + minAgeMs: 3 * DAY_MS, + }); + assert.equal(result.eligible, false); + assert.deepEqual(result.held.map(h => h.key), ['fresh@1.0.1']); + assert.match(result.held[0].reason, /0\.5 days ago/); +}); + +test('fails closed on lookup errors, missing publish times and non-registry versions', async () => { + const entries = addedPackageVersions( + lockfile([]), + lockfile(['broken@1.0.0', 'unpublished@1.0.0', "'gitdep@https://codeload.github.com/o/r/tar.gz/abc'"]), + ); + const result = await checkReleaseAge(entries, { + fetchPublishTime: timesFrom({ 'broken@1.0.0': new Error('npm registry returned 503') }), + now, + minAgeMs: DAY_MS, + }); + assert.equal(result.eligible, false); + assert.deepEqual( + Object.fromEntries(result.held.map(h => [h.name, h.reason])), + { + broken: 'registry lookup failed: npm registry returned 503', + gitdep: 'not a registry version', + unpublished: 'no publish time on npm', + }, + ); +}); + +test('report lists held versions behind a stable marker', () => { + const report = formatReport( + { eligible: false, checked: 1, held: [{ key: 'fresh@1.0.1', reason: 'too new' }] }, + 3, + ); + assert.match(report, /^/); + assert.match(report, /- `fresh@1\.0\.1`: too new/); +}); diff --git a/.github/scripts/tests/lockfile-refresh-merge.test.mjs b/.github/scripts/tests/lockfile-refresh-merge.test.mjs new file mode 100644 index 00000000000..94a60c167de --- /dev/null +++ b/.github/scripts/tests/lockfile-refresh-merge.test.mjs @@ -0,0 +1,68 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; + +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 < pnpm-lock\.yaml\)" != "\$\(cat "\$held\/source-lock\.sha256"\)" \]; then[\s\S]*?exit 0\n fi\n cp "\$held\/pnpm-lock\.yaml" pnpm-lock\.yaml/, + "a held lock must only be reused while master still has the lock it was resolved from", + ); + 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, /cp pnpm-lock\.yaml "\$RUNNER_TEMP\/held-upload\/pnpm-lock\.yaml"/); + assert.match(stage, /sha256sum < "\$RUNNER_TEMP\/base-pnpm-lock\.yaml" > "\$RUNNER_TEMP\/held-upload\/source-lock\.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"/); +}); diff --git a/.github/workflows/refresh-lockfile.yml b/.github/workflows/refresh-lockfile.yml index 59f47b01531..2f057fa0ae8 100644 --- a/.github/workflows/refresh-lockfile.yml +++ b/.github/workflows/refresh-lockfile.yml @@ -4,6 +4,9 @@ on: push: branches: - master + schedule: + # Retry refreshes held by the release-age gate once their versions are old enough. + - cron: "23 6 * * *" workflow_dispatch: concurrency: @@ -14,7 +17,11 @@ jobs: refresh: runs-on: ubuntu-latest timeout-minutes: 10 + # Holds the lockfile bot key. Restrict this environment to master so no + # other branch or workflow can mint a token that bypasses master rules. + environment: lockfile-refresh permissions: + actions: read contents: write pull-requests: write @@ -36,6 +43,34 @@ jobs: # claim the shared cache key with an empty archive before full installs. package-manager-cache: false + # Start from the lockfile the previous run held, so resolution keeps those + # versions while their manifests are unchanged and they can age past the + # gate. Read it from that run's artifact, not the PR branch: anyone with + # write access can push to chore/refresh-lockfile. + - name: Reuse versions from a held refresh + env: + GH_TOKEN: ${{ github.token }} + run: | + run_id="$( + gh run list --workflow refresh-lockfile.yml --branch master --status success --limit 20 \ + --json databaseId,event \ + --jq '[.[] | select(.event == "push" or .event == "schedule" or .event == "workflow_dispatch")][0].databaseId // empty' + )" + held="$RUNNER_TEMP/held-lock" + if [ -z "$run_id" ] || ! gh run download "$run_id" --name held-pnpm-lock --dir "$held" 2>/dev/null; then + echo "The previous refresh held nothing. Resolving from master." + exit 0 + fi + # Reuse the held lock only while master still has the lock it was + # resolved from. A lock change on master, such as a maintainer's + # correction, supersedes it. + if [ "$(sha256sum < pnpm-lock.yaml)" != "$(cat "$held/source-lock.sha256")" ]; then + echo "The lockfile on master changed after run $run_id held its refresh. Resolving from master." + exit 0 + fi + cp "$held/pnpm-lock.yaml" pnpm-lock.yaml + echo "Resolving from the lockfile held by run $run_id." + - name: Refresh pnpm lockfile run: pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile @@ -65,6 +100,7 @@ jobs: if git diff --quiet -- pnpm-lock.yaml; then echo "Lockfile unchanged, nothing to do." echo "pr_url=" >> "$GITHUB_OUTPUT" + echo "head_sha=" >> "$GITHUB_OUTPUT" exit 0 fi @@ -117,11 +153,11 @@ jobs: - Pinned pnpm resolution completed with lifecycle scripts disabled. - The report proof refresh accepted the reviewed committed baseline. - - Required PR checks and reviews must pass before merge. + - The workflow rejected changes outside the lock and report proof files. ## Risks - Dependency changes require review. Unknown report lock baselines remain blocked. This update changes no permissions or paid enable gates. + The lockfile bot merges this PR without CI or review when every added package version is at least 3 days old on npm. Otherwise the PR waits for a maintainer, and the daily refresh tries again. The bot merges only the exact commit this workflow generated from manifests already reviewed on master. Unknown report lock baselines remain blocked. This update changes no permissions or paid enable gates. ## Model Used @@ -131,7 +167,7 @@ jobs: - [x] Thinking path and problem description included - [x] Generated lock and matching report proof included - - [ ] Required CI checks and reviews pass + - [x] Release-age gate decides between bot merge and maintainer review PR_BODY pr_url="$(gh pr create \ --head "$BRANCH" \ @@ -142,10 +178,81 @@ jobs: echo "PR already exists: $pr_url" fi echo "pr_url=$pr_url" >> "$GITHUB_OUTPUT" + echo "head_sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" - - name: Enable auto-merge for lockfile PR + # Resolution picks the newest matching version for any added or widened + # dependency. Merge without review only when every added version has been + # on npm long enough for a hijacked release to be caught and pulled. + - name: Check release age of added package versions + id: release-age if: steps.upsert-pr.outputs.pr_url != '' + run: | + git show "$GITHUB_SHA:pnpm-lock.yaml" > "$RUNNER_TEMP/base-pnpm-lock.yaml" + node .github/scripts/check-lockfile-release-age.mjs \ + --base "$RUNNER_TEMP/base-pnpm-lock.yaml" \ + --head pnpm-lock.yaml \ + --min-age-days 3 \ + --report "$RUNNER_TEMP/release-age-report.md" + cat "$RUNNER_TEMP/release-age-report.md" >> "$GITHUB_STEP_SUMMARY" + + - name: Hold lockfile PR for review + if: steps.release-age.outputs.eligible == 'false' env: GH_TOKEN: ${{ github.token }} + PR_URL: ${{ steps.upsert-pr.outputs.pr_url }} run: | - gh pr merge --auto --squash --delete-branch "${{ steps.upsert-pr.outputs.pr_url }}" + # Keep one report comment per PR instead of adding one on every run. + pr_number="${PR_URL##*/}" + comment_id="$( + gh api --paginate "repos/$GITHUB_REPOSITORY/issues/$pr_number/comments" \ + --jq '.[] | select(.user.login == "github-actions[bot]" and (.body | startswith(""))) | .id' | + head -n 1 + )" + if [ -n "$comment_id" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$comment_id" \ + -F "body=@$RUNNER_TEMP/release-age-report.md" > /dev/null + else + gh pr comment "$PR_URL" --body-file "$RUNNER_TEMP/release-age-report.md" + fi + echo "::notice title=Lockfile PR held::Added package versions are too new to merge without review. See $PR_URL" + + - name: Stage held lockfile + if: steps.release-age.outputs.eligible == 'false' + run: | + mkdir -p "$RUNNER_TEMP/held-upload" + cp pnpm-lock.yaml "$RUNNER_TEMP/held-upload/pnpm-lock.yaml" + sha256sum < "$RUNNER_TEMP/base-pnpm-lock.yaml" > "$RUNNER_TEMP/held-upload/source-lock.sha256" + + - name: Save held lockfile for the next refresh + if: steps.release-age.outputs.eligible == 'false' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: held-pnpm-lock + path: ${{ runner.temp }}/held-upload/ + if-no-files-found: error + retention-days: 7 + + # Mint the bypass token only after resolution has finished, so no earlier + # step can read it. The lockfile bot is a master ruleset bypass actor in + # pull_request mode: it can merge PRs past checks and review, but cannot + # push to master directly. See doc/RELEASE-AUTOMATION-SETUP.md. + - name: Create lockfile bot token + id: bot-token + if: steps.release-age.outputs.eligible == 'true' + uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3.3.0 + with: + client-id: ${{ vars.LOCKFILE_BOT_CLIENT_ID }} + private-key: ${{ secrets.LOCKFILE_BOT_PRIVATE_KEY }} + permission-contents: write + permission-pull-requests: write + + - name: Merge lockfile PR + if: steps.release-age.outputs.eligible == 'true' + env: + GH_TOKEN: ${{ steps.bot-token.outputs.token }} + PR_URL: ${{ steps.upsert-pr.outputs.pr_url }} + HEAD_SHA: ${{ steps.upsert-pr.outputs.head_sha }} + run: | + # Merge only the commit this job pushed. Anything else pushed to the + # branch since then makes GitHub refuse the merge. + gh pr merge "$PR_URL" --squash --admin --delete-branch --match-head-commit "$HEAD_SHA" diff --git a/doc/RELEASE-AUTOMATION-SETUP.md b/doc/RELEASE-AUTOMATION-SETUP.md index 1c65ae12824..324a6adb07e 100644 --- a/doc/RELEASE-AUTOMATION-SETUP.md +++ b/doc/RELEASE-AUTOMATION-SETUP.md @@ -219,6 +219,27 @@ Current file: If `@cryppadotta` is not the right reviewer identity in the public repo, change it before enabling enforcement. +## 8.1. Let the Lockfile Bot Merge Lockfile Refreshes + +`.github/workflows/refresh-lockfile.yml` regenerates `pnpm-lock.yaml` from the manifests on `master` and merges the result without CI or review. Contributors never commit the lockfile, so the bot is the only path for lockfile changes. + +Resolution keeps locked versions, but it picks the newest matching version for an added or widened dependency. A release-age gate (`.github/scripts/check-lockfile-release-age.mjs`) therefore checks every package version that the refresh adds. The bot merges only when each one has been on npm for at least 3 days. If any version is newer, or the gate cannot verify it, the bot leaves a comment on the PR and does not merge. A maintainer can review and merge it, or the daily scheduled refresh merges it after the versions are old enough. A held run saves its lockfile, and the hash of the `master` lockfile it started from, as an artifact. The next run resolves from that lockfile only while `master` still has the same lockfile. Held versions therefore stay put while their manifests are unchanged, even when newer releases appear, and a lockfile change on `master` replaces them. The gate also holds any added entry whose resolution is not a plain npm registry tarball. Change the age with `--min-age-days` in the workflow. + +The merge uses a dedicated GitHub App. Do not reuse `commitperclip` or bypass the built-in `github-actions` app. Those identities are available to other workflows, including release jobs that run dependency install scripts, so a bypass for them would let any of those workflows land code on `master`. + +1. Create an organization-owned GitHub App, for example `paperclip-lockfile-bot`: + - Disable the webhook. + - Repository permissions: `Contents: Read and write`, `Pull requests: Read and write`. Leave everything else at `No access`. + - Allow installation only on this account. +2. Install the app on `paperclipai/paperclip` only. +3. Create a GitHub environment named `lockfile-refresh`. Under deployment branches and tags, allow only `master`. Add no required reviewers. +4. In that environment, add: + - variable `LOCKFILE_BOT_CLIENT_ID`: the app's client ID + - secret `LOCKFILE_BOT_PRIVATE_KEY`: a private key generated for the app +5. In the `master` ruleset, add the app as a bypass actor with bypass mode `For pull requests only`. The app can then merge PRs past required checks and code owner review, but it cannot push to `master` directly. + +The workflow mints the app token after dependency resolution finishes and merges only the commit it pushed (`--match-head-commit`). A commit anyone else pushes to `chore/refresh-lockfile` makes the merge fail. + ## 9. Protect Release Infrastructure Specifically These files should always trigger code owner review: diff --git a/tests/runner-e2e/OPERATIONS.md b/tests/runner-e2e/OPERATIONS.md index 732ef2ca680..6174437ee69 100644 --- a/tests/runner-e2e/OPERATIONS.md +++ b/tests/runner-e2e/OPERATIONS.md @@ -978,9 +978,11 @@ without registry resolution. The worker target and its lock artifact never control report dependencies. An already-reviewed output lock is used unchanged. An unknown committed lock fails before install; review the correction with dependency changes. The existing lockfile-refresh bot runs `prepare-report-lock.mjs --refresh` and includes the correction and hashes in its -lockfile PR. The PR includes the root lock, which is owned by CODEOWNERS; -the bot requests normal auto-merge and cannot bypass required code-owner review -or checks. It refuses to bless an unrecognized committed lock. Paid authorization +lockfile PR. The lockfile bot merges that PR without checks or code-owner review +when every added package version is at least 3 days old on npm; otherwise the PR +waits for a maintainer (see `doc/RELEASE-AUTOMATION-SETUP.md` §8.1). The bot +merges only the commit the workflow generated. It refuses to bless an +unrecognized committed lock. Paid authorization and report jobs still apply only the reviewed offline correction. The original failed run remains failed when retained evidence gets a new presentation.