refactor(opencode): simplify database effect reset lifecycle

packages/opencode/src/effect/app-runtime.ts

@@ -1,6 +1,5 @@
-import { Layer } from "effect"
+import { Layer, ManagedRuntime } from "effect"
 import { attach } from "./run-service"
-import { makeManagedRuntime } from "./managed-runtime"
 import * as Observability from "@opencode-ai/core/effect/observability"
 
 import { AppFileSystem } from "@opencode-ai/core/filesystem"
@@ -52,6 +51,7 @@ import { ShareNext } from "@/share/share-next"
 import { SessionShare } from "@/share/session"
 import { SyncEvent } from "@/sync"
 import { Npm } from "@opencode-ai/core/npm"
+import { memoMap } from "@opencode-ai/core/effect/memo-map"
 
 export const AppLayer = Layer.mergeAll(
   Npm.defaultLayer,

packages/opencode/src/storage/db-effect.ts

@@ -1,18 +1,20 @@
 import { Database } from "@/storage/db"
-import { Context, Effect, Layer } from "effect"
+import { Context, Layer } from "effect"
 import type { EffectSQLiteDatabase } from "@opencode-ai/effect-drizzle-sqlite"
 import * as StorageSchema from "@/storage/schema"
 
-// Thin Effect Service over the module-global `Database.Client` lazy. The DB
-// lifecycle is owned by `Database.open` / `Database.close`, not by this
-// layer. Any runtime (see `effect/managed-runtime.ts`) that consumes this
-// layer through the shared layer memoMap must be disposed before
-// `Database.close()` so its memoized Service value does not outlive the
-// underlying SQLite handle. See `test/fixture/db.ts:resetDatabase`.
 export class Service extends Context.Service<Service, EffectSQLiteDatabase<typeof StorageSchema>>()(
   "@opencode/DatabaseEffect",
 ) {}
 
-export const layer = Layer.effect(Service, Effect.sync(Database.Client))
+const client = new Proxy({} as EffectSQLiteDatabase<typeof StorageSchema>, {
+  get(_target, property) {
+    const db = Database.Client()
+    const value = Reflect.get(db, property)
+    return typeof value === "function" ? value.bind(db) : value
+  },
+})
+
+export const layer = Layer.succeed(Service, client)
 
 export * as DatabaseEffect from "./db-effect"

packages/opencode/src/storage/db.ts

@@ -48,9 +48,9 @@ export type Transaction = Parameters<Parameters<Client["transaction"]>[0]>[0]
 type Journal = { sql: string; timestamp: number; name: string }[]
 
 // Drizzle's migrate overloads trigger expensive variance checks here; narrow to the journal overload we actually use.
-const migrateFromJournal = migrate as unknown as (db: SQLiteBunDatabase, entries: Journal) => void
+const migrateFromJournal = migrate as unknown as (db: Client, entries: Journal) => void
 
-function applyMigrations(db: SQLiteBunDatabase, entries: Journal) {
+function applyMigrations(db: Client, entries: Journal) {
   migrateFromJournal(db, entries)
 }
 
@@ -122,10 +122,10 @@ export function open() {
 
 export const Client = lazy(open)
 
-export function close(client = Client.peek()) {
-  if (!client) return
-  client.$client.close()
-  Client.resetIf(client)
+export function close() {
+  if (!Client.loaded()) return
+  Client().$client.close()
+  Client.reset()
 }
 
 export type TxOrDb = Transaction | Client

packages/opencode/test/effect/managed-runtime.test.ts

@@ -1,78 +0,0 @@
-import { describe, expect, test } from "bun:test"
-import { Context, Effect, Layer } from "effect"
-import { makeManagedRuntime } from "@/effect/managed-runtime"
-import { lazy } from "@/util/lazy"
-
-class Counter extends Context.Service<Counter, { readonly value: number }>()("@test/Counter") {}
-
-const layerWith = (value: number) => Layer.succeed(Counter, { value })
-
-describe("makeManagedRuntime", () => {
-  test("disposing an unbuilt runtime is a no-op", async () => {
-    const rt = makeManagedRuntime(layerWith(0))
-    expect(rt.peek()).toBeUndefined()
-    await rt.dispose()
-    expect(rt.peek()).toBeUndefined()
-  })
-
-  test("disposing rebuilds on next access", async () => {
-    const rt = makeManagedRuntime(layerWith(7))
-    const first = rt()
-    const value = await first.runPromise(
-      Effect.gen(function* () {
-        return (yield* Counter).value
-      }),
-    )
-    expect(value).toBe(7)
-
-    await rt.dispose()
-    expect(rt.peek()).toBeUndefined()
-
-    const second = rt()
-    expect(second).not.toBe(first)
-    expect(rt.peek()).toBe(second)
-    await rt.dispose()
-  })
-
-  test("dispose() does not clobber a runtime that was rebuilt mid-dispose", async () => {
-    // Simulates a race where dispose() ran on instance A, then someone
-    // invoked the lazy and got a fresh instance B before dispose() returned.
-    // The resetIf guard must leave instance B intact.
-    const rt = makeManagedRuntime(layerWith(1))
-    const first = rt()
-    rt.reset() // force-eject the lazy
-    const second = rt() // build a new instance, distinct from first
-    expect(second).not.toBe(first)
-
-    // Calling dispose() now should tear down `second` (the current value),
-    // not the orphaned `first`.
-    await rt.dispose()
-    expect(rt.peek()).toBeUndefined()
-  })
-})
-
-describe("lazy.resetIf", () => {
-  test("resets when the value matches", () => {
-    const factory = lazy(() => ({}))
-    const value = factory()
-    expect(factory.peek()).toBe(value)
-    factory.resetIf(value)
-    expect(factory.peek()).toBeUndefined()
-  })
-
-  test("leaves the lazy intact when the value does not match", () => {
-    const factory = lazy(() => ({}))
-    const captured = factory()
-    factory.reset()
-    const fresh = factory()
-    expect(fresh).not.toBe(captured)
-    factory.resetIf(captured)
-    expect(factory.peek()).toBe(fresh)
-  })
-
-  test("is a no-op on an unloaded lazy", () => {
-    const factory = lazy(() => ({}))
-    factory.resetIf({} as never)
-    expect(factory.peek()).toBeUndefined()
-  })
-})

packages/opencode/test/fixture/db.ts

@@ -2,12 +2,6 @@ import { rm } from "fs/promises"
 import { Database } from "@/storage/db"
 import { disposeAllInstances } from "./fixture"
 
-// Order matters and must stay serial: every runtime that transitively consumes
-// `DatabaseEffect.layer` shares the global layer memoMap with the others, so
-// each one's memoized Service value still references the live SQLite handle.
-// We dispose every runtime/handler first, then close the DB. If a future
-// module-scoped runtime is added that depends on the DB, register its
-// dispose() here.
 export async function resetDatabase() {
   await disposeAllInstances().catch(() => undefined)
   Database.close()

packages/opencode/test/storage/db-effect.test.ts

@@ -1,6 +1,5 @@
 import { afterEach, describe, expect, test } from "bun:test"
 import { Effect, ManagedRuntime } from "effect"
-import { memoMap } from "@opencode-ai/core/effect/memo-map"
 import { Database } from "@/storage/db"
 import { DatabaseEffect } from "@/storage/db-effect"
 import { resetDatabase } from "../fixture/db"
@@ -25,17 +24,15 @@ describe("DatabaseEffect.layer", () => {
     }
   })
 
-  test("rebuilds a fresh handle after Database.close + runtime dispose", async () => {
-    const rt1 = ManagedRuntime.make(DatabaseEffect.layer)
-    const first = await rt1.runPromise(Effect.sync(() => Database.Client().$client))
+  test("service resolves a fresh handle after Database.close", async () => {
+    const rt = ManagedRuntime.make(DatabaseEffect.layer)
+    const first = await rt.runPromise(Effect.sync(() => Database.Client().$client))
     expect(first.prepare("SELECT 1 as n").get()).toEqual({ n: 1 })
 
-    await rt1.dispose()
     Database.close()
 
-    const rt2 = ManagedRuntime.make(DatabaseEffect.layer)
     try {
-      const second = await rt2.runPromise(
+      const second = await rt.runPromise(
         Effect.gen(function* () {
           const db = yield* DatabaseEffect.Service
           return db.$client
@@ -44,34 +41,24 @@ describe("DatabaseEffect.layer", () => {
       expect(second).not.toBe(first)
       expect(second.prepare("SELECT 1 as n").get()).toEqual({ n: 1 })
     } finally {
-      await rt2.dispose()
+      await rt.dispose()
     }
   })
-})
 
-// Regression for the memoMap lifecycle bug. The shared layer memoMap caches
-// every `DatabaseEffect.layer` build across every runtime built with
-// `makeManagedRuntime`. If a runtime that consumed the layer is NOT disposed
-// before `Database.close()`, the cached Service value (a Drizzle wrapper
-// over a now-closed `bun:sqlite` handle) persists in the memoMap and any
-// subsequent runtime that consumes the layer reuses it and operates on a
-// closed handle.
-//
-// `test/fixture/db.ts:resetDatabase` disposes every module-scoped runtime
-// before closing the DB to release the memoMap entries. The two tests below
-// pin both halves of the invariant.
-describe("DatabaseEffect.layer + shared memoMap lifecycle", () => {
-  test("disposing a runtime releases its memoMap entry so the next build sees a fresh DB handle", async () => {
-    const rt1 = ManagedRuntime.make(DatabaseEffect.layer, { memoMap })
-    const captured = await rt1.runPromise(Effect.sync(() => Database.Client().$client))
+  test("a runtime kept alive over Database.close uses the refreshed handle", async () => {
+    const rt = ManagedRuntime.make(DatabaseEffect.layer)
+    const captured = await rt.runPromise(
+      Effect.gen(function* () {
+        const db = yield* DatabaseEffect.Service
+        return db.$client
+      }),
+    )
     expect(captured.prepare("SELECT 1 as n").get()).toEqual({ n: 1 })
 
-    await rt1.dispose()
     Database.close()
 
-    const rt2 = ManagedRuntime.make(DatabaseEffect.layer, { memoMap })
     try {
-      const fresh = await rt2.runPromise(
+      const fresh = await rt.runPromise(
         Effect.gen(function* () {
           const db = yield* DatabaseEffect.Service
           return db.$client
@@ -80,40 +67,7 @@ describe("DatabaseEffect.layer + shared memoMap lifecycle", () => {
       expect(fresh).not.toBe(captured)
       expect(fresh.prepare("SELECT 1 as n").get()).toEqual({ n: 1 })
     } finally {
-      await rt2.dispose()
-    }
-  })
-
-  test("a stale runtime kept alive over Database.close poisons later memoMap consumers", async () => {
-    const stale = ManagedRuntime.make(DatabaseEffect.layer, { memoMap })
-    const captured = await stale.runPromise(
-      Effect.gen(function* () {
-        const db = yield* DatabaseEffect.Service
-        return db.$client
-      }),
-    )
-    expect(captured.prepare("SELECT 1 as n").get()).toEqual({ n: 1 })
-
-    // Intentionally do NOT dispose `stale` before closing the DB. This is
-    // the shape of the bug `resetDatabase` guards against.
-    Database.close()
-
-    const next = ManagedRuntime.make(DatabaseEffect.layer, { memoMap })
-    try {
-      const seen = await next.runPromise(
-        Effect.gen(function* () {
-          const db = yield* DatabaseEffect.Service
-          return db.$client
-        }),
-      )
-      // The memoMap returned the same stale handle because `stale` was
-      // never disposed. The underlying connection is closed, so any query
-      // on the handle throws.
-      expect(seen).toBe(captured)
-      expect(() => seen.prepare("SELECT 1 as n").get()).toThrow()
-    } finally {
-      await next.dispose()
-      await stale.dispose()
+      await rt.dispose()
     }
   })
 })