Skip to content

Read injected dependencies from the lockfile and stop inverting the resolver in the frozen check #145

Closed

Description

@zkochan

Found by the defect-hotspots skill (pnpm/pnpm#17021) on 2026-10-17 over 90 days of main (086165da7a). The root cause comes from reading the commits and code; it was not reproduced. The design steps are proposals for a maintainer to settle.

Loop: Injected workspace dependencies (about 20 fixes, 2 open)

Properties.

  • A: non-frozen and frozen installs agree on whether the lockfile matches every injected package's manifest, with shared and per-project lockfiles.
  • B: an injected copy holds the publish directory's packlist whichever path wrote it.

Members.

Flaw.

Design change.

  1. Read injection from the lockfile (injected_deps(lockfile, importer)). This fixes injectWorkspacePackages: true skips changed injected dependency manifests during optimistic install (12.14.0) pnpm#17007 in one PR.
  2. One InjectedSource per directory, used by the resolver, freshness, fetch, and both syncers.
  3. Record an injected package's declared specifiers in its packages: entry. Freshness becomes plain equality, and v11 can drop the frozen skip, which fixes 11.x: --frozen-lockfile accepts stale injected workspace dependencies with separate lockfiles pnpm#17008.

Tests.

  • Sequence matrix in both stacks: {shared, per-project lockfile} × {three injection forms} × {edit dep, peer, optional peer, catalog peer, none} × {filtered, not}. Steps: install, mutate, install, then frozen install from a clean tree.
  • Copy-equivalence: every writer's copy equals the packlist.

The shared test harnesses that cover this loop and the others are tracked in #141.


Written by an agent (Claude Code, claude-opus-5-5).

Activity

  1. self-assigned this
    on Oct 17, 2026
  2. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 1 (read injection from the lockfile) is in pnpm/pnpm#17025, which also fixes pnpm/pnpm#17007. Steps 2 and 3 are still open.


    Written by an agent (Claude Code, claude-opus-5-5).

  3. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 2, property B (file selection): pnpm/pnpm#17033 makes every writer of an injected copy select the files the install selects. That covers the lifecycle refresh during install, the syncInjectedDepsAfterScripts sync, and the edit watcher, in both v11 and v12. The symlink difference it leaves is filed separately. pnpm/pnpm#17022 is the v11 fix for pnpm/pnpm#17008, and it has a changes-requested review.


    Written by an agent (Claude Code, claude-opus-5-5).

  4. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Progress:


    Written by an agent (Claude Code, claude-opus-5-5).

  5. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Progress:


    Written by an agent (Claude Code, claude-opus-5-5).

  6. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    The last step-2 item is not a bug in practice. Only @pnpm/napi embedders can make publishConfig differ between a project's importer manifest and its dependencyManifest. The embedders that use dependencyManifest (Bit) strip workspace-sibling dependencies, not publishConfig. The resolver reads publishConfig from the dependency view. The syncer reads it from the importer manifest, which is the only one the lifecycle runner holds. Plumbing the other view through for a divergence no caller produces isn't worth it now. If an embedder ever needs it, that's where to start.

    Step 2 is done apart from that. #147 tracks the symlink follow-ups. What remains is step 3: record an injected package's declared specs in its packages: entry and make freshness an equality check. That changes the lockfile format in both versions.


    Written by an agent (Claude Code, claude-opus-5-5).

  7. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 3 design proposal: record an injected package's declared dependencies

    Status: proposal for a maintainer to settle. Nothing implemented.

    Goal

    The lockfile freshness check for an injected (directory-resolved) package becomes an equality comparison between what the lockfile recorded and what the package declares now. Today it compares the manifest against the resolved snapshot by inverting the resolver's rules:

    Importers already work by equality: each records its specifier.

    Proposed format

    Only packages: entries with a directory resolution get the new field:

    packages:
      lib@file:packages/lib:
        resolution: {directory: packages/lib, type: directory}
        declared:
          dependencies:
            is-positive: ^1.0.0
            shared: 'catalog:'
          optionalDependencies:
            fsevents: ^2.3.0
          peerDependencies:
            react: '>=18'
          peerDependenciesMeta:
            react: {optional: true}
    • Name. It can't be dependencies: v11 merges packages: and snapshots: entries in memory, and snapshots already use dependencies for resolved versions. Alternatives are specifiers (mirroring the importers' specifier) or manifest.
    • What is recorded. The manifest fields as the resolver read them, before any hook:

    Why raw, pre-hook values

    Every transformation between the raw manifest and the snapshot is already tracked by its own lockfile field:

    Input Already tracked by
    overrides overrides:
    packageExtensions packageExtensionsChecksum
    pnpmfile readPackage pnpmfileChecksum, untrackedPnpmfileReadPackageHook
    catalogs catalogs:, importers only, see question 3

    Recording raw values means the freshness check never runs hooks. That would let the injected_manifests hooking step from pnpm/pnpm#17047 go away again.

    Freshness after the change

    • Field present: the entry is fresh if and only if the current raw fields equal the recorded ones. This is stricter than today: a spec edit that the locked version still satisfies (^1.0.0 → ^1.0.1 with 1.0.5 locked) triggers a re-resolve, as it does for importers.
    • Field absent (a lockfile written before this change, or rewritten by an older pnpm): today's logic stays as the fallback. That's v12's inverse check, and v11's frozen skip (or fix: reject frozen install when an injected package's dependencies drift with separate lockfiles pnpm#17022 if it lands).
    • Removing the inverse check therefore waits for a lockfile version that guarantees the field.

    Compatibility

    • Older versions drop the field. Both current CLIs drop an unknown packages: field when they save the lockfile: v12's PackageMetadata has no catch-all, and v11's convertToLockfileObject keeps an allow-listed set of keys. In a team mixing versions, the field appears and disappears in diffs until everyone upgrades. The fallback keeps frozen installs working throughout.
    • Both CLIs at once. It ships in v11 and v12 together so both write the same lockfile.

    Questions to settle

    1. The field name and shape (declared, specifiers, or manifest), and whether peers are recorded raw inside it as proposed.
    2. Raw (pre-hook) values, as proposed, or the resolver's post-hook manifest. Post-hook values would keep the freshness check running hooks.
    3. Catalogs: extend catalogs: to the catalog entries injected packages use, which keeps the recorded values raw. The alternative is to record catalog: specs already dereferenced, which mixes raw and resolved values in one field.
    4. Old lockfiles on --frozen-lockfile: keep the fallback as proposed, or fail and require one non-frozen install. Failing would break CI for every injected workspace until it re-locks.
    5. Lockfile version: stay on 9.0 and rely on the fallback, or bump it so that a missing field can be treated as outdated and the inverse check deleted sooner.
    6. fix: reject frozen install when an injected package's dependencies drift with separate lockfiles pnpm#17022: land it as v11's fallback for old lockfiles, or close it in favor of this.

    Written by an agent (Claude Code, claude-opus-5-5).

  8. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 3, revised: share the resolver's rules instead of recording specs

    Status: replaces the proposal above, which is withdrawn. A lockfile field would be additive for readers, but older pnpm versions drop it when they save. Mixed-version teams would see it churn, and the hand-written check could only go after a lockfile-version bump.

    Approach

    The freshness check of an injected (directory-resolved) package calls the resolver's own rules instead of inverting them by hand. The resolver already has both halves:

    Question Resolver code today Freshness code today
    Which dependencies does this manifest ask for? child_specs_of: extract_children, specs shadowed by the package's own peers dropped, catalog: specs resolved directory_deps.rs check_local_dep_group_freshness, peers.rs PeerShadowing
    Can the locked version serve this spec? keeps_locked_version, the reuse gate spec.rs spec_satisfies_snapshot_dep

    Both resolver functions currently take the resolver's internal context. The work is to split out context-free versions that the resolver and the freshness check both call. After that, peers.rs and spec.rs are deleted. A resolver rule added later applies to the check without a second edit, which is the loop this task describes.

    No lockfile change: no new field, no lockfileVersion bump, and nothing changes for older pnpm versions.

    Plan

    1. v12: extract the two functions from resolving-deps-resolver into context-free form. First, run them beside the current check in the freshness tests and the injected e2e suites, so differences surface. Then switch the check over and delete the hand-written rules.
    2. v11: the same move against its resolver (getNonDevWantedDependencies, peer handling, and the locked-version reuse check), replacing the body of localDirectoryDepIsUpToDate. fix: reject frozen install when an injected package's dependencies drift with separate lockfiles pnpm#17022 either lands first and gets simplified by this, or is folded in.

    Still unchanged

    The check keeps running pnpmfile hooks and packageExtensions, because the resolver runs them. pnpm/pnpm#17047 supplies the hooked manifests for a project with its own lockfile.


    Written by an agent (Claude Code, claude-opus-5-5).

  9. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 3 progress (revised approach). The v12 part is done:

    • fix(package-manager): read an injected package's children the way the resolver does pnpm#17053: the freshness check of an injected package takes its children from the resolver: manifest_child_specs, peer shadowing, and catalogs through one walked_child_specs shared with the walk and the warm-up. This fixed frozen-install rejections for bundledDependencies and for a name declared in both dependencies and optionalDependencies.
    • fix(package-manager): judge an injected package's locked versions the way the resolver does pnpm#17062: the check asks the resolver's reuse gate (locked_child_admits) whether a locked version still satisfies a spec. This fixed named-registry spec changes going unnoticed, and the resolver reusing the wrong package for a bare named-registry spec.
    • Still hand-written: file:, link: and workspace: specs (spec.rs) and the peer comparison (peers.rs, which already uses the resolver's peer_shadowed_dependency_names and own_dependency_names). The resolver has no reuse gate for local specs, because it resolves them every time.

    Open: the v11 part. Either localDirectoryDepIsUpToDate calls v11's resolver functions, or pnpm/pnpm#17022 lands as v11's fix (it has a changes-requested review).


    Written by an agent (Claude Code, claude-opus-5-5).

  10. zkochan commented on Oct 17, 2026

    @zkochan
    MemberAuthor

    Step 3 progress: the v11 part is in pnpm/pnpm#17065 (draft until CI is green).

    • localDirectoryDepIsUpToDate now uses the resolver's rules, as the v12 check does:
      • getNonDevWantedDependencies and prepareDependencyManifest (newly extracted) decide the children.
      • peerDependenciesWithoutOwn gives the peers.
      • lockedVersionSatisfies, built on getRangeOfLockedPackage, judges a locked version.
    • The frozen install no longer skips the check, which fixes 11.x: --frozen-lockfile accepts stale injected workspace dependencies with separate lockfiles pnpm#17008. skipLocalDirectoryDependencies is removed.
    • Two v12 fixes found along the way:
      • A child that became optional is now outdated on a frozen install.
      • A frozen install of a file: directory without a package.json used to fail. It now passes.

    After pnpm/pnpm#17065, step 3 is done in both stacks. The sequence-matrix tests are still open.


    Written by an agent (Claude Code, claude-opus-5-5).

  11. zkochan commented on Oct 18, 2026

    @zkochan
    MemberAuthor

    pnpm/pnpm#17065 merged, so step 3 is done in both stacks, and pnpm/pnpm#17008 is closed. Two things changed from the earlier progress note:

    • pnpm deploy still skips the check. A deploy lockfile records bound peers as plain dependencies, so the check would reject it.
    • In both stacks, a git, tarball, or dist-tag spec can't be judged from the lockfile. A --frozen-lockfile install accepts what the lockfile recorded. A normal install re-resolves.

    Next: the sequence-matrix tests from the issue. In v12 they will follow repeat_install_oracle.rs (#141). The copy-equivalence tests are still open too.


    Written by an agent (Claude Code, claude-opus-5-5).

  12. zkochan commented on Oct 18, 2026

    @zkochan
    MemberAuthor

    The sequence-matrix tests are in pnpm/pnpm#17078, for both CLIs. Each runs 60 cases: {shared, per-project lockfile} × {three injection forms} × {no edit, a dependency, a peer, an optional peer, a catalog peer} × {filtered, full}.

    Still open: the copy-equivalence tests.


    Written by an agent (Claude Code, claude-opus-5-5).

  13. zkochan commented on Oct 18, 2026

    @zkochan
    MemberAuthor

    The copy-equivalence tests are in pnpm/pnpm#17083, for both CLIs. Every writer of an injected copy must leave exactly the files the install selects: the install with its refresh after prepare, the sync after a listed script, the same sync after a source edit, and a frozen install into a clean tree. The cases cover {shared, per-project lockfile} × {isolated, hoisted} × deployAllFiles.

    The tests found a bug in both CLIs: with sharedWorkspaceLockfile: false, the sync after a syncInjectedDepsAfterScripts script, and the edit watcher, never updated the injected copies. The same PR fixes it.

    That is the last open item from this task.


    Written by an agent (Claude Code, claude-opus-5-5).

  14. zkochan commented on Oct 18, 2026

    @zkochan
    MemberAuthor

    All steps are done:

    Follow-ups tracked separately:


    Written by an agent (Claude Code, claude-opus-5-5).

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions