From 6d3318aa15014356356b725cabb8b0bcdb37ac26 Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Fri, 3 Jul 2026 10:40:20 +0200 Subject: [PATCH] [ENG-3120] Fall back to the installed truth when the lockfile is stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified live on the ENG-3120 Lovable sample app: `npm install -D @patchstack/connect` (the README's own instruction) plants a package-lock.json in a bun-managed project. Lovable's native dependency flow then updates package.json / bun lockfile / node_modules but never that file — and detectLockfile's fixed priority reads package-lock.json first, so every scan and mark-build fingerprint froze at the moment of that npm install. Dependencies added afterwards (including vulnerable ones) would never reach the manifest: a silent false-safe one layer below the parity system. scanLockfile now validates each source against package.json (dependencies + devDependencies, ignoring file:/link:/workspace:/portal: specifiers). A source missing declared dependencies is a fossil: fall through to the next candidate, ultimately walking node_modules/ — the installed truth regardless of package manager — and surface warnings on the new Manifest.warnings field (printed by scan and mark-build). A fully-consistent lockfile stays authoritative, no-package.json behaves exactly as before, and nothing fails harder than today. README: recommend the project's own package manager for installs (bun add -d on Lovable/Bolt) and document the stale-lockfile behavior. --- README.md | 8 ++- src/cli.ts | 6 ++ src/parsers/consistency.ts | 56 ++++++++++++++++ src/parsers/index.ts | 91 +++++++++++++++++++++++++- src/types.ts | 6 ++ tests/parsers.test.ts | 127 ++++++++++++++++++++++++++++++++++++- 6 files changed, 289 insertions(+), 5 deletions(-) create mode 100644 src/parsers/consistency.ts diff --git a/README.md b/README.md index 0881842..ae9f9af 100644 --- a/README.md +++ b/README.md @@ -17,9 +17,11 @@ npm install --save-dev @patchstack/connect npx @patchstack/connect scan ``` +> **Use your project's own package manager.** On bun-managed projects (Lovable, Bolt, most vibe-coding platforms) install with `bun add -d @patchstack/connect` instead — running `npm install` there plants a `package-lock.json` that the platform's native dependency flow never updates again, leaving a stale lockfile next to the live one. The connector detects and works around that (see *Stale lockfiles* below), but not creating the fossil is better. + That's it. The first `scan`: -1. Reads your `package-lock.json`. +1. Reads your lockfile (see *Supported lockfiles*). 2. POSTs the package list to Patchstack with **no** UUID. 3. Patchstack provisions a fresh site and returns its UUID. 4. The connector writes the UUID to `.patchstackrc.json` so the next `scan` targets the same site. @@ -123,6 +125,10 @@ That's the entire payload. No source code, no environment variables, no file pat If both a Bun lockfile and `node_modules/` are present, the connector walks `node_modules/` to enumerate the installed packages. Run `bun install` (or `npm install`) before scanning so the directory is populated. +### Stale lockfiles + +Every scanned source is validated against `package.json`: if the chosen lockfile is missing dependencies that `package.json` declares, it is treated as a fossil (e.g. a `package-lock.json` created by a one-off `npm install` in a bun-managed project) and the connector falls through to the next source — ultimately walking `node_modules/`, the installed truth — and prints a warning naming the stale file. Delete the stale lockfile to silence the warning. Without this, the manifest and the build fingerprint would silently freeze while the real dependency set drifts. + ## Development ```bash diff --git a/src/cli.ts b/src/cli.ts index 165b47c..5833bbf 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -123,6 +123,9 @@ async function runScan(args: ParsedArgs): Promise { cliEndpoint: getStringFlag(args.flags, 'endpoint'), }); const manifest = await scanLockfile(process.cwd()); + for (const warning of manifest.warnings ?? []) { + console.warn(`patchstack: ${warning}`); + } const { payload, stats } = buildWirePayload(manifest); console.log( @@ -211,6 +214,9 @@ async function runMarkBuild(args: ParsedArgs): Promise { let checksum: string | null = null; try { const manifest = await scanLockfile(cwd); + for (const warning of manifest.warnings ?? []) { + console.warn(`mark-build: ${warning}`); + } const { payload } = buildWirePayload(manifest); checksum = computeManifestChecksum(payload.packages); } catch (err) { diff --git a/src/parsers/consistency.ts b/src/parsers/consistency.ts new file mode 100644 index 0000000..fac2bb5 --- /dev/null +++ b/src/parsers/consistency.ts @@ -0,0 +1,56 @@ +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import type { PackageEntry } from '../types.js'; + +/** + * Registry-installable dependency names declared in package.json + * (dependencies + devDependencies). Non-registry specifiers (file:, link:, + * workspace:, portal:) are excluded — lockfile parsers skip those too, so + * their absence from a parsed set says nothing about staleness. Returns [] + * when package.json is missing or unreadable (nothing to validate against). + */ +export async function readDeclaredDependencyNames(cwd: string): Promise { + let raw: string; + try { + raw = await readFile(path.join(cwd, 'package.json'), 'utf8'); + } catch { + return []; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return []; + } + if (typeof parsed !== 'object' || parsed === null) { + return []; + } + + const names = new Set(); + for (const group of ['dependencies', 'devDependencies'] as const) { + const deps = (parsed as Record)[group]; + if (typeof deps !== 'object' || deps === null) { + continue; + } + for (const [name, spec] of Object.entries(deps as Record)) { + if (typeof spec === 'string' && /^(file|link|workspace|portal):/.test(spec)) { + continue; + } + names.add(name); + } + } + + return [...names]; +} + +/** + * Declared names absent from a parsed package set. A non-empty result means + * the source predates package.json — the fossil-lockfile failure mode where + * e.g. `npm install` planted a package-lock.json once and the platform's + * native package manager (bun on Lovable) never updates it again. + */ +export function missingDependencies(declared: string[], packages: PackageEntry[]): string[] { + const present = new Set(packages.map((entry) => entry.name)); + return declared.filter((name) => !present.has(name)); +} diff --git a/src/parsers/index.ts b/src/parsers/index.ts index 59d7aa3..c4727f8 100644 --- a/src/parsers/index.ts +++ b/src/parsers/index.ts @@ -1,6 +1,7 @@ import { access } from 'node:fs/promises'; import path from 'node:path'; import { PatchstackError, type Manifest, type PackageEntry } from '../types.js'; +import { missingDependencies, readDeclaredDependencyNames } from './consistency.js'; import { parseNpmLockfile } from './npm.js'; import { walkNodeModules } from './node_modules.js'; import { parsePnpmLockfile } from './pnpm.js'; @@ -69,9 +70,93 @@ export async function detectLockfile(cwd: string): Promise { } export async function scanLockfile(cwd: string): Promise { - const detected = await detectLockfile(cwd); - const packages = await runStrategy(detected, cwd); - return { ecosystem: detected.ecosystem, packages }; + const candidates = await presentLockfiles(cwd); + if (candidates.length === 0) { + // Preserve the exact historical error for the no-lockfile case. + await detectLockfile(cwd); + } + + // Validate each source against package.json before trusting it. A lockfile + // missing declared dependencies is a fossil — e.g. `npm install` planted a + // package-lock.json once on a bun-managed platform (Lovable), and the native + // dependency flow never updates it again. Trusting it would freeze the + // manifest and the build fingerprint while the real dependency set drifts. + const declared = await readDeclaredDependencyNames(cwd); + const warnings: string[] = []; + let firstParsed: { packages: PackageEntry[]; filename: string } | null = null; + let walkTried = false; + + for (const candidate of candidates) { + let packages: PackageEntry[]; + try { + packages = await runStrategy(candidate, cwd); + } catch { + continue; + } + walkTried ||= candidate.strategy === 'node-modules-walk'; + firstParsed ??= { packages, filename: candidate.filename }; + + const missing = missingDependencies(declared, packages); + if (missing.length === 0) { + return manifestWith(packages, warnings); + } + warnings.push(staleWarning(candidate.filename, missing)); + } + + // Last resort: the installed truth. node_modules reflects what the build + // actually compiles against, whichever package manager wrote it. + if (!walkTried) { + try { + const packages = await walkNodeModules(cwd); + const missing = missingDependencies(declared, packages); + if (missing.length > 0) { + warnings.push(staleWarning('node_modules/', missing)); + } + warnings.push('Scanned node_modules/ instead. Delete the stale lockfile to silence this warning.'); + return manifestWith(packages, warnings); + } catch { + // fall through to the best lockfile we managed to parse + } + } + + if (firstParsed === null) { + const expected = LOCKFILE_CANDIDATES.map((candidate) => candidate.filename).join(', '); + throw new PatchstackError( + `No readable lockfile found in ${cwd}. Expected one of: ${expected}.`, + 'LOCKFILE_NOT_FOUND', + ); + } + + warnings.push( + `No fully-consistent source found; reporting ${firstParsed.filename}. The manifest may understate the real dependency set.`, + ); + + return manifestWith(firstParsed.packages, warnings); +} + +function manifestWith(packages: PackageEntry[], warnings: string[]): Manifest { + return warnings.length > 0 + ? { ecosystem: 'npm', packages, warnings } + : { ecosystem: 'npm', packages }; +} + +function staleWarning(source: string, missing: string[]): string { + const sample = missing.slice(0, 3).join(', '); + const suffix = missing.length > 3 ? `, +${missing.length - 3} more` : ''; + return `${source} looks stale: package.json declares ${missing.length} dependenc${missing.length === 1 ? 'y' : 'ies'} it does not contain (${sample}${suffix}).`; +} + +async function presentLockfiles(cwd: string): Promise { + const probed = await Promise.all( + LOCKFILE_CANDIDATES.map(async (candidate) => { + const filePath = path.join(cwd, candidate.filename); + return { ...candidate, filePath, present: await exists(filePath) }; + }), + ); + + return probed + .filter((candidate) => candidate.present) + .map(({ filename, strategy, filePath }) => ({ ecosystem: 'npm' as const, filename, strategy, filePath })); } async function runStrategy( diff --git a/src/types.ts b/src/types.ts index a6b8cbc..9c8883c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -18,6 +18,12 @@ export interface PackageEntry { export interface Manifest { ecosystem: Ecosystem; packages: PackageEntry[]; + /** + * Human-readable scan diagnostics — set when the preferred lockfile looked + * stale (missing dependencies declared in package.json) and another source + * was used, or when no fully-consistent source existed. Never fatal. + */ + warnings?: string[]; } export interface Config { diff --git a/tests/parsers.test.ts b/tests/parsers.test.ts index fab3e92..dbdca3b 100644 --- a/tests/parsers.test.ts +++ b/tests/parsers.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { copyFile, mkdtemp, rm } from 'node:fs/promises'; +import { copyFile, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -89,3 +89,128 @@ describe('scanLockfile', () => { expect(manifest.packages.length).toBeGreaterThan(0); }); }); + +describe('stale lockfile detection', () => { + let dir: string; + + beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), 'ps-stale-')); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + async function writeJson(rel: string, value: unknown): Promise { + const full = path.join(dir, rel); + await mkdir(path.dirname(full), { recursive: true }); + await writeFile(full, JSON.stringify(value, null, 2)); + } + + /** Minimal npm lockfile v3 with the given node_modules packages. */ + function lockfileV3(packages: Record): unknown { + const entries: Record = { + '': { name: 'fixture-app', version: '1.0.0' }, + }; + for (const [name, version] of Object.entries(packages)) { + entries[`node_modules/${name}`] = { version }; + } + return { name: 'fixture-app', lockfileVersion: 3, packages: entries }; + } + + async function installNodeModules(packages: Record): Promise { + for (const [name, version] of Object.entries(packages)) { + await writeJson(path.join('node_modules', name, 'package.json'), { name, version }); + } + } + + it('falls back to the installed truth when package-lock.json misses declared dependencies', async () => { + // The Lovable failure mode: `npm install` planted a package-lock.json once, + // then the platform's native (bun) flow added dayjs — updating package.json, + // bun.lockb and node_modules, but never the npm lockfile. The fossil must + // not win: scan the source that actually covers the declared dependencies. + await writeJson('package.json', { + name: 'fixture-app', + dependencies: { axios: '^1.6.0', dayjs: '^1.11.0' }, + }); + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0' })); + await writeFile(path.join(dir, 'bun.lockb'), 'binary-placeholder'); + await installNodeModules({ axios: '1.6.0', dayjs: '1.11.10' }); + + const manifest = await scanLockfile(dir); + const names = manifest.packages.map((p) => p.name); + + expect(names).toContain('dayjs'); + expect(manifest.warnings?.join(' ')).toMatch(/package-lock\.json.*dayjs/s); + }); + + it('uses node_modules as a last resort even without a bun lockfile', async () => { + await writeJson('package.json', { + name: 'fixture-app', + dependencies: { dayjs: '^1.11.0' }, + }); + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0' })); + await installNodeModules({ axios: '1.6.0', dayjs: '1.11.10' }); + + const manifest = await scanLockfile(dir); + + expect(manifest.packages.map((p) => p.name)).toContain('dayjs'); + expect(manifest.warnings?.length).toBeGreaterThan(0); + }); + + it('keeps a consistent package-lock.json without warnings', async () => { + await writeJson('package.json', { + name: 'fixture-app', + dependencies: { axios: '^1.6.0' }, + devDependencies: { dayjs: '^1.11.0' }, + }); + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0', dayjs: '1.11.10' })); + // node_modules deliberately different: a consistent lockfile stays authoritative. + await installNodeModules({ axios: '1.6.0' }); + + const manifest = await scanLockfile(dir); + + expect(manifest.packages.map((p) => p.name).sort()).toEqual(['axios', 'dayjs']); + expect(manifest.warnings ?? []).toEqual([]); + }); + + it('ignores non-registry specifiers when judging staleness', async () => { + await writeJson('package.json', { + name: 'fixture-app', + dependencies: { + axios: '^1.6.0', + 'local-lib': 'file:../local-lib', + 'workspace-lib': 'workspace:*', + }, + }); + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0' })); + + const manifest = await scanLockfile(dir); + + expect(manifest.packages.map((p) => p.name)).toEqual(['axios']); + expect(manifest.warnings ?? []).toEqual([]); + }); + + it('returns the best available source with a warning when nothing is fully consistent', async () => { + await writeJson('package.json', { + name: 'fixture-app', + dependencies: { axios: '^1.6.0', dayjs: '^1.11.0' }, + }); + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0' })); + // No node_modules at all: still scan (never fail harder than today), but say so. + + const manifest = await scanLockfile(dir); + + expect(manifest.packages.map((p) => p.name)).toEqual(['axios']); + expect(manifest.warnings?.length).toBeGreaterThan(0); + }); + + it('behaves exactly as before when there is no package.json to validate against', async () => { + await writeJson('package-lock.json', lockfileV3({ axios: '1.6.0' })); + + const manifest = await scanLockfile(dir); + + expect(manifest.packages.map((p) => p.name)).toEqual(['axios']); + expect(manifest.warnings ?? []).toEqual([]); + }); +});