From 4eb75209e82e331c1e7623339aec009118255047 Mon Sep 17 00:00:00 2001 From: CreatorGhost Date: Sun, 12 Jul 2026 01:16:44 +0530 Subject: [PATCH 1/2] fix(tui): avoid duplicate project initialization Ported from upstream anomalyco/opencode#35998. --- .../opencode/src/project/instance-store.ts | 85 +++++++++++++++---- .../opencode/test/project/instance.test.ts | 36 +++++++- 2 files changed, 103 insertions(+), 18 deletions(-) diff --git a/packages/opencode/src/project/instance-store.ts b/packages/opencode/src/project/instance-store.ts index 720549ddaff7..776f2206566c 100644 --- a/packages/opencode/src/project/instance-store.ts +++ b/packages/opencode/src/project/instance-store.ts @@ -32,8 +32,11 @@ export const use = serviceUse(Service) interface Entry { readonly deferred: Deferred.Deferred + readonly owners: Set } +const inFlight = new Map() + const layer: Layer.Layer = Layer.effect( Service, Effect.gen(function* () { @@ -41,6 +44,7 @@ const layer: Layer.Layer() + const owner = Symbol("InstanceStore") const boot = (input: LoadInput & { directory: string }) => Effect.gen(function* () { @@ -66,13 +70,40 @@ const layer: Layer.Layer { if (cache.get(directory) !== entry) return false cache.delete(directory) + entry.owners.delete(owner) return true }) + const releaseEntry = (entry: Entry) => + Effect.sync(() => { + return entry.owners.delete(owner) + }) + + const acquireEntry = (directory: string, entry: Entry) => + Effect.sync(() => { + cache.set(directory, entry) + entry.owners.add(owner) + }) + + const clearInFlight = (directory: string, entry: Entry) => + Effect.sync(() => { + if (inFlight.get(directory) === entry) inFlight.delete(directory) + }) + + const awaitEntry = (directory: string, entry: Entry) => + Effect.gen(function* () { + const exit = yield* Deferred.await(entry.deferred).pipe(Effect.exit) + if (Exit.isFailure(exit)) yield* removeEntry(directory, entry) + return yield* exit + }) + const completeLoad = (directory: string, input: LoadInput, entry: Entry) => Effect.gen(function* () { const exit = yield* Effect.exit(boot({ ...input, directory })) - if (Exit.isFailure(exit)) yield* removeEntry(directory, entry) + if (Exit.isFailure(exit)) { + yield* removeEntry(directory, entry) + yield* clearInFlight(directory, entry) + } yield* Deferred.done(entry.deferred, exit).pipe(Effect.asVoid) }) @@ -97,11 +128,15 @@ const layer: Layer.Layer 0) return false yield* disposeContext(ctx) - if (cache.get(directory) !== entry) return false - cache.delete(directory) + return true + }) + + const disposeEntry = Effect.fnUntraced(function* (directory: string, entry: Entry, ctx: InstanceContext) { + if (!(yield* removeEntry(directory, entry))) return false + yield* disposeReleasedEntry(entry, ctx) return true }) @@ -110,15 +145,22 @@ const layer: Layer.Layer Effect.gen(function* () { const existing = cache.get(directory) - if (existing) return yield* restore(Deferred.await(existing.deferred)) + if (existing) return yield* restore(awaitEntry(directory, existing)) - const entry: Entry = { deferred: Deferred.makeUnsafe() } + const running = inFlight.get(directory) + if (running) { + yield* acquireEntry(directory, running) + return yield* restore(awaitEntry(directory, running)) + } + + const entry: Entry = { deferred: Deferred.makeUnsafe(), owners: new Set([owner]) } cache.set(directory, entry) + inFlight.set(directory, entry) yield* Effect.gen(function* () { yield* Effect.logInfo("creating instance", { directory: directory }) yield* completeLoad(directory, input, entry) - }).pipe(Effect.forkIn(scope, { startImmediately: true })) - return yield* restore(Deferred.await(entry.deferred)) + }).pipe(Effect.ensuring(clearInFlight(directory, entry)), Effect.forkIn(scope, { startImmediately: true })) + return yield* restore(awaitEntry(directory, entry)) }), ).pipe(Effect.withSpan("InstanceStore.load")) } @@ -128,28 +170,34 @@ const layer: Layer.Layer Effect.gen(function* () { const previous = cache.get(directory) - const entry: Entry = { deferred: Deferred.makeUnsafe() } + const entry: Entry = { deferred: Deferred.makeUnsafe(), owners: new Set([owner]) } cache.set(directory, entry) yield* Effect.gen(function* () { yield* Effect.logInfo("reloading instance", { directory: directory }) if (previous) { - yield* Deferred.await(previous.deferred).pipe(Effect.ignore) - yield* Effect.promise(() => runDisposers(directory)) - yield* emitDisposed({ directory, project: input.project?.id }) + const exit = yield* Deferred.await(previous.deferred).pipe(Effect.exit) + yield* releaseEntry(previous) + if (Exit.isSuccess(exit)) yield* disposeReleasedEntry(previous, exit.value) } yield* completeLoad(directory, input, entry) }).pipe(Effect.forkIn(scope, { startImmediately: true })) - return yield* restore(Deferred.await(entry.deferred)) + return yield* restore(awaitEntry(directory, entry)) }), ).pipe(Effect.withSpan("InstanceStore.reload")) } const dispose = Effect.fn("InstanceStore.dispose")(function* (ctx: InstanceContext) { const entry = cache.get(ctx.directory) - if (!entry) return yield* disposeContext(ctx) + if (!entry) { + yield* disposeContext(ctx) + return + } const exit = yield* Deferred.await(entry.deferred).pipe(Effect.exit) - if (Exit.isFailure(exit)) return yield* removeEntry(ctx.directory, entry).pipe(Effect.asVoid) + if (Exit.isFailure(exit)) { + yield* removeEntry(ctx.directory, entry).pipe(Effect.asVoid) + return + } if (exit.value !== ctx) return yield* disposeEntry(ctx.directory, entry, ctx).pipe(Effect.asVoid) }) @@ -159,7 +207,10 @@ const layer: Layer.Layer + LayerNode.compile(LayerNode.group([InstanceStore.node, CrossSpawnSpawner.node]), [ + [InstanceStore.bootstrapNode, noopBootstrap], + ]) + const setBootstrap = (run: Effect.Effect) => Effect.acquireRelease( Effect.sync(() => { @@ -121,6 +126,35 @@ describe("InstanceStore", () => { }), ) + it.live("dedupes concurrent loads across store services while init is in flight", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const firstContext = yield* Layer.build(storeLayer()) + const secondContext = yield* Layer.build(storeLayer()) + const firstStore = Context.get(firstContext, InstanceStore.Service) + const secondStore = Context.get(secondContext, InstanceStore.Service) + const started = yield* Deferred.make() + const release = yield* Deferred.make() + let initialized = 0 + + yield* setBootstrap( + Effect.gen(function* () { + initialized++ + yield* Deferred.succeed(started, undefined) + yield* Deferred.await(release) + }), + ) + const first = yield* firstStore.load({ directory: dir }).pipe(Effect.forkScoped) + yield* Deferred.await(started) + const second = yield* secondStore.load({ directory: dir }).pipe(Effect.forkScoped) + + yield* Deferred.succeed(release, undefined) + const [firstCtx, secondCtx] = yield* Effect.all([Fiber.join(first), Fiber.join(second)]) + expect(secondCtx).toBe(firstCtx) + expect(initialized).toBe(1) + }), + ) + it.live("removes failed loads from the cache", () => Effect.gen(function* () { const dir = yield* tmpdirScoped({ git: true }) From d81bbe16d65e6a9087603013f46753da30d54592 Mon Sep 17 00:00:00 2001 From: CreatorGhost Date: Sun, 12 Jul 2026 21:28:20 +0530 Subject: [PATCH 2/2] fix(project): settle instance deferred on init interruption Addresses Opus review of PR #22: if an in-flight instance init fiber was interrupted (owning layer scope closed mid-init), the shared entry.deferred was never settled and cross-store joiners parked on it hung forever. completeLoad now settles the deferred on every exit via Effect.ensuring. Deferred.interrupt proved insufficient (it only woke the creating store's awaiter, not cross-store joiners), so a concrete failure exit is used. Adds two regression tests: refcounted cross-store disposal, and settling on init interruption. --- .../opencode/src/project/instance-store.ts | 12 ++- .../opencode/test/project/instance.test.ts | 86 ++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/project/instance-store.ts b/packages/opencode/src/project/instance-store.ts index 776f2206566c..7bc6f823e94b 100644 --- a/packages/opencode/src/project/instance-store.ts +++ b/packages/opencode/src/project/instance-store.ts @@ -105,7 +105,17 @@ const layer: Layer.Layer Effect.sync(() => diff --git a/packages/opencode/test/project/instance.test.ts b/packages/opencode/test/project/instance.test.ts index 8fcb34694f5f..02f5de28f3fa 100644 --- a/packages/opencode/test/project/instance.test.ts +++ b/packages/opencode/test/project/instance.test.ts @@ -1,7 +1,7 @@ import { describe, expect } from "bun:test" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" -import { Context, Deferred, Effect, Fiber, Layer } from "effect" +import { Context, Deferred, Effect, Exit, Fiber, Layer, Scope } from "effect" import { InstanceRef } from "../../src/effect/instance-ref" import { registerDisposer } from "../../src/effect/instance-registry" import { InstanceBootstrap } from "../../src/project/bootstrap" @@ -155,6 +155,90 @@ describe("InstanceStore", () => { }), ) + it.live("refcounts disposal so a shared instance survives until the last owner disposes", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + // Two genuinely independent stores (separate memo maps) that share only the + // module-global in-flight map, so the second load joins via `inFlight` and + // becomes a co-owner of the shared entry. + const firstStoreScope = yield* Scope.make() + const secondStoreScope = yield* Scope.make() + const firstContext = yield* Layer.buildWithMemoMap(storeLayer(), Layer.makeMemoMapUnsafe(), firstStoreScope) + const secondContext = yield* Layer.buildWithMemoMap(storeLayer(), Layer.makeMemoMapUnsafe(), secondStoreScope) + const firstStore = Context.get(firstContext, InstanceStore.Service) + const secondStore = Context.get(secondContext, InstanceStore.Service) + const disposed: Array = [] + yield* registerDisposerScoped(async (directory) => { + disposed.push(directory) + }) + + const started = yield* Deferred.make() + const release = yield* Deferred.make() + yield* setBootstrap( + Effect.gen(function* () { + yield* Deferred.succeed(started, undefined) + yield* Deferred.await(release) + }), + ) + const first = yield* firstStore.load({ directory: dir }).pipe(Effect.forkScoped) + yield* Deferred.await(started) + const second = yield* secondStore.load({ directory: dir }).pipe(Effect.forkScoped) + // Cross-store dedup happens via the module-global in-flight map, which is only + // populated while init is in flight. Let the second load park on the shared entry + // (becoming a co-owner) before releasing init; otherwise it would race the + // in-flight cleanup and build its own instance. + yield* Effect.sleep("100 millis") + yield* Deferred.succeed(release, undefined) + const [firstCtx, secondCtx] = yield* Effect.all([Fiber.join(first), Fiber.join(second)]) + expect(secondCtx).toBe(firstCtx) + + // Disposing via the first store leaves the second store as an owner: not disposed. + yield* firstStore.dispose(firstCtx) + expect(disposed).toEqual([]) + + // Disposing via the last owner disposes exactly once. + yield* secondStore.dispose(secondCtx) + expect(disposed).toEqual([dir]) + + yield* Scope.close(firstStoreScope, Exit.void) + yield* Scope.close(secondStoreScope, Exit.void) + }), + ) + + it.live("interrupting an in-flight init settles the deferred instead of hanging joiners", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + // Build the store in a scope we can close to interrupt its in-flight init fiber. + // buildWithMemoMap binds the store's resources (including the forked init fiber) + // to storeScope, so Scope.close interrupts the init. Any caller — including a + // cross-store joiner parked on the shared entry.deferred — must then settle with a + // failure rather than hang forever. + const storeScope = yield* Scope.make() + const storeContext = yield* Layer.buildWithMemoMap(storeLayer(), Layer.makeMemoMapUnsafe(), storeScope) + const store = Context.get(storeContext, InstanceStore.Service) + const started = yield* Deferred.make() + const release = yield* Deferred.make() + yield* setBootstrap( + Effect.gen(function* () { + yield* Deferred.succeed(started, undefined) + yield* Deferred.await(release) // never released: init stays in flight + }), + ) + const loader = yield* store.load({ directory: dir }).pipe(Effect.forkScoped) + yield* Deferred.await(started) + + // Tear down the store that owns the in-flight init fiber. + yield* Scope.close(storeScope, Exit.void) + + const outcome = yield* Fiber.join(loader).pipe( + Effect.as("ok" as const), + Effect.catchCause(() => Effect.succeed("failed" as const)), + Effect.timeoutOrElse({ duration: "5 seconds", orElse: () => Effect.succeed("hung" as const) }), + ) + expect(outcome).toBe("failed") + }), + ) + it.live("removes failed loads from the cache", () => Effect.gen(function* () { const dir = yield* tmpdirScoped({ git: true })