From 39834a75d014a4104f057eedd8a92d15d80a0471 Mon Sep 17 00:00:00 2001 From: Declan Carroll Date: Thu, 17 Sep 2026 09:08:26 +0000 Subject: [PATCH] ci: Validate regenerated sync lockfiles (no-changelog) (#38921) --- .github/scripts/sync-master-to-3x.mjs | 14 +++- .github/scripts/sync-master-to-3x.test.mjs | 82 +++++++++++++++++++--- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/.github/scripts/sync-master-to-3x.mjs b/.github/scripts/sync-master-to-3x.mjs index 3d6013a5992..665a7fe84d5 100644 --- a/.github/scripts/sync-master-to-3x.mjs +++ b/.github/scripts/sync-master-to-3x.mjs @@ -150,6 +150,14 @@ export function resolveMechanicalPath({ git, pnpm, path, masterSha, log = consol // is required: pnpm flips `--frozen-lockfile` on by default when CI=true. log(`Regenerating ${path} with pnpm...`); pnpm(['install', '--lockfile-only', '--no-frozen-lockfile']); + try { + // A lockfile-only install does not apply patches. Validate the regenerated lockfile + // with a real install before treating it as a mechanical resolution. + pnpm(['install', '--frozen-lockfile', '--ignore-scripts']); + } catch (error) { + git(['checkout', '--conflict=merge', '--', path]); + throw error; + } git(['add', '--', path]); return; } @@ -305,6 +313,7 @@ export function reconcileWithMergeTreeAtTip({ */ export function reconcileLockfileAtTip({ git, pnpm, masterSha, log = console.log }) { pnpm(['install', '--lockfile-only', '--no-frozen-lockfile']); + pnpm(['install', '--frozen-lockfile', '--ignore-scripts']); if (attempt(git, ['diff', '--quiet', '--', LOCKFILE]).ok) return; if (git(['rev-parse', 'HEAD']) === masterSha) { throw new Error('Lockfile reconciliation would amend a master commit; refusing.'); @@ -328,8 +337,9 @@ export function reconcileLockfileAtTip({ git, pnpm, masterSha, log = console.log * nothing in the diff to suggest a decision was made. * * The lockfile is left with its markers when a manifest is among the code conflicts - * (regenerating is meaningless until the manifests are resolved) or when the regen fails - * transiently — flagged via `lockfileDeferred` so the PR body carries the instruction. + * (regenerating is meaningless until the manifests are resolved) or when regeneration or + * frozen-install validation fails — flagged via `lockfileDeferred` so the PR body carries + * the instruction. * * 3.x never carries the markers at its tip, and not for long in its history either: this * merge commit is dropped by the next replay, which takes the queue's commits only. diff --git a/.github/scripts/sync-master-to-3x.test.mjs b/.github/scripts/sync-master-to-3x.test.mjs index cf41390943a..c97835f80bf 100644 --- a/.github/scripts/sync-master-to-3x.test.mjs +++ b/.github/scripts/sync-master-to-3x.test.mjs @@ -181,12 +181,15 @@ test('resolveMechanicalPath takes the blob from master, or the deletion when mas ); }); -test('resolveMechanicalPath regenerates the lockfile with pnpm and stages it', () => { +test('resolveMechanicalPath regenerates and validates the lockfile before staging it', () => { const git = makeStub(); const pnpm = makeStub(); resolveMechanicalPath({ git, pnpm, path: LOCKFILE, masterSha: MASTER, log: () => {} }); // `--no-frozen-lockfile` is load-bearing: pnpm defaults to frozen when CI=true. - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); + assert.deepEqual(pnpm.calls, [ + ['install', '--lockfile-only', '--no-frozen-lockfile'], + ['install', '--frozen-lockfile', '--ignore-scripts'], + ]); assert.ok(git.calls.some((a) => a[0] === 'add' && a.includes(LOCKFILE))); }); @@ -416,9 +419,28 @@ test('reconcileLockfileAtTip folds an inconsistent lockfile into the tip commit, ]); const pnpm = makeStub(); reconcileLockfileAtTip({ git: inconsistent, pnpm, masterSha: MASTER, log: () => {} }); - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); + assert.deepEqual(pnpm.calls, [ + ['install', '--lockfile-only', '--no-frozen-lockfile'], + ['install', '--frozen-lockfile', '--ignore-scripts'], + ]); assert.ok(inconsistent.calls.some((a) => a[0] === 'commit' && a.includes('--amend'))); + const validationGit = makeStub(); + const invalid = makeStub([ + [(a) => a.includes('--frozen-lockfile'), fail('ERR_PNPM_PATCH_FAILED')], + ]); + assert.throws( + () => + reconcileLockfileAtTip({ + git: validationGit, + pnpm: invalid, + masterSha: MASTER, + log: () => {}, + }), + /command failed/, + ); + assert.equal(validationGit.calls.length, 0, 'validation must fail before diff or amend checks'); + const atMasterTip = makeStub([ [(a) => a[0] === 'diff', fail()], [(a) => a[0] === 'rev-parse', MASTER], @@ -662,9 +684,12 @@ test('sync auto-resolves a lockfile-only conflict during the replay — no PR, n await sync({ git, gh, pnpm, env, log: () => {} }); - // The stall regen plus the tip reconciliation check. + // The stall regen and the tip reconciliation each run regeneration and validation. assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); - assert.equal(pnpm.calls.length, 2); + assert.deepEqual(pnpm.calls[1], ['install', '--frozen-lockfile', '--ignore-scripts']); + assert.deepEqual(pnpm.calls[2], ['install', '--lockfile-only', '--no-frozen-lockfile']); + assert.deepEqual(pnpm.calls[3], ['install', '--frozen-lockfile', '--ignore-scripts']); + assert.equal(pnpm.calls.length, 4); assert.ok(git.calls.some((a) => a[0] === 'add' && a.includes(LOCKFILE))); assert.ok(git.calls.some((a) => a[0] === 'rebase' && a[1] === '--continue')); const push = git.calls.find((a) => a[0] === 'push'); @@ -811,7 +836,10 @@ test('buildConflictBranch pre-resolves mechanical files so only code conflicts r assert.deepEqual(files, ['packages/cli/x.ts']); assert.deepEqual(preResolved, [LOCKFILE, POPULARITY]); assert.equal(lockfileDeferred, false); - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); + assert.deepEqual(pnpm.calls, [ + ['install', '--lockfile-only', '--no-frozen-lockfile'], + ['install', '--frozen-lockfile', '--ignore-scripts'], + ]); assert.ok( git.calls.some((a) => a[0] === 'checkout' && a[1] === MASTER && a.includes(POPULARITY)), ); @@ -837,7 +865,39 @@ test('buildConflictBranch defers the lockfile when a manifest is conflicted too' assert.equal(pnpm.calls.length, 0, 'regen is meaningless until the manifests are resolved'); }); -test('buildConflictBranch degrades to a deferred lockfile when the regen fails', () => { +test('buildConflictBranch restores and defers a lockfile that fails frozen validation', () => { + const git = makeStub([ + [(a) => a[0] === 'merge', fail('CONFLICT')], + [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}`], + ]); + const pnpm = makeStub([ + [(a) => a.includes('--frozen-lockfile'), fail('ERR_PNPM_PATCH_FAILED')], + ]); + + const { files, preResolved, lockfileDeferred } = buildConflictBranch({ + git, + pnpm, + masterSha: MASTER, + log: () => {}, + }); + + assert.deepEqual(files, ['packages/cli/x.ts']); + assert.deepEqual(preResolved, []); + assert.equal(lockfileDeferred, true); + assert.deepEqual(pnpm.calls, [ + ['install', '--lockfile-only', '--no-frozen-lockfile'], + ['install', '--frozen-lockfile', '--ignore-scripts'], + ]); + assert.ok( + git.calls.some( + (a) => + a[0] === 'checkout' && a[1] === '--conflict=merge' && a.at(-1) === LOCKFILE, + ), + 'the committed conflict branch must contain the original lockfile markers', + ); +}); + +test('buildConflictBranch leaves the original lockfile conflict untouched when regeneration fails', () => { const git = makeStub([ [(a) => a[0] === 'merge', fail('CONFLICT')], [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}`], @@ -854,6 +914,12 @@ test('buildConflictBranch degrades to a deferred lockfile when the regen fails', assert.deepEqual(files, ['packages/cli/x.ts']); assert.deepEqual(preResolved, []); assert.equal(lockfileDeferred, true); + assert.equal(pnpm.calls.length, 1); + assert.equal( + git.calls.some((a) => a[0] === 'checkout' && a[1] === '--conflict=merge'), + false, + 'the initial failure leaves the unmodified conflict markers in place', + ); assert.ok( git.calls.some((a) => a[0] === 'commit'), 'the conflict branch must still be committed', @@ -931,7 +997,7 @@ test('sync reports only the code conflicts on a mixed conflict, with mechanical await sync({ git, gh, pnpm, env, fetchFn: okFetch(['alice']), log: () => {} }); - assert.equal(pnpm.calls.length, 1, 'the lockfile is regenerated for the conflict branch'); + assert.equal(pnpm.calls.length, 2, 'the lockfile is regenerated and validated'); const create = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'create'); const body = create[create.indexOf('--body') + 1];