fix(ci): regenerate stale stacked lockfiles (#12461)
## Thinking Path > - Paperclip uses trusted GitHub Actions workflows to verify every pull request > - Native stacked pull requests use another pull request branch as their base > - A parent layer can change a package manifest without committing `pnpm-lock.yaml` > - A child layer can inherit that manifest change without changing a manifest itself > - The current policy skips lockfile regeneration for that child and downstream frozen installs fail > - This pull request validates the complete merge tree and shares a regenerated lockfile only when needed > - The benefit is reliable stacked pull request verification without weakening the trusted workflow boundary ## Linked Issues or Issue Description **What happened?** A stacked child pull request inherited a package manifest change from its parent. The child did not change a manifest itself. The policy job skipped lockfile regeneration. Downstream jobs tried to restore an artifact that did not exist and then failed during frozen dependency installation. **Expected behavior** The policy job must validate the complete pull request merge tree. It must upload a regenerated lockfile when the checked-in lockfile is stale, including on a stacked child layer. **Steps to reproduce** 1. Create a parent pull request that changes `package.json` without committing `pnpm-lock.yaml`. 2. Create a child pull request on that branch without another manifest change. 3. Run the trusted pull request workflow for the child. 4. Observe that frozen dependency installation fails because no `pr-lockfile` artifact exists. **Paperclip version or commit** `f173ee09fa5c2ced7806bba47b54c3df853ab4df` **Deployment mode** GitHub Actions trusted pull request workflow. **Agent adapter(s) involved** Not adapter-specific. This is a core CI workflow bug. ## What Changed - Regenerate the lockfile from every checked-out merge tree. - Compare the generated lockfile with the checked-in copy before upload. - Download the artifact only when the policy job reports that it uploaded one. - Fail closed when a reported artifact is missing. - Add a workflow contract test for stacked lockfile handling. - Keep the caller pinned to the last merged trusted SHA; after this implementation merges, a separate activation PR will advance the immutable pin to its merge commit. ## Verification - `actionlint .github/workflows/pr-trusted.yml` - `node --test scripts/__tests__/e2e-shard.test.mjs` ## Risks - The policy job runs one lockfile-only install for every pull request. This can add a small amount of CI time. - A missing artifact now fails immediately when the policy job reports an upload. This is intentional because it exposes workflow corruption. - No runtime or product behavior changes. - The implementation/activation split is intentional: unmerged PR-authored workflow code must never execute on trusted runners. ## Model Used OpenAI Codex with model `gpt-5`, reasoning, tool use, and code execution. ## 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
This commit is contained in:
Dotta
GitHub
parent
e127faa14c
commit
1da6b37fc5
|
|
@ -220,7 +220,7 @@ jobs:
|
|||
policy:
|
||||
needs: [gate]
|
||||
runs-on: ${{ needs.gate.outputs.runner }}
|
||||
timeout-minutes: 5
|
||||
timeout-minutes: 10
|
||||
outputs:
|
||||
lockfile_regenerated: ${{ steps.regen_lockfile.outputs.regenerated }}
|
||||
|
||||
|
|
@ -254,6 +254,7 @@ jobs:
|
|||
uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7
|
||||
with:
|
||||
node-version: 24
|
||||
cache: pnpm
|
||||
|
||||
- name: Validate migration ordering against target branch
|
||||
run: >-
|
||||
|
|
@ -296,22 +297,20 @@ jobs:
|
|||
PAPERCLIP_RELEASE_BOOTSTRAP_BASE_SHA="${{ github.event.pull_request.base.sha }}" \
|
||||
node ./scripts/check-release-package-bootstrap.mjs "${changed_paths[@]}"
|
||||
|
||||
- name: Validate dependency resolution when manifests change
|
||||
- name: Validate dependency resolution and regenerate stale lockfile
|
||||
id: regen_lockfile
|
||||
run: |
|
||||
changed="$(git diff --name-only "${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}")"
|
||||
manifest_pattern='(^|/)package\.json$|^pnpm-workspace\.yaml$|^\.npmrc$|^pnpmfile\.(cjs|js|mjs)$|^patches/'
|
||||
if printf '%s\n' "$changed" | grep -Eq "$manifest_pattern"; then
|
||||
pnpm install --lockfile-only --ignore-scripts --no-frozen-lockfile
|
||||
echo "regenerated=1" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
cp pnpm-lock.yaml "$RUNNER_TEMP/pnpm-lock.before.yaml"
|
||||
pnpm install --lockfile-only --ignore-scripts --no-frozen-lockfile
|
||||
if cmp -s "$RUNNER_TEMP/pnpm-lock.before.yaml" pnpm-lock.yaml; then
|
||||
echo "regenerated=0" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "regenerated=1" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
# Manifest-only PRs (where pnpm-lock.yaml stays at base because the policy
|
||||
# job above blocks committing it) need the regenerated lockfile for the
|
||||
# downstream `pnpm install --frozen-lockfile` steps. Upload it here so
|
||||
# every job consumes the same hash without recomputing.
|
||||
# Manifest-only and stacked PRs keep pnpm-lock.yaml at the default branch.
|
||||
# Upload a regenerated copy whenever the checked-out merge tree needs one.
|
||||
# Every downstream job then consumes the same hash without recomputing.
|
||||
- name: Upload regenerated lockfile for downstream jobs
|
||||
if: steps.regen_lockfile.outputs.regenerated == '1'
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
|
||||
|
|
@ -339,8 +338,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
@ -425,8 +424,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
@ -486,8 +485,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
@ -550,8 +549,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
@ -586,8 +585,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
@ -663,8 +662,8 @@ jobs:
|
|||
version: 9.15.4
|
||||
|
||||
- name: Restore regenerated PR lockfile (if policy uploaded one)
|
||||
if: needs.policy.outputs.lockfile_regenerated == '1'
|
||||
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
|
||||
continue-on-error: true
|
||||
with:
|
||||
name: pr-lockfile
|
||||
path: .
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ const durationsManifest = path.join(repoRoot, "scripts", "e2e-shard-durations.js
|
|||
const playwrightConfig = path.join(repoRoot, "tests", "e2e", "playwright.config.ts");
|
||||
const prCallerWorkflow = path.join(repoRoot, ".github", "workflows", "pr.yml");
|
||||
const trustedPrWorkflowPath = ".github/workflows/pr-trusted.yml";
|
||||
const trustedPrWorkflow = path.join(repoRoot, trustedPrWorkflowPath);
|
||||
|
||||
const SHARD_COUNT = 3;
|
||||
|
||||
|
|
@ -181,3 +182,41 @@ test("the trusted PR workflow passes the shard's spec filter to Playwright witho
|
|||
"pr-trusted.yml e2e_shards must invoke `pnpm run test:e2e $specs`",
|
||||
);
|
||||
});
|
||||
|
||||
test("the trusted PR workflow regenerates stale stacked lockfiles", () => {
|
||||
// Implementation PRs validate the workflow under development here. The
|
||||
// caller remains pinned to the last merged trusted SHA until a separate
|
||||
// activation PR advances it, so unmerged PR code never runs on trusted
|
||||
// infrastructure.
|
||||
const workflow = readFileSync(trustedPrWorkflow, "utf8");
|
||||
assert.match(
|
||||
workflow,
|
||||
/policy:\n needs: \[gate\][\s\S]{0,160}timeout-minutes: 10/,
|
||||
"the unconditional resolution step needs the same timeout headroom as the lockfile refresh workflow",
|
||||
);
|
||||
assert.match(
|
||||
workflow,
|
||||
/- name: Setup Node\.js\n uses: actions\/setup-node@[0-9a-f]+[^\n]*\n with:\n node-version: 24\n cache: pnpm/,
|
||||
"the policy job must restore the pnpm cache before dependency resolution",
|
||||
);
|
||||
assert.match(
|
||||
workflow,
|
||||
/pnpm install --lockfile-only --ignore-scripts --no-frozen-lockfile/,
|
||||
"the policy job must validate the complete merge tree instead of only the current PR layer",
|
||||
);
|
||||
assert.match(
|
||||
workflow,
|
||||
/cmp -s "\$RUNNER_TEMP\/pnpm-lock\.before\.yaml" pnpm-lock\.yaml/,
|
||||
"the policy job must upload a lockfile only when regeneration changed it",
|
||||
);
|
||||
|
||||
const restoreSteps = workflow.match(
|
||||
/- name: Restore regenerated PR lockfile \(if policy uploaded one\)\n if: needs\.policy\.outputs\.lockfile_regenerated == '1'/g,
|
||||
) ?? [];
|
||||
assert.equal(restoreSteps.length, 6, "every downstream install job must restore a required regenerated artifact");
|
||||
assert.doesNotMatch(
|
||||
workflow,
|
||||
/- name: Restore regenerated PR lockfile \(if policy uploaded one\)[\s\S]{0,220}continue-on-error:/,
|
||||
"a missing artifact must fail after the policy job says it uploaded one",
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue