chore(deps): raise the Node floor jsdom 30 requires, retire the dead nanoid@5 override - #436
Merged
Merged
Conversation
…nanoid@5 override Follow-up to the dependabot batch (#411, #412, #414, #415, #418, #419, #420, #421). Two gaps those merges left behind, both of the "nothing is red, and that is the problem" kind: - `engines.node` was `>=22.13.0`, which admits Node 22.13-22.22.1, 23.x and 24.0-24.14. jsdom 30 (#415) rejects all three (`^22.22.2 || ^24.15.0 || >=26.0.0`), and nanoid 6 (#419) additionally excludes odd majors. CI pins Node 24 so the mismatch is invisible there — it would only have shown up on a contributor's machine. DoD 2e asks for exactly this reconciliation on a toolchain bump. - `nanoid@5: ^5.1.16` became a dead scope the moment #419 moved both consumers to `^6.0.1`: no nanoid 5 is left anywhere in the lockfile, so the pin speaks for nothing. Removing it changes no resolution (verified: the lockfile diff carries no nanoid version change). Its registry row in `docs/{en,vi}/security/dependency-overrides.md` already stated this exact removal condition. `nanoid@3` stays — `next` -> `postcss` still needs it. Also corrects two now-stale doc claims: the `nanoid@3` row referred to "the 5.x pin below", and the react-router@8 discussion cited the workspace floor as `>=22`, which was the stated reason its `engines.node >=22.22.0` requirement was out of reach. That objection is now moot; only the port itself remains. Backlog: B24 closes. B22 is widened — `apps/docs/vitest.config.ts` hits the same Vite `configLoader: 'native'` warning as `apps/studio`, and not only for `__dirname`. Two new entries: B30, `scripts/version-check.mjs` is wired to nothing and fails when run (it asserts `engines.node === '>=20'`), which is why no gate spoke up here at all; B31, `perf-k6.yml` has failed on every push for at least 30 runs, predating this batch. typecheck 17/17, build 9/9, `pnpm check:all` green, `pnpm audit --prod --audit-level high` clean. Docs pair re-stamped (en->vi, code-verified).
khuepm
added a commit
that referenced
this pull request
Aug 31, 2026
Rebasing this branch hit a conflict because #434 and #436 both assigned `B30`, to unrelated findings — a `localStorage` flake in `analytics-consent.test.tsx` and the dead `version-check.mjs` guard. Whichever merged second had to be renumbered by hand (`B33`), and nothing announced the collision; it showed up only as a rebase conflict. That is precisely the failure `registry:check` already prevents for the `#` column of the Setup Impact Registry, which once carried duplicate #20/#31/#32 rows. The backlog table was simply never included. It matters more than a cosmetic id clash: backlog ids are referenced *by id* from other rows ("Nối tiếp B13", "cùng class với B10", "Xem B24") and from CHANGELOG entries, so a silent renumber breaks cross-references that no test covers. `check-registry-numbering.mjs` now walks both tables — `#` in setup-impact.md and `B<n>` in out-of-scope-backlog.md — reporting the offending line numbers, the next safe id, and a reminder to keep whichever occurrence other rows cite. Both tables still fail closed if their shape changes and the scan parses zero rows, so the guard cannot quietly stop guarding (the B30/B33 lesson, applied to the guard itself). Verified both directions: injecting a duplicate `B30` exits 1 naming lines 53 and 56 and suggesting B34; removing it exits 0. Already reachable through `pnpm check:all`, so pre-commit and CI pick it up with no wiring change. Logged as B34 (fixed).
khuepm
added a commit
that referenced
this pull request
Aug 31, 2026
…flows (#441) * fix(ci): perf-k6 was an invalid workflow file, and nothing lints workflows `perf-k6.yml` had failed on every push for weeks. It was not the load-test job running and breaking — its `on:` never declared `push` at all. The `perf-gate` job's `if:` referenced `env.PERF_K6_FULL_RUN`, and the `env` context does not exist in a job-level `if:` (only `github`, `inputs`, `needs`, `vars`). That does not evaluate to empty: it makes the entire file unparseable, so GitHub never resolved `name:` or any job and recorded a bare failed run against every event, `on:` filters included. The tell was the API reporting the run's `name` as the file path with an empty job list. Fixed by using `vars.PERF_K6_FULL_RUN`, which is available at job level. The workflow now honours `on:` and stops running on push. Set the repo variable `PERF_K6_FULL_RUN=true` to let the nightly schedule run the full compose + k6 job; unset stops the scheduled run after `validate-scripts`. Dropped the workflow-level `env` entry, which nothing else read. The more useful fix is the second one. The reason a broken workflow stayed broken for weeks is that nothing checked this class: an unparseable workflow looks, from the outside, identical to a workflow that ran and failed — and because it is not a required check, no gate objected. New `workflow-lint` job runs `actionlint`, pinned by version and SHA-256 rather than adding another third-party action SHA to keep current. Zero findings across `.github/workflows` today, shellcheck included over the `run:` blocks that drive Postgres, Redis and the deploy steps. Not adding a DoD section for this: the guard is mechanical and self-enforcing, which DoD 6 prefers over a checklist item. Also raises the `@types/react-dom` override from `19.2.4` to `19.2.5`. The pin is exact, so it wins over whatever the manifests declare — which is why the pending minor-and-patch group bump (manifests to `^19.2.5`) fails `drift:check`: the ranges stopped intersecting. Raising the override first clears that without the group PR touching it, and `19.2.5` still satisfies the current `^19.2.4` declarations, so it changes nothing for anyone not on the group bump. Verified in the lockfile: every importer now resolves 19.2.5, and the whole lockfile diff is that propagation. Backlog B31 closes with the root cause recorded. actionlint 0 findings, `pnpm check:all` green, typecheck 17/17, build 9/9, `pnpm audit --prod --audit-level high` clean, `pnpm install --frozen-lockfile` verified against a fresh checkout of main. * fix(ci): close the five shellcheck findings the new workflow gate surfaced The `workflow-lint` job failed on its first real run, and the failure was correct. actionlint runs shellcheck over `run:` blocks only when the binary is on PATH and **skips it silently** otherwise — shellcheck was not installed on my machine, so the local run reported clean while the runner (ubuntu-latest ships it) found five pre-existing issues. Installed shellcheck 0.11.0 locally, reproduced all five, fixed them rather than lowering the gate: - `deploy-cms.yml` — `run: pnpm ... run build:${TARGET_ENV}` and the matching `deploy:` step, both unquoted (SC2086). Behaviourally a no-op today because TARGET_ENV is `production`/`staging`, but it is the exact class of quoting bug the comment on this job claims to care about. - `perf-k6.yml` — two `for i in $(seq ...)` readiness loops that never use `i` (SC2034), now `for _`. - `release.yml` — three consecutive `>> release-notes.md` redirects (SC2129), now one grouped `{ ... } >> release-notes.md`. Output is byte-identical: `git log --pretty=format:` emits no trailing newline, so the closing `echo` is still there. Also makes the gate unable to weaken quietly: the job now asserts `shellcheck --version` before running actionlint. Without that, a future runner image dropping shellcheck would silently stop checking every `run:` block — which is the same failure mode as B30, and I just demonstrated it on myself. actionlint exit 0 across `.github/workflows` with shellcheck actually present. * chore(guards): registry:check now covers the backlog ID column too Rebasing this branch hit a conflict because #434 and #436 both assigned `B30`, to unrelated findings — a `localStorage` flake in `analytics-consent.test.tsx` and the dead `version-check.mjs` guard. Whichever merged second had to be renumbered by hand (`B33`), and nothing announced the collision; it showed up only as a rebase conflict. That is precisely the failure `registry:check` already prevents for the `#` column of the Setup Impact Registry, which once carried duplicate #20/#31/#32 rows. The backlog table was simply never included. It matters more than a cosmetic id clash: backlog ids are referenced *by id* from other rows ("Nối tiếp B13", "cùng class với B10", "Xem B24") and from CHANGELOG entries, so a silent renumber breaks cross-references that no test covers. `check-registry-numbering.mjs` now walks both tables — `#` in setup-impact.md and `B<n>` in out-of-scope-backlog.md — reporting the offending line numbers, the next safe id, and a reminder to keep whichever occurrence other rows cite. Both tables still fail closed if their shape changes and the scan parses zero rows, so the guard cannot quietly stop guarding (the B30/B33 lesson, applied to the guard itself). Verified both directions: injecting a duplicate `B30` exits 1 naming lines 53 and 56 and suggesting B34; removing it exits 0. Already reachable through `pnpm check:all`, so pre-commit and CI pick it up with no wiring change. Logged as B34 (fixed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the dependabot batch merged today (#411, #412, #414, #415, #418, #419, #420, #421). Those PRs were all green and are verified good; this closes two gaps they left behind, both of the "nothing is red, and that is the problem" kind.
engines.nodefloor>=22.13.0admits Node 22.13–22.22.1, 23.x and 24.0–24.14. jsdom 30 (#415) rejects all three (^22.22.2 || ^24.15.0 || >=26.0.0), and nanoid 6 (#419) additionally excludes odd majors. CI pins Node 24, so the mismatch is invisible there — it would only ever have surfaced on a contributor's machine. Raised to^22.22.2 || ^24.15.0 || >=26.0.0..nvmrc(24) already satisfies it; no CI change needed.This is DoD §2e ("bump toolchain → đối chiếu
engines.nodecủa nó vớiengines.nodecủa repo và.nvmrc") applied.Dead
nanoid@5overrideThe moment #419 moved
apps/cmsandpackages/databaseto^6.0.1, the scoped overridenanoid@5: ^5.1.16stopped speaking for anything — no nanoid 5 remains anywhere in the lockfile. Removed from bothpackage.jsonandpnpm-workspace.yaml, plus its registry row indocs/{en,vi}/security/dependency-overrides.md, whose "Remove when" column already stated this exact condition.Verified it was genuinely dead: the lockfile diff carries no nanoid version change.
nanoid@3stays —next→postcssstill resolves it.Worth recording for next time: B24 predicted this would need a new
nanoid@6scope. It didn't. 6.0.1 is the latest release, so there is no advisory floor to force, anddrift:checkstayed green throughout because a scoped override only speaks for its own range. The scope didn't need updating, it needed deleting.Two doc claims that went stale
nanoid@3row said it was scoped "so it cannot fight the 5.x pin below". There is no 5.x pin below any more.react-router@8discussion cited the workspace floor asengines.node >=22as part of why react-router 8's>=22.22.0was out of reach. With the new floor that objection is gone, and so is the React peer objection (workspace is on React 19.2.8). What remains is the port itself. Corrected rather than left to mislead the next reader into thinking there is still a version blocker.EN is the source locale for this pair; VI updated in the same commit and the pair re-stamped with
stamp-pair.mjs --verified(DoD §4a).Backlog
fixed(nanoid 5→6), with a note on why the predictednanoid@6scope was unnecessary.apps/studio/vitest.config.ts;apps/docs/vitest.config.tshits the same ViteconfigLoader: 'native'warning, and not only for__dirname— also a JSON import without import attributes. The "check the other configs too" line in that entry turned out to describe existing work, not future work.scripts/version-check.mjsis referenced by nothing — notpackage.jsonscripts (pnpm version:checkrunssync-version.mjs, a different file), not any workflow, not the pre-commit hook — and it fails when run, because it assertsengines.node === '>=20'. So the one mechanised guard overengines.nodehas not been running. That is precisely why nothing flagged the jsdom 30 floor.perf-k6.ymlhas failed on every push for at least 30 consecutive runs, includingb124d953from before this batch. Not caused by these merges, not a required check — but a permanently red workflow trains people to ignore red.testTimeoutto 15s; that lifted the ceiling without closing the class. At default turbo concurrencypnpm testproduced 6–8 failures acrossapps/studioandapps/cms, all of themTest timed out in 15000ms.TURBO_CONCURRENCY=1gives 12/12 tasks green, 0 failures, and per-package runs are green too — so it is resource contention, not broken tests. This matters because.husky/pre-commitruns exactlypnpm test: it blocks valid commits at random, and the obvious workaround is--no-verify, which disables the guard entirely. Hit it while making this very commit.Verification
pnpm check:allgreen (settings:checknow reports 16 overrides,drift:check20 declarations vs 16 overrides)pnpm typecheck17/17 ·pnpm build9/9 ·pnpm lintcleanpnpm test12/12 tasks green (serialized — see B32)pnpm audit --prod --audit-level high→ no known vulnerabilitiespnpm docs:i18n:verify→ 0 findings; pair reports up-to-date after stampingNot in scope
Still open by design: #432 (group of 22 — needs the
@types/react-domoverride raised to19.2.5first, itsdrift:checkfailure is legitimate), #416 (framer-motion 13 — green, butAnimatePresencebehaviour changed and landing has no component test infra per B17, so it wants a visual check), #417 (TypeScript 7 —apps/docstsconfig usesbaseUrl, removed in TS 7).