Move service state into InstanceState, flatten service facades (#18483)

packages/opencode/test/effect/instance-state.test.ts

@@ -0,0 +1,384 @@
+import { afterEach, expect, test } from "bun:test"
+import { Duration, Effect, Layer, ManagedRuntime, ServiceMap } from "effect"
+import { InstanceState } from "../../src/effect/instance-state"
+import { Instance } from "../../src/project/instance"
+import { tmpdir } from "../fixture/fixture"
+
+async function access<A, E>(state: InstanceState<A, E>, dir: string) {
+  return Instance.provide({
+    directory: dir,
+    fn: () => Effect.runPromise(InstanceState.get(state)),
+  })
+}
+
+afterEach(async () => {
+  await Instance.disposeAll()
+})
+
+test("InstanceState caches values per directory", async () => {
+  await using tmp = await tmpdir()
+  let n = 0
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make(() => Effect.sync(() => ({ n: ++n })))
+
+        const a = yield* Effect.promise(() => access(state, tmp.path))
+        const b = yield* Effect.promise(() => access(state, tmp.path))
+
+        expect(a).toBe(b)
+        expect(n).toBe(1)
+      }),
+    ),
+  )
+})
+
+test("InstanceState isolates directories", async () => {
+  await using one = await tmpdir()
+  await using two = await tmpdir()
+  let n = 0
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((dir) => Effect.sync(() => ({ dir, n: ++n })))
+
+        const a = yield* Effect.promise(() => access(state, one.path))
+        const b = yield* Effect.promise(() => access(state, two.path))
+        const c = yield* Effect.promise(() => access(state, one.path))
+
+        expect(a).toBe(c)
+        expect(a).not.toBe(b)
+        expect(n).toBe(2)
+      }),
+    ),
+  )
+})
+
+test("InstanceState invalidates on reload", async () => {
+  await using tmp = await tmpdir()
+  const seen: string[] = []
+  let n = 0
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make(() =>
+          Effect.acquireRelease(
+            Effect.sync(() => ({ n: ++n })),
+            (value) =>
+              Effect.sync(() => {
+                seen.push(String(value.n))
+              }),
+          ),
+        )
+
+        const a = yield* Effect.promise(() => access(state, tmp.path))
+        yield* Effect.promise(() => Instance.reload({ directory: tmp.path }))
+        const b = yield* Effect.promise(() => access(state, tmp.path))
+
+        expect(a).not.toBe(b)
+        expect(seen).toEqual(["1"])
+      }),
+    ),
+  )
+})
+
+test("InstanceState invalidates on disposeAll", async () => {
+  await using one = await tmpdir()
+  await using two = await tmpdir()
+  const seen: string[] = []
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((ctx) =>
+          Effect.acquireRelease(
+            Effect.sync(() => ({ dir: ctx.directory })),
+            (value) =>
+              Effect.sync(() => {
+                seen.push(value.dir)
+              }),
+          ),
+        )
+
+        yield* Effect.promise(() => access(state, one.path))
+        yield* Effect.promise(() => access(state, two.path))
+        yield* Effect.promise(() => Instance.disposeAll())
+
+        expect(seen.sort()).toEqual([one.path, two.path].sort())
+      }),
+    ),
+  )
+})
+
+test("InstanceState.get reads the current directory lazily", async () => {
+  await using one = await tmpdir()
+  await using two = await tmpdir()
+
+  interface Api {
+    readonly get: () => Effect.Effect<string>
+  }
+
+  class Test extends ServiceMap.Service<Test, Api>()("@test/InstanceStateLazy") {
+    static readonly layer = Layer.effect(
+      Test,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((ctx) => Effect.sync(() => ctx.directory))
+        const get = InstanceState.get(state)
+
+        return Test.of({
+          get: Effect.fn("Test.get")(function* () {
+            return yield* get
+          }),
+        })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(Test.layer)
+
+  try {
+    const a = await Instance.provide({
+      directory: one.path,
+      fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+    })
+    const b = await Instance.provide({
+      directory: two.path,
+      fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+    })
+
+    expect(a).toBe(one.path)
+    expect(b).toBe(two.path)
+  } finally {
+    await rt.dispose()
+  }
+})
+
+test("InstanceState preserves directory across async boundaries", async () => {
+  await using one = await tmpdir({ git: true })
+  await using two = await tmpdir({ git: true })
+  await using three = await tmpdir({ git: true })
+
+  interface Api {
+    readonly get: () => Effect.Effect<{ directory: string; worktree: string; project: string }>
+  }
+
+  class Test extends ServiceMap.Service<Test, Api>()("@test/InstanceStateAsync") {
+    static readonly layer = Layer.effect(
+      Test,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((ctx) =>
+          Effect.sync(() => ({
+            directory: ctx.directory,
+            worktree: ctx.worktree,
+            project: ctx.project.id,
+          })),
+        )
+
+        return Test.of({
+          get: Effect.fn("Test.get")(function* () {
+            yield* Effect.promise(() => Bun.sleep(1))
+            yield* Effect.sleep(Duration.millis(1))
+            for (let i = 0; i < 100; i++) {
+              yield* Effect.yieldNow
+            }
+            for (let i = 0; i < 100; i++) {
+              yield* Effect.promise(() => Promise.resolve())
+            }
+            yield* Effect.sleep(Duration.millis(2))
+            yield* Effect.promise(() => Bun.sleep(1))
+            return yield* InstanceState.get(state)
+          }),
+        })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(Test.layer)
+
+  try {
+    const [a, b, c] = await Promise.all([
+      Instance.provide({
+        directory: one.path,
+        fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+      }),
+      Instance.provide({
+        directory: two.path,
+        fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+      }),
+      Instance.provide({
+        directory: three.path,
+        fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+      }),
+    ])
+
+    expect(a).toEqual({ directory: one.path, worktree: one.path, project: a.project })
+    expect(b).toEqual({ directory: two.path, worktree: two.path, project: b.project })
+    expect(c).toEqual({ directory: three.path, worktree: three.path, project: c.project })
+    expect(a.project).not.toBe(b.project)
+    expect(a.project).not.toBe(c.project)
+    expect(b.project).not.toBe(c.project)
+  } finally {
+    await rt.dispose()
+  }
+})
+
+test("InstanceState survives high-contention concurrent access", async () => {
+  const N = 20
+  const dirs = await Promise.all(Array.from({ length: N }, () => tmpdir()))
+
+  interface Api {
+    readonly get: () => Effect.Effect<string>
+  }
+
+  class Test extends ServiceMap.Service<Test, Api>()("@test/HighContention") {
+    static readonly layer = Layer.effect(
+      Test,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((ctx) => Effect.sync(() => ctx.directory))
+
+        return Test.of({
+          get: Effect.fn("Test.get")(function* () {
+            // Interleave many async hops to maximize chance of ALS corruption
+            for (let i = 0; i < 10; i++) {
+              yield* Effect.promise(() => Bun.sleep(Math.random() * 3))
+              yield* Effect.yieldNow
+              yield* Effect.promise(() => Promise.resolve())
+            }
+            return yield* InstanceState.get(state)
+          }),
+        })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(Test.layer)
+
+  try {
+    const results = await Promise.all(
+      dirs.map((d) =>
+        Instance.provide({
+          directory: d.path,
+          fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+        }),
+      ),
+    )
+
+    for (let i = 0; i < N; i++) {
+      expect(results[i]).toBe(dirs[i].path)
+    }
+  } finally {
+    await rt.dispose()
+    for (const d of dirs) await d[Symbol.asyncDispose]()
+  }
+})
+
+test("InstanceState correct after interleaved init and dispose", async () => {
+  await using one = await tmpdir()
+  await using two = await tmpdir()
+
+  interface Api {
+    readonly get: () => Effect.Effect<string>
+  }
+
+  class Test extends ServiceMap.Service<Test, Api>()("@test/InterleavedDispose") {
+    static readonly layer = Layer.effect(
+      Test,
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make((ctx) =>
+          Effect.promise(async () => {
+            await Bun.sleep(5) // slow init
+            return ctx.directory
+          }),
+        )
+
+        return Test.of({
+          get: Effect.fn("Test.get")(function* () {
+            return yield* InstanceState.get(state)
+          }),
+        })
+      }),
+    )
+  }
+
+  const rt = ManagedRuntime.make(Test.layer)
+
+  try {
+    // Init both directories
+    const a = await Instance.provide({
+      directory: one.path,
+      fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+    })
+    expect(a).toBe(one.path)
+
+    // Dispose one directory, access the other concurrently
+    const [, b] = await Promise.all([
+      Instance.reload({ directory: one.path }),
+      Instance.provide({
+        directory: two.path,
+        fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+      }),
+    ])
+    expect(b).toBe(two.path)
+
+    // Re-access disposed directory - should get fresh state
+    const c = await Instance.provide({
+      directory: one.path,
+      fn: () => rt.runPromise(Test.use((svc) => svc.get())),
+    })
+    expect(c).toBe(one.path)
+  } finally {
+    await rt.dispose()
+  }
+})
+
+test("InstanceState mutation in one directory does not leak to another", async () => {
+  await using one = await tmpdir()
+  await using two = await tmpdir()
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make(() => Effect.sync(() => ({ count: 0 })))
+
+        // Mutate state in directory one
+        const s1 = yield* Effect.promise(() => access(state, one.path))
+        s1.count = 42
+
+        // Access directory two — should be independent
+        const s2 = yield* Effect.promise(() => access(state, two.path))
+        expect(s2.count).toBe(0)
+
+        // Confirm directory one still has the mutation
+        const s1again = yield* Effect.promise(() => access(state, one.path))
+        expect(s1again.count).toBe(42)
+        expect(s1again).toBe(s1) // same reference
+      }),
+    ),
+  )
+})
+
+test("InstanceState dedupes concurrent lookups", async () => {
+  await using tmp = await tmpdir()
+  let n = 0
+
+  await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const state = yield* InstanceState.make(() =>
+          Effect.promise(async () => {
+            n += 1
+            await Bun.sleep(10)
+            return { n }
+          }),
+        )
+
+        const [a, b] = yield* Effect.promise(() => Promise.all([access(state, tmp.path), access(state, tmp.path)]))
+        expect(a).toBe(b)
+        expect(n).toBe(1)
+      }),
+    ),
+  )
+})

packages/opencode/test/effect/run-service.test.ts

@@ -0,0 +1,46 @@
+import { expect, test } from "bun:test"
+import { Effect, Layer, ServiceMap } from "effect"
+import { makeRunPromise } from "../../src/effect/run-service"
+
+class Shared extends ServiceMap.Service<Shared, { readonly id: number }>()("@test/Shared") {}
+
+test("makeRunPromise shares dependent layers through the shared memo map", async () => {
+  let n = 0
+
+  const shared = Layer.effect(
+    Shared,
+    Effect.sync(() => {
+      n += 1
+      return Shared.of({ id: n })
+    }),
+  )
+
+  class One extends ServiceMap.Service<One, { readonly get: () => Effect.Effect<number> }>()("@test/One") {}
+  const one = Layer.effect(
+    One,
+    Effect.gen(function* () {
+      const svc = yield* Shared
+      return One.of({
+        get: Effect.fn("One.get")(() => Effect.succeed(svc.id)),
+      })
+    }),
+  ).pipe(Layer.provide(shared))
+
+  class Two extends ServiceMap.Service<Two, { readonly get: () => Effect.Effect<number> }>()("@test/Two") {}
+  const two = Layer.effect(
+    Two,
+    Effect.gen(function* () {
+      const svc = yield* Shared
+      return Two.of({
+        get: Effect.fn("Two.get")(() => Effect.succeed(svc.id)),
+      })
+    }),
+  ).pipe(Layer.provide(shared))
+
+  const runOne = makeRunPromise(One, one)
+  const runTwo = makeRunPromise(Two, two)
+
+  expect(await runOne((svc) => svc.get())).toBe(1)
+  expect(await runTwo((svc) => svc.get())).toBe(1)
+  expect(n).toBe(1)
+})

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

@@ -1,128 +0,0 @@
-import { afterEach, describe, expect, test } from "bun:test"
-import { Effect } from "effect"
-import { runtime, runPromiseInstance } from "../../src/effect/runtime"
-import { Auth } from "../../src/auth/effect"
-import { Instances } from "../../src/effect/instances"
-import { Instance } from "../../src/project/instance"
-import { ProviderAuth } from "../../src/provider/auth"
-import { Vcs } from "../../src/project/vcs"
-import { Question } from "../../src/question"
-import { tmpdir } from "../fixture/fixture"
-
-/**
- * Integration tests for the Effect runtime and LayerMap-based instance system.
- *
- * Each instance service layer has `.pipe(Layer.fresh)` at its definition site
- * so it is always rebuilt per directory, while shared dependencies are provided
- * outside the fresh boundary and remain memoizable.
- *
- * These tests verify the invariants using object identity (===) on the real
- * production services — not mock services or return-value checks.
- */
-
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const grabInstance = (service: any) => runPromiseInstance(service.use(Effect.succeed))
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const grabGlobal = (service: any) => runtime.runPromise(service.use(Effect.succeed))
-
-describe("effect/runtime", () => {
-  afterEach(async () => {
-    await Instance.disposeAll()
-  })
-
-  test("global services are shared across directories", async () => {
-    await using one = await tmpdir({ git: true })
-    await using two = await tmpdir({ git: true })
-
-    // Auth is a global service — it should be the exact same object
-    // regardless of which directory we're in.
-    const authOne = await Instance.provide({
-      directory: one.path,
-      fn: () => grabGlobal(Auth.Service),
-    })
-
-    const authTwo = await Instance.provide({
-      directory: two.path,
-      fn: () => grabGlobal(Auth.Service),
-    })
-
-    expect(authOne).toBe(authTwo)
-  })
-
-  test("instance services with global deps share the global (ProviderAuth → Auth)", async () => {
-    await using one = await tmpdir({ git: true })
-    await using two = await tmpdir({ git: true })
-
-    // ProviderAuth depends on Auth via defaultLayer.
-    // The instance service itself should be different per directory,
-    // but the underlying Auth should be shared.
-    const paOne = await Instance.provide({
-      directory: one.path,
-      fn: () => grabInstance(ProviderAuth.Service),
-    })
-
-    const paTwo = await Instance.provide({
-      directory: two.path,
-      fn: () => grabInstance(ProviderAuth.Service),
-    })
-
-    // Different directories → different ProviderAuth instances.
-    expect(paOne).not.toBe(paTwo)
-
-    // But the global Auth is the same object in both.
-    const authOne = await Instance.provide({
-      directory: one.path,
-      fn: () => grabGlobal(Auth.Service),
-    })
-    const authTwo = await Instance.provide({
-      directory: two.path,
-      fn: () => grabGlobal(Auth.Service),
-    })
-    expect(authOne).toBe(authTwo)
-  })
-
-  test("instance services are shared within the same directory", async () => {
-    await using tmp = await tmpdir({ git: true })
-
-    await Instance.provide({
-      directory: tmp.path,
-      fn: async () => {
-        expect(await grabInstance(Vcs.Service)).toBe(await grabInstance(Vcs.Service))
-        expect(await grabInstance(Question.Service)).toBe(await grabInstance(Question.Service))
-      },
-    })
-  })
-
-  test("different directories get different service instances", async () => {
-    await using one = await tmpdir({ git: true })
-    await using two = await tmpdir({ git: true })
-
-    const vcsOne = await Instance.provide({
-      directory: one.path,
-      fn: () => grabInstance(Vcs.Service),
-    })
-
-    const vcsTwo = await Instance.provide({
-      directory: two.path,
-      fn: () => grabInstance(Vcs.Service),
-    })
-
-    expect(vcsOne).not.toBe(vcsTwo)
-  })
-
-  test("disposal rebuilds services with a new instance", async () => {
-    await using tmp = await tmpdir({ git: true })
-
-    await Instance.provide({
-      directory: tmp.path,
-      fn: async () => {
-        const before = await grabInstance(Question.Service)
-
-        await runtime.runPromise(Instances.use((map) => map.invalidate(Instance.directory)))
-
-        const after = await grabInstance(Question.Service)
-        expect(after).not.toBe(before)
-      },
-    })
-  })
-})