From 280da5a1a31305ac12441d1b914a0b3e228431da Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Fri, 21 Aug 2026 07:48:14 -0400 Subject: [PATCH 1/2] fix(deps): guard against Windows->Linux lockfile drift, and close the TODO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the long-running lockfile drift TODO by adding the guard it was actually asking for, and answering the three items it left open. Background: package-lock.json is authored on Windows and installed by CI on Linux. npm resolves optional/bundled subtrees per platform, so a Windows-written lockfile can omit entries `npm ci` on Linux requires — CI then dies at the install step before any test runs. It broke main twice (2026-05-17 @emnapi/*, 2026-08-21 ajv/64f18f0), and both times it was found a merge later. Why `npm ci --dry-run` is not the guard Measured against a known-broken lockfile: npm ci --dry-run Windows: exit 0 Linux: exit 1 npm ci --dry-run --os=linux --cpu=x64 Windows: exit 0 npm's lock/manifest sync check ignores --os/--cpu, so the drift is undetectable with `npm ci` from a Windows machine. A hook built on it would pass every time and still break CI. This is the reason the previous two incidents were only ever caught downstream. The check scripts/check-lockfile.mjs asserts a platform-independent invariant instead: the lockfile must already be what Linux resolution produces. It relocks a throwaway copy with `npm install --package-lock-only --os=linux --cpu=x64` and diffs, reporting the exact node_modules/... keys that moved. Verified idempotent, so a correct lockfile gives a zero diff. Against the real broken lockfile it names all six drifted entries — from Windows, where `npm ci --dry-run` reported success. npm run deps:verify # check npm run deps:relock # regenerate canonically (also --fix) Enforced in two places - .githooks/pre-commit — runs only when package-lock.json is staged. Installed by the `prepare` script (git config core.hooksPath .githooks), so a fresh clone picks it up on first npm install. Bypass: git commit --no-verify. - .github/workflows/test.yml — new `lockfile` job, every push and PR, on Linux. Authoritative and unskippable. The test job's `npm ci` would also fail on drift, but with npm's cryptic "Missing: … from lock file" and no remediation. Verified end-to-end: staging the deduped/broken lockfile blocks the commit and names the entries; staging the good one passes. Offline behavior: the check needs the registry. Locally it warns and exits 0 when npm is unreachable, so an offline commit is not blocked; in CI it fails instead. Also in this commit - package-lock.json: one metadata line (fast-deep-equal devOptional -> dev), a leftover of the bogus hoisted ajv. Makes the lockfile exactly canonical, so the zero-diff invariant holds. - .gitattributes: forces LF on .githooks/**. With core.autocrlf=true a Mac/Linux clone would otherwise get a CRLF shell script, which fails to execute. - CLAUDE.md: the rule, why a green local `npm ci --dry-run` proves nothing, and a warning not to run `npm ci` while next dev holds a native .node file open. - deps-known-issues.md: the durable writeup — both incidents, the measurements, and why this is not fixable upstream. Answering the TODO's open items 1. `npm install --include=optional` — moot; superseded by the confirmed recipe. 2. `--os=linux --cpu=x64` — confirmed, and now wrapped in `deps:relock`. 3. Move dep work to Linux/WSL — not needed. The flags make Windows produce a correct lockfile, and the guard proves it before the commit lands. 5. Are the wasm32-wasi packages removable? No. @tailwindcss/oxide-wasm32-wasi and @unrs/resolver-binding-wasm32-wasi are optionalDependencies of @tailwindcss/oxide and unrs-resolver, both cpu:["wasm32"] — transitive, not ours. Excluding them needs --omit=optional, which would also drop every platform's native binary. The guard is the fix, not removal. TODO removed; its content now lives in deps-known-issues.md, which is where state that outlives a single fix belongs. Co-Authored-By: Claude Opus 5 (1M context) --- .../TODO/investigate-emnapi-lockfile-drift.md | 80 --------- .claude/references/deps-known-issues.md | 71 ++++++++ .gitattributes | 5 + .githooks/pre-commit | 23 +++ .github/workflows/test.yml | 19 +++ CLAUDE.md | 13 ++ package-lock.json | 2 +- package.json | 5 +- scripts/check-lockfile.mjs | 155 ++++++++++++++++++ 9 files changed, 291 insertions(+), 82 deletions(-) delete mode 100644 .claude/TODO/investigate-emnapi-lockfile-drift.md create mode 100644 .gitattributes create mode 100755 .githooks/pre-commit create mode 100644 scripts/check-lockfile.mjs diff --git a/.claude/TODO/investigate-emnapi-lockfile-drift.md b/.claude/TODO/investigate-emnapi-lockfile-drift.md deleted file mode 100644 index 8f3171b9..00000000 --- a/.claude/TODO/investigate-emnapi-lockfile-drift.md +++ /dev/null @@ -1,80 +0,0 @@ -# TODO: Investigate `@emnapi/*` lockfile drift on Windows `npm install` - -**Created:** 2026-05-17 -**Severity:** Annoying — CI breaks every time someone bumps a dep on Windows. - -**Workaround in place:** Manually re-add the four `@emnapi/*` lockfile entries by hand after every Windows `npm install`. Tracked by commits `dc5e439` and the follow-up patch after `ecaa009`. - -> **2026-08-21 update — the regeneration recipe is now verified, the systemic guard is not.** -> -> A second incident (`ajv`, introduced by `64f18f0`) broke `npm ci` on `main` for every branch. -> Fixing it confirmed **option 2 below works and is safe**: -> -> ```bash -> npm install --package-lock-only --os=linux --cpu=x64 -> ``` -> -> Run from Windows, this restored every missing nested/bundled entry — including the -> `@tailwindcss/oxide-wasm32-wasi` → `@emnapi/*` subtree this TODO is about — and pruned -> **nothing**. Platform-specific entry counts were byte-identical before and after -> (76 win32, 75 darwin, 46 linux-x64, 40 android), so `--os`/`--cpu` steer resolution -> without narrowing the lockfile to one platform. `--package-lock-only` never touches -> `node_modules`, so it is safe to run mid-session. -> -> Verified by `npm ci` on Linux (exit 0, 587 packages) before pushing. -> -> **What is still missing is item 4: the guard.** Both incidents reached `main` and were -> found one merge later. Until `npm ci --dry-run` runs on a lockfile change, there will be -> a third incident. That is the remaining work in this TODO — the manual fix is solved. - -## Symptom - -CI `npm ci` on Ubuntu fails with: - -``` -npm error Missing: @emnapi/runtime@1.10.0 from lock file -npm error Missing: @emnapi/core@1.10.0 from lock file -``` - -…immediately after a dep bump that was prepared on Windows. The same pattern has now hit twice: - -- `bdd2e47` (npm dedupe on Windows) → fixed by `dc5e439` -- `ecaa009` (tsx + @vitejs/plugin-react bump on Windows) → fixed by the follow-up commit to this TODO - -## Hypothesis - -The `@emnapi/core` and `@emnapi/runtime` packages are pulled in as **optional, peer** deps of `@rolldown/binding-wasm32-wasi` (a Linux-only optional dep). When `npm install` runs on Windows, npm prunes them out of the lockfile because the parent `@rolldown/binding-wasm32-wasi` doesn't resolve on Windows. CI on Linux then reads `package.json`, sees the requirement, and the lockfile is "missing" entries → `npm ci` refuses to proceed. - -## Things to try - -1. **`npm install --include=optional`** on Windows — does this preserve the Linux-only optional graph? If yes, document it as the required install command and add to CLAUDE.md / contributing guide. -2. ~~**`npm install --os=linux --cpu=x64`**~~ — **CONFIRMED WORKING**, see the update at the top. - Use `npm install --package-lock-only --os=linux --cpu=x64`; the `--package-lock-only` part - matters, since it keeps `node_modules` untouched. This is now the documented fix for this - class of drift. -3. **Move all dep-bump work to a Linux container or WSL** so lockfiles are always generated against the CI platform. -4. **Pre-commit hook or CI guard** — **this is the remaining work.** Rather than detecting the - four `@emnapi/*` entries specifically (the `ajv` incident had nothing to do with emnapi), run - the generic check: on any commit touching `package-lock.json`, run `npm ci --dry-run` on Linux - and fail fast. That catches every variant of this drift at the commit that introduces it - instead of one merge later. The `/audit-deps` skill is the natural home. -5. **Investigate whether `@tailwindcss/oxide-wasm32-wasi` and `@rolldown/binding-wasm32-wasi` are actually needed** — if neither is being used at build/runtime, removing them eliminates the source of the optional/peer entanglement. - -## How to verify a fix - -After applying a candidate fix on Windows: - -```pwsh -Remove-Item -Recurse -Force node_modules -npm install # or whatever variant is being tested -git diff package-lock.json # the @emnapi/core and @emnapi/runtime entries should still be present -``` - -Then push to a branch and confirm CI's `npm ci` step succeeds on Ubuntu. - -## References - -- `dc5e439` — prior manual fix with full context in commit message -- `bdd2e47` — original drift introduction (npm dedupe) -- `ecaa009` — second drift introduction (this incident) -- CI run that failed: https://github.com/MinistryPlatform-Community/MPNext/actions/runs/25991275743 diff --git a/.claude/references/deps-known-issues.md b/.claude/references/deps-known-issues.md index c1e11500..487bdf14 100644 --- a/.claude/references/deps-known-issues.md +++ b/.claude/references/deps-known-issues.md @@ -83,6 +83,77 @@ Last audit: **2026-08-21 (run 2)** — report at `.claude/reports/deps-audit-202 | `chalk` | `^5.6.2` → `^6.0.0` | `npm run setup:check` renders colored output, all 8 checks run | 2026-08-21 | | `@testing-library/jest-dom` | `^6.9.1` → `^7.0.1` | 279/279 tests pass; `@testing-library/dom@^10.4.1` promoted transitive → explicit `devDependency` as v7 requires | 2026-08-21 | +## Lockfile platform drift (Windows -> Linux CI) + +**Resolved 2026-08-21 with a guard. Read this before touching `package-lock.json`.** + +`package-lock.json` is authored on Windows and installed by CI on Linux. npm resolves +optional and bundled subtrees per platform, so a lockfile written on Windows can omit +entries `npm ci` on Linux requires. CI then dies at the install step with a cryptic +`Missing: … from lock file`, before any test runs. + +It happened twice, and both times reached `main` and were found a merge later: + +| Date | Trigger | Damage | +|---|---|---| +| 2026-05-17 | `npm dedupe` on Windows | `@emnapi/*` subtree under `@tailwindcss/oxide-wasm32-wasi` pruned; fixed by hand | +| 2026-08-21 | `64f18f0` "Package Update Cleanup" | `ajv` hoisted to top level; `main` red for ~45 min; fixed in PR #72 | + +### The rule + +```bash +npm run deps:relock # the ONLY supported way to regenerate the lockfile +npm run deps:verify # check it (runs in CI and in the pre-commit hook) +``` + +`deps:relock` is `npm install --package-lock-only --os=linux --cpu=x64`. Verified +2026-08-21: it restores every missing nested/bundled entry, prunes nothing, and does not +narrow the lockfile to one platform — platform entry counts were byte-identical before and +after (win32 76, darwin 75, linux-x64 46, android 40). It is idempotent, and +`--package-lock-only` never touches `node_modules`, so it is safe to run mid-session. + +**Never** regenerate with a bare `npm install` or `npm dedupe` on Windows. Measured against +the fixed lockfile: a plain `npm install --package-lock-only` is harmless (2 metadata lines), +but `npm dedupe --package-lock-only` re-breaks it in one command — 114 lines, stripping the +nested `eslint/node_modules/ajv` subtrees and re-hoisting `ajv@6.15.0`, reproducing the exact +`64f18f0` failure. + +### Why the guard is not `npm ci --dry-run` + +Measured 2026-08-21 against a known-broken lockfile: + +| Command | Windows | Linux | +|---|---|---| +| `npm ci --dry-run` | **exit 0** | exit 1 | +| `npm ci --dry-run --os=linux --cpu=x64` | **exit 0** | — | + +npm's lock/manifest sync check ignores `--os`/`--cpu`, so **the drift is undetectable with +`npm ci` from a Windows machine**. A hook built on it would pass every time and still break CI. + +`scripts/check-lockfile.mjs` instead asserts an invariant that holds on any platform: *the +lockfile must already be what Linux resolution produces.* It relocks a throwaway copy and +diffs, reporting the exact `node_modules/...` keys that moved. Against the real broken +lockfile it names all six entries, from Windows. + +### Where it runs + +- **pre-commit** — `.githooks/pre-commit`, only when `package-lock.json` is staged. + Auto-installed by the `prepare` script (`git config core.hooksPath .githooks`), so a fresh + clone gets it on first `npm install`. Bypass with `git commit --no-verify`. +- **CI** — the `lockfile` job in `.github/workflows/test.yml`, on every push and PR. This is + the authoritative check; it runs on Linux and cannot be skipped. + +Offline behavior: the check needs the registry. Locally it warns and passes when npm is +unreachable (so an offline commit is not blocked); in CI (`process.env.CI`) it fails instead. + +### Not fixable upstream + +`@tailwindcss/oxide-wasm32-wasi` and `@unrs/resolver-binding-wasm32-wasi` are +`optionalDependencies` of `@tailwindcss/oxide` and `unrs-resolver` respectively, both with +`cpu: ["wasm32"]`. They are transitive and not ours to remove — the only way to exclude them is +`--omit=optional`, which would also drop every platform's native binary. The WASM-fallback +entanglement is inherent to those upstream packages, so the guard is the fix, not removal. + ## Open items awaiting a decision (not blockers) | Item | Detail | Raised | diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..56010316 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,5 @@ +# Git hooks must keep LF endings. This repo is developed on Windows with +# core.autocrlf=true, which would otherwise check the hook out with CRLF — and a +# shell script with CRLF line endings fails to execute on macOS and Linux +# (`/bin/sh^M: bad interpreter`). +.githooks/** text eol=lf diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 00000000..26145606 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,23 @@ +#!/bin/sh +# +# Blocks commits that would land a lockfile CI cannot install. +# +# Only runs when package-lock.json is actually staged, so it costs nothing on a +# normal commit. See scripts/check-lockfile.mjs for why `npm ci --dry-run` is +# not sufficient here (it exits 0 on Windows against a lockfile that breaks +# Linux CI). +# +# Installed by the `prepare` script in package.json, which points +# core.hooksPath at this directory. To bypass once: git commit --no-verify + +if git diff --cached --name-only --diff-filter=ACM | grep -q '^package-lock\.json$'; then + echo "package-lock.json is staged — checking it against Linux resolution..." + if ! node scripts/check-lockfile.mjs; then + echo "" + echo "Commit blocked. Run 'npm run deps:relock', stage package-lock.json, and retry." + echo "To commit anyway: git commit --no-verify" + exit 1 + fi +fi + +exit 0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2c4e1c2c..963fe563 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,6 +7,25 @@ on: branches: [main] jobs: + # Fails fast and legibly when package-lock.json drifts from what Linux + # resolution produces. The `test` job's `npm ci` would also die on this, but + # with npm's cryptic "Missing: … from lock file" and no remediation. Drift + # generated on Windows is invisible to `npm ci --dry-run` there, so this is the + # authoritative check — see scripts/check-lockfile.mjs. + lockfile: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - name: Check package-lock.json for platform drift + run: node scripts/check-lockfile.mjs + test: runs-on: ubuntu-latest diff --git a/CLAUDE.md b/CLAUDE.md index 45e28fa5..bfe65ccf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,6 +35,19 @@ Ministry Platform is a shared production database containing real church member - **Generate MP Types**: `npm run mp:generate:models` (generates TypeScript types + Zod schemas from Ministry Platform API, cleans output directory first) - **Tests**: `npm test` (Vitest in watch mode), `npm run test:run` (single run), `npm run test:coverage` (with coverage) - **Setup**: `npm run setup` (interactive project setup wizard), `npm run setup:check` (validate setup without changes) +- **Dependencies**: `npm run deps:relock` (regenerate `package-lock.json` — the only supported way), `npm run deps:verify` (check it for platform drift) + +### Dependency Rule — MANDATORY + +**Never regenerate `package-lock.json` with a bare `npm install` or `npm dedupe` on Windows.** Use `npm run deps:relock`. + +This repo's lockfile is authored on Windows and installed by CI on Linux. npm resolves optional and bundled subtrees per platform, so a Windows-generated lockfile can omit entries `npm ci` on Linux requires — CI then dies at the install step before any test runs. This broke `main` twice (2026-05-17 `@emnapi/*`, 2026-08-21 `ajv`). One `npm dedupe` on Windows is enough to reproduce it. + +Critically, **`npm ci --dry-run` cannot detect this on Windows** — it exits 0 there against a lockfile that fails on Linux, and `--os`/`--cpu` do not change that. So a green local check proves nothing; run `npm run deps:verify`, which asserts the lockfile already matches Linux resolution. + +A pre-commit hook (`.githooks/pre-commit`, auto-installed via the `prepare` script) and a `lockfile` CI job both enforce this. Full detail: **[Dependency Known Issues](.claude/references/deps-known-issues.md)** § Lockfile platform drift. + +Also: do not run `npm ci` while `next dev` is running — it deletes `node_modules` first, then aborts on a locked native `.node` file, leaving the tree half-installed. Stop the dev server first. ### Type Generation Notes diff --git a/package-lock.json b/package-lock.json index 403a96c3..8512410d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7164,7 +7164,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/fast-glob": { diff --git a/package.json b/package.json index 5074070f..ac86fa47 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,10 @@ "mp:generate:models": "tsx src/lib/providers/ministry-platform/scripts/generate-types.ts -o src/lib/providers/ministry-platform/models --zod --clean", "mp:generate:storedprocs": "tsx src/lib/providers/ministry-platform/scripts/generate-storedprocs.ts", "setup": "tsx scripts/setup.ts", - "setup:check": "tsx scripts/setup.ts --check" + "setup:check": "tsx scripts/setup.ts --check", + "deps:verify": "node scripts/check-lockfile.mjs", + "deps:relock": "node scripts/check-lockfile.mjs --fix", + "prepare": "git config core.hooksPath .githooks || exit 0" }, "dependencies": { "@heroicons/react": "^2.2.0", diff --git a/scripts/check-lockfile.mjs b/scripts/check-lockfile.mjs new file mode 100644 index 00000000..9b796ca1 --- /dev/null +++ b/scripts/check-lockfile.mjs @@ -0,0 +1,155 @@ +#!/usr/bin/env node +/** + * Lockfile drift guard. + * + * WHY THIS EXISTS + * + * `package-lock.json` is generated on Windows and consumed by CI on Linux. npm + * resolves optional/bundled dependency subtrees per platform, so a lockfile + * written on Windows can be missing entries that `npm ci` on Linux requires. + * When that happens CI dies at the install step, before a single test runs, with + * a cryptic `Missing: … from lock file`. It has happened twice: + * + * 2026-05-17 @emnapi/* subtree pruned (npm dedupe on Windows) + * 2026-08-21 ajv hoisted to top level (64f18f0, broke main for ~45min) + * + * WHY IT IS NOT JUST `npm ci --dry-run` + * + * Measured 2026-08-21: against a known-broken lockfile, `npm ci --dry-run` exits + * 0 on Windows and fails on Linux. Adding `--os=linux --cpu=x64` does NOT change + * that — npm's lock/manifest sync check ignores those overrides. So the drift is + * genuinely invisible to `npm ci` from a Windows machine, and a pre-commit hook + * built on it would pass every time while still breaking CI. + * + * THE CHECK + * + * Instead of asking "does this lockfile install?", ask "is this lockfile already + * what Linux resolution would produce?": + * + * npm install --package-lock-only --os=linux --cpu=x64 + * + * on a throwaway copy, then diff. Verified idempotent, so a correct lockfile + * gives a zero diff, and any drift — this variant or the next one — shows up as + * added/removed `node_modules/...` keys. `--package-lock-only` never touches + * node_modules, so this is safe to run at any time, including mid-dev-server. + * + * USAGE + * + * node scripts/check-lockfile.mjs # verify (npm run deps:verify) + * node scripts/check-lockfile.mjs --fix # rewrite canonically (npm run deps:relock) + * + * Exit 0 = clean, 1 = drift (or, in CI, could-not-verify). + */ + +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, copyFileSync, readFileSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const LOCKFILE = join(REPO_ROOT, 'package-lock.json'); +const MANIFEST = join(REPO_ROOT, 'package.json'); + +// The flags that make Windows resolve the way Linux CI does. Keep in sync with +// the `deps:relock` script and .claude/references/deps-known-issues.md. +const RELOCK_FLAGS = [ + 'install', + '--package-lock-only', + '--os=linux', + '--cpu=x64', + '--ignore-scripts', + '--no-audit', + '--no-fund', +]; + +const fix = process.argv.includes('--fix'); +const isCI = Boolean(process.env.CI); + +/** Normalize line endings so a CRLF checkout does not read as drift. */ +const normalize = (s) => s.replace(/\r\n/g, '\n'); + +/** Extract the `node_modules/...` keys so we can report what actually moved. */ +function lockfileKeys(text) { + try { + return new Set(Object.keys(JSON.parse(text).packages ?? {})); + } catch { + return new Set(); + } +} + +let scratch; +try { + scratch = mkdtempSync(join(tmpdir(), 'mpnext-lockcheck-')); + copyFileSync(MANIFEST, join(scratch, 'package.json')); + copyFileSync(LOCKFILE, join(scratch, 'package-lock.json')); + + try { + execFileSync('npm', RELOCK_FLAGS, { + cwd: scratch, + stdio: 'pipe', + shell: process.platform === 'win32', + }); + } catch (err) { + // Almost always a registry/network problem. Do not block a local commit for + // it — but never let it pass silently in CI, where the network is expected + // and this check is authoritative. + const detail = String(err.stderr || err.message || '').trim().split('\n').slice(-3).join('\n'); + if (isCI) { + console.error('✗ lockfile check could not run in CI — treating as failure.\n'); + console.error(detail); + process.exit(1); + } + console.warn('⚠ lockfile check skipped: could not reach the npm registry.'); + console.warn(' CI will still verify this. Details:\n ' + detail.replace(/\n/g, '\n ')); + process.exit(0); + } + + const committed = normalize(readFileSync(LOCKFILE, 'utf8')); + const canonical = normalize(readFileSync(join(scratch, 'package-lock.json'), 'utf8')); + + if (committed === canonical) { + console.log('✓ package-lock.json matches Linux resolution — no drift.'); + process.exit(0); + } + + if (fix) { + // Write with the repo's existing newline style rather than forcing LF. + const usesCRLF = readFileSync(LOCKFILE, 'utf8').includes('\r\n'); + writeFileSync(LOCKFILE, usesCRLF ? canonical.replace(/\n/g, '\r\n') : canonical); + console.log('✓ package-lock.json rewritten to match Linux resolution.'); + console.log(' Review `git diff package-lock.json`, then commit it.'); + process.exit(0); + } + + const before = lockfileKeys(committed); + const after = lockfileKeys(canonical); + const removed = [...after].filter((k) => !before.has(k)); // missing from the committed lock + const added = [...before].filter((k) => !after.has(k)); // present but shouldn't be + + console.error('✗ package-lock.json does not match Linux resolution.'); + console.error(' `npm ci` on CI will likely fail at the install step.\n'); + + if (removed.length) { + console.error(` Missing ${removed.length} entr${removed.length === 1 ? 'y' : 'ies'} that Linux needs:`); + for (const k of removed.slice(0, 12)) console.error(` + ${k}`); + if (removed.length > 12) console.error(` … and ${removed.length - 12} more`); + console.error(''); + } + if (added.length) { + console.error(` Has ${added.length} entr${added.length === 1 ? 'y' : 'ies'} Linux resolution does not produce:`); + for (const k of added.slice(0, 12)) console.error(` - ${k}`); + if (added.length > 12) console.error(` … and ${added.length - 12} more`); + console.error(''); + } + if (!removed.length && !added.length) { + console.error(' Same package set, but metadata differs (e.g. dev vs devOptional flags).\n'); + } + + console.error(' Fix: npm run deps:relock (then commit package-lock.json)'); + console.error(' Never fix this with a bare `npm install` or `npm dedupe` on Windows —'); + console.error(' those are what cause it. See .claude/references/deps-known-issues.md.'); + process.exit(1); +} finally { + if (scratch) rmSync(scratch, { recursive: true, force: true }); +} From 16079229df352f478b5685e66e38ba2f111aa59c Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Fri, 21 Aug 2026 07:51:59 -0400 Subject: [PATCH 2/2] fix(deps): compare lockfile tree shape, not bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `lockfile` CI job failed on its first run while `test` passed on the same commit — proof the byte-for-byte comparison was wrong, not the lockfile. CI's node 22 ships npm 10.x; developers here run npm 11.x. The two write metadata flags (`dev` vs `devOptional`, …) differently, so a byte diff reports drift that cannot break an install. Concretely: the committed lockfile differs from npm 10's output by one `fast-deep-equal` flag, and `npm ci` installed it cleanly in the same workflow run that the byte check rejected. A check that cries wolf is a check everyone learns to skip. Now compares tree shape only — which `node_modules/...` entries exist and at which version — and ignores metadata. That is exactly what `npm ci` validates, and it is stable across npm versions and platforms. Verified four ways: committed lockfile, Windows (npm 11.16) -> pass committed lockfile, Linux (npm 10.8) -> pass, notes the metadata-only gap broken lockfile, Windows -> fail, names all 6 entries broken lockfile, Linux -> fail, "ajv: 6.15.0 -> 8.20.0" plus missing fast-uri That last line is npm's own complaint (`Invalid: lock file's ajv@6.15.0 does not satisfy ajv@8.20.0`, `Missing: fast-uri@3.1.5`) derived independently, which is good evidence the semantic check tracks what npm actually enforces. Also adds version-mismatch reporting, which the byte comparison could not produce. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/references/deps-known-issues.md | 15 ++++- scripts/check-lockfile.mjs | 89 ++++++++++++++++--------- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/.claude/references/deps-known-issues.md b/.claude/references/deps-known-issues.md index 487bdf14..dc0108b3 100644 --- a/.claude/references/deps-known-issues.md +++ b/.claude/references/deps-known-issues.md @@ -132,8 +132,19 @@ npm's lock/manifest sync check ignores `--os`/`--cpu`, so **the drift is undetec `scripts/check-lockfile.mjs` instead asserts an invariant that holds on any platform: *the lockfile must already be what Linux resolution produces.* It relocks a throwaway copy and -diffs, reporting the exact `node_modules/...` keys that moved. Against the real broken -lockfile it names all six entries, from Windows. +compares. Against the real broken lockfile it names all six drifted entries, from Windows. + +The comparison is **semantic, not byte-for-byte** — it compares the tree shape (which +`node_modules/...` entries exist, and at which version) and ignores npm metadata flags. +That matters: CI's node 22 ships npm 10.x while developers here run npm 11.x, and the two +write flags like `dev` vs `devOptional` differently. A byte comparison fails on differences +that cannot break an install — verified 2026-08-21, when a lockfile differing only in one +`fast-deep-equal` flag installed cleanly on CI (`test` job green) while a byte-diff rejected +it. Reporting harmless diffs as failures is how a check gets ignored. + +The semantic comparison tracks npm's own validation closely. On the broken lockfile it +reports `ajv: 6.15.0 -> 8.20.0` and a missing `fast-uri`, which is what `npm ci` itself says +(`Invalid: lock file's ajv@6.15.0 does not satisfy ajv@8.20.0`, `Missing: fast-uri@3.1.5`). ### Where it runs diff --git a/scripts/check-lockfile.mjs b/scripts/check-lockfile.mjs index 9b796ca1..055eda29 100644 --- a/scripts/check-lockfile.mjs +++ b/scripts/check-lockfile.mjs @@ -28,10 +28,20 @@ * * npm install --package-lock-only --os=linux --cpu=x64 * - * on a throwaway copy, then diff. Verified idempotent, so a correct lockfile - * gives a zero diff, and any drift — this variant or the next one — shows up as - * added/removed `node_modules/...` keys. `--package-lock-only` never touches - * node_modules, so this is safe to run at any time, including mid-dev-server. + * on a throwaway copy, then compare. + * + * The comparison is deliberately SEMANTIC, not byte-for-byte. What breaks + * `npm ci` is the tree shape: entries that are missing, entries that shouldn't + * be there, or a different resolved version. Metadata flags (`dev` vs + * `devOptional`, `license`, …) do not break an install, and npm writes them + * slightly differently across versions — CI's node 22 ships npm 10.x while + * developers here run npm 11.x. A byte comparison therefore reports drift that + * cannot break anything, and would train everyone to ignore this check. + * Confirmed 2026-08-21: a lockfile differing only in one `devOptional` → `dev` + * flag installed cleanly on CI while a byte-diff flagged it. + * + * `--package-lock-only` never touches node_modules, so this is safe to run at + * any time, including mid-dev-server. * * USAGE * @@ -69,13 +79,19 @@ const isCI = Boolean(process.env.CI); /** Normalize line endings so a CRLF checkout does not read as drift. */ const normalize = (s) => s.replace(/\r\n/g, '\n'); -/** Extract the `node_modules/...` keys so we can report what actually moved. */ -function lockfileKeys(text) { - try { - return new Set(Object.keys(JSON.parse(text).packages ?? {})); - } catch { - return new Set(); +/** + * The install-relevant shape of a lockfile: which packages exist, and at which + * version. Everything else is metadata that npm writes inconsistently across + * versions and that cannot break `npm ci`. + */ +function treeShape(text) { + const packages = JSON.parse(text).packages ?? {}; + const shape = new Map(); + for (const [key, entry] of Object.entries(packages)) { + if (key === '') continue; // the root project entry, not an installed package + shape.set(key, entry?.version ?? null); } + return shape; } let scratch; @@ -108,8 +124,26 @@ try { const committed = normalize(readFileSync(LOCKFILE, 'utf8')); const canonical = normalize(readFileSync(join(scratch, 'package-lock.json'), 'utf8')); - if (committed === canonical) { - console.log('✓ package-lock.json matches Linux resolution — no drift.'); + const before = treeShape(committed); + const after = treeShape(canonical); + + const missing = [...after.keys()].filter((k) => !before.has(k)); + const extra = [...before.keys()].filter((k) => !after.has(k)); + const changed = [...after.keys()] + .filter((k) => before.has(k) && before.get(k) !== after.get(k)) + .map((k) => `${k}: ${before.get(k)} -> ${after.get(k)}`); + + const drifted = missing.length || extra.length || changed.length; + + if (!drifted) { + if (committed !== canonical && !fix) { + // Same tree, different metadata. Harmless for npm ci — say so and pass, + // rather than crying wolf. `npm run deps:relock` normalizes it if desired. + console.log('✓ package-lock.json tree matches Linux resolution — no drift.'); + console.log(' (Byte differences remain in npm metadata only; harmless for `npm ci`.)'); + } else { + console.log('✓ package-lock.json matches Linux resolution — no drift.'); + } process.exit(0); } @@ -122,29 +156,20 @@ try { process.exit(0); } - const before = lockfileKeys(committed); - const after = lockfileKeys(canonical); - const removed = [...after].filter((k) => !before.has(k)); // missing from the committed lock - const added = [...before].filter((k) => !after.has(k)); // present but shouldn't be + const report = (label, items, sign) => { + if (!items.length) return; + console.error(` ${label} (${items.length}):`); + for (const k of items.slice(0, 12)) console.error(` ${sign} ${k}`); + if (items.length > 12) console.error(` … and ${items.length - 12} more`); + console.error(''); + }; console.error('✗ package-lock.json does not match Linux resolution.'); - console.error(' `npm ci` on CI will likely fail at the install step.\n'); + console.error(' `npm ci` on CI will fail at the install step.\n'); - if (removed.length) { - console.error(` Missing ${removed.length} entr${removed.length === 1 ? 'y' : 'ies'} that Linux needs:`); - for (const k of removed.slice(0, 12)) console.error(` + ${k}`); - if (removed.length > 12) console.error(` … and ${removed.length - 12} more`); - console.error(''); - } - if (added.length) { - console.error(` Has ${added.length} entr${added.length === 1 ? 'y' : 'ies'} Linux resolution does not produce:`); - for (const k of added.slice(0, 12)) console.error(` - ${k}`); - if (added.length > 12) console.error(` … and ${added.length - 12} more`); - console.error(''); - } - if (!removed.length && !added.length) { - console.error(' Same package set, but metadata differs (e.g. dev vs devOptional flags).\n'); - } + report('Missing entries that Linux needs', missing, '+'); + report('Entries Linux resolution does not produce', extra, '-'); + report('Version mismatches', changed, '~'); console.error(' Fix: npm run deps:relock (then commit package-lock.json)'); console.error(' Never fix this with a bare `npm install` or `npm dedupe` on Windows —');