Skip to content

Compute an injected package's copy contents in one place, and read its manifest through one function #220

Open

Description

@zkochan

Found by a defect-hotspot pass over the 90 days to 2026-10-26. This overlaps the freshness-decider task.

Scope change on 2026-10-29. The children half, property (a) as first written, moved to #228: the lockfile records a directory package's specifiers, so the checkers compare them with the manifest instead of deriving children, peers and spec policy. This task keeps the manifest reader, now without children and peers, and the copy contents.

Property

  • (a) Manifest. The resolver, the frozen-lockfile check, the repeat-install check, deploy and every copy writer read a directory package's manifest from one function. That manifest can be a publish directory, an in-memory dependencyManifest, or one rewritten by a readPackage hook. Which children and peers it yields is the resolver's business alone (Record a directory package's specifiers in the lockfile and make the frozen check an equality #228).
  • (b) Contents. Every writer of an injected copy produces the files and links a clean install does. The writers are the install, the sync after scripts, the edit watcher and the dedicated-lockfile sync.

Fixes in the loop

Seed commits: pnpm/pnpm#15356, pnpm/pnpm#15702, pnpm/pnpm#15881.

Issues: pnpm/pnpm#17094, pnpm/pnpm#17057, pnpm/pnpm#17284, pnpm/pnpm#17280, pnpm/pnpm#16683, pnpm/pnpm#17447, pnpm/pnpm#17473.

Design flaw

Proposed change (about 1 week per CLI)

  1. A resolver-owned local_directory_manifest(dir, ctx) returning the hooked manifest. It takes the hooks, publish directory and dependencyManifest into account. The resolver and every checker call it. In v11, lockfile.verification imports it from deps-resolver. Children, peers and spec policy are not part of it: Record a directory package's specifiers in the lockfile and make the frozen check an equality #228 records them.
  2. One function per CLI that decides an injected copy's file set: source directory, packlist mode and symlink rule. The fetcher, restorer, syncer and watcher call it, and callers stop passing the flag.
  3. One helper that locates the injectedDeps records for every lockfile layout.

Tests that would have caught the loop

injected_freshness_matrix.rs and injectedFreshnessMatrix.ts are oracle-based, but their edit list contains only cases that had already broken. The matrix axes for hook source and directory location are part of #228.

  • directory_manifest_reader_agrees_across_callers: for each manifest source (disk, publish directory, in-memory dependencyManifest, readPackage hook, packageExtensions), the resolver, the frozen check and the repeat-install check read the same manifest.
  • injected_copy_writers_agree: vary lockfile layout × publish directory × deployAllFiles × a symlinked tree. Every writer must match a clean install.
  • dedicated_lockfile_check_sees_root_hooks.

Fixes the change makes redundant


Written by an agent (Claude Code, claude-opus-5-5). Scope narrowed on 2026-10-29 by an agent (Claude Code, claude-fable-5-1).

Activity

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

    @zkochan
    MemberAuthor

    pnpm/pnpm#17408 covers item 2 of the proposed change for pnpm v12. pnpm_directory_fetcher::CopyFiles now decides an injected copy's file set (packlist or every file, how symlinks are listed, no path escape). The install, the custom fetcher's directory callback, the sync after scripts, the edit watcher, the dedicated-lockfile sync and pnpm deploy all read their source through it. Behavior does not change.

    This is partial. Still open:

    • items 1 and 3;
    • one symlink rule shared by the importer and DirPatcher;
    • the v11 half. In v11 this would rename includeOnlyPackageFiles on published packages, so it is held back. The PR explains why.

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

  3. zkochan commented on Oct 27, 2026

    @zkochan
    MemberAuthor

    pnpm/pnpm#17431 takes the manifest part of item 1, for pnpm v12. The lockfile checks now read every directory package that no workspace project describes through the resolver's hook chain (ManifestTransformHooks::apply). That covers file: dependencies outside the workspace and anything nested below them. It fixes two v12 bugs that 11.28.9 does not have:

    • a readPackage hook that rewrote such a package's dependencies made --frozen-lockfile fail;
    • ignoredOptionalDependencies on such a package made every repeat install resolve again.

    Status of the rest:


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

  4. changed the title [-]Compute an injected package's children and copy contents in one place[/-] [+]Compute an injected package's copy contents in one place, and read its manifest through one function[/+] on Oct 29, 2026
  5. zkochan commented on Nov 8, 2026

    @zkochan
    MemberAuthor

    Status check against main (2026-11-08).

    Item 2 is done for v12 (pnpm/pnpm#17408), item 3 is done (pnpm/pnpm#17083), and the shared symlink rule landed in pnpm/pnpm#17415.

    Left: item 1, a shared local_directory_manifest reader, and the v11 half.

    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

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