refactor(instance-store): consolidate dispose helpers (#25424)

This commit is contained in:
Kit Langton
2026-05-02 11:21:40 -04:00
committed by GitHub
parent 31ed4602e1
commit b09b7d28b8
9 changed files with 37 additions and 22 deletions
+1 -1
View File
@@ -9,7 +9,7 @@ export async function bootstrap<T>(directory: string, cb: () => Promise<T>) {
const result = await cb() const result = await cb()
return result return result
} finally { } finally {
await InstanceStore.runtime.runPromise((s) => s.dispose(Instance.current)) await InstanceStore.disposeInstance(Instance.current)
} }
}, },
}) })
+1 -1
View File
@@ -88,7 +88,7 @@ export const rpc = {
async shutdown() { async shutdown() {
Log.Default.info("worker shutting down") Log.Default.info("worker shutting down")
await InstanceStore.runtime.runPromise((s) => s.disposeAll()) await InstanceStore.disposeAllInstances()
if (server) await server.stop(true) if (server) await server.stop(true)
}, },
} }
+6 -5
View File
@@ -13,7 +13,6 @@ import { Env } from "../env"
import { applyEdits, modify } from "jsonc-parser" import { applyEdits, modify } from "jsonc-parser"
import { type InstanceContext } from "../project/instance" import { type InstanceContext } from "../project/instance"
import { InstanceStore } from "../project/instance-store" import { InstanceStore } from "../project/instance-store"
import { InstanceRef } from "@/effect/instance-ref"
import { InstallationLocal, InstallationVersion } from "@opencode-ai/core/installation/version" import { InstallationLocal, InstallationVersion } from "@opencode-ai/core/installation/version"
import { existsSync } from "fs" import { existsSync } from "fs"
import { GlobalBus } from "@/bus/global" import { GlobalBus } from "@/bus/global"
@@ -739,15 +738,17 @@ export const layer = Layer.effect(
.writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2)) .writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2))
.pipe(Effect.orDie) .pipe(Effect.orDie)
if (options?.dispose !== false) { if (options?.dispose !== false) {
const ctx = yield* InstanceRef // Fail loudly if no instance is bound — silently skipping would
if (ctx) yield* Effect.promise(() => InstanceStore.runtime.runPromise((s) => s.dispose(ctx))) // 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))
} }
}) })
const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) { const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) {
yield* invalidateGlobal yield* invalidateGlobal
const task = InstanceStore.runtime const task = InstanceStore.disposeAllInstances()
.runPromise((s) => s.disposeAll())
.catch(() => undefined) .catch(() => undefined)
.finally(() => .finally(() =>
GlobalBus.emit("event", { GlobalBus.emit("event", {
@@ -1,7 +1,7 @@
import { GlobalBus } from "@/bus/global" import { GlobalBus } from "@/bus/global"
import { WorkspaceContext } from "@/control-plane/workspace-context" import { WorkspaceContext } from "@/control-plane/workspace-context"
import { InstanceRef } from "@/effect/instance-ref" import { InstanceRef } from "@/effect/instance-ref"
import { disposeInstance } from "@/effect/instance-registry" import { disposeInstance as runDisposers } from "@/effect/instance-registry"
import { makeRuntime } from "@/effect/run-service" import { makeRuntime } from "@/effect/run-service"
import { AppFileSystem } from "@opencode-ai/core/filesystem" import { AppFileSystem } from "@opencode-ai/core/filesystem"
import { Context, Deferred, Duration, Effect, Exit, Layer, Scope } from "effect" 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(
const disposeContext = Effect.fn("InstanceStore.disposeContext")(function* (ctx: InstanceContext) { const disposeContext = Effect.fn("InstanceStore.disposeContext")(function* (ctx: InstanceContext) {
yield* Effect.logInfo("disposing instance", { directory: ctx.directory }) yield* Effect.logInfo("disposing instance", { directory: ctx.directory })
yield* Effect.promise(() => disposeInstance(ctx.directory)) yield* Effect.promise(() => runDisposers(ctx.directory))
yield* emitDisposed({ directory: ctx.directory, project: ctx.project.id }) yield* emitDisposed({ directory: ctx.directory, project: ctx.project.id })
}) })
@@ -135,7 +135,7 @@ export const layer: Layer.Layer<Service, never, Project.Service> = Layer.effect(
yield* Effect.logInfo("reloading instance", { directory }) yield* Effect.logInfo("reloading instance", { directory })
if (previous) { if (previous) {
yield* Deferred.await(previous.deferred).pipe(Effect.ignore) yield* Deferred.await(previous.deferred).pipe(Effect.ignore)
yield* Effect.promise(() => disposeInstance(directory)) yield* Effect.promise(() => runDisposers(directory))
yield* emitDisposed({ directory, project: input.project?.id }) yield* emitDisposed({ directory, project: input.project?.id })
} }
yield* completeLoad(directory, input, entry) yield* completeLoad(directory, input, entry)
@@ -197,4 +197,11 @@ export const defaultLayer = layer.pipe(Layer.provide(Project.defaultLayer))
export const runtime = makeRuntime(Service, 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" export * as InstanceStore from "./instance-store"
+9 -2
View File
@@ -42,10 +42,17 @@ export const Instance = {
restore<R>(ctx: InstanceContext, fn: () => R): R { restore<R>(ctx: InstanceContext, fn: () => R): R {
return context.provide(ctx, fn) return context.provide(ctx, fn)
}, },
// followup: `reload` survives because `test/server/project-init-git.test.ts`
// spies on this exact method. Once that test asserts on `InstanceStore.reloadInstance`
// (or moves to an Effect runtime), this wrapper can drop.
async reload(input: InstanceStore.LoadInput) { async reload(input: InstanceStore.LoadInput) {
return InstanceStore.runtime.runPromise((store) => store.reload(input)) return InstanceStore.reloadInstance(input)
}, },
// followup: `dispose` survives for legacy fixtures that read `Instance.current`
// out of ALS (e.g. `test/fixture/fixture.ts` `provideTmpdirInstance`,
// `test/question/question.test.ts` cancellation tests). Convert those to call
// `InstanceStore.disposeInstance(ctx)` directly once `Instance.provide` is gone.
async dispose() { async dispose() {
return InstanceStore.runtime.runPromise((store) => store.dispose(Instance.current)) return InstanceStore.disposeInstance(Instance.current)
}, },
} }
@@ -200,7 +200,7 @@ export const GlobalRoutes = lazy(() =>
}, },
}), }),
async (c) => { async (c) => {
await InstanceStore.runtime.runPromise((s) => s.disposeAll()) await InstanceStore.disposeAllInstances()
GlobalBus.emit("event", { GlobalBus.emit("event", {
directory: "global", directory: "global",
payload: { payload: {
@@ -63,7 +63,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => {
}, },
}), }),
async (c) => { async (c) => {
await InstanceStore.runtime.runPromise((s) => s.dispose(Instance.current)) await InstanceStore.disposeInstance(Instance.current)
return c.json(true) return c.json(true)
}, },
) )
@@ -81,7 +81,11 @@ export const ProjectRoutes = lazy(() =>
Project.Service.use((svc) => svc.initGit({ directory: dir, project: prev })), 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) if (next.id === prev.id && next.vcs === prev.vcs && next.worktree === prev.worktree) return c.json(next)
await Instance.reload({ directory: dir, worktree: dir, project: next }) await Instance.reload({
directory: dir,
worktree: dir,
project: next,
})
return c.json(next) return c.json(next)
}, },
) )
+3 -7
View File
@@ -9,15 +9,11 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
import type { Config } from "@/config/config" import type { Config } from "@/config/config"
import { InstanceRef } from "../../src/effect/instance-ref" import { InstanceRef } from "../../src/effect/instance-ref"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { TestLLMServer } from "../lib/llm-server" import { TestLLMServer } from "../lib/llm-server"
// Test helper for tearing down all loaded instances. Used in afterEach hooks. // Re-export for test ergonomics. The implementation lives next to the runtime
// Replaces direct Instance.disposeAll() calls so the legacy promise method can be removed. // it consumes; see `InstanceStore.disposeAllInstances` for the rationale.
// IMPORTANT: must use InstanceStore.runtime, not AppRuntime or a test-layer Service — export { disposeAllInstances } from "../../src/project/instance-store"
// Instance.provide loads instances into InstanceStore.runtime's Service cache, and that
// Service is built per-runtime (not shared via memoMap across Effect.runPromise boundaries).
export const disposeAllInstances = () => InstanceStore.runtime.runPromise((s) => s.disposeAll())
// Strip null bytes from paths (defensive fix for CI environment issues) // Strip null bytes from paths (defensive fix for CI environment issues)
function sanitizePath(p: string): string { function sanitizePath(p: string): string {