refactor(question): destroy Question facade (#22092)

packages/opencode/src/question/index.ts

@@ -2,7 +2,6 @@ import { Deferred, Effect, Layer, Schema, Context } from "effect"
 import { Bus } from "@/bus"
 import { BusEvent } from "@/bus/bus-event"
 import { InstanceState } from "@/effect/instance-state"
-import { makeRuntime } from "@/effect/run-service"
 import { SessionID, MessageID } from "@/session/schema"
 import { Log } from "@/util/log"
 import z from "zod"
@@ -199,26 +198,4 @@ export namespace Question {
   )
 
   export const defaultLayer = layer.pipe(Layer.provide(Bus.layer))
-
-  const { runPromise } = makeRuntime(Service, defaultLayer)
-
-  export async function ask(input: {
-    sessionID: SessionID
-    questions: Info[]
-    tool?: { messageID: MessageID; callID: string }
-  }): Promise<Answer[]> {
-    return runPromise((s) => s.ask(input))
-  }
-
-  export async function reply(input: { requestID: QuestionID; answers: Answer[] }) {
-    return runPromise((s) => s.reply(input))
-  }
-
-  export async function reject(requestID: QuestionID) {
-    return runPromise((s) => s.reject(requestID))
-  }
-
-  export async function list() {
-    return runPromise((s) => s.list())
-  }
 }

packages/opencode/src/server/instance/question.ts

@@ -3,6 +3,7 @@ import { describeRoute, validator } from "hono-openapi"
 import { resolver } from "hono-openapi"
 import { QuestionID } from "@/question/schema"
 import { Question } from "../../question"
+import { AppRuntime } from "@/effect/app-runtime"
 import z from "zod"
 import { errors } from "../error"
 import { lazy } from "../../util/lazy"
@@ -27,7 +28,7 @@ export const QuestionRoutes = lazy(() =>
         },
       }),
       async (c) => {
-        const questions = await Question.list()
+        const questions = await AppRuntime.runPromise(Question.Service.use((svc) => svc.list()))
         return c.json(questions)
       },
     )
@@ -59,10 +60,14 @@ export const QuestionRoutes = lazy(() =>
       async (c) => {
         const params = c.req.valid("param")
         const json = c.req.valid("json")
-        await Question.reply({
-          requestID: params.requestID,
-          answers: json.answers,
-        })
+        await AppRuntime.runPromise(
+          Question.Service.use((svc) =>
+            svc.reply({
+              requestID: params.requestID,
+              answers: json.answers,
+            }),
+          ),
+        )
         return c.json(true)
       },
     )
@@ -92,7 +97,7 @@ export const QuestionRoutes = lazy(() =>
       ),
       async (c) => {
         const params = c.req.valid("param")
-        await Question.reject(params.requestID)
+        await AppRuntime.runPromise(Question.Service.use((svc) => svc.reject(params.requestID)))
         return c.json(true)
       },
     ),

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

@@ -4,6 +4,20 @@ import { Instance } from "../../src/project/instance"
 import { QuestionID } from "../../src/question/schema"
 import { tmpdir } from "../fixture/fixture"
 import { SessionID } from "../../src/session/schema"
+import { AppRuntime } from "../../src/effect/app-runtime"
+
+const ask = (input: {
+  sessionID: SessionID
+  questions: Question.Info[]
+  tool?: { messageID: any; callID: string }
+}) => AppRuntime.runPromise(Question.Service.use((svc) => svc.ask(input)))
+
+const list = () => AppRuntime.runPromise(Question.Service.use((svc) => svc.list()))
+
+const reply = (input: { requestID: QuestionID; answers: Question.Answer[] }) =>
+  AppRuntime.runPromise(Question.Service.use((svc) => svc.reply(input)))
+
+const reject = (id: QuestionID) => AppRuntime.runPromise(Question.Service.use((svc) => svc.reject(id)))
 
 afterEach(async () => {
   await Instance.disposeAll()
@@ -11,9 +25,9 @@ afterEach(async () => {
 
 /** Reject all pending questions so dangling Deferred fibers don't hang the test. */
 async function rejectAll() {
-  const pending = await Question.list()
+  const pending = await list()
   for (const req of pending) {
-    await Question.reject(req.id)
+    await reject(req.id)
   }
 }
 
@@ -22,7 +36,7 @@ test("ask - returns pending promise", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const promise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions: [
           {
@@ -58,16 +72,16 @@ test("ask - adds to pending list", async () => {
         },
       ]
 
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions,
       })
 
-      const pending = await Question.list()
+      const pending = await list()
       expect(pending.length).toBe(1)
       expect(pending[0].questions).toEqual(questions)
       await rejectAll()
-      await askPromise.catch(() => {})
+      await promise.catch(() => {})
     },
   })
 })
@@ -90,20 +104,20 @@ test("reply - resolves the pending ask with answers", async () => {
         },
       ]
 
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions,
       })
 
-      const pending = await Question.list()
+      const pending = await list()
       const requestID = pending[0].id
 
-      await Question.reply({
+      await reply({
         requestID,
         answers: [["Option 1"]],
       })
 
-      const answers = await askPromise
+      const answers = await promise
       expect(answers).toEqual([["Option 1"]])
     },
   })
@@ -114,7 +128,7 @@ test("reply - removes from pending list", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions: [
           {
@@ -128,17 +142,17 @@ test("reply - removes from pending list", async () => {
         ],
       })
 
-      const pending = await Question.list()
+      const pending = await list()
       expect(pending.length).toBe(1)
 
-      await Question.reply({
+      await reply({
         requestID: pending[0].id,
         answers: [["Option 1"]],
       })
-      await askPromise
+      await promise
 
-      const pendingAfter = await Question.list()
-      expect(pendingAfter.length).toBe(0)
+      const after = await list()
+      expect(after.length).toBe(0)
     },
   })
 })
@@ -148,7 +162,7 @@ test("reply - does nothing for unknown requestID", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      await Question.reply({
+      await reply({
         requestID: QuestionID.make("que_unknown"),
         answers: [["Option 1"]],
       })
@@ -164,7 +178,7 @@ test("reject - throws RejectedError", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions: [
           {
@@ -178,10 +192,10 @@ test("reject - throws RejectedError", async () => {
         ],
       })
 
-      const pending = await Question.list()
-      await Question.reject(pending[0].id)
+      const pending = await list()
+      await reject(pending[0].id)
 
-      await expect(askPromise).rejects.toBeInstanceOf(Question.RejectedError)
+      await expect(promise).rejects.toBeInstanceOf(Question.RejectedError)
     },
   })
 })
@@ -191,7 +205,7 @@ test("reject - removes from pending list", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions: [
           {
@@ -205,14 +219,14 @@ test("reject - removes from pending list", async () => {
         ],
       })
 
-      const pending = await Question.list()
+      const pending = await list()
       expect(pending.length).toBe(1)
 
-      await Question.reject(pending[0].id)
-      askPromise.catch(() => {}) // Ignore rejection
+      await reject(pending[0].id)
+      promise.catch(() => {}) // Ignore rejection
 
-      const pendingAfter = await Question.list()
-      expect(pendingAfter.length).toBe(0)
+      const after = await list()
+      expect(after.length).toBe(0)
     },
   })
 })
@@ -222,7 +236,7 @@ test("reject - does nothing for unknown requestID", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      await Question.reject(QuestionID.make("que_unknown"))
+      await reject(QuestionID.make("que_unknown"))
       // Should not throw
     },
   })
@@ -254,19 +268,19 @@ test("ask - handles multiple questions", async () => {
         },
       ]
 
-      const askPromise = Question.ask({
+      const promise = ask({
         sessionID: SessionID.make("ses_test"),
         questions,
       })
 
-      const pending = await Question.list()
+      const pending = await list()
 
-      await Question.reply({
+      await reply({
         requestID: pending[0].id,
         answers: [["Build"], ["Dev"]],
       })
 
-      const answers = await askPromise
+      const answers = await promise
       expect(answers).toEqual([["Build"], ["Dev"]])
     },
   })
@@ -279,7 +293,7 @@ test("list - returns all pending requests", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const p1 = Question.ask({
+      const p1 = ask({
         sessionID: SessionID.make("ses_test1"),
         questions: [
           {
@@ -290,7 +304,7 @@ test("list - returns all pending requests", async () => {
         ],
       })
 
-      const p2 = Question.ask({
+      const p2 = ask({
         sessionID: SessionID.make("ses_test2"),
         questions: [
           {
@@ -301,7 +315,7 @@ test("list - returns all pending requests", async () => {
         ],
       })
 
-      const pending = await Question.list()
+      const pending = await list()
       expect(pending.length).toBe(2)
       await rejectAll()
       p1.catch(() => {})
@@ -315,7 +329,7 @@ test("list - returns empty when no pending", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const pending = await Question.list()
+      const pending = await list()
       expect(pending.length).toBe(0)
     },
   })
@@ -328,7 +342,7 @@ test("questions stay isolated by directory", async () => {
   const p1 = Instance.provide({
     directory: one.path,
     fn: () =>
-      Question.ask({
+      ask({
         sessionID: SessionID.make("ses_one"),
         questions: [
           {
@@ -343,7 +357,7 @@ test("questions stay isolated by directory", async () => {
   const p2 = Instance.provide({
     directory: two.path,
     fn: () =>
-      Question.ask({
+      ask({
         sessionID: SessionID.make("ses_two"),
         questions: [
           {
@@ -357,11 +371,11 @@ test("questions stay isolated by directory", async () => {
 
   const onePending = await Instance.provide({
     directory: one.path,
-    fn: () => Question.list(),
+    fn: () => list(),
   })
   const twoPending = await Instance.provide({
     directory: two.path,
-    fn: () => Question.list(),
+    fn: () => list(),
   })
 
   expect(onePending.length).toBe(1)
@@ -371,11 +385,11 @@ test("questions stay isolated by directory", async () => {
 
   await Instance.provide({
     directory: one.path,
-    fn: () => Question.reject(onePending[0].id),
+    fn: () => reject(onePending[0].id),
   })
   await Instance.provide({
     directory: two.path,
-    fn: () => Question.reject(twoPending[0].id),
+    fn: () => reject(twoPending[0].id),
   })
 
   await p1.catch(() => {})
@@ -385,10 +399,10 @@ test("questions stay isolated by directory", async () => {
 test("pending question rejects on instance dispose", async () => {
   await using tmp = await tmpdir({ git: true })
 
-  const ask = Instance.provide({
+  const pending = Instance.provide({
     directory: tmp.path,
     fn: () => {
-      return Question.ask({
+      return ask({
         sessionID: SessionID.make("ses_dispose"),
         questions: [
           {
@@ -400,7 +414,7 @@ test("pending question rejects on instance dispose", async () => {
       })
     },
   })
-  const result = ask.then(
+  const result = pending.then(
     () => "resolved" as const,
     (err) => err,
   )
@@ -408,8 +422,8 @@ test("pending question rejects on instance dispose", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const pending = await Question.list()
-      expect(pending).toHaveLength(1)
+      const items = await list()
+      expect(items).toHaveLength(1)
       await Instance.dispose()
     },
   })
@@ -420,10 +434,10 @@ test("pending question rejects on instance dispose", async () => {
 test("pending question rejects on instance reload", async () => {
   await using tmp = await tmpdir({ git: true })
 
-  const ask = Instance.provide({
+  const pending = Instance.provide({
     directory: tmp.path,
     fn: () => {
-      return Question.ask({
+      return ask({
         sessionID: SessionID.make("ses_reload"),
         questions: [
           {
@@ -435,7 +449,7 @@ test("pending question rejects on instance reload", async () => {
       })
     },
   })
-  const result = ask.then(
+  const result = pending.then(
     () => "resolved" as const,
     (err) => err,
   )
@@ -443,8 +457,8 @@ test("pending question rejects on instance reload", async () => {
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
-      const pending = await Question.list()
-      expect(pending).toHaveLength(1)
+      const items = await list()
+      expect(items).toHaveLength(1)
       await Instance.reload({ directory: tmp.path })
     },
   })