diff --git a/.specgit.yaml b/.specgit.yaml index 787fc00f5..90d1c2e85 100644 --- a/.specgit.yaml +++ b/.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 diff --git a/packages/core/src/npm.ts b/packages/core/src/npm.ts index b5a59f2dc..f567339f1 100644 --- a/packages/core/src/npm.ts +++ b/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)) } diff --git a/packages/core/test/npm.test.ts b/packages/core/test/npm.test.ts index 7818ddfb6..96e75e6b5 100644 --- a/packages/core/test/npm.test.ts +++ b/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() + }) }) diff --git a/packages/opencode/test/config/tui-plugin-lock.test.ts b/packages/opencode/test/config/tui-plugin-lock.test.ts new file mode 100644 index 000000000..284b8f562 --- /dev/null +++ b/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 = (name: string, value: string | undefined, self: Effect.Effect) => + 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) + }), + ), +)