mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-21 05:52:35 +08:00
refactor(cli): remove Question compatibility facade
This commit is contained in:
@@ -6,8 +6,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"
|
||||
@@ -109,7 +108,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)
|
||||
@@ -141,7 +140,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")
|
||||
}))
|
||||
|
||||
@@ -180,7 +179,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
|
||||
@@ -210,7 +209,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]],
|
||||
})
|
||||
@@ -233,7 +232,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)
|
||||
}))
|
||||
|
||||
@@ -312,7 +311,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)
|
||||
}))
|
||||
|
||||
@@ -426,7 +425,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]],
|
||||
})
|
||||
@@ -529,7 +528,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 },
|
||||
)
|
||||
})
|
||||
|
||||
@@ -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"
|
||||
@@ -628,59 +627,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 () => {
|
||||
const session = await Session.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 SessionPrompt.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
|
||||
@@ -746,70 +692,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 { 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 { Suggestion } from "../../../src/kilocode/suggestion" // kilocode_change
|
||||
@@ -55,6 +56,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) => {},
|
||||
}
|
||||
}
|
||||
|
||||
// kilocode_change start
|
||||
afterEach(() => {
|
||||
mock.restore()
|
||||
@@ -444,15 +453,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)
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
@@ -463,12 +475,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: {} })
|
||||
})
|
||||
@@ -511,8 +523,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")
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
@@ -531,6 +547,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)
|
||||
@@ -578,15 +625,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)
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
@@ -599,11 +649,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({
|
||||
@@ -900,10 +981,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,
|
||||
@@ -912,6 +989,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" })
|
||||
@@ -932,7 +1013,6 @@ describe("RemoteSender", () => {
|
||||
const bus = fakeBus()
|
||||
|
||||
spyOn(Suggestion, "list").mockResolvedValue([])
|
||||
spyOn(Question, "list").mockResolvedValue([])
|
||||
|
||||
const sender = RemoteSender.create({
|
||||
conn,
|
||||
@@ -940,6 +1020,7 @@ describe("RemoteSender", () => {
|
||||
log: nolog,
|
||||
subscribe: bus.subscribe,
|
||||
provide: async (input: any) => input.fn(),
|
||||
question: questions(),
|
||||
permission: permissions([
|
||||
{
|
||||
id: "permission_1",
|
||||
@@ -987,7 +1068,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,
|
||||
@@ -995,6 +1075,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",
|
||||
@@ -1032,7 +1113,6 @@ describe("RemoteSender", () => {
|
||||
actions: [{ label: "Skip", prompt: "skip" }],
|
||||
} as any,
|
||||
])
|
||||
spyOn(Question, "list").mockResolvedValue([])
|
||||
|
||||
const sender = RemoteSender.create({
|
||||
conn,
|
||||
@@ -1041,6 +1121,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"
|
||||
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"
|
||||
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
|
||||
@@ -206,11 +208,16 @@ it.instance(
|
||||
)
|
||||
|
||||
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 },
|
||||
)
|
||||
@@ -275,9 +282,20 @@ it.instance(
|
||||
{ git: true },
|
||||
)
|
||||
|
||||
it.instance("reject - does nothing for unknown requestID", () => rejectEffect(QuestionID.make("que_unknown")), {
|
||||
git: true,
|
||||
})
|
||||
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 },
|
||||
)
|
||||
|
||||
// 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),
|
||||
|
||||
Reference in New Issue
Block a user