Clear all 7 Dependabot alerts (drop stale npm lockfile, bump js-yaml + brace-expansion)

[than]

Summary

• Deletes package-lock.json — the repo migrated to pnpm in chore: modernize toolchain, drop node-fetch, migrate to pnpm #6 (packageManager: pnpm@11.4.0), but the npm lockfile returned in security: clear js-yaml DoS advisory via override #11 and carried 3 of the 7 alerts on a tree nothing installs from
• pnpm update js-yaml brace-expansion — js-yaml 3.15.1, brace-expansion 1.1.18 / 5.0.9, all past the patched versions; all dev-scope transitives

Verification

• pnpm audit: no known vulnerabilities
• 58 tests pass on this branch (main's suite; the commit message says 71 — that count is from PR v3.1.0: per-identity API tokens, whoami, vendor gitignore #14's branch, my error), npm run build clean

🤖 Generated with Claude Code

[claude]

Review

Scope is right and the change is well-targeted. The only thing that moves in pnpm-lock.yaml is dev-scope transitives — js-yaml 3.14.2 → 3.15.1, brace-expansion 1.1.15/2.1.1/5.0.6 → 1.1.18/2.1.4/5.0.9, plus an incidental react-is 19.2.6 → 19.2.8. The dependencies block (chalk 5.6.2, commander 14.0.3) is byte-identical, and files only ships dist/** + scripts, so the published tarball and the runtime path (pattern detection, hook install, API sync) are untouched. Deleting package-lock.json is safe from a CI standpoint too — neither workflow installs dependencies, and nothing in the repo references the npm lockfile. No src/ changes, so nothing to say on the correctness/TS axes here.

I could not run pnpm install --frozen-lockfile in this environment, so the notes below come from static reading of the manifests and lockfile.

a. The overrides field in package.json is dead config under pnpm

package.json still carries npm's top-level field:

"overrides": { "js-yaml": "^4.1.2" }

pnpm doesn't read top-level overrides — it reads pnpm.overrides in package.json, or the overrides: key in pnpm-workspace.yaml (10.6+). The lockfile confirms it was never applied: js-yaml resolves to 3.15.1 under @istanbuljs/load-nyc-config, and pnpm-lock.yaml has no overrides: block in its header (pnpm writes one whenever overrides are active). With package-lock.json gone, that field now affects zero install paths in this repo while still reading like a live security pin — exactly the kind of thing that gets trusted during the next alert triage.

Suggested fix: delete it rather than port it. 3.15.1 is already past the patched 3.13.1, so the pin buys nothing, and migrating it verbatim would likely be worse — @istanbuljs/load-nyc-config is written against js-yaml 3.x (safeLoad, which 4.x removed), so forcing ^4.1.2 risks breaking coverage config loading. If you do want a pin, put it in pnpm-workspace.yaml under overrides: and verify a coverage run afterward.

b. Nothing stops package-lock.json from coming back

The PR body notes the npm lockfile returned in #11. README.md:275 still tells contributors to run npm install in the dev-setup block, which is the mechanism — npm ignores pnpm-lock.yaml and regenerates its own. Two one-line changes make this cleanup durable instead of a thing you redo next quarter:

• README.md:275 → pnpm install (leave lines 23 and 55 alone — npm install -g git-slot-machine is correct for end users)
• add package-lock.json to .gitignore

Optional, since no CI job currently installs anything: a small job running pnpm install --frozen-lockfile + pnpm test would catch both lockfile drift and a re-added npm lockfile at PR time.

c. Heads-up: brace-expansion 5.x dropped Node 18

The engines field changed with the bump:

- brace-expansion@5.0.6: engines: {node: 18 || 20 || >=22}
+ brace-expansion@5.0.9: engines: {node: 20 || >=22}

package.json still declares engines: { node: ">=18" }. Not a defect — this is dev-scope only (via minimatch under jest/eslint), so published-CLI consumers on Node 18 are unaffected, and with no .npmrc setting engine-strict, pnpm warns rather than fails. But the dev toolchain now effectively wants Node 20+, and with no CI matrix nothing surfaces that to a contributor on 18 beyond an install warning. Worth a line in the README dev section, or bumping the floor deliberately.

Minor

The commit message still claims 71 tests; you corrected it to 58 in the PR body. Worth fixing in the squash message so git log doesn't carry the wrong number.

None of the above blocks merge — (a) is the one I'd want done before this lands, since it's the difference between "js-yaml is pinned" being true and being decoration. I couldn't independently verify the alert counts (the Dependabot API isn't reachable from here), so those rest on your pnpm audit run.

[claude]

Re-review (after a81d9f4)

The follow-up commit does exactly what the earlier review asked for on point (a), and it does it the right way — deleted rather than ported to pnpm-workspace.yaml. Two independent signals confirm the field was genuinely inert rather than "load-bearing but silently satisfied":

• pnpm-lock.yaml's header still has no overrides: block, before or after. pnpm writes one whenever overrides are active, so its absence at both ends means nothing was ever being enforced.
• Removing the field produced zero lockfile churn. A live js-yaml: ^4.1.2 override would have forced a re-resolve of @istanbuljs/load-nyc-config's dependency; instead js-yaml@3.15.1 sits unchanged. Dead config, correctly identified.

And the reasoning in the commit message — that porting it verbatim would have been worse than deleting it — holds: @istanbuljs/load-nyc-config calls safeLoad, which js-yaml 4.x removed, so a real ^4.1.2 override would have broken coverage config loading the moment it started working.

Same caveat as last time: no pnpm binary in this environment, so pnpm install --frozen-lockfile is unverified and the alert counts still rest on your pnpm audit run. Everything above is from reading the manifests and lockfile.

Nothing in src/ moved, and the dependencies block is still byte-identical, so the runtime path (pattern detection, post-commit hook, API sync) is untouched — no correctness surface here.

Still open from the previous review

The lockfile can still come back. README.md:275 continues to tell contributors npm install in the dev-setup block, followed by npm link / npm run build / npm test. That block is the mechanism that regenerated package-lock.json in #11 — npm ignores pnpm-lock.yaml and writes its own. Deleting the file without changing the instruction that recreates it means this cleanup has a shelf life. Switching line 275 to pnpm install (and ideally the three lines under it to pnpm) plus adding package-lock.json to .gitignore is what makes it durable. Lines 23 and 55 are correct as-is — npm install -g git-slot-machine is right for end users.

Node 18 in the dev tree. brace-expansion@5.0.9 narrowed its engines from 18 || 20 || >=22 to 20 || >=22, while package.json still declares engines: { node: ">=18" }. Still not a defect — dev-scope only, via minimatch under jest/eslint, and published-CLI consumers are unaffected. But with no CI job that installs anything, a contributor on Node 18 gets an install warning and no other signal.

Both are follow-ups, not blockers, and both would be caught automatically by the one thing this repo doesn't have: a CI job running pnpm install --frozen-lockfile && pnpm test. That single job covers lockfile drift, a re-added npm lockfile, and the engines floor at PR time — probably higher leverage than any of the individual fixes above.

Aside, out of scope

pnpm-workspace.yaml uses allowBuilds: to unblock unrs-resolver's install script. I can't verify without a pnpm binary, but I don't believe that's a key pnpm reads — the pnpm 10+ field for this is onlyBuiltDependencies: (a list of package names). If that's right, it's the same failure mode you just fixed in package.json: config that reads as enforced and isn't. Worth a pnpm install on a clean store to see whether the build actually runs or is still being blocked. Pre-existing and untouched by this PR — separate change if it pans out.

Minor

The commit message on 67c3c48 still says 71 tests; you corrected it to 58 in the PR body. Worth fixing in the squash message so git log doesn't carry the wrong number.

[claude]

Re-review (no new commits since a81d9f4)

Head is unchanged since the last pass, so this is mostly confirmation. The two prior reviews cover the substance and the author acted on the one blocking item (overrides deleted rather than ported) — I agree with that resolution and won't restate it. Two things I can add.

Verified: the standing --frozen-lockfile caveat

Both earlier reviews closed with "no pnpm binary here, so pnpm install --frozen-lockfile is unverified." That check is decidable statically, and it passes. --frozen-lockfile compares three things against the lockfile:

• Importer specifiers — pnpm-lock.yaml:9-41 lists exactly the ten specifiers in package.json (chalk ^5.3.0, commander ^14.0.3, plus the eight devDependencies), no more, no less.
• settings — autoInstallPeers: true / excludeLinksFromLockfile: false, untouched by this PR.
• overrides — absent from the lockfile header and now absent from config on both sides. This is the part that mattered: removing the overrides field could only have broken the frozen check if the lockfile had recorded one. It didn't, which is the same evidence that proved the field inert.

So the second commit is safe on that axis. Independently, the full inventory of the lockfile diff is three packages — js-yaml 3.14.2 → 3.15.1, brace-expansion 1.1.15/2.1.1/5.0.6 → 1.1.18/2.1.4/5.0.9, react-is 19.2.6 → 19.2.8 — with the dependencies block byte-identical. Nothing in src/ moved, so pattern detection, hook install, and API sync are untouched. No correctness surface.

New: composer.lock is a scanned manifest this PR does not cover

The repo tracks composer.json (laravel/boost ^2.4, dev-only) and a 5,179-line composer.lock resolving 72 PHP packages — "packages": [], everything under packages-dev. Dependabot scans Composer lockfiles automatically, with no dependabot.yml needed.

That matters for the title's claim. "All 7" is sourced from pnpm audit, which cannot see the Composer tree — so it cannot distinguish "7 alerts, all npm" from "7 alerts, some Composer." The body's accounting (3 on package-lock.json, the rest js-yaml/brace-expansion in the npm tree) adds up cleanly if every alert was npm-side, but that is an assumption the verification step did not test. Worth filtering the alert list by dependency.manifest_path before merging under that title — if any alert sits on composer.lock, the title overstates the change and the alert stays open with nobody watching it.

Separately: 72 PHP dev packages is a large scanned surface for a repo whose published artifact is dist/** plus scripts. composer.lock is not in files, so it ships nothing. If laravel/boost is local tooling rather than something contributors need, deleting both files removes an entire ecosystem from alert triage permanently — strictly more leverage than any single bump. Out of scope here; worth a follow-up either way.

Standing follow-ups (unchanged, not blockers)

• README.md:275 still says npm install, which is the mechanism that regenerated package-lock.json in security: clear js-yaml DoS advisory via override #11. pnpm install plus package-lock.json in .gitignore is what makes this durable. Lines 23 and 55 are correct as-is.
• brace-expansion@5.0.9 narrowed engines to 20 || >=22 while package.json declares >=18. Dev-scope only; consumers unaffected.
• A CI job running pnpm install --frozen-lockfile && pnpm test would catch lockfile drift, a re-added npm lockfile, and the engines floor at PR time. Still the highest-leverage item on the list.

Approve. The change does what it says for the npm tree and carries no runtime risk. Confirm the alert manifests match the title before you squash, and fix the 71 → 58 test count in the commit message.