Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 79 additions & 18 deletions packages/opencode/src/project/instance-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,19 @@ export const use = serviceUse(Service)

interface Entry {
readonly deferred: Deferred.Deferred<InstanceContext>
readonly owners: Set<symbol>
}

const inFlight = new Map<string, Entry>()

const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Service> = Layer.effect(
Service,
Effect.gen(function* () {
const project = yield* Project.Service
const bootstrap = yield* InstanceBootstrap.Service
const scope = yield* Scope.Scope
const cache = new Map<string, Entry>()
const owner = Symbol("InstanceStore")

const boot = (input: LoadInput & { directory: string }) =>
Effect.gen(function* () {
Expand All @@ -66,15 +70,52 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
Effect.sync(() => {
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)
})
}).pipe(
// Guarantee the deferred is settled even if this fiber is interrupted before
// Deferred.done runs (e.g. the owning layer scope closes mid-init). Otherwise
// cross-store joiners parked on entry.deferred would hang forever. Settling an
// already-settled deferred is a no-op, so the normal (success/failure) paths are
// unaffected. A concrete failure exit is used rather than Deferred.interrupt
// because the latter does not reliably wake joiners parked from another store.
Effect.ensuring(
Deferred.done(entry.deferred, Exit.die(new Error("instance initialization interrupted"))).pipe(Effect.asVoid),
),
)

const emitDisposed = (input: { directory: string; project?: string }) =>
Effect.sync(() =>
Expand All @@ -97,11 +138,15 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
yield* emitDisposed({ directory: ctx.directory, project: ctx.project.id })
})

const disposeEntry = Effect.fnUntraced(function* (directory: string, entry: Entry, ctx: InstanceContext) {
if (cache.get(directory) !== entry) return false
const disposeReleasedEntry = Effect.fnUntraced(function* (entry: Entry, ctx: InstanceContext) {
if (entry.owners.size > 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
})

Expand All @@ -110,15 +155,22 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
return Effect.uninterruptibleMask((restore) =>
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 running = inFlight.get(directory)
if (running) {
yield* acquireEntry(directory, running)
return yield* restore(awaitEntry(directory, running))
}

const entry: Entry = { deferred: Deferred.makeUnsafe<InstanceContext>() }
const entry: Entry = { deferred: Deferred.makeUnsafe<InstanceContext>(), 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"))
}
Expand All @@ -128,28 +180,34 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
return Effect.uninterruptibleMask((restore) =>
Effect.gen(function* () {
const previous = cache.get(directory)
const entry: Entry = { deferred: Deferred.makeUnsafe<InstanceContext>() }
const entry: Entry = { deferred: Deferred.makeUnsafe<InstanceContext>(), 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)
})
Expand All @@ -159,7 +217,10 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
const entry = cache.get(directory)
if (!entry) return
const exit = yield* Deferred.await(entry.deferred).pipe(Effect.exit)
if (Exit.isFailure(exit)) return yield* removeEntry(directory, entry).pipe(Effect.asVoid)
if (Exit.isFailure(exit)) {
yield* removeEntry(directory, entry).pipe(Effect.asVoid)
return
}
yield* disposeEntry(directory, entry, exit.value).pipe(Effect.asVoid)
})

Expand Down
120 changes: 119 additions & 1 deletion packages/opencode/test/project/instance.test.ts
Original file line number Diff line number Diff line change
@@ -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 { 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"
Expand All @@ -21,6 +21,11 @@ const it = testEffect(
]),
)

const storeLayer = () =>
LayerNode.compile(LayerNode.group([InstanceStore.node, CrossSpawnSpawner.node]), [
[InstanceStore.bootstrapNode, noopBootstrap],
])

const setBootstrap = (run: Effect.Effect<void>) =>
Effect.acquireRelease(
Effect.sync(() => {
Expand Down Expand Up @@ -121,6 +126,119 @@ 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<void>()
const release = yield* Deferred.make<void>()
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("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<string> = []
yield* registerDisposerScoped(async (directory) => {
disposed.push(directory)
})

const started = yield* Deferred.make<void>()
const release = yield* Deferred.make<void>()
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<void>()
const release = yield* Deferred.make<void>()
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 })
Expand Down
Loading