From 77bffaaf8a5c66cbf70389587129f3739d9e5b05 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 2 May 2026 11:31:03 -0400 Subject: [PATCH] refactor: delete Instance.dispose and Instance.reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converts the remaining ~12 callers from the legacy Instance.{dispose,reload} wrappers to the InstanceStore.{disposeInstance,reloadInstance} helpers added in #25424, then deletes the wrappers. project-init-git.test.ts drops its spyOn(Instance, 'reload') in favor of asserting on the bus event count (disposedEvents helper) — the bus event is the actual behavioral signal the test cares about, the spy was implementation detail. After this: - src/project/instance.ts has 4 methods (provide, current/directory/worktree/project getters, bind, restore) - The spy-induced flake on project-init-git tests goes away (verified locally) Tests: 2336/2337 pass (the 1 failure is the pre-existing session.created flake on dev). --- packages/opencode/src/project/instance.ts | 13 ----------- .../src/server/routes/instance/project.ts | 7 ++---- .../test/effect/instance-state.test.ts | 5 ++-- packages/opencode/test/fixture/fixture.ts | 3 ++- packages/opencode/test/mcp/lifecycle.test.ts | 3 ++- .../opencode/test/permission/next.test.ts | 7 +++--- .../opencode/test/project/worktree.test.ts | 5 ++-- .../opencode/test/question/question.test.ts | 5 ++-- .../opencode/test/server/httpapi-mcp.test.ts | 3 ++- .../test/server/httpapi-provider.test.ts | 3 ++- .../test/server/project-init-git.test.ts | 23 ++++++------------- 11 files changed, 30 insertions(+), 47 deletions(-) diff --git a/packages/opencode/src/project/instance.ts b/packages/opencode/src/project/instance.ts index 22c2779ce1b6..5b2bcf6b32e7 100644 --- a/packages/opencode/src/project/instance.ts +++ b/packages/opencode/src/project/instance.ts @@ -42,17 +42,4 @@ export const Instance = { restore(ctx: InstanceContext, fn: () => R): R { 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) - }, } diff --git a/packages/opencode/src/server/routes/instance/project.ts b/packages/opencode/src/server/routes/instance/project.ts index 14c8c87b0955..04cc432d08ad 100644 --- a/packages/opencode/src/server/routes/instance/project.ts +++ b/packages/opencode/src/server/routes/instance/project.ts @@ -2,6 +2,7 @@ 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 { Project } from "@/project/project" import z from "zod" import { ProjectID } from "@/project/schema" @@ -81,11 +82,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 Instance.reload({ - directory: dir, - worktree: dir, - project: next, - }) + await InstanceStore.reloadInstance({ directory: dir, worktree: dir, project: next }) return c.json(next) }, ) diff --git a/packages/opencode/test/effect/instance-state.test.ts b/packages/opencode/test/effect/instance-state.test.ts index 02945ac53ff9..0a8972ca4a68 100644 --- a/packages/opencode/test/effect/instance-state.test.ts +++ b/packages/opencode/test/effect/instance-state.test.ts @@ -3,6 +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 { Instance } from "../../src/project/instance" import { disposeAllInstances, provideInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" @@ -69,7 +70,7 @@ it.live("InstanceState invalidates on reload", () => ) 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) expect(a).not.toBe(b) @@ -269,7 +270,7 @@ it.live("InstanceState correct after interleaved init and dispose", () => 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)), ], { concurrency: "unbounded" }, diff --git a/packages/opencode/test/fixture/fixture.ts b/packages/opencode/test/fixture/fixture.ts index 23dd61d8803c..1b193e382ab7 100644 --- a/packages/opencode/test/fixture/fixture.ts +++ b/packages/opencode/test/fixture/fixture.ts @@ -8,6 +8,7 @@ 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 { Instance } from "../../src/project/instance" import { TestLLMServer } from "../lib/llm-server" @@ -149,7 +150,7 @@ export function provideTmpdirInstance( ? Effect.promise(() => Instance.provide({ directory: path, - fn: () => Instance.dispose(), + fn: () => InstanceStore.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 1b459481f30f..59fa54ceab0f 100644 --- a/packages/opencode/test/mcp/lifecycle.test.ts +++ b/packages/opencode/test/mcp/lifecycle.test.ts @@ -1,4 +1,5 @@ import { test, expect, mock, beforeEach } from "bun:test" +import { InstanceStore } from "../../src/project/instance-store" import { Effect } from "effect" import type { MCP as MCPNS } from "../../src/mcp/index" @@ -197,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 Instance.dispose() + await InstanceStore.disposeInstance(Instance.current) }, }) } diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 850ad2dedd27..5a0c4740219b 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -6,6 +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 { disposeAllInstances, provideInstance, provideTmpdirInstance, tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" import { MessageID, SessionID } from "../../src/session/schema" @@ -998,7 +999,7 @@ it.live("pending permission rejects on instance dispose", () => }).pipe(run, Effect.forkScoped) 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) expect(Exit.isFailure(exit)).toBe(true) @@ -1021,7 +1022,7 @@ it.live("pending permission rejects on instance reload", () => }).pipe(run, Effect.forkScoped) 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) 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) expect(pending).toHaveLength(1) - yield* Effect.promise(() => Instance.reload({ directory: dir })) + yield* Effect.promise(() => InstanceStore.reloadInstance({ directory: dir })) const exit = yield* Fiber.await(fiber) expect(Exit.isFailure(exit)).toBe(true) diff --git a/packages/opencode/test/project/worktree.test.ts b/packages/opencode/test/project/worktree.test.ts index fac82fad34b1..3593c3ba0005 100644 --- a/packages/opencode/test/project/worktree.test.ts +++ b/packages/opencode/test/project/worktree.test.ts @@ -5,6 +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 { Worktree } from "../../src/worktree" import { disposeAllInstances, provideInstance, provideTmpdirInstance } from "../fixture/fixture" import { testEffect } from "../lib/effect" @@ -136,7 +137,7 @@ describe("Worktree", () => { expect(props.name).toBe(info.name) 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* svc.remove({ directory: info.directory }) }), @@ -156,7 +157,7 @@ describe("Worktree", () => { expect(info.branch).toBe("opencode/test-workspace") 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* svc.remove({ directory: info.directory }) }), diff --git a/packages/opencode/test/question/question.test.ts b/packages/opencode/test/question/question.test.ts index 14cf1aefa6ce..83968a6f8c1a 100644 --- a/packages/opencode/test/question/question.test.ts +++ b/packages/opencode/test/question/question.test.ts @@ -1,6 +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 { QuestionID } from "../../src/question/schema" import { disposeAllInstances, tmpdir } from "../fixture/fixture" import { SessionID } from "../../src/session/schema" @@ -421,7 +422,7 @@ test("pending question rejects on instance dispose", async () => { fn: async () => { const items = await list() 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 () => { const items = await list() expect(items).toHaveLength(1) - await Instance.reload({ directory: tmp.path }) + await InstanceStore.reloadInstance({ directory: tmp.path }) }, }) diff --git a/packages/opencode/test/server/httpapi-mcp.test.ts b/packages/opencode/test/server/httpapi-mcp.test.ts index d81d749f1d6b..32f6343ad82d 100644 --- a/packages/opencode/test/server/httpapi-mcp.test.ts +++ b/packages/opencode/test/server/httpapi-mcp.test.ts @@ -5,6 +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 { Server } from "../../src/server/server" import * as Log from "@opencode-ai/core/util/log" import { resetDatabase } from "../fixture/db" @@ -57,7 +58,7 @@ function withMcpProject(self: (dir: string) => Effect.Effect) }), ) 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)) diff --git a/packages/opencode/test/server/httpapi-provider.test.ts b/packages/opencode/test/server/httpapi-provider.test.ts index 3ff3893005e8..5714f719a591 100644 --- a/packages/opencode/test/server/httpapi-provider.test.ts +++ b/packages/opencode/test/server/httpapi-provider.test.ts @@ -3,6 +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 { Server } from "../../src/server/server" import * as Log from "@opencode-ai/core/util/log" import { resetDatabase } from "../fixture/db" @@ -89,7 +90,7 @@ function withProviderProject(self: (dir: string) => Effect.Effect - 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)) diff --git a/packages/opencode/test/server/project-init-git.test.ts b/packages/opencode/test/server/project-init-git.test.ts index 0177cde82f70..48e28aa5acc2 100644 --- a/packages/opencode/test/server/project-init-git.test.ts +++ b/packages/opencode/test/server/project-init-git.test.ts @@ -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 path from "path" import { GlobalBus } from "../../src/bus/global" import { Snapshot } from "../../src/snapshot" -import { Instance } from "../../src/project/instance" import { Server } from "../../src/server/server" import { Filesystem } from "@/util/filesystem" import * as Log from "@opencode-ai/core/util/log" @@ -16,6 +15,9 @@ afterEach(async () => { 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", () => { test("initializes git and reloads immediately", async () => { await using tmp = await tmpdir() @@ -24,8 +26,6 @@ describe("project.initGit endpoint", () => { const fn = (evt: { directory?: string; payload: { type: string } }) => { seen.push(evt) } - const reload = Instance.reload - const reloadSpy = spyOn(Instance, "reload").mockImplementation((input) => reload(input)) GlobalBus.on("event", fn) try { @@ -42,10 +42,8 @@ describe("project.initGit endpoint", () => { vcs: "git", worktree: tmp.path, }) - expect(reloadSpy).toHaveBeenCalledTimes(1) - expect(seen.some((evt) => evt.directory === tmp.path && evt.payload.type === "server.instance.disposed")).toBe( - true, - ) + // Reload behavior: bus emits exactly one server.instance.disposed for the directory. + expect(disposedEvents(seen, tmp.path)).toBe(1) expect(await Filesystem.exists(path.join(tmp.path, ".git", "opencode"))).toBe(false) const current = await app.request("/project/current", { @@ -70,7 +68,6 @@ describe("project.initGit endpoint", () => { ).toBeTruthy() } finally { await disposeAllInstances() - reloadSpy.mockRestore() GlobalBus.off("event", fn) } }) @@ -82,8 +79,6 @@ describe("project.initGit endpoint", () => { const fn = (evt: { directory?: string; payload: { type: string } }) => { seen.push(evt) } - const reload = Instance.reload - const reloadSpy = spyOn(Instance, "reload").mockImplementation((input) => reload(input)) GlobalBus.on("event", fn) try { @@ -98,10 +93,7 @@ describe("project.initGit endpoint", () => { vcs: "git", worktree: tmp.path, }) - expect( - seen.filter((evt) => evt.directory === tmp.path && evt.payload.type === "server.instance.disposed").length, - ).toBe(0) - expect(reloadSpy).toHaveBeenCalledTimes(0) + expect(disposedEvents(seen, tmp.path)).toBe(0) const current = await app.request("/project/current", { headers: { @@ -115,7 +107,6 @@ describe("project.initGit endpoint", () => { }) } finally { await disposeAllInstances() - reloadSpy.mockRestore() GlobalBus.off("event", fn) } })