From 19c440ef9b4fb1c5ee159360f257f66d620c1a61 Mon Sep 17 00:00:00 2001 From: Sami Jawhar Date: Tue, 28 Apr 2026 16:31:11 +0000 Subject: [PATCH] fix(npm): resolve cached package name from install dir for non-registry specs Npm.add cache fast-path used npa(pkg).name as the package name. For non-registry specs (remote tarball URLs, git+https://, github: shorthand, file: paths), npa returns name=undefined and the code fell back to the spec string itself, then looked at /node_modules// which does not exist. Plugins/packages installed from these sources fail to load on the second and subsequent runs, even though the fresh-install path works correctly. The fresh-install path uses tree.edgesOut.values().next().value.to.{name, path} from Arborist. For the cache fast-path, derive the actual installed package name from /package.json's first dependency entry (the same source Npm.install reads to detect dirty installs). Fallback chain: package.json first dep -> npa(pkg).name -> spec string. --- packages/core/src/npm.ts | 24 +++++-- packages/core/test/npm.test.ts | 119 +++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 4 deletions(-) diff --git a/packages/core/src/npm.ts b/packages/core/src/npm.ts index 92e40427681b..9f4e5473622f 100644 --- a/packages/core/src/npm.ts +++ b/packages/core/src/npm.ts @@ -112,16 +112,32 @@ export const layer = Layer.effect( const add = Effect.fn("Npm.add")(function* (pkg: string) { const dir = directory(pkg) - const name = (() => { + const npaName = (() => { try { - return npa(pkg).name ?? pkg + return npa(pkg).name ?? undefined } catch { - return pkg + return undefined } })() if (yield* afs.existsSafe(dir)) { - return resolveEntryPoint(name, path.join(dir, "node_modules", name)) + // For non-registry specs (remote tarball URLs, git+https://, github: + // shorthand, file: paths), npa(pkg).name returns undefined — only + // registry packages have inferable names from the spec alone. Read + // the install-root package.json (written by Arborist on the initial + // install) to recover the actual installed package name from its + // first dependency entry, matching what the fresh-install path + // computes from the arborist tree below. + const cachedPkg = yield* afs.readJson(path.join(dir, "package.json")).pipe(Effect.option) + const installedName = (() => { + if (Option.isSome(cachedPkg)) { + const deps = (cachedPkg.value as { dependencies?: Record })?.dependencies + const first = deps && Object.keys(deps)[0] + if (first) return first + } + return npaName ?? pkg + })() + return resolveEntryPoint(installedName, path.join(dir, "node_modules", installedName)) } const tree = yield* reify({ dir, add: [pkg] }) diff --git a/packages/core/test/npm.test.ts b/packages/core/test/npm.test.ts index 3e94a08692c8..111f5a5ec56f 100644 --- a/packages/core/test/npm.test.ts +++ b/packages/core/test/npm.test.ts @@ -1,6 +1,13 @@ import fs from "fs/promises" +import os from "os" import path from "path" import { describe, expect, test } from "bun:test" +import { Effect, Layer } from "effect" +import { NodeFileSystem } from "@effect/platform-node" +import { AppFileSystem } from "@opencode-ai/core/filesystem" +import { Global } from "@opencode-ai/core/global" +import { EffectFlock } from "@opencode-ai/core/util/effect-flock" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Npm } from "@opencode-ai/core/npm" import { tmpdir } from "./fixture/tmpdir" @@ -54,3 +61,115 @@ describe("Npm.install", () => { await expect(fs.stat(path.join(tmp.path, "node_modules", "dev-pkg"))).rejects.toThrow() }) }) + +describe("Npm.add cache fast-path", () => { + // Build a layer where Global.cache points at the given dir. + // Other Global fields aren't read by Npm.add but Service.of requires them. + function cacheLayer(cacheDir: string) { + const tmp = os.tmpdir() + return Npm.layer.pipe( + Layer.provide(EffectFlock.layer), + Layer.provide(AppFileSystem.layer), + Layer.provide( + Layer.succeed( + Global.Service, + Global.Service.of({ + home: tmp, + data: tmp, + cache: cacheDir, + config: tmp, + state: tmp, + bin: tmp, + log: tmp, + }), + ), + ), + Layer.provide(NodeFileSystem.layer), + Layer.provide(CrossSpawnSpawner.defaultLayer), + ) + } + + // Simulate a populated cache install for `pkg` whose actual installed + // package name is `installedName`. Mirrors what Arborist produces: + // /packages//package.json (declares dep) + // /packages//node_modules//package.json + async function seedCache(cacheDir: string, pkg: string, installedName: string) { + const installRoot = path.join(cacheDir, "packages", Npm.sanitize(pkg)) + const pkgDir = path.join(installRoot, "node_modules", installedName) + await fs.mkdir(pkgDir, { recursive: true }) + await Bun.write( + path.join(installRoot, "package.json"), + JSON.stringify({ dependencies: { [installedName]: pkg } }), + ) + await Bun.write( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: installedName, version: "1.0.0", main: "index.js" }), + ) + await Bun.write(path.join(pkgDir, "index.js"), "module.exports = {}") + return { installRoot, pkgDir } + } + + test("resolves remote tarball URL to actual installed package directory", async () => { + await using tmp = await tmpdir() + const url = "https://example.com/releases/v1.0.0/example-pkg-1.0.0.tgz" + const installed = "@example/scoped-pkg" + const { pkgDir } = await seedCache(tmp.path, url, installed) + + const result = await Effect.runPromise( + Npm.Service.use((svc) => svc.add(url)).pipe(Effect.provide(cacheLayer(tmp.path))), + ) + + expect(result.directory).toBe(pkgDir) + }) + + test("resolves git+https spec to actual installed package directory", async () => { + await using tmp = await tmpdir() + const spec = "git+https://github.com/example/some-repo.git" + const installed = "@example/some-repo" + const { pkgDir } = await seedCache(tmp.path, spec, installed) + + const result = await Effect.runPromise( + Npm.Service.use((svc) => svc.add(spec)).pipe(Effect.provide(cacheLayer(tmp.path))), + ) + + expect(result.directory).toBe(pkgDir) + }) + + test("resolves github: shorthand to actual installed package directory", async () => { + await using tmp = await tmpdir() + const spec = "github:example/another-repo" + const installed = "another-repo" + const { pkgDir } = await seedCache(tmp.path, spec, installed) + + const result = await Effect.runPromise( + Npm.Service.use((svc) => svc.add(spec)).pipe(Effect.provide(cacheLayer(tmp.path))), + ) + + expect(result.directory).toBe(pkgDir) + }) + + test("resolves local tarball file: spec to actual installed package directory", async () => { + await using tmp = await tmpdir() + const spec = "file:/some/local/path/example-1.0.0.tgz" + const installed = "@example/local-pkg" + const { pkgDir } = await seedCache(tmp.path, spec, installed) + + const result = await Effect.runPromise( + Npm.Service.use((svc) => svc.add(spec)).pipe(Effect.provide(cacheLayer(tmp.path))), + ) + + expect(result.directory).toBe(pkgDir) + }) + + test("still resolves registry packages correctly", async () => { + await using tmp = await tmpdir() + const spec = "prettier" + const { pkgDir } = await seedCache(tmp.path, spec, spec) + + const result = await Effect.runPromise( + Npm.Service.use((svc) => svc.add(spec)).pipe(Effect.provide(cacheLayer(tmp.path))), + ) + + expect(result.directory).toBe(pkgDir) + }) +})