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..dc0108b3 100644 --- a/.claude/references/deps-known-issues.md +++ b/.claude/references/deps-known-issues.md @@ -83,6 +83,88 @@ 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 +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 + +- **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..055eda29 --- /dev/null +++ b/scripts/check-lockfile.mjs @@ -0,0 +1,180 @@ +#!/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 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 + * + * 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'); + +/** + * 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; +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')); + + 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); + } + + 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 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 fail at the install step.\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 —'); + 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 }); +}