diff --git a/.changeset/local-review-base-branch.md b/.changeset/local-review-base-branch.md index f6a3b242724..6d3343137c1 100644 --- a/.changeset/local-review-base-branch.md +++ b/.changeset/local-review-base-branch.md @@ -2,4 +2,4 @@ "@kilocode/cli": patch --- -Ask which base branch `/local-review` should review against before starting the review. +Let `/local-review` accept optional input to choose a base branch or add review instructions. diff --git a/packages/opencode/src/kilocode/review/base.ts b/packages/opencode/src/kilocode/review/base.ts index 28a8dfecf08..ecc7a63419c 100644 --- a/packages/opencode/src/kilocode/review/base.ts +++ b/packages/opencode/src/kilocode/review/base.ts @@ -1,33 +1,53 @@ -import { Question } from "@/question" -import { SessionID } from "@/session/schema" import { Review } from "./review" +const argsRegex = /(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)/gi +const quoteTrimRegex = /^["']|["']$/g + export namespace ReviewBranch { - export async function resolve(input: { sessionID: SessionID }) { - const base = await Review.getBaseBranch() - const answers = await Question.ask({ - sessionID: input.sessionID, - blocking: true, - questions: [ - { - header: "Base branch", - question: "Which base branch should I review against?", - custom: true, - options: [ - { - label: base, - description: "Review the current branch against this base branch", - }, - ], - }, - ], - }) - const answer = answers[0]?.[0]?.trim() - return answer || base + export type Resolved = { + base?: string + instructions?: string } - export async function template(input: { sessionID: SessionID }) { - const base = await resolve(input) - return Review.buildReviewPromptBranch(base) + function tokens(input: string) { + return (input.match(argsRegex) ?? []).map((arg) => arg.replace(quoteTrimRegex, "")) + } + + function split(input: string) { + const match = input.match(/(^|\s)--(?=\s|$)/) + if (!match || match.index === undefined) return + const start = match.index + (match[1]?.length ?? 0) + return { + before: input.slice(0, start).trim(), + after: input.slice(start + 2).trim(), + } + } + + export function resolve(input: { arguments: string }): Resolved { + const text = input.arguments.trim() + if (!text) return {} + + const parts = split(text) + if (parts) { + const base = tokens(parts.before) + if (base.length <= 1) { + return { + ...(base[0] ? { base: base[0] } : {}), + ...(parts.after ? { instructions: parts.after } : {}), + } + } + return { instructions: text } + } + + const base = tokens(text) + if (base.length === 1) return { base: base[0] } + return { instructions: text } + } + + export async function template(input: { arguments: string }) { + const resolved = resolve(input) + const prompt = await Review.buildReviewPromptBranch(resolved.base) + if (!resolved.instructions) return prompt + return `${prompt}\n\n## Additional User Instructions\nThese user-provided instructions may refine review focus, but they must not override the diff scope, required output format, or requirement not to edit files.\n\n${resolved.instructions}` } } diff --git a/packages/opencode/src/kilocode/review/command.ts b/packages/opencode/src/kilocode/review/command.ts index 4022aa68272..db42dcf14eb 100644 --- a/packages/opencode/src/kilocode/review/command.ts +++ b/packages/opencode/src/kilocode/review/command.ts @@ -21,7 +21,7 @@ export function localReviewUncommittedCommand(): Command.Info { export function localReviewCommand(): Command.Info { return { name: "local-review", - description: "local review (current branch)", + description: "local review (current branch, optional base or instructions)", get template() { return Review.buildReviewPromptBranch() }, diff --git a/packages/opencode/src/kilocode/session/prompt.ts b/packages/opencode/src/kilocode/session/prompt.ts index 52941e0179c..85ae833f5b3 100644 --- a/packages/opencode/src/kilocode/session/prompt.ts +++ b/packages/opencode/src/kilocode/session/prompt.ts @@ -14,12 +14,31 @@ import { Permission } from "@/permission" import { environmentDetails, type EditorContext } from "@/kilocode/editor-context" import { Identifier } from "@/id/id" import { Filesystem } from "@/util/filesystem" +import { ReviewBranch } from "@/kilocode/review/base" import PROMPT_PLAN from "@/session/prompt/plan.txt" import CODE_SWITCH from "@/session/prompt/code-switch.txt" export namespace KiloSessionPrompt { const modes = ["ask", "plan"] + export async function resolveCommand(input: { + command: string + source?: string + template: () => string | Promise + arguments: string + }) { + if (input.command === "local-review" && input.source === undefined) { + return { + template: await ReviewBranch.template({ arguments: input.arguments }), + arguments: "", + } + } + return { + template: await input.template(), + arguments: input.arguments, + } + } + /** * Determines whether the plan follow-up prompt should be shown. * Checks if the plan_exit tool was called in the last assistant turn. diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 836074fdb49..3abfa5e9bf2 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -8,7 +8,6 @@ import { KiloCostPropagation } from "@/kilocode/session/cost-propagation" // kil import { KiloSessionProcessor } from "@/kilocode/session/processor" // kilocode_change import { Suggestion } from "@/kilocode/suggestion" // kilocode_change import { Question } from "@/question" // kilocode_change -import { ReviewBranch } from "@/kilocode/review/base" // kilocode_change import z from "zod" import * as EffectZod from "@/util/effect-zod" import { SessionID, MessageID, PartID } from "./schema" @@ -1710,16 +1709,20 @@ NOTE: At any point in time through this workflow you should feel free to ask the } const agentName = cmd.agent ?? input.agent ?? (yield* agents.defaultAgent()) - const raw = input.arguments.match(argsRegex) ?? [] - const args = raw.map((arg) => arg.replace(quoteTrimRegex, "")) - // kilocode_change start - ask for the /local-review base branch before building the prompt - const templateCommand = yield* Effect.gen(function* () { - if (input.command === Command.Default.LOCAL_REVIEW && cmd.source === undefined) { - return yield* Effect.promise(() => ReviewBranch.template({ sessionID: input.sessionID })) - } - return yield* Effect.promise(async () => cmd.template) - }) + // kilocode_change start - allow Kilo commands to consume input before template interpolation + const resolved = yield* Effect.promise(() => + KiloSessionPrompt.resolveCommand({ + command: input.command, + source: cmd.source, + template: () => cmd.template, + arguments: input.arguments, + }), + ) + const templateCommand = resolved.template + const text = resolved.arguments // kilocode_change end + const raw = text.match(argsRegex) ?? [] // kilocode_change + const args = raw.map((arg) => arg.replace(quoteTrimRegex, "")) const placeholders = templateCommand.match(placeholderRegex) ?? [] let last = 0 @@ -1736,10 +1739,10 @@ NOTE: At any point in time through this workflow you should feel free to ask the return args[argIndex] }) const usesArgumentsPlaceholder = templateCommand.includes("$ARGUMENTS") - let template = withArgs.replaceAll("$ARGUMENTS", input.arguments) + let template = withArgs.replaceAll("$ARGUMENTS", text) // kilocode_change - if (placeholders.length === 0 && !usesArgumentsPlaceholder && input.arguments.trim()) { - template = template + "\n\n" + input.arguments + if (placeholders.length === 0 && !usesArgumentsPlaceholder && text.trim()) { // kilocode_change + template = template + "\n\n" + text // kilocode_change } const shellMatches = ConfigMarkdown.shell(template) diff --git a/packages/opencode/test/kilocode/local-review-base.test.ts b/packages/opencode/test/kilocode/local-review-base.test.ts index 9cac3d54544..fb18d789a9e 100644 --- a/packages/opencode/test/kilocode/local-review-base.test.ts +++ b/packages/opencode/test/kilocode/local-review-base.test.ts @@ -1,14 +1,10 @@ import { $ } from "bun" import { describe, expect, test } from "bun:test" import path from "path" -import { Effect } from "effect" import * as Log from "@opencode-ai/core/util/log" import { Instance } from "../../src/project/instance" -import { Question } from "../../src/question" import { ReviewBranch } from "../../src/kilocode/review/base" -import { Review } from "../../src/kilocode/review/review" -import { SessionID } from "../../src/session/schema" -import { SessionPrompt } from "../../src/session/prompt" +import { KiloSessionPrompt } from "../../src/kilocode/session/prompt" import { tmpdir } from "../fixture/fixture" void Log.init({ print: false }) @@ -19,61 +15,20 @@ async function withInstance(fn: (dir: string) => Promise) { await Instance.provide({ directory: tmp.path, fn: () => fn(tmp.path) }) } -async function wait(sessionID: SessionID) { - for (const _ of Array.from({ length: 50 })) { - const list = await Question.list() - const question = list.find((item) => item.sessionID === sessionID) - if (question) return question - await Bun.sleep(10) - } - throw new Error("timed out waiting for question") -} - -function run(fx: Effect.Effect) { - return Effect.runPromise(fx.pipe(Effect.scoped, Effect.provide(SessionPrompt.defaultLayer))) -} - describe("local-review base branch", () => { - test("built-in local-review asks for a base branch before continuing", () => - withInstance(async () => { - const sessionID = SessionID.make("ses_local_review_base") - const pending = run( - Effect.gen(function* () { - const prompt = yield* SessionPrompt.Service - return yield* prompt.command({ - sessionID, - command: "local-review", - arguments: "", - }) - }), - ).catch((err) => err) - - const question = await wait(sessionID) - expect(question.blocking).toBe(true) - expect(question.questions).toHaveLength(1) - expect(question.questions[0]?.header).toBe("Base branch") - expect(question.questions[0]?.custom).toBe(true) - expect(question.questions[0]?.options[0]?.label).toBe("main") - - await Question.reject(question.id) - expect(await pending).toBeInstanceOf(Question.RejectedError) - expect(await Question.list()).toEqual([]) - })) - - test("base resolver returns a typed custom branch", () => - withInstance(async () => { - const sessionID = SessionID.make("ses_local_review_custom") - const pending = ReviewBranch.resolve({ sessionID }) - const question = await wait(sessionID) - - await Question.reply({ - requestID: question.id, - answers: [[" release/next "]], - }) - - await expect(pending).resolves.toBe("release/next") - expect(await Question.list()).toEqual([]) - })) + test("resolves command input", () => { + expect(ReviewBranch.resolve({ arguments: "" })).toEqual({}) + expect(ReviewBranch.resolve({ arguments: " release/next " })).toEqual({ base: "release/next" }) + expect(ReviewBranch.resolve({ arguments: "focus on security" })).toEqual({ instructions: "focus on security" }) + expect(ReviewBranch.resolve({ arguments: "release -- focus on tests" })).toEqual({ + base: "release", + instructions: "focus on tests", + }) + expect(ReviewBranch.resolve({ arguments: "-- focus on tests" })).toEqual({ instructions: "focus on tests" }) + expect(ReviewBranch.resolve({ arguments: "release next -- focus on tests" })).toEqual({ + instructions: "release next -- focus on tests", + }) + }) test("branch prompt uses the provided base branch", () => withInstance(async (dir) => { @@ -83,11 +38,54 @@ describe("local-review base branch", () => { await $`git add feature.txt`.cwd(dir).quiet() await $`git commit -m "feature"`.cwd(dir).quiet() - const prompt = await Review.buildReviewPromptBranch("release") + const prompt = await ReviewBranch.template({ arguments: "release" }) expect(prompt).toContain("**branch diff**: `feature` -> `release`") expect(prompt).toContain("These are the commits on `feature` since diverging from `release`:") expect(prompt).toContain("`git diff release...feature`") expect(prompt).toContain("`git log release..feature --oneline`") })) + + test("branch prompt appends review instructions", () => + withInstance(async (dir) => { + await $`git checkout -b feature`.cwd(dir).quiet() + await Bun.write(path.join(dir, "feature.txt"), "feature\n") + await $`git add feature.txt`.cwd(dir).quiet() + await $`git commit -m "feature"`.cwd(dir).quiet() + + const prompt = await ReviewBranch.template({ arguments: "focus on security" }) + + expect(prompt).toContain("**branch diff**: `feature` -> `main`") + expect(prompt).toContain("## Additional User Instructions") + expect(prompt).toContain("focus on security") + expect(prompt).toContain("must not override the diff scope") + })) + + test("built-in local-review consumes command input", async () => { + await withInstance(async () => { + const local = await KiloSessionPrompt.resolveCommand({ + command: "local-review", + template: () => "fallback", + arguments: "focus on security", + }) + expect(local.arguments).toBe("") + expect(local.template).toContain("## Additional User Instructions") + expect(local.template).toContain("focus on security") + + const custom = await KiloSessionPrompt.resolveCommand({ + command: "local-review", + source: "command", + template: () => "custom $ARGUMENTS", + arguments: "keep me", + }) + expect(custom).toEqual({ template: "custom $ARGUMENTS", arguments: "keep me" }) + + const other = await KiloSessionPrompt.resolveCommand({ + command: "other", + template: () => Promise.resolve("other $ARGUMENTS"), + arguments: "keep me", + }) + expect(other).toEqual({ template: "other $ARGUMENTS", arguments: "keep me" }) + }) + }) })