From 8dc5b1cbbacdcafa2b8e7e3c73fcf00987f40a37 Mon Sep 17 00:00:00 2001 From: akkerkid <68400858+akkerkid@users.noreply.github.com> Date: Wed, 6 May 2026 14:17:27 -0400 Subject: [PATCH] =?UTF-8?q?add=20rule=2009:=20frontend=20dep=20changes=20?= =?UTF-8?q?=E2=80=94=20full=20lockfile=20regenerate=20+=20npm=20ci=20verif?= =?UTF-8?q?y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codifies the workflow that prevents the PR-#55 (vite 5→6) class of failure: the autobox runs npm 9.2.0; npm install there can produce a lockfile that's silently incomplete under stricter npm 10+ — npm test and npm run build on the autobox don't catch it because they reuse the pre-populated node_modules. Reviewer hits 16+ "Missing: from lock file" errors when they try a fresh npm ci. Rule: - Always rm -rf node_modules + package-lock.json + npm install (don't trust surgical npm install ) - Verify with npm ci (strict) before committing - Capture both build and test output verbatim in PR body - Audit subagent gets a Q6 to flag orphan lockfile churn Caught the day after PR-#55 was rebased + audited ✅ ship; surfaced when reviewer ran an ephemeral node:20-alpine build/test as belt-and-suspenders verification before merging. --- 09-frontend-dep-changes.md | 100 +++++++++++++++++++++++++++++++++++++ 11-work-phase.md | 9 ++++ README.md | 1 + 3 files changed, 110 insertions(+) create mode 100644 09-frontend-dep-changes.md diff --git a/09-frontend-dep-changes.md b/09-frontend-dep-changes.md new file mode 100644 index 0000000..fd9dfcc --- /dev/null +++ b/09-frontend-dep-changes.md @@ -0,0 +1,100 @@ +# 09 — Frontend dependency changes + +When your work touches `frontend/package.json` or `frontend/package-lock.json` (e.g. a `bot-eligible` Dependabot bump like #45), the lockfile is the artifact that matters most. The rest of this rule is about producing a lockfile that's *reproducible under strict install* — not just one that happens to work on the autobox. + +## Why this rule exists + +The autobox runs npm 9.2.0. Newer npm majors (10+) enforce stricter listing of transitive deps in `package-lock.json` than npm 9 does. A lockfile generated by `npm install ` under npm 9 can be silently missing entries that npm 10's `npm ci` then refuses (16+ "Missing: from lock file" errors). + +The autobox's own `npm test` and `npm run build` reuse the bot's pre-populated `node_modules` and never re-resolve, so this defect is invisible locally but blocks any reviewer with a newer npm — and blocks reproducible CI/CD installs in general. + +This was the failure mode of the PR-#55 vite bump (caught at review time on 2026-05-06, after a clean rebase + green local tests). + +## The mandatory workflow for frontend dep changes + +### Step 1: Regenerate the lockfile from scratch + +Don't surgically edit `package-lock.json`. Don't trust `npm install ` to keep the rest of the lockfile honest. Always do a full delete + regenerate: + +```bash +cd /home/bot/work/meshcore-planner/frontend +rm -rf node_modules package-lock.json +npm install --no-audit --no-fund +``` + +If the bump requires a coupled-tooling jump (e.g. vite 5→6 forces vitest 1→3 because vitest 1 bundles vite 5 internally), edit `package.json` to bump BOTH versions FIRST, then run the regenerate above. The single regenerate produces a self-consistent lockfile. + +### Step 2: Verify strict reproducibility + +```bash +rm -rf node_modules +npm ci --no-audit --no-fund +``` + +`npm ci` is strict: every entry in `node_modules` must be in the lockfile and vice versa. If this fails, the lockfile is incomplete — go back to Step 1 and check that `package.json` is sane. + +If `npm ci` passes under npm 9 but you suspect npm-major-skew issues (the PR-#55 failure mode), additionally validate against a newer npm: + +```bash +# Only if the autobox provisioner has a newer node available; otherwise skip +# and rely on Step 1's full-regenerate to be sufficient. +node20 --version 2>/dev/null && { + rm -rf node_modules + npx --package=npm@10 -- npm ci --no-audit --no-fund +} +``` + +If a newer npm isn't available on the autobox, document that in the PR body's audit block: "verified `npm ci` under npm 9.2.0; reviewer should re-verify under their own npm version before merging." + +### Step 3: Run build + tests + +```bash +npm run build # capture full output, not just exit code +npm test # capture full output, not just exit code +``` + +Both must succeed end-to-end. Capture the output verbatim for the PR body — see Step 5. + +### Step 4: Commit BOTH files together + +```bash +git add package.json package-lock.json +git commit -m "deps(frontend): bump → ()" +``` + +Never commit one without the other. A `package.json` change without a corresponding lockfile update — or vice versa — produces an inconsistent state that fails `npm ci` for everyone. + +### Step 5: PR body requirements + +In addition to the standard Q1-Q5 audit (per `12-pre-pr-review.md`), the PR body MUST include: + +- **`npm ci` output** (literal capture, last 5-10 lines showing "added N packages" with no "Missing:" errors). This is the reproducibility proof; it stands in for "tests pass" for the lockfile-correctness dimension. +- **`npm run build` output** (last 5-10 lines including the "✓ built in Xs" line). +- **`npm test` output** (full pass/fail summary including the actual test count — verify this matches reality; don't reuse a stale count from a prior iteration). +- **Breaking-change matrix** for any major-version bump, citing the upstream migration guide and confirming each breaking change does or doesn't affect this codebase. + +### Step 6: Pre-PR audit subagent — additional check for frontend dep diffs + +When you dispatch the Phase 3 review subagent (per `12-pre-pr-review.md`), include this extra question for any diff touching `frontend/package*.json`: + +> Q6: Does the lockfile diff *only* contain entries that are direct or transitive consequences of the `package.json` change, OR are there orphan removals/additions suggesting a partial regenerate? If you see lockfile churn that doesn't trace back to a package.json delta, flag it as ⚠ revise. + +The subagent should grep the lockfile diff for added/removed top-level packages and confirm each one is reachable from a `package.json` dependency. + +## Anti-patterns (each one is a ❌ block) + +- Editing `package-lock.json` by hand to "fix" something +- Running `npm install ` and committing without regenerating from scratch when the bump is non-trivial (major version, peer-dep cascade, etc.) +- Reusing test-count or build-time numbers from an earlier audit block without re-running and re-capturing +- Claiming "build passes" without `npm ci` having succeeded first +- Shipping a `package.json` change with no `package-lock.json` change, or vice versa + +## When to block instead of ship + +`bot-blocked-need-decision` if: + +- The bump introduces a peer-dep version conflict you can't resolve without picking a winner across multiple packages +- The breaking-change migration guide says "manual code changes required" and the changes touch protected paths (anything under `app/` requires CODEOWNERS review per `02-meshcore-invariants.md`) +- `npm audit` post-bump reveals a NEW high/critical advisory introduced by the bump itself (vs. a pre-existing one on main) + +Pre-existing audit hits on main are NOT a reason to block this PR — file a separate `bot-eligible` issue for them. diff --git a/11-work-phase.md b/11-work-phase.md index a033f9c..f116b39 100644 --- a/11-work-phase.md +++ b/11-work-phase.md @@ -55,6 +55,15 @@ Commit messages: conventional-commits style, footer `refs #N`. If you're adding an ingest path, see `backend/app/provenance.py` and don't merge without it. +## Frontend dependency changes + +If your work touches `frontend/package.json` or `frontend/package-lock.json`, +the lockfile-correctness rules in `09-frontend-dep-changes.md` apply. +TL;DR: full regenerate (delete `node_modules` + lockfile, `npm install`), +verify `npm ci` succeeds, capture build + test output verbatim. Don't +trust `npm install ` to keep the lockfile honest under newer npm +versions than the autobox's npm 9. + ## State file updates — REQUIRED, not optional You MUST write `/home/bot/.bot-state.md` BEFORE the iteration ends. diff --git a/README.md b/README.md index 7a64e17..e1c807e 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,7 @@ start of every Ralph loop iteration before any work begins. | 06-feasibility-check.md | Pre-flight: data / hardware / decision | | 07-no-secrets.md | Never write secret values to any file or message | | 08-rebase-and-retest.md | When main has moved under your PR (label `bot-rebase` or `@meshomatic please rebase` comment) | +| 09-frontend-dep-changes.md | Lockfile-correct workflow for `frontend/package*.json` bumps (regenerate, `npm ci` verify, capture output) | | 10-inbox-phase.md | Phase 1 of each iteration: find work | | 11-work-phase.md | Phase 2: TDD on the chosen issue | | 12-pre-pr-review.md | Phase 3: fresh-context subagent audit |