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(