Skip to content
This repository was archived by the owner on Jul 21, 2026. It is now read-only.
This repository was archived by the owner on Jul 21, 2026. It is now read-only.

feat(install): validate .modules.yaml on read — error on layout drift #464

Description

@zkochan

Background

Pacquet's frozen-lockfile install writes node_modules/.modules.yaml (#331) with the resolved nodeLinker, hoistPattern, publicHoistPattern, includedDependencies, storeDir, virtualStoreDir, and virtualStoreDirMaxLength. Roadmap (#299) lists .modules.yaml validation as a remaining Tier-4 polish item:

.modules.yaml validation triggering re-install on layout change. Upstream's validateModules forces a full re-import when nodeLinker / hoistPattern / publicHoistPattern / etc. differ between runs. Pacquet writes .modules.yaml (#331) but does not compare on the read side, so layout-affecting setting changes are silently ignored.

Direct follow-up to #445 (hoisting). After that PR landed, the on-disk manifest records hoistPattern + publicHoistPattern + hoistedDependencies, but pacquet never compares them on the next install. As a result, changing pnpm-workspace.yaml's hoistPattern: ['*']hoistPattern: ['only-this-pkg'] between two pacquet install --frozen-lockfile runs leaves the old hoist symlinks in place — silent layout drift.

What upstream does

Upstream's pipeline at pnpm v11 94240bc046:

  • validateModules.tsModules (read from disk) vs the current install options:
    • modules.virtualStoreDirMaxLength !== opts.virtualStoreDirMaxLengthVIRTUAL_STORE_DIR_MAX_LENGTH_DIFF
    • modules.publicHoistPattern ?? [] != opts.publicHoistPattern ?? []PUBLIC_HOIST_PATTERN_DIFF
    • opts.currentHoistPattern ?? [] != opts.hoistPattern ?? []HOIST_PATTERN_DIFF
    • opts.include[depsField] != modules.included[depsField] (per dependency field) → INCLUDED_DEPS_CONFLICT
    • On any error, opts.forceNewModules decides whether to purge the affected importer's node_modules and re-install, or to fail.
  • checkCompatibility/index.ts — invoked per importer:
    • modules.layoutVersion !== LAYOUT_VERSIONModulesBreakingChangeError
    • modules.storeDir != opts.storeDir (path.relative-aware) → UnexpectedStoreError
    • modules.virtualStoreDir != opts.virtualStoreDirUnexpectedVirtualStoreDirError

virtualStoreOnly installs are exempted from the hoist-pattern check (upstream writes empty patterns deliberately so the next install knows to redo hoisting).

Today

crates/modules-yaml ships read_modules_manifest and the loader handles legacy shamefullyHoist translation, but no install path calls it. crates/package-manager/src/install.rs only writes — it never reads a pre-existing manifest. So all five drift categories above silently pass through pacquet today.

Layout-affecting settings users may flip between installs and not realize:

  • hoistPattern / publicHoistPattern from pnpm-workspace.yaml — the biggest gap; feat: add hoisting support (hoistPattern + publicHoistPattern) (#435) #445's foundation work now records both but the comparison side is missing.
  • nodeLinkerisolatedhoisted (the latter not implemented yet, but the field is persisted).
  • storeDir — relocating the store between installs.
  • Dependency groups (included) — pacquet install --prod followed by pacquet install would silently merge prod-only and dev layouts.

Plan

A. Read + per-importer per-axis compare

  • In Install::run (frozen-lockfile path), call read_modules_manifest::<RealApi>(&config.modules_dir) before the install dispatcher. Ok(None) (no manifest) is the first-install case → no validation.
  • Port validateModules's five axes as a separate validate_modules function in crates/package-manager/src/. Inputs: the loaded Modules, the current Config, the current IncludedDependencies. Output: typed Result<(), ValidateModulesError> with one variant per axis.
  • Port checkCompatibility's three axes (layoutVersion, storeDir, virtualStoreDir) — LayoutVersion already errors on deserialize, but the path comparisons must run for the install to fail with a clean diagnostic rather than fall through.
  • Surface errors with miette codes that match upstream's PUBLIC_HOIST_PATTERN_DIFF / HOIST_PATTERN_DIFF / INCLUDED_DEPS_CONFLICT / VIRTUAL_STORE_DIR_MAX_LENGTH_DIFF / UNEXPECTED_STORE / UNEXPECTED_VIRTUAL_STORE_DIR so the user-facing messages match pnpm.

B. forceNewModules purge path

  • Wire force: bool from pacquet install --force (today a CLI flag exists but only affects extraction). On force, validate-and-purge instead of validate-and-error.
  • Port removeContentsOfDir's allowlist (.bin, .modules.yaml, and the virtual-store-dir resolve-to-equal exception) so the purge doesn't clobber adjacent project metadata.
  • Run the purge per affected importer (rather than the whole node_modules) when the diff is HOIST_PATTERN_DIFF or INCLUDED_DEPS_CONFLICT — upstream's importersToPurge shape.
  • Skip the user-prompt path; pacquet doesn't have an interactive flow (upstream's enquirer.prompt only fires when stdin is a TTY anyway). Always proceed when --force is on.

C. virtualStoreOnly exemption

  • Mirror upstream's !modules.virtualStoreOnly gate on the hoist-pattern diffs. Pacquet doesn't implement virtualStoreOnly install yet, but the field is on Modules — guard against future activation without changing this path.

D. Tests

  • Unit tests for validate_modules covering each diff axis: same/diff hoist pattern, same/diff public hoist pattern, same/diff included groups, same/diff store dir, same/diff virtual store dir, same/diff max-length.
  • Integration test: install with hoistPattern: ['*'], then re-install with hoistPattern: ['banned-only'] → fails with HOIST_PATTERN_DIFF. Add --force variant → purges + re-installs cleanly.
  • Integration test: install with default include, then re-install with --prod → fails with INCLUDED_DEPS_CONFLICT.
  • Port the relevant cases from upstream's installing/deps-installer/test/install/hoist.ts:209 (hoistPattern=* throws exception when executed on node_modules installed w/o the option) and :220 (hoistPattern=undefined throws exception when executed on node_modules installed with hoist-pattern=*). Both are listed in plans/TEST_PORTING.md and currently stubbed as known_failures against partial-install (Partial install with --frozen-lockfile: read+write node_modules/.pnpm/lock.yaml and skip unchanged snapshots #433) — they're actually blocked on this validation, not partial install.

Interactions

Related


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

Metadata

Metadata

Assignees

No one assigned

    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