From 3070c7a13252883c6210590dadf97ca89fb32653 Mon Sep 17 00:00:00 2001 From: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:03:39 +0000 Subject: [PATCH] fix(question): reject empty questions array instead of hanging Both Question.ask (v1) and QuestionV2.ask registered a pending request for an empty questions array, leaving nothing to render or answer so the session waited indefinitely. Fail fast before registering the pending deferred, on both the existing question tool and the V2 core question tool paths. Closes #41549 --- packages/core/src/question.ts | 38 +++++++++++-------- packages/core/test/question.test.ts | 12 +++++- packages/opencode/src/question/index.ts | 5 +++ .../opencode/test/question/question.test.ts | 16 ++++++++ 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/packages/core/src/question.ts b/packages/core/src/question.ts index 79e0ea5e0384..14900791c2ff 100644 --- a/packages/core/src/question.ts +++ b/packages/core/src/question.ts @@ -6,6 +6,8 @@ import { Question } from "@opencode-ai/schema/question" import { EventV2 } from "./event" import { SessionSchema } from "./session/schema" +const EMPTY_QUESTIONS_ERROR = "QuestionV2.ask requires at least one question" + export const ID = Question.ID export type ID = typeof ID.Type @@ -91,22 +93,26 @@ const layer = Layer.effect( ) const ask = Effect.fn("QuestionV2.ask")((input: AskInput) => - Effect.uninterruptibleMask((restore) => - Effect.gen(function* () { - const id = ID.ascending() - const deferred = yield* Deferred.make, RejectedError>() - const request: Request = { id, ...input } - pending.set(id, { request, deferred }) - return yield* events.publish(Event.Asked, request).pipe( - Effect.andThen(restore(Deferred.await(deferred))), - Effect.ensuring( - Effect.sync(() => { - pending.delete(id) - }), - ), - ) - }), - ), + // An empty questions array has nothing to render or answer; registering a + // pending request for it would wait forever. Reject it up front instead. + input.questions.length === 0 + ? Effect.die(new Error(EMPTY_QUESTIONS_ERROR)) + : Effect.uninterruptibleMask((restore) => + Effect.gen(function* () { + const id = ID.ascending() + const deferred = yield* Deferred.make, RejectedError>() + const request: Request = { id, ...input } + pending.set(id, { request, deferred }) + return yield* events.publish(Event.Asked, request).pipe( + Effect.andThen(restore(Deferred.await(deferred))), + Effect.ensuring( + Effect.sync(() => { + pending.delete(id) + }), + ), + ) + }), + ), ) const reply = Effect.fn("QuestionV2.reply")((input: ReplyInput) => diff --git a/packages/core/test/question.test.ts b/packages/core/test/question.test.ts index 03d61a956507..ec4fa4a114c4 100644 --- a/packages/core/test/question.test.ts +++ b/packages/core/test/question.test.ts @@ -1,5 +1,5 @@ import { describe, expect } from "bun:test" -import { Context, Deferred, Effect, Exit, Fiber, Layer, Scope } from "effect" +import { Cause, Context, Deferred, Effect, Exit, Fiber, Layer, Scope } from "effect" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { EventV2 } from "@opencode-ai/core/event" @@ -111,4 +111,14 @@ describe("QuestionV2", () => { yield* Scope.close(secondScope, Exit.void) }), ) + + it.effect("fails fast when asked with no questions instead of registering a dead request", () => + Effect.gen(function* () { + const service = yield* QuestionV2.Service + const exit = yield* service.ask({ sessionID, questions: [] }).pipe(Effect.timeout("1 second"), Effect.exit) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.pretty(exit.cause)).toContain("at least one question") + expect(yield* service.list()).toEqual([]) + }), + ) }) diff --git a/packages/opencode/src/question/index.ts b/packages/opencode/src/question/index.ts index 8afc141072d5..b9d387241786 100644 --- a/packages/opencode/src/question/index.ts +++ b/packages/opencode/src/question/index.ts @@ -24,6 +24,8 @@ export const Replied = QuestionV1.Replied export const Rejected = QuestionV1.Rejected export const Event = QuestionV1.Event +const EMPTY_QUESTIONS_ERROR = "Question.ask requires at least one question" + export class RejectedError extends Schema.TaggedErrorClass()("QuestionRejectedError", {}) { override get message() { return "The user dismissed this question" @@ -89,6 +91,9 @@ const layer = Layer.effect( questions: ReadonlyArray tool?: Tool }) { + // An empty questions array has nothing to render or answer; registering a + // pending request for it would wait forever. Reject it up front instead. + if (input.questions.length === 0) return yield* Effect.die(new Error(EMPTY_QUESTIONS_ERROR)) const pending = (yield* InstanceState.get(state)).pending const id = QuestionID.ascending() yield* Effect.logInfo("asking", { id, questions: input.questions.length }) diff --git a/packages/opencode/test/question/question.test.ts b/packages/opencode/test/question/question.test.ts index 9adb1e0c9ff7..cd9fc7d76ede 100644 --- a/packages/opencode/test/question/question.test.ts +++ b/packages/opencode/test/question/question.test.ts @@ -119,6 +119,22 @@ it.instance( { git: true }, ) +it.instance( + "ask - fails fast when questions is empty", + () => + Effect.gen(function* () { + const exit = yield* askEffect({ + sessionID: SessionID.make("ses_test"), + questions: [], + }).pipe(Effect.timeout("1 second"), Effect.exit) + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.pretty(exit.cause)).toContain("at least one question") + // An empty ask must not register a never-resolving pending request. + expect(yield* listEffect).toHaveLength(0) + }), + { git: true }, +) + // reply tests it.instance(