fix(arborist): skip lockfile entries for optional deps with incomplete manifests - #9343
Merged
Merged
Conversation
owlstronaut
reviewed
May 13, 2026
ecanturk
force-pushed
the
fix/optional-missing-version
branch
3 times, most recently
from
May 14, 2026 06:12
e988ac0 to
4ce71e2
Compare
…e manifests
When using a proxy/upstream registry, fetching manifests for platform-specific
optional dependencies that the proxy has not cached can return incomplete
metadata (missing version field). This caused npm to write lockfile entries like:
"node_modules/@esbuild/aix-ppc64": { "optional": true }
without a version, resolved, or integrity field. Subsequent `npm ci` runs would
fail with "Invalid Version:" errors.
Fix (both layers are independently needed):
1. build-ideal-tree: When a registry manifest lacks a version field, treat it
as a load failure (EINCOMPLETEMANIFEST) so that #pruneFailedOptional() marks
it inert. Only applies to registry specs, not file: dependencies which may
legitimately omit version.
2. shrinkwrap: In commit(), inventory iteration does not filter inert nodes, so
a separate guard skips writing entries where the node is optional, has no
version, is not the root, and has a registry-resolved URL (https://). Local
deps (file: or resolved=null) are preserved.
Closes: npm#9342
ecanturk
force-pushed
the
fix/optional-missing-version
branch
from
May 14, 2026 06:41
4ce71e2 to
8f34f13
Compare
owlstronaut
suggested changes
May 15, 2026
Co-authored-by: Michael Smith <owlstronaut@github.com>
owlstronaut
approved these changes
May 15, 2026
Contributor
|
🎉 Backport to |
nicolasalt
added a commit
to nikzlabs/shipit
that referenced
this pull request
Sep 7, 2026
## Summary Renovate's agent-CLI PR #2663 omitted a Claude Code optional platform record and required manual lockfile repair. Reproduce the cause, retain the working CI gate, and make recovery clear at the failure instead of adding branch-writing automation. Closes planning#523 ## Rationale The confirmed cause is **npm resolution under Renovate's seven-day `--before` cutoff**, not a libc filter or Renovate deleting a record after generation. Claude Code 2.1.252 was published on 2026-08-31 at 17:07:28.168Z, ARM64 musl at 17:05:01.263Z, and ARM64 glibc at 17:30:23.691Z. The bot commit at 2026-09-07T17:20:07Z falls inside that publication gap shifted by seven days. On Linux x64, npm 11.15.0 with the previous lock, updated manifest, and Renovate's parent-record preparation produces a lock **byte-identical to bot commit 84be11e** at cutoff 17:20:07Z. Changing only the cutoff to 17:31:00Z produces a lock **byte-identical to repair 8edd772**. npm 11.15.0 is the controlled reproduction version; the hosted job's exact npm version was not available. npm 11.12.1 reproduces the omission but also writes an empty optional placeholder, covered by upstream npm/cli#9343. The installed-tree issue npm/cli#4828 is not needed: these reproductions have no node_modules. History contains two distinct defective updates among 38 reachable lockfile-changing commits, including release copies and incident PR commits: 2.1.232 (three records, repaired by 18e97fe) and 2.1.252 (one record). **Accept the occasional failed initial PR and reduce recovery work. This change does not guarantee complete initial Renovate PRs.** Running npm twice repeats the cutoff; a longer cooldown moves the same race. Rebasing on every base update is conditional and adds CI work. Hosted Renovate cannot use postUpgradeTasks, while branch-writing automation needs write credentials and coordination with force pushes. That upkeep is not justified by the observed incidents. Keep exact pins, the seven-day policy, file-based grouping, automerge off, and both CI/release validation gates. Prefer Renovate rebase/retry after all platform releases clear the cutoff; unlike a manual fix commit, that does not rely on a patch that a later bot rebase can discard. ## Changes - CI: name the nested lockfile check and add an error annotation with bot retry and manual repair instructions. Run the same npm ci command and preserve its original exit status so the existing gate stays strict. - CLI strategy guide: replace the unproven registry-cache explanation with the reproduced cutoff cause, record the decision against extra automation, and explain manual-commit/rebase behavior. - Recovery: include --min-release-age=7 in manual regeneration so the fallback also applies the cutoff to newly resolved optional packages. Document the older npm empty-placeholder error and how to undo only a failed repair attempt before retrying. - Release workflow: point its unchanged validation gate to the same recovery guidance. Mark this documentation/check improvement complete in the existing checklist. ## Test plan - Reproduced the original npm ci EUSAGE at 84be11e; verified the missing ARM64 glibc record and the 16-line repair. - Compared npm 11.15.0 early/late cutoff output with both historical files using cmp: both exact matches. - Confirmed a second install at the early cutoff does not repair the missing record. - Ran the changed CI shell on the broken fixture and current lock: exit 1 with annotation / exit 0 without annotation, with neither lock modified. An injected unrelated npm failure retains exit 42. - Tested the manual repair with npm 11.12.1 and a controlled clock: early repair fails validation; late repair matches the fixed lock exactly and passes validation. Documented the empty-placeholder limitation found by this test. - Parsed both workflows as YAML; npm run lint:dev (no changed TypeScript); npm run typecheck; git diff --check. - Independent ShipIt reviewer confirmed the cause and recommendation; addressed the recovery cutoff, release guidance, and documentation findings. Full incident record and upstream references: nikzlabs/shipit-planning#523.
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
2 participants
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.
What
When using a proxy/upstream registry (e.g. Azure Artifacts), npm 11 writes lockfile entries for platform-specific optional dependencies without
version,resolved, orintegrityfields. Subsequentnpm cifails withInvalid Version:errors.Example broken lockfile entry:
Why
#fetchManifest()inbuild-ideal-tree.jsfetches manifests for ALL platform variants of optional dependencies. Proxy registries that haven't cached packages for non-current platforms return incomplete metadata (missingversionfield). npm creates a Node with emptypkg.version, andmetaFromNode()writes{"optional": true}to the lockfile without version.npm 10 never attempted to resolve non-current-platform variants, so this only affects npm 11+.
How
Two-layer defense:
build-ideal-tree.js(#nodeFromSpec): When a registry manifest lacks aversionfield, treat it as anEINCOMPLETEMANIFESTload failure so that#pruneFailedOptional()marks it inert. Only applies tospec.registryspecs (notfile:dependencies which may legitimately omit version).shrinkwrap.js(commit()): Skip writing entries where the node is optional, has no version, and is not the root. This acts as a defense-in-depth layer.Testing
workspaces/arborist/test/shrinkwrap.js— simulates proxy registry scenario where an optional dep node has no versionshrinkwrap.js45/45,build-ideal-tree.js114/114)Closes #9342