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 |