Skip to content

update-packages: ncu bump forks Yarn transitive resolutions and leaves peer ranges behind #110

Description

@cratis-direct

Found while fixing the Chronicle.TypeScript daily update run (Cratis/Chronicle.TypeScript actions run 36236292822, red since 2026-09-19). Two gaps in update-packages.yml's NPM path, both reproducible from a clean checkout:

1. ncu -u + yarn install can fork a transitive dependency into two instances.

The update step bumps the direct range in the manifest, then yarn install resolves it. Yarn keeps the existing lockfile resolutions for transitive descriptors that still satisfy their ranges, so the bumped direct dependency becomes a new descriptor alongside the old one:

  • Source/package.json asks for @grpc/grpc-js@^1.14.5 → new lockfile entry 1.14.5, hoisted to the root
  • nice-grpc still asks for @grpc/grpc-js@^1.14.0 → existing entry 1.14.4 kept, nested under node_modules/nice-grpc/

Two instances of a package whose classes carry private state are nominally incompatible, so tsc fails with TS2322/TS2345 on every call that hands a value from one instance to the other (nodeLinker: node-modules here). Running yarn dedupe after the update-install merges the descriptors again and would prevent this class of failure for every Yarn repository using this workflow, not just the one I fixed.

2. npm-check-updates does not update peerDependencies, so ranges drift behind the pins they describe.

npx npm-check-updates -u -w -x ... uses the default dependency-type set, which excludes peer. Since ncu skips a range that already satisfies the latest version, a package that appears both as a pinned devDependency and as a peer range gets its pin bumped while the peer range stays put. Chronicle.TypeScript has a spec asserting peerDependencies["@cratis/fundamentals"] === "^" + devDependencies pin (the range is a compatibility claim, so it must not admit versions below the tested one), so every within-range fundamentals release fails the update run. --dep prod,dev,peer,optional fixes it; I worked around it in-repo with an .ncurc.yml declaring dep: [prod, dev, peer, optional], which the workflow's own command picks up.

Reference fix in the caller repository: Cratis/Chronicle.TypeScript#117.

Activity

  1. cratis-direct commented on Sep 26, 2026

    @cratis-direct
    Author

    Thanks for opening this - it has been received and will be looked at.


    Posted by Direct (AI) - an autonomous agent, not a person. Review accordingly.

  2. cratis-direct commented on Sep 26, 2026

    @cratis-direct
    Author

    Investigation

    Both reported defects reproduce against the workflow's exact commands, and both proposed fixes verify. Everything needed for a confident fix is settled — no blocking questions.

    Reproduction evidence (all with the workflow's real commands, not code reading):

    1. Forked transitive resolution — reproduced. Built a Yarn 4.18.1 fixture (nodeLinker: node-modules) with @grpc/grpc-js (direct) + nice-grpc@2.1.0 (depends on @grpc/grpc-js@^1.7.3), with the lockfile holding a merged entry at 1.14.4 (the state a repo is in before 1.14.5, published 2026-09-17, matching the reported run). After the manifest bump ^1.14.0 → ^1.14.5 + yarn install, the lockfile forked: ^1.14.5 → 1.14.5 hoisted at root, ^1.7.3 → 1.14.4 nested — two instances on disk (node_modules/@grpc/grpc-js and node_modules/nice-grpc/node_modules/@grpc/grpc-js). yarn dedupe then merged both descriptors into one entry at 1.14.5 with a single instance; a second dedupe was a byte-identical no-op (exit 0).
    2. Peer range drift — reproduced. ncu 23.1.0 with the workflow's exact npx npm-check-updates -u -w -x typescript bumped a pinned devDependencies.ms 2.1.2 → 2.1.3 while leaving peerDependencies.ms: ^2.1.2 behind. With --dep prod,dev,peer,optional both moved together (2.1.3 / ^2.1.3), preserving the peer === "^" + pin invariant Chronicle.TypeScript asserts.
    3. Guard requirement — measured. yarn dedupe on Yarn 1.22.22 exits 1 ("The dedupe command isn't necessary"), so an unguarded call would fail the whole update run for Yarn 1 callers; it exists since Yarn 3.1. Also measured: ncu's default set currently does auto-bump packageManager pins (npm 9.0.0 → 12.1.0); --dep prod,dev,peer,optional stops that — a deliberate, documentable narrowing since a package-manager pin is a reviewed choice (and yarn's npm dist-tag is still 1.22.22, so Yarn pins were never bumpable anyway).

    SUGGESTED-TIER: balanced

    Plan

    1. .github/workflows/update-packages.yml — "Update NPM packages" step (the only ncu call site)

    • Change npx npm-check-updates -u -w -x "$exclusions" → npx npm-check-updates -u -w --dep prod,dev,peer,optional -x "$exclusions", with a comment: peer ranges must move with the pins they describe (compatibility claims must not admit versions below the tested one); packageManager pins are deliberately excluded as a reviewed choice.
    • After yarn install, add a version-guarded dedupe:
    case "$(yarn --version | cut -d. -f1)" in
      1 | 2) echo "Yarn $(yarn --version) has no dedupe command; skipping the transitive merge." ;;
      *) yarn dedupe ;;
    esac

    with a comment explaining the fork mechanism (new descriptor from the bump + lockfile retention for still-satisfying transitive descriptors → two instances of private-state-carrying packages → TS2322/TS2345), that dedupe is a no-op when nothing needs merging, and that it shipped in Yarn 3.1 (Yarn 1 exits 1; Yarn 2 predates it — those keep today's plain-install behavior; a 3.0.x caller, if any exists, fails visibly rather than silently).

    • Both modes keep working unchanged: the freeze step's git add --update and direct mode's git add -A pick up dedupe's lockfile merge; dedupe runs before builds, so yarn ci still validates the final tree.

    2. .github/scripts/tests/update-packages.test.py — extend the offline harness

    • Stub: print YARN_VERSION (default 4.18.1) for yarn --version; add the new exact ncu command string to the stub's update-command set.
    • Update test_npm_exclusions_are_data_not_shell_source's expected argv to include --dep, prod,dev,peer,optional.
    • New test: ncu → yarn install → yarn dedupe appear in that order.
    • New test: with YARN_VERSION set to 1.22.22 and 2.4.3, the step still succeeds and never calls yarn dedupe.
    • Keep the existing structural assertion that the step contains no ${{ (new lines are pure bash).

    3. README.md — "Reviewed package updates" section

    Add ~3 lines documenting the NPM contract: prod/dev/peer/optional updated in one pass so peer ranges track their pins; packageManager pins never auto-updated; Yarn Berry (3.1+) callers get a post-install dedupe so a bumped direct range cannot leave a second older transitive instance.

    4. Verification

    • python3 .github/scripts/tests/update-packages.test.py (baseline currently 45/45 OK) and bootstrap-package-update-safety.test.py; actionlint on update-packages.yml and verify-package-updates.yml (already path-triggered in verify-package-updates.yml, which picks up both edited files).
    • The end-to-end behavior of both fixes is already verified against real Yarn 4.18.1 / Yarn 1.22.22 / ncu 23.1.0 as described above.

    Settled — do not redo

    • Both bugs and both fixes are empirically verified (evidence above); the caller-side workaround in Chronicle.TypeScript#117 stays as is — the workflow fix protects every Yarn caller without it.
    • Repo conventions to respect: tests execute the actual inline Bash offline with stubbed toolchains; exit codes propagate; no output swallowing; documentation updated with the change.

    Judgment calls made (flagged, not blocking)

    • packageManager leaves the auto-update set as a side effect of the issue's own proposed --dep list — treated as intentional and documented in README and the step comment.
    • The dedupe guard keys on Yarn major version (1/2 skip) rather than an unguarded call, because Yarn 1's yarn dedupe exits 1 and would break every Yarn 1 caller's daily update.

    Suggested tier for implementation: balanced


    Posted by Direct (AI) - an autonomous agent, not a person. Review accordingly.

  3. self-assigned this
    on Oct 1, 2026
  4. woksin commented on Oct 2, 2026

    @woksin
    Contributor

    Fixed by #134.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions