Merge remote-tracking branch 'origin/main' into feature-read-xlsx-extraction

# Conflicts: # packages/opencode/src/tool/read.ts
2026-09-21 05:52:35 +08:00 · 2026-05-29 17:22:42 +02:00
parent f7830cb7e5 ebe04b9913
commit 277d09d636
30 changed files with 737 additions and 402 deletions
@@ -7,8 +7,7 @@ import { SessionID, MessageID, PartID } from "../../src/session/schema"
 import { ModelID, ProviderID } from "../../src/provider/schema"
 import { Instance } from "../../src/project/instance"
 import { WithInstance } from "../../src/project/with-instance"
-import { PlanFollowup } from "../../src/kilocode/plan-followup"
-import { Question } from "../../src/question"
+import { PlanFollowup, PlanFollowupRuntime } from "../../src/kilocode/plan-followup"
 import { Session } from "../../src/session/session"
 import { MessageV2 } from "../../src/session/message-v2"
 import { SessionPrompt } from "../../src/session/prompt"
@@ -123,7 +122,7 @@ async function seed(input: {

 async function waitQuestion(sessionID: string) {
  for (let i = 0; i < 50; i++) {
-    const list = await Question.list()
+    const list = await PlanFollowupRuntime.question.list()
    const question = list.find((item) => item.sessionID === sessionID)
    if (question) return question
    await Bun.sleep(10)
@@ -155,7 +154,7 @@ describe("plan_exit detection", () => {
      expect(question).toBeDefined()
      if (!question) return
      expect(question.questions[0].header).toBe("Implement")
-      await Question.reject(question.id)
+      await PlanFollowupRuntime.question.reject(question.id)
      await expect(pending).resolves.toBe("break")
    }))

@@ -194,7 +193,7 @@ describe("plan_exit detection", () => {
          PlanFollowup.ANSWER_CONTINUE,
        ])
        expect(question.questions[0].options.find((item) => item.label === PlanFollowup.ANSWER_CONTINUE)?.mode).toBe("code")
-        await Question.reject(question.id)
+        await PlanFollowupRuntime.question.reject(question.id)
        await expect(pending).resolves.toBe("break")
      } finally {
        if (prev === undefined) delete process.env.KILO_CLIENT
@@ -224,7 +223,7 @@ describe("plan_exit detection", () => {
      const question = await waitQuestion(seeded.sessionID)
      expect(question).toBeDefined()
      if (!question) return
-      await Question.reply({
+      await PlanFollowupRuntime.question.reply({
        requestID: question.id,
        answers: [[PlanFollowup.ANSWER_CONTINUE]],
      })
@@ -247,7 +246,7 @@ describe("plan_exit detection", () => {
        text: "Here is a partial plan, I have questions",
      })
      expect(SessionPrompt.shouldAskPlanFollowup({ messages: seeded.messages, abort: AbortSignal.any([]) })).toBe(false)
-      const list = await Question.list()
+      const list = await PlanFollowupRuntime.question.list()
      expect(list).toHaveLength(0)
    }))

@@ -326,7 +325,7 @@ describe("plan_exit detection", () => {
      expect(SessionPrompt.shouldAskPlanFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false)

      // Confirm no questions were posted
-      const list = await Question.list()
+      const list = await PlanFollowupRuntime.question.list()
      expect(list).toHaveLength(0)
    }))

@@ -440,7 +439,7 @@ describe("plan_exit detection", () => {
      const question = await waitQuestion(seeded.sessionID)
      expect(question).toBeDefined()
      if (!question) return
-      await Question.reply({
+      await PlanFollowupRuntime.question.reply({
        requestID: question.id,
        answers: [[PlanFollowup.ANSWER_CONTINUE]],
      })
@@ -543,7 +542,7 @@ describe("plan_exit detection", () => {
      expect(question).toBeDefined()
      if (!question) return
      expect(question.questions[0].header).toBe("Implement")
-      await Question.reply({
+      await PlanFollowupRuntime.question.reply({
        requestID: question.id,
        answers: [[PlanFollowup.ANSWER_CONTINUE]],
      })
@@ -36,9 +36,9 @@ describe("prompt.ts Kilo-specific invariants", () => {
    // an in-flight handle.process blocked on a pending tool prompt can return.
    // Critically, the block must NOT call state.cancel or KiloSessionPromptQueue.reserve —
    // either of those would abort the running streamText mid-tokens, which was
-    // the #9332 regression. Order: dismissAll(Suggestion) → dismissAll(Question) → enqueue.
+    // the #9332 regression. Order: dismissAll(Suggestion), question.dismissAll, enqueue.
    const block = content.match(
-      /kilocode_change start[^\n]*unblock tools[\s\S]*?Suggestion\.dismissAll[\s\S]*?Question\.dismissAll[\s\S]*?KiloSessionPromptQueue\.enqueue/,
+      /kilocode_change start[^\n]*unblock tools[\s\S]*?Suggestion\.dismissAll[\s\S]*?question\.dismissAll[\s\S]*?KiloSessionPromptQueue\.enqueue/,
    )
    expect(block).not.toBeNull()
    expect(content).not.toMatch(/state\.cancel\(input\.sessionID\)/)
@@ -1,174 +1,119 @@
-import { describe, expect, test } from "bun:test"
-import { Effect } from "effect"
+import { describe, expect } from "bun:test"
+import { Cause, Effect, Exit, Fiber, Layer } from "effect"
+import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
 import { KiloSessionPromptQueue } from "../../src/kilocode/session/prompt-queue"
-import { WithInstance } from "../../src/project/with-instance"
 import { Question } from "../../src/question"
 import { MessageID, SessionID } from "../../src/session/schema"
-import { tmpdir } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
+
+const it = testEffect(Layer.mergeAll(Question.defaultLayer, CrossSpawnSpawner.defaultLayer))
+
+const prompt = [
+  {
+    header: "Continue?",
+    question: "Should I continue?",
+    options: [
+      { label: "Yes", description: "Go" },
+      { label: "No", description: "Stop" },
+    ],
+  },
+]
+
+const waitFor = (question: Question.Interface, count: number) =>
+  Effect.gen(function* () {
+    for (let i = 0; i < 50; i++) {
+      const pending = yield* question.list()
+      if (pending.length >= count) return pending
+      yield* Effect.sleep("10 millis")
+    }
+    return yield* Effect.fail(new Error(`timed out waiting for ${count} pending question request(s)`))
+  })

 describe("Question.dismissAll", () => {
-  test("rejects pending asks for the target session and clears them", async () => {
-    await using tmp = await tmpdir({ git: true })
-    await WithInstance.provide({
-      directory: tmp.path,
-      fn: async () => {
+  it.instance(
+    "rejects pending asks for the target session and clears them",
+    () =>
+      Effect.gen(function* () {
+        const question = yield* Question.Service
        const sesA = SessionID.make("ses_a")
        const sesB = SessionID.make("ses_b")
+        const a1 = yield* question.ask({ sessionID: sesA, questions: prompt }).pipe(Effect.forkScoped)
+        const a2 = yield* question.ask({ sessionID: sesA, questions: prompt }).pipe(Effect.forkScoped)
+        const b1 = yield* question.ask({ sessionID: sesB, questions: prompt }).pipe(Effect.forkScoped)

-        const a1 = Question.ask({
-          sessionID: sesA,
-          questions: [
-            {
-              header: "Continue?",
-              question: "Should I continue?",
-              options: [
-                { label: "Yes", description: "Go" },
-                { label: "No", description: "Stop" },
-              ],
-            },
-          ],
-        }).catch((err) => {
-          if (err instanceof Question.RejectedError) return "rejected"
-          throw err
-        })
+        expect(yield* waitFor(question, 3)).toHaveLength(3)
+        yield* question.dismissAll(sesA)

-        const a2 = Question.ask({
-          sessionID: sesA,
-          questions: [
-            {
-              header: "Retry?",
-              question: "Try again?",
-              options: [
-                { label: "Retry", description: "Retry" },
-                { label: "Cancel", description: "Cancel" },
-              ],
-            },
-          ],
-        }).catch((err) => {
-          if (err instanceof Question.RejectedError) return "rejected"
-          throw err
-        })
-
-        const b1 = Question.ask({
-          sessionID: sesB,
-          questions: [
-            {
-              header: "Deploy?",
-              question: "Deploy now?",
-              options: [
-                { label: "Ship", description: "Ship" },
-                { label: "Wait", description: "Wait" },
-              ],
-            },
-          ],
-        }).catch((err) => {
-          if (err instanceof Question.RejectedError) return "rejected-b"
-          throw err
-        })
-
-        // Wait for all three asks to register so we can dismiss them.
-        for (let i = 0; i < 50; i++) {
-          if ((await Question.list()).length >= 3) break
-          await Bun.sleep(10)
+        for (const fiber of [a1, a2]) {
+          const exit = yield* Fiber.await(fiber)
+          expect(Exit.isFailure(exit)).toBe(true)
+          if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(Question.RejectedError)
        }
-        expect(await Question.list()).toHaveLength(3)

-        // Track whether B's promise settles.
-        let settled = false
-        b1.then(() => {
-          settled = true
-        })
+        yield* Effect.sleep("10 millis")

-        await Question.dismissAll("ses_a")
-
-        expect(await a1).toBe("rejected")
-        expect(await a2).toBe("rejected")
-
-        await new Promise((r) => setTimeout(r, 10))
-        expect(settled).toBe(false)
-
-        const remaining = await Question.list()
+        const remaining = yield* question.list()
        expect(remaining).toHaveLength(1)
        expect(remaining[0]?.sessionID).toBe(sesB)

-        await Question.reject(remaining[0]!.id)
-        expect(await b1).toBe("rejected-b")
-      },
-    })
-  })
+        yield* question.reject(remaining[0]!.id)
+        const exit = yield* Fiber.await(b1)
+        expect(Exit.isFailure(exit)).toBe(true)
+        if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(Question.RejectedError)
+      }),
+    { git: true },
+  )

-  test("is a no-op when no questions exist", async () => {
-    await using tmp = await tmpdir({ git: true })
-    await WithInstance.provide({
-      directory: tmp.path,
-      fn: async () => {
-        await Question.dismissAll("ses_missing")
-        expect(await Question.list()).toEqual([])
-      },
-    })
-  })
+  it.instance(
+    "is a no-op when no questions exist",
+    () =>
+      Effect.gen(function* () {
+        const question = yield* Question.Service
+        yield* question.dismissAll(SessionID.make("ses_missing"))
+        expect(yield* question.list()).toEqual([])
+      }),
+    { git: true },
+  )

-  test("ask rejects immediately when a followup is queued on the session", async () => {
-    // When a newer prompt has already been enqueued on the session, a tool
-    // that subsequently calls Question.ask would otherwise block the run until
-    // the user manually dismisses it. Verify the pre-emptive hasFollowup check
-    // rejects with RejectedError before any pending entry is registered.
-    await using tmp = await tmpdir({ git: true })
-    await WithInstance.provide({
-      directory: tmp.path,
-      fn: async () => {
+  it.instance(
+    "ask rejects immediately when a followup is queued on the session",
+    () =>
+      Effect.gen(function* () {
+        const question = yield* Question.Service
        const sessionID = SessionID.make("ses_auto_ask")
        const started = Promise.withResolvers<void>()
        const release = Promise.withResolvers<void>()

-        // Slot 1 stays running so activeSince is pinned to its seq.
-        const first = Effect.runPromise(
-          KiloSessionPromptQueue.enqueue(
-            sessionID,
-            MessageID.make("message_ask_1"),
-            Effect.gen(function* () {
-              started.resolve()
-              yield* Effect.promise(() => release.promise)
-              return "first" as const
-            }),
-            Effect.succeed("first-cancelled" as const),
-          ),
-        )
-        await started.promise
+        const first = yield* KiloSessionPromptQueue.enqueue(
+          sessionID,
+          MessageID.make("message_ask_1"),
+          Effect.gen(function* () {
+            started.resolve()
+            yield* Effect.promise(() => release.promise)
+            return "first" as const
+          }),
+          Effect.succeed("first-cancelled" as const),
+        ).pipe(Effect.forkScoped)
+        yield* Effect.promise(() => started.promise)

-        // Slot 2 arrives while slot 1 is active — latest > activeSince.
-        const second = Effect.runPromise(
-          KiloSessionPromptQueue.enqueue(
-            sessionID,
-            MessageID.make("message_ask_2"),
-            Effect.succeed("second" as const),
-            Effect.succeed("second-cancelled" as const),
-          ),
-        )
-        await Bun.sleep(10)
+        const second = yield* KiloSessionPromptQueue.enqueue(
+          sessionID,
+          MessageID.make("message_ask_2"),
+          Effect.succeed("second" as const),
+          Effect.succeed("second-cancelled" as const),
+        ).pipe(Effect.forkScoped)
+        yield* Effect.sleep("10 millis")
        expect(KiloSessionPromptQueue.hasFollowup(sessionID)).toBe(true)

-        await expect(
-          Question.ask({
-            sessionID,
-            questions: [
-              {
-                header: "Continue?",
-                question: "Should I continue?",
-                options: [
-                  { label: "Yes", description: "Go" },
-                  { label: "No", description: "Stop" },
-                ],
-              },
-            ],
-          }),
-        ).rejects.toBeInstanceOf(Question.RejectedError)
-        expect(await Question.list()).toEqual([])
+        const exit = yield* question.ask({ sessionID, questions: prompt }).pipe(Effect.exit)
+        expect(Exit.isFailure(exit)).toBe(true)
+        if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(Question.RejectedError)
+        expect(yield* question.list()).toEqual([])

        release.resolve()
-        expect(await first).toBe("first")
-        expect(await second).toBe("second")
-      },
-    })
-  })
+        expect(yield* Fiber.join(first)).toBe("first")
+        expect(yield* Fiber.join(second)).toBe("second")
+      }),
+    { git: true },
+  )
 })
@@ -0,0 +1,199 @@
+import { describe, expect } from "bun:test"
+import { Cause, Effect, Exit, Layer } from "effect"
+import path from "path"
+import { TextReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js"
+import { Agent } from "../../src/agent/agent"
+import * as CrossSpawnSpawner from "@opencode-ai/core/cross-spawn-spawner"
+import { AppFileSystem } from "@opencode-ai/core/filesystem"
+import { LSP } from "../../src/lsp/lsp"
+import { Instruction } from "../../src/session/instruction"
+import { MessageID, SessionID } from "../../src/session/schema"
+import { ReadTool } from "../../src/tool/read"
+import { Tool } from "../../src/tool/tool"
+import { Truncate } from "../../src/tool/truncate"
+import { provideInstance, tmpdirScoped } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
+
+const ctx: Tool.Context = {
+  sessionID: SessionID.make("ses_test-docx"),
+  messageID: MessageID.make(""),
+  callID: "",
+  agent: "code",
+  abort: AbortSignal.any([]),
+  messages: [],
+  metadata: () => Effect.void,
+  ask: () => Effect.void,
+}
+
+const expanded: Tool.Context = { ...ctx, extra: { includeDirectoryFiles: true } }
+
+const it = testEffect(
+  Layer.mergeAll(
+    Agent.defaultLayer,
+    AppFileSystem.defaultLayer,
+    CrossSpawnSpawner.defaultLayer,
+    Instruction.defaultLayer,
+    LSP.defaultLayer,
+    Truncate.defaultLayer,
+  ),
+)
+
+const init = Effect.fn("ReadDocxTest.init")(function* () {
+  const info = yield* ReadTool
+  return yield* Tool.init(info)
+})
+
+const run = Effect.fn("ReadDocxTest.run")(function* (
+  args: Tool.InferParameters<typeof ReadTool>,
+  next: Tool.Context = ctx,
+) {
+  const tool = yield* init()
+  return yield* tool.execute(args, next)
+})
+
+const exec = Effect.fn("ReadDocxTest.exec")(function* (
+  dir: string,
+  args: Tool.InferParameters<typeof ReadTool>,
+  next: Tool.Context = ctx,
+) {
+  return yield* provideInstance(dir)(run(args, next))
+})
+
+const fail = Effect.fn("ReadDocxTest.fail")(function* (dir: string, args: Tool.InferParameters<typeof ReadTool>) {
+  const exit = yield* exec(dir, args).pipe(Effect.exit)
+  if (Exit.isFailure(exit)) {
+    const err = Cause.squash(exit.cause)
+    return err instanceof Error ? err : new Error(String(err))
+  }
+  throw new Error("expected read to fail")
+})
+
+const put = Effect.fn("ReadDocxTest.put")(function* (filepath: string, content: string | Uint8Array) {
+  const fs = yield* AppFileSystem.Service
+  yield* fs.writeWithDirs(filepath, content)
+})
+
+const document = async (paragraphs: string[], extra = "") => {
+  const writer = new ZipWriter(new Uint8ArrayWriter())
+  await writer.add(
+    "[Content_Types].xml",
+    new TextReader(
+      '<?xml version="1.0" encoding="UTF-8" standalone="yes"?>' +
+        '<Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types">' +
+        '<Default Extension="rels" ContentType="application/vnd.openxmlformats-package.relationships+xml"/>' +
+        '<Default Extension="xml" ContentType="application/xml"/>' +
+        '<Override PartName="/word/document.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml"/>' +
+        "</Types>",
+    ),
+  )
+  await writer.add(
+    "_rels/.rels",
+    new TextReader(
+      '<?xml version="1.0" encoding="UTF-8" standalone="yes"?>' +
+        '<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships">' +
+        '<Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument" Target="word/document.xml"/>' +
+        "</Relationships>",
+    ),
+  )
+  await writer.add(
+    "word/document.xml",
+    new TextReader(
+      '<?xml version="1.0" encoding="UTF-8" standalone="yes"?>' +
+        '<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"><w:body>' +
+        paragraphs.map((text) => `<w:p><w:r><w:t>${text}</w:t></w:r></w:p>`).join("") +
+        extra +
+        "</w:body></w:document>",
+    ),
+  )
+  return writer.close()
+}
+
+describe("kilocode DOCX reads", () => {
+  it.live("extracts paragraph text from .docx and .DOCX files", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const bytes = yield* Effect.promise(() => document(["First paragraph", "Second paragraph"]))
+
+      for (const ext of ["docx", "DOCX"]) {
+        const filepath = path.join(dir, `sample.${ext}`)
+        yield* put(filepath, bytes)
+        const result = yield* exec(dir, { filePath: filepath })
+
+        expect(result.output).toContain("1: First paragraph")
+        expect(result.output).toContain("Second paragraph")
+        expect(result.attachments).toBeUndefined()
+      }
+    }),
+  )
+
+  it.live("applies normal read pagination to extracted text", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const filepath = path.join(dir, "paged.docx")
+      yield* put(filepath, yield* Effect.promise(() => document(["First paragraph", "Second paragraph"])))
+
+      const result = yield* exec(dir, { filePath: filepath, limit: 1 })
+
+      expect(result.output).toContain("1: First paragraph")
+      expect(result.output).not.toContain("Second paragraph")
+      expect(result.output).toContain("Use offset=2")
+      expect(result.metadata.truncated).toBe(true)
+    }),
+  )
+
+  it.live("fails clearly for malformed DOCX files", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const filepath = path.join(dir, "invalid.docx")
+      yield* put(filepath, new Uint8Array([0x50, 0x4b, 0x03, 0x04]))
+
+      const err = yield* fail(dir, { filePath: filepath })
+
+      expect(err.message).toContain("Failed to extract text from DOCX file")
+      expect(err.message).toContain(filepath)
+    }),
+  )
+
+  it.live("includes extraction warnings for unsupported document elements", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const filepath = path.join(dir, "warning.docx")
+      yield* put(filepath, yield* Effect.promise(() => document(["Readable text"], "<w:unsupported/>")))
+
+      const result = yield* exec(dir, { filePath: filepath })
+
+      expect(result.output).toContain("Readable text")
+      expect(result.output).toContain("DOCX extraction warnings")
+    }),
+  )
+
+  it.live("does not expand DOCX content in directory reads", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const folder = path.join(dir, "folder")
+      yield* put(path.join(folder, "sample.docx"), yield* Effect.promise(() => document(["Hidden paragraph"])))
+
+      const result = yield* exec(dir, { filePath: folder }, expanded)
+
+      expect(result.output).toContain("sample.docx")
+      expect(result.output).not.toContain("Hidden paragraph")
+    }),
+  )
+
+  it.live("preserves PDF attachments and rejects unsupported binary files", () =>
+    Effect.gen(function* () {
+      const dir = yield* tmpdirScoped()
+      const pdf = path.join(dir, "sample.pdf")
+      const doc = path.join(dir, "sample.doc")
+      yield* put(pdf, "%PDF-1.7\nfixture")
+      yield* put(doc, new Uint8Array([0x00, 0x01, 0x02]))
+
+      const result = yield* exec(dir, { filePath: pdf })
+      const err = yield* fail(dir, { filePath: doc })
+
+      expect(result.output).toBe("PDF read successfully")
+      expect(result.attachments?.[0].mime).toBe("application/pdf")
+      expect(err.message).toContain("Cannot read binary file")
+    }),
+  )
+})
@@ -165,6 +165,7 @@ function makeHttp() {
      Layer.provideMerge(proc),
      Layer.provideMerge(registry),
      Layer.provideMerge(trunc),
+      Layer.provideMerge(question), // kilocode_change - SessionPrompt now dismisses questions via its service dependency
      Layer.provide(Instruction.defaultLayer),
      Layer.provide(SystemPrompt.defaultLayer),
      Layer.provideMerge(deps),
@@ -158,6 +158,7 @@ function makeHttp() {
      Layer.provideMerge(proc),
      Layer.provideMerge(registry),
      Layer.provideMerge(trunc),
+      Layer.provideMerge(question), // kilocode_change - SessionPrompt now dismisses questions via its service dependency
      Layer.provide(Instruction.defaultLayer),
      Layer.provide(SystemPrompt.defaultLayer),
      Layer.provideMerge(deps),
@@ -4,7 +4,6 @@ import { Effect } from "effect"
 import { Bus } from "../../src/bus"
 import { KiloSessionPromptQueue } from "@/kilocode/session/prompt-queue"
 import { Suggestion } from "../../src/kilocode/suggestion"
-import { Question } from "../../src/question"
 import { ModelID, ProviderID } from "../../src/provider/schema"
 import { WithInstance } from "../../src/project/with-instance"
 import { Session } from "../../src/session/session"
@@ -662,62 +661,6 @@ describe("session prompt queue", () => {
    })
  })

-  test("new prompt dismisses a pending question", async () => {
-    const asked = Promise.withResolvers<void>()
-    const rejected = Promise.withResolvers<void>()
-    await using tmp = await tmpdir({ git: true })
-
-    await WithInstance.provide({
-      directory: tmp.path,
-      fn: async () =>
-        scoped(tmp.path, async (prompt) => {
-          const session = await sessions.create({ title: "Question unblock regression" })
-          const offAsked = Bus.subscribe(Question.Event.Asked, (event) => {
-            if (event.properties.sessionID === session.id) asked.resolve()
-          })
-          const offRejected = Bus.subscribe(Question.Event.Rejected, (event) => {
-            if (event.properties.sessionID === session.id) rejected.resolve()
-          })
-
-          try {
-            const pending = Question.ask({
-              sessionID: session.id,
-              questions: [
-                {
-                  header: "Continue?",
-                  question: "Should I continue?",
-                  options: [
-                    { label: "Yes", description: "Go ahead" },
-                    { label: "No", description: "Stop" },
-                  ],
-                },
-              ],
-            }).catch((err) => {
-              if (err instanceof Question.RejectedError) return "rejected"
-              throw err
-            })
-
-            await asked.promise
-            await Effect.runPromise(
-              prompt.prompt({
-                sessionID: session.id,
-                agent: "code",
-                parts: [{ type: "text", text: "replacement prompt" }],
-                noReply: true,
-              }),
-            )
-            await rejected.promise
-
-            expect(await pending).toBe("rejected")
-            expect(await Question.list()).toEqual([])
-          } finally {
-            offAsked()
-            offRejected()
-          }
-        }),
-    })
-  })
-
  test("auto-dismisses a suggestion shown after a queued prompt", async () => {
    // Reverse ordering of the "new prompt dismisses a pending suggestion" test:
    // queue the follow-up first, then open the blocker. Suggestion.show must see
@@ -783,70 +726,4 @@ describe("session prompt queue", () => {
    })
  })

-  test("auto-dismisses a question shown after a queued prompt", async () => {
-    await using tmp = await tmpdir({ git: true })
-    await WithInstance.provide({
-      directory: tmp.path,
-      fn: async () => {
-        const sessionID = SessionID.make("ses_auto_question")
-        const started = Promise.withResolvers<void>()
-        const release = Promise.withResolvers<void>()
-
-        const first = Effect.runPromise(
-          KiloSessionPromptQueue.enqueue(
-            sessionID,
-            MessageID.make("message_auto_q_1"),
-            Effect.gen(function* () {
-              started.resolve()
-              yield* Effect.promise(() => release.promise)
-              return "first" as const
-            }),
-            Effect.succeed("first-cancelled" as const),
-          ),
-        )
-        await started.promise
-
-        const second = Effect.runPromise(
-          KiloSessionPromptQueue.enqueue(
-            sessionID,
-            MessageID.make("message_auto_q_2"),
-            Effect.succeed("second" as const),
-            Effect.succeed("second-cancelled" as const),
-          ),
-        )
-        await Bun.sleep(10)
-        expect(KiloSessionPromptQueue.hasFollowup(sessionID)).toBe(true)
-
-        let asked = 0
-        const offAsked = Bus.subscribe(Question.Event.Asked, (event) => {
-          if (event.properties.sessionID === sessionID) asked++
-        })
-        try {
-          await expect(
-            Question.ask({
-              sessionID,
-              questions: [
-                {
-                  header: "Continue?",
-                  question: "Should I continue?",
-                  options: [
-                    { label: "Yes", description: "Go ahead" },
-                    { label: "No", description: "Stop" },
-                  ],
-                },
-              ],
-            }),
-          ).rejects.toBeInstanceOf(Question.RejectedError)
-        } finally {
-          offAsked()
-        }
-        expect(asked).toBe(0)
-        expect(await Question.list()).toEqual([])
-
-        release.resolve()
-        expect(await first).toBe("first")
-        expect(await second).toBe("second")
-      },
-    })
-  })
 })
@@ -6,6 +6,7 @@ import type { RemoteWS } from "../../../src/kilo-sessions/remote-ws"
 import type { RemoteProtocol } from "../../../src/kilo-sessions/remote-protocol"
 import type { SessionPrompt } from "../../../src/session/prompt"
 import { Question } from "../../../src/question"
+import { QuestionID } from "../../../src/question/schema"
 import { Permission } from "../../../src/permission"
 import { PermissionID } from "../../../src/permission/schema"
 import { ModelID, ProviderID } from "../../../src/provider/schema"
@@ -57,6 +58,14 @@ function permissions(items: Permission.Request[] = []) {
  }
 }

+function questions(items: Question.Request[] = []) {
+  return {
+    list: async () => items,
+    reply: async (_input: Parameters<Question.Interface["reply"]>[0]) => {},
+    reject: async (_requestID: QuestionID) => {},
+  }
+}
+
 function prompts(calls: SessionPrompt.PromptInput[]) {
  return async (input: SessionPrompt.PromptInput) => {
    calls.push(input)
@@ -461,15 +470,18 @@ describe("RemoteSender", () => {

  test("question_reply sends response after work completes", async () => {
    const { conn, sent } = fakeConn()
-    let provideCalled = false
+    const calls: Parameters<Question.Interface["reply"]>[0][] = []
    const sender = RemoteSender.create({
      conn,
      directory: "/tmp/test",
      log: nolog,
      subscribe: fakeBus().subscribe,
-      provide: async () => {
-        provideCalled = true
-        return {} as any
+      provide: async <R>(input: { directory: string; init?: Effect.Effect<void>; fn: () => R }) => input.fn(),
+      question: {
+        ...questions(),
+        reply: async (input) => {
+          calls.push(input)
+        },
      },
    })

@@ -480,12 +492,12 @@ describe("RemoteSender", () => {
      data: { requestID: "r1", answers: [["yes"]] },
    })

-    // Response not sent synchronously — waits for provide to finish
+    // Response not sent synchronously - waits for provide to finish.
    expect(sent).toHaveLength(0)

    await new Promise((r) => setTimeout(r, 10))

-    expect(provideCalled).toBe(true)
+    expect(calls).toEqual([{ requestID: QuestionID.make("r1"), answers: [["yes"]] }])
    expect(sent).toHaveLength(1)
    expect(sent[0]).toEqual({ type: "response", id: "req_q", result: {} })
  })
@@ -528,8 +540,12 @@ describe("RemoteSender", () => {
      directory: "/tmp/test",
      log: nolog,
      subscribe: fakeBus().subscribe,
-      provide: async () => {
-        throw new Error("boom")
+      provide: async <R>(input: { directory: string; init?: Effect.Effect<void>; fn: () => R }) => input.fn(),
+      question: {
+        ...questions(),
+        reply: async () => {
+          throw new Error("boom")
+        },
      },
    })

@@ -548,6 +564,37 @@ describe("RemoteSender", () => {
    expect(sent[0].error).toContain("boom")
  })

+  test("question_reply reports unknown request errors", async () => {
+    const { conn, sent } = fakeConn()
+    const sender = RemoteSender.create({
+      conn,
+      directory: "/tmp/test",
+      log: nolog,
+      subscribe: fakeBus().subscribe,
+      provide: async <R>(input: { directory: string; init?: Effect.Effect<void>; fn: () => R }) => input.fn(),
+      question: {
+        ...questions(),
+        reply: async (input) => {
+          throw new Question.NotFoundError({ requestID: input.requestID })
+        },
+      },
+    })
+
+    sender.handle({
+      type: "command",
+      id: "req_q_missing",
+      command: "question_reply",
+      data: { requestID: "missing", answers: [["yes"]] },
+    })
+
+    await new Promise((r) => setTimeout(r, 10))
+
+    expect(sent).toHaveLength(1)
+    expect(sent[0].type).toBe("response")
+    expect(sent[0].id).toBe("req_q_missing")
+    expect(sent[0].error).toContain("Question.NotFoundError")
+  })
+
  test("suggestion_accept sends response after work completes", async () => {
    const { conn, sent } = fakeConn()
    const accept = spyOn(Suggestion, "accept").mockResolvedValue(true)
@@ -595,15 +642,18 @@ describe("RemoteSender", () => {

  test("question_reject sends response after work completes", async () => {
    const { conn, sent } = fakeConn()
-    let provideCalled = false
+    const calls: QuestionID[] = []
    const sender = RemoteSender.create({
      conn,
      directory: "/tmp/test",
      log: nolog,
      subscribe: fakeBus().subscribe,
-      provide: async () => {
-        provideCalled = true
-        return {} as any
+      provide: async <R>(input: { directory: string; init?: Effect.Effect<void>; fn: () => R }) => input.fn(),
+      question: {
+        ...questions(),
+        reject: async (requestID) => {
+          calls.push(requestID)
+        },
      },
    })

@@ -616,11 +666,42 @@ describe("RemoteSender", () => {

    await new Promise((r) => setTimeout(r, 10))

-    expect(provideCalled).toBe(true)
+    expect(calls).toEqual([QuestionID.make("r1")])
    expect(sent).toHaveLength(1)
    expect(sent[0]).toEqual({ type: "response", id: "req_qr", result: {} })
  })

+  test("question_reject reports unknown request errors", async () => {
+    const { conn, sent } = fakeConn()
+    const sender = RemoteSender.create({
+      conn,
+      directory: "/tmp/test",
+      log: nolog,
+      subscribe: fakeBus().subscribe,
+      provide: async <R>(input: { directory: string; init?: Effect.Effect<void>; fn: () => R }) => input.fn(),
+      question: {
+        ...questions(),
+        reject: async (requestID) => {
+          throw new Question.NotFoundError({ requestID })
+        },
+      },
+    })
+
+    sender.handle({
+      type: "command",
+      id: "req_qr_missing",
+      command: "question_reject",
+      data: { requestID: "missing" },
+    })
+
+    await new Promise((r) => setTimeout(r, 10))
+
+    expect(sent).toHaveLength(1)
+    expect(sent[0].type).toBe("response")
+    expect(sent[0].id).toBe("req_qr_missing")
+    expect(sent[0].error).toContain("Question.NotFoundError")
+  })
+
  test("question_reject with invalid data sends error response", () => {
    const { conn, sent } = fakeConn()
    const sender = RemoteSender.create({
@@ -917,10 +998,6 @@ describe("RemoteSender", () => {
    const bus = fakeBus()

    spyOn(Suggestion, "list").mockResolvedValue([])
-    spyOn(Question, "list").mockResolvedValue([
-      { id: "question_1", sessionID: "ses_target", questions: [{ type: "text", text: "Continue?" }] } as any,
-      { id: "question_2", sessionID: "ses_other", questions: [{ type: "text", text: "Unrelated?" }] } as any,
-    ])

    const sender = RemoteSender.create({
      conn,
@@ -929,6 +1006,10 @@ describe("RemoteSender", () => {
      subscribe: bus.subscribe,
      provide: async (input: any) => input.fn(),
      permission: permissions(),
+      question: questions([
+        { id: "question_1", sessionID: "ses_target", questions: [{ type: "text", text: "Continue?" }] } as any,
+        { id: "question_2", sessionID: "ses_other", questions: [{ type: "text", text: "Unrelated?" }] } as any,
+      ]),
    })

    sender.handle({ type: "subscribe", sessionId: "ses_target" })
@@ -949,7 +1030,6 @@ describe("RemoteSender", () => {
    const bus = fakeBus()

    spyOn(Suggestion, "list").mockResolvedValue([])
-    spyOn(Question, "list").mockResolvedValue([])

    const sender = RemoteSender.create({
      conn,
@@ -957,6 +1037,7 @@ describe("RemoteSender", () => {
      log: nolog,
      subscribe: bus.subscribe,
      provide: async (input: any) => input.fn(),
+      question: questions(),
      permission: permissions([
        {
          id: "permission_1",
@@ -1004,7 +1085,6 @@ describe("RemoteSender", () => {
    spyOn(Suggestion, "list").mockResolvedValue([
      { id: "sug_1", sessionID: "ses_other", text: "Review?", actions: [] } as any,
    ])
-    spyOn(Question, "list").mockResolvedValue([{ id: "question_1", sessionID: "ses_other", questions: [] } as any])

    const sender = RemoteSender.create({
      conn,
@@ -1012,6 +1092,7 @@ describe("RemoteSender", () => {
      log: nolog,
      subscribe: bus.subscribe,
      provide: async (input: any) => input.fn(),
+      question: questions([{ id: "question_1", sessionID: "ses_other", questions: [] } as any]),
      permission: permissions([
        {
          id: "permission_1",
@@ -1049,7 +1130,6 @@ describe("RemoteSender", () => {
        actions: [{ label: "Skip", prompt: "skip" }],
      } as any,
    ])
-    spyOn(Question, "list").mockResolvedValue([])

    const sender = RemoteSender.create({
      conn,
@@ -1058,6 +1138,7 @@ describe("RemoteSender", () => {
      subscribe: bus.subscribe,
      provide: async (input: any) => input.fn(),
      permission: permissions(),
+      question: questions(),
    })

    sender.handle({ type: "subscribe", sessionId: "ses_target" })
@@ -1,11 +1,11 @@
-import { afterEach, expect, test } from "bun:test"
+import { afterEach, expect } from "bun:test" // kilocode_change - blocking behavior now uses the scoped service test helper
 import { Cause, Effect, Exit, Fiber, Layer } from "effect"
 import { Question } from "../../src/question"
 import { Instance } from "../../src/project/instance"
 import { WithInstance } from "../../src/project/with-instance"
 import { InstanceRuntime } from "../../src/project/instance-runtime"
 import { QuestionID } from "../../src/question/schema"
-import { disposeAllInstances, provideInstance, reloadTestInstance, tmpdir, tmpdirScoped } from "../fixture/fixture"
+import { disposeAllInstances, provideInstance, reloadTestInstance, tmpdirScoped } from "../fixture/fixture" // kilocode_change - blocking coverage no longer uses the Promise facade fixture
 import { SessionID } from "../../src/session/schema"
 import { testEffect } from "../lib/effect"
 import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
@@ -15,6 +15,7 @@ const it = testEffect(Layer.mergeAll(Question.defaultLayer, CrossSpawnSpawner.de
 const askEffect = Effect.fn("QuestionTest.ask")(function* (input: {
  sessionID: SessionID
  questions: ReadonlyArray<Question.Info>
+  blocking?: boolean // kilocode_change
  tool?: Question.Tool
 }) {
  const question = yield* Question.Service
@@ -110,12 +111,11 @@ it.instance(
 )

 // kilocode_change start - review follow-up uses non-blocking question prompts
-test("ask - preserves blocking flag", async () => {
-  await using tmp = await tmpdir({ git: true })
-  await WithInstance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      const askPromise = Question.ask({
+it.instance(
+  "ask - preserves blocking flag",
+  () =>
+    Effect.gen(function* () {
+      const fiber = yield* askEffect({
        sessionID: SessionID.make("ses_test"),
        blocking: false,
        questions: [
@@ -125,16 +125,18 @@ test("ask - preserves blocking flag", async () => {
            options: [{ label: "Start", description: "Run review" }],
          },
        ],
-      })
+      }).pipe(Effect.forkScoped)

-      const pending = await Question.list()
+      const pending = yield* waitForPending(1)
      expect(pending[0]?.blocking).toBe(false)

-      await Question.reject(pending[0].id)
-      await expect(askPromise).rejects.toBeInstanceOf(Question.RejectedError)
-    },
-  })
-})
+      yield* rejectEffect(pending[0].id)
+      const exit = yield* Fiber.await(fiber)
+      expect(Exit.isFailure(exit)).toBe(true)
+      if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(Question.RejectedError)
+    }),
+  { git: true },
+)
 // kilocode_change end

 // reply tests
@@ -205,15 +207,22 @@ it.instance(
  { git: true },
 )

+// kilocode_change start - preserve upstream unknown-request failure behavior during facade migration
 it.instance(
-  "reply - does nothing for unknown requestID",
+  "reply - fails for unknown requestID",
  () =>
-    replyEffect({
-      requestID: QuestionID.make("que_unknown"),
-      answers: [["Option 1"]],
+    Effect.gen(function* () {
+      const id = QuestionID.make("que_unknown")
+      const exit = yield* replyEffect({ requestID: id, answers: [["Option 1"]] }).pipe(Effect.exit)
+      expect(Exit.isFailure(exit)).toBe(true)
+      if (!Exit.isFailure(exit)) return
+      const err = Cause.squash(exit.cause)
+      expect(err).toBeInstanceOf(Question.NotFoundError)
+      if (err instanceof Question.NotFoundError) expect(err.requestID).toBe(id)
    }),
  { git: true },
 )
+// kilocode_change end

 // reject tests

@@ -275,9 +284,22 @@ it.instance(
  { git: true },
 )

-it.instance("reject - does nothing for unknown requestID", () => rejectEffect(QuestionID.make("que_unknown")), {
-  git: true,
-})
+// kilocode_change start - preserve upstream unknown-request failure behavior during facade migration
+it.instance(
+  "reject - fails for unknown requestID",
+  () =>
+    Effect.gen(function* () {
+      const id = QuestionID.make("que_unknown")
+      const exit = yield* rejectEffect(id).pipe(Effect.exit)
+      expect(Exit.isFailure(exit)).toBe(true)
+      if (!Exit.isFailure(exit)) return
+      const err = Cause.squash(exit.cause)
+      expect(err).toBeInstanceOf(Question.NotFoundError)
+      if (err instanceof Question.NotFoundError) expect(err.requestID).toBe(id)
+    }),
+  { git: true },
+)
+// kilocode_change end

 // multiple questions tests

@@ -213,6 +213,7 @@ function makeHttp() {
      Layer.provideMerge(proc),
      Layer.provideMerge(registry),
      Layer.provideMerge(trunc),
+      Layer.provideMerge(question), // kilocode_change - SessionPrompt now dismisses questions via its service dependency
      Layer.provide(Instruction.defaultLayer),
      Layer.provide(SystemPrompt.defaultLayer),
      Layer.provideMerge(deps),
@@ -396,6 +397,51 @@ it.live("loop calls LLM and returns assistant message", () =>
  ),
 )

+// kilocode_change start - replacement prompts unblock pending Question service requests
+it.live("new prompt dismisses a pending question", () =>
+  provideTmpdirServer(
+    Effect.fnUntraced(function* () {
+      const prompt = yield* SessionPrompt.Service
+      const sessions = yield* Session.Service
+      const question = yield* Question.Service
+      const chat = yield* sessions.create({ title: "Question unblock regression" })
+      const pending = yield* question
+        .ask({
+          sessionID: chat.id,
+          questions: [
+            {
+              header: "Continue?",
+              question: "Should I continue?",
+              options: [
+                { label: "Yes", description: "Go ahead" },
+                { label: "No", description: "Stop" },
+              ],
+            },
+          ],
+        })
+        .pipe(Effect.forkScoped)
+      yield* waitFor(
+        "pending question",
+        question.list().pipe(Effect.map((items) => items.find((item) => item.sessionID === chat.id))),
+      )
+
+      yield* prompt.prompt({
+        sessionID: chat.id,
+        agent: "build",
+        parts: [{ type: "text", text: "replacement prompt" }],
+        noReply: true,
+      })
+
+      const exit = yield* Fiber.await(pending)
+      expect(Exit.isFailure(exit)).toBe(true)
+      if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(Question.RejectedError)
+      expect(yield* question.list()).toEqual([])
+    }),
+    { git: true, config: providerCfg },
+  ),
+)
+// kilocode_change end
+
 it.live("prompt emits v2 prompted and synthetic events", () =>
  provideTmpdirServer(
    Effect.fnUntraced(function* () {
@@ -150,6 +150,7 @@ function makeHttp() {
      Layer.provideMerge(proc),
      Layer.provideMerge(registry),
      Layer.provideMerge(trunc),
+      Layer.provideMerge(question), // kilocode_change - SessionPrompt now dismisses questions via its service dependency
      Layer.provide(Instruction.defaultLayer),
      Layer.provide(SystemPrompt.defaultLayer),
      Layer.provideMerge(deps),