refactor(question): effectify QuestionService (#17432)

packages/opencode/test/question/question.test.ts

@@ -5,6 +5,14 @@ import { QuestionID } from "../../src/question/schema"
 import { tmpdir } from "../fixture/fixture"
 import { SessionID } from "../../src/session/schema"
 
+/** Reject all pending questions so dangling Deferred fibers don't hang the test. */
+async function rejectAll() {
+  const pending = await Question.list()
+  for (const req of pending) {
+    await Question.reject(req.id)
+  }
+}
+
 test("ask - returns pending promise", async () => {
   await using tmp = await tmpdir({ git: true })
   await Instance.provide({
@@ -24,6 +32,8 @@ test("ask - returns pending promise", async () => {
         ],
       })
       expect(promise).toBeInstanceOf(Promise)
+      await rejectAll()
+      await promise.catch(() => {})
     },
   })
 })
@@ -44,7 +54,7 @@ test("ask - adds to pending list", async () => {
         },
       ]
 
-      Question.ask({
+      const askPromise = Question.ask({
         sessionID: SessionID.make("ses_test"),
         questions,
       })
@@ -52,6 +62,8 @@ test("ask - adds to pending list", async () => {
       const pending = await Question.list()
       expect(pending.length).toBe(1)
       expect(pending[0].questions).toEqual(questions)
+      await rejectAll()
+      await askPromise.catch(() => {})
     },
   })
 })
@@ -98,7 +110,7 @@ test("reply - removes from pending list", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      Question.ask({
+      const askPromise = Question.ask({
         sessionID: SessionID.make("ses_test"),
         questions: [
           {
@@ -119,6 +131,7 @@ test("reply - removes from pending list", async () => {
         requestID: pending[0].id,
         answers: [["Option 1"]],
       })
+      await askPromise
 
       const pendingAfter = await Question.list()
       expect(pendingAfter.length).toBe(0)
@@ -262,7 +275,7 @@ test("list - returns all pending requests", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      Question.ask({
+      const p1 = Question.ask({
         sessionID: SessionID.make("ses_test1"),
         questions: [
           {
@@ -273,7 +286,7 @@ test("list - returns all pending requests", async () => {
         ],
       })
 
-      Question.ask({
+      const p2 = Question.ask({
         sessionID: SessionID.make("ses_test2"),
         questions: [
           {
@@ -286,6 +299,9 @@ test("list - returns all pending requests", async () => {
 
       const pending = await Question.list()
       expect(pending.length).toBe(2)
+      await rejectAll()
+      p1.catch(() => {})
+      p2.catch(() => {})
     },
   })
 })

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

@@ -5,7 +5,7 @@ import { Instance } from "../../src/project/instance"
 import { InstanceState } from "../../src/util/instance-state"
 import { tmpdir } from "../fixture/fixture"
 
-async function access<A, E>(state: InstanceState.State<A, E>, dir: string) {
+async function access<A, E>(state: InstanceState<A, E>, dir: string) {
   return Instance.provide({
     directory: dir,
     fn: () => Effect.runPromise(InstanceState.get(state)),
@@ -23,9 +23,7 @@ test("InstanceState caches values for the same instance", async () => {
   await Effect.runPromise(
     Effect.scoped(
       Effect.gen(function* () {
-        const state = yield* InstanceState.make({
-          lookup: () => Effect.sync(() => ({ n: ++n })),
-        })
+        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))
@@ -45,9 +43,7 @@ test("InstanceState isolates values by directory", async () => {
   await Effect.runPromise(
     Effect.scoped(
       Effect.gen(function* () {
-        const state = yield* InstanceState.make({
-          lookup: (dir) => Effect.sync(() => ({ dir, n: ++n })),
-        })
+        const state = yield* InstanceState.make((dir) => Effect.sync(() => ({ dir, n: ++n })))
 
         const x = yield* Effect.promise(() => access(state, a.path))
         const y = yield* Effect.promise(() => access(state, b.path))
@@ -69,13 +65,12 @@ test("InstanceState is disposed on instance reload", async () => {
   await Effect.runPromise(
     Effect.scoped(
       Effect.gen(function* () {
-        const state = yield* InstanceState.make({
-          lookup: () => Effect.sync(() => ({ n: ++n })),
-          release: (value) =>
-            Effect.sync(() => {
-              seen.push(String(value.n))
-            }),
-        })
+        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 }))
@@ -96,13 +91,12 @@ test("InstanceState is disposed on disposeAll", async () => {
   await Effect.runPromise(
     Effect.scoped(
       Effect.gen(function* () {
-        const state = yield* InstanceState.make({
-          lookup: (dir) => Effect.sync(() => ({ dir })),
-          release: (value) =>
-            Effect.sync(() => {
-              seen.push(value.dir)
-            }),
-        })
+        const state = yield* InstanceState.make((dir) =>
+          Effect.acquireRelease(
+            Effect.sync(() => ({ dir })),
+            (value) => Effect.sync(() => { seen.push(value.dir) }),
+          ),
+        )
 
         yield* Effect.promise(() => access(state, a.path))
         yield* Effect.promise(() => access(state, b.path))
@@ -121,14 +115,13 @@ test("InstanceState dedupes concurrent lookups for the same directory", async ()
   await Effect.runPromise(
     Effect.scoped(
       Effect.gen(function* () {
-        const state = yield* InstanceState.make({
-          lookup: () =>
-            Effect.promise(async () => {
-              n += 1
-              await Bun.sleep(10)
-              return { n }
-            }),
-        })
+        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)