From ec6473e95ede22e87b9ea41bca2b53474fb8fda5 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 6 May 2026 02:00:36 +0200 Subject: [PATCH] fix: align git-hosted store readers with the integrity-keyed writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the lockfile pins integrity for git-hosted tarballs, every post-fetch reader (pkg-finder, store status, FUSE mounter, skipIfHasSideEffectsCache in building/after-install) looks the package up under storeIndexKey(integrity, pkgId). Three issues were exposed: 1. pkg-finder ran parse(packageId) and rebuilt name@version, but packageIdFromSnapshot returns the URL alone for git-hosted (the `:` in `https://` makes tryGetPackageId strip the `name@` prefix). parse(URL) returns {} and the lookup key became `…\tundefined@undefined`. Use packageId directly — that's the form the writer in package-requester uses. 2. The FUSE handler and the after-install side-effects-cache lookup used `${name}@${version}`, which doesn't match the resolver's pkg.id for git-hosted (URL). Use `nonSemverVersion ?? ${name}@${version}` so the readers compute the same key the writer uses. 3. On the first install, integrity isn't known when getFilesIndexFilePath runs, so the writer parks the index under gitHostedStoreIndexKey. Once the worker computes integrity, the git-hosted fetcher now also registers the entry under storeIndexKey(integrity, pkgId), so subsequent integrity-based lookups (the same install or any later one) resolve to the same data without a refetch. The license test that walks the lockfile right after install now passes locally; that's the case that was failing in CI. --- Written by an agent (Claude Code, claude-opus-4-7). --- building/after-install/src/index.ts | 5 +++- fetching/fetcher-base/src/index.ts | 7 ++++++ .../src/gitHostedTarballFetcher.ts | 18 ++++++++++++++- .../package-requester/src/packageRequester.ts | 1 + .../daemon/src/createFuseHandlers.ts | 8 ++++++- store/pkg-finder/src/index.ts | 7 +++--- .../test/readPackageFileMap.test.ts | 23 +++++++++++++++++++ 7 files changed, 63 insertions(+), 6 deletions(-) diff --git a/building/after-install/src/index.ts b/building/after-install/src/index.ts index 474341e965..9e4ef1c3ba 100644 --- a/building/after-install/src/index.ts +++ b/building/after-install/src/index.ts @@ -358,7 +358,10 @@ async function _rebuild ( } const resolution = (pkgSnapshot.resolution as TarballResolution) let sideEffectsCacheKey: string | undefined - const pkgId = `${pkgInfo.name}@${pkgInfo.version}` + // Match the resolver-supplied pkg.id used by the writer in + // @pnpm/installing.package-requester: that's the tarball URL for + // git-hosted packages (nonSemverVersion) and `name@version` otherwise. + const pkgId = pkgInfo.nonSemverVersion ?? `${pkgInfo.name}@${pkgInfo.version}` if (opts.skipIfHasSideEffectsCache && resolution.integrity) { const filesIndexFile = storeIndexKey(resolution.integrity!.toString(), pkgId) const pkgFilesIndex = storeIndex!.get(filesIndexFile) as PackageFilesIndex | undefined diff --git a/fetching/fetcher-base/src/index.ts b/fetching/fetcher-base/src/index.ts index d28f284ddd..d213b40b20 100644 --- a/fetching/fetcher-base/src/index.ts +++ b/fetching/fetcher-base/src/index.ts @@ -15,6 +15,13 @@ export interface PkgNameVersion { export interface FetchOptions { allowBuild?: AllowBuild filesIndexFile: string + /** + * Resolution id of the package being fetched. The git-hosted tarball fetcher + * uses this to register a second store index entry under + * `storeIndexKey(integrity, pkgId)` once the integrity has been computed, + * so subsequent integrity-based lookups can find the package. + */ + pkgId?: string lockfileDir: string onStart?: (totalSize: number | null, attempt: number) => void onProgress?: (downloaded: number) => void diff --git a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts index c570b8391a..9129079684 100644 --- a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts @@ -6,7 +6,7 @@ import type { FetchFunction, FetchOptions } from '@pnpm/fetching.fetcher-base' import { packlist } from '@pnpm/fs.packlist' import { globalWarn } from '@pnpm/logger' import type { Cafs, FilesMap } from '@pnpm/store.cafs-types' -import type { StoreIndex } from '@pnpm/store.index' +import { type StoreIndex, storeIndexKey } from '@pnpm/store.index' import type { BundledManifest } from '@pnpm/types' import { addFilesFromDir } from '@pnpm/worker' @@ -38,6 +38,22 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction if (prepareResult.ignoredBuild) { globalWarn(`The git-hosted package fetched from "${resolution.tarball}" has to be built but the build scripts were ignored.`) } + // Register a second index entry under storeIndexKey(integrity, pkgId). + // The original filesIndexFile is gitHostedStoreIndexKey-derived (the + // resolver doesn't know the integrity yet on a first install), but every + // post-fetch reader (pkg-finder, store status, FUSE mounter, + // skipIfHasSideEffectsCache in building/after-install) looks the package + // up under the integrity key once the lockfile carries integrity. Without + // this duplicate, those readers raise ENOENT immediately after install. + if (integrity && opts.pkgId) { + const integrityKey = storeIndexKey(integrity, opts.pkgId) + if (integrityKey !== opts.filesIndexFile) { + const data = fetcherOpts.storeIndex.get(opts.filesIndexFile) + if (data) { + fetcherOpts.storeIndex.set(integrityKey, data) + } + } + } return { filesMap: prepareResult.filesMap, manifest: prepareResult.manifest ?? manifest, diff --git a/installing/package-requester/src/packageRequester.ts b/installing/package-requester/src/packageRequester.ts index 662eb17da8..7e0ff5a95a 100644 --- a/installing/package-requester/src/packageRequester.ts +++ b/installing/package-requester/src/packageRequester.ts @@ -554,6 +554,7 @@ function fetchToStore ( { allowBuild: opts.allowBuild, filesIndexFile, + pkgId: opts.pkg.id, lockfileDir: opts.lockfileDir, readManifest: opts.fetchRawManifest, onProgress: (downloaded) => { diff --git a/modules-mounter/daemon/src/createFuseHandlers.ts b/modules-mounter/daemon/src/createFuseHandlers.ts index d580bb0f9a..1b4ac84aee 100644 --- a/modules-mounter/daemon/src/createFuseHandlers.ts +++ b/modules-mounter/daemon/src/createFuseHandlers.ts @@ -176,7 +176,13 @@ export function createFuseHandlersFromLockfile (lockfile: LockfileObject, storeD const pkgSnapshot = lockfile.packages?.[depPath as DepPath] if (pkgSnapshot == null) return undefined const nameVer = nameVerFromPkgSnapshot(depPath, pkgSnapshot) - const pkgIndexFilePath = storeIndexKey((pkgSnapshot.resolution as TarballResolution).integrity!, `${nameVer.name}@${nameVer.version}`) + // Match the resolver-supplied pkg.id used by the writer in + // @pnpm/installing.package-requester: that's the tarball URL for + // git-hosted packages (nonSemverVersion) and `name@version` otherwise. + const pkgIndexFilePath = storeIndexKey( + (pkgSnapshot.resolution as TarballResolution).integrity!, + nameVer.nonSemverVersion ?? `${nameVer.name}@${nameVer.version}` + ) const pkgIndex = storeIndex.get(pkgIndexFilePath) as PackageFilesIndex pkgSnapshotCache.set(depPath, { ...nameVer, diff --git a/store/pkg-finder/src/index.ts b/store/pkg-finder/src/index.ts index 7912a18eb0..a225221978 100644 --- a/store/pkg-finder/src/index.ts +++ b/store/pkg-finder/src/index.ts @@ -1,6 +1,5 @@ import path from 'node:path' -import { parse } from '@pnpm/deps.path' import { fetchFromDir } from '@pnpm/fetching.directory-fetcher' import type { Resolution } from '@pnpm/resolving.resolver-base' import { getFilePathByModeInCafs, type PackageFilesIndex } from '@pnpm/store.cafs' @@ -48,10 +47,12 @@ export async function readPackageFileMap ( let pkgIndexFilePath: string if (isPackageWithIntegrity) { - const parsedId = parse(packageId) + // The writer in @pnpm/installing.package-requester keys the index file by + // the resolution id (`name@version` for npm tarballs, the tarball URL for + // git-hosted ones), so we must use the same key here. pkgIndexFilePath = storeIndexKey( packageResolution.integrity as string, - parsedId.nonSemverVersion ?? `${parsedId.name}@${parsedId.version}` + packageId ) } else if (!packageResolution.type && 'tarball' in packageResolution && packageResolution.tarball) { pkgIndexFilePath = gitHostedStoreIndexKey(packageId, { built: true }) diff --git a/store/pkg-finder/test/readPackageFileMap.test.ts b/store/pkg-finder/test/readPackageFileMap.test.ts index a6728bb146..9b89303dce 100644 --- a/store/pkg-finder/test/readPackageFileMap.test.ts +++ b/store/pkg-finder/test/readPackageFileMap.test.ts @@ -64,6 +64,29 @@ describe('readPackageFileMap', () => { expect(result!.has('index.js')).toBe(true) }) + it('should resolve git-hosted tarball packages with integrity', async () => { + // After we started pinning integrity for git-hosted tarballs, the + // packageId here is the URL alone (tryGetPackageId strips the `name@` + // because the URL contains `:`). The reader must therefore key by the + // URL — the same key the writer used. + const integrity = 'sha512-abc123gitHosted' + const pkgId = 'https://codeload.github.com/stevemao/left-pad/tar.gz/cafe1234' + const key = storeIndexKey(integrity, pkgId) + + storeIndex.set(key, createFilesIndex()) + + const resolution: TarballResolution = { + integrity, + tarball: pkgId, + } + + const result = await readPackageFileMap(resolution, pkgId, defaultOpts()) + + expect(result).toBeDefined() + expect(result!.has('package.json')).toBe(true) + expect(result!.has('index.js')).toBe(true) + }) + it('should resolve git-hosted tarball packages (no type, has tarball)', async () => { const pkgId = 'left-pad@https://codeload.github.com/stevemao/left-pad/tar.gz/abc123' const key = gitHostedStoreIndexKey(pkgId, { built: true })