build: restore libc discriminator on linux lockfile entries#1163
Conversation
npm 11 silently strips the `libc` field from @optave/codegraph-linux-* entries in package-lock.json on `npm install`, even though the published packages declare it correctly. Without the discriminator, npm cannot tell linux-x64-gnu apart from linux-x64-musl when resolving the lockfile, so the wrong native binary may load on Alpine/musl hosts. Restore the field and add a CI guard (run before `npm install` in the lint job) that fails any PR which lands a lockfile missing the discriminator — Dependabot bumps and contributor installs would otherwise silently regress this again. Closes #1160
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #1163: build: restore libc discriminator on linux lockfile entries Root Cause AnalysisProblem Being Solved: npm 11+ silently strips Why This Approach: Direct field restoration + CI verification guard is justified because:
Risk Assessment: Low risk - addresses real compatibility issue with appropriate safeguards Phase 2: Configuration Change Detection ✅No HIGH-RISK changes detected:
Phase 3: Test Integrity ✅No test integrity concerns:
Phase 4: Code Quality ✅scripts/verify-lockfile-libc.mjs:47 security review:
CI integration:
package-lock.json changes:
Phase 5: Backlog Compliance
Final Recommendation
Strengths:
No blocking concerns identified. |
Greptile SummaryThis PR restores the
Confidence Score: 5/5Safe to merge — restores three lockfile fields and adds a focused CI guard with no changes to runtime application code. The lockfile edits are mechanical field restorations. The verifier script is self-contained, reads a fixed path, and has a clear pass/fail contract. The CI placement (before npm install) is the correct ordering to catch the regression on the committed file. No application logic is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Checkout repo\nlockfile has libc field] --> B[Setup Node.js 22]
B --> C[node scripts/verify-lockfile-libc.mjs]
C --> D{libc field\npresent & correct?}
D -- YES --> E[npm install\nnpm 11 strips libc on disk]
D -- NO --> F[Exit 1\nPR blocked]
E --> G[npx @biomejs/biome check]
G --> H[lint job passes]
Reviews (4): Last reviewed commit: "fix: resolve lockfile path relative to s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| '@optave/codegraph-linux-x64-musl': 'musl', | ||
| }; | ||
|
|
||
| const lock = JSON.parse(readFileSync('package-lock.json', 'utf8')); |
There was a problem hiding this comment.
readFileSync('package-lock.json', 'utf8') resolves against the process working directory. Running node scripts/verify-lockfile-libc.mjs from the scripts/ subdirectory (a natural mistake) throws ENOENT: no such file or directory, open 'package-lock.json' with no hint of the real problem. In CI this is fine since the step always runs from the repo root, but a contributor running it locally from the wrong directory gets a confusing error rather than a clear failure message.
There was a problem hiding this comment.
Good catch — fixed in b72df7e. The script now resolves package-lock.json relative to import.meta.url so it works from any CWD (repo root, scripts/ subdirectory, or anywhere else). Verified manually from all three locations.
| const EXPECTED_LIBC = { | ||
| '@optave/codegraph-linux-arm64-gnu': 'glibc', | ||
| '@optave/codegraph-linux-x64-gnu': 'glibc', | ||
| '@optave/codegraph-linux-x64-musl': 'musl', | ||
| }; |
There was a problem hiding this comment.
arm64-musl not covered by the verifier
The CI parity job's inline Node script already maps linux-arm64-musl → @optave/codegraph-linux-arm64-musl, suggesting the package is expected to ship. If that package is added to the lockfile in a future PR, the verifier won't catch npm 11 stripping its libc: ["musl"] field — the same silent regression this PR is designed to prevent. Adding it to EXPECTED_LIBC now (even if the package is currently missing from the lockfile, the missing from package-lock.json branch handles that gracefully) would make the guard future-proof.
There was a problem hiding this comment.
Good future-proofing thought, but adding @optave/codegraph-linux-arm64-musl to EXPECTED_LIBC today would actually break CI: the package isn't in package.json's optionalDependencies yet, so it's absent from package-lock.json, and the current verifier treats a missing entry as a hard failure (failures.push(${pkgName}: missing from package-lock.json)), not a graceful skip.
The CI parity-job mapping references the package optimistically, but it's not actually shipped today. I've opened #1168 to track adding it to EXPECTED_LIBC the moment it lands in optionalDependencies, and to consider whether the "missing" branch should be relaxed to a warn-and-skip so future additions can be made future-proof without immediately failing.
Use import.meta.url to find package-lock.json regardless of the process CWD. Previously running 'node scripts/verify-lockfile-libc.mjs' from the scripts/ subdirectory failed with a confusing ENOENT instead of running the actual check.
1 participant
Summary
libcfield on@optave/codegraph-linux-{arm64-gnu,x64-gnu,x64-musl}entries inpackage-lock.json. npm 11 silently strips this field onnpm install, even though the published packages declare it (npm view @optave/codegraph-linux-x64-musl libc→'musl').scripts/verify-lockfile-libc.mjsand wire it into thelintCI job before thenpm installstep, so any PR (including Dependabot bumps) that lands a lockfile missing the discriminator now fails fast.Without the discriminator, npm cannot disambiguate
linux-x64-gnuvslinux-x64-muslwhen resolving from the lockfile, so the wrong native binary may load on Alpine/musl hosts.Investigation: the strip behavior is reproducible with npm 11.6.0 even after
npm cache clean --force, and affects other napi-rs projects (e.g.@biomejs/cli-linux-*entries in this same lockfile). There is no root-package.jsonfield that constrains libc on optional deps, so the only robust fix is to commit the field directly and guard regressions in CI.Closes #1160
Test plan
node scripts/verify-lockfile-libc.mjspasses on the new lockfilenpm install --package-lock-only --ignore-scripts(which is why the CI step runs before install)