fix(cli): resolve plan exit approval submissions (#10895)

* fix(cli): resolve plan exit approval submissions

* refactor(cli): remove plan followup question fallback
This commit is contained in:
Joshua Lambert
2026-06-04 06:08:33 -04:00
committed by GitHub
parent 38eb5879f5
commit 2e1945c287
7 changed files with 219 additions and 37 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---
Allow plan approval submissions to complete after planning finishes.
+20 -22
View File
@@ -26,7 +26,6 @@ import z from "zod"
const agents = lazy(() => makeRuntime(Agent.Service, Agent.defaultLayer))
const providers = lazy(() => makeRuntime(Provider.Service, Provider.defaultLayer))
const questions = lazy(() => makeRuntime(Question.Service, Question.defaultLayer))
const todo = lazy(() => makeRuntime(Todo.Service, Todo.defaultLayer))
const llm = lazy(() => makeRuntime(LLM.Service, LLM.defaultLayer))
const pending = new Map<SessionID, AbortController>()
@@ -38,20 +37,6 @@ export const PlanFollowupRuntime = {
model(providerID: ProviderID, modelID: ModelID): Promise<Provider.Model> {
return providers().runPromise((svc) => svc.getModel(providerID, modelID))
},
question: {
ask(input: Parameters<Question.Interface["ask"]>[0]) {
return questions().runPromise((svc) => svc.ask(input))
},
list() {
return questions().runPromise((svc) => svc.list())
},
reject(requestID: Parameters<Question.Interface["reject"]>[0]) {
return questions().runPromise((svc) => svc.reject(requestID))
},
reply(input: Parameters<Question.Interface["reply"]>[0]) {
return questions().runPromise((svc) => svc.reply(input))
},
},
todo: {
get(sessionID: SessionID) {
return todo().runPromise((svc) => svc.get(sessionID))
@@ -288,8 +273,15 @@ export namespace PlanFollowup {
return msg
}
function prompt(input: { sessionID: SessionID; abort: AbortSignal }) {
const promise = PlanFollowupRuntime.question.ask({
type QuestionRuntime = {
ask: (input: Parameters<Question.Interface["ask"]>[0]) => Promise<ReadonlyArray<Question.Answer>>
list: () => Promise<ReadonlyArray<Question.Request>>
reject: (requestID: Parameters<Question.Interface["reject"]>[0]) => Promise<void>
}
function prompt(input: { sessionID: SessionID; abort: AbortSignal; question: QuestionRuntime }) {
if (input.abort.aborted) return Promise.resolve(undefined)
const promise = input.question.ask({
sessionID: input.sessionID,
questions: [
{
@@ -323,10 +315,15 @@ export namespace PlanFollowup {
})
const listener = () =>
PlanFollowupRuntime.question.list().then((qs) => {
const match = qs.find((q) => q.sessionID === input.sessionID)
if (match) PlanFollowupRuntime.question.reject(match.id)
})
input.question
.list()
.then((qs) => {
const match = qs.find((q) => q.sessionID === input.sessionID)
return match ? input.question.reject(match.id) : undefined
})
.catch((error) => {
log.warn("failed to reject aborted plan follow-up question", { sessionID: input.sessionID, error })
})
input.abort.addEventListener("abort", listener, { once: true })
return promise
@@ -478,6 +475,7 @@ export namespace PlanFollowup {
sessionID: SessionID
messages: MessageV2.WithParts[]
abort: AbortSignal
question: QuestionRuntime
}): Promise<"continue" | "break"> {
if (input.abort.aborted) return "break"
@@ -491,7 +489,7 @@ export namespace PlanFollowup {
const user = latest.find((msg) => msg.info.role === "user")?.info
if (!user || user.role !== "user" || !user.model) return "break"
const answers = await prompt({ sessionID: input.sessionID, abort: input.abort })
const answers = await prompt({ sessionID: input.sessionID, abort: input.abort, question: input.question })
if (!answers) {
Telemetry.trackPlanFollowup(input.sessionID, "dismissed")
return "break"
@@ -14,6 +14,7 @@ import { PlanFollowup } from "@/kilocode/plan-followup"
import { KiloSession } from "@/kilocode/session"
import { KiloSessionMessageOrder } from "@/kilocode/session/message-order"
import { Permission } from "@/permission"
import { Question } from "@/question"
import { environmentDetails, type EditorContext } from "@/kilocode/editor-context"
import { Identifier } from "@/id/id"
import { Filesystem } from "@/util/filesystem"
@@ -48,6 +49,7 @@ export namespace KiloSessionPrompt {
sessionID: SessionID
messages: MessageV2.WithParts[]
abort: AbortSignal
question: Pick<Question.Interface, "ask" | "list" | "reject">
}): Promise<"continue" | "break"> {
if (!shouldAskPlanFollowup({ messages: input.messages, abort: input.abort })) return "break"
const ask = InstanceState.bind(PlanFollowup.ask)
@@ -55,6 +57,16 @@ export namespace KiloSessionPrompt {
sessionID: input.sessionID,
messages: input.messages,
abort: input.abort,
// Keep the request in the listener-local Question service so HTTP replies can resolve it.
question: {
ask: InstanceState.bind((request: Parameters<Question.Interface["ask"]>[0]) =>
Effect.runPromise(input.question.ask(request)),
),
list: InstanceState.bind(() => Effect.runPromise(input.question.list())),
reject: InstanceState.bind((requestID: Parameters<Question.Interface["reject"]>[0]) =>
Effect.runPromise(input.question.reject(requestID)),
),
},
})
return action === "continue" ? "continue" : "break"
}
+2 -2
View File
@@ -1544,7 +1544,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
KiloSessionPrompt.shouldAskPlanFollowup({ messages: msgs, abort: AbortSignal.any([]) })
) {
const action = yield* Effect.promise((signal) =>
KiloSessionPrompt.askPlanFollowup({ sessionID, messages: msgs, abort: signal }),
KiloSessionPrompt.askPlanFollowup({ sessionID, messages: msgs, abort: signal, question }),
)
if (action === "continue") continue
yield* slog.info("exiting loop")
@@ -1561,7 +1561,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
) {
// kilocode_change start - ask follow-up when plan_exit tool was called
const action = yield* Effect.promise((signal) =>
KiloSessionPrompt.askPlanFollowup({ sessionID, messages: msgs, abort: signal }),
KiloSessionPrompt.askPlanFollowup({ sessionID, messages: msgs, abort: signal, question }),
)
if (action === "continue") continue
// kilocode_change end
@@ -1,4 +1,5 @@
import { describe, expect, test } from "bun:test"
import { AsyncResource } from "async_hooks"
import { Effect } from "effect"
import fs from "fs/promises"
import path from "path"
@@ -7,7 +8,10 @@ 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, PlanFollowupRuntime } from "../../src/kilocode/plan-followup"
import { PlanFollowup } from "../../src/kilocode/plan-followup"
import { KiloSessionPrompt } from "../../src/kilocode/session/prompt"
import { makeRuntime } from "../../src/effect/run-service"
import { Question } from "../../src/question"
import { Session } from "../../src/session/session"
import { MessageV2 } from "../../src/session/message-v2"
import { SessionPrompt } from "../../src/session/prompt"
@@ -29,6 +33,22 @@ const sessions = {
Effect.runPromise(Session.Service.use((svc) => svc.updatePart(part)).pipe(Effect.provide(Session.defaultLayer))),
}
const runtime = makeRuntime(Question.Service, Question.defaultLayer)
const questions = {
ask(input: Parameters<Question.Interface["ask"]>[0]) {
return runtime.runPromise((svc) => svc.ask(input))
},
list() {
return runtime.runPromise((svc) => svc.list())
},
reject(requestID: Parameters<Question.Interface["reject"]>[0]) {
return runtime.runPromise((svc) => svc.reject(requestID))
},
reply(input: Parameters<Question.Interface["reply"]>[0]) {
return runtime.runPromise((svc) => svc.reply(input))
},
}
const model = {
providerID: ProviderID.make("openai"),
modelID: ModelID.make("gpt-4"),
@@ -122,7 +142,7 @@ async function seed(input: {
async function waitQuestion(sessionID: string) {
for (let i = 0; i < 50; i++) {
const list = await PlanFollowupRuntime.question.list()
const list = await questions.list()
const question = list.find((item) => item.sessionID === sessionID)
if (question) return question
await Bun.sleep(10)
@@ -145,6 +165,7 @@ describe("plan_exit detection", () => {
expect(SessionPrompt.shouldAskPlanFollowup({ messages: seeded.messages, abort: AbortSignal.any([]) })).toBe(true)
const pending = PlanFollowup.ask({
question: questions,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -154,10 +175,126 @@ describe("plan_exit detection", () => {
expect(question).toBeDefined()
if (!question) return
expect(question.questions[0].header).toBe("Implement")
await PlanFollowupRuntime.question.reject(question.id)
await questions.reject(question.id)
await expect(pending).resolves.toBe("break")
}))
test("KiloSessionPrompt resolves plan follow-up through the supplied question service", () =>
withInstance(async () => {
const seeded = await seed({
text: "Here is the plan",
tools: [
{
tool: "plan_exit",
input: {},
output: "Plan is ready. Ending planning turn.",
},
],
})
const result = await Effect.runPromise(
Effect.gen(function* () {
const question = yield* Question.Service
const pending = KiloSessionPrompt.askPlanFollowup({
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
question,
})
const item = yield* Effect.gen(function* () {
for (let i = 0; i < 50; i++) {
const request = (yield* question.list()).find((entry) => entry.sessionID === seeded.sessionID)
if (request) return request
yield* Effect.sleep("10 millis")
}
throw new Error("timed out waiting for listener-local plan follow-up question")
})
yield* question.reply({ requestID: item.id, answers: [[PlanFollowup.ANSWER_CONTINUE]] })
return yield* Effect.promise(() => pending)
}).pipe(Effect.provide(Question.defaultLayer)),
)
expect(result).toBe("continue")
}))
test("KiloSessionPrompt cleans listener-local plan follow-up when aborted outside instance context", () => {
const outside = new AsyncResource("plan-followup-abort-test")
return withInstance(async () => {
const seeded = await seed({
text: "Here is the plan",
tools: [
{
tool: "plan_exit",
input: {},
output: "Plan is ready. Ending planning turn.",
},
],
})
const result = await Effect.runPromise(
Effect.gen(function* () {
const question = yield* Question.Service
const abort = new AbortController()
const pending = KiloSessionPrompt.askPlanFollowup({
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: abort.signal,
question,
})
yield* Effect.gen(function* () {
for (let i = 0; i < 50; i++) {
const request = (yield* question.list()).find((entry) => entry.sessionID === seeded.sessionID)
if (request) return request
yield* Effect.sleep("10 millis")
}
throw new Error("timed out waiting for listener-local plan follow-up question")
})
outside.runInAsyncScope(() => abort.abort())
const action = yield* Effect.promise(() =>
Promise.race([pending, Bun.sleep(1_000).then(() => "timeout" as const)]),
)
expect(yield* question.list()).toEqual([])
return action
}).pipe(Effect.provide(Question.defaultLayer)),
)
expect(result).toBe("break")
}).finally(() => outside.emitDestroy())
})
test("PlanFollowup skips prompt when aborted while resolving the plan", () =>
withInstance(async () => {
const seeded = await seed({
text: "Here is the plan",
tools: [
{
tool: "plan_exit",
input: {},
output: "Plan is ready. Ending planning turn.",
},
],
})
const abort = new AbortController()
const pending = PlanFollowup.ask({
question: questions,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: abort.signal,
})
abort.abort()
const result = await Promise.race([pending, Bun.sleep(1_000).then(() => "timeout" as const)])
const list = () => questions.list().then((qs) => qs.filter((q) => q.sessionID === seeded.sessionID))
try {
expect(result).toBe("break")
expect(await list()).toEqual([])
} finally {
for (const item of await list()) {
await questions.reject(item.id)
}
}
}))
test("JetBrains client enables plan follow-up with custom answer", () =>
withInstance(async () => {
const prev = process.env.KILO_CLIENT
@@ -179,6 +316,7 @@ describe("plan_exit detection", () => {
)
const pending = PlanFollowup.ask({
question: questions,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -197,7 +335,7 @@ describe("plan_exit detection", () => {
expect(question.questions[0].options.find((item) => item.label === PlanFollowup.ANSWER_CONTINUE)?.mode).toBe(
"code",
)
await PlanFollowupRuntime.question.reject(question.id)
await questions.reject(question.id)
await expect(pending).resolves.toBe("break")
} finally {
if (prev === undefined) delete process.env.KILO_CLIENT
@@ -219,6 +357,7 @@ describe("plan_exit detection", () => {
})
const pending = PlanFollowup.ask({
question: questions,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -227,7 +366,7 @@ describe("plan_exit detection", () => {
const question = await waitQuestion(seeded.sessionID)
expect(question).toBeDefined()
if (!question) return
await PlanFollowupRuntime.question.reply({
await questions.reply({
requestID: question.id,
answers: [[PlanFollowup.ANSWER_CONTINUE]],
})
@@ -250,7 +389,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 PlanFollowupRuntime.question.list()
const list = await questions.list()
expect(list).toHaveLength(0)
}))
@@ -329,7 +468,7 @@ describe("plan_exit detection", () => {
expect(SessionPrompt.shouldAskPlanFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false)
// Confirm no questions were posted
const list = await PlanFollowupRuntime.question.list()
const list = await questions.list()
expect(list).toHaveLength(0)
}))
@@ -435,6 +574,7 @@ describe("plan_exit detection", () => {
await Bun.write(plan, "Do implementation step 1")
const pending = PlanFollowup.ask({
question: questions,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -443,7 +583,7 @@ describe("plan_exit detection", () => {
const question = await waitQuestion(seeded.sessionID)
expect(question).toBeDefined()
if (!question) return
await PlanFollowupRuntime.question.reply({
await questions.reply({
requestID: question.id,
answers: [[PlanFollowup.ANSWER_CONTINUE]],
})
@@ -537,6 +677,7 @@ describe("plan_exit detection", () => {
// PlanFollowup.ask should find plan text from the earlier assistant and show prompt
const pending = PlanFollowup.ask({
question: questions,
sessionID: session.id,
messages,
abort: AbortSignal.any([]),
@@ -546,7 +687,7 @@ describe("plan_exit detection", () => {
expect(question).toBeDefined()
if (!question) return
expect(question.questions[0].header).toBe("Implement")
await PlanFollowupRuntime.question.reply({
await questions.reply({
requestID: question.id,
answers: [[PlanFollowup.ANSWER_CONTINUE]],
})
@@ -14,6 +14,7 @@ import { Question } from "../../src/question"
import { Session } from "../../src/session/session"
import { MessageV2 } from "../../src/session/message-v2"
import { AppRuntime } from "../../src/effect/app-runtime"
import { makeRuntime } from "../../src/effect/run-service"
import { SessionStatus } from "../../src/session/status"
import { Todo } from "../../src/session/todo"
import { Global } from "@opencode-ai/core/global"
@@ -25,15 +26,19 @@ import { tmpdir } from "../fixture/fixture"
Log.init({ print: false })
process.env.KILO_CLIENT = "cli"
const runtime = makeRuntime(Question.Service, Question.defaultLayer)
const question = {
ask(input: Parameters<Question.Interface["ask"]>[0]) {
return runtime.runPromise((svc) => svc.ask(input))
},
list() {
return AppRuntime.runPromise(Question.Service.use((svc) => svc.list()))
return runtime.runPromise((svc) => svc.list())
},
reply(input: Parameters<Question.Interface["reply"]>[0]) {
return AppRuntime.runPromise(Question.Service.use((svc) => svc.reply(input)))
return runtime.runPromise((svc) => svc.reply(input))
},
reject(requestID: Parameters<Question.Interface["reject"]>[0]) {
return AppRuntime.runPromise(Question.Service.use((svc) => svc.reject(requestID)))
return runtime.runPromise((svc) => svc.reject(requestID))
},
}
@@ -266,6 +271,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Step one\n2. Step two" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -283,6 +289,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Build" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -312,6 +319,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Build" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -343,6 +351,7 @@ describe("plan follow-up", () => {
process.env.KILO_CLIENT = "vscode"
const seeded = await seed({ text: "1. Build" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -370,6 +379,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Build" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -428,6 +438,7 @@ describe("plan follow-up", () => {
}
const seeded = await seed({ text: "1. Build\n2. Test" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -460,6 +471,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Build\n2. Test" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -497,6 +509,7 @@ describe("plan follow-up", () => {
KiloSessionPromptQueue.retarget(seeded.sessionID, original)
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -613,6 +626,7 @@ describe("plan follow-up", () => {
})
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -719,6 +733,7 @@ describe("plan follow-up", () => {
fn: async () => sessions(),
})
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -795,6 +810,7 @@ describe("plan follow-up", () => {
}
const seeded = await seed({ text: "1. Build\n2. Test" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -847,6 +863,7 @@ describe("plan follow-up", () => {
}
const seeded = await seed({ text: "1. Build\n2. Test" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -882,6 +899,7 @@ describe("plan follow-up", () => {
}
const seeded = await seed({ text: "1. Build\n2. Test", variant: planVar })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -942,6 +960,7 @@ describe("plan follow-up", () => {
})
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -1031,6 +1050,7 @@ describe("plan follow-up", () => {
}
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -1107,6 +1127,7 @@ describe("plan follow-up", () => {
}
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -1205,6 +1226,7 @@ describe("plan follow-up", () => {
}
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -1246,6 +1268,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: " " })
const result = await PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
@@ -1261,6 +1284,7 @@ describe("plan follow-up", () => {
abort.abort()
const result = await PlanFollowup.ask({
question,
sessionID: SessionID.make("ses_test"),
messages: [],
abort: abort.signal,
@@ -1274,6 +1298,7 @@ describe("plan follow-up", () => {
const abort = new AbortController()
const seeded = await seed({ text: "1. Step one\n2. Step two" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: abort.signal,
@@ -1293,6 +1318,7 @@ describe("plan follow-up", () => {
withInstance(async () => {
const seeded = await seed({ text: "1. Build\n2. Test" })
const pending = PlanFollowup.ask({
question,
sessionID: seeded.sessionID,
messages: seeded.messages,
abort: AbortSignal.any([]),
+1 -1
View File
@@ -37,7 +37,7 @@ const testAllow: Record<string, { count: number; reason: string }> = {
"effect/app-runtime-logger.test.ts": { count: 6, reason: "tests AppRuntime behavior" },
"kilocode/config-resilience.test.ts": { count: 4, reason: "existing runtime integration test" },
"kilocode/config-validation.test.ts": { count: 2, reason: "existing runtime integration test" },
"kilocode/plan-followup.test.ts": { count: 7, reason: "existing runtime integration test" },
"kilocode/plan-followup.test.ts": { count: 4, reason: "existing runtime integration test" },
"kilocode/server/config-overlay.test.ts": { count: 3, reason: "server config cache integration test" },
"kilocode/session/platform-attribution.test.ts": { count: 5, reason: "existing runtime integration test" },
"kilocode/session-prompt-queue.test.ts": { count: 5, reason: "prompt queue legacy instance bridge regression" },