From 67407c1a3a3d9048419d19234cc4d597e45c7367 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 14:31:04 -0700 Subject: [PATCH 1/7] fix: only install @opencode-ai/plugin into config dirs that can import it Config loading (and TuiConfig) forked an `@npmcli/arborist` reify of `@opencode-ai/plugin` (~60 packages) into every config dir on every start. Arborist runs in-process, so on a fresh v0.11.0 install it saturated Bun's event loop: `altimate-code serve` accepted no HTTP request for 5 minutes (the Altimate Base consent POST only landed once the install gave up) and `run` froze for ~2.5 minutes after the model had already answered. The starved EffectFlock heartbeat then made the `npm-install:` lock look stale, a second waiter stole it, and the holder's release died with `ReleaseError: metadata missing`. The package is only importable by local `{tool,tools,plugin,plugins}/*.{js,ts}` sources and `file://` plugins under the dir, so gate the install on those (or on an existing `node_modules`, to keep already-installed dirs current). A bare config dir, which is what every new user has, no longer installs anything. - `ConfigPlugin.needsDependencies(dir, plugins)`; used by `Config` and `TuiConfig`, which had a second unconditional copy of the install - PURE-mode skip and the `.gitignore` bootstrap are unchanged - tests: recording `Npm.Service` layer option in `config.test.ts`, nine cases (bare dir skips, tool/tools and plugins sources, existing node_modules, in-dir `file://` plugin as string and tuple, npm spec and out-of-dir `file://` plugin do not install) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WrT7MEUL5CYvpjf9cJbeQR --- packages/opencode/src/config/config.ts | 5 +- packages/opencode/src/config/plugin.ts | 35 ++++- packages/opencode/src/config/tui.ts | 5 +- packages/opencode/test/config/config.test.ts | 130 ++++++++++++++++++- 4 files changed, 170 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 4ee63401a3..675f491bb1 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -575,7 +575,10 @@ export const layer = Layer.effect( // workspace or package cache, so this install fails+retries against the sandbox network and // waitForDependencies() (Fiber.join) then HANGS the process on exit — every subprocess test // that runs a prompt times out. PURE already means "no external plugin discovery + install". - if (!Flag.OPENCODE_PURE) { + // Outside PURE, only install where a local plugin/tool source can import the package; the + // unconditional in-process arborist reify froze fresh installs for minutes (see + // ConfigPlugin.needsDependencies). + if (!Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, result.plugin)) { const dep = yield* npmSvc .install(dir, { add: [ diff --git a/packages/opencode/src/config/plugin.ts b/packages/opencode/src/config/plugin.ts index 60bba4d636..485da044a8 100644 --- a/packages/opencode/src/config/plugin.ts +++ b/packages/opencode/src/config/plugin.ts @@ -1,6 +1,9 @@ import { Glob } from "@opencode-ai/core/util/glob" import { ConfigPluginV1 } from "@opencode-ai/core/v1/config/plugin" -import { pathToFileURL } from "url" +// altimate_change start — upstream_fix: needsDependencies (below) +import { fileURLToPath, pathToFileURL } from "url" +import { existsSync } from "fs" +// altimate_change end import { isPathPluginSpec, parsePluginSpecifier, resolvePathPluginTarget } from "@/plugin/shared" import path from "path" @@ -37,6 +40,36 @@ export function pluginOptions(plugin: ConfigPluginV1.Spec): ConfigPluginV1.Optio return Array.isArray(plugin) ? plugin[1] : undefined } +// altimate_change start — upstream_fix: only install @opencode-ai/plugin where something can import it. +// Upstream reifies a ~60-package @npmcli/arborist tree into EVERY config dir on every start, in-process +// (Config and TuiConfig both do it). On a fresh v0.11.0 install (2026-09-09) that saturated Bun's event +// loop: `serve` accepted no HTTP request for 5 minutes and `run` froze for ~2.5 minutes until the install +// finished; the starved EffectFlock heartbeat made the lock look stale, a second waiter stole it, and the +// holder's release died with "metadata missing". The package is only importable by local tool/plugin +// sources and file:// plugins under the dir, so install only for those, or to keep an existing +// node_modules current. +const SOURCE_GLOB = "{tool,tools,plugin,plugins}/*.{js,ts}" + +export function needsDependencies(dir: string, plugins: readonly ConfigPluginV1.Spec[] | undefined): boolean { + if (existsSync(path.join(dir, "node_modules"))) return true + try { + if (Glob.scanSync(SOURCE_GLOB, { cwd: dir, dot: true, symlink: true }).length > 0) return true + } catch { + // An unreadable dir cannot hold importable sources; fall through to the declared specs. + } + return (plugins ?? []).some((plugin) => { + const spec = pluginSpecifier(plugin) + if (!spec.startsWith("file://")) return false + try { + const rel = path.relative(dir, fileURLToPath(spec)) + return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel) + } catch { + return false + } + }) +} +// altimate_change end + // Path-like specs are resolved relative to the config file that declared them so merges later on do not // accidentally reinterpret `./plugin.ts` relative to some other directory. export async function resolvePluginSpec( diff --git a/packages/opencode/src/config/tui.ts b/packages/opencode/src/config/tui.ts index 2ae0e2f80d..133f0861b0 100644 --- a/packages/opencode/src/config/tui.ts +++ b/packages/opencode/src/config/tui.ts @@ -273,7 +273,10 @@ export const layer = Layer.effect( const data = yield* loadState({ directory, worktree }) // altimate_change end const deps = yield* Effect.forEach( - data.dirs, + // altimate_change start — upstream_fix: same lazy gate as Config; the unconditional in-process + // arborist install froze fresh installs for minutes (see ConfigPlugin.needsDependencies). + data.dirs.filter((dir) => ConfigPlugin.needsDependencies(dir, data.config.plugin)), + // altimate_change end (dir) => npm .install(dir, { diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index e438545369..4bfe09a1e1 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -42,6 +42,7 @@ import { ConfigPluginV1 } from "@opencode-ai/core/v1/config/plugin" import { AccountTest } from "../fake/account" import { AuthTest } from "../fake/auth" import { NpmTest } from "../fake/npm" +import { Npm } from "@opencode-ai/core/npm" /** Infra layer that provides FileSystem, Path, ChildProcessSpawner for test fixtures */ const infra = CrossSpawnSpawner.defaultLayer.pipe( @@ -104,6 +105,7 @@ const configLayer = ( auth?: Layer.Layer account?: Layer.Layer client?: HttpClient.HttpClient + npm?: Layer.Layer } = {}, ) => Config.layer.pipe( @@ -112,7 +114,7 @@ const configLayer = ( Layer.provide(options.auth ?? AuthTest.empty), Layer.provide(options.account ?? AccountTest.empty), Layer.provideMerge(infra), - Layer.provide(NpmTest.noop), + Layer.provide(options.npm ?? NpmTest.noop), Layer.provide(Layer.succeed(HttpClient.HttpClient, options.client ?? unexpectedHttp)), Layer.provideMerge(FSUtil.defaultLayer), ) @@ -1014,7 +1016,9 @@ it.effect("does not try to install dependencies in read-only OPENCODE_CONFIG_DIR }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) -it.effect("installs dependencies in writable OPENCODE_CONFIG_DIR", () => +// altimate_change start — upstream_fix: a bare dir no longer installs; this asserts the .gitignore bootstrap +it.effect("bootstraps .gitignore in a writable OPENCODE_CONFIG_DIR even when no install is needed", () => +// altimate_change end Effect.gen(function* () { const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") @@ -1032,6 +1036,128 @@ it.effect("installs dependencies in writable OPENCODE_CONFIG_DIR", () => }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) +// altimate_change start — upstream_fix: the config-dir @opencode-ai/plugin install is lazy +const recordingNpm = () => { + const dirs: string[] = [] + const layer = Layer.mock(Npm.Service)({ + install: (dir: string) => Effect.sync(() => void dirs.push(dir)), + }) + return { dirs, layer } +} + +const loadConfigDirWithDependencies = (dir: string, configDir: string) => + withProcessEnv( + "OPENCODE_CONFIG_DIR", + configDir, + Config.Service.use((svc) => svc.get().pipe(Effect.andThen(svc.waitForDependencies()))).pipe( + provideInstanceEffect(dir), + ), + ) + +describe("config dir plugin dependency install", () => { + const npm = recordingNpm() + const npmIt = configIt({ npm: npm.layer }) + beforeEach(() => npm.dirs.splice(0)) + + npmIt.effect("skips the install in a bare writable config dir but still writes .gitignore", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.ensureDir(configDir) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).not.toContain(configDir) + expect(yield* FSUtil.use.readFileString(path.join(configDir, ".gitignore"))).toContain("package-lock.json") + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs when the config dir has a local tool source", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.writeWithDirs(path.join(configDir, "tools", "hello.ts"), "export default {}\n") + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs when the config dir has a local plugin source", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.writeWithDirs(path.join(configDir, "plugins", "hello.js"), "export default {}\n") + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("keeps an already-installed node_modules current", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.ensureDir(path.join(configDir, "node_modules")) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs when the config dir has a singular tool/ source dir", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.writeWithDirs(path.join(configDir, "tool", "hello.ts"), "export default {}\n") + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs for a [file://, options] tuple plugin under the config dir", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + const pluginFile = path.join(configDir, "my-plugin.ts") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: [[pathToFileURL(pluginFile).href, { enabled: true }]] }) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("does not install for an npm plugin spec in a bare config dir", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* writeConfigEffect(configDir, { plugin: ["some-npm-plugin@1.0.0"] }) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).not.toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("does not install for a file:// plugin that lives outside the config dir", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + const pluginFile = path.join(dir, "elsewhere", "my-plugin.ts") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(pluginFile).href] }) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).not.toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs for a file:// plugin that lives under the config dir", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + const pluginFile = path.join(configDir, "my-plugin.ts") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(pluginFile).href] }) + yield* loadConfigDirWithDependencies(dir, configDir) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) +}) +// altimate_change end + // Note: deduplication and serialization of npm installs is now handled by the // core Npm.Service (via EffectFlock). Those behaviors are tested in the core // package's npm tests, not here. From cef44a6ac4e94d2127f467bbbfa40ec6014c4699 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 15:00:16 -0700 Subject: [PATCH 2/7] fix: preserve local plugin dependencies across config sources and paths --- packages/opencode/src/config/config.ts | 64 +++++----- packages/opencode/src/config/plugin.ts | 13 +- packages/opencode/test/config/config.test.ts | 120 +++++++++++++++++-- packages/opencode/test/config/tui.test.ts | 42 +++++++ 4 files changed, 189 insertions(+), 50 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 675f491bb1..37ac0bf32e 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -543,8 +543,6 @@ export const layer = Layer.effect( yield* Effect.logDebug("loading config from OPENCODE_CONFIG_DIR", { path: Flag.OPENCODE_CONFIG_DIR }) } - const deps: Fiber.Fiber[] = [] - for (const dir of directories) { // altimate_change start - support both .altimate-code and .opencode config dirs if (dir.endsWith(".altimate-code") || dir.endsWith(".opencode") || dir === Flag.OPENCODE_CONFIG_DIR) { @@ -570,38 +568,6 @@ export const layer = Layer.effect( yield* ensureGitignore(dir).pipe(Effect.orDie) - // altimate_change start — upstream_fix: skip the background @opencode-ai/plugin install in - // PURE mode. The compiled CLI in an isolated HOME (subprocess tests / OPENCODE_PURE) has no - // workspace or package cache, so this install fails+retries against the sandbox network and - // waitForDependencies() (Fiber.join) then HANGS the process on exit — every subprocess test - // that runs a prompt times out. PURE already means "no external plugin discovery + install". - // Outside PURE, only install where a local plugin/tool source can import the package; the - // unconditional in-process arborist reify froze fresh installs for minutes (see - // ConfigPlugin.needsDependencies). - if (!Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, result.plugin)) { - const dep = yield* npmSvc - .install(dir, { - add: [ - { - name: "@opencode-ai/plugin", - version: InstallationLocal ? undefined : InstallationVersion, - }, - ], - }) - .pipe( - Effect.exit, - Effect.tap((exit) => - Exit.isFailure(exit) - ? Effect.logWarning("background dependency install failed", { dir, error: String(exit.cause) }) - : Effect.void, - ), - Effect.asVoid, - Effect.forkDetach, - ) - deps.push(dep) - } - // altimate_change end - result.command = mergeDeep(result.command ?? {}, yield* Effect.promise(() => ConfigCommand.load(dir))) result.agent = mergeDeep(result.agent ?? {}, yield* Effect.promise(() => ConfigAgent.load(dir))) result.agent = mergeDeep(result.agent ?? {}, yield* Effect.promise(() => ConfigAgent.loadMode(dir))) @@ -705,6 +671,36 @@ export const layer = Layer.effect( // altimate_change end } + // altimate_change start — upstream_fix: decide installs only after every config source has + // merged. Inline, account, managed, and later-directory configs can declare a file plugin + // under any earlier directory. Keep the PURE skip and retain fibers for waitForDependencies. + const deps: Fiber.Fiber[] = [] + for (const dir of directories) { + if (!Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, result.plugin)) { + const dep = yield* npmSvc + .install(dir, { + add: [ + { + name: "@opencode-ai/plugin", + version: InstallationLocal ? undefined : InstallationVersion, + }, + ], + }) + .pipe( + Effect.exit, + Effect.tap((exit) => + Exit.isFailure(exit) + ? Effect.logWarning("background dependency install failed", { dir, error: String(exit.cause) }) + : Effect.void, + ), + Effect.asVoid, + Effect.forkDetach, + ) + deps.push(dep) + } + } + // altimate_change end + for (const [name, mode] of Object.entries(result.mode ?? {})) { result.agent = mergeDeep(result.agent ?? {}, { [name]: { diff --git a/packages/opencode/src/config/plugin.ts b/packages/opencode/src/config/plugin.ts index 485da044a8..b6212f36a6 100644 --- a/packages/opencode/src/config/plugin.ts +++ b/packages/opencode/src/config/plugin.ts @@ -2,7 +2,8 @@ import { Glob } from "@opencode-ai/core/util/glob" import { ConfigPluginV1 } from "@opencode-ai/core/v1/config/plugin" // altimate_change start — upstream_fix: needsDependencies (below) import { fileURLToPath, pathToFileURL } from "url" -import { existsSync } from "fs" +import { existsSync, realpathSync } from "fs" +import { FSUtil } from "@opencode-ai/core/fs-util" // altimate_change end import { isPathPluginSpec, parsePluginSpecifier, resolvePathPluginTarget } from "@/plugin/shared" import path from "path" @@ -61,8 +62,14 @@ export function needsDependencies(dir: string, plugins: readonly ConfigPluginV1. const spec = pluginSpecifier(plugin) if (!spec.startsWith("file://")) return false try { - const rel = path.relative(dir, fileURLToPath(spec)) - return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel) + const file = fileURLToPath(spec) + try { + // Bun resolves imports through symlinks; compare the locations that will use node_modules. + return FSUtil.contains(realpathSync(dir), realpathSync(file)) + } catch { + // Preserve lexical detection for paths that cannot yet be resolved on disk. + return FSUtil.contains(dir, file) + } } catch { return false } diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 4bfe09a1e1..df5cfcd980 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -1045,26 +1045,31 @@ const recordingNpm = () => { return { dirs, layer } } -const loadConfigDirWithDependencies = (dir: string, configDir: string) => +const loadConfigDirWithDependencies = ( + dir: string, + configDir: string, + npm: ReturnType, + options: Parameters[0] = {}, +) => withProcessEnv( "OPENCODE_CONFIG_DIR", configDir, Config.Service.use((svc) => svc.get().pipe(Effect.andThen(svc.waitForDependencies()))).pipe( + Effect.provide(configLayer({ ...options, npm: npm.layer })), provideInstanceEffect(dir), ), ) describe("config dir plugin dependency install", () => { - const npm = recordingNpm() - const npmIt = configIt({ npm: npm.layer }) - beforeEach(() => npm.dirs.splice(0)) + const npmIt = testEffect(Layer.mergeAll(infra, FSUtil.defaultLayer)) npmIt.effect("skips the install in a bare writable config dir but still writes .gitignore", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* FSUtil.use.ensureDir(configDir) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).not.toContain(configDir) expect(yield* FSUtil.use.readFileString(path.join(configDir, ".gitignore"))).toContain("package-lock.json") }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), @@ -1072,89 +1077,178 @@ describe("config dir plugin dependency install", () => { npmIt.effect("installs when the config dir has a local tool source", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* FSUtil.use.writeWithDirs(path.join(configDir, "tools", "hello.ts"), "export default {}\n") - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("installs when the config dir has a local plugin source", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* FSUtil.use.writeWithDirs(path.join(configDir, "plugins", "hello.js"), "export default {}\n") - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("keeps an already-installed node_modules current", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* FSUtil.use.ensureDir(path.join(configDir, "node_modules")) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("installs when the config dir has a singular tool/ source dir", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* FSUtil.use.writeWithDirs(path.join(configDir, "tool", "hello.ts"), "export default {}\n") - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("installs for a [file://, options] tuple plugin under the config dir", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") const pluginFile = path.join(configDir, "my-plugin.ts") yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") yield* writeConfigEffect(configDir, { plugin: [[pathToFileURL(pluginFile).href, { enabled: true }]] }) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("does not install for an npm plugin spec in a bare config dir", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") yield* writeConfigEffect(configDir, { plugin: ["some-npm-plugin@1.0.0"] }) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).not.toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("does not install for a file:// plugin that lives outside the config dir", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") const pluginFile = path.join(dir, "elsewhere", "my-plugin.ts") yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(pluginFile).href] }) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).not.toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) npmIt.effect("installs for a file:// plugin that lives under the config dir", () => Effect.gen(function* () { + const npm = recordingNpm() const dir = yield* tmpdirScoped() const configDir = path.join(dir, "configdir") const pluginFile = path.join(configDir, "my-plugin.ts") yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(pluginFile).href] }) - yield* loadConfigDirWithDependencies(dir, configDir) + yield* loadConfigDirWithDependencies(dir, configDir, npm) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + for (const source of ["inline", "managed", "later directory", "account"] as const) { + npmIt.effect(`installs for a local plugin declared by ${source} config after directory discovery`, () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, ".opencode") + const pluginFile = path.join(configDir, "my-plugin.ts") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + const config = { plugin: [pathToFileURL(pluginFile).href] } + if (source === "managed") yield* writeManagedSettingsEffect(config) + const laterDir = path.join(dir, "configdir") + if (source === "later directory") yield* writeConfigEffect(laterDir, config) + const account = Layer.mock(Account.Service)({ + active: () => + Effect.succeed( + Option.some({ + id: AccountID.make("account-1"), + email: "user@example.com", + url: "https://control.example.com", + active_org_id: OrgID.make("org-1"), + }), + ), + config: () => Effect.succeed(Option.some(config)), + token: () => Effect.succeed(Option.none()), + }) + const load = loadConfigDirWithDependencies( + dir, + source === "later directory" ? laterDir : configDir, + npm, + source === "account" ? { account } : {}, + ) + yield* source === "inline" ? withProcessEnv("OPENCODE_CONFIG_CONTENT", JSON.stringify(config), load) : load + expect(npm.dirs.filter((item) => item === configDir)).toHaveLength(1) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + } + + npmIt.effect("installs for a package plugin rooted at the config directory itself", () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.writeWithDirs(path.join(configDir, "package.json"), JSON.stringify({ main: "index.js" })) + yield* FSUtil.use.writeWithDirs(path.join(configDir, "index.js"), "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: ["."] }) + yield* loadConfigDirWithDependencies(dir, configDir, npm) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + + npmIt.effect("installs for a descendant plugin whose name starts with two dots", () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + const pluginFile = path.join(configDir, "..plugins", "hello.ts") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(pluginFile).href] }) + yield* loadConfigDirWithDependencies(dir, configDir, npm) expect(npm.dirs).toContain(configDir) }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) + + for (const alias of ["config directory", "plugin file"] as const) { + npmIt.effect(`installs when the ${alias} uses a symlink alias`, () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const realDir = path.join(dir, "real") + const pluginFile = path.join(realDir, "hello.ts") + const aliasDir = path.join(dir, "alias") + yield* FSUtil.use.writeWithDirs(pluginFile, "export default {}\n") + yield* Effect.promise(() => fs.symlink(realDir, aliasDir, process.platform === "win32" ? "junction" : "dir")) + const configDir = alias === "config directory" ? aliasDir : realDir + const spec = alias === "config directory" ? pluginFile : path.join(aliasDir, "hello.ts") + yield* writeConfigEffect(configDir, { plugin: [pathToFileURL(spec).href] }) + yield* loadConfigDirWithDependencies(dir, configDir, npm) + expect(npm.dirs).toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + } }) // altimate_change end diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index 0e28e76fc6..f9b0251587 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -4,6 +4,10 @@ import path from "path" import { pathToFileURL } from "url" import { Effect, Layer } from "effect" import { FSUtil } from "@opencode-ai/core/fs-util" +// altimate_change start — recording installs verifies the TUI dependency gate without network access. +import { Npm } from "@opencode-ai/core/npm" +import { Project } from "@/project/project" +// altimate_change end import { Global } from "@opencode-ai/core/global" // altimate_change — TuiConfig reads its global config from core's Global (app=opencode), while the // server Config service reads from opencode's Global (app=altimate-code). The alignment test below @@ -86,6 +90,44 @@ const getTuiPluginOrigins = (directory: string) => Effect.provide(TuiConfig.defaultLayer.pipe(Layer.provide(Layer.succeed(CurrentWorkingDirectory, directory)))), ) +// altimate_change start — cover the TUI call site independently from server Config. +for (const source of ["bare", "npm", "file", "tool", "node_modules", "outside"] as const) { + it.instance(`tui dependency install gate: ${source}`, () => + withCleanState( + Effect.gen(function* () { + const fs = yield* FSUtil.Service + const test = yield* TestInstance + const local = path.join(test.directory, ".opencode") + const installs: string[] = [] + yield* fs.makeDirectory(local, { recursive: true }) + const plugin = source === "file" ? "./custom.ts" : source === "outside" ? "../custom.ts" : "npm-plugin@1.0.0" + if (source !== "bare") yield* fs.writeJson(path.join(local, "tui.json"), { plugin: [plugin] }) + if (source === "file" || source === "outside") { + yield* fs.writeWithDirs(path.resolve(local, plugin), "export default {}\n") + } + if (source === "tool") yield* fs.writeWithDirs(path.join(local, "tools", "hello.ts"), "export default {}\n") + if (source === "node_modules") yield* fs.makeDirectory(path.join(local, "node_modules")) + + const npm = Layer.mock(Npm.Service)({ + install: (dir: string) => Effect.yieldNow.pipe(Effect.andThen(Effect.sync(() => void installs.push(dir)))), + }) + yield* TuiConfig.Service.use((svc) => svc.get().pipe(Effect.andThen(svc.waitForDependencies()))).pipe( + Effect.provide( + TuiConfig.layer.pipe( + Layer.provide(Layer.succeed(CurrentWorkingDirectory, test.directory)), + Layer.provide(Project.defaultLayer), + Layer.provide(npm), + Layer.provide(FSUtil.defaultLayer), + ), + ), + ) + expect(installs).toEqual(["file", "tool", "node_modules"].includes(source) ? [local] : []) + }), + ), + ) +} +// altimate_change end + it.instance("keeps server and tui plugin merge semantics aligned", () => withCleanState( Effect.gen(function* () { From 45f63c4f3d33b8a4962c118dd7c3a6623ee06e3e Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 15:20:28 -0700 Subject: [PATCH 3/7] fix: honor PURE in TUI installs and guard cold startup in CI --- .github/workflows/ci.yml | 8 ++ .github/workflows/release.yml | 9 +++ packages/opencode/src/config/tui.ts | 2 +- .../test/cli/serve/fresh-start.test.ts | 81 +++++++++++++++++++ packages/opencode/test/config/config.test.ts | 36 +++++++++ packages/opencode/test/config/tui.test.ts | 74 +++++++++-------- packages/opencode/test/lib/cli-process.ts | 25 ++++-- 7 files changed, 194 insertions(+), 41 deletions(-) create mode 100644 packages/opencode/test/cli/serve/fresh-start.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01e360cfd0..e8bf965846 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -639,6 +639,14 @@ jobs: ALTIMATE_BASE_GATEWAY_URL: https://gateway.test MODELS_DEV_API_JSON: test/tool/fixtures/models-api.json + # altimate_change start — --version and PURE-mode tests never exercise ordinary config installs. + - name: Cold-start config regression (compiled, non-PURE) + working-directory: packages/opencode + env: + OPENCODE_TEST_CLI: ${{ github.workspace }}/packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate-code + run: bun test test/cli/serve/fresh-start.test.ts --timeout 90000 + # altimate_change end + - name: Build dbt-tools run: bun run build working-directory: packages/dbt-tools diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 21a1ae9062..e04aea2e31 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -134,6 +134,15 @@ jobs: env -u NODE_PATH "$BINARY" --version echo "Smoke test passed: standalone binary starts hermetically" + # altimate_change start — --version and PURE-mode tests never exercise ordinary config installs. + - name: Cold-start config regression (compiled, non-PURE) + if: matrix.name == 'linux-x64' + working-directory: packages/opencode + env: + OPENCODE_TEST_CLI: ${{ github.workspace }}/packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate-code + run: bun test test/cli/serve/fresh-start.test.ts --timeout 90000 + # altimate_change end + - name: Upload build artifact uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: diff --git a/packages/opencode/src/config/tui.ts b/packages/opencode/src/config/tui.ts index 133f0861b0..5401c53822 100644 --- a/packages/opencode/src/config/tui.ts +++ b/packages/opencode/src/config/tui.ts @@ -275,7 +275,7 @@ export const layer = Layer.effect( const deps = yield* Effect.forEach( // altimate_change start — upstream_fix: same lazy gate as Config; the unconditional in-process // arborist install froze fresh installs for minutes (see ConfigPlugin.needsDependencies). - data.dirs.filter((dir) => ConfigPlugin.needsDependencies(dir, data.config.plugin)), + data.dirs.filter((dir) => !Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, data.config.plugin)), // altimate_change end (dir) => npm diff --git a/packages/opencode/test/cli/serve/fresh-start.test.ts b/packages/opencode/test/cli/serve/fresh-start.test.ts new file mode 100644 index 0000000000..1b3280eed9 --- /dev/null +++ b/packages/opencode/test/cli/serve/fresh-start.test.ts @@ -0,0 +1,81 @@ +// altimate_change start — exercise ordinary cold startup, which the PURE subprocess defaults skip. +import { expect } from "bun:test" +import { existsSync } from "node:fs" +import { mkdir, writeFile } from "node:fs/promises" +import { pathToFileURL } from "node:url" +import path from "node:path" +import { Effect } from "effect" +import { HttpClient } from "effect/unstable/http" +import { cliIt } from "../../lib/cli-process" + +cliIt.live( + "fresh non-PURE startup serves config without installing plugin dependencies", + ({ home, opencode }) => + Effect.gen(function* () { + const configDir = path.join(home, ".opencode") + yield* Effect.promise(() => mkdir(configDir)) + // An outside, dependency-free plugin makes /provider/auth await Config.waitForDependencies. + // Without this barrier, assertions can race a detached install that has not reached npm yet. + const plugin = path.join(home, "startup-probe.ts") + const loaded = path.join(home, "plugin-loaded") + yield* Effect.promise(() => + writeFile( + plugin, + `export default async () => { await Bun.write(${JSON.stringify(loaded)}, "ready"); return {} }`, + ), + ) + const requests: string[] = [] + const registry = yield* Effect.acquireRelease( + Effect.sync(() => + Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + requests.push(new URL(request.url).pathname) + return new Response("Unexpected package installation during fresh startup", { status: 503 }) + }, + }), + ), + (server) => Effect.sync(() => server.stop(true)), + ) + const server = yield* opencode.serve({ + hostname: "127.0.0.1", + readyTimeoutMs: 30_000, + extraArgs: ["--print-logs"], + env: { + OPENCODE_PURE: "0", + OPENCODE_CONFIG_DIR: configDir, + OPENCODE_CONFIG_CONTENT: JSON.stringify({ plugin: [pathToFileURL(plugin).href] }), + OPENCODE_DISABLE_DEFAULT_PLUGINS: "1", + ALTIMATE_TELEMETRY_DISABLED: "1", + npm_config_registry: registry.url.href, + npm_config_fetch_retries: "0", + npm_config_fetch_timeout: "1000", + }, + }) + const client = yield* HttpClient.HttpClient + for (const route of ["/config", "/provider/auth", "/provider", "/global/health"]) { + yield* Effect.gen(function* () { + const response = yield* client.get(`${server.url}${route}`) + expect(response.status).toBe(200) + yield* response.json + }).pipe(Effect.timeout("15 seconds")) + } + expect(existsSync(loaded)).toBe(true) + // Prove config loading ran, then reject installation even if it failed quickly instead of hanging. + expect(existsSync(path.join(configDir, ".gitignore"))).toBe(true) + expect(requests).toEqual([]) + expect(server.stderr()).not.toContain("background dependency install failed") + for (const dir of [ + configDir, + path.join(home, ".config", "altimate-code"), + path.join(home, ".config", "opencode"), + ]) { + for (const artifact of ["node_modules", "package.json", "package-lock.json"]) { + expect(existsSync(path.join(dir, artifact))).toBe(false) + } + } + }), + 90_000, +) +// altimate_change end diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index df5cfcd980..9502869ff7 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -1249,6 +1249,42 @@ describe("config dir plugin dependency install", () => { }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), ) } + for (const source of ["file", "tool", "node_modules"] as const) { + npmIt.effect(`PURE skips config dependencies even with ${source}`, () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + yield* FSUtil.use.ensureDir(configDir) + if (source === "node_modules") yield* FSUtil.use.ensureDir(path.join(configDir, "node_modules")) + if (source === "tool") + yield* FSUtil.use.writeWithDirs(path.join(configDir, "tools", "hello.ts"), "export default {}\n") + if (source === "file") { + yield* FSUtil.use.writeWithDirs(path.join(configDir, "hello.ts"), "export default {}\n") + yield* writeConfigEffect(configDir, { plugin: ["./hello.ts"] }) + } + yield* withProcessEnv("OPENCODE_PURE", "1", loadConfigDirWithDependencies(dir, configDir, npm)) + expect(npm.dirs).toEqual([]) + expect(yield* FSUtil.use.readFileString(path.join(configDir, ".gitignore"))).toContain("package-lock.json") + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) + } + + npmIt.effect("does not install for a plugin symlink that resolves outside the config directory", () => + Effect.gen(function* () { + const npm = recordingNpm() + const dir = yield* tmpdirScoped() + const configDir = path.join(dir, "configdir") + const externalDir = path.join(dir, "external") + yield* FSUtil.use.writeWithDirs(path.join(externalDir, "hello.ts"), "export default {}\n") + yield* FSUtil.use.ensureDir(configDir) + const aliasDir = path.join(configDir, "linked") + yield* Effect.promise(() => fs.symlink(externalDir, aliasDir, process.platform === "win32" ? "junction" : "dir")) + yield* writeConfigEffect(configDir, { plugin: ["./linked/hello.ts"] }) + yield* loadConfigDirWithDependencies(dir, configDir, npm) + expect(npm.dirs).not.toContain(configDir) + }).pipe(Effect.provide(testInstanceStoreLayer), Effect.provide(CrossSpawnSpawner.defaultLayer)), + ) }) // altimate_change end diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index f9b0251587..306b0cdb37 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -92,39 +92,47 @@ const getTuiPluginOrigins = (directory: string) => // altimate_change start — cover the TUI call site independently from server Config. for (const source of ["bare", "npm", "file", "tool", "node_modules", "outside"] as const) { - it.instance(`tui dependency install gate: ${source}`, () => - withCleanState( - Effect.gen(function* () { - const fs = yield* FSUtil.Service - const test = yield* TestInstance - const local = path.join(test.directory, ".opencode") - const installs: string[] = [] - yield* fs.makeDirectory(local, { recursive: true }) - const plugin = source === "file" ? "./custom.ts" : source === "outside" ? "../custom.ts" : "npm-plugin@1.0.0" - if (source !== "bare") yield* fs.writeJson(path.join(local, "tui.json"), { plugin: [plugin] }) - if (source === "file" || source === "outside") { - yield* fs.writeWithDirs(path.resolve(local, plugin), "export default {}\n") - } - if (source === "tool") yield* fs.writeWithDirs(path.join(local, "tools", "hello.ts"), "export default {}\n") - if (source === "node_modules") yield* fs.makeDirectory(path.join(local, "node_modules")) - - const npm = Layer.mock(Npm.Service)({ - install: (dir: string) => Effect.yieldNow.pipe(Effect.andThen(Effect.sync(() => void installs.push(dir)))), - }) - yield* TuiConfig.Service.use((svc) => svc.get().pipe(Effect.andThen(svc.waitForDependencies()))).pipe( - Effect.provide( - TuiConfig.layer.pipe( - Layer.provide(Layer.succeed(CurrentWorkingDirectory, test.directory)), - Layer.provide(Project.defaultLayer), - Layer.provide(npm), - Layer.provide(FSUtil.defaultLayer), - ), - ), - ) - expect(installs).toEqual(["file", "tool", "node_modules"].includes(source) ? [local] : []) - }), - ), - ) + for (const pure of [false, true]) { + it.instance(`tui dependency install gate: ${source}${pure ? " (PURE)" : ""}`, () => + withEnv( + "OPENCODE_PURE", + pure ? "1" : "0", + withCleanState( + Effect.gen(function* () { + const fs = yield* FSUtil.Service + const test = yield* TestInstance + const local = path.join(test.directory, ".opencode") + const installs: string[] = [] + yield* fs.makeDirectory(local, { recursive: true }) + const plugin = + source === "file" ? "./custom.ts" : source === "outside" ? "../custom.ts" : "npm-plugin@1.0.0" + if (source !== "bare") yield* fs.writeJson(path.join(local, "tui.json"), { plugin: [plugin] }) + if (source === "file" || source === "outside") { + yield* fs.writeWithDirs(path.resolve(local, plugin), "export default {}\n") + } + if (source === "tool") yield* fs.writeWithDirs(path.join(local, "tools", "hello.ts"), "export default {}\n") + if (source === "node_modules") yield* fs.makeDirectory(path.join(local, "node_modules")) + + const npm = Layer.mock(Npm.Service)({ + install: (dir: string) => + Effect.yieldNow.pipe(Effect.andThen(Effect.sync(() => void installs.push(dir)))), + }) + yield* TuiConfig.Service.use((svc) => svc.get().pipe(Effect.andThen(svc.waitForDependencies()))).pipe( + Effect.provide( + TuiConfig.layer.pipe( + Layer.provide(Layer.succeed(CurrentWorkingDirectory, test.directory)), + Layer.provide(Project.defaultLayer), + Layer.provide(npm), + Layer.provide(FSUtil.defaultLayer), + ), + ), + ) + expect(installs).toEqual(!pure && ["file", "tool", "node_modules"].includes(source) ? [local] : []) + }), + ), + ), + ) + } } // altimate_change end diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index 41ef24aac6..7fef31a178 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -34,11 +34,11 @@ const opencodeRoot = path.resolve(import.meta.dir, "../../") const cliEntry = path.join(opencodeRoot, "src/index.ts") const bunExecutable = process.env.BUN_EXECUTABLE || process.execPath || "bun" -// Subprocess tests spawn the CLI once per test. CI runs them in a dedicated bounded pass with -// `bun run src` (--max-concurrency=2) — robust even under heavy load. We do NOT use a prebuilt binary: -// OPENCODE_TEST_CLI is still honored for local experiments, but the compiled binary has a load-triggered -// hang on the run+mock happy path (it never exits under CPU pressure), so CI never sets it. If you do set -// it locally, resolve to ABSOLUTE (spawns run with cwd=) and note tests may hang under load. +// Subprocess tests spawn the CLI once per test. CI runs the general suite in a dedicated bounded pass +// with `bun run src` (--max-concurrency=2). The compiled binary has a load-triggered hang on the run+mock +// happy path, so that suite does not set OPENCODE_TEST_CLI. The dedicated cold-start serve regression +// does use it in binary/release checks; it exercises HTTP startup without running a model. Resolve +// OPENCODE_TEST_CLI to ABSOLUTE because spawns run with cwd=. // (config.ts also skips its background `@opencode-ai/plugin` install under OPENCODE_PURE — without that a // fresh-HOME binary hangs on exit joining the failed install fiber; OPENCODE_PURE is set in isolatedEnv.) const prebuiltCli = process.env.OPENCODE_TEST_CLI ? path.resolve(process.env.OPENCODE_TEST_CLI) : undefined @@ -157,6 +157,8 @@ export type ServeHandle = { readonly kill: () => void // Resolves with the exit code once the process exits. Bun returns a number. readonly exited: Promise + // altimate_change — let startup checks detect failed background work as well as HTTP failures. + readonly stderr: () => string } // `opencode acp` speaks newline-delimited JSON-RPC over stdin/stdout. It is @@ -374,10 +376,17 @@ export function withCliFixture( }), ), (p) => - Effect.promise(() => { + // altimate_change start — a stalled install may also prevent graceful server shutdown. + Effect.promise(async () => { p.kill() - return p.exited + const timeout = setTimeout(() => p.kill("SIGKILL"), 5_000) + try { + await p.exited + } finally { + clearTimeout(timeout) + } }).pipe(Effect.ignore), + // altimate_change end ) // Tail buffer so timeout failures can include stderr context. The fork @@ -424,6 +433,8 @@ export function withCliFixture( proc.kill() }, exited: proc.exited as Promise, + // altimate_change — expose the already-drained diagnostic output. + stderr: () => stderrChunks.join(""), } satisfies ServeHandle }) From f88cc2c155aa9dae8f691030629cf8b1e8799df3 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 18:55:01 -0700 Subject: [PATCH 4/7] fix: consolidate the PURE install gate, run the cold-start check on PRs, settle test stderr Review follow-ups on the lazy plugin-install change: - `ConfigPlugin.shouldInstallDependencies(dir, plugins)` folds the `!Flag.OPENCODE_PURE` check into one helper used by both `Config` and `TuiConfig`, so the install policy cannot drift between the two call sites - the compiled-binary cold-start regression moves out of the push-only `sanity-verdaccio` job into its own `cold-start-regression` job gated like `typescript`, so a PR that reintroduces the first-run stall fails PR CI; the `OPENCODE_TEST_CLI` NOTE now cross-references this deliberate, bounded exception - `release.yml` reuses the smoke test's `find`-resolved binary path instead of a second hardcoded copy - test harness: the stderr tail buffer is bounded at 64 KB and `stderr()` settles the drain fiber before returning, so `not.toContain("background dependency install failed")` cannot false-pass on a chunk that has not landed yet Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WrT7MEUL5CYvpjf9cJbeQR --- .github/workflows/ci.yml | 63 ++++++++++++++++--- .github/workflows/release.yml | 8 ++- packages/opencode/src/config/config.ts | 2 +- packages/opencode/src/config/plugin.ts | 11 ++++ packages/opencode/src/config/tui.ts | 2 +- .../test/cli/serve/fresh-start.test.ts | 2 +- packages/opencode/test/lib/cli-process.ts | 51 +++++++++++++-- 7 files changed, 123 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8bf965846..e7373a3f59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -171,6 +171,12 @@ jobs: # pressure). The robust answer is the dedicated bounded subprocess pass below: `bun run src` (no # such hang) at --max-concurrency=2, which stays green even under heavy load (~43s locally at # load 21). Do not reintroduce OPENCODE_TEST_CLI for these tests without fixing that binary hang. + # altimate_change start — deliberate exception: the cold-start-regression job below DOES set + # OPENCODE_TEST_CLI, for exactly one test (test/cli/serve/fresh-start.test.ts). That is safe + # despite the warning above because it is a single bounded cold-start smoke check with its own + # --timeout, not the general run+mock subprocess suite this NOTE is about — the load-triggered + # hang needs sustained CPU pressure across many concurrent mock round-trips to manifest. + # altimate_change end - name: SDK codegen is reproducible # The v2 gen tree is committed AND regenerated on every release build @@ -639,14 +645,6 @@ jobs: ALTIMATE_BASE_GATEWAY_URL: https://gateway.test MODELS_DEV_API_JSON: test/tool/fixtures/models-api.json - # altimate_change start — --version and PURE-mode tests never exercise ordinary config installs. - - name: Cold-start config regression (compiled, non-PURE) - working-directory: packages/opencode - env: - OPENCODE_TEST_CLI: ${{ github.workspace }}/packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate-code - run: bun test test/cli/serve/fresh-start.test.ts --timeout 90000 - # altimate_change end - - name: Build dbt-tools run: bun run build working-directory: packages/dbt-tools @@ -658,6 +656,55 @@ jobs: env: ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + # altimate_change start — cold-start config regression, split out of sanity-verdaccio so it also + # runs on PRs. sanity-verdaccio stays push-only by design (Docker Compose, too slow for PRs — see + # its header above); this check only needs the compiled binary, so it gets its own job gated like + # `typescript` above: on PRs that touch TS, and unconditionally on push (safety net). + # --------------------------------------------------------------------------- + cold-start-regression: + name: Cold-start Config Regression + needs: changes + if: needs.changes.outputs.typescript == 'true' || github.event_name == 'push' + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - uses: oven-sh/setup-bun@ecf28ddc73e819eb6fa29df6b34ef8921c743461 # v2 + with: + bun-version: "1.3.14" + + - name: Cache Bun dependencies + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: ~/.bun/install/cache + key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} + restore-keys: | + bun-${{ runner.os }}- + + - name: Install dependencies + run: bun install + + - name: Build CLI binary + # target-index=1 = linux-x64 (see release.yml matrix) + run: bun run packages/opencode/script/build.ts --target-index=1 + env: + OPENCODE_VERSION: 0.0.0-sanity-${{ github.sha }} + OPENCODE_RELEASE: "1" + ALTIMATE_BASE_GATEWAY_URL: https://gateway.test + MODELS_DEV_API_JSON: test/tool/fixtures/models-api.json + + # --version and PURE-mode tests never exercise ordinary config installs. This is the + # deliberate exception to the "no OPENCODE_TEST_CLI" NOTE in the `typescript` job above: a + # single bounded cold-start check with its own --timeout, not the general run+mock subprocess + # suite, so the compiled-binary load hang that NOTE warns about cannot stall CI here. + - name: Cold-start config regression (compiled, non-PURE) + working-directory: packages/opencode + env: + OPENCODE_TEST_CLI: ${{ github.workspace }}/packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate-code + run: bun test test/cli/serve/fresh-start.test.ts --timeout 90000 + # altimate_change end + marker-guard: name: Marker Guard runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e04aea2e31..24fa9a1319 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -113,6 +113,7 @@ jobs: # that compile fine but crash at runtime. - name: Smoke test binary if: matrix.name == 'linux-x64' + id: smoke-test run: | # Resolve to an absolute path before we cd away from the workspace. # Test `altimate-code` — the binary the platform package actually ships @@ -124,6 +125,9 @@ jobs: exit 1 fi chmod +x "$BINARY" + # altimate_change — share the resolved path with the cold-start step below so a dist-layout + # change can't make this step pass while the other fails on a stale hardcoded path. + echo "binary=$BINARY" >> "$GITHUB_OUTPUT" # Run with NO pre-set NODE_PATH AND from a directory with no # node_modules anywhere upward. Bun's compiled binary would @@ -139,7 +143,9 @@ jobs: if: matrix.name == 'linux-x64' working-directory: packages/opencode env: - OPENCODE_TEST_CLI: ${{ github.workspace }}/packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate-code + # Reuse the path the smoke test just resolved via `find` instead of a second hardcoded + # copy of it — a dist-layout change would otherwise break one of these two steps silently. + OPENCODE_TEST_CLI: ${{ steps.smoke-test.outputs.binary }} run: bun test test/cli/serve/fresh-start.test.ts --timeout 90000 # altimate_change end diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 37ac0bf32e..23dc88ac78 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -676,7 +676,7 @@ export const layer = Layer.effect( // under any earlier directory. Keep the PURE skip and retain fibers for waitForDependencies. const deps: Fiber.Fiber[] = [] for (const dir of directories) { - if (!Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, result.plugin)) { + if (ConfigPlugin.shouldInstallDependencies(dir, result.plugin)) { const dep = yield* npmSvc .install(dir, { add: [ diff --git a/packages/opencode/src/config/plugin.ts b/packages/opencode/src/config/plugin.ts index b6212f36a6..8eeeb2bc9a 100644 --- a/packages/opencode/src/config/plugin.ts +++ b/packages/opencode/src/config/plugin.ts @@ -4,6 +4,7 @@ import { ConfigPluginV1 } from "@opencode-ai/core/v1/config/plugin" import { fileURLToPath, pathToFileURL } from "url" import { existsSync, realpathSync } from "fs" import { FSUtil } from "@opencode-ai/core/fs-util" +import { Flag } from "@opencode-ai/core/flag/flag" // altimate_change end import { isPathPluginSpec, parsePluginSpecifier, resolvePathPluginTarget } from "@/plugin/shared" import path from "path" @@ -75,6 +76,16 @@ export function needsDependencies(dir: string, plugins: readonly ConfigPluginV1. } }) } + +// PURE mode skips dependency installs entirely: isolated-HOME environments and subprocess tests run +// with no package cache, so the install attempt fails, npm retries, and the process hangs past exit. +// Fold that check in here so both call sites (Config and TuiConfig) stay in sync. +export function shouldInstallDependencies( + dir: string, + plugins: readonly ConfigPluginV1.Spec[] | undefined, +): boolean { + return !Flag.OPENCODE_PURE && needsDependencies(dir, plugins) +} // altimate_change end // Path-like specs are resolved relative to the config file that declared them so merges later on do not diff --git a/packages/opencode/src/config/tui.ts b/packages/opencode/src/config/tui.ts index 5401c53822..f8319951b9 100644 --- a/packages/opencode/src/config/tui.ts +++ b/packages/opencode/src/config/tui.ts @@ -275,7 +275,7 @@ export const layer = Layer.effect( const deps = yield* Effect.forEach( // altimate_change start — upstream_fix: same lazy gate as Config; the unconditional in-process // arborist install froze fresh installs for minutes (see ConfigPlugin.needsDependencies). - data.dirs.filter((dir) => !Flag.OPENCODE_PURE && ConfigPlugin.needsDependencies(dir, data.config.plugin)), + data.dirs.filter((dir) => ConfigPlugin.shouldInstallDependencies(dir, data.config.plugin)), // altimate_change end (dir) => npm diff --git a/packages/opencode/test/cli/serve/fresh-start.test.ts b/packages/opencode/test/cli/serve/fresh-start.test.ts index 1b3280eed9..b35e71f331 100644 --- a/packages/opencode/test/cli/serve/fresh-start.test.ts +++ b/packages/opencode/test/cli/serve/fresh-start.test.ts @@ -65,7 +65,7 @@ cliIt.live( // Prove config loading ran, then reject installation even if it failed quickly instead of hanging. expect(existsSync(path.join(configDir, ".gitignore"))).toBe(true) expect(requests).toEqual([]) - expect(server.stderr()).not.toContain("background dependency install failed") + expect(yield* server.stderr()).not.toContain("background dependency install failed") for (const dir of [ configDir, path.join(home, ".config", "altimate-code"), diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index 7fef31a178..6059e3857d 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -64,6 +64,11 @@ function fromBunStream(name: string, get: () => ReadableStream) { }) } +// Cap on the stderr tail buffer below. A long-lived serve/acp process can log for the life of a +// whole test file; without a bound the buffer would grow unboundedly. 64 KB is far more than any +// single failure/timeout message needs for context. +const STDERR_TAIL_BYTES = 64 * 1024 + // Long-lived processes (serve, acp) all want the same stderr drain: read every // chunk, push to a tail buffer, swallow stream errors (the child closing the // pipe is normal). `log: true` surfaces a real protocol error to logs so a @@ -72,12 +77,47 @@ function forkStderrDrain(stream: ReadableStream, into: string[]) { return Effect.forkScoped( fromBunStream("stderr", () => stream).pipe( Stream.decodeText(), - Stream.runForEach((chunk) => Effect.sync(() => into.push(chunk))), + Stream.runForEach((chunk) => + Effect.sync(() => { + into.push(chunk) + // Trim from the front so `into` never holds more than STDERR_TAIL_BYTES worth of text + // (length in UTF-16 code units, close enough for a test diagnostics buffer). + let total = into.reduce((sum, c) => sum + c.length, 0) + while (total > STDERR_TAIL_BYTES && into.length > 1) { + total -= into.shift()!.length + } + if (total > STDERR_TAIL_BYTES) { + const excess = total - STDERR_TAIL_BYTES + into[0] = into[0].slice(excess) + } + }), + ), Effect.ignore({ log: true }), ), ) } +// `forkStderrDrain` appends off its own I/O callback, which can still be catching up with data the +// child already wrote (and that a caller's own await — e.g. an HTTP round trip to the same process — +// already observed the effect of) by the time a test wants to assert on stderr content. Poll until +// the buffer goes quiet for one full interval, bounded so a genuinely idle pipe can't hang the test; +// this makes `not.toContain(...)` assertions deterministic instead of racing the drain fiber. +function settledStderr(chunks: string[]) { + return Effect.gen(function* () { + let previous = -1 + while (chunks.length !== previous) { + previous = chunks.length + yield* Effect.sleep("20 millis") + } + return chunks.join("") + }).pipe( + Effect.timeoutOrElse({ + duration: "500 millis", + orElse: () => Effect.sync(() => chunks.join("")), + }), + ) +} + function isolatedEnv(home: string, configJson: string): Record { return { OPENCODE_TEST_HOME: home, @@ -158,7 +198,9 @@ export type ServeHandle = { // Resolves with the exit code once the process exits. Bun returns a number. readonly exited: Promise // altimate_change — let startup checks detect failed background work as well as HTTP failures. - readonly stderr: () => string + // Returns an Effect (not a plain string) so callers settle the drain fiber before reading — see + // settledStderr — rather than racing a chunk that hasn't landed in the buffer yet. + readonly stderr: () => Effect.Effect } // `opencode acp` speaks newline-delimited JSON-RPC over stdin/stdout. It is @@ -433,8 +475,9 @@ export function withCliFixture( proc.kill() }, exited: proc.exited as Promise, - // altimate_change — expose the already-drained diagnostic output. - stderr: () => stderrChunks.join(""), + // altimate_change — expose the diagnostic output, settled so the caller doesn't race the + // drain fiber (see settledStderr). + stderr: () => settledStderr(stderrChunks), } satisfies ServeHandle }) From 2e54ba41afc547991fa41bdfd75b855548e38e6e Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 19:25:25 -0700 Subject: [PATCH 5/7] test: read serve stderr deterministically after stop() instead of sleep-polling `settledStderr` polled the drain buffer with `Effect.sleep`, which `test/AGENTS.md` forbids for forked-fiber readiness, and compared `chunks.length`, which the 64 KB trim can hold constant while output is still arriving. The only deterministic read is after the child exits: the pipe closes, the drain stream ends, and `Fiber.join` proves every chunk landed. - `ServeHandle.stop()`: SIGTERM with a SIGKILL fallback, resolves with the exit code, idempotent alongside the scope finalizer - `ServeHandle.stderr()`: awaits exit and joins the drain fiber - `fresh-start.test.ts` stops the server before asserting on stderr Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WrT7MEUL5CYvpjf9cJbeQR --- .../test/cli/serve/fresh-start.test.ts | 2 + packages/opencode/test/lib/cli-process.ts | 55 +++++++++++-------- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/opencode/test/cli/serve/fresh-start.test.ts b/packages/opencode/test/cli/serve/fresh-start.test.ts index b35e71f331..f9f3715d41 100644 --- a/packages/opencode/test/cli/serve/fresh-start.test.ts +++ b/packages/opencode/test/cli/serve/fresh-start.test.ts @@ -65,6 +65,8 @@ cliIt.live( // Prove config loading ran, then reject installation even if it failed quickly instead of hanging. expect(existsSync(path.join(configDir, ".gitignore"))).toBe(true) expect(requests).toEqual([]) + // Stop first: stderr is only complete once the child exited and the drain fiber joined. + yield* server.stop() expect(yield* server.stderr()).not.toContain("background dependency install failed") for (const dir of [ configDir, diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index 6059e3857d..363c4b89ea 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -20,7 +20,7 @@ import { test, type TestOptions } from "bun:test" import { FSUtil } from "@opencode-ai/core/fs-util" import { AppProcess } from "@opencode-ai/core/process" -import { Deferred, Duration, Effect, Layer, Queue, Scope, Stream } from "effect" +import { Deferred, Duration, Effect, Fiber, Layer, Queue, Scope, Stream } from "effect" import { FetchHttpClient, HttpClient } from "effect/unstable/http" import { ChildProcess } from "effect/unstable/process" import path from "node:path" @@ -97,25 +97,16 @@ function forkStderrDrain(stream: ReadableStream, into: string[]) { ) } -// `forkStderrDrain` appends off its own I/O callback, which can still be catching up with data the -// child already wrote (and that a caller's own await — e.g. an HTTP round trip to the same process — -// already observed the effect of) by the time a test wants to assert on stderr content. Poll until -// the buffer goes quiet for one full interval, bounded so a genuinely idle pipe can't hang the test; -// this makes `not.toContain(...)` assertions deterministic instead of racing the drain fiber. -function settledStderr(chunks: string[]) { +// `forkStderrDrain` appends off its own I/O callback, so reading the buffer while the child is +// alive races the drain fiber (test/AGENTS.md: no sleep-based waiting for a forked fiber). The only +// deterministic read is after the child has exited: the pipe closes, the drain stream ends, and +// joining the fiber proves every chunk landed. `ServeHandle.stop()` provides that sequence. +function drainedStderr(exited: Promise, drain: Fiber.Fiber, chunks: string[]) { return Effect.gen(function* () { - let previous = -1 - while (chunks.length !== previous) { - previous = chunks.length - yield* Effect.sleep("20 millis") - } + yield* Effect.promise(() => exited) + yield* Fiber.join(drain) return chunks.join("") - }).pipe( - Effect.timeoutOrElse({ - duration: "500 millis", - orElse: () => Effect.sync(() => chunks.join("")), - }), - ) + }) } function isolatedEnv(home: string, configJson: string): Record { @@ -198,8 +189,11 @@ export type ServeHandle = { // Resolves with the exit code once the process exits. Bun returns a number. readonly exited: Promise // altimate_change — let startup checks detect failed background work as well as HTTP failures. - // Returns an Effect (not a plain string) so callers settle the drain fiber before reading — see - // settledStderr — rather than racing a chunk that hasn't landed in the buffer yet. + // Terminates the child (SIGTERM, SIGKILL after 5 s) and resolves with its exit code. Idempotent; + // the scope finalizer performs the same shutdown if a test never calls it. + readonly stop: () => Effect.Effect + // Complete stderr, available deterministically only after the child exited: awaits exit and joins + // the drain fiber (see drainedStderr). Call stop() first in a test that wants to assert on it. readonly stderr: () => Effect.Effect } @@ -434,7 +428,7 @@ export function withCliFixture( // Tail buffer so timeout failures can include stderr context. The fork // also keeps the OS pipe buffer from filling and wedging the child. const stderrChunks: string[] = [] - yield* forkStderrDrain(proc.stderr, stderrChunks) + const stderrDrain = yield* forkStderrDrain(proc.stderr, stderrChunks) // Watch stdout line-by-line for the listening sentinel. Format // (see src/cli/cmd/serve.ts): @@ -475,9 +469,22 @@ export function withCliFixture( proc.kill() }, exited: proc.exited as Promise, - // altimate_change — expose the diagnostic output, settled so the caller doesn't race the - // drain fiber (see settledStderr). - stderr: () => settledStderr(stderrChunks), + // altimate_change start — deterministic shutdown + stderr read (see drainedStderr). + stop: () => + Effect.promise(async () => { + if (proc.exitCode === null) { + proc.kill() + const timeout = setTimeout(() => proc.kill("SIGKILL"), 5_000) + try { + await proc.exited + } finally { + clearTimeout(timeout) + } + } + return (await proc.exited) as number + }), + stderr: () => drainedStderr(proc.exited as Promise, stderrDrain, stderrChunks), + // altimate_change end } satisfies ServeHandle }) From ad858015f4c3f7e585fe842e8eec3b4900a627e4 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 19:31:27 -0700 Subject: [PATCH 6/7] test: describe ServeHandle.stderr() as the retained tail, not complete output Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WrT7MEUL5CYvpjf9cJbeQR --- packages/opencode/test/lib/cli-process.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index 363c4b89ea..0a9da89d2d 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -192,8 +192,9 @@ export type ServeHandle = { // Terminates the child (SIGTERM, SIGKILL after 5 s) and resolves with its exit code. Idempotent; // the scope finalizer performs the same shutdown if a test never calls it. readonly stop: () => Effect.Effect - // Complete stderr, available deterministically only after the child exited: awaits exit and joins - // the drain fiber (see drainedStderr). Call stop() first in a test that wants to assert on it. + // The retained stderr tail (last STDERR_TAIL_BYTES), available deterministically only after the + // child exited: awaits exit and joins the drain fiber (see drainedStderr). Call stop() first in a + // test that wants to assert on it; a `not.toContain` check is only as strong as the tail window. readonly stderr: () => Effect.Effect } From 3beb7bd3ce6c222be0051fe5a3354476ff9609b9 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 9 Sep 2026 19:31:38 -0700 Subject: [PATCH 7/7] test: reject incomplete stderr in startup regression checks --- .../test/cli/serve/fresh-start.test.ts | 5 +- packages/opencode/test/lib/cli-process.ts | 70 ++++--------------- packages/opencode/test/lib/cli-stderr.test.ts | 62 ++++++++++++++++ packages/opencode/test/lib/cli-stderr.ts | 38 ++++++++++ 4 files changed, 115 insertions(+), 60 deletions(-) create mode 100644 packages/opencode/test/lib/cli-stderr.test.ts create mode 100644 packages/opencode/test/lib/cli-stderr.ts diff --git a/packages/opencode/test/cli/serve/fresh-start.test.ts b/packages/opencode/test/cli/serve/fresh-start.test.ts index f9f3715d41..f93a3a79e9 100644 --- a/packages/opencode/test/cli/serve/fresh-start.test.ts +++ b/packages/opencode/test/cli/serve/fresh-start.test.ts @@ -64,10 +64,11 @@ cliIt.live( expect(existsSync(loaded)).toBe(true) // Prove config loading ran, then reject installation even if it failed quickly instead of hanging. expect(existsSync(path.join(configDir, ".gitignore"))).toBe(true) - expect(requests).toEqual([]) // Stop first: stderr is only complete once the child exited and the drain fiber joined. yield* server.stop() - expect(yield* server.stderr()).not.toContain("background dependency install failed") + const stderr = yield* server.stderr().pipe(Effect.timeout("10 seconds")) + expect(requests).toEqual([]) + expect(stderr).not.toContain("background dependency install failed") for (const dir of [ configDir, path.join(home, ".config", "altimate-code"), diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index 0a9da89d2d..0e0fe53c94 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -20,7 +20,7 @@ import { test, type TestOptions } from "bun:test" import { FSUtil } from "@opencode-ai/core/fs-util" import { AppProcess } from "@opencode-ai/core/process" -import { Deferred, Duration, Effect, Fiber, Layer, Queue, Scope, Stream } from "effect" +import { Deferred, Duration, Effect, Layer, Queue, Scope, Stream } from "effect" import { FetchHttpClient, HttpClient } from "effect/unstable/http" import { ChildProcess } from "effect/unstable/process" import path from "node:path" @@ -29,6 +29,7 @@ import * as fsPromises from "node:fs/promises" import { TestLLMServer } from "./llm-server" import { testProviderConfig } from "./test-provider" import { it } from "./effect" +import { captureStderr } from "./cli-stderr" const opencodeRoot = path.resolve(import.meta.dir, "../../") const cliEntry = path.join(opencodeRoot, "src/index.ts") @@ -64,51 +65,6 @@ function fromBunStream(name: string, get: () => ReadableStream) { }) } -// Cap on the stderr tail buffer below. A long-lived serve/acp process can log for the life of a -// whole test file; without a bound the buffer would grow unboundedly. 64 KB is far more than any -// single failure/timeout message needs for context. -const STDERR_TAIL_BYTES = 64 * 1024 - -// Long-lived processes (serve, acp) all want the same stderr drain: read every -// chunk, push to a tail buffer, swallow stream errors (the child closing the -// pipe is normal). `log: true` surfaces a real protocol error to logs so a -// regression doesn't silently disappear. -function forkStderrDrain(stream: ReadableStream, into: string[]) { - return Effect.forkScoped( - fromBunStream("stderr", () => stream).pipe( - Stream.decodeText(), - Stream.runForEach((chunk) => - Effect.sync(() => { - into.push(chunk) - // Trim from the front so `into` never holds more than STDERR_TAIL_BYTES worth of text - // (length in UTF-16 code units, close enough for a test diagnostics buffer). - let total = into.reduce((sum, c) => sum + c.length, 0) - while (total > STDERR_TAIL_BYTES && into.length > 1) { - total -= into.shift()!.length - } - if (total > STDERR_TAIL_BYTES) { - const excess = total - STDERR_TAIL_BYTES - into[0] = into[0].slice(excess) - } - }), - ), - Effect.ignore({ log: true }), - ), - ) -} - -// `forkStderrDrain` appends off its own I/O callback, so reading the buffer while the child is -// alive races the drain fiber (test/AGENTS.md: no sleep-based waiting for a forked fiber). The only -// deterministic read is after the child has exited: the pipe closes, the drain stream ends, and -// joining the fiber proves every chunk landed. `ServeHandle.stop()` provides that sequence. -function drainedStderr(exited: Promise, drain: Fiber.Fiber, chunks: string[]) { - return Effect.gen(function* () { - yield* Effect.promise(() => exited) - yield* Fiber.join(drain) - return chunks.join("") - }) -} - function isolatedEnv(home: string, configJson: string): Record { return { OPENCODE_TEST_HOME: home, @@ -192,10 +148,9 @@ export type ServeHandle = { // Terminates the child (SIGTERM, SIGKILL after 5 s) and resolves with its exit code. Idempotent; // the scope finalizer performs the same shutdown if a test never calls it. readonly stop: () => Effect.Effect - // The retained stderr tail (last STDERR_TAIL_BYTES), available deterministically only after the - // child exited: awaits exit and joins the drain fiber (see drainedStderr). Call stop() first in a - // test that wants to assert on it; a `not.toContain` check is only as strong as the tail window. - readonly stderr: () => Effect.Effect + // Call stop() first: this waits for exit and stderr EOF, failing if output was + // truncated or unreadable so negative assertions cannot pass on partial logs. + readonly stderr: () => Effect.Effect } // `opencode acp` speaks newline-delimited JSON-RPC over stdin/stdout. It is @@ -428,8 +383,7 @@ export function withCliFixture( // Tail buffer so timeout failures can include stderr context. The fork // also keeps the OS pipe buffer from filling and wedging the child. - const stderrChunks: string[] = [] - const stderrDrain = yield* forkStderrDrain(proc.stderr, stderrChunks) + const stderr = yield* captureStderr(proc.stderr) // Watch stdout line-by-line for the listening sentinel. Format // (see src/cli/cmd/serve.ts): @@ -456,7 +410,7 @@ export function withCliFixture( Effect.fail( new Error( `opencode serve did not become ready within ${readyTimeoutMs}ms\n` + - `stderr (last 2000):\n${stderrChunks.join("").slice(-2000)}`, + `stderr (last 2000):\n${stderr.tail().slice(-2000)}`, ), ), }), @@ -470,8 +424,8 @@ export function withCliFixture( proc.kill() }, exited: proc.exited as Promise, - // altimate_change start — deterministic shutdown + stderr read (see drainedStderr). - stop: () => + // altimate_change start — deterministic shutdown + complete stderr read. + stop: Effect.fn("opencode.serve.stop")(() => Effect.promise(async () => { if (proc.exitCode === null) { proc.kill() @@ -484,7 +438,8 @@ export function withCliFixture( } return (await proc.exited) as number }), - stderr: () => drainedStderr(proc.exited as Promise, stderrDrain, stderrChunks), + ), + stderr: () => Effect.promise(() => proc.exited).pipe(Effect.andThen(stderr.complete)), // altimate_change end } satisfies ServeHandle }) @@ -526,8 +481,7 @@ export function withCliFixture( }).pipe(Effect.ignore), ) - const stderrChunks: string[] = [] - yield* forkStderrDrain(proc.stderr, stderrChunks) + yield* captureStderr(proc.stderr) // Each ndjson line becomes one queue entry. JSON.parse failures are // surfaced as the raw string so a malformed protocol message doesn't diff --git a/packages/opencode/test/lib/cli-stderr.test.ts b/packages/opencode/test/lib/cli-stderr.test.ts new file mode 100644 index 0000000000..638b493b05 --- /dev/null +++ b/packages/opencode/test/lib/cli-stderr.test.ts @@ -0,0 +1,62 @@ +// altimate_change start — incomplete stderr must never make a negative assertion pass. +import { expect } from "bun:test" +import { Cause, Effect, Exit, Fiber, Result } from "effect" +import * as TestClock from "effect/testing/TestClock" +import { it } from "./effect" +import { captureStderr } from "./cli-stderr" + +it.effect("stderr waits for EOF even after a quiet interval", () => + Effect.gen(function* () { + const pipe = new TransformStream() + const writer = pipe.writable.getWriter() + const stderr = yield* captureStderr(pipe.readable) + yield* Effect.promise(() => writer.write(new TextEncoder().encode("startup\n"))) + let complete = false + const read = yield* stderr.complete.pipe( + Effect.tap(() => + Effect.sync(() => { + complete = true + }), + ), + Effect.forkScoped, + ) + // Advance the old 20ms quiet interval AND its 500ms partial-result fallback. + yield* TestClock.adjust("1 second") + expect(complete).toBe(false) + yield* Effect.promise(() => writer.write(new TextEncoder().encode("background dependency install failed\n"))) + yield* Effect.promise(() => writer.close()) + expect(yield* Fiber.join(read)).toBe("startup\nbackground dependency install failed\n") + }), +) + +it.effect("stderr rejects truncated output even when the tail looks clean", () => + Effect.gen(function* () { + const pipe = new TransformStream() + const writer = pipe.writable.getWriter() + const stderr = yield* captureStderr(pipe.readable) + for (const chunk of ["background dependency install failed\n", "a".repeat(64 * 1024), "b".repeat(64 * 1024)]) { + yield* Effect.promise(() => writer.write(new TextEncoder().encode(chunk))) + } + yield* Effect.promise(() => writer.close()) + const result = yield* Effect.result(stderr.complete) + expect(Result.isFailure(result)).toBe(true) + if (Result.isFailure(result)) expect(result.failure.message).toContain("stderr capture truncated") + expect(stderr.tail()).toBe("b".repeat(64 * 1024)) + }), +) + +it.effect("stderr propagates pipe read failures", () => + Effect.gen(function* () { + const stderr = yield* captureStderr( + new ReadableStream({ + start(controller) { + controller.error(new Error("broken pipe")) + }, + }), + ) + const exit = yield* Effect.exit(stderr.complete) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.pretty(exit.cause)).toContain("broken pipe") + }), +) +// altimate_change end diff --git a/packages/opencode/test/lib/cli-stderr.ts b/packages/opencode/test/lib/cli-stderr.ts new file mode 100644 index 0000000000..704373622a --- /dev/null +++ b/packages/opencode/test/lib/cli-stderr.ts @@ -0,0 +1,38 @@ +// altimate_change start — bounded stderr capture for subprocess assertions. +import { Effect, Fiber, Stream } from "effect" + +// UTF-16 code units, matching String.length and String.slice. +const STDERR_TAIL_CHARS = 64 * 1024 + +export const captureStderr = Effect.fn("CliStderr.capture")(function* (stream: ReadableStream) { + let tail = "" + let truncated = false + const drain = yield* Stream.fromReadableStream({ + evaluate: () => stream, + onError: (cause) => new Error(`stderr stream error: ${String(cause)}`), + }).pipe( + Stream.decodeText(), + Stream.runForEach((chunk) => + Effect.sync(() => { + const text = tail + chunk + truncated ||= text.length > STDERR_TAIL_CHARS + tail = text.slice(-STDERR_TAIL_CHARS) + }), + ), + Effect.tapError(Effect.logError), + Effect.forkScoped, + ) + + return { + // A bounded snapshot is sufficient for timeout diagnostics. + tail: () => tail, + // Negative assertions require EOF and all output. Fail closed if the drain + // failed or the cap discarded an earlier error; inactivity proves neither. + complete: Effect.gen(function* () { + yield* Fiber.join(drain) + if (truncated) return yield* Effect.fail(new Error("stderr capture truncated; cannot assert on complete output")) + return tail + }), + } +}) +// altimate_change end