From d7f95cdfdfc760db1e3b53e31e14ee0cf72de3b2 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 15:53:28 -0400 Subject: [PATCH 1/6] fix(instance): run bootstrap from instance store --- packages/opencode/src/cli/bootstrap.ts | 6 +-- packages/opencode/src/cli/cmd/tui/worker.ts | 7 ++- packages/opencode/src/config/config.ts | 10 ++-- packages/opencode/src/effect/app-runtime.ts | 20 ++----- .../opencode/src/project/bootstrap-service.ts | 9 ++++ packages/opencode/src/project/bootstrap.ts | 14 +++-- .../opencode/src/project/instance-runtime.ts | 27 ++++++++++ .../opencode/src/project/instance-store.ts | 15 ++---- packages/opencode/src/project/instance.ts | 6 +-- packages/opencode/src/server/routes/global.ts | 4 +- .../httpapi/middleware/instance-context.ts | 10 ++-- .../server/routes/instance/httpapi/server.ts | 6 +-- .../src/server/routes/instance/index.ts | 4 +- .../src/server/routes/instance/middleware.ts | 2 - .../src/server/routes/instance/project.ts | 10 +--- packages/opencode/src/server/workspace.ts | 4 +- .../agent/plugin-agent-regression.test.ts | 51 ++++++++++++++++++ .../test/effect/instance-state.test.ts | 6 +-- packages/opencode/test/fixture/fixture.ts | 8 +-- packages/opencode/test/mcp/lifecycle.test.ts | 4 +- .../opencode/test/permission/next.test.ts | 8 +-- .../instance-bootstrap-regression.test.ts | 53 +++++++++++++++++++ .../opencode/test/project/instance.test.ts | 7 ++- .../opencode/test/project/worktree.test.ts | 16 +++--- .../opencode/test/question/question.test.ts | 6 +-- .../server/httpapi-instance-context.test.ts | 6 +-- .../opencode/test/server/httpapi-mcp.test.ts | 4 +- .../test/server/httpapi-provider.test.ts | 4 +- 28 files changed, 216 insertions(+), 111 deletions(-) create mode 100644 packages/opencode/src/project/bootstrap-service.ts create mode 100644 packages/opencode/src/project/instance-runtime.ts create mode 100644 packages/opencode/test/agent/plugin-agent-regression.test.ts create mode 100644 packages/opencode/test/project/instance-bootstrap-regression.test.ts diff --git a/packages/opencode/src/cli/bootstrap.ts b/packages/opencode/src/cli/bootstrap.ts index da90ec4033cd..81a085d68959 100644 --- a/packages/opencode/src/cli/bootstrap.ts +++ b/packages/opencode/src/cli/bootstrap.ts @@ -1,17 +1,15 @@ import { Instance } from "../project/instance" -import { InstanceStore } from "../project/instance-store" -import { getBootstrapRunEffect } from "../effect/app-runtime" +import { InstanceRuntime } from "../project/instance-runtime" export async function bootstrap(directory: string, cb: () => Promise) { return Instance.provide({ directory, - init: await getBootstrapRunEffect(), fn: async () => { try { const result = await cb() return result } finally { - await InstanceStore.disposeInstance(Instance.current) + await InstanceRuntime.disposeInstance(Instance.current) } }, }) diff --git a/packages/opencode/src/cli/cmd/tui/worker.ts b/packages/opencode/src/cli/cmd/tui/worker.ts index dd6f7e246d79..143236edf35e 100644 --- a/packages/opencode/src/cli/cmd/tui/worker.ts +++ b/packages/opencode/src/cli/cmd/tui/worker.ts @@ -2,7 +2,7 @@ import { Installation } from "@/installation" import { Server } from "@/server/server" import * as Log from "@opencode-ai/core/util/log" import { Instance } from "@/project/instance" -import { InstanceStore } from "@/project/instance-store" +import { InstanceRuntime } from "@/project/instance-runtime" import { Rpc } from "@/util/rpc" import { upgrade } from "@/cli/upgrade" import { Config } from "@/config/config" @@ -10,7 +10,7 @@ import { GlobalBus } from "@/bus/global" import { Flag } from "@opencode-ai/core/flag/flag" import { writeHeapSnapshot } from "node:v8" import { Heap } from "@/cli/heap" -import { AppRuntime, getBootstrapRunEffect } from "@/effect/app-runtime" +import { AppRuntime } from "@/effect/app-runtime" import { ensureProcessMetadata } from "@opencode-ai/core/util/opencode-process" ensureProcessMetadata("worker") @@ -77,7 +77,6 @@ export const rpc = { async checkUpgrade(input: { directory: string }) { await Instance.provide({ directory: input.directory, - init: await getBootstrapRunEffect(), fn: async () => { await upgrade().catch(() => {}) }, @@ -89,7 +88,7 @@ export const rpc = { async shutdown() { Log.Default.info("worker shutting down") - await InstanceStore.disposeAllInstances() + await InstanceRuntime.disposeAllInstances() if (server) await server.stop(true) }, } diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 46a31cf1c400..4e39adfb478b 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -12,7 +12,6 @@ import { Auth } from "../auth" import { Env } from "../env" import { applyEdits, modify } from "jsonc-parser" import { type InstanceContext } from "../project/instance" -import { InstanceStore } from "../project/instance-store" import { InstallationLocal, InstallationVersion } from "@opencode-ai/core/installation/version" import { existsSync } from "fs" import { GlobalBus } from "@/bus/global" @@ -47,6 +46,10 @@ import { Npm } from "@opencode-ai/core/npm" const log = Log.create({ service: "config" }) +async function loadInstanceRuntime() { + return (await import("../project/instance-runtime")).InstanceRuntime +} + // Custom merge function that concatenates array fields instead of replacing them // Keep remeda's deep conditional merge type out of hot config-loading paths; TS profiling showed it dominates here. function mergeConfig(target: Info, source: Info): Info { @@ -742,13 +745,14 @@ export const layer = Layer.effect( // mask "config update without an active instance" bugs. The throw // comes from `Instance.current` inside `InstanceState.context`. const ctx = yield* InstanceState.context - yield* Effect.promise(() => InstanceStore.disposeInstance(ctx)) + yield* Effect.promise(async () => (await loadInstanceRuntime()).disposeInstance(ctx)) } }) const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) { yield* invalidateGlobal - const task = InstanceStore.disposeAllInstances() + const task = loadInstanceRuntime() + .then((runtime) => runtime.disposeAllInstances()) .catch(() => undefined) .finally(() => GlobalBus.emit("event", { diff --git a/packages/opencode/src/effect/app-runtime.ts b/packages/opencode/src/effect/app-runtime.ts index 66f3a9b37821..901738646cf9 100644 --- a/packages/opencode/src/effect/app-runtime.ts +++ b/packages/opencode/src/effect/app-runtime.ts @@ -1,4 +1,4 @@ -import { Effect, Layer, ManagedRuntime } from "effect" +import { Layer, ManagedRuntime } from "effect" import { attach } from "./run-service" import * as Observability from "@opencode-ai/core/effect/observability" @@ -40,8 +40,7 @@ import { Command } from "@/command" import { Truncate } from "@/tool/truncate" import { ToolRegistry } from "@/tool/registry" import { Format } from "@/format" -import { InstanceBootstrap } from "@/project/bootstrap" -import { InstanceStore } from "@/project/instance-store" +import { InstanceRuntime } from "@/project/instance-runtime" import { Project } from "@/project/project" import { Vcs } from "@/project/vcs" import { Workspace } from "@/control-plane/workspace" @@ -94,8 +93,7 @@ export const AppLayer = Layer.mergeAll( Truncate.defaultLayer, ToolRegistry.defaultLayer, Format.defaultLayer, - InstanceBootstrap.defaultLayer, - InstanceStore.defaultLayer, + InstanceRuntime.layer, Project.defaultLayer, Vcs.defaultLayer, Workspace.defaultLayer, @@ -132,15 +130,3 @@ export const AppRuntime: Runtime = { }, dispose: () => rt.dispose(), } - -let bootstrapRun: Promise> -export function getBootstrapRunEffect(): Promise> { - if (!bootstrapRun) { - bootstrapRun = AppRuntime.runPromise( - Effect.gen(function* () { - return (yield* InstanceBootstrap.Service).run - }), - ) - } - return bootstrapRun -} diff --git a/packages/opencode/src/project/bootstrap-service.ts b/packages/opencode/src/project/bootstrap-service.ts new file mode 100644 index 000000000000..b20cc54cd623 --- /dev/null +++ b/packages/opencode/src/project/bootstrap-service.ts @@ -0,0 +1,9 @@ +import { Context, Effect } from "effect" + +export interface Interface { + readonly run: Effect.Effect +} + +export class Service extends Context.Service()("@opencode/InstanceBootstrap") {} + +export * as InstanceBootstrap from "./bootstrap-service" diff --git a/packages/opencode/src/project/bootstrap.ts b/packages/opencode/src/project/bootstrap.ts index 9f77de2d4dc9..ea2aa2e84899 100644 --- a/packages/opencode/src/project/bootstrap.ts +++ b/packages/opencode/src/project/bootstrap.ts @@ -10,21 +10,19 @@ import { Command } from "../command" import { InstanceState } from "@/effect/instance-state" import { FileWatcher } from "@/file/watcher" import { ShareNext } from "@/share/share-next" -import { Context, Effect, Layer } from "effect" +import { Effect, Layer } from "effect" import { Config } from "@/config/config" +import { Service } from "./bootstrap-service" -export interface Interface { - readonly run: Effect.Effect -} - -export class Service extends Context.Service()("@opencode/InstanceBootstrap") {} +export { Service } from "./bootstrap-service" +export type { Interface } from "./bootstrap-service" export const layer = Layer.effect( Service, Effect.gen(function* () { // Yield each bootstrap dep at layer init so `run` itself has R = never. - // This breaks the circular declaration loop through Config → Instance → InstanceStore - // (instance-store.ts only yields this Service tag, never the impl-side services). + // InstanceStore imports only the lightweight tag from bootstrap-service.ts, + // so it can depend on bootstrap without importing this implementation graph. const bus = yield* Bus.Service const config = yield* Config.Service const file = yield* File.Service diff --git a/packages/opencode/src/project/instance-runtime.ts b/packages/opencode/src/project/instance-runtime.ts new file mode 100644 index 000000000000..dafefe842e27 --- /dev/null +++ b/packages/opencode/src/project/instance-runtime.ts @@ -0,0 +1,27 @@ +import { makeRuntime } from "@/effect/run-service" +import { type InstanceContext } from "./instance-context" +import { InstanceStore, type LoadInput } from "./instance-store" +import { Effect, Layer } from "effect" + +// Bridge for Promise/ALS callers that cannot yet yield InstanceStore.Service. +// This keeps InstanceStore itself low-level while still giving legacy Hono and +// CLI paths the production bootstrap implementation. Delete this module once +// those callers are migrated to Effect boundaries that provide InstanceStore +// directly, like the HttpApi middleware does. +// Keep the bootstrap implementation import lazy: Instance is imported broadly, +// and importing the app bootstrap graph at module load can trigger ESM cycles. +export const layer = Layer.unwrap( + Effect.promise(async () => { + const { InstanceBootstrap } = await import("./bootstrap") + return InstanceStore.defaultLayer.pipe(Layer.provide(InstanceBootstrap.defaultLayer)) + }), +) + +const runtime = makeRuntime(InstanceStore.Service, layer) + +export const load = (input: LoadInput) => runtime.runPromise((store) => store.load(input)) +export const disposeInstance = (ctx: InstanceContext) => runtime.runPromise((store) => store.dispose(ctx)) +export const disposeAllInstances = () => runtime.runPromise((store) => store.disposeAll()) +export const reloadInstance = (input: LoadInput) => runtime.runPromise((store) => store.reload(input)) + +export * as InstanceRuntime from "./instance-runtime" diff --git a/packages/opencode/src/project/instance-store.ts b/packages/opencode/src/project/instance-store.ts index 00075be64b81..41adcbc7cfd6 100644 --- a/packages/opencode/src/project/instance-store.ts +++ b/packages/opencode/src/project/instance-store.ts @@ -2,10 +2,10 @@ import { GlobalBus } from "@/bus/global" import { WorkspaceContext } from "@/control-plane/workspace-context" import { InstanceRef } from "@/effect/instance-ref" import { disposeInstance as runDisposers } from "@/effect/instance-registry" -import { makeRuntime } from "@/effect/run-service" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Context, Deferred, Duration, Effect, Exit, Layer, Scope } from "effect" import { type InstanceContext } from "./instance-context" +import { InstanceBootstrap } from "./bootstrap-service" import * as Project from "./project" export interface LoadInput { @@ -36,10 +36,11 @@ interface Entry { readonly deferred: Deferred.Deferred } -export const layer: Layer.Layer = Layer.effect( +export const layer: Layer.Layer = Layer.effect( Service, Effect.gen(function* () { const project = yield* Project.Service + const bootstrap = yield* InstanceBootstrap.Service const scope = yield* Scope.Scope const cache = new Map() @@ -59,6 +60,7 @@ export const layer: Layer.Layer = Layer.effect( project: result.project, })), ) + yield* bootstrap.run.pipe(Effect.provideService(InstanceRef, ctx)) if (input.init) yield* input.init.pipe(Effect.provideService(InstanceRef, ctx)) return ctx }).pipe(Effect.withSpan("InstanceStore.boot")) @@ -195,13 +197,4 @@ export const layer: Layer.Layer = Layer.effect( export const defaultLayer = layer.pipe(Layer.provide(Project.defaultLayer)) -export const runtime = makeRuntime(Service, defaultLayer) - -// Promise-returning helpers for callers without an Effect runtime in scope. -// They route through `runtime` (not a yielded Service from a fresh runtime) -// so they share the cache that `Instance.provide` populates. -export const disposeInstance = (ctx: InstanceContext) => runtime.runPromise((store) => store.dispose(ctx)) -export const disposeAllInstances = () => runtime.runPromise((store) => store.disposeAll()) -export const reloadInstance = (input: LoadInput) => runtime.runPromise((store) => store.reload(input)) - export * as InstanceStore from "./instance-store" diff --git a/packages/opencode/src/project/instance.ts b/packages/opencode/src/project/instance.ts index 5b2bcf6b32e7..81977affc33f 100644 --- a/packages/opencode/src/project/instance.ts +++ b/packages/opencode/src/project/instance.ts @@ -1,15 +1,13 @@ import { Effect } from "effect" import { context, type InstanceContext } from "./instance-context" -import { InstanceStore } from "./instance-store" +import { InstanceRuntime } from "./instance-runtime" export type { InstanceContext } from "./instance-context" export type { LoadInput } from "./instance-store" export const Instance = { async provide(input: { directory: string; init?: Effect.Effect; fn: () => R }): Promise { - const ctx = await InstanceStore.runtime.runPromise((store) => - store.load({ directory: input.directory, init: input.init }), - ) + const ctx = await InstanceRuntime.load({ directory: input.directory, init: input.init }) return context.provide(ctx, async () => input.fn()) }, get current() { diff --git a/packages/opencode/src/server/routes/global.ts b/packages/opencode/src/server/routes/global.ts index f40a58453629..7a3adcfc7a52 100644 --- a/packages/opencode/src/server/routes/global.ts +++ b/packages/opencode/src/server/routes/global.ts @@ -8,7 +8,7 @@ import { SyncEvent } from "@/sync" import { GlobalBus } from "@/bus/global" import { AppRuntime } from "@/effect/app-runtime" import { AsyncQueue } from "@/util/queue" -import { InstanceStore } from "../../project/instance-store" +import { InstanceRuntime } from "../../project/instance-runtime" import { Installation } from "@/installation" import { InstallationVersion } from "@opencode-ai/core/installation/version" import * as Log from "@opencode-ai/core/util/log" @@ -200,7 +200,7 @@ export const GlobalRoutes = lazy(() => }, }), async (c) => { - await InstanceStore.disposeAllInstances() + await InstanceRuntime.disposeAllInstances() GlobalBus.emit("event", { directory: "global", payload: { diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/instance-context.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/instance-context.ts index 0e82da31b3ac..d4913696d299 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/middleware/instance-context.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/instance-context.ts @@ -1,5 +1,4 @@ import { WorkspaceRef } from "@/effect/instance-ref" -import { InstanceBootstrap } from "@/project/bootstrap" import { InstanceStore } from "@/project/instance-store" import { Effect, Layer } from "effect" import { HttpRouter, HttpServerResponse } from "effect/unstable/http" @@ -24,12 +23,11 @@ function decode(input: string): string { function provideInstanceContext( effect: Effect.Effect, store: InstanceStore.Interface, - bootstrap: InstanceBootstrap.Interface, ): Effect.Effect { return Effect.gen(function* () { const route = yield* WorkspaceRouteContext return yield* store.provide( - { directory: decode(route.directory), init: bootstrap.run }, + { directory: decode(route.directory) }, effect.pipe(Effect.provideService(WorkspaceRef, route.workspaceID)), ) }) @@ -39,15 +37,13 @@ export const instanceContextLayer = Layer.effect( InstanceContextMiddleware, Effect.gen(function* () { const store = yield* InstanceStore.Service - const bootstrap = yield* InstanceBootstrap.Service - return InstanceContextMiddleware.of((effect) => provideInstanceContext(effect, store, bootstrap)) + return InstanceContextMiddleware.of((effect) => provideInstanceContext(effect, store)) }), ) export const instanceRouterMiddleware = HttpRouter.middleware()( Effect.gen(function* () { const store = yield* InstanceStore.Service - const bootstrap = yield* InstanceBootstrap.Service - return (effect) => provideInstanceContext(effect, store, bootstrap) + return (effect) => provideInstanceContext(effect, store) }), ) diff --git a/packages/opencode/src/server/routes/instance/httpapi/server.ts b/packages/opencode/src/server/routes/instance/httpapi/server.ts index 767bfc31db86..ce1b21372999 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/server.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/server.ts @@ -18,8 +18,7 @@ import { LSP } from "@/lsp/lsp" import { MCP } from "@/mcp" import { Permission } from "@/permission" import { Installation } from "@/installation" -import { InstanceBootstrap } from "@/project/bootstrap" -import { InstanceStore } from "@/project/instance-store" +import { InstanceRuntime } from "@/project/instance-runtime" import { Plugin } from "@/plugin" import { Project } from "@/project/project" import { ProviderAuth } from "@/provider/auth" @@ -153,8 +152,7 @@ export function createRoutes(corsOptions?: CorsOptions) { Format.defaultLayer, LSP.defaultLayer, Installation.defaultLayer, - InstanceBootstrap.defaultLayer, - InstanceStore.defaultLayer, + InstanceRuntime.layer, MCP.defaultLayer, ModelsDev.defaultLayer, Permission.defaultLayer, diff --git a/packages/opencode/src/server/routes/instance/index.ts b/packages/opencode/src/server/routes/instance/index.ts index 530c02345aa1..4baef09384d4 100644 --- a/packages/opencode/src/server/routes/instance/index.ts +++ b/packages/opencode/src/server/routes/instance/index.ts @@ -6,7 +6,7 @@ import z from "zod" import { Format } from "@/format" import { TuiRoutes } from "./tui" import { Instance } from "@/project/instance" -import { InstanceStore } from "@/project/instance-store" +import { InstanceRuntime } from "@/project/instance-runtime" import { Vcs } from "@/project/vcs" import { Agent } from "@/agent/agent" import { Skill } from "@/skill" @@ -63,7 +63,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => { }, }), async (c) => { - await InstanceStore.disposeInstance(Instance.current) + await InstanceRuntime.disposeInstance(Instance.current) return c.json(true) }, ) diff --git a/packages/opencode/src/server/routes/instance/middleware.ts b/packages/opencode/src/server/routes/instance/middleware.ts index db7b9b52f942..494459500d43 100644 --- a/packages/opencode/src/server/routes/instance/middleware.ts +++ b/packages/opencode/src/server/routes/instance/middleware.ts @@ -1,6 +1,5 @@ import type { MiddlewareHandler } from "hono" import { Instance } from "@/project/instance" -import { getBootstrapRunEffect } from "@/effect/app-runtime" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { WorkspaceContext } from "@/control-plane/workspace-context" import { WorkspaceID } from "@/control-plane/schema" @@ -23,7 +22,6 @@ export function InstanceMiddleware(workspaceID?: WorkspaceID): MiddlewareHandler async fn() { return Instance.provide({ directory, - init: await getBootstrapRunEffect(), async fn() { return next() }, diff --git a/packages/opencode/src/server/routes/instance/project.ts b/packages/opencode/src/server/routes/instance/project.ts index 01a45c2fb935..3d8bb605bd58 100644 --- a/packages/opencode/src/server/routes/instance/project.ts +++ b/packages/opencode/src/server/routes/instance/project.ts @@ -2,13 +2,12 @@ import { Hono } from "hono" import { describeRoute, validator } from "hono-openapi" import { resolver } from "hono-openapi" import { Instance } from "@/project/instance" -import { InstanceStore } from "@/project/instance-store" +import { InstanceRuntime } from "@/project/instance-runtime" import { Project } from "@/project/project" import z from "zod" import { ProjectID } from "@/project/schema" import { errors } from "../../error" import { lazy } from "@/util/lazy" -import { getBootstrapRunEffect } from "@/effect/app-runtime" import { jsonRequest, runRequest } from "./trace" export const ProjectRoutes = lazy(() => @@ -82,12 +81,7 @@ export const ProjectRoutes = lazy(() => Project.Service.use((svc) => svc.initGit({ directory: dir, project: prev })), ) if (next.id === prev.id && next.vcs === prev.vcs && next.worktree === prev.worktree) return c.json(next) - await InstanceStore.reloadInstance({ - directory: dir, - worktree: dir, - project: next, - init: await getBootstrapRunEffect(), - }) + await InstanceRuntime.reloadInstance({ directory: dir, worktree: dir, project: next }) return c.json(next) }, ) diff --git a/packages/opencode/src/server/workspace.ts b/packages/opencode/src/server/workspace.ts index 0036c9ab464c..dbf693e8fc27 100644 --- a/packages/opencode/src/server/workspace.ts +++ b/packages/opencode/src/server/workspace.ts @@ -5,7 +5,7 @@ import { WorkspaceID } from "@/control-plane/schema" import { WorkspaceContext } from "@/control-plane/workspace-context" import { Workspace } from "@/control-plane/workspace" import { Flag } from "@opencode-ai/core/flag/flag" -import { getBootstrapRunEffect, AppRuntime } from "@/effect/app-runtime" +import { AppRuntime } from "@/effect/app-runtime" import { Instance } from "@/project/instance" import { Session } from "@/session/session" import { SessionID } from "@/session/schema" @@ -94,13 +94,11 @@ export function WorkspaceRouterMiddleware(upgrade: UpgradeWebSocket): Middleware const target = await adapter.target(workspace) if (target.type === "local") { - const init = await getBootstrapRunEffect() return WorkspaceContext.provide({ workspaceID: WorkspaceID.make(workspaceID), fn: () => Instance.provide({ directory: target.directory, - init, async fn() { return next() }, diff --git a/packages/opencode/test/agent/plugin-agent-regression.test.ts b/packages/opencode/test/agent/plugin-agent-regression.test.ts new file mode 100644 index 000000000000..89e8a66407ba --- /dev/null +++ b/packages/opencode/test/agent/plugin-agent-regression.test.ts @@ -0,0 +1,51 @@ +import { afterEach, expect, test } from "bun:test" +import path from "path" +import { pathToFileURL } from "url" +import { AppRuntime } from "../../src/effect/app-runtime" +import { Agent } from "../../src/agent/agent" +import { Instance } from "../../src/project/instance" +import { disposeAllInstances, tmpdir } from "../fixture/fixture" + +afterEach(async () => { + await disposeAllInstances() +}) + +test("plugin-registered agents appear in Agent.list", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginFile = path.join(dir, "plugin.ts") + await Bun.write( + pluginFile, + [ + "export default async () => ({", + " config: async (cfg) => {", + " cfg.agent = cfg.agent ?? {}", + " cfg.agent.plugin_added = {", + ' description: "Added by a plugin via the config hook",', + ' mode: "subagent",', + " }", + " },", + "})", + "", + ].join("\n"), + ) + await Bun.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + plugin: [pathToFileURL(pluginFile).href], + }), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const agents = await AppRuntime.runPromise(Agent.Service.use((svc) => svc.list())) + const added = agents.find((agent) => agent.name === "plugin_added") + expect(added?.description).toBe("Added by a plugin via the config hook") + expect(added?.mode).toBe("subagent") + }, + }) +}) diff --git a/packages/opencode/test/effect/instance-state.test.ts b/packages/opencode/test/effect/instance-state.test.ts index 0a8972ca4a68..a1c267063f3c 100644 --- a/packages/opencode/test/effect/instance-state.test.ts +++ b/packages/opencode/test/effect/instance-state.test.ts @@ -3,7 +3,7 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { $ } from "bun" import { Context, Deferred, Duration, Effect, Exit, Fiber, Layer } from "effect" import { InstanceState } from "@/effect/instance-state" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Instance } from "../../src/project/instance" import { disposeAllInstances, provideInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" @@ -70,7 +70,7 @@ it.live("InstanceState invalidates on reload", () => ) const a = yield* access(state, dir) - yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir })) + yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) const b = yield* access(state, dir) expect(a).not.toBe(b) @@ -270,7 +270,7 @@ it.live("InstanceState correct after interleaved init and dispose", () => const [, b] = yield* Effect.all( [ - Effect.promise(() => InstanceStore.reloadInstance({ directory: one })), + Effect.promise(() => InstanceRuntime.reloadInstance({ directory: one })), Test.use((svc) => svc.get()).pipe(provideInstance(two)), ], { concurrency: "unbounded" }, diff --git a/packages/opencode/test/fixture/fixture.ts b/packages/opencode/test/fixture/fixture.ts index 1b193e382ab7..7d45dd49e280 100644 --- a/packages/opencode/test/fixture/fixture.ts +++ b/packages/opencode/test/fixture/fixture.ts @@ -8,13 +8,13 @@ import type * as Scope from "effect/Scope" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" import type { Config } from "@/config/config" import { InstanceRef } from "../../src/effect/instance-ref" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Instance } from "../../src/project/instance" import { TestLLMServer } from "../lib/llm-server" // Re-export for test ergonomics. The implementation lives next to the runtime -// it consumes; see `InstanceStore.disposeAllInstances` for the rationale. -export { disposeAllInstances } from "../../src/project/instance-store" +// it consumes; see `InstanceRuntime.disposeAllInstances` for the rationale. +export { disposeAllInstances } from "../../src/project/instance-runtime" // Strip null bytes from paths (defensive fix for CI environment issues) function sanitizePath(p: string): string { @@ -150,7 +150,7 @@ export function provideTmpdirInstance( ? Effect.promise(() => Instance.provide({ directory: path, - fn: () => InstanceStore.disposeInstance(Instance.current), + fn: () => InstanceRuntime.disposeInstance(Instance.current), }), ).pipe(Effect.ignore) : Effect.void, diff --git a/packages/opencode/test/mcp/lifecycle.test.ts b/packages/opencode/test/mcp/lifecycle.test.ts index 59fa54ceab0f..2ba487f3f555 100644 --- a/packages/opencode/test/mcp/lifecycle.test.ts +++ b/packages/opencode/test/mcp/lifecycle.test.ts @@ -1,5 +1,5 @@ import { test, expect, mock, beforeEach } from "bun:test" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Effect } from "effect" import type { MCP as MCPNS } from "../../src/mcp/index" @@ -198,7 +198,7 @@ function withInstance( fn: async () => { await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer))) // dispose instance to clean up state between tests - await InstanceStore.disposeInstance(Instance.current) + await InstanceRuntime.disposeInstance(Instance.current) }, }) } diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index c615e55e5ed7..674d0011d1b5 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -6,7 +6,7 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Permission } from "../../src/permission" import { PermissionID } from "../../src/permission/schema" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { disposeAllInstances, provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" import { MessageID, SessionID } from "../../src/session/schema" @@ -1000,7 +1000,7 @@ it.live("pending permission rejects on instance dispose", () => expect(yield* waitForPending(1).pipe(run)).toHaveLength(1) yield* Effect.promise(() => - Instance.provide({ directory: dir, fn: () => void InstanceStore.disposeInstance(Instance.current) }), + Instance.provide({ directory: dir, fn: () => void InstanceRuntime.disposeInstance(Instance.current) }), ) const exit = yield* Fiber.await(fiber) @@ -1024,7 +1024,7 @@ it.live("pending permission rejects on instance reload", () => }).pipe(run, Effect.forkScoped) expect(yield* waitForPending(1).pipe(run)).toHaveLength(1) - yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir })) + yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) const exit = yield* Fiber.await(fiber) expect(Exit.isFailure(exit)).toBe(true) @@ -1118,7 +1118,7 @@ it.live("ask - abort should clear pending request", () => const pending = yield* waitForPending(1).pipe(run) expect(pending).toHaveLength(1) - yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir })) + yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) const exit = yield* Fiber.await(fiber) expect(Exit.isFailure(exit)).toBe(true) diff --git a/packages/opencode/test/project/instance-bootstrap-regression.test.ts b/packages/opencode/test/project/instance-bootstrap-regression.test.ts new file mode 100644 index 000000000000..ea3937315031 --- /dev/null +++ b/packages/opencode/test/project/instance-bootstrap-regression.test.ts @@ -0,0 +1,53 @@ +import { afterEach, expect, test } from "bun:test" +import { existsSync } from "node:fs" +import path from "node:path" +import { pathToFileURL } from "node:url" +import { Instance } from "../../src/project/instance" +import { disposeAllInstances, tmpdir } from "../fixture/fixture" + +// Instance.provide must run bootstrap before fn. The plugin config hook writes +// a marker file, and fn deliberately avoids touching Plugin or config so the +// marker only exists if bootstrap ran at the instance boundary. + +afterEach(async () => { + await disposeAllInstances() +}) + +test("Instance.provide runs InstanceBootstrap before fn (boundary invariant)", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const marker = path.join(dir, "config-hook-fired") + const pluginFile = path.join(dir, "plugin.ts") + await Bun.write( + pluginFile, + [ + `const MARKER = ${JSON.stringify(marker)}`, + "export default async () => ({", + " config: async () => {", + ' await Bun.write(MARKER, "ran")', + " },", + "})", + "", + ].join("\n"), + ) + await Bun.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + plugin: [pathToFileURL(pluginFile).href], + }), + ) + return marker + }, + }) + + // The body of `fn` deliberately does not yield Plugin, read config, or + // touch any service that would force Plugin.state to materialize on + // demand. The only way the marker gets written is if bootstrap ran. + await Instance.provide({ + directory: tmp.path, + fn: async () => "ok", + }) + + expect(existsSync(tmp.extra)).toBe(true) +}) diff --git a/packages/opencode/test/project/instance.test.ts b/packages/opencode/test/project/instance.test.ts index 852c58ef41cc..bc8809af9cc8 100644 --- a/packages/opencode/test/project/instance.test.ts +++ b/packages/opencode/test/project/instance.test.ts @@ -3,12 +3,17 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Effect, Fiber, Layer } from "effect" import { InstanceRef } from "../../src/effect/instance-ref" import { registerDisposer } from "../../src/effect/instance-registry" +import { InstanceBootstrap } from "../../src/project/bootstrap-service" import { Instance } from "../../src/project/instance" import { InstanceStore } from "../../src/project/instance-store" import { disposeAllInstances, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" -const it = testEffect(Layer.mergeAll(InstanceStore.defaultLayer, CrossSpawnSpawner.defaultLayer)) +const noopBootstrap = Layer.succeed(InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.void })) + +const it = testEffect( + Layer.mergeAll(InstanceStore.defaultLayer, CrossSpawnSpawner.defaultLayer).pipe(Layer.provide(noopBootstrap)), +) afterEach(async () => { await disposeAllInstances() diff --git a/packages/opencode/test/project/worktree.test.ts b/packages/opencode/test/project/worktree.test.ts index 806c47615b39..60c66981d55b 100644 --- a/packages/opencode/test/project/worktree.test.ts +++ b/packages/opencode/test/project/worktree.test.ts @@ -5,7 +5,7 @@ import path from "path" import { Cause, Effect, Exit, Layer } from "effect" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Worktree } from "../../src/worktree" import { disposeAllInstances, provideInstance, provideTmpdirInstance } from "../fixture/fixture" import { testEffect } from "../lib/effect" @@ -138,9 +138,10 @@ describe("Worktree", () => { expect(props.branch).toBe(info.branch) yield* Effect.promise(() => - InstanceStore.runtime.runPromise((s) => - s.load({ directory: info.directory }).pipe(Effect.flatMap(s.dispose)), - ), + Instance.provide({ + directory: info.directory, + fn: () => InstanceRuntime.disposeInstance(Instance.current), + }), ) yield* Effect.promise(() => Bun.sleep(100)) yield* svc.remove({ directory: info.directory }) @@ -162,9 +163,10 @@ describe("Worktree", () => { yield* Effect.promise(() => ready) yield* Effect.promise(() => - InstanceStore.runtime.runPromise((s) => - s.load({ directory: info.directory }).pipe(Effect.flatMap(s.dispose)), - ), + Instance.provide({ + directory: info.directory, + fn: () => InstanceRuntime.disposeInstance(Instance.current), + }), ) yield* Effect.promise(() => Bun.sleep(100)) yield* svc.remove({ directory: info.directory }) diff --git a/packages/opencode/test/question/question.test.ts b/packages/opencode/test/question/question.test.ts index 83968a6f8c1a..694a37e99fe4 100644 --- a/packages/opencode/test/question/question.test.ts +++ b/packages/opencode/test/question/question.test.ts @@ -1,7 +1,7 @@ import { afterEach, test, expect } from "bun:test" import { Question } from "../../src/question" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { QuestionID } from "../../src/question/schema" import { disposeAllInstances, tmpdir } from "../fixture/fixture" import { SessionID } from "../../src/session/schema" @@ -422,7 +422,7 @@ test("pending question rejects on instance dispose", async () => { fn: async () => { const items = await list() expect(items).toHaveLength(1) - await InstanceStore.disposeInstance(Instance.current) + await InstanceRuntime.disposeInstance(Instance.current) }, }) @@ -457,7 +457,7 @@ test("pending question rejects on instance reload", async () => { fn: async () => { const items = await list() expect(items).toHaveLength(1) - await InstanceStore.reloadInstance({ directory: tmp.path }) + await InstanceRuntime.reloadInstance({ directory: tmp.path }) }, }) diff --git a/packages/opencode/test/server/httpapi-instance-context.test.ts b/packages/opencode/test/server/httpapi-instance-context.test.ts index ece01cf32329..f311de2b4af1 100644 --- a/packages/opencode/test/server/httpapi-instance-context.test.ts +++ b/packages/opencode/test/server/httpapi-instance-context.test.ts @@ -11,9 +11,8 @@ import { registerAdapter } from "../../src/control-plane/adapters" import type { WorkspaceAdapter } from "../../src/control-plane/types" import { Workspace } from "../../src/control-plane/workspace" import { InstanceRef, WorkspaceRef } from "../../src/effect/instance-ref" -import { InstanceBootstrap } from "../../src/project/bootstrap" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" import { Project } from "../../src/project/project" import { disposeMiddleware, markInstanceForDisposal } from "../../src/server/routes/instance/httpapi/lifecycle" import { instanceRouterMiddleware } from "../../src/server/routes/instance/httpapi/middleware/instance-context" @@ -42,8 +41,7 @@ const it = testEffect( testStateLayer, NodeHttpServer.layerTest, NodeServices.layer, - InstanceBootstrap.defaultLayer, - InstanceStore.defaultLayer, + InstanceRuntime.layer, Project.defaultLayer, Workspace.defaultLayer, ), diff --git a/packages/opencode/test/server/httpapi-mcp.test.ts b/packages/opencode/test/server/httpapi-mcp.test.ts index 6f2b4cee38f2..396d04feb81e 100644 --- a/packages/opencode/test/server/httpapi-mcp.test.ts +++ b/packages/opencode/test/server/httpapi-mcp.test.ts @@ -5,7 +5,7 @@ import { Flag } from "@opencode-ai/core/flag/flag" import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server" import { McpPaths } from "../../src/server/routes/instance/httpapi/groups/mcp" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Server } from "../../src/server/server" import * as Log from "@opencode-ai/core/util/log" import { resetDatabase } from "../fixture/db" @@ -59,7 +59,7 @@ function withMcpProject(self: (dir: string) => Effect.Effect) ) yield* Effect.addFinalizer(() => Effect.promise(() => - Instance.provide({ directory: dir, fn: () => InstanceStore.disposeInstance(Instance.current) }), + Instance.provide({ directory: dir, fn: () => InstanceRuntime.disposeInstance(Instance.current) }), ).pipe(Effect.ignore), ) diff --git a/packages/opencode/test/server/httpapi-provider.test.ts b/packages/opencode/test/server/httpapi-provider.test.ts index b4cec9115fa6..8118aa7842b7 100644 --- a/packages/opencode/test/server/httpapi-provider.test.ts +++ b/packages/opencode/test/server/httpapi-provider.test.ts @@ -3,7 +3,7 @@ import { Effect, FileSystem, Layer, Path } from "effect" import { NodeFileSystem, NodePath } from "@effect/platform-node" import { Flag } from "@opencode-ai/core/flag/flag" import { Instance } from "../../src/project/instance" -import { InstanceStore } from "../../src/project/instance-store" +import { InstanceRuntime } from "../../src/project/instance-runtime" import { Server } from "../../src/server/server" import * as Log from "@opencode-ai/core/util/log" import { resetDatabase } from "../fixture/db" @@ -91,7 +91,7 @@ function withProviderProject(self: (dir: string) => Effect.Effect Effect.promise(() => - Instance.provide({ directory: dir, fn: () => InstanceStore.disposeInstance(Instance.current) }), + Instance.provide({ directory: dir, fn: () => InstanceRuntime.disposeInstance(Instance.current) }), ).pipe(Effect.ignore), ) From de0321b4fedd5c5ef8b179a3a7c7fd484b00a278 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 17:11:15 -0400 Subject: [PATCH 2/6] test(instance): isolate fixture bootstrap runtime --- packages/opencode/test/config/config.test.ts | 16 ++-- packages/opencode/test/config/tui.test.ts | 4 +- .../test/effect/instance-state.test.ts | 7 +- packages/opencode/test/fixture/fixture.ts | 47 +++++++---- .../opencode/test/permission/next.test.ts | 12 ++- .../test/plugin/auth-override.test.ts | 77 ++++++++++++++----- .../test/plugin/loader-shared.test.ts | 46 ++++++++--- 7 files changed, 147 insertions(+), 62 deletions(-) diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 5b2e91e374ca..ffad0ae57e07 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -12,7 +12,7 @@ import { Account } from "../../src/account/account" import { AccessToken, AccountID, OrgID } from "../../src/account/schema" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Env } from "../../src/env" -import { disposeAllInstances, provideTmpdirInstance } from "../fixture/fixture" +import { disposeAllInstances, provideTestInstance, provideTmpdirInstance } from "../fixture/fixture" import { tmpdir } from "../fixture/fixture" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { testEffect } from "../lib/effect" @@ -550,7 +550,7 @@ test("validates config schema and throws on invalid fields", async () => { }) }, }) - await Instance.provide({ + await provideTestInstance({ directory: tmp.path, fn: async () => { // Strict schema should throw an error for invalid fields @@ -565,7 +565,7 @@ test("throws error for invalid JSON", async () => { await Filesystem.write(path.join(dir, "opencode.json"), "{ invalid json }") }, }) - await Instance.provide({ + await provideTestInstance({ directory: tmp.path, fn: async () => { await expect(load()).rejects.toThrow() @@ -1061,7 +1061,7 @@ test("resolves scoped npm plugins in config", async () => { }, }) - await Instance.provide({ + await provideTestInstance({ directory: tmp.path, fn: async () => { const config = await load() @@ -1099,7 +1099,7 @@ test("merges plugin arrays from global and local configs", async () => { }, }) - await Instance.provide({ + await provideTestInstance({ directory: path.join(tmp.path, "project"), fn: async () => { const config = await load() @@ -1258,7 +1258,7 @@ test("deduplicates duplicate plugins from global and local configs", async () => }, }) - await Instance.provide({ + await provideTestInstance({ directory: path.join(tmp.path, "project"), fn: async () => { const config = await load() @@ -1307,7 +1307,7 @@ test("keeps plugin origins aligned with merged plugin list", async () => { }, }) - await Instance.provide({ + await provideTestInstance({ directory: path.join(tmp.path, "project"), fn: async () => { const cfg = await load() @@ -2096,7 +2096,7 @@ describe("deduplicatePluginOrigins", () => { }, }) - await Instance.provide({ + await provideTestInstance({ directory: path.join(tmp.path, "project"), fn: async () => { const config = await load() diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index 46a3f0626365..903af71fc28e 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, expect, test } from "bun:test" import path from "path" import fs from "fs/promises" -import { tmpdir } from "../fixture/fixture" +import { provideTestInstance, tmpdir } from "../fixture/fixture" import { Instance } from "../../src/project/instance" import { TuiConfig } from "../../src/cli/cmd/tui/config/tui" import { Config } from "@/config/config" @@ -87,7 +87,7 @@ test("keeps server and tui plugin merge semantics aligned", async () => { }, }) - await Instance.provide({ + await provideTestInstance({ directory: tmp.path, fn: async () => { const server = await load() diff --git a/packages/opencode/test/effect/instance-state.test.ts b/packages/opencode/test/effect/instance-state.test.ts index a1c267063f3c..f5e693388327 100644 --- a/packages/opencode/test/effect/instance-state.test.ts +++ b/packages/opencode/test/effect/instance-state.test.ts @@ -3,9 +3,8 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { $ } from "bun" import { Context, Deferred, Duration, Effect, Exit, Fiber, Layer } from "effect" import { InstanceState } from "@/effect/instance-state" -import { InstanceRuntime } from "../../src/project/instance-runtime" import { Instance } from "../../src/project/instance" -import { disposeAllInstances, provideInstance, tmpdirScoped } from "../fixture/fixture" +import { disposeAllInstances, provideInstance, reloadTestInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" const it = testEffect(CrossSpawnSpawner.defaultLayer) @@ -70,7 +69,7 @@ it.live("InstanceState invalidates on reload", () => ) const a = yield* access(state, dir) - yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) + yield* Effect.promise(() => reloadTestInstance({ directory: dir })) const b = yield* access(state, dir) expect(a).not.toBe(b) @@ -270,7 +269,7 @@ it.live("InstanceState correct after interleaved init and dispose", () => const [, b] = yield* Effect.all( [ - Effect.promise(() => InstanceRuntime.reloadInstance({ directory: one })), + Effect.promise(() => reloadTestInstance({ directory: one })), Test.use((svc) => svc.get()).pipe(provideInstance(two)), ], { concurrency: "unbounded" }, diff --git a/packages/opencode/test/fixture/fixture.ts b/packages/opencode/test/fixture/fixture.ts index 7d45dd49e280..38017e516cd7 100644 --- a/packages/opencode/test/fixture/fixture.ts +++ b/packages/opencode/test/fixture/fixture.ts @@ -1,20 +1,44 @@ import { $ } from "bun" +import * as Observability from "@opencode-ai/core/effect/observability" import * as fs from "fs/promises" import os from "os" import path from "path" -import { Effect, Context } from "effect" +import { Effect, Context, Layer, ManagedRuntime } from "effect" import type * as PlatformError from "effect/PlatformError" import type * as Scope from "effect/Scope" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" import type { Config } from "@/config/config" import { InstanceRef } from "../../src/effect/instance-ref" +import { InstanceBootstrap } from "../../src/project/bootstrap-service" import { InstanceRuntime } from "../../src/project/instance-runtime" +import { InstanceStore } from "../../src/project/instance-store" import { Instance } from "../../src/project/instance" import { TestLLMServer } from "../lib/llm-server" -// Re-export for test ergonomics. The implementation lives next to the runtime -// it consumes; see `InstanceRuntime.disposeAllInstances` for the rationale. -export { disposeAllInstances } from "../../src/project/instance-runtime" +const noopBootstrap = Layer.succeed(InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.void })) +const testInstanceRuntime = ManagedRuntime.make( + InstanceStore.defaultLayer.pipe(Layer.provide(noopBootstrap), Layer.provideMerge(Observability.layer)), +) + +const runTestInstanceStore = (fn: (store: InstanceStore.Interface) => Effect.Effect) => + testInstanceRuntime.runPromise(InstanceStore.Service.use(fn)) + +export async function provideTestInstance(input: { directory: string; init?: Effect.Effect; fn: () => R }) { + const ctx = await runTestInstanceStore((store) => store.load({ directory: input.directory, init: input.init })) + try { + return await Instance.restore(ctx, () => input.fn()) + } finally { + await runTestInstanceStore((store) => store.dispose(ctx)) + } +} + +export async function reloadTestInstance(input: { directory: string }) { + return runTestInstanceStore((store) => store.reload(input)) +} + +export async function disposeAllInstances() { + await Promise.all([InstanceRuntime.disposeAllInstances(), runTestInstanceStore((store) => store.disposeAll())]) +} // Strip null bytes from paths (defensive fix for CI environment issues) function sanitizePath(p: string): string { @@ -129,12 +153,10 @@ export const provideInstance = (directory: string) => (self: Effect.Effect): Effect.Effect => Effect.contextWith((services: Context.Context) => - Effect.promise(async () => - Instance.provide({ - directory, - fn: () => Effect.runPromiseWith(services)(self.pipe(Effect.provideService(InstanceRef, Instance.current))), - }), - ), + Effect.promise(async () => { + const ctx = await runTestInstanceStore((store) => store.load({ directory })) + return Instance.restore(ctx, () => Effect.runPromiseWith(services)(self.pipe(Effect.provideService(InstanceRef, ctx)))) + }), ) export function provideTmpdirInstance( @@ -148,10 +170,7 @@ export function provideTmpdirInstance( yield* Effect.addFinalizer(() => provided ? Effect.promise(() => - Instance.provide({ - directory: path, - fn: () => InstanceRuntime.disposeInstance(Instance.current), - }), + runTestInstanceStore((store) => store.load({ directory: path }).pipe(Effect.flatMap((ctx) => store.dispose(ctx)))), ).pipe(Effect.ignore) : Effect.void, ) diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 674d0011d1b5..4d66784d8163 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -7,7 +7,13 @@ import { Permission } from "../../src/permission" import { PermissionID } from "../../src/permission/schema" import { Instance } from "../../src/project/instance" import { InstanceRuntime } from "../../src/project/instance-runtime" -import { disposeAllInstances, provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture" +import { + disposeAllInstances, + provideInstance, + provideTmpdirInstance, + reloadTestInstance, + tmpdirScoped, +} from "../fixture/fixture" import { testEffect } from "../lib/effect" import { MessageID, SessionID } from "../../src/session/schema" @@ -1024,7 +1030,7 @@ it.live("pending permission rejects on instance reload", () => }).pipe(run, Effect.forkScoped) expect(yield* waitForPending(1).pipe(run)).toHaveLength(1) - yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) + yield* Effect.promise(() => reloadTestInstance({ directory: dir })) const exit = yield* Fiber.await(fiber) expect(Exit.isFailure(exit)).toBe(true) @@ -1118,7 +1124,7 @@ it.live("ask - abort should clear pending request", () => const pending = yield* waitForPending(1).pipe(run) expect(pending).toHaveLength(1) - yield* Effect.promise(() => InstanceRuntime.reloadInstance({ directory: dir })) + yield* Effect.promise(() => reloadTestInstance({ directory: dir })) const exit = yield* Fiber.await(fiber) expect(Exit.isFailure(exit)).toBe(true) diff --git a/packages/opencode/test/plugin/auth-override.test.ts b/packages/opencode/test/plugin/auth-override.test.ts index 4bee9857963c..eba302e775bb 100644 --- a/packages/opencode/test/plugin/auth-override.test.ts +++ b/packages/opencode/test/plugin/auth-override.test.ts @@ -1,11 +1,46 @@ import { describe, expect, test } from "bun:test" import path from "path" import fs from "fs/promises" -import { Effect } from "effect" -import { tmpdir } from "../fixture/fixture" -import { Instance } from "../../src/project/instance" +import { pathToFileURL } from "url" +import { Effect, Layer } from "effect" +import { provideTestInstance, tmpdir } from "../fixture/fixture" import { ProviderAuth } from "@/provider/auth" import { ProviderID } from "../../src/provider/schema" +import { Plugin } from "@/plugin" +import { Config } from "@/config/config" +import { Auth } from "@/auth" +import { Bus } from "@/bus" + +function layer(directory: string, plugins: string[]) { + return ProviderAuth.layer.pipe( + Layer.provide(Auth.defaultLayer), + Layer.provide( + Plugin.layer.pipe( + Layer.provide(Bus.layer), + Layer.provide( + Layer.mock(Config.Service)({ + get: () => + Effect.succeed({ + plugin: plugins, + plugin_origins: plugins.map((plugin) => ({ + spec: plugin, + source: path.join(directory, "opencode.json"), + scope: "local" as const, + })), + }), + getGlobal: () => Effect.succeed({}), + getConsoleState: () => Effect.die("not implemented"), + update: () => Effect.void, + updateGlobal: () => Effect.die("not implemented"), + invalidate: () => Effect.void, + directories: () => Effect.succeed([directory]), + waitForDependencies: () => Effect.void, + }), + ), + ), + ), + ) +} describe("plugin.auth-override", () => { test("user plugin overrides built-in github-copilot auth", async () => { @@ -37,23 +72,25 @@ describe("plugin.auth-override", () => { await using plain = await tmpdir() - const methods = await Instance.provide({ - directory: tmp.path, - fn: async () => { - return Effect.runPromise( - ProviderAuth.Service.use((svc) => svc.methods()).pipe(Effect.provide(ProviderAuth.defaultLayer)), - ) - }, - }) - - const plainMethods = await Instance.provide({ - directory: plain.path, - fn: async () => { - return Effect.runPromise( - ProviderAuth.Service.use((svc) => svc.methods()).pipe(Effect.provide(ProviderAuth.defaultLayer)), - ) - }, - }) + const plugin = pathToFileURL(path.join(tmp.path, ".opencode", "plugin", "custom-copilot-auth.ts")).href + const [methods, plainMethods] = await Promise.all([ + provideTestInstance({ + directory: tmp.path, + fn: async () => { + return Effect.runPromise( + ProviderAuth.Service.use((svc) => svc.methods()).pipe(Effect.provide(layer(tmp.path, [plugin]))), + ) + }, + }), + provideTestInstance({ + directory: plain.path, + fn: async () => { + return Effect.runPromise( + ProviderAuth.Service.use((svc) => svc.methods()).pipe(Effect.provide(layer(plain.path, []))), + ) + }, + }), + ]) const copilot = methods[ProviderID.make("github-copilot")] expect(copilot).toBeDefined() diff --git a/packages/opencode/test/plugin/loader-shared.test.ts b/packages/opencode/test/plugin/loader-shared.test.ts index e24cd05070fa..bf02fbd2782b 100644 --- a/packages/opencode/test/plugin/loader-shared.test.ts +++ b/packages/opencode/test/plugin/loader-shared.test.ts @@ -1,9 +1,9 @@ import { afterAll, afterEach, describe, expect, spyOn, test } from "bun:test" -import { Effect } from "effect" +import { Effect, Layer } from "effect" import fs from "fs/promises" import path from "path" import { pathToFileURL } from "url" -import { disposeAllInstances, tmpdir } from "../fixture/fixture" +import { disposeAllInstances, provideInstance, tmpdir } from "../fixture/fixture" import { Filesystem } from "@/util/filesystem" const disableDefault = process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS @@ -12,7 +12,8 @@ process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS = "1" const { Plugin } = await import("../../src/plugin/index") const { PluginLoader } = await import("../../src/plugin/loader") const { readPackageThemes } = await import("../../src/plugin/shared") -const { Instance } = await import("../../src/project/instance") +const { Config } = await import("../../src/config/config") +const { Bus } = await import("../../src/bus") const { Npm } = await import("@opencode-ai/core/npm") afterAll(() => { @@ -28,14 +29,37 @@ afterEach(async () => { }) async function load(dir: string) { - return Instance.provide({ - directory: dir, - fn: async () => - Effect.gen(function* () { - const plugin = yield* Plugin.Service - yield* plugin.list() - }).pipe(Effect.provide(Plugin.defaultLayer), Effect.runPromise), - }) + const source = path.join(dir, "opencode.json") + const config = (await Bun.file(source).json()) as { plugin?: Array]> } + const plugins = config.plugin ?? [] + return Effect.gen(function* () { + const plugin = yield* Plugin.Service + yield* plugin.list() + }).pipe( + Effect.provide( + Plugin.layer.pipe( + Layer.provide(Bus.layer), + Layer.provide( + Layer.mock(Config.Service)({ + get: () => + Effect.succeed({ + plugin: plugins, + plugin_origins: plugins.map((plugin) => ({ spec: plugin, source, scope: "local" as const })), + }), + getGlobal: () => Effect.succeed({}), + getConsoleState: () => Effect.die("not implemented"), + update: () => Effect.void, + updateGlobal: () => Effect.die("not implemented"), + invalidate: () => Effect.void, + directories: () => Effect.succeed([dir]), + waitForDependencies: () => Effect.void, + }), + ), + ), + ), + provideInstance(dir), + Effect.runPromise, + ) } describe("plugin.loader.shared", () => { From 83a407f18336df98bdaf63715b046b7ad31d46d9 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 18:46:27 -0400 Subject: [PATCH 3/6] refactor(config): move disposal to lifecycle boundaries --- .../specs/effect/config-lifecycle-plan.md | 342 ++++++++++++++++++ packages/opencode/src/cli/cmd/tui/worker.ts | 10 +- packages/opencode/src/config/config.ts | 40 +- .../opencode/src/server/global-lifecycle.ts | 25 ++ packages/opencode/src/server/routes/global.ts | 24 +- .../src/server/routes/instance/config.ts | 4 + .../routes/instance/httpapi/groups/global.ts | 1 + .../instance/httpapi/handlers/config.ts | 2 +- .../instance/httpapi/handlers/global.ts | 15 +- packages/opencode/test/config/config.test.ts | 39 +- packages/opencode/test/config/tui.test.ts | 6 +- packages/opencode/test/fixture/config.ts | 23 ++ .../test/plugin/auth-override.test.ts | 10 +- .../test/plugin/loader-shared.test.ts | 10 +- .../opencode/test/session/compaction.test.ts | 3 +- .../opencode/test/session/instruction.test.ts | 17 +- packages/opencode/test/tool/registry.test.ts | 42 ++- .../opencode/test/tool/truncation.test.ts | 3 +- 18 files changed, 509 insertions(+), 107 deletions(-) create mode 100644 packages/opencode/specs/effect/config-lifecycle-plan.md create mode 100644 packages/opencode/src/server/global-lifecycle.ts create mode 100644 packages/opencode/test/fixture/config.ts diff --git a/packages/opencode/specs/effect/config-lifecycle-plan.md b/packages/opencode/specs/effect/config-lifecycle-plan.md new file mode 100644 index 000000000000..35c960bcb297 --- /dev/null +++ b/packages/opencode/specs/effect/config-lifecycle-plan.md @@ -0,0 +1,342 @@ +# Config Lifecycle Plan + +## Goal + +Remove instance disposal from `Config` so config loading/writing stays a pure config concern and runtime lifecycle invalidation happens at the caller/orchestration boundary. + +This specifically removes the need for `Config` to import or lazily import `InstanceRuntime`. + +## Current Coupling + +`src/config/config.ts` currently does three separate things: + +1. Load and cache global config. +2. Load, merge, and write project/global config files. +3. Dispose instances when config changes. + +The third responsibility is the problem. + +Current disposal paths: + +1. `Config.update(config)` writes project `config.json`, then disposes the active instance unless `options.dispose === false`. +2. `Config.updateGlobal(config)` writes global config, then calls `Config.invalidate()` if the file changed. +3. `Config.invalidate(wait)` invalidates the global config cache, disposes all instances, and emits a global disposed event. + +## Desired Ownership + +`Config` should own: + +1. Reading config files. +2. Parsing and merging config. +3. Writing project/global config files. +4. Invalidating only its own global config cache. + +Callers should own: + +1. Disposing the current instance after a project config update. +2. Disposing all instances after a global config update or explicit reload. +3. Emitting server/global lifecycle events after disposal. + +## Concrete API Changes + +### `src/config/config.ts` + +1. Remove `loadInstanceRuntime()`. +2. Remove `InstanceRuntime`/`InstanceStore`/lifecycle imports from config. +3. Change `Interface.update` from: + +```ts +readonly update: (config: Info, options?: { dispose?: boolean }) => Effect.Effect +``` + +to: + +```ts +readonly update: (config: Info) => Effect.Effect +``` + +4. Change `Config.update` implementation to only write the project `config.json`. +5. Change `Interface.invalidate` to a config-only cache invalidation method, or rename it for clarity. + +Preferred final shape: + +```ts +readonly invalidate: () => Effect.Effect +``` + +`invalidate()` should only run `invalidateGlobal`. + +6. Change `Config.updateGlobal` to write global config, invalidate only config-global cache when changed, and return whether the file changed. + +Preferred final shape: + +```ts +readonly updateGlobal: (config: Info) => Effect.Effect<{ info: Info; changed: boolean }> +``` + +Implementation detail: + +```ts +if (changed) yield* invalidate() +return { info: next, changed } +``` + +Public API routes should still return only `result.info`; `changed` is for lifecycle orchestration only. + +## Caller Updates + +### Legacy instance config route + +File: `src/server/routes/instance/config.ts` + +Current: + +```ts +const cfg = yield* Config.Service +yield* cfg.update(config) +return config +``` + +Change to: + +```ts +const cfg = yield* Config.Service +yield* cfg.update(config) +const store = yield* InstanceStore.Service +yield* store.dispose(Instance.current) +return config +``` + +Imports needed: + +```ts +import { Instance } from "@/project/instance" +import { InstanceStore } from "@/project/instance-store" +``` + +Rationale: this route is an instance-scoped orchestration boundary, so it should own the instance disposal after writing project config. + +### Effect HttpApi instance config route + +File: `src/server/routes/instance/httpapi/handlers/config.ts` + +Current: + +```ts +yield* configSvc.update(ctx.payload, { dispose: false }) +yield* markInstanceForDisposal(yield* InstanceState.context) +return ctx.payload +``` + +Change to: + +```ts +yield* configSvc.update(ctx.payload) +yield* markInstanceForDisposal(yield* InstanceState.context) +return ctx.payload +``` + +Rationale: this route already has correct ownership. It writes config first, then delegates disposal to HttpApi lifecycle middleware. + +### Legacy global config route + +File: `src/server/routes/global.ts` + +Current: + +```ts +const next = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.updateGlobal(config))) +return c.json(next) +``` + +Change to run config write, then if the file changed, schedule the same dispose-all/global-disposed side effect that `Config.invalidate(false)` currently schedules. + +Important behavior to preserve: + +1. Do not dispose instances when the serialized global config did not change. +2. Do not make the HTTP response wait for instance disposal. Current `updateGlobal -> invalidate()` schedules disposal asynchronously when `wait` is omitted. + +Preferred implementation shape: + +```ts +const result = await AppRuntime.runPromise( + Effect.gen(function* () { + const cfg = yield* Config.Service + return yield* cfg.updateGlobal(config) + }), +) +if (result.changed) void AppRuntime.runPromise(disposeAllInstancesAndEmitGlobalDisposed).catch(() => undefined) +return c.json(result.info) +``` + +Imports needed: + +```ts +import { Effect } from "effect" +import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" +``` + +`src/server/routes/global.ts` already defines `GlobalDisposedEvent`; move that event definition to the shared helper module or re-export it from there so `/dispose`, legacy global config update, and HttpApi global config update use one event source. + +### Effect HttpApi global config route + +File: `src/server/routes/instance/httpapi/handlers/global.ts` + +Current: + +```ts +return yield* config.updateGlobal(ctx.payload) +``` + +Change to preserve the existing changed-only and async-disposal semantics: + +```ts +const result = yield* config.updateGlobal(ctx.payload) +if (result.changed) bridge.fork(disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true })) +return result.info +``` + +Imports needed: + +```ts +import { EffectBridge } from "@/effect/bridge" +import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" +``` + +Also yield a stable bridge at handler construction: + +```ts +const bridge = yield* EffectBridge.make() +``` + +Do not use `Effect.forkScoped` for this fire-and-forget disposal; the request scope can close before disposal finishes. + +`src/server/routes/instance/httpapi/handlers/global.ts` already yields `InstanceStore.Service` for `/dispose`. Keep `/dispose` strict, or use the shared helper with `swallowErrors: false` so explicit disposal failures still surface. + +### TUI worker reload + +File: `src/cli/cmd/tui/worker.ts` + +Current: + +```ts +await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.invalidate(true))) +``` + +Change to: + +```ts +await AppRuntime.runPromise( + Effect.gen(function* () { + const cfg = yield* Config.Service + const store = yield* InstanceStore.Service + yield* cfg.invalidate() + yield* store.disposeAll() + }), +) +``` + +Imports needed: + +```ts +import { Effect } from "effect" +import { InstanceStore } from "@/project/instance-store" +``` + +No global disposed event is required here unless existing TUI behavior depends on it. The current worker path only calls `Config.invalidate(true)` and does not directly interact with server event streams. + +## Helper Extraction + +If both global routes need identical "dispose all and emit global disposed" behavior, extract a helper outside `Config`. + +Preferred location: + +`src/server/global-lifecycle.ts` + +Suggested helper: + +```ts +export const emitGlobalDisposed = Effect.sync(() => + GlobalBus.emit("event", { + directory: "global", + payload: { + type: Event.Disposed.type, + properties: {}, + }, + }), +) + +export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn("Server.disposeAllInstancesAndEmitGlobalDisposed")(function* (options?: { + swallowErrors?: boolean +}) { + const store = yield* InstanceStore.Service + const dispose = store.disposeAll() + yield* (options?.swallowErrors ? dispose.pipe(Effect.catch(() => Effect.void)) : dispose) + yield* emitGlobalDisposed +}) +``` + +Use this helper only from server/global route code and explicit reload/dispose orchestration. Do not import it into `Config`. + +Use `swallowErrors: true` only for paths that previously swallowed disposal errors, such as config invalidation. Keep explicit `/dispose` strict by omitting `swallowErrors`. + +## Tests To Update + +### Config tests + +File: `test/config/config.test.ts` + +1. `save(...)`, `saveGlobal(...)`, and `clear(...)` helpers should still run against `Config.layer` only. +2. They should not need `InstanceRuntime`, `InstanceStore`, or no-op lifecycle mocks. +3. Existing config tests should continue to pass because config no longer disposes instances internally. + +### TUI config tests + +File: `test/config/tui.test.ts` + +1. The `clear` helper currently calls `Config.invalidate` through `AppRuntime`. +2. After `invalidate()` is config-only, this is fine and should not dispose instances. + +### Route behavior tests + +Add or update focused tests for lifecycle ownership: + +1. Legacy instance config route disposes only the active instance after project config update. +2. HttpApi instance config route still marks the active instance for disposal after project config update. +3. Legacy global config route disposes all instances after global config update. +4. HttpApi global config route disposes all instances after global config update. +5. Global config routes do not dispose instances when the config write is a no-op. + +Prefer existing route tests if they already cover config update behavior. Do not add broad integration tests unless necessary. + +Suggested new focused files if no existing test has the right harness: + +1. `test/server/global-config.test.ts` for legacy Hono global config update lifecycle. +2. `test/server/httpapi-global-config.test.ts` for Effect HttpApi global config update lifecycle. + +## Verification Commands + +Run from `packages/opencode`: + +```bash +bun typecheck +bun run test test/config/config.test.ts test/config/tui.test.ts +bun run test test/server/httpapi-config.test.ts test/server/httpapi-instance-context.test.ts test/server/httpapi-bridge.test.ts +bun run test test/server/global-config.test.ts test/server/httpapi-global-config.test.ts +bun run test test/project/instance-bootstrap-regression.test.ts test/agent/plugin-agent-regression.test.ts test/project/instance.test.ts +env -u OPENCODE_EXPERIMENTAL_WORKSPACES bun run test +``` + +## Non-Goals + +1. Do not remove `InstanceRuntime` entirely in this change. It is still needed for legacy Promise/ALS callers. +2. Do not change `InstanceStore` bootstrap ownership. +3. Do not change config parsing/merging semantics. +4. Do not make `Config.layer` depend on `InstanceStore.Service`. + +## Expected End State + +1. `src/config/config.ts` has no import or dynamic import of `InstanceRuntime`, `InstanceStore`, or server lifecycle helpers. +2. `Config.update` and `Config.updateGlobal` only write config and invalidate config-owned caches. +3. Instance disposal is visible at route/worker orchestration boundaries. +4. Tests that exercise config parsing/writing no longer need special lifecycle stubs. diff --git a/packages/opencode/src/cli/cmd/tui/worker.ts b/packages/opencode/src/cli/cmd/tui/worker.ts index 143236edf35e..e4fbeb2fbce5 100644 --- a/packages/opencode/src/cli/cmd/tui/worker.ts +++ b/packages/opencode/src/cli/cmd/tui/worker.ts @@ -12,6 +12,8 @@ import { writeHeapSnapshot } from "node:v8" import { Heap } from "@/cli/heap" import { AppRuntime } from "@/effect/app-runtime" import { ensureProcessMetadata } from "@opencode-ai/core/util/opencode-process" +import { Effect } from "effect" +import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" ensureProcessMetadata("worker") @@ -83,7 +85,13 @@ export const rpc = { }) }, async reload() { - await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.invalidate(true))) + await AppRuntime.runPromise( + Effect.gen(function* () { + const cfg = yield* Config.Service + yield* cfg.invalidate() + yield* disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }) + }), + ) }, async shutdown() { Log.Default.info("worker shutting down") diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 4e39adfb478b..a63d77013f07 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -14,8 +14,6 @@ import { applyEdits, modify } from "jsonc-parser" import { type InstanceContext } from "../project/instance" import { InstallationLocal, InstallationVersion } from "@opencode-ai/core/installation/version" import { existsSync } from "fs" -import { GlobalBus } from "@/bus/global" -import { Event } from "../server/event" import { Account } from "@/account/account" import { isRecord } from "@/util/record" import type { ConsoleState } from "./console-state" @@ -46,10 +44,6 @@ import { Npm } from "@opencode-ai/core/npm" const log = Log.create({ service: "config" }) -async function loadInstanceRuntime() { - return (await import("../project/instance-runtime")).InstanceRuntime -} - // Custom merge function that concatenates array fields instead of replacing them // Keep remeda's deep conditional merge type out of hot config-loading paths; TS profiling showed it dominates here. function mergeConfig(target: Info, source: Info): Info { @@ -292,9 +286,9 @@ export interface Interface { readonly get: () => Effect.Effect readonly getGlobal: () => Effect.Effect readonly getConsoleState: () => Effect.Effect - readonly update: (config: Info, options?: { dispose?: boolean }) => Effect.Effect - readonly updateGlobal: (config: Info) => Effect.Effect - readonly invalidate: (wait?: boolean) => Effect.Effect + readonly update: (config: Info) => Effect.Effect + readonly updateGlobal: (config: Info) => Effect.Effect<{ info: Info; changed: boolean }> + readonly invalidate: () => Effect.Effect readonly directories: () => Effect.Effect readonly waitForDependencies: () => Effect.Effect } @@ -733,38 +727,17 @@ export const layer = Layer.effect( ) }) - const update = Effect.fn("Config.update")(function* (config: Info, options?: { dispose?: boolean }) { + const update = Effect.fn("Config.update")(function* (config: Info) { const dir = yield* InstanceState.directory const file = path.join(dir, "config.json") const existing = yield* loadFile(file) yield* fs .writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2)) .pipe(Effect.orDie) - if (options?.dispose !== false) { - // Fail loudly if no instance is bound — silently skipping would - // mask "config update without an active instance" bugs. The throw - // comes from `Instance.current` inside `InstanceState.context`. - const ctx = yield* InstanceState.context - yield* Effect.promise(async () => (await loadInstanceRuntime()).disposeInstance(ctx)) - } }) - const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) { + const invalidate = Effect.fn("Config.invalidate")(function* () { yield* invalidateGlobal - const task = loadInstanceRuntime() - .then((runtime) => runtime.disposeAllInstances()) - .catch(() => undefined) - .finally(() => - GlobalBus.emit("event", { - directory: "global", - payload: { - type: Event.Disposed.type, - properties: {}, - }, - }), - ) - if (wait) yield* Effect.promise(() => task) - else void task }) const updateGlobal = Effect.fn("Config.updateGlobal")(function* (config: Info) { @@ -788,9 +761,8 @@ export const layer = Layer.effect( if (changed) yield* fs.writeFileString(file, updated).pipe(Effect.orDie) } - // Only tear down running instances if the config actually changed. if (changed) yield* invalidate() - return next + return { info: next, changed } }) return Service.of({ diff --git a/packages/opencode/src/server/global-lifecycle.ts b/packages/opencode/src/server/global-lifecycle.ts new file mode 100644 index 000000000000..c4d34b3ecfa3 --- /dev/null +++ b/packages/opencode/src/server/global-lifecycle.ts @@ -0,0 +1,25 @@ +import { GlobalBus } from "@/bus/global" +import { InstanceStore } from "@/project/instance-store" +import { Effect } from "effect" +import { Event } from "./event" + +export const emitGlobalDisposed = Effect.sync(() => + GlobalBus.emit("event", { + directory: "global", + payload: { + type: Event.Disposed.type, + properties: {}, + }, + }), +) + +export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn( + "Server.disposeAllInstancesAndEmitGlobalDisposed", +)(function* (options?: { swallowErrors?: boolean }) { + const store = yield* InstanceStore.Service + const dispose = store.disposeAll() + yield* (options?.swallowErrors ? dispose.pipe(Effect.catch(() => Effect.void)) : dispose) + yield* emitGlobalDisposed +}) + +export * as GlobalLifecycle from "./global-lifecycle" diff --git a/packages/opencode/src/server/routes/global.ts b/packages/opencode/src/server/routes/global.ts index 7a3adcfc7a52..4a491d95b6ae 100644 --- a/packages/opencode/src/server/routes/global.ts +++ b/packages/opencode/src/server/routes/global.ts @@ -1,25 +1,23 @@ import { Hono, type Context } from "hono" import { describeRoute, resolver, validator } from "hono-openapi" import { streamSSE } from "hono/streaming" -import { Effect, Schema } from "effect" +import { Effect } from "effect" import z from "zod" import { BusEvent } from "@/bus/bus-event" import { SyncEvent } from "@/sync" import { GlobalBus } from "@/bus/global" import { AppRuntime } from "@/effect/app-runtime" import { AsyncQueue } from "@/util/queue" -import { InstanceRuntime } from "../../project/instance-runtime" import { Installation } from "@/installation" import { InstallationVersion } from "@opencode-ai/core/installation/version" import * as Log from "@opencode-ai/core/util/log" import { lazy } from "../../util/lazy" import { Config } from "@/config/config" import { errors } from "../error" +import { disposeAllInstancesAndEmitGlobalDisposed } from "../global-lifecycle" const log = Log.create({ service: "server" }) -export const GlobalDisposedEvent = BusEvent.define("global.disposed", Schema.Struct({})) - async function streamEvents(c: Context, subscribe: (q: AsyncQueue) => () => void) { return streamSSE(c, async (stream) => { const q = new AsyncQueue() @@ -178,8 +176,13 @@ export const GlobalRoutes = lazy(() => validator("json", Config.Info.zod), async (c) => { const config = c.req.valid("json") - const next = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.updateGlobal(config))) - return c.json(next) + const result = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.updateGlobal(config))) + if (result.changed) { + void AppRuntime.runPromise(disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true })).catch( + () => undefined, + ) + } + return c.json(result.info) }, ) .post( @@ -200,14 +203,7 @@ export const GlobalRoutes = lazy(() => }, }), async (c) => { - await InstanceRuntime.disposeAllInstances() - GlobalBus.emit("event", { - directory: "global", - payload: { - type: GlobalDisposedEvent.type, - properties: {}, - }, - }) + await AppRuntime.runPromise(disposeAllInstancesAndEmitGlobalDisposed()) return c.json(true) }, ) diff --git a/packages/opencode/src/server/routes/instance/config.ts b/packages/opencode/src/server/routes/instance/config.ts index f055917b0c79..c665140c0f5f 100644 --- a/packages/opencode/src/server/routes/instance/config.ts +++ b/packages/opencode/src/server/routes/instance/config.ts @@ -2,6 +2,8 @@ import { Hono } from "hono" import { describeRoute, validator, resolver } from "hono-openapi" import z from "zod" import { Config } from "@/config/config" +import { Instance } from "@/project/instance" +import { InstanceStore } from "@/project/instance-store" import { Provider } from "@/provider/provider" import { errors } from "../../error" import { lazy } from "@/util/lazy" @@ -55,7 +57,9 @@ export const ConfigRoutes = lazy(() => jsonRequest("ConfigRoutes.update", c, function* () { const config = c.req.valid("json") const cfg = yield* Config.Service + const store = yield* InstanceStore.Service yield* cfg.update(config) + yield* store.dispose(Instance.current) return config }), ) diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/global.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/global.ts index 272b086065b8..75441b4ca4a3 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/global.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/global.ts @@ -1,6 +1,7 @@ import { Config } from "@/config/config" import { BusEvent } from "@/bus/bus-event" import { SyncEvent } from "@/sync" +import "@/server/event" import { Schema } from "effect" import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" import { described } from "./metadata" diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/config.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/config.ts index 58aa81098c75..753ba0313803 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/config.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/config.ts @@ -16,7 +16,7 @@ export const configHandlers = HttpApiBuilder.group(InstanceHttpApi, "config", (h }) const update = Effect.fn("ConfigHttpApi.update")(function* (ctx) { - yield* configSvc.update(ctx.payload, { dispose: false }) + yield* configSvc.update(ctx.payload) yield* markInstanceForDisposal(yield* InstanceState.context) return ctx.payload }) diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/global.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/global.ts index bcad2832e2e5..f9be57f4fd89 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/global.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/global.ts @@ -1,7 +1,8 @@ import { Config } from "@/config/config" import { GlobalBus, type GlobalEvent as GlobalBusEvent } from "@/bus/global" +import { EffectBridge } from "@/effect/bridge" import { Installation } from "@/installation" -import { InstanceStore } from "@/project/instance-store" +import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" import { InstallationVersion } from "@opencode-ai/core/installation/version" import * as Log from "@opencode-ai/core/util/log" import { Effect, Queue, Schema } from "effect" @@ -68,7 +69,7 @@ export const globalHandlers = HttpApiBuilder.group(RootHttpApi, "global", (handl Effect.gen(function* () { const config = yield* Config.Service const installation = yield* Installation.Service - const store = yield* InstanceStore.Service + const bridge = yield* EffectBridge.make() const health = Effect.fn("GlobalHttpApi.health")(function* () { return { healthy: true as const, version: InstallationVersion } @@ -83,15 +84,13 @@ export const globalHandlers = HttpApiBuilder.group(RootHttpApi, "global", (handl }) const configUpdate = Effect.fn("GlobalHttpApi.configUpdate")(function* (ctx) { - return yield* config.updateGlobal(ctx.payload) + const result = yield* config.updateGlobal(ctx.payload) + if (result.changed) bridge.fork(disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true })) + return result.info }) const dispose = Effect.fn("GlobalHttpApi.dispose")(function* () { - yield* store.disposeAll() - GlobalBus.emit("event", { - directory: "global", - payload: { type: "global.disposed", properties: {} }, - }) + yield* disposeAllInstancesAndEmitGlobalDisposed() return true }) diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index ffad0ae57e07..9c4cbd788c81 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -12,8 +12,9 @@ import { Account } from "../../src/account/account" import { AccessToken, AccountID, OrgID } from "../../src/account/schema" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Env } from "../../src/env" -import { disposeAllInstances, provideTestInstance, provideTmpdirInstance } from "../fixture/fixture" +import { provideTestInstance, provideTmpdirInstance } from "../fixture/fixture" import { tmpdir } from "../fixture/fixture" +import { InstanceRuntime } from "@/project/instance-runtime" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { testEffect } from "../lib/effect" @@ -41,6 +42,12 @@ const emptyAuth = Layer.mock(Auth.Service)({ const testFlock = EffectFlock.defaultLayer +const noopNpm = Layer.mock(Npm.Service)({ + install: () => Effect.void, + add: () => Effect.die("not implemented"), + which: () => Effect.succeed(Option.none()), +}) + const layer = Config.layer.pipe( Layer.provide(testFlock), Layer.provide(AppFileSystem.defaultLayer), @@ -48,7 +55,7 @@ const layer = Config.layer.pipe( Layer.provide(emptyAuth), Layer.provide(emptyAccount), Layer.provideMerge(infra), - Layer.provide(Npm.defaultLayer), + Layer.provide(noopNpm), ) const it = testEffect(layer) @@ -57,9 +64,17 @@ const load = () => Effect.runPromise(Config.Service.use((svc) => svc.get()).pipe const save = (config: Config.Info) => Effect.runPromise(Config.Service.use((svc) => svc.update(config)).pipe(Effect.scoped, Effect.provide(layer))) const saveGlobal = (config: Config.Info) => - Effect.runPromise(Config.Service.use((svc) => svc.updateGlobal(config)).pipe(Effect.scoped, Effect.provide(layer))) -const clear = (wait = false) => - Effect.runPromise(Config.Service.use((svc) => svc.invalidate(wait)).pipe(Effect.scoped, Effect.provide(layer))) + Effect.runPromise( + Config.Service.use((svc) => svc.updateGlobal(config)).pipe( + Effect.map((result) => result.info), + Effect.scoped, + Effect.provide(layer), + ), + ) +const clear = async (wait = false) => { + await Effect.runPromise(Config.Service.use((svc) => svc.invalidate()).pipe(Effect.scoped, Effect.provide(layer))) + if (wait) await InstanceRuntime.disposeAllInstances() +} const listDirs = () => Effect.runPromise(Config.Service.use((svc) => svc.directories()).pipe(Effect.scoped, Effect.provide(layer))) const ready = () => @@ -108,7 +123,7 @@ async function check(map: (dir: string) => string) { }, }) } finally { - await disposeAllInstances() + await InstanceRuntime.disposeAllInstances() ;(Global.Path as { config: string }).config = prev await clear() } @@ -483,6 +498,7 @@ test("resolves env templates in account config with account token", async () => Layer.provide(emptyAuth), Layer.provide(fakeAccount), Layer.provideMerge(infra), + Layer.provide(noopNpm), ) try { @@ -493,7 +509,7 @@ test("resolves env templates in account config with account token", async () => expect(config.provider?.["opencode"]?.options?.apiKey).toBe("st_test_token") }), ), - ).pipe(Effect.scoped, Effect.provide(layer), Effect.provide(Npm.defaultLayer), Effect.runPromise) + ).pipe(Effect.scoped, Effect.provide(layer), Effect.runPromise) } finally { if (originalControlToken !== undefined) { process.env["OPENCODE_CONSOLE_TOKEN"] = originalControlToken @@ -986,11 +1002,6 @@ test("installs dependencies in writable OPENCODE_CONFIG_DIR", async () => { const prev = process.env.OPENCODE_CONFIG_DIR process.env.OPENCODE_CONFIG_DIR = tmp.extra - const noopNpm = Layer.mock(Npm.Service)({ - install: () => Effect.void, - add: () => Effect.die("not implemented"), - which: () => Effect.succeed(Option.none()), - }) const testLayer = Config.layer.pipe( Layer.provide(testFlock), Layer.provide(AppFileSystem.defaultLayer), @@ -1883,7 +1894,7 @@ test("project config overrides remote well-known config", async () => { Layer.provide(fakeAuth), Layer.provide(emptyAccount), Layer.provideMerge(infra), - Layer.provide(Npm.defaultLayer), + Layer.provide(noopNpm), ) try { @@ -1941,7 +1952,7 @@ test("wellknown URL with trailing slash is normalized", async () => { Layer.provide(fakeAuth), Layer.provide(emptyAccount), Layer.provideMerge(infra), - Layer.provide(Npm.defaultLayer), + Layer.provide(noopNpm), ) try { diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index 903af71fc28e..870246f65526 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -3,6 +3,7 @@ import path from "path" import fs from "fs/promises" import { provideTestInstance, tmpdir } from "../fixture/fixture" import { Instance } from "../../src/project/instance" +import { InstanceRuntime } from "@/project/instance-runtime" import { TuiConfig } from "../../src/cli/cmd/tui/config/tui" import { Config } from "@/config/config" import { Global } from "@opencode-ai/core/global" @@ -13,7 +14,10 @@ import { CurrentWorkingDirectory } from "@/cli/cmd/tui/config/cwd" import { ConfigPlugin } from "@/config/plugin" const wintest = process.platform === "win32" ? test : test.skip -const clear = (wait = false) => AppRuntime.runPromise(Config.Service.use((svc) => svc.invalidate(wait))) +const clear = async (wait = false) => { + await AppRuntime.runPromise(Config.Service.use((svc) => svc.invalidate())) + if (wait) await InstanceRuntime.disposeAllInstances() +} const load = () => AppRuntime.runPromise(Config.Service.use((svc) => svc.get())) beforeEach(async () => { diff --git a/packages/opencode/test/fixture/config.ts b/packages/opencode/test/fixture/config.ts new file mode 100644 index 000000000000..4cd90c51bf5a --- /dev/null +++ b/packages/opencode/test/fixture/config.ts @@ -0,0 +1,23 @@ +import { Config } from "@/config/config" +import { emptyConsoleState } from "@/config/console-state" +import { Effect, Layer } from "effect" + +export function make(overrides: Partial = {}) { + return Config.Service.of({ + get: () => Effect.succeed({}), + getGlobal: () => Effect.succeed({}), + getConsoleState: () => Effect.succeed(emptyConsoleState), + update: () => Effect.void, + updateGlobal: (config) => Effect.succeed({ info: config, changed: false }), + invalidate: () => Effect.void, + directories: () => Effect.succeed([]), + waitForDependencies: () => Effect.void, + ...overrides, + }) +} + +export function layer(overrides?: Partial) { + return Layer.succeed(Config.Service, make(overrides)) +} + +export * as TestConfig from "./config" diff --git a/packages/opencode/test/plugin/auth-override.test.ts b/packages/opencode/test/plugin/auth-override.test.ts index eba302e775bb..4a4cef581ebd 100644 --- a/packages/opencode/test/plugin/auth-override.test.ts +++ b/packages/opencode/test/plugin/auth-override.test.ts @@ -7,9 +7,9 @@ import { provideTestInstance, tmpdir } from "../fixture/fixture" import { ProviderAuth } from "@/provider/auth" import { ProviderID } from "../../src/provider/schema" import { Plugin } from "@/plugin" -import { Config } from "@/config/config" import { Auth } from "@/auth" import { Bus } from "@/bus" +import { TestConfig } from "../fixture/config" function layer(directory: string, plugins: string[]) { return ProviderAuth.layer.pipe( @@ -18,7 +18,7 @@ function layer(directory: string, plugins: string[]) { Plugin.layer.pipe( Layer.provide(Bus.layer), Layer.provide( - Layer.mock(Config.Service)({ + TestConfig.layer({ get: () => Effect.succeed({ plugin: plugins, @@ -28,13 +28,7 @@ function layer(directory: string, plugins: string[]) { scope: "local" as const, })), }), - getGlobal: () => Effect.succeed({}), - getConsoleState: () => Effect.die("not implemented"), - update: () => Effect.void, - updateGlobal: () => Effect.die("not implemented"), - invalidate: () => Effect.void, directories: () => Effect.succeed([directory]), - waitForDependencies: () => Effect.void, }), ), ), diff --git a/packages/opencode/test/plugin/loader-shared.test.ts b/packages/opencode/test/plugin/loader-shared.test.ts index bf02fbd2782b..8c55950afffc 100644 --- a/packages/opencode/test/plugin/loader-shared.test.ts +++ b/packages/opencode/test/plugin/loader-shared.test.ts @@ -12,9 +12,9 @@ process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS = "1" const { Plugin } = await import("../../src/plugin/index") const { PluginLoader } = await import("../../src/plugin/loader") const { readPackageThemes } = await import("../../src/plugin/shared") -const { Config } = await import("../../src/config/config") const { Bus } = await import("../../src/bus") const { Npm } = await import("@opencode-ai/core/npm") +const { TestConfig } = await import("../fixture/config") afterAll(() => { if (disableDefault === undefined) { @@ -40,19 +40,13 @@ async function load(dir: string) { Plugin.layer.pipe( Layer.provide(Bus.layer), Layer.provide( - Layer.mock(Config.Service)({ + TestConfig.layer({ get: () => Effect.succeed({ plugin: plugins, plugin_origins: plugins.map((plugin) => ({ spec: plugin, source, scope: "local" as const })), }), - getGlobal: () => Effect.succeed({}), - getConsoleState: () => Effect.die("not implemented"), - update: () => Effect.void, - updateGlobal: () => Effect.die("not implemented"), - invalidate: () => Effect.void, directories: () => Effect.succeed([dir]), - waitForDependencies: () => Effect.void, }), ), ), diff --git a/packages/opencode/test/session/compaction.test.ts b/packages/opencode/test/session/compaction.test.ts index f35e044d7baa..f3f7cbaef7b2 100644 --- a/packages/opencode/test/session/compaction.test.ts +++ b/packages/opencode/test/session/compaction.test.ts @@ -26,6 +26,7 @@ import { Snapshot } from "../../src/snapshot" import { ProviderTest } from "../fake/provider" import { testEffect } from "../lib/effect" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { TestConfig } from "../fixture/config" void Log.init({ print: false }) @@ -208,7 +209,7 @@ function layer(result: "continue" | "compact") { function cfg(compaction?: Config.Info["compaction"]) { const base = Config.Info.zod.parse({}) - return Layer.mock(Config.Service)({ + return TestConfig.layer({ get: () => Effect.succeed({ ...base, compaction }), }) } diff --git a/packages/opencode/test/session/instruction.test.ts b/packages/opencode/test/session/instruction.test.ts index f80081759426..3bb38c87867a 100644 --- a/packages/opencode/test/session/instruction.test.ts +++ b/packages/opencode/test/session/instruction.test.ts @@ -5,8 +5,6 @@ import { FetchHttpClient } from "effect/unstable/http" import { NodeFileSystem } from "@effect/platform-node" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { AppFileSystem } from "@opencode-ai/core/filesystem" -import { Config } from "@/config/config" -import { emptyConsoleState } from "@/config/console-state" import { ModelID, ProviderID } from "../../src/provider/schema" import { Instruction } from "../../src/session/instruction" import type { MessageV2 } from "../../src/session/message-v2" @@ -14,22 +12,11 @@ import { MessageID, PartID, SessionID } from "../../src/session/schema" import { Global } from "@opencode-ai/core/global" import { provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" +import { TestConfig } from "../fixture/config" const it = testEffect(Layer.mergeAll(CrossSpawnSpawner.defaultLayer, NodeFileSystem.layer)) -const configLayer = Layer.succeed( - Config.Service, - Config.Service.of({ - get: () => Effect.succeed({}), - getGlobal: () => Effect.succeed({}), - getConsoleState: () => Effect.succeed(emptyConsoleState), - update: () => Effect.void, - updateGlobal: (config) => Effect.succeed(config), - invalidate: () => Effect.void, - directories: () => Effect.succeed([]), - waitForDependencies: () => Effect.void, - }), -) +const configLayer = TestConfig.layer() const instructionLayer = (global: Partial) => Instruction.layer.pipe( diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts index 0cd3ec4d18a8..f9ac07831ae4 100644 --- a/packages/opencode/test/tool/registry.test.ts +++ b/packages/opencode/test/tool/registry.test.ts @@ -7,10 +7,50 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { ToolRegistry } from "@/tool/registry" import { disposeAllInstances, provideTmpdirInstance } from "../fixture/fixture" import { testEffect } from "../lib/effect" +import { TestConfig } from "../fixture/config" +import { AppFileSystem } from "@opencode-ai/core/filesystem" +import { Plugin } from "@/plugin" +import { Question } from "@/question" +import { Todo } from "@/session/todo" +import { Skill } from "@/skill" +import { Agent } from "@/agent/agent" +import { Session } from "@/session/session" +import { Provider } from "@/provider/provider" +import { LSP } from "@/lsp/lsp" +import { Instruction } from "@/session/instruction" +import { Bus } from "@/bus" +import { FetchHttpClient } from "effect/unstable/http" +import { Format } from "@/format" +import { Ripgrep } from "@/file/ripgrep" +import * as Truncate from "@/tool/truncate" +import { InstanceState } from "@/effect/instance-state" const node = CrossSpawnSpawner.defaultLayer +const configLayer = TestConfig.layer({ + directories: () => InstanceState.directory.pipe(Effect.map((dir) => [path.join(dir, ".opencode")])), +}) + +const registryLayer = ToolRegistry.layer.pipe( + Layer.provide(configLayer), + Layer.provide(Plugin.defaultLayer), + Layer.provide(Question.defaultLayer), + Layer.provide(Todo.defaultLayer), + Layer.provide(Skill.defaultLayer), + Layer.provide(Agent.defaultLayer), + Layer.provide(Session.defaultLayer), + Layer.provide(Provider.defaultLayer), + Layer.provide(LSP.defaultLayer), + Layer.provide(Instruction.defaultLayer), + Layer.provide(AppFileSystem.defaultLayer), + Layer.provide(Bus.layer), + Layer.provide(FetchHttpClient.layer), + Layer.provide(Format.defaultLayer), + Layer.provide(node), + Layer.provide(Ripgrep.defaultLayer), + Layer.provide(Truncate.defaultLayer), +) -const it = testEffect(Layer.mergeAll(ToolRegistry.defaultLayer, node)) +const it = testEffect(Layer.mergeAll(registryLayer, node)) afterEach(async () => { await disposeAllInstances() diff --git a/packages/opencode/test/tool/truncation.test.ts b/packages/opencode/test/tool/truncation.test.ts index 9a01f95cd1c7..e836b23ebea2 100644 --- a/packages/opencode/test/tool/truncation.test.ts +++ b/packages/opencode/test/tool/truncation.test.ts @@ -9,6 +9,7 @@ import { Filesystem } from "@/util/filesystem" import path from "path" import { testEffect } from "../lib/effect" import { writeFileStringScoped } from "../lib/filesystem" +import { TestConfig } from "../fixture/config" const FIXTURES_DIR = path.join(import.meta.dir, "fixtures") const ROOT = path.resolve(import.meta.dir, "..", "..") @@ -19,7 +20,7 @@ const configuredLayer = (cfg: Config.Info) => Layer.mergeAll( Truncate.defaultLayer, NodeFileSystem.layer, - Layer.mock(Config.Service)({ get: () => Effect.succeed(cfg) }), + TestConfig.layer({ get: () => Effect.succeed(cfg) }), ) const configuredIt = (cfg: Config.Info) => testEffect(configuredLayer(cfg)) From 055df44448e5260c24cfde7910d2b3dee945cd7c Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 19:23:21 -0400 Subject: [PATCH 4/6] fix(lifecycle): tighten disposal boundaries --- .../opencode/src/project/instance-runtime.ts | 10 +++++----- .../opencode/src/server/global-lifecycle.ts | 18 +++++++++++++++--- .../src/server/routes/instance/config.ts | 5 ++--- .../src/server/routes/instance/index.ts | 1 - packages/opencode/test/config/tui.test.ts | 1 - .../opencode/test/plugin/auth-override.test.ts | 2 +- 6 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/opencode/src/project/instance-runtime.ts b/packages/opencode/src/project/instance-runtime.ts index dafefe842e27..a30bf5610711 100644 --- a/packages/opencode/src/project/instance-runtime.ts +++ b/packages/opencode/src/project/instance-runtime.ts @@ -3,11 +3,11 @@ import { type InstanceContext } from "./instance-context" import { InstanceStore, type LoadInput } from "./instance-store" import { Effect, Layer } from "effect" -// Bridge for Promise/ALS callers that cannot yet yield InstanceStore.Service. -// This keeps InstanceStore itself low-level while still giving legacy Hono and -// CLI paths the production bootstrap implementation. Delete this module once -// those callers are migrated to Effect boundaries that provide InstanceStore -// directly, like the HttpApi middleware does. +// Production InstanceStore wiring plus a bridge for Promise/ALS callers that +// cannot yet yield InstanceStore.Service. This keeps InstanceStore itself +// low-level while still giving legacy Hono and CLI paths the production +// bootstrap implementation. Delete the Promise helpers once those callers are +// migrated to Effect boundaries that provide InstanceStore directly. // Keep the bootstrap implementation import lazy: Instance is imported broadly, // and importing the app bootstrap graph at module load can trigger ESM cycles. export const layer = Layer.unwrap( diff --git a/packages/opencode/src/server/global-lifecycle.ts b/packages/opencode/src/server/global-lifecycle.ts index c4d34b3ecfa3..fbc300fad7f3 100644 --- a/packages/opencode/src/server/global-lifecycle.ts +++ b/packages/opencode/src/server/global-lifecycle.ts @@ -1,8 +1,11 @@ import { GlobalBus } from "@/bus/global" import { InstanceStore } from "@/project/instance-store" +import * as Log from "@opencode-ai/core/util/log" import { Effect } from "effect" import { Event } from "./event" +const log = Log.create({ service: "server" }) + export const emitGlobalDisposed = Effect.sync(() => GlobalBus.emit("event", { directory: "global", @@ -17,9 +20,18 @@ export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn( "Server.disposeAllInstancesAndEmitGlobalDisposed", )(function* (options?: { swallowErrors?: boolean }) { const store = yield* InstanceStore.Service - const dispose = store.disposeAll() - yield* (options?.swallowErrors ? dispose.pipe(Effect.catch(() => Effect.void)) : dispose) - yield* emitGlobalDisposed + yield* Effect.gen(function* () { + yield* (options?.swallowErrors + ? store.disposeAll().pipe( + Effect.catchCause((cause) => + Effect.sync(() => { + log.warn("global disposal failed", { cause }) + }), + ), + ) + : store.disposeAll()) + yield* emitGlobalDisposed + }).pipe(Effect.uninterruptible) }) export * as GlobalLifecycle from "./global-lifecycle" diff --git a/packages/opencode/src/server/routes/instance/config.ts b/packages/opencode/src/server/routes/instance/config.ts index c665140c0f5f..96a7e756de49 100644 --- a/packages/opencode/src/server/routes/instance/config.ts +++ b/packages/opencode/src/server/routes/instance/config.ts @@ -1,8 +1,7 @@ import { Hono } from "hono" import { describeRoute, validator, resolver } from "hono-openapi" -import z from "zod" import { Config } from "@/config/config" -import { Instance } from "@/project/instance" +import { InstanceState } from "@/effect/instance-state" import { InstanceStore } from "@/project/instance-store" import { Provider } from "@/provider/provider" import { errors } from "../../error" @@ -59,7 +58,7 @@ export const ConfigRoutes = lazy(() => const cfg = yield* Config.Service const store = yield* InstanceStore.Service yield* cfg.update(config) - yield* store.dispose(Instance.current) + yield* store.dispose(yield* InstanceState.context) return config }), ) diff --git a/packages/opencode/src/server/routes/instance/index.ts b/packages/opencode/src/server/routes/instance/index.ts index 4baef09384d4..f0da2f3d856a 100644 --- a/packages/opencode/src/server/routes/instance/index.ts +++ b/packages/opencode/src/server/routes/instance/index.ts @@ -25,7 +25,6 @@ import { ExperimentalRoutes } from "./experimental" import { ProviderRoutes } from "./provider" import { EventRoutes } from "./event" import { SyncRoutes } from "./sync" -import { InstanceMiddleware } from "./middleware" import { jsonRequest } from "./trace" export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => { diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index 870246f65526..a3f2a1b5fb3a 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -2,7 +2,6 @@ import { afterEach, beforeEach, expect, test } from "bun:test" import path from "path" import fs from "fs/promises" import { provideTestInstance, tmpdir } from "../fixture/fixture" -import { Instance } from "../../src/project/instance" import { InstanceRuntime } from "@/project/instance-runtime" import { TuiConfig } from "../../src/cli/cmd/tui/config/tui" import { Config } from "@/config/config" diff --git a/packages/opencode/test/plugin/auth-override.test.ts b/packages/opencode/test/plugin/auth-override.test.ts index 4a4cef581ebd..c77c0ca1c02a 100644 --- a/packages/opencode/test/plugin/auth-override.test.ts +++ b/packages/opencode/test/plugin/auth-override.test.ts @@ -91,7 +91,7 @@ describe("plugin.auth-override", () => { expect(copilot.length).toBe(1) expect(copilot[0].label).toBe("Test Override Auth") expect(plainMethods[ProviderID.make("github-copilot")][0].label).not.toBe("Test Override Auth") - }, 30000) // Increased timeout for plugin installation + }, 30000) }) const file = path.join(import.meta.dir, "../../src/plugin/index.ts") From d466a8ec77af646ee7541d0a75cfead4d3521b51 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 19:25:58 -0400 Subject: [PATCH 5/6] test(instance): cover legacy bootstrap entrypoints --- .../instance-bootstrap-regression.test.ts | 48 +++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/opencode/test/project/instance-bootstrap-regression.test.ts b/packages/opencode/test/project/instance-bootstrap-regression.test.ts index ea3937315031..bb8d43e0152d 100644 --- a/packages/opencode/test/project/instance-bootstrap-regression.test.ts +++ b/packages/opencode/test/project/instance-bootstrap-regression.test.ts @@ -1,20 +1,25 @@ import { afterEach, expect, test } from "bun:test" +import { Hono } from "hono" import { existsSync } from "node:fs" import path from "node:path" import { pathToFileURL } from "node:url" +import { bootstrap as cliBootstrap } from "../../src/cli/bootstrap" import { Instance } from "../../src/project/instance" +import { InstanceRuntime } from "../../src/project/instance-runtime" +import { InstanceMiddleware } from "../../src/server/routes/instance/middleware" import { disposeAllInstances, tmpdir } from "../fixture/fixture" -// Instance.provide must run bootstrap before fn. The plugin config hook writes -// a marker file, and fn deliberately avoids touching Plugin or config so the -// marker only exists if bootstrap ran at the instance boundary. +// These regressions cover the legacy instance-loading paths fixed by PRs +// #25389 and #25449. The plugin config hook writes a marker file, and the test +// bodies deliberately avoid touching Plugin or config directly. The marker only +// exists if InstanceBootstrap ran at the instance boundary. afterEach(async () => { await disposeAllInstances() }) -test("Instance.provide runs InstanceBootstrap before fn (boundary invariant)", async () => { - await using tmp = await tmpdir({ +async function bootstrapFixture() { + return tmpdir({ init: async (dir) => { const marker = path.join(dir, "config-hook-fired") const pluginFile = path.join(dir, "plugin.ts") @@ -40,10 +45,11 @@ test("Instance.provide runs InstanceBootstrap before fn (boundary invariant)", a return marker }, }) +} + +test("Instance.provide runs InstanceBootstrap before fn (boundary invariant)", async () => { + await using tmp = await bootstrapFixture() - // The body of `fn` deliberately does not yield Plugin, read config, or - // touch any service that would force Plugin.state to materialize on - // demand. The only way the marker gets written is if bootstrap ran. await Instance.provide({ directory: tmp.path, fn: async () => "ok", @@ -51,3 +57,29 @@ test("Instance.provide runs InstanceBootstrap before fn (boundary invariant)", a expect(existsSync(tmp.extra)).toBe(true) }) + +test("CLI bootstrap runs InstanceBootstrap before callback", async () => { + await using tmp = await bootstrapFixture() + + await cliBootstrap(tmp.path, async () => "ok") + + expect(existsSync(tmp.extra)).toBe(true) +}) + +test("legacy Hono instance middleware runs InstanceBootstrap before next handler", async () => { + await using tmp = await bootstrapFixture() + const app = new Hono().use(InstanceMiddleware()).get("/probe", (c) => c.text("ok")) + + const response = await app.request("/probe", { headers: { "x-opencode-directory": tmp.path } }) + + expect(response.status).toBe(200) + expect(existsSync(tmp.extra)).toBe(true) +}) + +test("InstanceRuntime.reloadInstance runs InstanceBootstrap", async () => { + await using tmp = await bootstrapFixture() + + await InstanceRuntime.reloadInstance({ directory: tmp.path }) + + expect(existsSync(tmp.extra)).toBe(true) +}) From 3d02aa6becfb01da4e12ff4ba6696b393396b4fd Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 19:27:29 -0400 Subject: [PATCH 6/6] chore: remove config lifecycle planning doc --- .../specs/effect/config-lifecycle-plan.md | 342 ------------------ 1 file changed, 342 deletions(-) delete mode 100644 packages/opencode/specs/effect/config-lifecycle-plan.md diff --git a/packages/opencode/specs/effect/config-lifecycle-plan.md b/packages/opencode/specs/effect/config-lifecycle-plan.md deleted file mode 100644 index 35c960bcb297..000000000000 --- a/packages/opencode/specs/effect/config-lifecycle-plan.md +++ /dev/null @@ -1,342 +0,0 @@ -# Config Lifecycle Plan - -## Goal - -Remove instance disposal from `Config` so config loading/writing stays a pure config concern and runtime lifecycle invalidation happens at the caller/orchestration boundary. - -This specifically removes the need for `Config` to import or lazily import `InstanceRuntime`. - -## Current Coupling - -`src/config/config.ts` currently does three separate things: - -1. Load and cache global config. -2. Load, merge, and write project/global config files. -3. Dispose instances when config changes. - -The third responsibility is the problem. - -Current disposal paths: - -1. `Config.update(config)` writes project `config.json`, then disposes the active instance unless `options.dispose === false`. -2. `Config.updateGlobal(config)` writes global config, then calls `Config.invalidate()` if the file changed. -3. `Config.invalidate(wait)` invalidates the global config cache, disposes all instances, and emits a global disposed event. - -## Desired Ownership - -`Config` should own: - -1. Reading config files. -2. Parsing and merging config. -3. Writing project/global config files. -4. Invalidating only its own global config cache. - -Callers should own: - -1. Disposing the current instance after a project config update. -2. Disposing all instances after a global config update or explicit reload. -3. Emitting server/global lifecycle events after disposal. - -## Concrete API Changes - -### `src/config/config.ts` - -1. Remove `loadInstanceRuntime()`. -2. Remove `InstanceRuntime`/`InstanceStore`/lifecycle imports from config. -3. Change `Interface.update` from: - -```ts -readonly update: (config: Info, options?: { dispose?: boolean }) => Effect.Effect -``` - -to: - -```ts -readonly update: (config: Info) => Effect.Effect -``` - -4. Change `Config.update` implementation to only write the project `config.json`. -5. Change `Interface.invalidate` to a config-only cache invalidation method, or rename it for clarity. - -Preferred final shape: - -```ts -readonly invalidate: () => Effect.Effect -``` - -`invalidate()` should only run `invalidateGlobal`. - -6. Change `Config.updateGlobal` to write global config, invalidate only config-global cache when changed, and return whether the file changed. - -Preferred final shape: - -```ts -readonly updateGlobal: (config: Info) => Effect.Effect<{ info: Info; changed: boolean }> -``` - -Implementation detail: - -```ts -if (changed) yield* invalidate() -return { info: next, changed } -``` - -Public API routes should still return only `result.info`; `changed` is for lifecycle orchestration only. - -## Caller Updates - -### Legacy instance config route - -File: `src/server/routes/instance/config.ts` - -Current: - -```ts -const cfg = yield* Config.Service -yield* cfg.update(config) -return config -``` - -Change to: - -```ts -const cfg = yield* Config.Service -yield* cfg.update(config) -const store = yield* InstanceStore.Service -yield* store.dispose(Instance.current) -return config -``` - -Imports needed: - -```ts -import { Instance } from "@/project/instance" -import { InstanceStore } from "@/project/instance-store" -``` - -Rationale: this route is an instance-scoped orchestration boundary, so it should own the instance disposal after writing project config. - -### Effect HttpApi instance config route - -File: `src/server/routes/instance/httpapi/handlers/config.ts` - -Current: - -```ts -yield* configSvc.update(ctx.payload, { dispose: false }) -yield* markInstanceForDisposal(yield* InstanceState.context) -return ctx.payload -``` - -Change to: - -```ts -yield* configSvc.update(ctx.payload) -yield* markInstanceForDisposal(yield* InstanceState.context) -return ctx.payload -``` - -Rationale: this route already has correct ownership. It writes config first, then delegates disposal to HttpApi lifecycle middleware. - -### Legacy global config route - -File: `src/server/routes/global.ts` - -Current: - -```ts -const next = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.updateGlobal(config))) -return c.json(next) -``` - -Change to run config write, then if the file changed, schedule the same dispose-all/global-disposed side effect that `Config.invalidate(false)` currently schedules. - -Important behavior to preserve: - -1. Do not dispose instances when the serialized global config did not change. -2. Do not make the HTTP response wait for instance disposal. Current `updateGlobal -> invalidate()` schedules disposal asynchronously when `wait` is omitted. - -Preferred implementation shape: - -```ts -const result = await AppRuntime.runPromise( - Effect.gen(function* () { - const cfg = yield* Config.Service - return yield* cfg.updateGlobal(config) - }), -) -if (result.changed) void AppRuntime.runPromise(disposeAllInstancesAndEmitGlobalDisposed).catch(() => undefined) -return c.json(result.info) -``` - -Imports needed: - -```ts -import { Effect } from "effect" -import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" -``` - -`src/server/routes/global.ts` already defines `GlobalDisposedEvent`; move that event definition to the shared helper module or re-export it from there so `/dispose`, legacy global config update, and HttpApi global config update use one event source. - -### Effect HttpApi global config route - -File: `src/server/routes/instance/httpapi/handlers/global.ts` - -Current: - -```ts -return yield* config.updateGlobal(ctx.payload) -``` - -Change to preserve the existing changed-only and async-disposal semantics: - -```ts -const result = yield* config.updateGlobal(ctx.payload) -if (result.changed) bridge.fork(disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true })) -return result.info -``` - -Imports needed: - -```ts -import { EffectBridge } from "@/effect/bridge" -import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" -``` - -Also yield a stable bridge at handler construction: - -```ts -const bridge = yield* EffectBridge.make() -``` - -Do not use `Effect.forkScoped` for this fire-and-forget disposal; the request scope can close before disposal finishes. - -`src/server/routes/instance/httpapi/handlers/global.ts` already yields `InstanceStore.Service` for `/dispose`. Keep `/dispose` strict, or use the shared helper with `swallowErrors: false` so explicit disposal failures still surface. - -### TUI worker reload - -File: `src/cli/cmd/tui/worker.ts` - -Current: - -```ts -await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.invalidate(true))) -``` - -Change to: - -```ts -await AppRuntime.runPromise( - Effect.gen(function* () { - const cfg = yield* Config.Service - const store = yield* InstanceStore.Service - yield* cfg.invalidate() - yield* store.disposeAll() - }), -) -``` - -Imports needed: - -```ts -import { Effect } from "effect" -import { InstanceStore } from "@/project/instance-store" -``` - -No global disposed event is required here unless existing TUI behavior depends on it. The current worker path only calls `Config.invalidate(true)` and does not directly interact with server event streams. - -## Helper Extraction - -If both global routes need identical "dispose all and emit global disposed" behavior, extract a helper outside `Config`. - -Preferred location: - -`src/server/global-lifecycle.ts` - -Suggested helper: - -```ts -export const emitGlobalDisposed = Effect.sync(() => - GlobalBus.emit("event", { - directory: "global", - payload: { - type: Event.Disposed.type, - properties: {}, - }, - }), -) - -export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn("Server.disposeAllInstancesAndEmitGlobalDisposed")(function* (options?: { - swallowErrors?: boolean -}) { - const store = yield* InstanceStore.Service - const dispose = store.disposeAll() - yield* (options?.swallowErrors ? dispose.pipe(Effect.catch(() => Effect.void)) : dispose) - yield* emitGlobalDisposed -}) -``` - -Use this helper only from server/global route code and explicit reload/dispose orchestration. Do not import it into `Config`. - -Use `swallowErrors: true` only for paths that previously swallowed disposal errors, such as config invalidation. Keep explicit `/dispose` strict by omitting `swallowErrors`. - -## Tests To Update - -### Config tests - -File: `test/config/config.test.ts` - -1. `save(...)`, `saveGlobal(...)`, and `clear(...)` helpers should still run against `Config.layer` only. -2. They should not need `InstanceRuntime`, `InstanceStore`, or no-op lifecycle mocks. -3. Existing config tests should continue to pass because config no longer disposes instances internally. - -### TUI config tests - -File: `test/config/tui.test.ts` - -1. The `clear` helper currently calls `Config.invalidate` through `AppRuntime`. -2. After `invalidate()` is config-only, this is fine and should not dispose instances. - -### Route behavior tests - -Add or update focused tests for lifecycle ownership: - -1. Legacy instance config route disposes only the active instance after project config update. -2. HttpApi instance config route still marks the active instance for disposal after project config update. -3. Legacy global config route disposes all instances after global config update. -4. HttpApi global config route disposes all instances after global config update. -5. Global config routes do not dispose instances when the config write is a no-op. - -Prefer existing route tests if they already cover config update behavior. Do not add broad integration tests unless necessary. - -Suggested new focused files if no existing test has the right harness: - -1. `test/server/global-config.test.ts` for legacy Hono global config update lifecycle. -2. `test/server/httpapi-global-config.test.ts` for Effect HttpApi global config update lifecycle. - -## Verification Commands - -Run from `packages/opencode`: - -```bash -bun typecheck -bun run test test/config/config.test.ts test/config/tui.test.ts -bun run test test/server/httpapi-config.test.ts test/server/httpapi-instance-context.test.ts test/server/httpapi-bridge.test.ts -bun run test test/server/global-config.test.ts test/server/httpapi-global-config.test.ts -bun run test test/project/instance-bootstrap-regression.test.ts test/agent/plugin-agent-regression.test.ts test/project/instance.test.ts -env -u OPENCODE_EXPERIMENTAL_WORKSPACES bun run test -``` - -## Non-Goals - -1. Do not remove `InstanceRuntime` entirely in this change. It is still needed for legacy Promise/ALS callers. -2. Do not change `InstanceStore` bootstrap ownership. -3. Do not change config parsing/merging semantics. -4. Do not make `Config.layer` depend on `InstanceStore.Service`. - -## Expected End State - -1. `src/config/config.ts` has no import or dynamic import of `InstanceRuntime`, `InstanceStore`, or server lifecycle helpers. -2. `Config.update` and `Config.updateGlobal` only write config and invalidate config-owned caches. -3. Instance disposal is visible at route/worker orchestration boundaries. -4. Tests that exercise config parsing/writing no longer need special lifecycle stubs.