Skip to content

fix(deps): repair corrupted pnpm lockfile and reduce recurrence risk - #53

Merged
Th3Mouk merged 6 commits into
mainfrom
fix/pnpm-lockfile-duplicate-key
Sep 3, 2026
Merged

fix(deps): repair corrupted pnpm lockfile and reduce recurrence risk#53
Th3Mouk merged 6 commits into
mainfrom
fix/pnpm-lockfile-duplicate-key

Conversation

@Th3Mouk

@Th3Mouk Th3Mouk commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Summary

main's pnpm-lock.yaml has had a duplicated tinyglobby@0.2.17 mapping key since 2026-06-03 (commit 9e8d3db, PR #44 — a grouped Dependabot update bumping 5 dev dependencies at once). That's invalid YAML, and it breaks pnpm install --frozen-lockfile — the very first step of the validate CI job.

Every push-triggered CI run on main since that commit has failed at that first step, before lint, typecheck, or tests ever get a chance to run. This includes ordinary human commits, not just Dependabot ones. Fixing that first step peeled back three more layers that had been silently broken the whole time (see below) — CI hadn't actually completed on main in three months.

Why this merged despite CI being red

main has no branch protection (confirmed via the API — 404 Branch not protected). The Dependabot auto-merge workflow uses gh pr merge --auto, which only waits for required status checks. With none configured, GitHub considers a PR mergeable as soon as there's no conflict — independent of whether validate passed. Concretely, on PR #44: validate (22.12.x) and validate (24.x) both show FAILURE, completing at 23:54:21 / 23:54:26, while the PR's mergedAt is 23:54:12 — merged before its own CI even finished.

This PR intentionally does not touch that. No branch protection, no required status checks, nothing that would make auto-merge (or a manual merge) wait or block — per explicit instruction to leave that exactly as-is. It only fixes the corruption itself and reduces (not eliminates) the odds of it recurring, via levers that don't touch the merge path.

Changes

  • pnpm-lock.yaml / npm-shrinkwrap.json: regenerated from the current package.json, which was never itself corrupted — only the lockfile's Dependabot-applied patch was. pnpm install --frozen-lockfile now succeeds.
  • .github/dependabot.yml: removed the prod-dependencies / dev-dependencies groups for the npm ecosystem. A grouped, multi-package update is the most likely trigger for this class of Dependabot+pnpm lockfile-patching bug; single-package PRs are smaller, safer, more independently-verifiable patches. github-actions grouping is untouched — different ecosystem, not implicated here.
  • src/validation/policies.ts, src/workspace/schema/repository.ts: a dead import and an unnecessarily-exported schema, both pre-existing and unrelated — only surfaced now because lint/knip haven't actually run against main in three months.
  • .github/workflows/ci.yml: two related fixes.
    • npm install -g npm@11.19.1 added on every leg: the npm bundled with 22.12.x (10.9.0) crashes ("Cannot read properties of null (reading 'edgesOut')") inside the npm install --package-lock-only that scripts/sync-npm-shrinkwrap.mjs runs — a real npm Arborist bug, reproduced against this repo's unmodified package.json in matching Linux containers, unrelated to any change here. It's invoked both by test:npm-shrinkwrap and, unconditionally on every leg, by test:pack's prepack hook.
    • pnpm test:npm-shrinkwrap pinned to the 24.x leg only: even a non-crashing npm resolves a genuinely different answer on 22.12.x than on 24.x, because a transitive devDependency (@napi-rs/wasm-runtime) gates its newest release behind engines.node: ^22.13.0 || >=23.5.0, which 22.12.0 satisfies neither branch of. There is no single npm-shrinkwrap.json both legs would agree is current, so the check needs one authoritative leg — same pattern already used for pnpm test / pnpm test:coverage:ci. test:pack itself needed no such scoping: its --write mode doesn't compare against the committed file, so the per-leg resolution difference doesn't matter there.
  • src/adapters/git/git-adapter.ts: fixed a separate, unrelated regression found while getting this PR green — repo git pull's autostash-conflict detection string-matched "Applying autostash resulted in conflicts", but git 2.55 (running in CI) reworded that message. Real conflicts were silently going undetected (status: "updated" instead of a thrown error), leaving conflict markers in the working tree. Reproduced across git 2.39/2.45/2.50/2.55 in matching containers; "resulted in conflicts" is the one substring present in both the old and new wording, and --ff-only already rules out the pull's own merge ever producing that text.

Side effect

Properly regenerating the lockfile also re-resolves shell-quote to 1.10.0 (from 1.8.3, already within the existing ^1.8.3 range in package.json) — which happens to fix two unrelated security advisories (one critical, one high) that were stuck because the broken lockfile prevented any resolution from moving for three months. pnpm audit --prod now reports no known vulnerabilities. @types/shell-quote is dropped from devDependencies since shell-quote@1.10.0 ships its own types natively.

Residual risk (stated plainly)

Without something gating the merge path — which is explicitly out of scope here — a future grouped or otherwise unlucky Dependabot update could corrupt the lockfile again and still merge. This PR reduces the odds; it does not eliminate them.

Test plan

  • pnpm install --frozen-lockfile (the exact command CI runs) succeeds
  • pnpm check fully green locally: knip, lint, format, typecheck, audit, build, test, runtime-dependencies, smoke, pack
  • CI green on this PR, both matrix legs (validate 22.12.x, validate 24.x)

Th3Mouk and others added 5 commits September 3, 2026 17:26
main's pnpm-lock.yaml has had a duplicated `tinyglobby@0.2.17` mapping
key since 2026-06-03 (commit 9e8d3db, a grouped Dependabot PR bumping
5 dev dependencies at once). This broke `pnpm install --frozen-lockfile`,
which is the very first step of the validate CI job - every push-triggered
CI run on main since that commit has failed at that step, before lint,
typecheck, or tests ever got a chance to run.

That job failing doesn't block merges: main has no branch protection, so
`gh pr merge --auto` (used by the Dependabot automerge workflow) merges
as soon as GitHub considers a PR mergeable, which for an unprotected
branch has nothing to do with whether validate actually passed. Per
explicit instruction, this fix does not touch that: no branch protection,
no required status checks, nothing that would make auto-merge wait or
block. It only addresses the corruption and the one lever available to
reduce its recurrence without touching the merge path.

Changes:
- pnpm-lock.yaml / npm-shrinkwrap.json: regenerated from the current
  package.json (which was never itself corrupted - only the lockfile's
  patch application broke). This also naturally re-resolves shell-quote
  to 1.10.0, which happens to fix two unrelated advisories (a critical
  and a high severity one) that were stuck because the broken lockfile
  prevented any resolution from moving for three months.
- .github/dependabot.yml: removed the prod-dependencies/dev-dependencies
  groups for the npm ecosystem. Grouped multi-package updates are the
  most likely trigger for this class of Dependabot+pnpm lockfile-patching
  bug; single-package PRs are smaller, safer patches. github-actions
  grouping is untouched (different ecosystem, doesn't touch the pnpm
  lockfile, not implicated here).
- src/validation/policies.ts, src/workspace/schema/repository.ts: a dead
  import and an unnecessarily-exported schema, both pre-existing and
  unrelated to this fix - only surfaced now because lint/knip haven't
  actually run against main in three months.

Residual risk, stated plainly: without some check gating the merge path
(explicitly out of scope here), a future grouped or otherwise unlucky
Dependabot update could corrupt the lockfile again and merge anyway.
This change reduces the odds; it does not eliminate them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ap check

Once the frozen-lockfile install actually succeeds, pnpm test:npm-shrinkwrap
fails on CI's bundled npm (10.9.0, shipped with the Node 22.12.x/24.x
setup-node actions) with:

  Cannot read properties of null (reading 'edgesOut')

thrown from inside `npm install --package-lock-only --ignore-scripts`
(scripts/sync-npm-shrinkwrap.mjs). Reproduced in isolation: npm 10.9.0
crashes on this repo's unmodified package.json every time; npm 11.19.0
and 12.0.2 both resolve it cleanly. This is an npm-side Arborist bug,
not something caused by this repo's dependency graph.

pnpm remains the package manager for everything else in this job - only
the transient npm binary used by the shrinkwrap sync script is upgraded.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
npm@12.0.2 requires Node >=22.22.2, which the 22.12.x leg of the CI
matrix doesn't satisfy - that leg failed with an npm engines error
before even reaching the shrinkwrap check this was meant to fix.
11.19.1 (npm's engines: ^20.17.0 || >=22.9.0) covers both matrix legs
and still resolves the Arborist edgesOut crash - reproduced locally
against this repo's package.json with both 11.19.1 and 12.0.2.
… npm

Node 24.x already bundles npm 11.19.0, which doesn't hit the Arborist
crash - so the npm upgrade step from the previous commit is unnecessary
once the check itself is scoped correctly. It has to be scoped: even a
non-crashing npm resolves a genuinely different answer on 22.12.x than
on 24.x, because a transitive devDependency (@napi-rs/wasm-runtime)
requires Node ^22.13.0 or >=23.5.0 for its newest release - 22.12.0
satisfies neither, so npm falls back to an older version there. There
is no single npm-shrinkwrap.json that both legs would agree is current.

Verified both directions in matching Linux containers (node:22-bullseye,
node:24-bullseye): the committed npm-shrinkwrap.json is exactly what
24.x produces.
Git 2.55 changed the wording of the message git pull --autostash prints
when reapplying the stash conflicts - it no longer starts with "Applying
autostash". The exact-phrase check here missed it entirely, so a real
conflict went undetected: the pull returned status "updated" instead of
throwing, leaving conflict markers in the working tree with no signal
to the caller.

Reproduced across git 2.39, 2.45, 2.50, and 2.55 (the version running
in CI) in matching containers: "resulted in conflicts" is the one
substring present in both the old and new wording. --ff-only already
rules out the pull's own merge ever producing a conflict, so this text
can only come from the autostash pop, same as before.
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Removing it after scoping test:npm-shrinkwrap to 24.x was premature:
test:pack's prepack hook (pnpm sync:npm-shrinkwrap && pnpm build) calls
the same vulnerable npm install --package-lock-only unconditionally on
every leg, regardless of which check triggered it. 22.12.x's bundled
npm still crashes there even though test:npm-shrinkwrap itself no
longer runs on that leg.

The --write mode this hook uses doesn't compare against the committed
file, it just overwrites it locally before packing - so the per-leg
resolution difference (also 22.12.x vs 24.x, same @napi-rs/wasm-runtime
cause) doesn't matter here: verify-packed-artifact.mjs compares the
freshly-written shrinkwrap against what actually got installed from
the tarball in the same job, which are always self-consistent since
both come from the same npm+Node combination.

Verified directly against Node 22.12.0 + npm 11.19.1 (no crash, correct
"out of date" report in --check mode; --write mode gets past the same
install step cleanly).
@Th3Mouk
Th3Mouk merged commit fb41470 into main Sep 3, 2026
7 checks passed
@Th3Mouk
Th3Mouk deleted the fix/pnpm-lockfile-duplicate-key branch September 3, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants