Skip to content

Commit b09b7d2

Browse files
authored
refactor(instance-store): consolidate dispose helpers (#25424)
1 parent 31ed460 commit b09b7d2

9 files changed

Lines changed: 37 additions & 22 deletions

File tree

packages/opencode/src/cli/bootstrap.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export async function bootstrap<T>(directory: string, cb: () => Promise<T>) {
99
const result = await cb()
1010
return result
1111
} finally {
12-
await InstanceStore.runtime.runPromise((s) => s.dispose(Instance.current))
12+
await InstanceStore.disposeInstance(Instance.current)
1313
}
1414
},
1515
})

packages/opencode/src/cli/cmd/tui/worker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export const rpc = {
8888
async shutdown() {
8989
Log.Default.info("worker shutting down")
9090

91-
await InstanceStore.runtime.runPromise((s) => s.disposeAll())
91+
await InstanceStore.disposeAllInstances()
9292
if (server) await server.stop(true)
9393
},
9494
}

packages/opencode/src/config/config.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { Env } from "../env"
1313
import { applyEdits, modify } from "jsonc-parser"
1414
import { type InstanceContext } from "../project/instance"
1515
import { InstanceStore } from "../project/instance-store"
16-
import { InstanceRef } from "@/effect/instance-ref"
1716
import { InstallationLocal, InstallationVersion } from "@opencode-ai/core/installation/version"
1817
import { existsSync } from "fs"
1918
import { GlobalBus } from "@/bus/global"
@@ -739,15 +738,17 @@ export const layer = Layer.effect(
739738
.writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2))
740739
.pipe(Effect.orDie)
741740
if (options?.dispose !== false) {
742-
const ctx = yield* InstanceRef
743-
if (ctx) yield* Effect.promise(() => InstanceStore.runtime.runPromise((s) => s.dispose(ctx)))
741+
// Fail loudly if no instance is bound — silently skipping would
742+
// mask "config update without an active instance" bugs. The throw
743+
// comes from `Instance.current` inside `InstanceState.context`.
744+
const ctx = yield* InstanceState.context
745+
yield* Effect.promise(() => InstanceStore.disposeInstance(ctx))
744746
}
745747
})
746748

747749
const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) {
748750
yield* invalidateGlobal
749-
const task = InstanceStore.runtime
750-
.runPromise((s) => s.disposeAll())
751+
const task = InstanceStore.disposeAllInstances()
751752
.catch(() => undefined)
752753
.finally(() =>
753754
GlobalBus.emit("event", {

packages/opencode/src/project/instance-store.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { GlobalBus } from "@/bus/global"
22
import { WorkspaceContext } from "@/control-plane/workspace-context"
33
import { InstanceRef } from "@/effect/instance-ref"
4-
import { disposeInstance } from "@/effect/instance-registry"
4+
import { disposeInstance as runDisposers } from "@/effect/instance-registry"
55
import { makeRuntime } from "@/effect/run-service"
66
import { AppFileSystem } from "@opencode-ai/core/filesystem"
77
import { Context, Deferred, Duration, Effect, Exit, Layer, Scope } from "effect"
@@ -94,7 +94,7 @@ export const layer: Layer.Layer<Service, never, Project.Service> = Layer.effect(
9494

9595
const disposeContext = Effect.fn("InstanceStore.disposeContext")(function* (ctx: InstanceContext) {
9696
yield* Effect.logInfo("disposing instance", { directory: ctx.directory })
97-
yield* Effect.promise(() => disposeInstance(ctx.directory))
97+
yield* Effect.promise(() => runDisposers(ctx.directory))
9898
yield* emitDisposed({ directory: ctx.directory, project: ctx.project.id })
9999
})
100100

@@ -135,7 +135,7 @@ export const layer: Layer.Layer<Service, never, Project.Service> = Layer.effect(
135135
yield* Effect.logInfo("reloading instance", { directory })
136136
if (previous) {
137137
yield* Deferred.await(previous.deferred).pipe(Effect.ignore)
138-
yield* Effect.promise(() => disposeInstance(directory))
138+
yield* Effect.promise(() => runDisposers(directory))
139139
yield* emitDisposed({ directory, project: input.project?.id })
140140
}
141141
yield* completeLoad(directory, input, entry)
@@ -197,4 +197,11 @@ export const defaultLayer = layer.pipe(Layer.provide(Project.defaultLayer))
197197

198198
export const runtime = makeRuntime(Service, defaultLayer)
199199

200+
// Promise-returning helpers for callers without an Effect runtime in scope.
201+
// They route through `runtime` (not a yielded Service from a fresh runtime)
202+
// so they share the cache that `Instance.provide` populates.
203+
export const disposeInstance = (ctx: InstanceContext) => runtime.runPromise((store) => store.dispose(ctx))
204+
export const disposeAllInstances = () => runtime.runPromise((store) => store.disposeAll())
205+
export const reloadInstance = (input: LoadInput) => runtime.runPromise((store) => store.reload(input))
206+
200207
export * as InstanceStore from "./instance-store"

packages/opencode/src/project/instance.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,17 @@ export const Instance = {
4242
restore<R>(ctx: InstanceContext, fn: () => R): R {
4343
return context.provide(ctx, fn)
4444
},
45+
// followup: `reload` survives because `test/server/project-init-git.test.ts`
46+
// spies on this exact method. Once that test asserts on `InstanceStore.reloadInstance`
47+
// (or moves to an Effect runtime), this wrapper can drop.
4548
async reload(input: InstanceStore.LoadInput) {
46-
return InstanceStore.runtime.runPromise((store) => store.reload(input))
49+
return InstanceStore.reloadInstance(input)
4750
},
51+
// followup: `dispose` survives for legacy fixtures that read `Instance.current`
52+
// out of ALS (e.g. `test/fixture/fixture.ts` `provideTmpdirInstance`,
53+
// `test/question/question.test.ts` cancellation tests). Convert those to call
54+
// `InstanceStore.disposeInstance(ctx)` directly once `Instance.provide` is gone.
4855
async dispose() {
49-
return InstanceStore.runtime.runPromise((store) => store.dispose(Instance.current))
56+
return InstanceStore.disposeInstance(Instance.current)
5057
},
5158
}

packages/opencode/src/server/routes/global.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ export const GlobalRoutes = lazy(() =>
200200
},
201201
}),
202202
async (c) => {
203-
await InstanceStore.runtime.runPromise((s) => s.disposeAll())
203+
await InstanceStore.disposeAllInstances()
204204
GlobalBus.emit("event", {
205205
directory: "global",
206206
payload: {

packages/opencode/src/server/routes/instance/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => {
6363
},
6464
}),
6565
async (c) => {
66-
await InstanceStore.runtime.runPromise((s) => s.dispose(Instance.current))
66+
await InstanceStore.disposeInstance(Instance.current)
6767
return c.json(true)
6868
},
6969
)

packages/opencode/src/server/routes/instance/project.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ export const ProjectRoutes = lazy(() =>
8181
Project.Service.use((svc) => svc.initGit({ directory: dir, project: prev })),
8282
)
8383
if (next.id === prev.id && next.vcs === prev.vcs && next.worktree === prev.worktree) return c.json(next)
84-
await Instance.reload({ directory: dir, worktree: dir, project: next })
84+
await Instance.reload({
85+
directory: dir,
86+
worktree: dir,
87+
project: next,
88+
})
8589
return c.json(next)
8690
},
8791
)

packages/opencode/test/fixture/fixture.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,11 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
99
import type { Config } from "@/config/config"
1010
import { InstanceRef } from "../../src/effect/instance-ref"
1111
import { Instance } from "../../src/project/instance"
12-
import { InstanceStore } from "../../src/project/instance-store"
1312
import { TestLLMServer } from "../lib/llm-server"
1413

15-
// Test helper for tearing down all loaded instances. Used in afterEach hooks.
16-
// Replaces direct Instance.disposeAll() calls so the legacy promise method can be removed.
17-
// IMPORTANT: must use InstanceStore.runtime, not AppRuntime or a test-layer Service —
18-
// Instance.provide loads instances into InstanceStore.runtime's Service cache, and that
19-
// Service is built per-runtime (not shared via memoMap across Effect.runPromise boundaries).
20-
export const disposeAllInstances = () => InstanceStore.runtime.runPromise((s) => s.disposeAll())
14+
// Re-export for test ergonomics. The implementation lives next to the runtime
15+
// it consumes; see `InstanceStore.disposeAllInstances` for the rationale.
16+
export { disposeAllInstances } from "../../src/project/instance-store"
2117

2218
// Strip null bytes from paths (defensive fix for CI environment issues)
2319
function sanitizePath(p: string): string {

0 commit comments

Comments
 (0)