diff --git a/packages/kilo-vscode/src/kilo-provider-utils.ts b/packages/kilo-vscode/src/kilo-provider-utils.ts index d409aedcc37..e9b04debfd2 100644 --- a/packages/kilo-vscode/src/kilo-provider-utils.ts +++ b/packages/kilo-vscode/src/kilo-provider-utils.ts @@ -167,7 +167,10 @@ export type WebviewMessage = } } | { type: "todoUpdated"; sessionID: string; items: unknown[] } - | { type: "questionRequest"; question: { id: string; sessionID: string; questions: unknown[]; tool?: unknown } } + | { + type: "questionRequest" + question: { id: string; sessionID: string; questions: unknown[]; blocking?: boolean; tool?: unknown } + } | { type: "questionResolved"; requestID: string } | { type: "sessionCreated"; session: ReturnType } | { type: "sessionUpdated"; session: ReturnType } @@ -241,6 +244,7 @@ export function mapSSEEventToWebviewMessage(event: Event, sessionID: string | un id: event.properties.id, sessionID: event.properties.sessionID, questions: event.properties.questions, + blocking: event.properties.blocking, tool: event.properties.tool, }, } diff --git a/packages/kilo-vscode/src/services/cli-backend/types.ts b/packages/kilo-vscode/src/services/cli-backend/types.ts index 48b8ab21c56..aa5e1b4efd2 100644 --- a/packages/kilo-vscode/src/services/cli-backend/types.ts +++ b/packages/kilo-vscode/src/services/cli-backend/types.ts @@ -5,7 +5,171 @@ // equivalents in @kilocode/sdk. All API types (Session, Event, Agent, // McpStatus, Config, etc.) should be imported from "@kilocode/sdk/v2/client". -/** Connection config used by the extension to reach the local CLI server */ +// Session status from SessionStatus.Info +export type SessionStatusInfo = + | { type: "idle" } + | { type: "retry"; attempt: number; message: string; next: number } + | { type: "busy" } + +// Token usage shape returned by the server on assistant messages +export interface TokenUsage { + input: number + output: number + reasoning?: number + cache?: { read: number; write: number } +} + +// Message types from MessageV2 +export interface MessageInfo { + id: string + sessionID: string + role: "user" | "assistant" + time: { + created: number + completed?: number + } + agent?: string + providerID?: string + modelID?: string + model?: { providerID: string; modelID: string } + mode?: string + parentID?: string + path?: { cwd: string; root: string } + error?: { name: string; data?: Record } + summary?: { title?: string; body?: string; diffs?: unknown[] } | boolean + cost?: number + tokens?: TokenUsage +} + +// Part types - simplified for UI display +export type MessagePart = + | { type: "text"; id: string; text: string } + | { type: "tool"; id: string; tool: string; state: ToolState } + | { type: "reasoning"; id: string; text: string } + +export type ToolState = + | { status: "pending"; input: Record } + | { status: "running"; input: Record; title?: string } + | { status: "completed"; input: Record; output: string; title: string } + | { status: "error"; input: Record; error: string } + +// Permission request from PermissionNext.Request +export interface PermissionRequest { + id: string + sessionID: string + permission: string + patterns: string[] + metadata: Record + always: string[] + tool?: { + messageID: string + callID: string + } +} + +// SSE Event types - based on BusEvent definitions +export type SSEEvent = + | { type: "server.connected"; properties: Record } + | { type: "server.heartbeat"; properties: Record } + | { type: "session.created"; properties: { info: SessionInfo } } + | { type: "session.updated"; properties: { info: SessionInfo } } + | { type: "session.status"; properties: { sessionID: string; status: SessionStatusInfo } } + | { type: "session.idle"; properties: { sessionID: string } } + | { type: "message.updated"; properties: { info: MessageInfo } } + | { type: "message.part.updated"; properties: { part: MessagePart; delta?: string } } + | { + type: "message.part.delta" + properties: { sessionID: string; messageID: string; partID: string; field: string; delta: string } + } + | { type: "permission.asked"; properties: PermissionRequest } + | { + type: "permission.replied" + properties: { sessionID: string; requestID: string; reply: "once" | "always" | "reject" } + } + | { type: "todo.updated"; properties: { sessionID: string; items: TodoItem[] } } + | { type: "question.asked"; properties: QuestionRequest } + | { type: "question.replied"; properties: { sessionID: string; requestID: string; answers: string[][] } } + | { type: "question.rejected"; properties: { sessionID: string; requestID: string } } + +export interface TodoItem { + id: string + content: string + status: "pending" | "in_progress" | "completed" +} + +// Question types from Question module +export interface QuestionOption { + label: string + description: string +} + +export interface QuestionInfo { + question: string + header: string + options: QuestionOption[] + multiple?: boolean + custom?: boolean +} + +export interface QuestionRequest { + id: string + sessionID: string + questions: QuestionInfo[] + blocking?: boolean + tool?: { + messageID: string + callID: string + } +} + +// Agent/mode info from the CLI /agent endpoint +export interface AgentInfo { + name: string + description?: string + mode: "subagent" | "primary" | "all" + native?: boolean + hidden?: boolean + color?: string +} + +// Provider/model types from provider catalog + +// Model definition from provider catalog +export interface ProviderModel { + id: string + name: string + inputPrice?: number + outputPrice?: number + contextLength?: number + releaseDate?: string + latest?: boolean + // Actual shape returned by the server (Provider.Model) + limit?: { context: number; input?: number; output: number } + variants?: Record> + capabilities?: { reasoning: boolean } +} + +// Provider definition +export interface Provider { + id: string + name: string + models: Record +} + +// Response from provider list endpoint +export interface ProviderListResponse { + all: Record + connected: string[] + default: Record // providerID → default modelID +} + +// Model selection (providerID + modelID pair) +export interface ModelSelection { + providerID: string + modelID: string +} + +// Server connection config export interface ServerConfig { baseUrl: string password: string diff --git a/packages/kilo-vscode/webview-ui/src/components/chat/ChatView.tsx b/packages/kilo-vscode/webview-ui/src/components/chat/ChatView.tsx index b5be42e3c01..baf3194a7db 100644 --- a/packages/kilo-vscode/webview-ui/src/components/chat/ChatView.tsx +++ b/packages/kilo-vscode/webview-ui/src/components/chat/ChatView.tsx @@ -26,11 +26,13 @@ export const ChatView: Component = (props) => { const hasMessages = () => session.messages().length > 0 const idle = () => session.status() !== "busy" const sessionQuestions = () => session.questions().filter((q) => q.sessionID === id()) + const blockingQuestions = () => sessionQuestions().filter((q) => q.blocking !== false) + const nonBlockingQuestions = () => sessionQuestions().filter((q) => q.blocking === false) const sessionPermissions = () => session.permissions().filter((p) => p.sessionID === id()) - const questionRequest = () => sessionQuestions().find((q) => !q.tool) + const questionRequest = () => blockingQuestions()[0] ?? nonBlockingQuestions()[0] const permissionRequest = () => sessionPermissions().find((p) => !p.tool) - const blocked = () => sessionPermissions().length > 0 || sessionQuestions().length > 0 + const blocked = () => sessionPermissions().length > 0 || blockingQuestions().length > 0 // When a bottom-dock permission/question disappears while the session is busy, // the scroll container grows taller. Dispatch a custom event so MessageList can diff --git a/packages/kilo-vscode/webview-ui/src/types/messages.ts b/packages/kilo-vscode/webview-ui/src/types/messages.ts index 4ac67521177..8652718f588 100644 --- a/packages/kilo-vscode/webview-ui/src/types/messages.ts +++ b/packages/kilo-vscode/webview-ui/src/types/messages.ts @@ -158,6 +158,7 @@ export interface QuestionRequest { id: string sessionID: string questions: QuestionInfo[] + blocking?: boolean tool?: { messageID: string callID: string diff --git a/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx b/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx index d60e51d0939..9b28b306280 100644 --- a/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx +++ b/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx @@ -138,6 +138,9 @@ export function Session() { if (session()?.parentID) return [] return children().flatMap((x) => sync.data.question[x.id] ?? []) }) + const blockingQuestions = createMemo(() => questions().filter((q) => q.blocking !== false)) // kilocode_change + const nonBlockingQuestions = createMemo(() => questions().filter((q) => q.blocking === false)) // kilocode_change + const question = createMemo(() => blockingQuestions()[0] ?? nonBlockingQuestions()[0]) // kilocode_change const pending = createMemo(() => { return messages().findLast((x) => x.role === "assistant" && !x.time.completed)?.id @@ -1123,11 +1126,17 @@ export function Session() { 0}> - 0}> - + + {(request) => ( + prompt?.focused ?? false} + /> + )} { prompt = r promptRef.set(r) @@ -1136,7 +1145,7 @@ export function Session() { r.set(route.initialPrompt) } }} - disabled={permissions().length > 0 || questions().length > 0} + disabled={permissions().length > 0 || blockingQuestions().length > 0} onSubmit={() => { toBottom() }} diff --git a/packages/opencode/src/cli/cmd/tui/routes/session/question.tsx b/packages/opencode/src/cli/cmd/tui/routes/session/question.tsx index fb5fc6f2373..01708385d80 100644 --- a/packages/opencode/src/cli/cmd/tui/routes/session/question.tsx +++ b/packages/opencode/src/cli/cmd/tui/routes/session/question.tsx @@ -10,7 +10,11 @@ import { SplitBorder } from "../../component/border" import { useTextareaKeybindings } from "../../component/textarea-keybindings" import { useDialog } from "../../ui/dialog" -export function QuestionPrompt(props: { request: QuestionRequest }) { +export function QuestionPrompt(props: { + request: QuestionRequest + nonBlocking?: boolean // kilocode_change + inputFocused?: () => boolean // kilocode_change +}) { const sdk = useSDK() const { theme } = useTheme() const keybind = useKeybind() @@ -126,6 +130,9 @@ export function QuestionPrompt(props: { request: QuestionRequest }) { // Skip processing if a dialog (e.g., command palette) is open if (dialog.stack.length > 0) return + // kilocode_change - avoid intrusive key capture for non-blocking review suggestions + if (props.nonBlocking && props.inputFocused?.()) return + // When editing custom answer textarea if (store.editing && !confirm()) { if (evt.name === "escape") { diff --git a/packages/opencode/src/kilocode/plan-followup.ts b/packages/opencode/src/kilocode/plan-followup.ts index ab11e366f0c..8cf2ac67298 100644 --- a/packages/opencode/src/kilocode/plan-followup.ts +++ b/packages/opencode/src/kilocode/plan-followup.ts @@ -108,6 +108,7 @@ export async function generateHandover(input: { export namespace PlanFollowup { const log = Log.create({ service: "plan.followup" }) + export const PLAN_PREFIX = "Implement the following plan:" export const ANSWER_NEW_SESSION = "Start new session" export const ANSWER_CONTINUE = "Continue here" @@ -216,7 +217,7 @@ export namespace PlanFollowup { Todo.get(input.sessionID), ]) - const sections = [`Implement the following plan:\n\n${input.plan}`] + const sections = [`${PLAN_PREFIX}\n\n${input.plan}`] if (handover) { sections.push(`## Handover from Planning Session\n\n${handover}`) diff --git a/packages/opencode/src/kilocode/review-followup.ts b/packages/opencode/src/kilocode/review-followup.ts new file mode 100644 index 00000000000..cc3208e1476 --- /dev/null +++ b/packages/opencode/src/kilocode/review-followup.ts @@ -0,0 +1,99 @@ +import { Flag } from "@/flag/flag" +import { Identifier } from "@/id/id" +import { Question } from "@/question" +import { Session } from "@/session" +import { MessageV2 } from "@/session/message-v2" +import { Review } from "@/kilocode/review/review" + +export namespace ReviewFollowup { + export const ANSWER_START = "Start code review" + export const ANSWER_SKIP = "Continue without review" + + async function inject(input: { sessionID: string; model: MessageV2.User["model"]; text: string }) { + const msg: MessageV2.User = { + id: Identifier.ascending("message"), + sessionID: input.sessionID, + role: "user", + time: { + created: Date.now(), + }, + agent: "code", + model: input.model, + } + await Session.updateMessage(msg) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: msg.id, + sessionID: input.sessionID, + type: "text", + text: input.text, + synthetic: true, + } satisfies MessageV2.TextPart) + } + + function prompt(input: { sessionID: string; abort: AbortSignal }) { + const promise = Question.ask({ + sessionID: input.sessionID, + blocking: Flag.KILO_CLIENT !== "vscode", + questions: [ + { + question: "Start an immediate review of uncommitted changes?", + header: "Code review", + custom: false, + options: [ + { + label: ANSWER_START, + description: "Run a local review for current uncommitted changes", + }, + { + label: ANSWER_SKIP, + description: "Dismiss the review suggestion and continue", + }, + ], + }, + ], + }) + + const listener = () => + Question.list().then((qs) => { + const match = qs.find((q) => q.sessionID === input.sessionID) + if (match) Question.reject(match.id) + }) + input.abort.addEventListener("abort", listener, { once: true }) + + return promise + .catch((error) => { + if (error instanceof Question.RejectedError) return undefined + throw error + }) + .finally(() => { + input.abort.removeEventListener("abort", listener) + }) + } + + export async function ask(input: { + sessionID: string + messages: MessageV2.WithParts[] + abort: AbortSignal + }): Promise<"continue" | "break"> { + if (input.abort.aborted) return "break" + + const user = input.messages + .slice() + .reverse() + .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 answer = answers?.[0]?.[0]?.trim() + if (answer !== ANSWER_START) return "break" + + const text = await Review.buildReviewPromptUncommitted() + await inject({ + sessionID: input.sessionID, + model: user.model, + text, + }) + return "continue" + } +} diff --git a/packages/opencode/src/question/index.ts b/packages/opencode/src/question/index.ts index 0fa1f2bfe41..fae0426bec7 100644 --- a/packages/opencode/src/question/index.ts +++ b/packages/opencode/src/question/index.ts @@ -40,6 +40,7 @@ export namespace Question { id: Identifier.schema("question"), sessionID: Identifier.schema("session"), questions: z.array(Info).describe("Questions to ask"), + blocking: z.boolean().optional().describe("Whether this question blocks prompt input (default: true)"), // kilocode_change tool: z .object({ messageID: z.string(), @@ -101,6 +102,7 @@ export namespace Question { export async function ask(input: { sessionID: string questions: Info[] + blocking?: boolean // kilocode_change tool?: { messageID: string; callID: string } }): Promise { const s = await state() @@ -113,6 +115,7 @@ export namespace Question { id, sessionID: input.sessionID, questions: input.questions, + blocking: input.blocking, // kilocode_change tool: input.tool, } s.pending[id] = { diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 17606be6ff1..81f3ccc4d4d 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -46,6 +46,7 @@ import { iife } from "@/util/iife" import { Shell } from "@/shell/shell" import { Truncate } from "@/tool/truncation" import { PlanFollowup } from "@/kilocode/plan-followup" // kilocode_change +import { ReviewFollowup } from "@/kilocode/review-followup" // kilocode_change // @ts-ignore globalThis.AI_SDK_LOG_WARNINGS = false @@ -72,6 +73,86 @@ export namespace SessionPrompt { msg.parts.some((p) => p.type === "tool" && p.tool === "plan_exit" && p.state.status === "completed"), ) } + + const reviewTools = new Set(["edit", "write", "multiedit", "apply_patch", "task"]) // kilocode_change + + // kilocode_change start - ask review follow-up only after first implementation turn per session + function reviewTurns(messages: MessageV2.WithParts[]) { + const ordered = messages.toSorted((a, b) => (a.info.id < b.info.id ? -1 : a.info.id > b.info.id ? 1 : 0)) + + const users = ordered.flatMap((msg, index) => + msg.info.role === "user" + ? [ + { + index, + user: msg.info as MessageV2.User, + }, + ] + : [], + ) + + return users.map((item, index) => ({ + user: item.user, + turn: ordered.slice(item.index + 1, users[index + 1]?.index ?? ordered.length), + })) + } + + function isImplementationTurn(input: { user: MessageV2.User; turn: MessageV2.WithParts[] }) { + if (!["code", "orchestrator"].includes(input.user.agent)) return false + + const hasPlanExit = input.turn.some((msg) => + msg.parts.some((part) => part.type === "tool" && part.tool === "plan_exit" && part.state.status === "completed"), + ) + if (hasPlanExit) return false + + return input.turn.some((msg) => + msg.parts.some((part) => part.type === "tool" && part.state.status === "completed" && reviewTools.has(part.tool)), + ) + } + + function hasPlanningContext(input: { turns: ReturnType; messages: MessageV2.WithParts[] }) { + // Same-session plan → code: a prior turn contains a completed plan_exit + const priorHasPlanExit = input.turns + .slice(0, -1) + .some((t) => + t.turn.some((msg) => + msg.parts.some((p) => p.type === "tool" && p.tool === "plan_exit" && p.state.status === "completed"), + ), + ) + if (priorHasPlanExit) return true + + // Cross-session handover: first user message starts with the plan prefix + const first = input.messages.find((m) => m.info.role === "user") + if (first) { + const text = first.parts + .filter((p): p is MessageV2.TextPart => p.type === "text" && !p.synthetic) + .map((p) => p.text) + .join("\n") + .trimStart() + if (text.startsWith(PlanFollowup.PLAN_PREFIX)) return true + } + + return false + } + // kilocode_change end + + // kilocode_change start - share review follow-up trigger logic with tests + export function shouldAskReviewFollowup(input: { messages: MessageV2.WithParts[]; abort: AbortSignal }) { + if (input.abort.aborted) return false + if (!["cli", "vscode"].includes(Flag.KILO_CLIENT)) return false + + const turns = reviewTurns(input.messages) + const latest = turns.at(-1) + if (!latest) return false + if (!isImplementationTurn(latest)) return false + + const alreadyImplemented = turns.slice(0, -1).some(isImplementationTurn) + if (alreadyImplemented) return false + + if (!hasPlanningContext({ turns, messages: input.messages })) return false + + return true + } // kilocode_change end const log = Log.create({ service: "session.prompt" }) @@ -364,6 +445,12 @@ export namespace SessionPrompt { const action = await PlanFollowup.ask({ sessionID, messages: msgs, abort }) if (action === "continue") continue } + // kilocode_change start - ask review follow-up after implementation turns + if (shouldAskReviewFollowup({ messages: msgs, abort })) { + const action = await ReviewFollowup.ask({ sessionID, messages: msgs, abort }) + if (action === "continue") continue + } + // kilocode_change end // kilocode_change end log.info("exiting loop", { sessionID }) break diff --git a/packages/opencode/test/kilocode/review-followup-detection.test.ts b/packages/opencode/test/kilocode/review-followup-detection.test.ts new file mode 100644 index 00000000000..dd6533fffc1 --- /dev/null +++ b/packages/opencode/test/kilocode/review-followup-detection.test.ts @@ -0,0 +1,491 @@ +import { describe, expect, test } from "bun:test" +import { Identifier } from "../../src/id/id" +import { PlanFollowup } from "../../src/kilocode/plan-followup" +import { Instance } from "../../src/project/instance" +import { Session } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { SessionPrompt } from "../../src/session/prompt" +import { Log } from "../../src/util/log" +import { tmpdir } from "../fixture/fixture" + +Log.init({ print: false }) + +const model = { + providerID: "openai", + modelID: "gpt-4", +} + +async function withInstance(fn: () => Promise) { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ directory: tmp.path, fn }) +} + +async function seed(input: { + agent: string + tools?: Array<{ tool: string; status?: MessageV2.ToolPart["state"]["status"] }> +}) { + const session = await Session.create({}) + const user = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: input.agent, + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: user.id, + sessionID: session.id, + type: "text", + text: "Do the work", + }) + + const assistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: user.id, + modelID: model.modelID, + providerID: model.providerID, + mode: input.agent, + agent: input.agent, + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(assistant) + + for (const tool of input.tools ?? []) { + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: assistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: tool.tool, + state: + tool.status === "error" + ? { + status: "error", + error: "boom", + input: {}, + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + } + : { + status: "completed", + input: {}, + output: "ok", + title: tool.tool, + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + } + + return Session.messages({ sessionID: session.id }) +} + +async function seedTwoImplementationTurns() { + const session = await Session.create({}) + + const firstUser = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: firstUser.id, + sessionID: session.id, + type: "text", + text: "Implement first step", + }) + + const firstAssistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: firstUser.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(firstAssistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: firstAssistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "edit", + state: { + status: "completed", + input: {}, + output: "ok", + title: "edit", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + const secondUser = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: secondUser.id, + sessionID: session.id, + type: "text", + text: "Implement second step", + }) + + const secondAssistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: secondUser.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(secondAssistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: secondAssistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "write", + state: { + status: "completed", + input: {}, + output: "ok", + title: "write", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + return Session.messages({ sessionID: session.id }) +} + +async function seedPlanThenImplementation() { + const session = await Session.create({}) + + // Turn 1: plan turn that ends with plan_exit + const planUser = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: planUser.id, + sessionID: session.id, + type: "text", + text: "Plan the feature", + }) + + const planAssistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: planUser.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(planAssistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: planAssistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "plan_exit", + state: { + status: "completed", + input: {}, + output: "ok", + title: "plan_exit", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + // Turn 2: implementation turn with edit tool + const implUser = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: implUser.id, + sessionID: session.id, + type: "text", + text: "Implement it", + }) + + const implAssistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: implUser.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(implAssistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: implAssistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "edit", + state: { + status: "completed", + input: {}, + output: "ok", + title: "edit", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + return Session.messages({ sessionID: session.id }) +} + +async function seedHandoverSession() { + const session = await Session.create({}) + + const user = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { created: Date.now() }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: user.id, + sessionID: session.id, + type: "text", + text: `${PlanFollowup.PLAN_PREFIX}\n\nStep 1: do something\nStep 2: do something else`, + }) + + const assistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { created: Date.now() }, + parentID: user.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + finish: "end_turn", + } + await Session.updateMessage(assistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: assistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "edit", + state: { + status: "completed", + input: {}, + output: "ok", + title: "edit", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + return Session.messages({ sessionID: session.id }) +} + +describe("review follow-up detection", () => { + test("does not trigger without plan context even with implementation tool", () => + withInstance(async () => { + const messages = await seed({ + agent: "code", + tools: [{ tool: "edit" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger for orchestrator turns without plan context", () => + withInstance(async () => { + const messages = await seed({ + agent: "orchestrator", + tools: [{ tool: "task" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger for orchestrator turns without implementation tools", () => + withInstance(async () => { + const messages = await seed({ + agent: "orchestrator", + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger for read-only turns", () => + withInstance(async () => { + const messages = await seed({ + agent: "code", + tools: [{ tool: "read" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger for non-implementation agents", () => + withInstance(async () => { + const messages = await seed({ + agent: "ask", + tools: [{ tool: "edit" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger when plan_exit exists in same turn", () => + withInstance(async () => { + const messages = await seed({ + agent: "code", + tools: [{ tool: "edit" }, { tool: "plan_exit" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger when implementation tool fails", () => + withInstance(async () => { + const messages = await seed({ + agent: "code", + tools: [{ tool: "edit", status: "error" }], + }) + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("does not trigger on later implementation turns in same session", () => + withInstance(async () => { + const messages = await seedTwoImplementationTurns() + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false) + })) + + test("triggers after same-session plan_exit followed by implementation turn", () => + withInstance(async () => { + const messages = await seedPlanThenImplementation() + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(true) + })) + + test("triggers when first user message starts with plan handover prefix", () => + withInstance(async () => { + const messages = await seedHandoverSession() + expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(true) + })) +}) diff --git a/packages/opencode/test/kilocode/review-followup.test.ts b/packages/opencode/test/kilocode/review-followup.test.ts new file mode 100644 index 00000000000..94e816cac61 --- /dev/null +++ b/packages/opencode/test/kilocode/review-followup.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, spyOn, test } from "bun:test" +import { Identifier } from "../../src/id/id" +import { ReviewFollowup } from "../../src/kilocode/review-followup" +import { Review } from "../../src/kilocode/review/review" +import { Instance } from "../../src/project/instance" +import { Question } from "../../src/question" +import { Session } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { Log } from "../../src/util/log" +import { tmpdir } from "../fixture/fixture" + +Log.init({ print: false }) + +const model = { + providerID: "openai", + modelID: "gpt-4", +} + +async function withInstance(fn: () => Promise) { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ directory: tmp.path, fn }) +} + +async function seed() { + const session = await Session.create({}) + const user = await Session.updateMessage({ + id: Identifier.ascending("message"), + role: "user", + sessionID: session.id, + time: { + created: Date.now(), + }, + agent: "code", + model, + }) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: user.id, + sessionID: session.id, + type: "text", + text: "Implement feature", + }) + + const assistant: MessageV2.Assistant = { + id: Identifier.ascending("message"), + role: "assistant", + sessionID: session.id, + time: { + created: Date.now(), + }, + parentID: user.id, + modelID: model.modelID, + providerID: model.providerID, + mode: "code", + agent: "code", + path: { + cwd: Instance.directory, + root: Instance.worktree, + }, + cost: 0, + tokens: { + total: 0, + input: 0, + output: 0, + reasoning: 0, + cache: { + read: 0, + write: 0, + }, + }, + finish: "end_turn", + } + await Session.updateMessage(assistant) + await Session.updatePart({ + id: Identifier.ascending("part"), + messageID: assistant.id, + sessionID: session.id, + type: "tool", + callID: Identifier.ascending("tool"), + tool: "edit", + state: { + status: "completed", + input: {}, + output: "done", + title: "edit", + metadata: {}, + time: { start: Date.now(), end: Date.now() }, + }, + } satisfies MessageV2.ToolPart) + + return { + sessionID: session.id, + messages: await Session.messages({ sessionID: session.id }), + } +} + +async function latestUser(sessionID: string) { + const messages = await Session.messages({ sessionID }) + return messages + .slice() + .reverse() + .find((item) => item.info.role === "user") +} + +describe("review follow-up", () => { + test("ask returns break when dismissed", () => + withInstance(async () => { + const seeded = await seed() + const pending = ReviewFollowup.ask({ + sessionID: seeded.sessionID, + messages: seeded.messages, + abort: AbortSignal.any([]), + }) + + const list = await Question.list() + expect(list).toHaveLength(1) + expect(list[0]?.blocking).toBe(false) + await Question.reject(list[0].id) + + await expect(pending).resolves.toBe("break") + })) + + test("ask injects review kickoff prompt when accepted", () => + withInstance(async () => { + const seeded = await seed() + const review = spyOn(Review, "buildReviewPromptUncommitted").mockResolvedValue("Run local review now") + await using _spy = { + [Symbol.dispose]() { + review.mockRestore() + }, + } + + const pending = ReviewFollowup.ask({ + sessionID: seeded.sessionID, + messages: seeded.messages, + abort: AbortSignal.any([]), + }) + + const list = await Question.list() + expect(list[0]?.blocking).toBe(false) + await Question.reply({ + requestID: list[0].id, + answers: [[ReviewFollowup.ANSWER_START]], + }) + + await expect(pending).resolves.toBe("continue") + expect(review).toHaveBeenCalledTimes(1) + + const user = await latestUser(seeded.sessionID) + expect(user?.info.role).toBe("user") + if (!user || user.info.role !== "user") return + expect(user.info.agent).toBe("code") + + const part = user.parts.find((item) => item.type === "text") + expect(part?.type).toBe("text") + if (!part || part.type !== "text") return + expect(part.text).toBe("Run local review now") + expect(part.synthetic).toBe(true) + })) +}) diff --git a/packages/opencode/test/question/question.test.ts b/packages/opencode/test/question/question.test.ts index cf24faa7d22..0b309c328a0 100644 --- a/packages/opencode/test/question/question.test.ts +++ b/packages/opencode/test/question/question.test.ts @@ -54,6 +54,33 @@ test("ask - adds to pending list", async () => { }) }) +test("ask - preserves blocking flag", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // kilocode_change - review follow-up uses non-blocking question prompts + const askPromise = Question.ask({ + sessionID: "ses_test", + blocking: false, + questions: [ + { + question: "Proceed with review suggestion?", + header: "Code review", + options: [{ label: "Start", description: "Run review" }], + }, + ], + }) + + const pending = await Question.list() + expect(pending[0]?.blocking).toBe(false) + + await Question.reject(pending[0].id) + await expect(askPromise).rejects.toBeInstanceOf(Question.RejectedError) + }, + }) +}) + // reply tests test("reply - resolves the pending ask with answers", async () => { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 5504e3b0459..3484f9de25d 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -658,6 +658,10 @@ export type QuestionRequest = { * Questions to ask */ questions: Array + /** + * Whether this question blocks prompt input (default: true) + */ + blocking?: boolean tool?: { messageID: string callID: string