Delete Instance.dispose and Instance.reload (#25427)

This commit is contained in:
Kit Langton
2026-05-02 11:44:16 -04:00
committed by GitHub
parent 7371db5cc6
commit 3b9155714d
11 changed files with 30 additions and 47 deletions
-13
View File
@@ -42,17 +42,4 @@ 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) {
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() {
return InstanceStore.disposeInstance(Instance.current)
},
} }
@@ -2,6 +2,7 @@ import { Hono } from "hono"
import { describeRoute, validator } from "hono-openapi" import { describeRoute, validator } from "hono-openapi"
import { resolver } from "hono-openapi" import { resolver } from "hono-openapi"
import { Instance } from "@/project/instance" import { Instance } from "@/project/instance"
import { InstanceStore } from "@/project/instance-store"
import { Project } from "@/project/project" import { Project } from "@/project/project"
import z from "zod" import z from "zod"
import { ProjectID } from "@/project/schema" import { ProjectID } from "@/project/schema"
@@ -81,11 +82,7 @@ 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({ await InstanceStore.reloadInstance({ directory: dir, worktree: dir, project: next })
directory: dir,
worktree: dir,
project: next,
})
return c.json(next) return c.json(next)
}, },
) )
@@ -3,6 +3,7 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
import { $ } from "bun" import { $ } from "bun"
import { Context, Deferred, Duration, Effect, Exit, Fiber, Layer } from "effect" import { Context, Deferred, Duration, Effect, Exit, Fiber, Layer } from "effect"
import { InstanceState } from "@/effect/instance-state" import { InstanceState } from "@/effect/instance-state"
import { InstanceStore } from "../../src/project/instance-store"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { disposeAllInstances, provideInstance, tmpdirScoped } from "../fixture/fixture" import { disposeAllInstances, provideInstance, tmpdirScoped } from "../fixture/fixture"
import { testEffect } from "../lib/effect" import { testEffect } from "../lib/effect"
@@ -69,7 +70,7 @@ it.live("InstanceState invalidates on reload", () =>
) )
const a = yield* access(state, dir) const a = yield* access(state, dir)
yield* Effect.promise(() => Instance.reload({ directory: dir })) yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir }))
const b = yield* access(state, dir) const b = yield* access(state, dir)
expect(a).not.toBe(b) expect(a).not.toBe(b)
@@ -269,7 +270,7 @@ it.live("InstanceState correct after interleaved init and dispose", () =>
const [, b] = yield* Effect.all( const [, b] = yield* Effect.all(
[ [
Effect.promise(() => Instance.reload({ directory: one })), Effect.promise(() => InstanceStore.reloadInstance({ directory: one })),
Test.use((svc) => svc.get()).pipe(provideInstance(two)), Test.use((svc) => svc.get()).pipe(provideInstance(two)),
], ],
{ concurrency: "unbounded" }, { concurrency: "unbounded" },
+2 -1
View File
@@ -8,6 +8,7 @@ import type * as Scope from "effect/Scope"
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" 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 { InstanceStore } from "../../src/project/instance-store"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { TestLLMServer } from "../lib/llm-server" import { TestLLMServer } from "../lib/llm-server"
@@ -149,7 +150,7 @@ export function provideTmpdirInstance<A, E, R>(
? Effect.promise(() => ? Effect.promise(() =>
Instance.provide({ Instance.provide({
directory: path, directory: path,
fn: () => Instance.dispose(), fn: () => InstanceStore.disposeInstance(Instance.current),
}), }),
).pipe(Effect.ignore) ).pipe(Effect.ignore)
: Effect.void, : Effect.void,
+2 -1
View File
@@ -1,4 +1,5 @@
import { test, expect, mock, beforeEach } from "bun:test" import { test, expect, mock, beforeEach } from "bun:test"
import { InstanceStore } from "../../src/project/instance-store"
import { Effect } from "effect" import { Effect } from "effect"
import type { MCP as MCPNS } from "../../src/mcp/index" import type { MCP as MCPNS } from "../../src/mcp/index"
@@ -197,7 +198,7 @@ function withInstance(
fn: async () => { fn: async () => {
await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer))) await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer)))
// dispose instance to clean up state between tests // dispose instance to clean up state between tests
await Instance.dispose() await InstanceStore.disposeInstance(Instance.current)
}, },
}) })
} }
@@ -6,6 +6,7 @@ import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
import { Permission } from "../../src/permission" import { Permission } from "../../src/permission"
import { PermissionID } from "../../src/permission/schema" import { PermissionID } from "../../src/permission/schema"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { disposeAllInstances, provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture" import { disposeAllInstances, provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture"
import { testEffect } from "../lib/effect" import { testEffect } from "../lib/effect"
import { MessageID, SessionID } from "../../src/session/schema" import { MessageID, SessionID } from "../../src/session/schema"
@@ -998,7 +999,7 @@ it.live("pending permission rejects on instance dispose", () =>
}).pipe(run, Effect.forkScoped) }).pipe(run, Effect.forkScoped)
expect(yield* waitForPending(1).pipe(run)).toHaveLength(1) expect(yield* waitForPending(1).pipe(run)).toHaveLength(1)
yield* Effect.promise(() => Instance.provide({ directory: dir, fn: () => void Instance.dispose() })) yield* Effect.promise(() => Instance.provide({ directory: dir, fn: () => void InstanceStore.disposeInstance(Instance.current) }))
const exit = yield* Fiber.await(fiber) const exit = yield* Fiber.await(fiber)
expect(Exit.isFailure(exit)).toBe(true) expect(Exit.isFailure(exit)).toBe(true)
@@ -1021,7 +1022,7 @@ it.live("pending permission rejects on instance reload", () =>
}).pipe(run, Effect.forkScoped) }).pipe(run, Effect.forkScoped)
expect(yield* waitForPending(1).pipe(run)).toHaveLength(1) expect(yield* waitForPending(1).pipe(run)).toHaveLength(1)
yield* Effect.promise(() => Instance.reload({ directory: dir })) yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir }))
const exit = yield* Fiber.await(fiber) const exit = yield* Fiber.await(fiber)
expect(Exit.isFailure(exit)).toBe(true) expect(Exit.isFailure(exit)).toBe(true)
@@ -1115,7 +1116,7 @@ it.live("ask - abort should clear pending request", () =>
const pending = yield* waitForPending(1).pipe(run) const pending = yield* waitForPending(1).pipe(run)
expect(pending).toHaveLength(1) expect(pending).toHaveLength(1)
yield* Effect.promise(() => Instance.reload({ directory: dir })) yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir }))
const exit = yield* Fiber.await(fiber) const exit = yield* Fiber.await(fiber)
expect(Exit.isFailure(exit)).toBe(true) expect(Exit.isFailure(exit)).toBe(true)
@@ -5,6 +5,7 @@ import path from "path"
import { Cause, Effect, Exit, Layer } from "effect" import { Cause, Effect, Exit, Layer } from "effect"
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { Worktree } from "../../src/worktree" import { Worktree } from "../../src/worktree"
import { disposeAllInstances, provideInstance, provideTmpdirInstance } from "../fixture/fixture" import { disposeAllInstances, provideInstance, provideTmpdirInstance } from "../fixture/fixture"
import { testEffect } from "../lib/effect" import { testEffect } from "../lib/effect"
@@ -136,7 +137,7 @@ describe("Worktree", () => {
expect(props.name).toBe(info.name) expect(props.name).toBe(info.name)
expect(props.branch).toBe(info.branch) expect(props.branch).toBe(info.branch)
yield* Effect.promise(() => Instance.dispose()).pipe(provideInstance(info.directory)) yield* Effect.promise(() => InstanceStore.runtime.runPromise((s) => s.load({ directory: info.directory }).pipe(Effect.flatMap(s.dispose))))
yield* Effect.promise(() => Bun.sleep(100)) yield* Effect.promise(() => Bun.sleep(100))
yield* svc.remove({ directory: info.directory }) yield* svc.remove({ directory: info.directory })
}), }),
@@ -156,7 +157,7 @@ describe("Worktree", () => {
expect(info.branch).toBe("opencode/test-workspace") expect(info.branch).toBe("opencode/test-workspace")
yield* Effect.promise(() => ready) yield* Effect.promise(() => ready)
yield* Effect.promise(() => Instance.dispose()).pipe(provideInstance(info.directory)) yield* Effect.promise(() => InstanceStore.runtime.runPromise((s) => s.load({ directory: info.directory }).pipe(Effect.flatMap(s.dispose))))
yield* Effect.promise(() => Bun.sleep(100)) yield* Effect.promise(() => Bun.sleep(100))
yield* svc.remove({ directory: info.directory }) yield* svc.remove({ directory: info.directory })
}), }),
@@ -1,6 +1,7 @@
import { afterEach, test, expect } from "bun:test" import { afterEach, test, expect } from "bun:test"
import { Question } from "../../src/question" import { Question } from "../../src/question"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { QuestionID } from "../../src/question/schema" import { QuestionID } from "../../src/question/schema"
import { disposeAllInstances, tmpdir } from "../fixture/fixture" import { disposeAllInstances, tmpdir } from "../fixture/fixture"
import { SessionID } from "../../src/session/schema" import { SessionID } from "../../src/session/schema"
@@ -421,7 +422,7 @@ test("pending question rejects on instance dispose", async () => {
fn: async () => { fn: async () => {
const items = await list() const items = await list()
expect(items).toHaveLength(1) expect(items).toHaveLength(1)
await Instance.dispose() await InstanceStore.disposeInstance(Instance.current)
}, },
}) })
@@ -456,7 +457,7 @@ test("pending question rejects on instance reload", async () => {
fn: async () => { fn: async () => {
const items = await list() const items = await list()
expect(items).toHaveLength(1) expect(items).toHaveLength(1)
await Instance.reload({ directory: tmp.path }) await InstanceStore.reloadInstance({ directory: tmp.path })
}, },
}) })
@@ -5,6 +5,7 @@ import { Flag } from "@opencode-ai/core/flag/flag"
import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server" import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server"
import { McpPaths } from "../../src/server/routes/instance/httpapi/groups/mcp" import { McpPaths } from "../../src/server/routes/instance/httpapi/groups/mcp"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { Server } from "../../src/server/server" import { Server } from "../../src/server/server"
import * as Log from "@opencode-ai/core/util/log" import * as Log from "@opencode-ai/core/util/log"
import { resetDatabase } from "../fixture/db" import { resetDatabase } from "../fixture/db"
@@ -57,7 +58,7 @@ function withMcpProject<A, E, R>(self: (dir: string) => Effect.Effect<A, E, R>)
}), }),
) )
yield* Effect.addFinalizer(() => yield* Effect.addFinalizer(() =>
Effect.promise(() => Instance.provide({ directory: dir, fn: () => Instance.dispose() })).pipe(Effect.ignore), Effect.promise(() => Instance.provide({ directory: dir, fn: () => InstanceStore.disposeInstance(Instance.current) })).pipe(Effect.ignore),
) )
return yield* self(dir).pipe(provideInstance(dir)) return yield* self(dir).pipe(provideInstance(dir))
@@ -3,6 +3,7 @@ import { Effect, FileSystem, Layer, Path } from "effect"
import { NodeFileSystem, NodePath } from "@effect/platform-node" import { NodeFileSystem, NodePath } from "@effect/platform-node"
import { Flag } from "@opencode-ai/core/flag/flag" import { Flag } from "@opencode-ai/core/flag/flag"
import { Instance } from "../../src/project/instance" import { Instance } from "../../src/project/instance"
import { InstanceStore } from "../../src/project/instance-store"
import { Server } from "../../src/server/server" import { Server } from "../../src/server/server"
import * as Log from "@opencode-ai/core/util/log" import * as Log from "@opencode-ai/core/util/log"
import { resetDatabase } from "../fixture/db" import { resetDatabase } from "../fixture/db"
@@ -89,7 +90,7 @@ function withProviderProject<A, E, R>(self: (dir: string) => Effect.Effect<A, E,
) )
yield* writeProviderAuthPlugin(dir) yield* writeProviderAuthPlugin(dir)
yield* Effect.addFinalizer(() => yield* Effect.addFinalizer(() =>
Effect.promise(() => Instance.provide({ directory: dir, fn: () => Instance.dispose() })).pipe(Effect.ignore), Effect.promise(() => Instance.provide({ directory: dir, fn: () => InstanceStore.disposeInstance(Instance.current) })).pipe(Effect.ignore),
) )
return yield* self(dir).pipe(provideInstance(dir)) return yield* self(dir).pipe(provideInstance(dir))
@@ -1,9 +1,8 @@
import { afterEach, describe, expect, spyOn, test } from "bun:test" import { afterEach, describe, expect, test } from "bun:test"
import { Effect } from "effect" import { Effect } from "effect"
import path from "path" import path from "path"
import { GlobalBus } from "../../src/bus/global" import { GlobalBus } from "../../src/bus/global"
import { Snapshot } from "../../src/snapshot" import { Snapshot } from "../../src/snapshot"
import { Instance } from "../../src/project/instance"
import { Server } from "../../src/server/server" import { Server } from "../../src/server/server"
import { Filesystem } from "@/util/filesystem" import { Filesystem } from "@/util/filesystem"
import * as Log from "@opencode-ai/core/util/log" import * as Log from "@opencode-ai/core/util/log"
@@ -16,6 +15,9 @@ afterEach(async () => {
await resetDatabase() await resetDatabase()
}) })
const disposedEvents = (seen: { directory?: string; payload: { type: string } }[], dir: string) =>
seen.filter((evt) => evt.directory === dir && evt.payload.type === "server.instance.disposed").length
describe("project.initGit endpoint", () => { describe("project.initGit endpoint", () => {
test("initializes git and reloads immediately", async () => { test("initializes git and reloads immediately", async () => {
await using tmp = await tmpdir() await using tmp = await tmpdir()
@@ -24,8 +26,6 @@ describe("project.initGit endpoint", () => {
const fn = (evt: { directory?: string; payload: { type: string } }) => { const fn = (evt: { directory?: string; payload: { type: string } }) => {
seen.push(evt) seen.push(evt)
} }
const reload = Instance.reload
const reloadSpy = spyOn(Instance, "reload").mockImplementation((input) => reload(input))
GlobalBus.on("event", fn) GlobalBus.on("event", fn)
try { try {
@@ -42,10 +42,8 @@ describe("project.initGit endpoint", () => {
vcs: "git", vcs: "git",
worktree: tmp.path, worktree: tmp.path,
}) })
expect(reloadSpy).toHaveBeenCalledTimes(1) // Reload behavior: bus emits exactly one server.instance.disposed for the directory.
expect(seen.some((evt) => evt.directory === tmp.path && evt.payload.type === "server.instance.disposed")).toBe( expect(disposedEvents(seen, tmp.path)).toBe(1)
true,
)
expect(await Filesystem.exists(path.join(tmp.path, ".git", "opencode"))).toBe(false) expect(await Filesystem.exists(path.join(tmp.path, ".git", "opencode"))).toBe(false)
const current = await app.request("/project/current", { const current = await app.request("/project/current", {
@@ -70,7 +68,6 @@ describe("project.initGit endpoint", () => {
).toBeTruthy() ).toBeTruthy()
} finally { } finally {
await disposeAllInstances() await disposeAllInstances()
reloadSpy.mockRestore()
GlobalBus.off("event", fn) GlobalBus.off("event", fn)
} }
}) })
@@ -82,8 +79,6 @@ describe("project.initGit endpoint", () => {
const fn = (evt: { directory?: string; payload: { type: string } }) => { const fn = (evt: { directory?: string; payload: { type: string } }) => {
seen.push(evt) seen.push(evt)
} }
const reload = Instance.reload
const reloadSpy = spyOn(Instance, "reload").mockImplementation((input) => reload(input))
GlobalBus.on("event", fn) GlobalBus.on("event", fn)
try { try {
@@ -98,10 +93,7 @@ describe("project.initGit endpoint", () => {
vcs: "git", vcs: "git",
worktree: tmp.path, worktree: tmp.path,
}) })
expect( expect(disposedEvents(seen, tmp.path)).toBe(0)
seen.filter((evt) => evt.directory === tmp.path && evt.payload.type === "server.instance.disposed").length,
).toBe(0)
expect(reloadSpy).toHaveBeenCalledTimes(0)
const current = await app.request("/project/current", { const current = await app.request("/project/current", {
headers: { headers: {
@@ -115,7 +107,6 @@ describe("project.initGit endpoint", () => {
}) })
} finally { } finally {
await disposeAllInstances() await disposeAllInstances()
reloadSpy.mockRestore()
GlobalBus.off("event", fn) GlobalBus.off("event", fn)
} }
}) })