fix: keep the path of git-hosted tarball resolutions in the lockfile (#12344)

* fix: keep the path of git-hosted tarball resolutions in the lockfile

* test(lockfile): cover git-hosted tarball path preservation in to_lockfile_form

Adds regression tests guarding that to_lockfile_form keeps the
TarballResolution.path of git-hosted subdirectory tarballs in both the
default and include_tarball_url branches, mirroring the TypeScript
toLockfileResolution tests added for pnpm/pnpm#12304.

* refactor(lockfile): simplify toLockfileResolution to a single kept-URL return

Invert the branching so the canonical-registry case is the early return and
the kept-URL resolution (integrity + tarball + optional gitHosted/path) is the
single fall-through. This removes the keepTarballUrl closure and the
preservingGitHosted helper; the URL-reconstruction check moves into
isCanonicalRegistryTarballUrl. No behavior change.

* refactor(lockfile): mirror the single kept-URL return in to_lockfile_form

Apply the same inversion as the TypeScript toLockfileResolution: the
canonical-registry case becomes the early return and the kept-URL resolution
is the single fall-through, removing the keep_url closure. The URL-match check
moves into is_canonical_registry_tarball_url. No behavior change; the three
to_lockfile_form tests still pass.

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/git-tarball-path-in-lockfile.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/lockfile.utils": patch
+"pnpm": patch
+---
+
+Git dependencies that point to a subdirectory of a repository (`repo#commit&path:/sub/dir`) keep their `path` in the lockfile again. Since the integrity of git-hosted tarballs started being pinned in the lockfile, any install that actually downloaded the tarball rebuilt the lockfile resolution as `{ integrity, tarball, gitHosted }` and dropped the `path` field, while installs served from the store kept it — so the field disappeared seemingly at random. Without `path`, later installs from that lockfile silently unpacked the repository root instead of the subdirectory [#12304](https://github.com/pnpm/pnpm/issues/12304).

lockfile/utils/src/toLockfileResolution.ts

@@ -26,49 +26,46 @@ export function toLockfileResolution (
   // legacy lockfiles read by callers that don't enrich the field).
   const gitHosted = (resolution as TarballResolution).gitHosted === true ||
     isGitHostedTarballUrl(tarball)
-  if (lockfileIncludeTarballUrl) {
-    return preservingGitHosted({
-      integrity: resolution['integrity'],
-      tarball,
-    }, gitHosted)
-  }
-  // Tarball URLs that cannot be reconstructed from the package name, version,
-  // and registry must always stay in the lockfile, otherwise the package can
-  // no longer be re-fetched. This covers local `file:` tarballs and tarballs
-  // served by git providers (GitHub, GitLab, Bitbucket).
-  if (tarball.startsWith('file:') || gitHosted) {
-    return preservingGitHosted({
-      integrity: resolution['integrity'],
-      tarball,
-    }, gitHosted)
-  }
-  // Sometimes packages are hosted under non-standard tarball URLs.
-  // For instance, when they are hosted on npm Enterprise. See https://github.com/pnpm/pnpm/issues/867
-  // Or in other weird cases, like https://github.com/pnpm/pnpm/issues/1072.
-  // Even when the user explicitly sets `lockfileIncludeTarballUrl: false`, we
-  // must preserve such URLs — otherwise the package cannot be re-fetched on a
-  // frozen-lockfile install (e.g. GitHub Packages tarballs at
-  // `https://npm.pkg.github.com/download/<scope>/<name>/<version>/<hash>`).
-  // `lockfileIncludeTarballUrl` only controls whether URLs that *can* be
-  // derived from name+version+registry are written.
-  const expectedTarball = getNpmTarballUrl(pkg.name, pkg.version, { registry })
-  const actualTarball = tarball.replaceAll('%2f', '/')
-  if (removeProtocol(expectedTarball) !== removeProtocol(actualTarball)) {
-    return preservingGitHosted({
-      integrity: resolution['integrity'],
-      tarball,
-    }, gitHosted)
+  // A standard registry tarball whose URL can be rebuilt from the package name,
+  // version, and registry is written as just `{ integrity }` — pnpm derives the
+  // URL on demand. Every other tarball must keep its URL or it can no longer be
+  // re-fetched on a frozen-lockfile install: `file:` tarballs, git-provider
+  // tarballs (GitHub/GitLab/Bitbucket), and non-standard registry URLs such as
+  // npm Enterprise (https://github.com/pnpm/pnpm/issues/867) or GitHub Packages
+  // `/download/` URLs. `lockfileIncludeTarballUrl` forces the URL to be kept.
+  if (
+    !lockfileIncludeTarballUrl &&
+    !gitHosted &&
+    !tarball.startsWith('file:') &&
+    isCanonicalRegistryTarballUrl(tarball, pkg, registry)
+  ) {
+    return { integrity: resolution['integrity'] }
   }
+  // The kept-URL form carries the `gitHosted` marker and the subdirectory `path`
+  // (`repo#commit&path:/sub/dir`, only ever set on git-hosted tarballs) so a
+  // git-hosted monorepo tarball still unpacks the right subfolder.
+  // See https://github.com/pnpm/pnpm/issues/12304.
+  const { path } = resolution as TarballResolution
   return {
     integrity: resolution['integrity'],
+    tarball,
+    ...(gitHosted ? { gitHosted: true } : {}),
+    ...(path == null ? {} : { path }),
   }
 }
 
-function preservingGitHosted<T extends { tarball?: string, integrity: string }> (
-  resolution: T,
-  gitHosted: boolean
-): T & { gitHosted?: boolean } {
-  return gitHosted ? { ...resolution, gitHosted: true } : resolution
+// Whether `tarball` is the canonical npm registry URL derived from the package
+// name, version, and registry — i.e. it can be dropped from the lockfile and
+// rebuilt on demand. The `%2f` unescape matches the URLs npm produces for
+// scoped packages.
+function isCanonicalRegistryTarballUrl (
+  tarball: string,
+  pkg: { name: string, version: string },
+  registry: string
+): boolean {
+  const expectedTarball = getNpmTarballUrl(pkg.name, pkg.version, { registry })
+  const actualTarball = tarball.replaceAll('%2f', '/')
+  return removeProtocol(expectedTarball) === removeProtocol(actualTarball)
 }
 
 // Inlined to avoid pulling @pnpm/fetching.pick-fetcher into the lockfile-utils

lockfile/utils/test/toLockfileResolution.ts

@@ -105,6 +105,37 @@ test('keeps git-hosted tarballs when lockfileIncludeTarballUrl is false', () =>
   })
 })
 
+test('keeps the path of a git-hosted tarball pointing to a subdirectory', () => {
+  // The path selects the subdirectory to extract from a monorepo tarball
+  // (`repo#commit&path:/sub/dir`). Dropping it makes later installs silently
+  // unpack the repository root. See https://github.com/pnpm/pnpm/issues/12304.
+  expect(toLockfileResolution(
+    { name: 'foo', version: '1.0.0' },
+    { integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', gitHosted: true, path: '/packages/foo' },
+    REGISTRY,
+    false
+  )).toEqual({
+    integrity: 'sha512-AAAA',
+    tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef',
+    gitHosted: true,
+    path: '/packages/foo',
+  })
+})
+
+test('keeps the path of a git-hosted tarball when lockfileIncludeTarballUrl is true', () => {
+  expect(toLockfileResolution(
+    { name: 'foo', version: '1.0.0' },
+    { integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', gitHosted: true, path: '/packages/foo' },
+    REGISTRY,
+    true
+  )).toEqual({
+    integrity: 'sha512-AAAA',
+    tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef',
+    gitHosted: true,
+    path: '/packages/foo',
+  })
+})
+
 test('records gitHosted on the lockfile entry when set on the resolution', () => {
   expect(toLockfileResolution(
     { name: 'foo', version: '1.0.0' },

pacquet/crates/lockfile/src/resolution.rs

@@ -299,24 +299,31 @@ impl LockfileResolution {
 
         let git_hosted =
             tarball.git_hosted == Some(true) || is_git_hosted_tarball_url(&tarball.tarball);
-        let keep_url = || {
-            LockfileResolution::Tarball(TarballResolution {
-                tarball: tarball.tarball.clone(),
-                integrity: Some(integrity.clone()),
-                git_hosted: git_hosted.then_some(true),
-                path: tarball.path.clone(),
-            })
-        };
-
-        if include_tarball_url || tarball.tarball.starts_with("file:") || git_hosted {
-            return keep_url();
+        // A standard registry tarball whose URL can be rebuilt from name+version+
+        // registry is written as just `{integrity}` — pnpm derives the URL on
+        // demand. Every other tarball must keep its URL or it can no longer be
+        // re-fetched on a frozen-lockfile install: `file:` tarballs, git-provider
+        // tarballs, and non-standard registry URLs (npm Enterprise, GitHub Packages
+        // `/download/` URLs). `include_tarball_url` forces the URL to be kept.
+        if !include_tarball_url
+            && !git_hosted
+            && !tarball.tarball.starts_with("file:")
+            && is_canonical_registry_tarball_url(&tarball.tarball, name, version, registry)
+        {
+            return LockfileResolution::Registry(RegistryResolution {
+                integrity: integrity.clone(),
+            });
         }
-        let expected = npm_tarball_url(name, version, registry);
-        let actual = tarball.tarball.replace("%2f", "/");
-        if remove_protocol(&expected) != remove_protocol(&actual) {
-            return keep_url();
-        }
-        LockfileResolution::Registry(RegistryResolution { integrity: integrity.clone() })
+        // The kept-URL form carries the `git_hosted` marker and the subdirectory
+        // `path` (`repo#commit&path:/sub/dir`, only ever set on git-hosted tarballs)
+        // so a git-hosted monorepo tarball still unpacks the right subfolder.
+        // See <https://github.com/pnpm/pnpm/issues/12304>.
+        LockfileResolution::Tarball(TarballResolution {
+            tarball: tarball.tarball.clone(),
+            integrity: Some(integrity.clone()),
+            git_hosted: git_hosted.then_some(true),
+            path: tarball.path.clone(),
+        })
     }
 }
 
@@ -335,6 +342,21 @@ pub fn npm_tarball_url(name: &str, version: &str, registry: &str) -> String {
     format!("{registry}{name}/-/{scopeless}-{version}.tgz")
 }
 
+/// Whether `tarball` is the canonical npm registry URL derived from `name`,
+/// `version`, and `registry` — i.e. it can be dropped from the lockfile and
+/// rebuilt on demand. The `%2f` unescape matches the URLs npm produces for
+/// scoped packages.
+fn is_canonical_registry_tarball_url(
+    tarball: &str,
+    name: &str,
+    version: &str,
+    registry: &str,
+) -> bool {
+    let expected = npm_tarball_url(name, version, registry);
+    let actual = tarball.replace("%2f", "/");
+    remove_protocol(&expected) == remove_protocol(&actual)
+}
+
 /// Default-vs-scope routing for an npm package. Mirrors pnpm's
 /// [`pickRegistryForPackage`](https://github.com/pnpm/pnpm/blob/main/config/pick-registry-for-package/src/index.ts).
 ///

pacquet/crates/lockfile/src/resolution/tests.rs

@@ -666,3 +666,53 @@ fn libc_matches_truth_table() {
     assert!(!libc_matches(None, Some("uclibc")));
     assert!(!libc_matches(Some("glibc"), Some("uclibc")));
 }
+
+const SHA512: &str = "sha512-gf6ZldcfCDyNXPRiW3lQjEP1Z9rrUM/4Cn7BZbv3SdTA82zxWRP8OmLwvGR974uuENhGCFgFdN11z3n1Ofpprg==";
+
+/// A reconstructible registry tarball URL is dropped, leaving only the
+/// integrity, so the path-preserving cases below are not just returning the
+/// input unchanged.
+#[test]
+fn to_lockfile_form_drops_reconstructible_registry_tarball() {
+    let resolution = LockfileResolution::Tarball(TarballResolution {
+        tarball: "https://registry.npmjs.org/foo/-/foo-1.0.0.tgz".to_string(),
+        integrity: Some(integrity(SHA512)),
+        git_hosted: None,
+        path: None,
+    });
+    let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", false);
+    assert_eq!(
+        actual,
+        LockfileResolution::Registry(RegistryResolution { integrity: integrity(SHA512) }),
+    );
+}
+
+/// The `path` selects the subdirectory to extract from a monorepo tarball
+/// (`repo#commit&path:/sub/dir`). Dropping it makes later installs silently
+/// unpack the repository root. See
+/// <https://github.com/pnpm/pnpm/issues/12304>.
+#[test]
+fn to_lockfile_form_keeps_git_hosted_subdirectory_path() {
+    let resolution = LockfileResolution::Tarball(TarballResolution {
+        tarball: "https://codeload.github.com/foo/bar/tar.gz/abc1234".to_string(),
+        integrity: Some(integrity(SHA512)),
+        git_hosted: Some(true),
+        path: Some("/packages/foo".to_string()),
+    });
+    let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", false);
+    assert_eq!(actual, resolution);
+}
+
+/// `include_tarball_url` takes the same kept-URL branch, so it must keep
+/// `path` too.
+#[test]
+fn to_lockfile_form_keeps_git_hosted_subdirectory_path_when_including_tarball_url() {
+    let resolution = LockfileResolution::Tarball(TarballResolution {
+        tarball: "https://codeload.github.com/foo/bar/tar.gz/abc1234".to_string(),
+        integrity: Some(integrity(SHA512)),
+        git_hosted: Some(true),
+        path: Some("/packages/foo".to_string()),
+    });
+    let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", true);
+    assert_eq!(actual, resolution);
+}