fix(core): preserve npm lock when plugin SDK resolves locally

.specgit.yaml

@@ -1,8 +1,8 @@
 version: 1
-delivery: event-retention-reclamation
+delivery: preserve-npm-lock
 context:
   kind: branch
-  branch: feat/524-event-retention-reclamation
+  branch: feat/541-preserve-npm-lock
 issues:
-  - 524
-pr: 537
+  - 541
+pr: 542

packages/core/src/npm.ts

@@ -217,7 +217,12 @@ export const layer = Layer.effect(
         remaining.push(pkg)
       }
 
-      if (remaining.length !== requested.length) {
+      // Only a mixed batch (part of the deps pinned locally/bundled, part still
+      // going to the registry) needs the lock dropped so the registry deps are
+      // re-resolved. An all-local/all-bundled batch must leave a valid lock
+      // untouched, or every startup deletes and rebuilds it; an all-registry
+      // batch never deleted it either.
+      if (remaining.length > 0 && remaining.length < requested.length) {
         yield* fs.remove(path.join(dir, "package-lock.json")).pipe(Effect.orElseSucceed(() => undefined))
       }
 

packages/core/test/npm.test.ts

@@ -71,6 +71,48 @@ describe("Npm.add", () => {
   })
 })
 
+interface AddSpec {
+  name: string
+  version?: string
+}
+
+const lockSnapshot = async (lockPath: string) => {
+  const [bytes, stat] = await Promise.all([fs.readFile(lockPath), fs.stat(lockPath)])
+  return { bytes: bytes.toString(), ino: stat.ino, mtimeMs: stat.mtimeMs }
+}
+
+const install = (dir: string, cache: string, add: AddSpec[] = []) =>
+  Effect.gen(function* () {
+    const npm = yield* Npm.Service
+    yield* npm.install(dir, add.length ? { add } : undefined)
+  }).pipe(Effect.scoped, Effect.provide(npmLayer(cache)), Effect.runPromise)
+
+// Seeds a project whose plugin dependency resolves from an existing local copy
+// (declared via a file: spec so the initial reify stays offline), then builds a
+// real package-lock.json through the genuine forcing path: one install with a
+// missing file: dependency reaches reify and writes the lock arborist owns.
+const seedPluginProject = async (dir: string) => {
+  const localPlugin = path.join(dir, "local-plugin")
+  await fs.mkdir(localPlugin, { recursive: true })
+  await writePackage(localPlugin, { name: PluginSdk.packageName, main: "index.js" })
+  await Bun.write(path.join(localPlugin, "index.js"), "export const plugin = true\n")
+
+  const helper = path.join(dir, "helper-dep")
+  await fs.mkdir(helper, { recursive: true })
+  await writePackage(helper, { name: "fixture-helper-dep", main: "index.js" })
+  await Bun.write(path.join(helper, "index.js"), "export const helper = true\n")
+
+  await writePackage(dir, {
+    name: "fixture",
+    dependencies: {
+      [PluginSdk.packageName]: "file:./local-plugin",
+      "fixture-helper-dep": "file:./helper-dep",
+    },
+  })
+
+  await install(dir, path.join(dir, "cache"), [{ name: "fixture-helper-dep", version: "file:./helper-dep" }])
+}
+
 describe("Npm.install", () => {
   test("respects omit from project .npmrc", async () => {
     await using tmp = await tmpdir()
@@ -96,38 +138,103 @@ describe("Npm.install", () => {
     await expect(fs.stat(path.join(tmp.path, "node_modules", "dev-pkg"))).rejects.toThrow()
   })
 
-  test("skips registry when plugin dependency already exists locally", async () => {
+  test("preserves package-lock across consecutive installs when plugin dependency already exists locally", async () => {
     await using tmp = await tmpdir()
-    await fs.mkdir(path.join(tmp.path, "node_modules", "@opencode-ai", "plugin"), { recursive: true })
-    await writePackage(path.join(tmp.path, "node_modules", "@opencode-ai", "plugin"), { name: "@opencode-ai/plugin" })
+    await seedPluginProject(tmp.path)
 
-    await Effect.gen(function* () {
-      const npm = yield* Npm.Service
-      yield* npm.install(tmp.path, { add: [{ name: "@opencode-ai/plugin", version: "1.17.11-main.3" }] })
-    }).pipe(Effect.scoped, Effect.provide(npmLayer(path.join(tmp.path, "cache"))), Effect.runPromise)
+    const lockPath = path.join(tmp.path, "package-lock.json")
+    const bootstrapped = await lockSnapshot(lockPath)
+
+    await install(tmp.path, path.join(tmp.path, "cache"), [
+      { name: PluginSdk.packageName, version: "1.17.11-main.3" },
+    ])
+    const afterFirst = await lockSnapshot(lockPath)
 
-    await expect(fs.stat(path.join(tmp.path, "package-lock.json"))).rejects.toThrow()
+    await install(tmp.path, path.join(tmp.path, "cache"), [
+      { name: PluginSdk.packageName, version: "1.17.11-main.3" },
+    ])
+    const afterSecond = await lockSnapshot(lockPath)
+
+    expect(afterFirst).toEqual(bootstrapped)
+    expect(afterSecond).toEqual(afterFirst)
   })
 
-  test("copies bundled plugin dependency before registry fallback", async () => {
+  test("copies bundled plugin dependency before registry fallback and preserves package-lock on re-install", async () => {
     await using tmp = await tmpdir()
+    await seedPluginProject(tmp.path)
     const bundled = path.join(tmp.path, "bundled-plugin-sdk")
+    const previous = process.env.OPENCODE_PLUGIN_SDK_PATH
     process.env.OPENCODE_PLUGIN_SDK_PATH = bundled
     await fs.mkdir(path.join(bundled, "src"), { recursive: true })
-    await writePackage(bundled, { name: "@opencode-ai/plugin", exports: { ".": "./src/index.ts", "./tui": "./src/tui.ts" } })
+    await writePackage(bundled, { name: PluginSdk.packageName, exports: { ".": "./src/index.ts", "./tui": "./src/tui.ts" } })
     await Bun.write(path.join(bundled, "src", "index.ts"), "export const plugin = true\n")
     await Bun.write(path.join(bundled, "src", "tui.ts"), "export const tui = true\n")
 
     try {
-      await Effect.gen(function* () {
-        const npm = yield* Npm.Service
-        yield* npm.install(tmp.path, { add: [{ name: "@opencode-ai/plugin" }] })
-      }).pipe(Effect.scoped, Effect.provide(npmLayer(path.join(tmp.path, "cache"))), Effect.runPromise)
+      await fs.rm(path.join(tmp.path, "node_modules"), { recursive: true, force: true })
+      const lockPath = path.join(tmp.path, "package-lock.json")
+      const bootstrapped = await lockSnapshot(lockPath)
+
+      await install(tmp.path, path.join(tmp.path, "cache"), [{ name: PluginSdk.packageName }])
+      await expect(
+        fs.stat(path.join(tmp.path, "node_modules", PluginSdk.packageName, "src", "tui.ts")),
+      ).resolves.toBeDefined()
+      const afterFirst = await lockSnapshot(lockPath)
 
-      await expect(fs.stat(path.join(tmp.path, "node_modules", "@opencode-ai", "plugin", "src", "tui.ts"))).resolves.toBeDefined()
-      await expect(fs.stat(path.join(tmp.path, "package-lock.json"))).rejects.toThrow()
+      await install(tmp.path, path.join(tmp.path, "cache"), [{ name: PluginSdk.packageName }])
+      const afterSecond = await lockSnapshot(lockPath)
+
+      expect(afterFirst).toEqual(bootstrapped)
+      expect(afterSecond).toEqual(afterFirst)
     } finally {
-      delete process.env.OPENCODE_PLUGIN_SDK_PATH
+      if (previous === undefined) delete process.env.OPENCODE_PLUGIN_SDK_PATH
+      else process.env.OPENCODE_PLUGIN_SDK_PATH = previous
     }
   })
+
+  test("still reifies package-lock when a local plugin install is mixed with a missing dependency", async () => {
+    await using tmp = await tmpdir()
+    await seedPluginProject(tmp.path)
+
+    const extra = path.join(tmp.path, "extra-dep")
+    await fs.mkdir(extra, { recursive: true })
+    await writePackage(extra, { name: "fixture-extra-dep", main: "index.js" })
+    await Bun.write(path.join(extra, "index.js"), "export const extra = true\n")
+
+    await install(tmp.path, path.join(tmp.path, "cache"), [
+      { name: PluginSdk.packageName, version: "1.17.11-main.3" },
+      { name: "fixture-extra-dep", version: "file:./extra-dep" },
+    ])
+
+    const lockPath = path.join(tmp.path, "package-lock.json")
+    const lock = JSON.parse(await fs.readFile(lockPath, "utf8"))
+    expect(lock.lockfileVersion).toBe(3)
+    expect(lock.packages[""].dependencies["fixture-extra-dep"]).toMatch(/^file:/)
+    expect(lock.packages["node_modules/fixture-extra-dep"]).toBeDefined()
+    await expect(fs.stat(path.join(tmp.path, "node_modules", "fixture-extra-dep"))).resolves.toBeDefined()
+  })
+
+  test("reifies package-lock when package.json drifts from the lock", async () => {
+    await using tmp = await tmpdir()
+    await seedPluginProject(tmp.path)
+
+    const drift = path.join(tmp.path, "drift-dep")
+    await fs.mkdir(drift, { recursive: true })
+    await writePackage(drift, { name: "fixture-drift-dep", main: "index.js" })
+    await Bun.write(path.join(drift, "index.js"), "export const drift = true\n")
+
+    const pkgPath = path.join(tmp.path, "package.json")
+    const pkg = JSON.parse(await fs.readFile(pkgPath, "utf8"))
+    pkg.dependencies["fixture-drift-dep"] = "file:./drift-dep"
+    await Bun.write(pkgPath, JSON.stringify(pkg, null, 2))
+
+    await install(tmp.path, path.join(tmp.path, "cache"))
+
+    const lockPath = path.join(tmp.path, "package-lock.json")
+    const lock = JSON.parse(await fs.readFile(lockPath, "utf8"))
+    expect(lock.lockfileVersion).toBe(3)
+    expect(lock.packages[""].dependencies).toMatchObject({ "fixture-drift-dep": "file:./drift-dep" })
+    expect(lock.packages["node_modules/fixture-drift-dep"]).toBeDefined()
+    await expect(fs.stat(path.join(tmp.path, "node_modules", "fixture-drift-dep"))).resolves.toBeDefined()
+  })
 })

packages/opencode/test/config/tui-plugin-lock.test.ts

@@ -0,0 +1,97 @@
+import { expect } from "bun:test"
+import fs from "fs/promises"
+import path from "path"
+import { Effect, Layer } from "effect"
+import { FSUtil } from "@opencode-ai/core/fs-util"
+import { PluginSdk } from "@opencode-ai/core/plugin-sdk"
+import { CurrentWorkingDirectory } from "@/config/tui-cwd"
+import { TuiConfig } from "../../src/config/tui"
+import { TestInstance } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
+
+const it = testEffect(Layer.mergeAll(FSUtil.defaultLayer))
+
+const withEnv = <A, E, R>(name: string, value: string | undefined, self: Effect.Effect<A, E, R>) =>
+  Effect.acquireUseRelease(
+    Effect.sync(() => {
+      const previous = process.env[name]
+      if (value === undefined) delete process.env[name]
+      else process.env[name] = value
+      return previous
+    }),
+    () => self,
+    (previous) =>
+      Effect.sync(() => {
+        if (previous === undefined) delete process.env[name]
+        else process.env[name] = previous
+      }),
+  )
+
+// Seeds the on-disk steady state of a config directory after a first successful
+// startup: package.json declaring the plugin sdk, node_modules populated from a
+// local copy, and a tui.json whose path plugin keeps dependency installs armed.
+const seedConfigDir = async (dir: string) => {
+  const localPlugin = path.join(dir, "local-plugin")
+  await fs.mkdir(localPlugin, { recursive: true })
+  await Bun.write(
+    path.join(localPlugin, "package.json"),
+    JSON.stringify({ name: PluginSdk.packageName, version: "1.0.0", main: "index.js" }),
+  )
+  await Bun.write(path.join(localPlugin, "index.js"), "export const plugin = true\n")
+
+  await Bun.write(
+    path.join(dir, "package.json"),
+    JSON.stringify({
+      name: "tui-deps-fixture",
+      version: "1.0.0",
+      dependencies: { [PluginSdk.packageName]: "file:./local-plugin" },
+    }),
+  )
+  await fs.cp(localPlugin, path.join(dir, "node_modules", ...PluginSdk.packageName.split("/")), { recursive: true })
+
+  await Bun.write(path.join(dir, "test-plugin.ts"), "export const fixture_plugin = true\n")
+  await Bun.write(path.join(dir, "tui.json"), JSON.stringify({ plugin: ["./test-plugin.ts"] }))
+}
+
+const lockSnapshot = async (lockPath: string) => {
+  const [bytes, stat] = await Promise.all([fs.readFile(lockPath), fs.stat(lockPath)])
+  return { bytes: bytes.toString(), ino: stat.ino, mtimeMs: stat.mtimeMs }
+}
+
+// One full TUI/config dependency initialization: a fresh TuiConfig layer build
+// (the startup path that forks npm.install for the config dir) followed by
+// waiting for the forked installs to settle.
+const startup = (directory: string, configDir: string) =>
+  withEnv(
+    "OPENCODE_CONFIG_DIR",
+    configDir,
+    TuiConfig.Service.use((svc) => svc.waitForDependencies()).pipe(
+      Effect.provide(TuiConfig.defaultLayer.pipe(Layer.provide(Layer.succeed(CurrentWorkingDirectory, directory)))),
+    ),
+  )
+
+it.instance("keeps the config package-lock stable across two dependency initializations", () =>
+  withEnv(
+    "npm_config_audit",
+    "false",
+    Effect.gen(function* () {
+      const test = yield* TestInstance
+      const configDir = path.join(test.directory, "deps-config")
+      yield* Effect.promise(() => fs.mkdir(configDir, { recursive: true }))
+      yield* Effect.promise(() => seedConfigDir(configDir))
+
+      const lockPath = path.join(configDir, "package-lock.json")
+
+      yield* startup(test.directory, configDir)
+      const afterFirst = yield* Effect.promise(() => lockSnapshot(lockPath))
+      const lock = JSON.parse(yield* Effect.promise(() => fs.readFile(lockPath, "utf8")))
+      expect(lock.lockfileVersion).toBe(3)
+      expect(lock.packages[""].dependencies[PluginSdk.packageName]).toMatch(/^file:/)
+
+      yield* startup(test.directory, configDir)
+      const afterSecond = yield* Effect.promise(() => lockSnapshot(lockPath))
+
+      expect(afterSecond).toEqual(afterFirst)
+    }),
+  ),
+)