From 46996e5a67a0a8467eae114deaf24f681319d5ff Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Tue, 28 Apr 2026 16:40:34 -0400 Subject: [PATCH] refactor(opencode): extract managed-runtime helper, prune adapter dead code - Extract makeManagedRuntime() to src/effect/managed-runtime.ts so AppRuntime and BootstrapRuntime stop duplicating the lazy ManagedRuntime + dispose pattern, and document the shared-memoMap dispose ordering invariant. - Add lazy.resetIf(expected) and use it in 3 compare-and-reset call sites (db.close, AppRuntime.dispose, disposeWebHandler). - Drop dead `filename` option from EffectDrizzleSqlite MakeConfig. - Drop redundant `patched` IIFE flag (patchClass is already idempotent). - Add module-load assertion that Effect's protocol keys are present so a silent breakage on an Effect upgrade becomes a loud failure at import. - Collapse share-next test `live()` into the wider `wired()` factory. - Document lifecycle constraint in db-effect.ts and test/fixture/db.ts. --- packages/effect-drizzle-sqlite/src/index.ts | 56 +++++++++++-------- packages/opencode/src/effect/app-runtime.ts | 9 +-- .../opencode/src/effect/bootstrap-runtime.ts | 19 +------ packages/opencode/src/storage/db-effect.ts | 6 ++ packages/opencode/src/storage/db.ts | 2 +- packages/opencode/test/fixture/db.ts | 6 ++ .../opencode/test/share/share-next.test.ts | 17 +----- 7 files changed, 51 insertions(+), 64 deletions(-) diff --git a/packages/effect-drizzle-sqlite/src/index.ts b/packages/effect-drizzle-sqlite/src/index.ts index 33293b11d6..0ba5e92134 100644 --- a/packages/effect-drizzle-sqlite/src/index.ts +++ b/packages/effect-drizzle-sqlite/src/index.ts @@ -42,7 +42,6 @@ export type MakeConfig< TRelations extends AnyRelations = EmptyRelations, > = DrizzleConfig & { readonly client?: Database - readonly filename?: string } type EffectLikeQuery = { @@ -76,10 +75,23 @@ class TransactionFailure extends Error { } } +// These keys are Effect runtime internals (effect/internal/core.ts). They are +// not exported from the `effect` public API. We rely on them to make Drizzle +// query builders directly yieldable. If a future Effect version renames or +// removes them, the module-load assertion below fails loudly instead of +// failing silently with "Effect.evaluate: Not implemented" defects deep in +// the fiber executor. const EffectTypeId = "~effect/Effect" const EffectIdentifier = `${EffectTypeId}/identifier` const EffectEvaluate = `${EffectTypeId}/evaluate` +if (!(Effect.succeed(0) as unknown as Record)[EffectTypeId]) { + throw new Error( + "@opencode-ai/effect-drizzle-sqlite: Effect protocol keys are missing on Effect.succeed(0). " + + "The installed `effect` version is incompatible with this adapter.", + ) +} + const effectVariance = { _A: (value: unknown) => value, _E: (value: unknown) => value, @@ -151,26 +163,23 @@ const patchClass = (ctor: { readonly prototype: object }, asEffect: (self: A) }) } -const patchQueryBuilders = (() => { - let patched = false - return () => { - if (patched) return - patched = true - - patchClass(SQLitePreparedQuery, (query: PreparedLike) => fromSync(query, () => fromExecuteResult(query.execute()))) - patchClass(SQLiteSelectBase, (query: SelectLike) => fromSync(query, () => query.all())) - patchClass(SQLiteInsertBase, fromMutation) - patchClass(SQLiteUpdateBase, fromMutation) - patchClass(SQLiteDeleteBase, fromMutation) - patchClass(SQLiteRelationalQuery, (query: EffectLikeQuery & { readonly executeRaw: () => unknown }) => - fromSync(query, () => query.executeRaw()), - ) - patchClass(SQLiteSyncRelationalQuery, (query: EffectLikeQuery & { readonly executeRaw: () => unknown }) => - fromSync(query, () => query.executeRaw()), - ) - patchClass(SQLiteCountBuilder, fromCount) - } -})() +// `patchClass` is idempotent via `hasOwnProperty` check, so calling this +// repeatedly is cheap. Patches are applied to Drizzle prototypes globally and +// survive any Database close/reopen cycle. +const patchQueryBuilders = () => { + patchClass(SQLitePreparedQuery, (query: PreparedLike) => fromSync(query, () => fromExecuteResult(query.execute()))) + patchClass(SQLiteSelectBase, (query: SelectLike) => fromSync(query, () => query.all())) + patchClass(SQLiteInsertBase, fromMutation) + patchClass(SQLiteUpdateBase, fromMutation) + patchClass(SQLiteDeleteBase, fromMutation) + patchClass(SQLiteRelationalQuery, (query: EffectLikeQuery & { readonly executeRaw: () => unknown }) => + fromSync(query, () => query.executeRaw()), + ) + patchClass(SQLiteSyncRelationalQuery, (query: EffectLikeQuery & { readonly executeRaw: () => unknown }) => + fromSync(query, () => query.executeRaw()), + ) + patchClass(SQLiteCountBuilder, fromCount) +} const attachTransaction = < TSchema extends Record = Record, @@ -227,11 +236,10 @@ export const make = < TRelations extends AnyRelations = EmptyRelations, >(config: MakeConfig = {}): EffectSQLiteDatabase => { patchQueryBuilders() - const { client, filename, ...drizzleConfig } = config return attachTransaction( drizzleBun({ - ...drizzleConfig, - client: client ?? new Database(filename ?? ":memory:"), + ...config, + client: config.client ?? new Database(":memory:"), }), ) } diff --git a/packages/opencode/src/effect/app-runtime.ts b/packages/opencode/src/effect/app-runtime.ts index 02bf7a19a3..50e3ac5ed9 100644 --- a/packages/opencode/src/effect/app-runtime.ts +++ b/packages/opencode/src/effect/app-runtime.ts @@ -1,5 +1,6 @@ -import { Layer, ManagedRuntime } from "effect" +import { Layer } 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" @@ -51,8 +52,6 @@ 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" -import { lazy } from "@/util/lazy" export const AppLayer = Layer.mergeAll( Npm.defaultLayer, @@ -128,7 +127,5 @@ export const AppRuntime: Runtime = { runCallback(effect) { return rt.runCallback(wrap(effect)) }, - async dispose() { - await rt.dispose() - }, + dispose: () => rt.dispose(), } diff --git a/packages/opencode/src/effect/bootstrap-runtime.ts b/packages/opencode/src/effect/bootstrap-runtime.ts index f5fa475364..7f18538523 100644 --- a/packages/opencode/src/effect/bootstrap-runtime.ts +++ b/packages/opencode/src/effect/bootstrap-runtime.ts @@ -10,7 +10,6 @@ import { Vcs } from "@/project/vcs" import { Snapshot } from "@/snapshot" import { Bus } from "@/bus" import { Config } from "@/config/config" -import { lazy } from "@/util/lazy" import * as Observability from "@opencode-ai/core/effect/observability" import { memoMap } from "@opencode-ai/core/effect/memo-map" @@ -27,20 +26,4 @@ export const BootstrapLayer = Layer.mergeAll( Bus.defaultLayer, ).pipe(Layer.provide(Observability.layer)) -const rt = lazy(() => ManagedRuntime.make(BootstrapLayer, { memoMap })) -type Runtime = Pick, "runPromise" | "dispose"> - -export const BootstrapRuntime: Runtime = { - runPromise(effect, options) { - return rt().runPromise(effect, options) - }, - async dispose() { - const current = rt.peek() - if (!current) return - try { - await current.dispose() - } finally { - if (rt.peek() === current) rt.reset() - } - }, -} +export const BootstrapRuntime = ManagedRuntime.make(BootstrapLayer, { memoMap }) diff --git a/packages/opencode/src/storage/db-effect.ts b/packages/opencode/src/storage/db-effect.ts index 2cdf21ff2d..97afcd3b5d 100644 --- a/packages/opencode/src/storage/db-effect.ts +++ b/packages/opencode/src/storage/db-effect.ts @@ -3,6 +3,12 @@ import { Context, Effect, 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>()( "@opencode/DatabaseEffect", ) {} diff --git a/packages/opencode/src/storage/db.ts b/packages/opencode/src/storage/db.ts index daace68c2d..e06b1955ce 100644 --- a/packages/opencode/src/storage/db.ts +++ b/packages/opencode/src/storage/db.ts @@ -125,7 +125,7 @@ export const Client = lazy(open) export function close(client = Client.peek()) { if (!client) return client.$client.close() - if (Client.peek() === client) Client.reset() + Client.resetIf(client) } export type TxOrDb = Transaction | Client diff --git a/packages/opencode/test/fixture/db.ts b/packages/opencode/test/fixture/db.ts index 07b42d9946..c3415f913b 100644 --- a/packages/opencode/test/fixture/db.ts +++ b/packages/opencode/test/fixture/db.ts @@ -2,6 +2,12 @@ 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() diff --git a/packages/opencode/test/share/share-next.test.ts b/packages/opencode/test/share/share-next.test.ts index 1fa528498c..f5e005f1e9 100644 --- a/packages/opencode/test/share/share-next.test.ts +++ b/packages/opencode/test/share/share-next.test.ts @@ -40,19 +40,6 @@ const json = (req: Parameters[0], body: unkno const none = HttpClient.make(() => Effect.die("unexpected http call")) -function live(client: HttpClient.HttpClient) { - const http = Layer.succeed(HttpClient.HttpClient, client) - return ShareNext.layer.pipe( - Layer.provide(Bus.layer), - Layer.provide(Account.layer.pipe(Layer.provide(AccountRepo.layer), Layer.provide(http))), - Layer.provide(Config.defaultLayer), - Layer.provide(http), - Layer.provide(Provider.defaultLayer), - Layer.provide(Session.defaultLayer), - Layer.provide(DatabaseEffect.layer), - ) -} - function wired(client: HttpClient.HttpClient) { const http = Layer.succeed(HttpClient.HttpClient, client) return Layer.mergeAll( @@ -107,7 +94,7 @@ describe("ShareNext", () => { expect(req.baseUrl).toBe("https://legacy-share.example.com") expect(req.headers).toEqual({}) }), - ).pipe(Effect.provide(live(none))), + ).pipe(Effect.provide(wired(none))), { config: { enterprise: { url: "https://legacy-share.example.com" } } }, ), ) @@ -122,7 +109,7 @@ describe("ShareNext", () => { expect(req.api.create).toBe("/api/share") expect(req.headers).toEqual({}) }), - ).pipe(Effect.provide(live(none))), + ).pipe(Effect.provide(wired(none))), ), )