From eff397b0193eac669496d3abefe5e251338b5cea Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 25 Aug 2026 03:56:15 +0900 Subject: [PATCH 1/3] fix(cli): tolerate patches that apply to nothing when pruning [RED-893] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A code bundle carries the workspace's whole pnpm `patchedDependencies` map but only a subset of its members, so a patch whose package belongs to an unbundled member applies to nothing. pnpm 10+ then aborts the lockfile pruner's temp-dir install with ERR_PNPM_UNUSED_PATCH and the unpruned lockfile ships instead — the outcome pruning exists to avoid. Pass --config.allowUnusedPatches=true so the prune produces a lockfile. The now-unused declaration still travels in the bundle; filtering it out follows. Co-Authored-By: Claude Opus 5 --- .gitattributes | 5 ++ .../pnpm-patched-workspace/package.json | 9 +++ .../packages/absent/package.json | 7 ++ .../packages/shimmed/package.json | 7 ++ .../packages/used/package.json | 7 ++ .../patches/ee-first@1.1.1.patch | 9 +++ .../patches/ms@2.1.3.patch | 9 +++ .../pnpm-patched-workspace/pnpm-lock.yaml | 64 +++++++++++++++++++ .../pnpm-workspace.yaml | 5 ++ .../__tests__/lockfile-pruner.spec.ts | 43 ++++++++++++- .../__tests__/package-manager.spec.ts | 4 +- .../package-files/package-manager.ts | 12 ++++ 12 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 .gitattributes create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/package.json create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/absent/package.json create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/shimmed/package.json create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/used/package.json create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ee-first@1.1.1.patch create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ms@2.1.3.patch create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-lock.yaml create mode 100644 packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-workspace.yaml diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..294df386 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,5 @@ +# Patch files are applied verbatim by package managers, and a diff whose line +# endings changed under checkout normalization may fail to apply. Keep them +# byte-identical across platforms. (pnpm normalizes line endings before hashing +# a patch, so this does not affect the `patch_hash=` it records in the lockfile.) +*.patch -text diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/package.json b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/package.json new file mode 100644 index 00000000..dc036a4c --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/package.json @@ -0,0 +1,9 @@ +{ + "name": "lockfile-pruner-fixture", + "private": true, + "dependencies": { + "@fixture/used": "workspace:*", + "@fixture/shimmed": "workspace:*", + "@fixture/absent": "workspace:*" + } +} diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/absent/package.json b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/absent/package.json new file mode 100644 index 00000000..2f515130 --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/absent/package.json @@ -0,0 +1,7 @@ +{ + "name": "@fixture/absent", + "version": "1.0.0", + "dependencies": { + "ee-first": "1.1.1" + } +} diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/shimmed/package.json b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/shimmed/package.json new file mode 100644 index 00000000..0c115778 --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/shimmed/package.json @@ -0,0 +1,7 @@ +{ + "name": "@fixture/shimmed", + "version": "1.0.0", + "dependencies": { + "isarray": "2.0.5" + } +} diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/used/package.json b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/used/package.json new file mode 100644 index 00000000..014958ab --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/packages/used/package.json @@ -0,0 +1,7 @@ +{ + "name": "@fixture/used", + "version": "1.0.0", + "dependencies": { + "ms": "2.1.3" + } +} diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ee-first@1.1.1.patch b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ee-first@1.1.1.patch new file mode 100644 index 00000000..cb47c57e --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ee-first@1.1.1.patch @@ -0,0 +1,9 @@ +diff --git a/index.js b/index.js +index 3333333..4444444 100644 +--- a/index.js ++++ b/index.js +@@ -1,3 +1,4 @@ ++// Patched by the lockfile-pruner fixture: unused once pruned, must be dropped. + /*! + * ee-first + */ diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ms@2.1.3.patch b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ms@2.1.3.patch new file mode 100644 index 00000000..48c838d8 --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/patches/ms@2.1.3.patch @@ -0,0 +1,9 @@ +diff --git a/index.js b/index.js +index 1111111..2222222 100644 +--- a/index.js ++++ b/index.js +@@ -1,3 +1,4 @@ ++// Patched by the lockfile-pruner fixture: applied, must survive pruning. + /** + * Helpers. + */ diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-lock.yaml b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-lock.yaml new file mode 100644 index 00000000..1b6f4eff --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-lock.yaml @@ -0,0 +1,64 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +patchedDependencies: + ee-first@1.1.1: + hash: 90b918fd6167721e405a502ac35adb29ec15497947e9d3b032d6da16a460b4af + path: patches/ee-first@1.1.1.patch + ms@2.1.3: + hash: 8efb625dd8ccb88e78507bea1f647ed25671bcda20a8554ea02a4122021736bb + path: patches/ms@2.1.3.patch + +importers: + + .: + dependencies: + '@fixture/absent': + specifier: workspace:* + version: link:packages/absent + '@fixture/shimmed': + specifier: workspace:* + version: link:packages/shimmed + '@fixture/used': + specifier: workspace:* + version: link:packages/used + + packages/absent: + dependencies: + ee-first: + specifier: 1.1.1 + version: 1.1.1(patch_hash=90b918fd6167721e405a502ac35adb29ec15497947e9d3b032d6da16a460b4af) + + packages/shimmed: + dependencies: + isarray: + specifier: 2.0.5 + version: 2.0.5 + + packages/used: + dependencies: + ms: + specifier: 2.1.3 + version: 2.1.3(patch_hash=8efb625dd8ccb88e78507bea1f647ed25671bcda20a8554ea02a4122021736bb) + +packages: + + ee-first@1.1.1: + resolution: {integrity: sha512-WMwm9LhRUo+WUaRN+vRuETqG89IgZphVSNkdFgeb6sS/E4OrDIN7t48CAewSHXc6C8lefD8KKfr5vY61brQlow==} + + isarray@2.0.5: + resolution: {integrity: sha512-xHjhDr3cNBK0BzdUJSPXZntQUx/mwMS5Rw4A7lPJ90XGAO6ISP/ePDNuo0vhqOZU+UD5JoodwCAAoZQd3FeAKw==} + + ms@2.1.3: + resolution: {integrity: sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==} + +snapshots: + + ee-first@1.1.1(patch_hash=90b918fd6167721e405a502ac35adb29ec15497947e9d3b032d6da16a460b4af): {} + + isarray@2.0.5: {} + + ms@2.1.3(patch_hash=8efb625dd8ccb88e78507bea1f647ed25671bcda20a8554ea02a4122021736bb): {} diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-workspace.yaml b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-workspace.yaml new file mode 100644 index 00000000..fc1b2fde --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner-fixtures/pnpm-patched-workspace/pnpm-workspace.yaml @@ -0,0 +1,5 @@ +packages: + - packages/* +patchedDependencies: + ms@2.1.3: patches/ms@2.1.3.patch + ee-first@1.1.1: patches/ee-first@1.1.1.patch diff --git a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner.spec.ts b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner.spec.ts index 6daaf169..ec20b86d 100644 --- a/packages/cli/src/services/check-parser/__tests__/lockfile-pruner.spec.ts +++ b/packages/cli/src/services/check-parser/__tests__/lockfile-pruner.spec.ts @@ -17,6 +17,11 @@ import { Err, Ok } from '../package-files/result.js' import { File } from '../parser.js' const PNPM_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'pnpm-workspace') +// Same shape as PNPM_FIXTURE_ROOT, plus two patched dependencies: `ms` is +// consumed by the bundled member, `ee-first` only by the member the bundle +// omits. Pruning therefore leaves the `ee-first` patch applying to nothing, +// which pnpm 10+ rejects unless the install tolerates unused patches. +const PNPM_PATCHED_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'pnpm-patched-workspace') const NPM_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'npm-workspace') const BUN_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'bun-workspace') const YARN_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'yarn-workspace') @@ -107,7 +112,7 @@ describe('lockfile-pruner', () => { // bundle where the `used` member is imported (real manifest), the `shimmed` // member is declared but unimported (faux manifest), and the `absent` // member is missing entirely. - const makeScenario = (root: string, lockfileName: string) => { + const makeScenario = (root: string, lockfileName: string, extraFiles: string[] = []) => { const used = new Package({ name: '@fixture/used', path: path.join(root, 'packages/used'), version: '1.0.0' }) const shimmed = new Package({ name: '@fixture/shimmed', path: path.join(root, 'packages/shimmed'), version: '1.0.0' }) const absent = new Package({ name: '@fixture/absent', path: path.join(root, 'packages/absent'), version: '1.0.0' }) @@ -137,11 +142,22 @@ describe('lockfile-pruner', () => { if (lockfileName === 'pnpm-lock.yaml') { files.set(...physical('pnpm-workspace.yaml')) } + for (const extra of extraFiles) { + files.set(...physical(extra)) + } return { workspace, files, used, shimmed, absent } } const makePnpmScenario = (root: string = PNPM_FIXTURE_ROOT) => makeScenario(root, 'pnpm-lock.yaml') + + // The patched fixture's bundle additionally carries the patch files, exactly + // as the auto-include does for a real bundle: pnpm hashes every declared + // patch file during resolution, so an install without them cannot run at all. + const makePnpmPatchedScenario = () => makeScenario(PNPM_PATCHED_FIXTURE_ROOT, 'pnpm-lock.yaml', [ + 'patches/ms@2.1.3.patch', + 'patches/ee-first@1.1.1.patch', + ]) const makeNpmScenario = (root: string = NPM_FIXTURE_ROOT) => makeScenario(root, 'package-lock.json') const makeBunScenario = (root: string = BUN_FIXTURE_ROOT) => makeScenario(root, 'bun.lock') const makeYarnScenario = (root: string = YARN_FIXTURE_ROOT) => makeScenario(root, 'yarn.lock') @@ -724,6 +740,31 @@ describe('lockfile-pruner', () => { expect(result.content).not.toContain('ee-first') }, 60_000) + it('prunes a workspace whose patch applies to nothing once pruned, with real pnpm', async () => { + const { workspace, files } = makePnpmPatchedScenario() + const result = await pruneBundledLockfile({ + workspace, + packageManager: new PNpmDetector(), + files, + env: testEnv(), + }) + + // Without --config.allowUnusedPatches the install aborts with + // ERR_PNPM_UNUSED_PATCH, because the `ee-first` patch has nothing left to + // apply to once the member consuming it is pruned away. + expect(result.status).toEqual('pruned') + if (result.status !== 'pruned') { + return + } + + // The patch that still applies keeps its marker; the one that no longer + // does loses it. Both declarations survive in the section regardless, + // because it mirrors the config rather than the dependency graph. + expect(result.content).toContain('patch_hash=8efb625dd8ccb88e78507bea1f647ed25671bcda20a8554ea02a4122021736bb') + expect(result.content).not.toContain('patch_hash=90b918fd6167721e405a502ac35adb29ec15497947e9d3b032d6da16a460b4af') + expect(result.content).toContain('ee-first@1.1.1:') + }, 60_000) + it('fails when npm silently replaces a workspace link with a registry package', async () => { const { workspace, files } = makeNpmScenario() // Simulate npm's registry substitution: the link entry for the used diff --git a/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts b/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts index 2795d223..c23cfc1f 100644 --- a/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts +++ b/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts @@ -575,11 +575,13 @@ describe('detectNearestConfigFiles', () => { }) describe('lockfileOnlyInstallCommand', () => { - it('regenerates the lockfile without installing for pnpm, pinning the lockfile location', () => { + it('regenerates the lockfile without installing for pnpm, pinning the lockfile location' + + ' and tolerating patches that apply to nothing', () => { const runnable = new PNpmDetector().lockfileOnlyInstallCommand() expect(runnable?.executable).toEqual('pnpm') expect(runnable?.args).toEqual([ 'install', '--lockfile-only', '--ignore-scripts', '--no-frozen-lockfile', '--lockfile-dir', '.', + '--config.allowUnusedPatches=true', ]) }) diff --git a/packages/cli/src/services/check-parser/package-files/package-manager.ts b/packages/cli/src/services/check-parser/package-files/package-manager.ts index 2a5328b3..0758c225 100644 --- a/packages/cli/src/services/check-parser/package-files/package-manager.ts +++ b/packages/cli/src/services/check-parser/package-files/package-manager.ts @@ -362,8 +362,20 @@ export class PNpmDetector extends PackageManagerDetector implements PackageManag // setting can move the lockfile write elsewhere; '.' resolves against // the subprocess working directory. Accepted by pnpm 8-11 (pnpm 11 // dropped the npmrc setting but kept the flag). + // + // --config.allowUnusedPatches is what lets a partial-workspace bundle be + // pruned at all when the project patches a dependency: a bundle carries + // the full patchedDependencies map but only a subset of the workspace, so + // a patch whose package belongs to an unbundled member applies to nothing + // and pnpm 10+ fails the install with ERR_PNPM_UNUSED_PATCH. The flag + // downgrades that to a warning, so the prune produces a lockfile instead + // of falling back; the unused declaration itself is filtered out of the + // bundle afterwards. Verified on pnpm 10 and 11: the setting does not + // reach the regenerated lockfile's `settings` section, so the snapshot + // comparisons below are unaffected. return new Runnable('pnpm', [ 'install', '--lockfile-only', '--ignore-scripts', '--no-frozen-lockfile', '--lockfile-dir', '.', + '--config.allowUnusedPatches=true', ]) } From 377cb02453bcd7e089d0fef3ddc31ef475647944 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 25 Aug 2026 05:38:50 +0900 Subject: [PATCH 2/3] feat(cli): decide which pnpm patch declarations a bundle still needs [RED-893] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds patched-dependencies.ts: given a bundle's declaring config plus the lockfile before and after pruning, it works out which `patchedDependencies` declarations no longer apply and produces the rewritten config and lockfile bytes that drop them. Nothing calls it yet. pnpm records each declaration's hash in the lockfile's `patchedDependencies` section and repeats it as `patch_hash=` wherever the patch was applied. The section mirrors the config rather than the dependency graph, so pruning leaves an unused declaration in place and the marker is what tells the two apart. The test is differential — the marker must be present before pruning and absent after — so a lockfile whose markers cannot be read drops nothing instead of dropping everything. Every other uncertainty declines the same way, leaving the bundle untouched: an unreadable config or lockfile, both declaration sites populated at once (pnpm honors only one, and which one depends on the major), a declaration the project's own lockfile never recorded, or a rewrite whose reparsed result does not match the original minus exactly the removed keys. Patch files are dropped only by normalized archive path, and only when no surviving declaration references them. Co-Authored-By: Claude Opus 5 --- .../__tests__/patched-dependencies.spec.ts | 527 +++++++++++++++++ .../check-parser/patched-dependencies.ts | 537 ++++++++++++++++++ 2 files changed, 1064 insertions(+) create mode 100644 packages/cli/src/services/check-parser/__tests__/patched-dependencies.spec.ts create mode 100644 packages/cli/src/services/check-parser/patched-dependencies.ts diff --git a/packages/cli/src/services/check-parser/__tests__/patched-dependencies.spec.ts b/packages/cli/src/services/check-parser/__tests__/patched-dependencies.spec.ts new file mode 100644 index 00000000..43bbb4b8 --- /dev/null +++ b/packages/cli/src/services/check-parser/__tests__/patched-dependencies.spec.ts @@ -0,0 +1,527 @@ +import { spawnSync } from 'node:child_process' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' + +import { afterAll, describe, expect, it } from 'vitest' + +import { PNpmDetector } from '../package-files/package-manager.js' +import { + findUnrepairedPatchKeys, + PatchConfigFile, + planPatchFilter, + readLockfilePatchHashes, + readPatchedDependencies, + rewriteYamlSection, + verifyRewrite, +} from '../patched-dependencies.js' + +const PNPM_PATCHED_FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'pnpm-patched-workspace') + +const MS_HASH = '8efb625dd8ccb88e78507bea1f647ed25671bcda20a8554ea02a4122021736bb' +const EE_FIRST_HASH = '90b918fd6167721e405a502ac35adb29ec15497947e9d3b032d6da16a460b4af' + +const workspaceYaml = (entries: string[]): PatchConfigFile => ({ + archivePath: 'pnpm-workspace.yaml', + kind: 'pnpm-workspace.yaml', + content: [ + 'packages:', + ' - packages/*', + 'minimumReleaseAge: 2880', + ...entries.length > 0 ? ['patchedDependencies:', ...entries] : [], + '', + ].join('\n'), +}) + +const packageJson = (entries: Record): PatchConfigFile => ({ + archivePath: 'package.json', + kind: 'package.json', + content: JSON.stringify({ + name: 'fixture', + private: true, + pnpm: { patchedDependencies: entries }, + }, null, 2), +}) + +// A lockfile carrying both declarations in its section, but a `patch_hash=` +// marker only for `ms` — the shape a prune produces once the member consuming +// the other patched package has been pruned away. +const prunedLockfile = [ + `lockfileVersion: '9.0'`, + ``, + `patchedDependencies:`, + ` ee-first@1.1.1:`, + ` hash: ${EE_FIRST_HASH}`, + ` path: patches/ee-first@1.1.1.patch`, + ` ms@2.1.3:`, + ` hash: ${MS_HASH}`, + ` path: patches/ms@2.1.3.patch`, + ``, + `importers:`, + ``, + ` packages/used:`, + ` dependencies:`, + ` ms:`, + ` specifier: 2.1.3`, + ` version: 2.1.3(patch_hash=${MS_HASH})`, + ``, + `snapshots:`, + ``, + ` ms@2.1.3(patch_hash=${MS_HASH}): {}`, + ``, +].join('\n') + +// The same workspace before pruning: both patches applied to something. +const originalLockfile = prunedLockfile + .replace('importers:\n', 'importers:\n\n packages/absent:\n dependencies:\n ee-first:\n' + + ` specifier: 1.1.1\n version: 1.1.1(patch_hash=${EE_FIRST_HASH})\n`) + +const bothEntries = [ + ` ms@2.1.3: patches/ms@2.1.3.patch`, + ` ee-first@1.1.1: patches/ee-first@1.1.1.patch`, +] + +describe('readPatchedDependencies()', () => { + it('reads the top-level map from pnpm-workspace.yaml', () => { + expect(readPatchedDependencies(workspaceYaml(bothEntries))).toEqual([ + { key: 'ms@2.1.3', patchPath: 'patches/ms@2.1.3.patch' }, + { key: 'ee-first@1.1.1', patchPath: 'patches/ee-first@1.1.1.patch' }, + ]) + }) + + it('reads the pnpm field from package.json', () => { + expect(readPatchedDependencies(packageJson({ 'ms@2.1.3': 'patches/ms@2.1.3.patch' }))).toEqual([ + { key: 'ms@2.1.3', patchPath: 'patches/ms@2.1.3.patch' }, + ]) + }) + + it('reports no declarations when the config has no section', () => { + expect(readPatchedDependencies(workspaceYaml([]))).toEqual([]) + expect(readPatchedDependencies({ + archivePath: 'package.json', + kind: 'package.json', + content: '{"name":"fixture"}', + })).toEqual([]) + }) + + it.each([['empty', ''], ['comment-only', '# nothing configured yet\n']])( + 'reads a %s pnpm-workspace.yaml as declaring nothing', (_label, content) => { + expect(readPatchedDependencies({ + archivePath: 'pnpm-workspace.yaml', + kind: 'pnpm-workspace.yaml', + content, + })).toEqual([]) + }) + + it('declines an unparseable config', () => { + expect(readPatchedDependencies({ + archivePath: 'package.json', + kind: 'package.json', + content: '{not json', + })).toBeUndefined() + }) + + it('declines a declaration whose value is not a patch path', () => { + expect(readPatchedDependencies({ + archivePath: 'pnpm-workspace.yaml', + kind: 'pnpm-workspace.yaml', + content: 'patchedDependencies:\n ms@2.1.3:\n path: patches/ms.patch\n', + })).toBeUndefined() + }) +}) + +describe('readLockfilePatchHashes()', () => { + it('reads the pnpm 10 object shape', () => { + expect(readLockfilePatchHashes(prunedLockfile)).toEqual(new Map([ + ['ee-first@1.1.1', EE_FIRST_HASH], + ['ms@2.1.3', MS_HASH], + ])) + }) + + it('reads the pnpm 11 bare-hash shape', () => { + const content = `lockfileVersion: '9.0'\n\npatchedDependencies:\n ms@2.1.3: ${MS_HASH}\n` + expect(readLockfilePatchHashes(content)).toEqual(new Map([['ms@2.1.3', MS_HASH]])) + }) + + it('reports an empty map when the lockfile records no patches', () => { + expect(readLockfilePatchHashes(`lockfileVersion: '9.0'\n`)).toEqual(new Map()) + }) + + // Every decline below must stay a decline: a caller that read one as "no + // patches recorded" would classify every declaration as unused and strip + // patches that are actually in force. + it('declines an unparseable lockfile', () => { + expect(readLockfilePatchHashes('\tbad: [yaml')).toBeUndefined() + }) + + it('declines a lockfile that is not a mapping', () => { + expect(readLockfilePatchHashes('- a\n- b\n')).toBeUndefined() + }) + + it('declines a patchedDependencies section that is not a mapping', () => { + expect(readLockfilePatchHashes(`patchedDependencies:\n - ms@2.1.3\n`)).toBeUndefined() + }) + + it.each([ + ['no hash', `patchedDependencies:\n ms@2.1.3:\n path: patches/ms.patch\n`], + ['an empty hash', `patchedDependencies:\n ms@2.1.3:\n hash: ''\n`], + ])('declines an entry with %s', (_label, content) => { + expect(readLockfilePatchHashes(content)).toBeUndefined() + }) +}) + +describe('verifyRewrite()', () => { + // The safety net behind every rewrite: if a serializer ever dropped or + // reshaped content the edit did not target, this is what catches it. + it('rejects a rewrite that lost content the edit did not target', () => { + expect(verifyRewrite('{"a":1,"b":2}', '{"a":1}', new Set(), JSON.parse)).toBeUndefined() + }) + + it('rejects a rewrite that changed an unrelated value', () => { + expect(verifyRewrite('{"a":1}', '{"a":2}', new Set(), JSON.parse)).toBeUndefined() + }) + + it('accepts a rewrite that removed exactly the targeted keys', () => { + const original = '{"patchedDependencies":{"ms@2.1.3":"p","ee@1":"q"},"other":true}' + const rewritten = '{"patchedDependencies":{"ms@2.1.3":"p"},"other":true}' + expect(verifyRewrite(original, rewritten, new Set(['ee@1']), JSON.parse)).toEqual(rewritten) + }) + + it('declines when the rewritten content cannot be reparsed', () => { + expect(verifyRewrite('{"a":1}', '{not json', new Set(), JSON.parse)).toBeUndefined() + }) +}) + +describe('rewriteYamlSection()', () => { + it('declines unparseable YAML rather than returning it unchanged', () => { + expect(rewriteYamlSection('\tbad: [yaml', new Set(['ms@2.1.3']))).toBeUndefined() + }) + + it('declines a patchedDependencies section that is not a mapping', () => { + expect(rewriteYamlSection(`patchedDependencies: notamap\n`, new Set(['ms@2.1.3']))) + .toBeUndefined() + }) + + it('leaves a document without the section untouched', () => { + const content = `lockfileVersion: '9.0'\n` + expect(rewriteYamlSection(content, new Set(['ms@2.1.3']))).toEqual(content) + }) +}) + +describe('planPatchFilter()', () => { + const plan = (configs: PatchConfigFile[], overrides: Partial<{ + originalLockfileContent: string + prunedLockfileContent: string + }> = {}) => planPatchFilter({ + configs, + originalLockfileContent: originalLockfile, + prunedLockfileContent: prunedLockfile, + ...overrides, + }) + + it('drops the declaration whose patch no longer applies and keeps the one that does', () => { + const result = plan([workspaceYaml(bothEntries)]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual(['patches/ee-first@1.1.1.patch']) + expect(result?.rewrittenConfig.archivePath).toEqual('pnpm-workspace.yaml') + expect(result?.rewrittenConfig.content).toContain('ms@2.1.3: patches/ms@2.1.3.patch') + expect(result?.rewrittenConfig.content).not.toContain('ee-first') + expect(result?.lockfileContent).toContain(`ms@2.1.3:`) + expect(result?.lockfileContent).not.toContain('ee-first') + }) + + it('preserves unrelated config content around the edit', () => { + const result = plan([workspaceYaml(bothEntries)]) + + expect(result?.rewrittenConfig.content).toContain('minimumReleaseAge: 2880') + expect(result?.rewrittenConfig.content).toContain('- packages/*') + }) + + it('removes the section entirely when its last entry goes', () => { + const onlyUnused = [` ee-first@1.1.1: patches/ee-first@1.1.1.patch`] + const result = plan([workspaceYaml(onlyUnused)]) + + expect(result?.rewrittenConfig.content).not.toContain('patchedDependencies') + expect(result?.rewrittenConfig.content).toContain('minimumReleaseAge: 2880') + }) + + it('edits the package.json site when that is where the patches are declared', () => { + const result = plan([ + workspaceYaml([]), + packageJson({ + 'ms@2.1.3': 'patches/ms@2.1.3.patch', + 'ee-first@1.1.1': 'patches/ee-first@1.1.1.patch', + }), + ]) + + expect(result?.rewrittenConfig.archivePath).toEqual('package.json') + expect(JSON.parse(result!.rewrittenConfig.content).pnpm.patchedDependencies) + .toEqual({ 'ms@2.1.3': 'patches/ms@2.1.3.patch' }) + }) + + it.each([ + ['a version range', 'ee-first@^1.1.0'], + ['a bare name', 'ee-first'], + ['a scoped name', '@fixture/ee-first@1.1.1'], + ])('matches %s key verbatim, exactly as the lockfile records it', (_label, key) => { + const configs = [workspaceYaml([` '${key}': patches/ee-first@1.1.1.patch`])] + const withKey = (content: string) => content + .replace(' ee-first@1.1.1:', ` '${key}':`) + + const result = planPatchFilter({ + configs, + originalLockfileContent: withKey(originalLockfile), + prunedLockfileContent: withKey(prunedLockfile), + }) + + expect(result?.unusedKeys).toEqual([key]) + }) + + it('leaves the bundle alone when more than one config declares patches', () => { + // pnpm picks one site and ignores the other wholesale, and which one wins + // depends on the major, so neither can be edited safely. + expect(plan([ + workspaceYaml(bothEntries), + packageJson({ 'ms@2.1.3': 'patches/ms@2.1.3.patch' }), + ])).toBeUndefined() + }) + + it('keeps a declaration the original lockfile never recorded', () => { + // The pnpm that wrote the lockfile may not read the site the key was + // declared in, so its absence is no evidence that the patch is unused. + const withoutEeFirst = originalLockfile + .replace(` ee-first@1.1.1:\n hash: ${EE_FIRST_HASH}\n path: patches/ee-first@1.1.1.patch\n`, '') + + expect(plan([workspaceYaml(bothEntries)], { originalLockfileContent: withoutEeFirst })) + .toBeUndefined() + }) + + it('does nothing when every declared patch still applies', () => { + expect(plan([workspaceYaml(bothEntries)], { prunedLockfileContent: originalLockfile })) + .toBeUndefined() + }) + + it('keeps a patch file a surviving declaration still references', () => { + const shared = [ + ` ms@2.1.3: patches/shared.patch`, + ` ee-first@1.1.1: patches/shared.patch`, + ] + const result = plan([workspaceYaml(shared)]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual([]) + }) + + it.each([ + ['the survivor spells the shared path differently', './patches/shared.patch', 'patches/shared.patch'], + ['the dropped key spells it differently', 'patches/shared.patch', './patches/shared.patch'], + ['a spelling needs normalizing', 'patches/sub/../shared.patch', 'patches/shared.patch'], + ])('keeps a shared patch file when %s', (_label, keptPath, unusedPath) => { + // Both declarations name one file; comparing the raw spellings rather than + // the archive paths would delete a file the surviving declaration needs. + const result = plan([workspaceYaml([ + ` ms@2.1.3: ${keptPath}`, + ` ee-first@1.1.1: ${unusedPath}`, + ])]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual([]) + }) + + it('drops no patch file at all when a surviving declaration points outside the bundle root', () => { + // An escaping path cannot be compared against the in-root candidates, so + // it might alias one of them; the declarations still go. + const result = plan([workspaceYaml([ + ` ms@2.1.3: ../outside/shared.patch`, + ` ee-first@1.1.1: patches/ee-first@1.1.1.patch`, + ])]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual([]) + }) + + it('declines when any bundled config cannot be parsed', () => { + // Filtering on the readable site alone would delete patch files the + // unreadable one may still declare. A UTF-8 BOM is enough to make + // JSON.parse reject a manifest that fs.readFile happily returned. + expect(plan([ + { archivePath: 'package.json', kind: 'package.json', content: '\uFEFF{"name":"fixture"}' }, + workspaceYaml(bothEntries), + ])).toBeUndefined() + }) + + it('declines when a config has duplicate keys', () => { + expect(plan([{ + archivePath: 'pnpm-workspace.yaml', + kind: 'pnpm-workspace.yaml', + content: 'packages:\n - a\npackages:\n - b\n', + }, packageJson({ 'ee-first@1.1.1': 'patches/ee-first@1.1.1.patch' })])).toBeUndefined() + }) + + it('drops nothing when the original lockfile shows no patch applied anywhere', () => { + // A lockfile this module cannot read markers out of must degrade to + // dropping nothing, never to dropping every declaration at once. + const markerless = originalLockfile.replace(/\(patch_hash=[^)]*\)/g, '') + expect(plan([workspaceYaml(bothEntries)], { originalLockfileContent: markerless })) + .toBeUndefined() + }) + + it.each([ + ['an empty path', ` ee-first@1.1.1: ''`], + ['a directory path', ` ee-first@1.1.1: patches/`], + ['a Windows-drive path', ` ee-first@1.1.1: 'E:\\proj\\patches\\ee.patch'`], + ['an absolute path', ` ee-first@1.1.1: /tmp/patches/ee.patch`], + ['a path that traverses out and back', ` ee-first@1.1.1: patches/sub/../../package.json`], + ])('never lists %s among the files to drop', (_label, entry) => { + const result = plan([workspaceYaml([` ms@2.1.3: patches/ms@2.1.3.patch`, entry])]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual([]) + }) + + it('declines when the pruned lockfile cannot be parsed', () => { + // Treating an unreadable lockfile as "records no patches" would mark every + // declaration unused and silently unpatch every dependency. + expect(plan([workspaceYaml(bothEntries)], { prunedLockfileContent: '\tbad: [yaml' })) + .toBeUndefined() + }) + + it('declines when the original lockfile cannot be parsed', () => { + expect(plan([workspaceYaml(bothEntries)], { originalLockfileContent: '\tbad: [yaml' })) + .toBeUndefined() + }) + + it('never drops a patch file that resolves outside the bundle root', () => { + const escaping = [` ee-first@1.1.1: ../outside/ee-first.patch`] + const result = plan([workspaceYaml(escaping)]) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual([]) + }) + + it('repairs the config but spares a patch the sectionless lockfile still marks as applied', () => { + // A lockfile that lost the section but kept its `patch_hash=` markers still + // pins that patch; dropping its declaration and file would leave the + // lockfile pinning a patch nothing declares. + const sectionless = prunedLockfile + .replace(/patchedDependencies:\n(?: {2}\S[^\n]*\n(?: {4}[^\n]*\n)*)+/, '') + const result = plan([workspaceYaml(bothEntries)], { prunedLockfileContent: sectionless }) + + expect(result?.unusedKeys).toEqual(['ee-first@1.1.1']) + expect(result?.droppedPatchPaths).toEqual(['patches/ee-first@1.1.1.patch']) + expect(result?.lockfileContent).toEqual(sectionless) + expect(result?.rewrittenConfig.content).toContain('ms@2.1.3') + expect(result?.rewrittenConfig.content).not.toContain('ee-first') + }) +}) + +describe('findUnrepairedPatchKeys()', () => { + it('reports a declaration the shipped lockfile no longer records', () => { + const shipped = prunedLockfile + .replace(` ee-first@1.1.1:\n hash: ${EE_FIRST_HASH}\n path: patches/ee-first@1.1.1.patch\n`, '') + + expect(findUnrepairedPatchKeys({ + configs: [workspaceYaml(bothEntries)], + originalLockfileContent: originalLockfile, + shippedLockfileContent: shipped, + })).toEqual(['ee-first@1.1.1']) + }) + + it('stays silent once the declaration has been filtered out of both', () => { + const result = planPatchFilter({ + configs: [workspaceYaml(bothEntries)], + originalLockfileContent: originalLockfile, + prunedLockfileContent: prunedLockfile, + }) + + expect(findUnrepairedPatchKeys({ + configs: [{ ...workspaceYaml(bothEntries), content: result!.rewrittenConfig.content }], + originalLockfileContent: originalLockfile, + shippedLockfileContent: result!.lockfileContent, + })).toEqual([]) + }) + + it('stays silent for a declaration the original lockfile never recorded', () => { + const withoutSection = originalLockfile + .replace(/patchedDependencies:\n(?: {2}\S[^\n]*\n(?: {4}[^\n]*\n)*)+/, '') + + expect(findUnrepairedPatchKeys({ + configs: [workspaceYaml(bothEntries)], + originalLockfileContent: withoutSection, + shippedLockfileContent: withoutSection, + })).toEqual([]) + }) +}) + +// The whole design rests on a hand-applied YAML edit producing exactly the +// bytes pnpm itself would write for a config that never declared the patch. If +// a future pnpm changes its lockfile formatting, this fails rather than letting +// the CLI ship a subtly different lockfile. +describe('rewriteYamlSection() against real pnpm', () => { + const tempDirs: string[] = [] + + afterAll(async () => { + await Promise.all(tempDirs.map(dir => fs.rm(dir, { recursive: true, force: true, maxRetries: 3 }))) + }) + + // Materializes what the pruner feeds its temp-dir install: the bundle's real + // manifests, dependency-free placeholders for the members it omits, and the + // patch files (pnpm hashes every declared patch during resolution). + const materialize = async (declaredPatches: string[]): Promise => { + const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'patched-deps-spec-'))) + tempDirs.push(dir) + + await fs.mkdir(path.join(dir, 'packages/used'), { recursive: true }) + await fs.mkdir(path.join(dir, 'packages/shimmed'), { recursive: true }) + await fs.mkdir(path.join(dir, 'packages/absent'), { recursive: true }) + await fs.mkdir(path.join(dir, 'patches'), { recursive: true }) + + for (const file of ['package.json', 'pnpm-lock.yaml', 'packages/used/package.json']) { + await fs.copyFile(path.join(PNPM_PATCHED_FIXTURE_ROOT, file), path.join(dir, file)) + } + for (const name of ['@fixture/shimmed', '@fixture/absent']) { + const target = name === '@fixture/shimmed' ? 'packages/shimmed' : 'packages/absent' + await fs.writeFile( + path.join(dir, target, 'package.json'), + JSON.stringify({ name, version: '1.0.0' }), + ) + } + for (const patch of declaredPatches) { + await fs.copyFile( + path.join(PNPM_PATCHED_FIXTURE_ROOT, 'patches', patch), + path.join(dir, 'patches', patch), + ) + } + + const entries = declaredPatches.map(patch => ` ${patch.replace(/\.patch$/, '')}: patches/${patch}`) + await fs.writeFile( + path.join(dir, 'pnpm-workspace.yaml'), + ['packages:', ' - packages/*', 'patchedDependencies:', ...entries, ''].join('\n'), + ) + + // The production command, not a copy of it: this test's whole point is that + // the bytes pnpm writes under the flags the pruner actually passes can be + // reproduced by editing, so it must break if those flags change. + const runnable = new PNpmDetector().lockfileOnlyInstallCommand() + const install = spawnSync(runnable.executable, runnable.args, + { cwd: dir, encoding: 'utf8', shell: process.platform === 'win32' }) + expect(install.status, install.stderr ?? '').toEqual(0) + + return await fs.readFile(path.join(dir, 'pnpm-lock.yaml'), 'utf8') + } + + it('reproduces the lockfile pnpm writes when the unused patch is not declared', async () => { + const withUnused = await materialize(['ms@2.1.3.patch', 'ee-first@1.1.1.patch']) + // Guards the comparison below from passing vacuously: pnpm must actually + // have kept the now-unused declaration in the section for the edit to have + // anything to remove. + expect(withUnused).toContain('ee-first@1.1.1:') + expect(withUnused).not.toContain(`patch_hash=${EE_FIRST_HASH}`) + + const edited = rewriteYamlSection(withUnused, new Set(['ee-first@1.1.1'])) + const authentic = await materialize(['ms@2.1.3.patch']) + + expect(edited).toEqual(authentic) + }, 120_000) +}) diff --git a/packages/cli/src/services/check-parser/patched-dependencies.ts b/packages/cli/src/services/check-parser/patched-dependencies.ts new file mode 100644 index 00000000..c82502fc --- /dev/null +++ b/packages/cli/src/services/check-parser/patched-dependencies.ts @@ -0,0 +1,537 @@ +import path from 'node:path' +import { isDeepStrictEqual } from 'node:util' + +import Debug from 'debug' +import { isMap, parse as parseYaml, parseDocument } from 'yaml' + +import { pathToPosix } from '../util.js' + +const debug = Debug('checkly:cli:services:check-parser:patched-dependencies') + +/** + * Serialization options that make a `yaml` round trip of a pnpm lockfile + * byte-identical to what pnpm itself writes. `lineWidth: 0` disables the + * folding that would otherwise rewrap long `resolution:` lines, and + * `flowCollectionPadding: false` reproduces pnpm's unpadded `{integrity: ...}` + * spelling. Both are required; with either one at its default the serializer + * reformats lines it was not asked to touch. + */ +const YAML_STRINGIFY_OPTIONS = { lineWidth: 0, flowCollectionPadding: false } as const + +const PATCHED_DEPENDENCIES = 'patchedDependencies' + +/** + * Where a pnpm project can declare `patchedDependencies`. Both are live: + * pnpm 10 reads the `package.json` field (and prefers it over + * `pnpm-workspace.yaml` when both are populated), while pnpm 11 ignores it + * and reads only `pnpm-workspace.yaml`. + */ +export type PatchConfigKind = 'pnpm-workspace.yaml' | 'package.json' + +/** One of the bundle's declaring config files. */ +export interface PatchConfigFile { + /** Posix path of the file within the archive, e.g. `pnpm-workspace.yaml`. */ + archivePath: string + kind: PatchConfigKind + content: string +} + +export interface RewrittenFile { + archivePath: string + content: string +} + +/** + * A complete, ready-to-apply description of the patch declarations to remove + * from a bundle. Everything is computed up front so that applying it is pure + * map mutation: a half-applied filter ships a bundle whose config and lockfile + * disagree, which fails the remote install outright. + */ +export interface PatchFilterPlan { + /** The `patchedDependencies` keys that no longer apply to anything. */ + unusedKeys: string[] + /** The declaring config, rewritten without those keys. */ + rewrittenConfig: RewrittenFile + /** + * Archive paths of patch files that only the removed declarations + * referenced. Paths resolving outside the workspace root are never listed. + */ + droppedPatchPaths: string[] + /** The pruned lockfile, rewritten without those keys. */ + lockfileContent: string +} + +export interface PlanPatchFilterOptions { + configs: PatchConfigFile[] + /** The workspace's own lockfile, as committed — before pruning. */ + originalLockfileContent: string + /** + * The lockfile the prune regenerated. Must come from a prune that passed the + * pruner's own verification (see `verifyPrunedLockfile`): that check is what + * guarantees the two lockfiles spell `patch_hash=` markers comparably, which + * is the premise the used/unused decision rests on. Handed an unverified + * regeneration — one written by a pnpm that disagrees about which + * declaration site is live — the comparison could read a patch that is + * genuinely in force as no longer applied. + */ + prunedLockfileContent: string +} + +interface PatchDeclaration { + key: string + /** The patch file path exactly as the config spells it. */ + patchPath: string +} + +/** + * Reads a config's `patchedDependencies` map. Returns an empty map when the + * config declares none, and `undefined` when the file cannot be parsed or the + * map is not shaped as expected — callers treat that as "do not filter". + */ +export function readPatchedDependencies (config: PatchConfigFile): PatchDeclaration[] | undefined { + let parsed: unknown + try { + parsed = config.kind === 'package.json' + ? JSON.parse(config.content) + // An empty or comment-only YAML document parses to null: an ordinary + // pnpm-workspace.yaml that declares no settings, not an unreadable file. + // JSON has no such spelling — JSON.parse throws on empty input. + : parseYaml(config.content) ?? {} + } catch (err) { + debug(`Could not parse ${config.archivePath}: ${err}`) + return undefined + } + + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return undefined + } + + const container = config.kind === 'package.json' + ? (parsed as Record).pnpm + : parsed + if (container === undefined || container === null) { + return [] + } + if (typeof container !== 'object' || Array.isArray(container)) { + return undefined + } + + const section = (container as Record)[PATCHED_DEPENDENCIES] + if (section === undefined || section === null) { + return [] + } + if (typeof section !== 'object' || Array.isArray(section)) { + return undefined + } + + const declarations: PatchDeclaration[] = [] + for (const [key, value] of Object.entries(section as Record)) { + // pnpm only accepts a path string here. Anything else is a config this + // code does not understand, so it declines to touch the bundle at all. + if (typeof value !== 'string') { + return undefined + } + declarations.push({ key, patchPath: value }) + } + return declarations +} + +/** + * Reads a lockfile's `patchedDependencies` section as key → patch hash. + * pnpm 10 records `{ hash, path }` objects and pnpm 11 a bare hash string; + * both are accepted. Returns `undefined` when the lockfile cannot be parsed. + */ +export function readLockfilePatchHashes (lockfileContent: string): Map | undefined { + let parsed: unknown + try { + parsed = parseYaml(lockfileContent) + } catch (err) { + debug(`Could not parse the lockfile: ${err}`) + return undefined + } + + const hashes = new Map() + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return undefined + } + + const section = (parsed as Record)[PATCHED_DEPENDENCIES] + if (section === undefined || section === null) { + return hashes + } + if (typeof section !== 'object' || Array.isArray(section)) { + return undefined + } + + for (const [key, value] of Object.entries(section as Record)) { + const hash = typeof value === 'string' ? value : value?.hash + if (typeof hash !== 'string' || hash === '') { + return undefined + } + hashes.set(key, hash) + } + return hashes +} + +/** + * Finds the single config whose `patchedDependencies` the bundle should be + * filtered against, or `undefined` when there is nothing to do or the configs + * cannot be used. + * + * Declining when more than one config declares a non-empty map is deliberate: + * pnpm picks ONE site and ignores the other wholesale, and which site wins + * depends on the pnpm major (10 prefers `package.json`, 11 ignores it + * entirely). Editing the losing site is not merely useless — emptying the + * winning one would promote the loser, changing which patches apply. + */ +function findDeclaringConfig ( + configs: PatchConfigFile[], +): { config: PatchConfigFile, declarations: PatchDeclaration[] } | undefined { + const declaring: Array<{ config: PatchConfigFile, declarations: PatchDeclaration[] }> = [] + + for (const config of configs) { + const declarations = readPatchedDependencies(config) + if (declarations === undefined) { + return undefined + } + if (declarations.length > 0) { + declaring.push({ config, declarations }) + } + } + + if (declaring.length > 1) { + debug('More than one config declares patchedDependencies; leaving the bundle alone') + return undefined + } + + return declaring[0] +} + +/** + * Whether a patch declaration still applies to something in `lockfileContent`. + * + * pnpm records the applied patch by hash: the `patchedDependencies` section + * pins `key → hash`, and that hash reappears as `patch_hash=` in the + * importer, snapshot and package entries of every package the patch was + * applied to. A declaration that applies to nothing keeps its section entry — + * the section mirrors the config, not the dependency graph — but its hash + * appears nowhere else, which makes the marker the sole discriminator. A + * whole-file search is safe because the section's own value never contains + * the literal `patch_hash=`. + */ +function hasMarker (hash: string | undefined, lockfileContent: string): boolean { + return hash !== undefined && lockfileContent.includes(`patch_hash=${hash}`) +} + +/** + * Whether the prune is what stopped a patch from applying. + * + * Deliberately differential rather than a bare absence test on the pruned + * lockfile. Absence alone would mean that a lockfile this module cannot read + * markers out of — a future pnpm that keeps the `9` version string but spells + * them differently — reads as "nothing applies anywhere" and every declaration + * is stripped at once, which is the one outcome the module exists to prevent. + * Requiring the marker in the original first makes that case drop nothing. + * + * The pruned side falls back to the original's hash because a lockfile can + * lose the `patchedDependencies` section while keeping its markers; without + * the fallback such a patch would read as no longer applied. + */ +function droppedByPrune ( + key: string, + prunedHashes: Map, + originalHashes: Map, + originalLockfileContent: string, + prunedLockfileContent: string, +): boolean { + const originalHash = originalHashes.get(key) + if (!hasMarker(originalHash, originalLockfileContent)) { + return false + } + return !hasMarker(prunedHashes.get(key) ?? originalHash, prunedLockfileContent) +} + +/** + * Resolves a declared patch path to its archive path, or `undefined` when it + * escapes the workspace root (the archive's own root) and therefore does not + * name a bundle entry this code may remove. + */ +function patchArchivePath (patchPath: string): string | undefined { + // Traversal is judged on the path AS WRITTEN, before any normalization: + // pathToPosix normalizes internally, which collapses `..` segments and would + // hide a path that leaves the root and comes back + // (`patches/sub/../../package.json` collapses to `package.json`, naming a + // bundle file that is not a patch at all). + if (patchPath.split(/[/\\]/).includes('..')) { + return undefined + } + + const normalized = pathToPosix(patchPath) + if (path.posix.isAbsolute(normalized)) { + return undefined + } + // `.` (what an empty path normalizes to) and a trailing slash name a + // directory, not a patch file, so they must never reach a caller that + // deletes bundle entries. + if (normalized === '.' || normalized.endsWith('/')) { + return undefined + } + // pathToPosix only rewrites the platform's own separator and strips C:/D: + // drive prefixes, so a spelling from another platform can survive looking + // relative yet still denote a path this comparison cannot align with. + if (normalized.includes('\\') || /^[A-Za-z]:/.test(normalized)) { + return undefined + } + return normalized +} + +/** + * Computes the patch declarations a bundle should stop shipping, together with + * the rewritten files that carry the removal. Returns `undefined` whenever the + * evidence is incomplete or an edit cannot be verified — the caller then ships + * the bundle unchanged, which is always safe. + */ +export function planPatchFilter (options: PlanPatchFilterOptions): PatchFilterPlan | undefined { + const { configs, originalLockfileContent, prunedLockfileContent } = options + + const declaring = findDeclaringConfig(configs) + if (declaring === undefined) { + return undefined + } + + const originalHashes = readLockfilePatchHashes(originalLockfileContent) + const prunedHashes = readLockfilePatchHashes(prunedLockfileContent) + if (originalHashes === undefined || prunedHashes === undefined) { + return undefined + } + + const unusedKeys: string[] = [] + // Both sets hold ARCHIVE paths, never the raw spelling: two declarations can + // point at one file with different spellings (`./patches/x.patch` and + // `patches/x.patch`), and comparing raw strings would drop a file a + // surviving declaration still needs. + const keptArchivePaths = new Set() + const candidateArchivePaths = new Set() + // A surviving declaration whose path resolves outside the bundle root cannot + // be compared against the in-root candidates at all, so no file is dropped + // in that case. The declarations and lockfile entries still go; an + // unreferenced patch file left in the bundle is inert. + let keptPathEscapesRoot = false + + for (const declaration of declaring.declarations) { + const archivePath = patchArchivePath(declaration.patchPath) + + // A key the project's own lockfile never recorded is no evidence of + // anything: the pnpm that wrote it may simply not read the site the key + // was declared in. Removing it would silently unpatch a dependency that a + // different pnpm on the runner would patch. + const recorded = originalHashes.has(declaration.key) + if (!recorded) { + debug(`Patch '${declaration.key}' is not recorded in the project's lockfile; leaving it alone`) + } + const dropped = recorded && droppedByPrune( + declaration.key, prunedHashes, originalHashes, originalLockfileContent, prunedLockfileContent, + ) + if (!dropped) { + if (archivePath === undefined) { + keptPathEscapesRoot = true + } else { + keptArchivePaths.add(archivePath) + } + continue + } + + unusedKeys.push(declaration.key) + if (archivePath === undefined) { + debug(`Patch path '${declaration.patchPath}' resolves outside the bundle root; leaving it alone`) + } else { + candidateArchivePaths.add(archivePath) + } + } + + if (unusedKeys.length === 0) { + return undefined + } + + const removed = new Set(unusedKeys) + + const configContent = rewriteConfig(declaring.config, removed) + if (configContent === undefined) { + return undefined + } + const rewrittenConfig: RewrittenFile = { + archivePath: declaring.config.archivePath, + content: configContent, + } + + const lockfileContent = rewriteYamlSection(prunedLockfileContent, removed) + if (lockfileContent === undefined) { + return undefined + } + + // Two declarations may share a patch file; keep it while any survivor still + // points at it. + const droppedPatchPaths = keptPathEscapesRoot + ? [] + : Array.from(candidateArchivePaths).filter(archivePath => !keptArchivePaths.has(archivePath)) + + return { unusedKeys, rewrittenConfig, droppedPatchPaths, lockfileContent } +} + +function rewriteConfig (config: PatchConfigFile, removed: Set): string | undefined { + return config.kind === 'package.json' + ? rewritePackageJson(config.content, removed) + : rewriteYamlSection(config.content, removed) +} + +/** + * Removes keys from a YAML document's top-level `patchedDependencies` map, + * dropping the map itself once it empties. Shared by `pnpm-workspace.yaml` and + * `pnpm-lock.yaml`, whose sections have the same shape. + * + * The result is verified structurally before it is returned: a serializer that + * reformatted or dropped anything else would ship silently, and for the + * lockfile it would also invalidate the verification the pruner already ran + * over the bytes it produced. + */ +export function rewriteYamlSection (content: string, removed: Set): string | undefined { + // parseDocument collects syntax errors on the document rather than throwing, + // so `errors` — not a try/catch — is what rejects malformed input here. + const doc = parseDocument(content) + if (doc.errors.length > 0) { + debug(`Could not parse YAML for rewriting: ${doc.errors[0].message}`) + return undefined + } + + const section = doc.get(PATCHED_DEPENDENCIES) + if (section === undefined || section === null) { + // Nothing to delete. The prune can legitimately return a lockfile that + // carries no section at all, in which case only the config needs editing. + return content + } + if (!isMap(section)) { + return undefined + } + + for (const key of removed) { + doc.deleteIn([PATCHED_DEPENDENCIES, key]) + } + if (section.items.length === 0) { + doc.delete(PATCHED_DEPENDENCIES) + } + + const rewritten = doc.toString(YAML_STRINGIFY_OPTIONS) + return verifyRewrite(content, rewritten, removed, parseYaml) +} + +function rewritePackageJson (content: string, removed: Set): string | undefined { + let parsed: any + try { + parsed = JSON.parse(content) + } catch (err) { + debug(`Could not parse package.json for rewriting: ${err}`) + return undefined + } + + const section = parsed?.pnpm?.[PATCHED_DEPENDENCIES] + if (section === null || typeof section !== 'object') { + return undefined + } + + for (const key of removed) { + delete section[key] + } + if (Object.keys(section).length === 0) { + delete parsed.pnpm[PATCHED_DEPENDENCIES] + } + + // Unlike the YAML path, which is byte-preserving, this reformats the whole + // manifest. The bundled copy is only ever an install input — nothing reads it + // back as text — and matching the original's formatting would mean carrying a + // JSON editor for no behavioral gain. + const rewritten = JSON.stringify(parsed, null, 2) + return verifyRewrite(content, rewritten, removed, JSON.parse) +} + +/** + * Asserts that a rewrite changed nothing but the intended keys, by comparing + * the reparsed result against the reparsed original with those keys deleted. + * A config can carry anything else pnpm understands — `catalog:`, `overrides`, + * `onlyBuiltDependencies` — and nothing downstream would notice it being + * mangled, so the check is what makes the edit safe to ship. + * + * Exported so that the rejection path — which no legitimate serializer output + * reaches, and which therefore cannot be driven through the callers — is + * directly testable. + */ +export function verifyRewrite ( + original: string, + rewritten: string, + removed: Set, + parse: (content: string) => any, +): string | undefined { + let expected: any + let actual: any + try { + expected = parse(original) + actual = parse(rewritten) + } catch (err) { + debug(`Could not reparse a rewritten file for verification: ${err}`) + return undefined + } + + for (const container of [expected, expected?.pnpm]) { + const section = container?.[PATCHED_DEPENDENCIES] + if (section === null || typeof section !== 'object') { + continue + } + for (const key of removed) { + delete section[key] + } + if (Object.keys(section).length === 0) { + delete container[PATCHED_DEPENDENCIES] + } + } + + if (!isDeepStrictEqual(expected, actual)) { + debug('A rewritten file did not match the expected structure; leaving the bundle alone') + return undefined + } + + return rewritten +} + +export interface FindUnrepairedPatchKeysOptions { + configs: PatchConfigFile[] + originalLockfileContent: string + /** The lockfile the bundle is about to ship. */ + shippedLockfileContent: string +} + +/** + * Reports declarations the shipped bundle still carries but its shipped + * lockfile no longer records — the precondition for the remote install failing + * with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH`. + * + * Scoped to keys the project's original lockfile recorded, so a declaration + * the project's own pnpm never honoured is not reported as a bundling problem. + */ +export function findUnrepairedPatchKeys (options: FindUnrepairedPatchKeysOptions): string[] { + const { configs, originalLockfileContent, shippedLockfileContent } = options + + const originalHashes = readLockfilePatchHashes(originalLockfileContent) + const shippedHashes = readLockfilePatchHashes(shippedLockfileContent) + if (originalHashes === undefined || shippedHashes === undefined) { + return [] + } + + const unrepaired = new Set() + for (const config of configs) { + for (const declaration of readPatchedDependencies(config) ?? []) { + if (originalHashes.has(declaration.key) && !shippedHashes.has(declaration.key)) { + unrepaired.add(declaration.key) + } + } + } + return Array.from(unrepaired).sort() +} From 2b1e36fe28bb3e84254ad07b69dba5bd578be4d7 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 25 Aug 2026 06:31:23 +0900 Subject: [PATCH 3/3] feat(cli): ship only the patches a code bundle still applies [RED-893] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the patch planner into the bundler: after the lockfile is pruned, declarations the pruned lockfile shows no longer apply are removed from the bundled pnpm config, their patch files are left out of the archive, and the matching lockfile entries go with them. The whole plan is applied in one pass of map mutation, because a half-applied filter ships a config and lockfile that disagree and fails the remote install either way it lands. Only a `.patch` directly under the conventional `patches/` directory is ever removed. A declared path can name any bundled file — an .npmrc carrying registry auth, a pnpmfile the lockfile records a checksum for, a fixture a check's own include glob pulled in — so a declaration pointing elsewhere loses its config and lockfile entry while the file stays. Whatever the step does or declines to do, it then checks what actually ships: if the bundled config still declares a patch the bundled lockfile does not record, the CLI says so, since that pair is what fails the runner's install. A rewritten root manifest ships as a synthesized file, so it is hashed the way on-disk manifests are, with `version` stripped — hashing it verbatim would discard the runner's dependency cache on a release bump that changed no install input. Co-Authored-By: Claude Opus 5 --- .../references/configure-playwright-checks.md | 2 +- .../check-parser/__tests__/bundler.spec.ts | 490 +++++++++++++++++- .../cli/src/services/check-parser/bundler.ts | 232 ++++++++- .../src/services/check-parser/cache-hash.ts | 10 +- .../services/check-parser/lockfile-pruner.ts | 8 + .../check-parser/patched-dependencies.ts | 18 + .../services/playwright-project-bundler.ts | 5 +- 7 files changed, 757 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ai-context/references/configure-playwright-checks.md b/packages/cli/src/ai-context/references/configure-playwright-checks.md index e2d16599..2ffb1615 100644 --- a/packages/cli/src/ai-context/references/configure-playwright-checks.md +++ b/packages/cli/src/ai-context/references/configure-playwright-checks.md @@ -13,7 +13,7 @@ - The `.npmrc` should reference a Checkly environment variable such as `${NPM_TOKEN}`. Tell the user that the token must exist in Checkly before `deploy` or `trigger`. Because `.npmrc` is uploaded automatically, warn users to reference credentials via environment variables (`${NPM_TOKEN}`) rather than embedding plaintext tokens. - Use `installCommand` only when the default package-manager install command is not enough. - For pnpm projects, the workspace-root pnpmfile (`.pnpmfile.cjs` / `.pnpmfile.mjs`) is bundled automatically so the remote install reproduces the lockfile's recorded `pnpmfileChecksum` — but only when two conditions hold. First, the lockfile must record a `pnpmfileChecksum` (pnpm writes one when `pnpm install` last ran with the pnpmfile in place); without one there is nothing for the remote install to reproduce and no pnpmfile is bundled. Second, the pnpmfile must be self-contained: it may load only side-effect-free Node.js builtins (`assert`, `buffer`, `crypto`, `events`, `path`, `punycode`, `querystring`, `string_decoder`, `url`, `util`) via literal `require('...')`/`import` specifiers, and must not reference `process`, `__dirname`, `__filename`, `import.meta`, `globalThis`, `global`, `eval`, `Function`, `require.resolve` or dynamically computed module paths — the remote install loads the pnpmfile before any dependencies exist and in a different environment. A pnpmfile that does not satisfy this (e.g. loads `fs`, local helper files or npm packages) is skipped with a warning and the remote install may re-resolve dependencies instead of using the lockfile; make the pnpmfile self-contained to avoid this. Since the pnpmfile is uploaded, avoid embedding secrets in it. -- In a workspace (monorepo) whose code bundle covers only part of the workspace, the bundled lockfile is pruned automatically: the CLI regenerates it (via `pnpm install --lockfile-only` / `npm install --package-lock-only` / `bun install --lockfile-only` / `yarn install --mode=update-lockfile` in a temp dir) so it only references the packages actually in the bundle — otherwise the remote install would try to fetch dependencies of workspace members that were omitted or shipped as dependency-free placeholder manifests, which fails outright for private packages. Supported for `pnpm-lock.yaml` versions 6/9, `package-lock.json` versions 2/3, the text `bun.lock` version 1 and Yarn Berry `yarn.lock` files (for bun projects, keep registry configuration in `.npmrc`, which bun reads: `bunfig.toml` is not carried into the regeneration — recorded resolutions keep their URLs, but whenever bun declines to reuse the lockfile — it is out of date with a manifest, or a workspace member's name collides with a registry dependency — bun re-resolves those entries against the wrong registry, disclosing the package names to it (typically the public registry), and pruning rejects the result with a warning; for yarn projects, `.yarnrc.yml` is likewise not carried into the regeneration, which is safe because Berry lockfiles are registry-agnostic and the regeneration reuses recorded resolutions without the network — settings like `approvedGitRepositories` and `npmScopes` only affect new resolutions, which pruning never performs — and the regeneration runs with yarn's network access disabled outright, since it never needs it: a lockfile that is out of date with a manifest then fails fast with a warning instead of resolving the missing package against the wrong registry and disclosing its name; yarn's hardened mode is disabled for the same reason; `yarn patch` files under `.yarn/patches` are bundled automatically because the regeneration reads them; a `yarn` binary that resolves to Yarn Classic on a Berry project is refused before it can run, because Classic would silently perform a full install); when a bundled lockfile over-describes a partial-workspace bundle but pruning cannot run — other lockfile formats, Yarn Classic v1 lockfiles, a `yarn` binary that resolves to Yarn Classic on a Berry project (set the `packageManager` field so Corepack provisions Yarn 2+), bun's binary `bun.lockb` (regenerate a text lockfile with `bun install --save-text-lockfile`), the package manager binary not being installed on the machine running the CLI, `excludeLinksFromLockfile`, a recorded pnpmfile checksum without a bundled pnpmfile, a workspace member whose version cannot be determined, among others — the original lockfile ships unchanged and the CLI prints a note saying so. Other skips are silent: nothing to prune (the bundle contains the full workspace, or regeneration produced identical bytes), no bundled lockfile to prune, or pruning disabled via `CHECKLY_LOCKFILE_PRUNE=0`; silent skip reasons are visible via `DEBUG='checkly:cli:services:check-parser:*'`. When pruning runs but cannot produce a provably pruned copy of the original — the lockfile is out of date with a `package.json`, the package manager could not run or timed out, the lockfile could not be read or written, or verification failed, among others — the original ships unchanged with a warning. Set `CHECKLY_LOCKFILE_PRUNE=0` to disable pruning. +- In a workspace (monorepo) whose code bundle covers only part of the workspace, the bundled lockfile is pruned automatically: the CLI regenerates it (via `pnpm install --lockfile-only` / `npm install --package-lock-only` / `bun install --lockfile-only` / `yarn install --mode=update-lockfile` in a temp dir) so it only references the packages actually in the bundle — otherwise the remote install would try to fetch dependencies of workspace members that were omitted or shipped as dependency-free placeholder manifests, which fails outright for private packages. Supported for `pnpm-lock.yaml` versions 6/9, `package-lock.json` versions 2/3, the text `bun.lock` version 1 and Yarn Berry `yarn.lock` files (for bun projects, keep registry configuration in `.npmrc`, which bun reads: `bunfig.toml` is not carried into the regeneration — recorded resolutions keep their URLs, but whenever bun declines to reuse the lockfile — it is out of date with a manifest, or a workspace member's name collides with a registry dependency — bun re-resolves those entries against the wrong registry, disclosing the package names to it (typically the public registry), and pruning rejects the result with a warning; for yarn projects, `.yarnrc.yml` is likewise not carried into the regeneration, which is safe because Berry lockfiles are registry-agnostic and the regeneration reuses recorded resolutions without the network — settings like `approvedGitRepositories` and `npmScopes` only affect new resolutions, which pruning never performs — and the regeneration runs with yarn's network access disabled outright, since it never needs it: a lockfile that is out of date with a manifest then fails fast with a warning instead of resolving the missing package against the wrong registry and disclosing its name; yarn's hardened mode is disabled for the same reason; `yarn patch` files under `.yarn/patches` are bundled automatically because the regeneration reads them; a `yarn` binary that resolves to Yarn Classic on a Berry project is refused before it can run, because Classic would silently perform a full install); when a bundled lockfile over-describes a partial-workspace bundle but pruning cannot run — other lockfile formats, Yarn Classic v1 lockfiles, a `yarn` binary that resolves to Yarn Classic on a Berry project (set the `packageManager` field so Corepack provisions Yarn 2+), bun's binary `bun.lockb` (regenerate a text lockfile with `bun install --save-text-lockfile`), the package manager binary not being installed on the machine running the CLI, `excludeLinksFromLockfile`, a recorded pnpmfile checksum without a bundled pnpmfile, a workspace member whose version cannot be determined, among others — the original lockfile ships unchanged and the CLI prints a note saying so. Other skips are silent: nothing to prune (the bundle contains the full workspace, or regeneration produced identical bytes), no bundled lockfile to prune, or pruning disabled via `CHECKLY_LOCKFILE_PRUNE=0`; silent skip reasons are visible via `DEBUG='checkly:cli:services:check-parser:*'`. When pruning runs but cannot produce a provably pruned copy of the original — the lockfile is out of date with a `package.json`, the package manager could not run or timed out, the lockfile could not be read or written, or verification failed, among others — the original ships unchanged with a warning. For pnpm projects that patch a dependency (`patchedDependencies` in `pnpm-workspace.yaml` or in the root `package.json`'s `pnpm` field), pruning also filters the patches: a bundle carries the whole map but only part of the workspace, so a patch whose package belongs to an unbundled member would apply to nothing, which pnpm rejects whenever it resolves the bundle (the CLI hits this while regenerating the lockfile). Once the lockfile has been pruned, the declarations it shows no longer apply are removed from the bundled config, their patch files are left out of the bundle, and the matching entries are removed from the bundled lockfile, so the three agree. Declarations the project's own lockfile never recorded are left alone (the pnpm that wrote it may not read the declaration site), as are projects that declare patches in both places at once, since pnpm honors only one of them and which one depends on the pnpm version, and patch files kept outside the conventional `patches/` directory (their declaration is still removed; only the file stays). Leaving a declaration in place is safe as long as the bundled lockfile still records it — pnpm only rejects an unused patch when it re-resolves — but a bundle whose config declares a patch its lockfile does not record can fail the remote install with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` or `ERR_PNPM_UNUSED_PATCH`, so the CLI prints a note naming those declarations; refresh the lockfile with your own install to clear it. Patch files no declaration references are bundled as-is. Set `CHECKLY_LOCKFILE_PRUNE=0` to disable pruning. - Checkly caches installed dependencies between runs, keyed off the workspace's lock file, every workspace member's `package.json` and `.npmrc` (whether or not the member is in the bundle), bundled pnpmfile contents, and the resolved `bundle.packages.embed` tarball set (filtered to what the pruned lockfile still references when pruning applied) — plus, as additional inputs, any synthesized placeholder manifests shipped in the bundle and the pruned lockfile when pruning applied. Because the bundle-specific inputs follow the bundle, the key can change without a file edit — e.g. when a different set of workspace members ends up in the bundle. To force a reinstall declaratively, set `caching.dependencyCache.version` (a string or a safe integer) at the top level of `checkly.config.ts` (not per check — one code bundle serves all Playwright Check Suites) and change its value whenever the cache should be invalidated; scheduled checks pick up the change on the next `checkly deploy`. Unset or empty-string values leave the cache key unchanged, so a dynamic value such as `version: process.env.DEPENDENCY_CACHE_VERSION` is safe when the variable is not always set. For a one-off reinstall during an ad-hoc run, use the `--refresh-cache` flag available on the run/test commands (`checkly test`, `checkly pw-test`, `checkly trigger`, `checkly checks run`) instead; the config value is the persistent knob that also applies to deployed, scheduled checks. - In Checkly CLI v8.0.0 and later, `include` patterns resolve relative to the Playwright config directory, not the project root. If `playwrightConfigPath` points to a subdirectory, adjust `include` globs. Example: `playwrightConfigPath: "./e2e/playwright.config.ts"` with a root fixture at `fixtures/data.json` needs `include: ["../fixtures/data.json"]`. - If dependencies come from a private registry that Checkly's infrastructure cannot reach (for example an intranet-only Nexus mirror), list them in `bundle.packages.embed` in `checkly.config.ts` — a top-level section: `bundle: { packages: { embed: ['@acme/private-utils', 'legacy-private-pkg@2.1.0'] } }`. Each entry is a package name (embeds every version found in the lockfile) or an exact `name@version` pin; names may contain `*` wildcards (`@acme/*`, `acme-*`, `@acme/*-utils`) where each `*` matches any run of characters except `/` (never crossing the scope separator); a `!` prefix (`!@acme/legacy`, `!@acme/*`, `!legacy@2.1.0`) turns an entry into an exclusion that removes the packages it matches from what the entries *before* it selected, so entries apply in order — `['@acme/*', '!@acme/legacy']` embeds the whole scope except `@acme/legacy`, while the reverse order embeds the whole scope because the exclusion runs before anything has been selected; as long as a spec matches at least one registry package, matches that cannot be embedded are skipped (workspace members silently, git/file/URL dependencies and integrity-less entries with a warning since the runner must fetch those itself), while a spec whose only matches cannot be embedded — or that matches nothing at all — is an error, except that exclusions never error (one that removes nothing is a no-op) and removing every package an earlier entry selected also silences that entry — no error, and no skip warning even for packages it matched but did not exclude, so use `DEBUG='checkly:cli:services:embedded-packages'` to see what such an entry reached; because exclusions only subtract, a list of nothing but `!` entries selects nothing, and a configuration whose entries select no packages at all is reported as a warning (packages dropped later by lockfile pruning are covered by the pruning note above); a pattern embeds every lockfile version of every package it matches, so scope it to the packages the runner genuinely cannot fetch. List every unreachable package by name, including private packages that only appear as transitive dependencies of other private packages — dependencies of listed packages are not embedded automatically. The CLI resolves entries against the workspace-root lockfile (`pnpm-lock.yaml`, `package-lock.json`, the text `bun.lock` or a Yarn Berry `yarn.lock` — Yarn Classic v1 lockfiles are not supported), reuses tarballs from local caches (its own, then npm's) or downloads them from the registry configured in `.npmrc` (only `.npmrc` — bun or yarn users whose registry credentials live solely in `bunfig.toml` or `.yarnrc.yml` must duplicate them into `.npmrc`, or downloads fail with an auth error), verifies each against the lockfile's integrity hash, and ships them inside the code bundle at `.checkly/embedded-packages/*.tgz`, where the runner serves them through a local registry during install. Yarn Berry lockfiles record no npm tarball integrity (Berry checksums cover yarn's own cache format), so the CLI resolves the tarball integrity from the registry's package metadata instead — one small metadata request per embedded package on every deploy (the per-version route, falling back to the full packument), even when the tarballs themselves come from a warm cache, so a yarn embed needs registry reachability at deploy time even on a warm cache. When the bundled lockfile is pruned to the code bundle's contents (see the pruning bullet above), the embedded set follows it: packages the pruned lockfile no longer references — dependencies of workspace members that are not part of the bundle — are neither embedded nor downloaded, even if an entry matches them. If a package unexpectedly stops being embedded, the usual cause is that only a workspace member outside the bundle depends on it, in which case the runner never installs it and nothing is wrong; if the checks genuinely need it, make the depending member part of the bundle (import it from check code) rather than disabling pruning — `CHECKLY_LOCKFILE_PRUNE=0` restores the unfiltered set but reintroduces the over-describing lockfile that pruning exists to prevent, so treat it as a last resort. Downloads are cached under the workspace root's `node_modules/.cache/checkly` (in a monorepo that is the repo root, not the member package; override with `CHECKLY_CACHE_DIR`; a per-user cache dir is the fallback when the project location isn't writable), so nothing lands in the project outside `node_modules`. CI setups that cache `node_modules` — or platforms that preserve `node_modules/.cache` — persist the tarballs automatically; otherwise persist `CHECKLY_CACHE_DIR` in CI to avoid re-downloading (note `npm ci` deletes `node_modules` wholesale, unlike incremental pnpm installs). The machine running `checkly deploy`/`test` needs registry access on a cold cache — but only for the tarballs actually shipped, not for pruned-away ones. Changing the resolved set of embedded packages invalidates the runner's dependency cache, so the next run reinstalls with the new tarballs. Applies to Playwright Check Suites only, not browser or multistep checks. diff --git a/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts b/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts index b3d42fb1..6ac6e835 100644 --- a/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts +++ b/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts @@ -7,7 +7,12 @@ import { list } from 'tar' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { BundleArchive, BundleTooLargeError, Bundler, embeddedPackageHashInputs, FinalizedBundleArchive } from '../bundler.js' -import { composeWorkspaceCacheHash, loadWorkspaceCacheHashInputs } from '../cache-hash.js' +import { + canonicalizePackageJson, + composeWorkspaceCacheHash, + loadWorkspaceCacheHashInputs, + PACKAGE_JSON_EXCLUDED_FIELDS, +} from '../cache-hash.js' import { CNpmDetector, npmPackageManager, PNpmDetector, Runnable } from '../package-files/package-manager.js' import { Package, Workspace } from '../package-files/workspace.js' import { Err, Ok } from '../package-files/result.js' @@ -702,3 +707,486 @@ describe('Bundler.finalize() embedded package materialization', () => { await expect(bundler.finalize()).rejects.toThrow(EmbeddedPackageError) }) }) + +describe('Bundler.finalize() patch filtering', () => { + let dir: string + let stderrWrites: string[] + + const MS_HASH = 'a'.repeat(64) + const EE_HASH = 'b'.repeat(64) + + const FAUX_MANIFEST = '{"name":"@fixture/m","version":"1.0.0"}' + + const patchedDependenciesSection = [ + `patchedDependencies:`, + ` ee-first@1.1.1:`, + ` hash: ${EE_HASH}`, + ` path: patches/ee-first@1.1.1.patch`, + ` ms@2.1.3:`, + ` hash: ${MS_HASH}`, + ` path: patches/ms@2.1.3.patch`, + ``, + ] + + // Both patches applied: `ms` in the bundled member, `ee-first` in the member + // the bundle omits. + const originalLockfile = () => [ + `lockfileVersion: '9.0'`, + ``, + ...patchedDependenciesSection, + `importers:`, + ``, + ` .: {}`, + ``, + ` packages/m:`, + ` dependencies:`, + ` ms:`, + ` specifier: 2.1.3`, + ` version: 2.1.3(patch_hash=${MS_HASH})`, + ``, + ` packages/absent:`, + ` dependencies:`, + ` ee-first:`, + ` specifier: 1.1.1`, + ` version: 1.1.1(patch_hash=${EE_HASH})`, + ``, + ].join('\n') + + // What the stub prune produces: the omitted member is gone, so the + // `ee-first` patch applies to nothing — but its declaration survives, since + // the section mirrors the config rather than the graph. + const prunedLockfile = () => [ + `lockfileVersion: '9.0'`, + ``, + ...patchedDependenciesSection, + `importers:`, + ``, + ` .: {}`, + ``, + ` packages/m:`, + ` dependencies:`, + ` ms:`, + ` specifier: 2.1.3`, + ` version: 2.1.3(patch_hash=${MS_HASH})`, + ``, + ].join('\n') + + const workspaceYaml = () => [ + `packages:`, + ` - packages/*`, + `minimumReleaseAge: 2880`, + `patchedDependencies:`, + ` ms@2.1.3: patches/ms@2.1.3.patch`, + ` ee-first@1.1.1: patches/ee-first@1.1.1.patch`, + ``, + ].join('\n') + + beforeEach(async () => { + dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'checkly-bundler-patch-'))) + await fs.mkdir(path.join(dir, 'packages/m'), { recursive: true }) + await fs.mkdir(path.join(dir, 'patches'), { recursive: true }) + await fs.writeFile(path.join(dir, 'package.json'), JSON.stringify({ + name: 'patch-fixture-root', + private: true, + })) + await fs.writeFile(path.join(dir, 'packages/m/package.json'), FAUX_MANIFEST) + await fs.writeFile(path.join(dir, 'pnpm-workspace.yaml'), workspaceYaml()) + await fs.writeFile(path.join(dir, 'pnpm-lock.yaml'), originalLockfile()) + await fs.writeFile(path.join(dir, 'patches/ms@2.1.3.patch'), 'ms patch\n') + await fs.writeFile(path.join(dir, 'patches/ee-first@1.1.1.patch'), 'ee-first patch\n') + + stderrWrites = [] + vi.spyOn(process.stderr, 'write').mockImplementation(chunk => { + const text = String(chunk) + if (!text.includes('checkly:cli:')) { + stderrWrites.push(text) + } + return true + }) + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + }) + + afterEach(async () => { + vi.restoreAllMocks() + vi.unstubAllEnvs() + await fs.rm(dir, { recursive: true, force: true }) + }) + + const makeWorkspace = () => new Workspace({ + root: new Package({ name: 'patch-fixture-root', path: dir }), + packages: [new Package({ name: '@fixture/m', path: path.join(dir, 'packages/m'), version: '1.0.0' })], + lockfile: Ok(path.join(dir, 'pnpm-lock.yaml')), + configFile: Ok(path.join(dir, 'pnpm-workspace.yaml')), + }) + + const stubPruningPackageManager = async () => { + await fs.writeFile(path.join(dir, 'pruned-lock.yaml'), prunedLockfile()) + const scriptPath = path.join(dir, 'prune.cjs') + await fs.writeFile(scriptPath, [ + `const fs = require('fs')`, + `fs.writeFileSync('pnpm-lock.yaml', fs.readFileSync(${JSON.stringify(path.join(dir, 'pruned-lock.yaml'))}, 'utf8'))`, + ].join('\n')) + return Object.assign(Object.create(new PNpmDetector()), { + lockfileOnlyInstallCommand: () => new Runnable('node', [scriptPath]), + }) + } + + const makeBundler = async () => { + const bundler = await Bundler.createForWorkspace(makeWorkspace(), { + tempDir: path.join(dir, 'out'), + packageManager: await stubPruningPackageManager(), + }) + bundler.registerFiles( + { filePath: path.join(dir, 'package.json'), physical: true }, + { filePath: path.join(dir, 'pnpm-workspace.yaml'), physical: true }, + { filePath: path.join(dir, 'pnpm-lock.yaml'), physical: true }, + { filePath: path.join(dir, 'patches/ms@2.1.3.patch'), physical: true }, + { filePath: path.join(dir, 'patches/ee-first@1.1.1.patch'), physical: true }, + { filePath: path.join(dir, 'packages/m/package.json'), physical: false, content: FAUX_MANIFEST }, + ) + return bundler + } + + const readArchive = async (archiveFile: string): Promise> => { + const contents = new Map() + await list({ file: archiveFile, onReadEntry: entry => { + const chunks: Buffer[] = [] + entry.on('data', chunk => chunks.push(chunk as Buffer)) + entry.on('end', () => contents.set(entry.path, Buffer.concat(chunks).toString('utf8'))) + entry.resume() + } }) + return contents + } + + it('drops the declaration, patch file and lockfile entry of a patch that no longer applies', async () => { + const bundler = await makeBundler() + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + expect([...contents.keys()]).toContain('patches/ms@2.1.3.patch') + expect([...contents.keys()]).not.toContain('patches/ee-first@1.1.1.patch') + + const config = contents.get('pnpm-workspace.yaml')! + expect(config).toContain('ms@2.1.3: patches/ms@2.1.3.patch') + expect(config).not.toContain('ee-first') + // Unrelated settings must survive the rewrite untouched. + expect(config).toContain('minimumReleaseAge: 2880') + + const lockfile = contents.get('pnpm-lock.yaml')! + expect(lockfile).toContain(`ms@2.1.3:`) + expect(lockfile).not.toContain('ee-first') + expect(lockfile).toContain(`patch_hash=${MS_HASH}`) + + // A successful filter leaves config and lockfile agreeing, so there is + // nothing to report. + expect(stderrWrites.join('')).toEqual('') + }) + + it('mixes the filtered lockfile into the cache hash', async () => { + const bundler = await makeBundler() + const archive = await bundler.finalize() + const shipped = (await readArchive(archive.archiveFile)).get('pnpm-lock.yaml')! + + const hashFor = async (lockfileContent: string) => + composeWorkspaceCacheHash(await loadWorkspaceCacheHashInputs(makeWorkspace()), { + embeddedPackages: undefined, + fauxPackageJsons: [ + { path: 'packages/m/package.json', raw: Buffer.from(FAUX_MANIFEST, 'utf8') }, + ], + prunedLockfile: { + name: 'pnpm-lock.yaml', + hash: createHash('sha256').update(lockfileContent).digest(), + }, + }) + + expect(bundler.cacheHash.toJSON()).toEqual(await hashFor(shipped)) + // The unfiltered pruned lockfile must not produce the same key, or a + // bundle would reuse a dependency cache built from different patches. + expect(bundler.cacheHash.toJSON()).not.toEqual(await hashFor(prunedLockfile())) + }) + + it('leaves the bundle alone when both config sites declare patches', async () => { + // pnpm picks one site and ignores the other wholesale, and which one wins + // depends on the major, so neither can be edited safely. + await fs.writeFile(path.join(dir, 'package.json'), JSON.stringify({ + name: 'patch-fixture-root', + private: true, + pnpm: { patchedDependencies: { 'ms@2.1.3': 'patches/ms@2.1.3.patch' } }, + })) + + const bundler = await makeBundler() + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + expect([...contents.keys()]).toContain('patches/ee-first@1.1.1.patch') + expect(contents.get('pnpm-workspace.yaml')).toContain('ee-first') + expect(contents.get('pnpm-lock.yaml')).toContain('ee-first') + expect(stderrWrites.join('')).toEqual('') + }) + + it('keeps a declaration the original lockfile never recorded, without reporting it', async () => { + // A pnpm that does not read the declaration site records nothing, which is + // no evidence that the patch is unused. + await fs.writeFile(path.join(dir, 'pnpm-lock.yaml'), [ + `lockfileVersion: '9.0'`, + ``, + `importers:`, + ``, + ` .: {}`, + ``, + ` packages/m: {}`, + ``, + ].join('\n')) + await fs.writeFile(path.join(dir, 'pruned-lock.yaml'), prunedLockfile()) + + const bundler = await makeBundler() + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + expect([...contents.keys()]).toContain('patches/ee-first@1.1.1.patch') + expect(contents.get('pnpm-workspace.yaml')).toContain('ee-first') + expect(stderrWrites.join('')).toEqual('') + }) + + it('reports a declaration the shipped lockfile does not record when it cannot repair it', async () => { + // The prune drops the section outright (a pnpm that does not read the + // declaration site) AND a second site declares patches, so the filtering + // declines and the mismatch survives into the bundle. + await fs.writeFile(path.join(dir, 'package.json'), JSON.stringify({ + name: 'patch-fixture-root', + private: true, + pnpm: { patchedDependencies: { 'ms@2.1.3': 'patches/ms@2.1.3.patch' } }, + })) + const bundler = await makeBundler() + await fs.writeFile(path.join(dir, 'pruned-lock.yaml'), [ + `lockfileVersion: '9.0'`, + ``, + `importers:`, + ``, + ` .: {}`, + ``, + ` packages/m: {}`, + ``, + ].join('\n')) + + await bundler.finalize() + + expect(stderrWrites.join('')).toContain('declares patches that the bundled lockfile does not record') + expect(stderrWrites.join('')).toContain('ee-first@1.1.1') + expect(stderrWrites.join('')).toContain('ms@2.1.3') + }) + + it('filters the root package.json when that is the declaring site', async () => { + // The YAML and JSON rewrite paths are different code; only this one puts a + // rewritten manifest into the cache hash as a faux package.json. + await fs.writeFile(path.join(dir, 'pnpm-workspace.yaml'), 'packages:\n - packages/*\n') + const manifest = { + name: 'patch-fixture-root', + private: true, + pnpm: { + patchedDependencies: { + 'ms@2.1.3': 'patches/ms@2.1.3.patch', + 'ee-first@1.1.1': 'patches/ee-first@1.1.1.patch', + }, + }, + } + await fs.writeFile(path.join(dir, 'package.json'), JSON.stringify(manifest, null, 2)) + + const bundler = await makeBundler() + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + expect(JSON.parse(contents.get('package.json')!).pnpm.patchedDependencies) + .toEqual({ 'ms@2.1.3': 'patches/ms@2.1.3.patch' }) + expect([...contents.keys()]).not.toContain('patches/ee-first@1.1.1.patch') + expect(contents.get('pnpm-lock.yaml')).not.toContain('ee-first') + + // The rewritten manifest is what ships, so it is what the dependency cache + // key must be computed from — canonicalized like any on-disk manifest, + // since it has an on-disk original. + const expected = composeWorkspaceCacheHash(await loadWorkspaceCacheHashInputs(makeWorkspace()), { + embeddedPackages: undefined, + fauxPackageJsons: [ + { + path: 'package.json', + raw: canonicalizePackageJson( + Buffer.from(contents.get('package.json')!, 'utf8'), + PACKAGE_JSON_EXCLUDED_FIELDS, + ), + }, + { path: 'packages/m/package.json', raw: Buffer.from(FAUX_MANIFEST, 'utf8') }, + ], + prunedLockfile: { + name: 'pnpm-lock.yaml', + hash: createHash('sha256').update(contents.get('pnpm-lock.yaml')!).digest(), + }, + }) + expect(bundler.cacheHash.toJSON()).toEqual(expected) + }) + + it('does not change the cache key when only the root manifest version is bumped', async () => { + // The rewritten manifest ships as a synthesized file, and hashing those + // verbatim would make a release bump alone discard the runner's dependency + // cache even though no install input changed. + const manifest = (version: string) => JSON.stringify({ + name: 'patch-fixture-root', + version, + private: true, + pnpm: { + patchedDependencies: { + 'ms@2.1.3': 'patches/ms@2.1.3.patch', + 'ee-first@1.1.1': 'patches/ee-first@1.1.1.patch', + }, + }, + }, null, 2) + await fs.writeFile(path.join(dir, 'pnpm-workspace.yaml'), 'packages:\n - packages/*\n') + + await fs.writeFile(path.join(dir, 'package.json'), manifest('1.0.0')) + const before = await makeBundler() + await before.finalize() + + await fs.writeFile(path.join(dir, 'package.json'), manifest('1.0.1')) + const after = await makeBundler() + await after.finalize() + + expect(after.cacheHash.toJSON()).toEqual(before.cacheHash.toJSON()) + }) + + it('never deletes a bundled file outside the conventional patches directory', async () => { + // A declared patch path can name any bundled file. Here the dropped + // declaration points at a `.patch` fixture that a check's own include glob + // put in the bundle; removing it would take content the check needs. + await fs.mkdir(path.join(dir, 'fixtures'), { recursive: true }) + await fs.writeFile(path.join(dir, 'fixtures/ee.patch'), 'fixture content\n') + await fs.writeFile(path.join(dir, 'pnpm-workspace.yaml'), [ + `packages:`, + ` - packages/*`, + `patchedDependencies:`, + ` ms@2.1.3: patches/ms@2.1.3.patch`, + ` ee-first@1.1.1: fixtures/ee.patch`, + ``, + ].join('\n')) + + const bundler = await makeBundler() + bundler.registerFiles({ filePath: path.join(dir, 'fixtures/ee.patch'), physical: true }) + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + // The declaration still goes; only the file survives. + expect(contents.get('pnpm-workspace.yaml')).not.toContain('ee-first') + expect([...contents.keys()]).toContain('fixtures/ee.patch') + expect(contents.get('fixtures/ee.patch')).toEqual('fixture content\n') + }) + + it('leaves patches alone when the bundled config is archived at another path', async () => { + // A config reached through a symlink is bundled at the link's path while + // its filePath points elsewhere. A virtual replacement takes its archive + // name from filePath, so it would land somewhere else than the entry it + // replaces — the step declines rather than move it. + await fs.mkdir(path.join(dir, 'elsewhere'), { recursive: true }) + await fs.writeFile(path.join(dir, 'elsewhere/pnpm-workspace.yaml'), workspaceYaml()) + + const bundler = await makeBundler() + bundler.registerFiles({ + filePath: path.join(dir, 'elsewhere/pnpm-workspace.yaml'), + physical: true, + archivePath: 'pnpm-workspace.yaml', + }) + + const archive = await bundler.finalize() + const contents = await readArchive(archive.archiveFile) + + expect([...contents.keys()]).toContain('patches/ee-first@1.1.1.patch') + expect(contents.get('pnpm-lock.yaml')).toContain('ee-first') + }) +}) + +// The tests above stub the prune. This one runs the real thing end to end: +// real pnpm regenerates the lockfile, and the filtering decides from what it +// actually wrote — the only place the flag, the prune and the filtering are +// exercised together. +describe('Bundler.finalize() patch filtering with real pnpm', () => { + const FIXTURE_ROOT = path.join(__dirname, 'lockfile-pruner-fixtures', 'pnpm-patched-workspace') + + let dir: string + let stderrWrites: string[] + + beforeEach(async () => { + dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'checkly-bundler-realpatch-'))) + await fs.cp(FIXTURE_ROOT, dir, { recursive: true }) + stderrWrites = [] + vi.spyOn(process.stderr, 'write').mockImplementation(chunk => { + const text = String(chunk) + if (!text.includes('checkly:cli:')) { + stderrWrites.push(text) + } + return true + }) + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + }) + + afterEach(async () => { + vi.restoreAllMocks() + vi.unstubAllEnvs() + await fs.rm(dir, { recursive: true, force: true }) + }) + + it('drops the patch the pruned workspace no longer applies, and keeps the one it does', async () => { + const member = (name: string) => + new Package({ name: `@fixture/${name}`, path: path.join(dir, 'packages', name), version: '1.0.0' }) + + const bundler = await Bundler.createForWorkspace(new Workspace({ + root: new Package({ name: 'lockfile-pruner-fixture', path: dir }), + packages: [member('used'), member('shimmed'), member('absent')], + lockfile: Ok(path.join(dir, 'pnpm-lock.yaml')), + configFile: Ok(path.join(dir, 'pnpm-workspace.yaml')), + }), { + tempDir: path.join(dir, 'out'), + packageManager: new PNpmDetector(), + }) + + // A partial-workspace bundle: `used` ships for real, `shimmed` as a + // dependency-free placeholder, `absent` not at all — so `ee-first`, and + // with it the patch on it, falls out of the dependency graph. + bundler.registerFiles( + { filePath: path.join(dir, 'package.json'), physical: true }, + { filePath: path.join(dir, 'pnpm-workspace.yaml'), physical: true }, + { filePath: path.join(dir, 'pnpm-lock.yaml'), physical: true }, + { filePath: path.join(dir, 'patches/ms@2.1.3.patch'), physical: true }, + { filePath: path.join(dir, 'patches/ee-first@1.1.1.patch'), physical: true }, + { filePath: path.join(dir, 'packages/used/package.json'), physical: true }, + { + filePath: path.join(dir, 'packages/shimmed/package.json'), + physical: false, + content: '{"name":"@fixture/shimmed","version":"1.0.0"}', + }, + ) + + const archive = await bundler.finalize() + + const contents = new Map() + await list({ file: archive.archiveFile, onReadEntry: entry => { + const chunks: Buffer[] = [] + entry.on('data', chunk => chunks.push(chunk as Buffer)) + entry.on('end', () => contents.set(entry.path, Buffer.concat(chunks).toString('utf8'))) + entry.resume() + } }) + + expect([...contents.keys()]).toContain('patches/ms@2.1.3.patch') + expect([...contents.keys()]).not.toContain('patches/ee-first@1.1.1.patch') + expect(contents.get('pnpm-workspace.yaml')).toContain('ms@2.1.3') + expect(contents.get('pnpm-workspace.yaml')).not.toContain('ee-first') + expect(contents.get('pnpm-lock.yaml')).toContain('ms@2.1.3') + expect(contents.get('pnpm-lock.yaml')).not.toContain('ee-first') + expect(stderrWrites.join('')).toEqual('') + + // Only the bundled COPY is rewritten. The user's own config, lockfile and + // patch files are inputs to bundling and must come out of it untouched. + expect(await fs.readFile(path.join(dir, 'pnpm-workspace.yaml'), 'utf8')) + .toEqual(await fs.readFile(path.join(FIXTURE_ROOT, 'pnpm-workspace.yaml'), 'utf8')) + expect(await fs.readFile(path.join(dir, 'pnpm-lock.yaml'), 'utf8')) + .toEqual(await fs.readFile(path.join(FIXTURE_ROOT, 'pnpm-lock.yaml'), 'utf8')) + expect(await fs.readFile(path.join(dir, 'patches/ee-first@1.1.1.patch'), 'utf8')) + .toEqual(await fs.readFile(path.join(FIXTURE_ROOT, 'patches/ee-first@1.1.1.patch'), 'utf8')) + }, 60_000) +}) diff --git a/packages/cli/src/services/check-parser/bundler.ts b/packages/cli/src/services/check-parser/bundler.ts index e53d6518..a6951899 100644 --- a/packages/cli/src/services/check-parser/bundler.ts +++ b/packages/cli/src/services/check-parser/bundler.ts @@ -21,10 +21,20 @@ import { ComputeWorkspaceCacheHashOptions, EmbeddedPackageInput, FauxPackageJsonInput, + canonicalizePackageJson, loadWorkspaceCacheHashInputs, LockfileInput, + PACKAGE_JSON_EXCLUDED_FIELDS, } from './cache-hash.js' import { pruneBundledLockfile } from './lockfile-pruner.js' +import { + findUnrepairedPatchKeys, + isRemovablePatchPath, + PatchConfigFile, + PatchConfigKind, + PatchFilterPlan, + planPatchFilter, +} from './patched-dependencies.js' import { PackageManager } from './package-files/package-manager.js' import { File } from './parser.js' import { Workspace } from './package-files/workspace.js' @@ -32,6 +42,13 @@ import { pathToPosix } from '../util.js' const debug = Debug('checkly:cli:services:check-parser:bundler') +/** + * The files pnpm accepts `patchedDependencies` in, which are also their + * archive paths: both live at the workspace root, and the workspace root is + * the bundle's strip prefix (see {@link Bundler.createForWorkspace}). + */ +const PATCH_CONFIG_KINDS: PatchConfigKind[] = ['pnpm-workspace.yaml', 'package.json'] + /** * Where a file goes in the archive. A file usually lands at its own path * relative to the bundle root, but a file bundled at the path of a symlink that @@ -488,6 +505,13 @@ interface PrunedLockfile extends LockfileInput { * hash, which only consumes the name and hash. */ content: string + /** Where the lockfile lives in the archive. */ + archivePath: string + /** + * The lockfile as it was before pruning, carried through for the patch + * filtering (see {@link Bundler.dropUnusedPatches}). + */ + originalContent: string } /** @@ -558,6 +582,14 @@ export class Bundler { #stripPrefix?: string #workspaceContext?: WorkspaceBundleContext #files = new Map() + /** + * Archive paths of manifests the patch filtering rewrote. Unlike a + * synthesized member manifest, a rewritten manifest has an on-disk original, + * so it is hashed the way on-disk manifests are — with `version` stripped — + * rather than verbatim. Hashing the raw bytes would make a release bump + * alone change the dependency cache key even though no install input did. + */ + #patchRewrittenManifests = new Set() private constructor (options: BundlerOptions) { const { @@ -692,7 +724,7 @@ export class Bundler { return } - const pruned = await this.#pruneLockfile(context) + const pruned = await this.#dropUnusedPatches(context, await this.#pruneLockfile(context)) const embeddedPackages = await this.#materializeEmbeddedPackages(context, pruned) const fauxPackageJsons: FauxPackageJsonInput[] = [] @@ -700,7 +732,13 @@ export class Bundler { if (file.physical || path.posix.basename(archivePath) !== 'package.json') { continue } - fauxPackageJsons.push({ path: archivePath, raw: Buffer.from(file.content, 'utf8') }) + const raw = Buffer.from(file.content, 'utf8') + fauxPackageJsons.push({ + path: archivePath, + raw: this.#patchRewrittenManifests.has(archivePath) + ? canonicalizePackageJson(raw, PACKAGE_JSON_EXCLUDED_FIELDS) + : raw, + }) } // Unconditional: with no faux manifests, no pruned lockfile and an @@ -772,6 +810,194 @@ export class Bundler { return kept } + /** + * Filters the bundle's pnpm patch declarations down to the ones the pruned + * lockfile shows still apply, dropping the matching patch files and lockfile + * entries with them. + * + * A bundle carries the workspace's whole `patchedDependencies` map but only + * a subset of its members, so a patch whose package belongs to an unbundled + * member ends up applying to nothing. pnpm rejects that outright when it + * re-resolves, which is why the prune install tolerates it (see + * PNpmDetector.lockfileOnlyInstallCommand) and the leftovers are cleaned up + * here instead — the pruned lockfile is the first point at which the bundle's + * real dependency graph is known. + * + * Every failure mode leaves the bundle exactly as pruning produced it, which + * installs correctly today; the reporting below covers the case where that + * fallback is nonetheless a bundle the runner would reject. + */ + async #dropUnusedPatches ( + context: WorkspaceBundleContext, + pruned: PrunedLockfile | undefined, + ): Promise { + if (pruned === undefined || context.packageManager.name !== 'pnpm') { + return pruned + } + + let configs: PatchConfigFile[] = [] + let result = pruned + try { + // `replaceable` is about whether the filtering may edit these files; + // whatever could be READ still feeds the diagnostic below, so a bundle + // this step declines to touch is not also a bundle it stays quiet about. + const read = await this.#readPatchConfigs() + configs = read.configs + if (read.replaceable) { + const plan = planPatchFilter({ + configs, + originalLockfileContent: pruned.originalContent, + prunedLockfileContent: pruned.content, + }) + if (plan !== undefined) { + const applied = this.#applyPatchFilter(context, pruned, plan) + if (applied !== undefined) { + result = applied + // The diagnostic below must see what actually ships, so the config + // is swapped for its rewritten bytes only once the apply took. + configs = configs.map(config => config.archivePath === plan.rewrittenConfig.archivePath + ? { ...config, content: plan.rewrittenConfig.content } + : config) + debug(`Dropped unused patch declarations: ${plan.unusedKeys.join(', ')}`) + } + } + } + } catch (err) { + debug(`Could not filter the bundle's patch declarations: ${err}`) + } + + // Evaluated on every path, including the ones that changed nothing: a + // config declaring a patch the shipped lockfile does not record is what + // makes the runner's install fail, and silently shipping that is the + // failure this step exists to prevent. + if (configs.length > 0) { + const unrepaired = findUnrepairedPatchKeys({ + configs, + originalLockfileContent: pruned.originalContent, + shippedLockfileContent: result.content, + }) + if (unrepaired.length > 0) { + process.stderr.write( + `Note: the bundled pnpm config declares patches that the bundled lockfile does not ` + + `record (${unrepaired.join(', ')}), which can fail the remote install. Your own lockfile ` + + `does record them, so run your package manager's install to refresh it rather than ` + + `removing the entries; set CHECKLY_LOCKFILE_PRUNE=0 to opt out of pruning entirely.\n`, + ) + } + } + + return result + } + + /** + * Reads the bundle's copies of the two files pnpm accepts + * `patchedDependencies` in. + * + * `replaceable` is false when a candidate exists but cannot be read or + * cannot be swapped for a rewritten copy; filtering one declaring config + * while leaving the other would manufacture the very config/lockfile + * mismatch this step removes. Whatever *could* be read is still returned, so + * the caller can report on a bundle it declines to edit. + */ + async #readPatchConfigs (): Promise<{ configs: PatchConfigFile[], replaceable: boolean }> { + const configs: PatchConfigFile[] = [] + let replaceable = true + + // The archive path and the kind are the same string here; see + // PATCH_CONFIG_KINDS for why. + for (const archivePath of PATCH_CONFIG_KINDS) { + const file = this.#files.get(archivePath) + if (file === undefined) { + continue + } + + // A virtual entry's archive name is always derived from its filePath, + // so a replacement can only stand in for an entry that already archives + // at that derived path. An entry bundled somewhere other than its own + // path — reached through a symlink, and carrying an explicit + // archivePath — cannot be replaced without moving it. + const replacementArchivePath = pathToPosix( + path.relative(this.#stripPrefix ?? '', file.filePath), + ) + if (replacementArchivePath !== archivePath) { + debug(`Bundled ${archivePath} would move to ${replacementArchivePath} if rewritten;` + + ` leaving patches alone`) + replaceable = false + } + + try { + const content = file.physical + ? await fs.readFile(file.filePath, 'utf8') + : file.content + configs.push({ archivePath, kind: archivePath, content }) + } catch (err) { + debug(`Could not read bundled ${archivePath}: ${err}`) + replaceable = false + } + } + + return { configs, replaceable } + } + + /** + * Applies a patch filter plan. Deliberately pure map mutation over + * already-computed content: a throw partway through would ship a bundle + * whose config, patch files and lockfile disagree, which fails the runner's + * install either way it lands. + */ + #applyPatchFilter ( + context: WorkspaceBundleContext, + pruned: PrunedLockfile, + plan: PatchFilterPlan, + ): PrunedLockfile | undefined { + // Resolved before anything is mutated: the config entry is what the plan + // was computed from, and rewriting the lockfile without it would ship the + // mismatch this step exists to remove. + const existing = this.#files.get(plan.rewrittenConfig.archivePath) + if (existing === undefined) { + return undefined + } + + this.#files.set(plan.rewrittenConfig.archivePath, { + // The archive name is derived from filePath, so the rewritten entry has + // to keep the original's. + filePath: existing.filePath, + physical: false, + content: plan.rewrittenConfig.content, + }) + if (path.posix.basename(plan.rewrittenConfig.archivePath) === 'package.json') { + this.#patchRewrittenManifests.add(plan.rewrittenConfig.archivePath) + } + + for (const patchPath of plan.droppedPatchPaths) { + // Defense in depth, as an allow-list rather than a list of things not to + // delete: a declared patch path is user input reaching a delete, and it + // can name any bundled file — an .npmrc carrying registry auth, a + // pnpmfile the lockfile records a checksum for, or a `.patch` fixture + // that a check's own `include` glob put in the bundle. Only the + // conventional location the auto-include adds patches from is removable; + // a patch kept elsewhere simply stays, inert, exactly as an + // unreferenced one does. + if (!isRemovablePatchPath(patchPath)) { + debug(`Refusing to drop ${patchPath}: outside the conventional patches directory`) + continue + } + this.#files.delete(patchPath) + } + + this.#files.set(pruned.archivePath, { + filePath: context.workspace.lockfile.unwrap(), + physical: false, + content: plan.lockfileContent, + }) + + return { + ...pruned, + content: plan.lockfileContent, + hash: createHash('sha256').update(plan.lockfileContent).digest(), + } + } + async #pruneLockfile (context: WorkspaceBundleContext): Promise { const result = await pruneBundledLockfile({ workspace: context.workspace, @@ -824,6 +1050,8 @@ export class Bundler { name: path.posix.basename(result.archivePath), hash: createHash('sha256').update(result.content).digest(), content: result.content, + archivePath: result.archivePath, + originalContent: result.originalContent, } } diff --git a/packages/cli/src/services/check-parser/cache-hash.ts b/packages/cli/src/services/check-parser/cache-hash.ts index bf941738..f5897c7e 100644 --- a/packages/cli/src/services/check-parser/cache-hash.ts +++ b/packages/cli/src/services/check-parser/cache-hash.ts @@ -106,7 +106,8 @@ export interface ComposeCacheHashInput { dependencyCacheVersion?: string /** * Every synthesized (non-physical) `package.json` actually shipped in the - * bundle — in practice the faux workspace member manifests. Unlike + * bundle: the faux workspace member manifests, and a root manifest rewritten + * to drop patch declarations the bundle no longer needs. Unlike * on-disk manifests — whose `version` is excluded because the pinned * lockfile absorbs it — a synthesized manifest's full content including * its version is load-bearing for the remote install (it decides whether @@ -124,7 +125,12 @@ export interface ComposeCacheHashInput { prunedLockfile?: LockfileInput } -const PACKAGE_JSON_EXCLUDED_FIELDS = ['version'] +/** + * Fields stripped from a package.json before hashing. `version` is excluded + * because the pinned lockfile already absorbs it, so a release bump must not + * invalidate a dependency cache whose install inputs are unchanged. + */ +export const PACKAGE_JSON_EXCLUDED_FIELDS = ['version'] /** * Encodes a value as JSON in a way that's stable across runs and machines. diff --git a/packages/cli/src/services/check-parser/lockfile-pruner.ts b/packages/cli/src/services/check-parser/lockfile-pruner.ts index 00a61ba9..2f1b8da4 100644 --- a/packages/cli/src/services/check-parser/lockfile-pruner.ts +++ b/packages/cli/src/services/check-parser/lockfile-pruner.ts @@ -37,6 +37,13 @@ export type PruneBundledLockfileResult = archivePath: string /** The regenerated lockfile contents. */ content: string + /** + * The workspace's lockfile as it was before pruning. Callers that compare + * the two — the patch filtering does, to tell a declaration pnpm dropped + * from one it never read — get the exact bytes this prune was computed + * from rather than re-reading a file that may since have changed. + */ + originalContent: string /** * Faux manifests synthesized for workspace members that bundled * manifests reference as links but that have no manifest in the bundle @@ -1287,6 +1294,7 @@ export async function pruneBundledLockfile ( status: 'pruned', archivePath: decision.lockfileArchivePath, content: regeneratedContent, + originalContent, backfilledManifests: Array.from(backfill.manifests.values()), } } catch (err) { diff --git a/packages/cli/src/services/check-parser/patched-dependencies.ts b/packages/cli/src/services/check-parser/patched-dependencies.ts index c82502fc..6478f4ea 100644 --- a/packages/cli/src/services/check-parser/patched-dependencies.ts +++ b/packages/cli/src/services/check-parser/patched-dependencies.ts @@ -20,6 +20,24 @@ const YAML_STRINGIFY_OPTIONS = { lineWidth: 0, flowCollectionPadding: false } as const PATCHED_DEPENDENCIES = 'patchedDependencies' +/** + * The directory pnpm's own `pnpm patch-commit` writes patches to, relative to + * the workspace root. Shared so that the auto-include that bundles patches and + * the filtering that removes them cannot drift apart: widening one without the + * other would silently leave orphaned patch files in every bundle. + */ +export const PNPM_PATCHES_DIR = 'patches' + +/** + * Whether an archive path names a patch file the filtering may remove from a + * bundle: a `.patch` directly under {@link PNPM_PATCHES_DIR}. A declaration + * may point anywhere, including at a file bundled for an unrelated reason, so + * only the conventional location is removable. + */ +export function isRemovablePatchPath (archivePath: string): boolean { + return new RegExp(`^${PNPM_PATCHES_DIR}/[^/]+\\.patch$`).test(archivePath) +} + /** * Where a pnpm project can declare `patchedDependencies`. Both are live: * pnpm 10 reads the `package.json` field (and prefers it over diff --git a/packages/cli/src/services/playwright-project-bundler.ts b/packages/cli/src/services/playwright-project-bundler.ts index a373d1d5..45c649c2 100644 --- a/packages/cli/src/services/playwright-project-bundler.ts +++ b/packages/cli/src/services/playwright-project-bundler.ts @@ -4,6 +4,7 @@ import path from 'node:path' import semver from 'semver' +import { PNPM_PATCHES_DIR } from './check-parser/patched-dependencies.js' import { File } from './check-parser/parser.js' import { detectNearestPackageJson, PackageManager } from './check-parser/package-files/package-manager.js' import { PackageJsonFile } from './check-parser/package-files/package-json-file.js' @@ -186,8 +187,8 @@ export function getAutoIncludes ( // for Yarn Berry's patch: protocol. A patch kept at a nonconventional // path makes the pruner fail closed with a warning instead. const patchesDirByManager: Record = { - pnpm: ['patches'], - bun: ['patches'], + pnpm: [PNPM_PATCHES_DIR], + bun: [PNPM_PATCHES_DIR], yarn: ['.yarn', 'patches'], } const patchesSegments = patchesDirByManager[packageManager.name]