diff --git a/CLAUDE.md b/CLAUDE.md index 2b4df6ecf..a76f87b5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,3 +120,4 @@ Source in `src/ai-context/`, built during `prepare`. Generates examples from fix - `CHECKLY_CACHE_DIR` — override the CLI's cache directory (embedded-package tarball downloads) - `CHECKLY_SKIP_NODE_VERSION_CHECK` — set to `1` to bypass the bin's hard Node version preflight (unsupported Node may then fail in unexpected ways); must be set in the shell environment — the preflight runs before `.env` is loaded - `CHECKLY_LOCKFILE_PRUNE` — set to `0` to disable pruning the bundled lockfile to the code bundle's contents; when a lockfile is bundled this also disables `bundle.packages.prune` (the manifest rewrite rolls back) +- `CHECKLY_LOCKFILE_PRUNE_TIMEOUT` — lockfile prune time budget in seconds (default 30; `0` disables the timeout); invalid values are ignored 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 f3a60a0f0..515f0dd54 100644 --- a/packages/cli/src/ai-context/references/configure-playwright-checks.md +++ b/packages/cli/src/ai-context/references/configure-playwright-checks.md @@ -16,8 +16,8 @@ - For pnpm projects, the workspace-root pnpmfile (`.pnpmfile.cjs` / `.pnpmfile.mjs`) is bundled automatically so the remote install can reproduce the lockfile — but only when the lockfile records a pnpmfile checksum (run `pnpm install` with the pnpmfile in place to record one; without it the pnpmfile is silently not bundled). The pnpmfile must also be self-contained: it may only load side-effect-free Node.js builtins (such as `path`, `crypto`, `util`) via literal `require('...')`/`import` specifiers, and must not reference `process`, `__dirname`, `import.meta`, or dynamically computed module paths. A pnpmfile that is not self-contained is skipped with a warning, and the remote install may re-resolve dependencies instead of using the lockfile. The pnpmfile is uploaded, so avoid embedding secrets in it. - In a workspace (monorepo) whose code bundle covers only part of the workspace, the bundled lockfile is pruned automatically so the remote install only fetches what the bundle needs, instead of failing on dependencies of workspace members that were left out. - Supported for `pnpm-lock.yaml` (v6/9), `package-lock.json` (v2/3), the text `bun.lock`, and Yarn Berry `yarn.lock`. Yarn Classic v1 lockfiles are not supported; for bun's binary `bun.lockb`, regenerate a text lockfile with `bun install --save-text-lockfile`. - - The workspace's package manager binary must be available on the machine running the CLI. - - When pruning was needed but cannot run or cannot verify its result, the original lockfile ships unchanged and the CLI prints a note explaining why — follow it. Skips are silent when there is nothing to prune, or when pruning is disabled via `CHECKLY_LOCKFILE_PRUNE=0`; silent skip reasons and per-entry embed match details are visible under `DEBUG='checkly:cli:services:check-parser:*'` and `DEBUG='checkly:cli:services:embedded-packages'`. + - The workspace's package manager binary must be available on the machine running the CLI. For pnpm and npm, pruning reuses cached registry metadata and only contacts the registry for packages missing from the cache; yarn Berry and bun regenerate offline. + - When pruning was needed but cannot run or cannot verify its result, the original lockfile ships unchanged and the CLI prints a note explaining why — follow it (a timed-out prune's note names `CHECKLY_LOCKFILE_PRUNE_TIMEOUT=`, which raises the default 30 s budget). Skips are silent when there is nothing to prune, or when pruning is disabled via `CHECKLY_LOCKFILE_PRUNE=0`; silent skip reasons, a timed-out prune's partial package manager output, and per-entry embed match details are visible under `DEBUG='checkly:cli:services:check-parser:*'` and `DEBUG='checkly:cli:services:embedded-packages'`. - For pnpm projects with `patchedDependencies`, patches belonging to unbundled workspace members are filtered out automatically. A note naming stale patch declarations means the lockfile is out of date with the config — refresh it with a regular install. - Checkly caches installed dependencies between runs, keyed off the workspace's dependency inputs (the lock file, every workspace member's `package.json` and `.npmrc` — whether or not the member is in the bundle) plus the bundle's own install inputs, so the key can change without a file edit, e.g. when a different set of workspace members lands in the bundle. To force a reinstall for deployed checks, set `runner.cache.install.version` (a string or a safe integer) at the top level of `checkly.config.ts` — not per check — and change its value whenever the cache should be invalidated; deployed, scheduled checks pick up the change on the next `checkly deploy`. If the config still declares the deprecated `caching.dependencyCache.version`, remove it — setting both fails config loading. Unset or empty 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 on `checkly test`, `checkly pw-test`, `checkly trigger`, or `checkly checks run` instead. - 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'] } }`. The CLI resolves the entries against the workspace-root lockfile (Yarn Classic v1 lockfiles are not supported), downloads their tarballs, verifies each tarball's integrity (recorded in the lockfile, or fetched from registry metadata for Yarn Berry), and ships them inside the code bundle so the runner can install them without reaching the registry. 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 e977521a0..1f146771b 100644 --- a/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts +++ b/packages/cli/src/services/check-parser/__tests__/bundler.spec.ts @@ -297,6 +297,7 @@ describe('Bundler.finalize() lockfile prune reporting', () => { return true }) vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE_TIMEOUT', '') }) afterEach(async () => { @@ -477,6 +478,7 @@ describe('Bundler.finalize() embedded package materialization', () => { return true }) vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE_TIMEOUT', '') }) afterEach(async () => { @@ -806,6 +808,7 @@ describe('Bundler.finalize() patch filtering', () => { return true }) vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE_TIMEOUT', '') }) afterEach(async () => { @@ -1189,6 +1192,7 @@ describe('Bundler.finalize() package pruning', () => { return true }) vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE_TIMEOUT', '') }) afterEach(async () => { @@ -1910,6 +1914,7 @@ describe('Bundler.finalize() patch filtering with real pnpm', () => { return true }) vi.stubEnv('CHECKLY_LOCKFILE_PRUNE', '') + vi.stubEnv('CHECKLY_LOCKFILE_PRUNE_TIMEOUT', '') }) afterEach(async () => { 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 80e927bc9..8c6227123 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 @@ -3,18 +3,23 @@ import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' +import Debug from 'debug' import { describe, it, expect, afterAll, vi } from 'vitest' import { pruneBundledLockfile, selectMaterializationEntries, shouldPruneLockfile, + redactDetail, + PruneBundledLockfileResult, } from '../lockfile-pruner.js' import { createFauxPackageFiles } from '../faux-package.js' +import { UNPRINTABLE_URL } from '../../embedded-packages/diagnostics.js' import { BunDetector, NpmDetector, PackageManager, PNpmDetector, Runnable, YarnDetector } from '../package-files/package-manager.js' import { Package, Workspace } from '../package-files/workspace.js' import { Err, Ok } from '../package-files/result.js' import { File } from '../parser.js' +import { captureStderr } from '../../../testing/capture-stderr.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 @@ -83,9 +88,13 @@ const rewriteBunLockScript = (...replacements: Array<[string, string]>): string const rewriteYarnLockScript = (...replacements: Array<[string, string]>): string => rewriteLockfileScript('yarn.lock', ...replacements) -// Ambient values (particularly CHECKLY_LOCKFILE_PRUNE) must not leak into -// test outcomes. -const testEnv = (): NodeJS.ProcessEnv => ({ ...process.env, CHECKLY_LOCKFILE_PRUNE: undefined }) +// Ambient values (CHECKLY_LOCKFILE_PRUNE and its timeout override) must not +// leak into test outcomes. +const testEnv = (): NodeJS.ProcessEnv => ({ + ...process.env, + CHECKLY_LOCKFILE_PRUNE: undefined, + CHECKLY_LOCKFILE_PRUNE_TIMEOUT: undefined, +}) // A PackageManager whose lockfile-only install command is replaced, for // exercising failure paths without a real package manager. @@ -297,6 +306,20 @@ describe('lockfile-pruner', () => { }) }) + describe('redactDetail()', () => { + it('collapses every URL to scheme and host and drops bare userinfo, leaving plain text alone', () => { + expect(redactDetail('GET https://user:pw@registry.example/secret-token/pkg?token=abc failed')) + .toEqual('GET https://registry.example failed') + expect(redactDetail('see http://localhost:4873/p and https://[bad failed')) + .toEqual(`see http://localhost:4873 and ${UNPRINTABLE_URL} failed`) + expect(redactDetail('HTTPS://user:pw@registry.example/secret-token/pkg')) + .toEqual('https://registry.example') + expect(redactDetail('registry //alice:p@ss@registry.example/ rejected')) + .toEqual('registry //registry.example/ rejected') + expect(redactDetail(' did you mean this? yes ')).toEqual('did you mean this? yes') + }) + }) + describe('pruneBundledLockfile()', () => { it('skips notably for an unsupported package manager even when the lockfile is unreadable', async () => { // Pins the ordering invariant in pruneBundledLockfile: the capability @@ -345,18 +368,100 @@ describe('lockfile-pruner', () => { }) }) - it('fails when the command times out', async () => { + it('fails when the command times out and debug-logs the partial output', async () => { + const { workspace, files } = makePnpmScenario() + // The timeout reason stays a one-liner; the child's partial output is + // only visible on the debug channel, so pin that it gets there. + const previouslyEnabled = Debug.disable() + Debug.enable('checkly:cli:services:check-parser:lockfile-pruner') + try { + let result: PruneBundledLockfileResult | undefined + const written = await captureStderr(async () => { + result = await pruneBundledLockfile({ + workspace, + // The URL travels via the environment (passed through to the + // child) rather than the script text, which the pruner echoes + // in its own "Running ..." debug line. + packageManager: stubPackageManager(new Runnable('node', ['-e', ` + process.stdout.write('x'.repeat(10_000) + '\\nOUTMARK ' + process.env.PRUNE_TEST_URL + '\\n') + process.stderr.write('ERRMARK\\n') + setInterval(() => {}, 1000) + `])), + files, + // Generous: the assertions need the child to have started and + // written before the kill, even on a loaded CI host. + timeoutMs: 2_000, + env: { ...testEnv(), PRUNE_TEST_URL: 'https://user:secret@registry.example/pkg?token=abc' }, + }) + }) + expect(result).toMatchObject({ status: 'failed', reason: expect.stringContaining('timed out') }) + // Assertions do not span the newline after the stream label: with + // DEBUG_COLORS on, debug re-prefixes every line. + const debugOutput = written.join('') + expect(debugOutput).toContain('partial stdout:') + expect(debugOutput).toContain('OUTMARK https://registry.example') + expect(debugOutput).toContain('partial stderr:') + expect(debugOutput).toContain('ERRMARK') + expect(debugOutput).not.toContain('secret') + expect(debugOutput).not.toContain('token=abc') + expect(debugOutput).not.toContain('/pkg') + // Only the tail survives the cap, marked as truncated. + expect(debugOutput).toContain('…') + expect(debugOutput).not.toContain('x'.repeat(10_000)) + } finally { + Debug.enable(previouslyEnabled) + } + }, 30_000) + + it('takes the time budget from CHECKLY_LOCKFILE_PRUNE_TIMEOUT in seconds', async () => { + const { workspace, files } = makePnpmScenario() + const result = await pruneBundledLockfile({ + workspace, + packageManager: stubPackageManager(new Runnable('node', ['-e', 'setInterval(() => {}, 1000)'])), + files, + env: { ...testEnv(), CHECKLY_LOCKFILE_PRUNE_TIMEOUT: '1' }, + }) + expect(result).toMatchObject({ + status: 'failed', + reason: 'node timed out after 1s; set CHECKLY_LOCKFILE_PRUNE_TIMEOUT= to raise it', + }) + }, 30_000) + + it('lets an explicit timeout option outrank CHECKLY_LOCKFILE_PRUNE_TIMEOUT', async () => { const { workspace, files } = makePnpmScenario() const result = await pruneBundledLockfile({ workspace, packageManager: stubPackageManager(new Runnable('node', ['-e', 'setInterval(() => {}, 1000)'])), files, timeoutMs: 500, - env: testEnv(), + env: { ...testEnv(), CHECKLY_LOCKFILE_PRUNE_TIMEOUT: '60' }, }) - expect(result).toMatchObject({ status: 'failed', reason: expect.stringContaining('timed out') }) + expect(result).toMatchObject({ status: 'failed', reason: expect.stringContaining('timed out after 500ms') }) }, 30_000) + it.each(['abc', '-5', '1.5', '2147484'])('ignores an unusable CHECKLY_LOCKFILE_PRUNE_TIMEOUT (%j)', async value => { + const { workspace, files } = makePnpmScenario() + // The stub exits at once, so the rejection is only observable on the + // debug channel; the outcome shows the parse did not fail the prune. + const previouslyEnabled = Debug.disable() + Debug.enable('checkly:cli:services:check-parser:lockfile-pruner') + try { + let result: PruneBundledLockfileResult | undefined + const written = await captureStderr(async () => { + result = await pruneBundledLockfile({ + workspace, + packageManager: stubPackageManager(new Runnable('node', ['-e', ''])), + files, + env: { ...testEnv(), CHECKLY_LOCKFILE_PRUNE_TIMEOUT: value }, + }) + }) + expect(result?.status).not.toEqual('failed') + expect(written.join('')).toContain(`Ignoring CHECKLY_LOCKFILE_PRUNE_TIMEOUT=${JSON.stringify(value)}`) + } finally { + Debug.enable(previouslyEnabled) + } + }) + it('skips when the regenerated lockfile is identical and nothing was backfilled', async () => { const { workspace, files } = makePnpmScenario() // With the absent member's real manifest in the bundle there is @@ -618,7 +723,7 @@ describe('lockfile-pruner', () => { // child's output, not in the displayed command line. const scriptPath = path.join(await makeTempDir(), 'fail.cjs') await fs.writeFile(scriptPath, ` - process.stdout.write('GET https://alice:sup3rsecret@registry.example.com/pkg failed ' + 'x'.repeat(600)) + process.stdout.write('GET https://alice:sup3rsecret@registry.example.com/pkg?token=abc failed ' + 'x'.repeat(600)) process.exit(1) `) const result = await pruneBundledLockfile({ @@ -632,7 +737,8 @@ describe('lockfile-pruner', () => { return } expect(result.reason).not.toContain('sup3rsecret') - expect(result.reason).toContain('registry.example.com') + expect(result.reason).not.toContain('token=abc') + expect(result.reason).toContain('https://registry.example.com failed') expect(result.reason).toContain('…') }) diff --git a/packages/cli/src/services/check-parser/lockfile-pruner.ts b/packages/cli/src/services/check-parser/lockfile-pruner.ts index 053f8864b..53126398d 100644 --- a/packages/cli/src/services/check-parser/lockfile-pruner.ts +++ b/packages/cli/src/services/check-parser/lockfile-pruner.ts @@ -14,6 +14,7 @@ import { lineage } from './package-files/walk.js' import { PackageManager, PathLookup } from './package-files/package-manager.js' import { Package, Workspace } from './package-files/workspace.js' import { File, VirtualFile } from './parser.js' +import { redactUrl } from '../embedded-packages/diagnostics.js' import { pathToPosix } from '../util.js' const debug = Debug('checkly:cli:services:check-parser:lockfile-pruner') @@ -95,6 +96,43 @@ const YARN_PROBE_MIN_INSTALL_BUDGET_MS = 1_000 const MAX_FAILURE_DETAIL_LENGTH = 400 +// Cap for the child's partial output logged at debug level when a prune +// times out. Generous enough to reach the stalled request past pnpm's +// progress lines, bounded so a chatty child cannot flood the log. +const MAX_DEBUG_OUTPUT_LENGTH = 8_192 + +const TIMEOUT_HINT = 'set CHECKLY_LOCKFILE_PRUNE_TIMEOUT= to raise it' + +// Node's timers cannot represent a delay beyond 2^31-1 ms and would clamp a +// larger one to 1 ms, turning a "raise the budget" request into an instant +// timeout, so anything past that is rejected as unusable. +const MAX_TIMEOUT_SECONDS = Math.floor(2_147_483_647 / 1000) + +/** + * Reads the prune time budget from CHECKLY_LOCKFILE_PRUNE_TIMEOUT, given in + * whole seconds; `0` disables the timeout (execa's own semantics). Anything + * else (unset, empty, negative, fractional, non-numeric, too large) yields + * undefined so the caller falls back to the default; an unusable value is + * only noted on the debug channel, since it cannot make a prune fail. + */ +function timeoutFromEnv (env: NodeJS.ProcessEnv): number | undefined { + const raw = env.CHECKLY_LOCKFILE_PRUNE_TIMEOUT + if (raw === undefined || raw === '') { + return undefined + } + if (!/^\d+$/.test(raw) || Number(raw) > MAX_TIMEOUT_SECONDS) { + debug(`Ignoring CHECKLY_LOCKFILE_PRUNE_TIMEOUT=${JSON.stringify(raw)}: not a whole number of seconds` + + ` up to ${MAX_TIMEOUT_SECONDS}`) + return undefined + } + return Number(raw) * 1000 +} + +/** Whole seconds where possible, so the reason matches the env var's unit. */ +function formatBudget (timeoutMs: number): string { + return timeoutMs % 1000 === 0 ? `${timeoutMs / 1000}s` : `${timeoutMs}ms` +} + /** * Environment keys that alter the very behavior the prune command pins with * explicit flags. Everything else (registry and auth configuration in @@ -874,16 +912,57 @@ function buildChildEnv (baseEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return env } +/** + * Scrubs credentials from package manager output before it is surfaced + * anywhere. Every http(s) URL is collapsed to scheme and host by + * `redactUrl`: registries carry tokens in userinfo, in the query string and + * in path segments (Gemfury-style `https://host//npm/`), and the host + * is all a stalled-request diagnosis needs. A scheme-less `//user:pw@host` + * (pnpm prints registry keys in that form) loses its userinfo. Redaction is + * by removal only; nothing is reconstructed from the credential-bearing + * parts. + */ +export function redactDetail (detail: string): string { + return detail + .replace(/https?:\/\/\S+/gi, url => redactUrl(url)) + // Greedy up to the last '@' before the path, so an unencoded '@' inside + // the password does not leave its tail behind. + .replace(/\/\/[^/\s]+@/g, '//') + .trim() +} + function sanitizeDetail (detail: string): string { - // Package manager output can embed registry URLs with userinfo credentials - // (pnpm does not redact them); scrub before surfacing anywhere. - const redacted = detail.replace(/\/\/[^/@\s]+@/g, '//').trim() + const redacted = redactDetail(detail) if (redacted.length <= MAX_FAILURE_DETAIL_LENGTH) { return redacted } return `${redacted.slice(0, MAX_FAILURE_DETAIL_LENGTH)}…` } +/** + * Logs a timed-out child's partial stdout/stderr at debug level. The tail + * is kept because the most recent output is where a stalled request shows; + * the user-facing reason stays a one-liner, so this is the only place the + * output survives. + */ +function debugTimedOutChildOutput (label: string, output: { stdout?: string, stderr?: string }): void { + if (!debug.enabled) { + return + } + for (const [stream, text] of [['stdout', output.stdout], ['stderr', output.stderr]] as const) { + const redacted = redactDetail(text ?? '') + if (redacted.length === 0) { + continue + } + const tail = redacted.length <= MAX_DEBUG_OUTPUT_LENGTH + ? redacted + : `…${redacted.slice(-MAX_DEBUG_OUTPUT_LENGTH)}` + // The output goes in as an argument, not the format string, so `%o` + // and `%%` sequences in it are printed verbatim. + debug('%s timed out; partial %s:\n%s', label, stream, tail) + } +} + // Larger files are not plausible manifests; the cap also keeps a scan of a // shared temp root from slurping an arbitrarily large unrelated file. const MAX_ANCESTOR_MANIFEST_BYTES = 4 * 1024 * 1024 @@ -974,9 +1053,11 @@ export async function pruneBundledLockfile ( packageManager, files, rewrittenManifests = new Set(), - timeoutMs = DEFAULT_TIMEOUT_MS, env = process.env, } = options + // An explicit option (tests) outranks the environment; the destructuring + // carries no default so that an omitted option is distinguishable. + const timeoutMs = options.timeoutMs ?? timeoutFromEnv(env) ?? DEFAULT_TIMEOUT_MS const decision = shouldPruneLockfile(workspace, files, env, rewrittenManifests) if (!decision.prune) { @@ -1150,7 +1231,8 @@ export async function pruneBundledLockfile ( if (probe.timedOut) { // The probe consumed the whole budget; spawning the install with // the ~zero remainder would only produce a confusing second kill. - return { status: 'failed', reason: `${runnable.executable} timed out after ${timeoutMs}ms` } + debugTimedOutChildOutput(`${runnable.executable} --version`, probe) + return { status: 'failed', reason: `${runnable.executable} timed out after ${formatBudget(timeoutMs)}; ${TIMEOUT_HINT}` } } if (timeoutMs > 0) { const remaining = timeoutMs - (Date.now() - probeStartedAt) @@ -1161,7 +1243,7 @@ export async function pruneBundledLockfile ( return { status: 'failed', reason: 'provisioning the yarn toolchain used up the prune time budget before the' - + ' lockfile could be regenerated; pre-install yarn or raise the timeout', + + ` lockfile could be regenerated; pre-install yarn or ${TIMEOUT_HINT}`, } } installTimeoutMs = remaining @@ -1218,7 +1300,8 @@ export async function pruneBundledLockfile ( }) if (result.timedOut) { - return { status: 'failed', reason: `${runnable.executable} timed out after ${installTimeoutMs}ms` } + debugTimedOutChildOutput(runnable.executable, result) + return { status: 'failed', reason: `${runnable.executable} timed out after ${formatBudget(installTimeoutMs)}; ${TIMEOUT_HINT}` } } if ((result as any).code === 'ENOENT') { // A spawn ENOENT can also mean the working directory vanished (a temp 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 06ee3e931..dbfdcf6ad 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 @@ -576,20 +576,23 @@ describe('detectNearestConfigFiles', () => { describe('lockfileOnlyInstallCommand', () => { it('regenerates the lockfile without installing for pnpm, pinning the lockfile location,' - + ' tolerating patches that apply to nothing and trusting the lockfile', () => { + + ' tolerating patches that apply to nothing, trusting the lockfile and preferring cached metadata', () => { 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', '--config.trustLockfile=true', + '--prefer-offline', ]) }) - it('regenerates the lockfile without installing for npm', () => { + it('regenerates the lockfile without installing for npm, preferring cached metadata', () => { const runnable = new NpmDetector().lockfileOnlyInstallCommand() expect(runnable?.executable).toEqual('npm') - expect(runnable?.args).toEqual(['install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--no-fund']) + expect(runnable?.args).toEqual([ + 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--no-fund', '--prefer-offline', + ]) }) it('regenerates the lockfile without installing for bun', () => { 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 7c15f4fdf..fe0a791d6 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 @@ -222,7 +222,18 @@ export class NpmDetector extends PackageManagerDetector implements PackageManage lockfileOnlyInstallCommand (): Runnable { // npm has no lockfile-location setting; package-lock.json always lives // next to the package.json npm operates on, so there is nothing to pin. - return new Runnable('npm', ['install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--no-fund']) + // + // --prefer-offline: the pruner's manifests deliberately differ from the + // lockfile, so npm re-resolves rather than taking its up-to-date fast + // path, and by default re-fetches every packument it considers stale. + // With the flag npm accepts cached packuments regardless of age and only + // contacts the registry for misses. A stale packument can at most change + // a fresh resolution, and the pruner's subset verification rejects any + // regenerated entry absent from the original lockfile, so the worst case + // is a fallback to the original lockfile, never a wrong one. + return new Runnable('npm', [ + 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--no-fund', '--prefer-offline', + ]) } addCommand (options: AddCommandOptions): Runnable { @@ -386,13 +397,36 @@ export class PNpmDetector extends PackageManagerDetector implements PackageManag // npm_config_trust_lockfile env var does not disable the verification // on pnpm 11. // - // Verified on pnpm 10 and 11: neither --config. setting reaches the - // regenerated lockfile's `settings` section, so the snapshot - // comparisons below are unaffected. + // --prefer-offline: the pruner's manifests deliberately differ from the + // lockfile (pruned members, faux manifests, a partial importer set), so + // pnpm never takes its "lockfile up to date, resolution skipped" fast + // path. It re-resolves, and for every package it cannot serve from the + // store it needs registry metadata; without the flag pnpm 10 only serves + // that from its metadata cache for exact-version specs, while range and + // catalog specs and optional dependencies are re-fetched on every prune. + // Behind a slow or intercepting proxy one stalled request plus pnpm's + // fetch retries exceeds the prune budget on its own. With the flag pnpm + // uses cached metadata for any spec type and only contacts the registry + // for misses, so a machine that has installed the project needs little + // or no network. The soft flag is deliberate: unlike yarn, which + // regenerates from locked entries and is run with the network blocked, + // pnpm resolves from metadata, and a cold cache (a machine that never + // installed the project) is a legitimate first-prune state that --offline + // would turn into a guaranteed fallback. A stale cached packument can at + // most change a fresh resolution, and the pruner's subset verification + // rejects any regenerated entry absent from the original lockfile, so + // the worst case is a fallback, never a wrong lockfile. + // + // The pruner does not compare the lockfile's `settings` section, so any + // setting pnpm records there ships to the runner unchecked and could + // mismatch the runner's own install configuration. Verified on pnpm 10 + // and 11: neither --config. setting nor --prefer-offline is recorded in + // `settings` (only autoInstallPeers and excludeLinksFromLockfile are). return new Runnable('pnpm', [ 'install', '--lockfile-only', '--ignore-scripts', '--no-frozen-lockfile', '--lockfile-dir', '.', '--config.allowUnusedPatches=true', '--config.trustLockfile=true', + '--prefer-offline', ]) } diff --git a/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts index 12fea932a..f51439d28 100644 --- a/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts +++ b/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts @@ -9,6 +9,7 @@ import Debug from 'debug' import { describe, it, expect, beforeEach, afterEach } from 'vitest' import { EmbeddedPackageError, EmbeddedPackagesMaterializer } from '../materializer.js' +import { captureStderr } from '../../../testing/capture-stderr.js' const fooTarball = Buffer.from('fake tarball content for @acme/foo') const fooIntegrity = `sha512-${createHash('sha512').update(fooTarball).digest('base64')}` @@ -30,21 +31,6 @@ packages: ` } -async function captureStderr (fn: () => Promise): Promise { - const written: string[] = [] - const original = process.stderr.write.bind(process.stderr) - process.stderr.write = ((chunk: string) => { - written.push(String(chunk)) - return true - }) as never - try { - await fn() - } finally { - process.stderr.write = original - } - return written -} - // The lead-in of the warning a configuration gets when it selects no // packages at all. const NOTHING_MATCHED = `No packages matched 'bundle.packages.embed'` diff --git a/packages/cli/src/testing/capture-stderr.ts b/packages/cli/src/testing/capture-stderr.ts new file mode 100644 index 000000000..da287d864 --- /dev/null +++ b/packages/cli/src/testing/capture-stderr.ts @@ -0,0 +1,19 @@ +/** + * Runs `fn` with `process.stderr.write` swapped for a collector and returns + * every chunk written meanwhile. Used to observe output that only goes to + * stderr, such as the `debug` channel, without a TTY. + */ +export async function captureStderr (fn: () => Promise): Promise { + const written: string[] = [] + const original = process.stderr.write.bind(process.stderr) + process.stderr.write = ((chunk: string) => { + written.push(String(chunk)) + return true + }) as never + try { + await fn() + } finally { + process.stderr.write = original + } + return written +}