mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
core: improve suggestion action prompt resolution to prevent deadlocks
When agents suggest actions (like code review), the prompts can now resolve slash-command templates directly instead of injecting synthetic user messages. This prevents potential session deadlocks and allows the agent to immediately begin working on the accepted action.
This commit is contained in:
@@ -19,4 +19,9 @@ You are Kilo, a highly skilled software engineer with extensive knowledge in man
|
||||
- If the `suggest` tool is available, use it for lightweight next-step nudges that the user can accept or dismiss.
|
||||
- When you have completed implementation work and you are at least 90% confident the task is done, use `suggest` to offer a code review of uncommitted changes.
|
||||
- Only suggest review when the user's request appears fully addressed. Do not suggest it after every edit or partial implementation turn.
|
||||
- Do not repeat a review suggestion that was already dismissed in this conversation.
|
||||
- Keep suggestion text concise, use at most 1-2 actions, and make each accepted action prompt self-contained.
|
||||
- When suggesting a code review, choose the right command for the action prompt:
|
||||
- `/local-review-uncommitted` — for reviewing uncommitted working-tree changes (staged, unstaged, and untracked files).
|
||||
- `/local-review` — for reviewing all committed changes on the current branch vs its base branch.
|
||||
- Prefer `/local-review-uncommitted` when the work you just did has not been committed yet.
|
||||
|
||||
@@ -7,8 +7,6 @@ import { PermissionNext } from "../permission/next"
|
||||
import { Ripgrep } from "../file/ripgrep"
|
||||
import { iife } from "@/util/iife"
|
||||
|
||||
const BUILTIN = Skill.BUILTIN_LOCATION // kilocode_change
|
||||
|
||||
export const SkillTool = Tool.define("skill", async (ctx) => {
|
||||
const skills = await Skill.all()
|
||||
|
||||
@@ -39,7 +37,8 @@ export const SkillTool = Tool.define("skill", async (ctx) => {
|
||||
"<available_skills>",
|
||||
// kilocode_change start - guard pathToFileURL for builtin skills
|
||||
...accessibleSkills.flatMap((skill) => {
|
||||
const loc = skill.location === BUILTIN ? BUILTIN : pathToFileURL(skill.location).href
|
||||
const loc =
|
||||
skill.location === Skill.BUILTIN_LOCATION ? Skill.BUILTIN_LOCATION : pathToFileURL(skill.location).href
|
||||
return [
|
||||
` <skill>`,
|
||||
` <name>${skill.name}</name>`,
|
||||
@@ -81,7 +80,7 @@ export const SkillTool = Tool.define("skill", async (ctx) => {
|
||||
})
|
||||
|
||||
// kilocode_change start - built-in skills have no filesystem directory
|
||||
if (skill.location === BUILTIN) {
|
||||
if (skill.location === Skill.BUILTIN_LOCATION) {
|
||||
return {
|
||||
title: `Loaded skill: ${skill.name}`,
|
||||
output: [
|
||||
@@ -93,7 +92,7 @@ export const SkillTool = Tool.define("skill", async (ctx) => {
|
||||
].join("\n"),
|
||||
metadata: {
|
||||
name: skill.name,
|
||||
dir: BUILTIN,
|
||||
dir: Skill.BUILTIN_LOCATION,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
// kilocode_change - new file
|
||||
import { Command } from "@/command"
|
||||
import { Flag } from "@/flag/flag"
|
||||
import { Identifier } from "@/id/id"
|
||||
import { Session } from "@/session"
|
||||
import { MessageV2 } from "@/session/message-v2"
|
||||
import { Log } from "@/util/log"
|
||||
import { Suggestion } from "@/suggestion"
|
||||
import z from "zod"
|
||||
import DESCRIPTION from "./suggest.txt"
|
||||
import { Tool } from "./tool"
|
||||
|
||||
const log = Log.create({ service: "tool.suggest" })
|
||||
|
||||
const Params = z.object({
|
||||
suggest: z.string().describe("Short suggestion text shown to the user"),
|
||||
actions: z.array(Suggestion.Action).min(1).max(2).describe("Available actions the user can take"),
|
||||
@@ -18,42 +19,39 @@ type Meta = {
|
||||
dismissed: boolean
|
||||
}
|
||||
|
||||
async function inject(input: { sessionID: string; user: MessageV2.User; agent: string; text: string }) {
|
||||
const msg: MessageV2.User = {
|
||||
id: Identifier.ascending("message"),
|
||||
sessionID: input.sessionID,
|
||||
role: "user",
|
||||
time: {
|
||||
created: Date.now(),
|
||||
},
|
||||
agent: input.agent,
|
||||
model: input.user.model,
|
||||
variant: input.user.variant,
|
||||
editorContext: input.user.editorContext,
|
||||
/**
|
||||
* If prompt starts with `/`, treat it as a slash-command reference.
|
||||
* Resolve the command template and return its content so the LLM can
|
||||
* act on it in the current turn — without injecting a synthetic user
|
||||
* message or trying to dispatch a command on the same session (which
|
||||
* would deadlock).
|
||||
*/
|
||||
async function resolve(prompt: string): Promise<string> {
|
||||
if (!prompt.startsWith("/")) return prompt
|
||||
|
||||
const name = prompt.slice(1).split(/\s/, 1)[0]
|
||||
if (!name) return prompt
|
||||
|
||||
const cmd = await Command.get(name)
|
||||
if (!cmd) {
|
||||
log.warn("unknown command in suggestion action", { name })
|
||||
return prompt
|
||||
}
|
||||
|
||||
try {
|
||||
const template = await cmd.template
|
||||
log.info("resolved command template", { name, length: template.length })
|
||||
return template
|
||||
} catch (err) {
|
||||
log.warn("failed to resolve command template", { name, err })
|
||||
return prompt
|
||||
}
|
||||
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)
|
||||
}
|
||||
|
||||
export const SuggestTool = Tool.define<typeof Params, Meta>("suggest", {
|
||||
description: DESCRIPTION,
|
||||
parameters: Params,
|
||||
async execute(params, ctx) {
|
||||
const user = ctx.messages
|
||||
.slice()
|
||||
.reverse()
|
||||
.find((msg) => msg.info.role === "user")?.info
|
||||
if (!user || user.role !== "user") {
|
||||
throw new Error("No user message found for suggestion context")
|
||||
}
|
||||
|
||||
const promise = Suggestion.show({
|
||||
sessionID: ctx.sessionID,
|
||||
text: params.suggest,
|
||||
@@ -90,12 +88,7 @@ export const SuggestTool = Tool.define<typeof Params, Meta>("suggest", {
|
||||
}
|
||||
}
|
||||
|
||||
await inject({
|
||||
sessionID: ctx.sessionID,
|
||||
user,
|
||||
agent: ctx.agent,
|
||||
text: action.prompt,
|
||||
})
|
||||
const resolved = await resolve(action.prompt)
|
||||
|
||||
const metadata: Meta = {
|
||||
accepted: action,
|
||||
@@ -104,7 +97,7 @@ export const SuggestTool = Tool.define<typeof Params, Meta>("suggest", {
|
||||
|
||||
return {
|
||||
title: `User accepted: ${action.label}`,
|
||||
output: `User accepted the suggestion "${action.label}". The accepted action prompt is: ${JSON.stringify(action.prompt)}. It has also been injected as a synthetic user message. Continue with that request now.`,
|
||||
output: `User accepted the suggestion "${action.label}". Carry out the following request now:\n\n${resolved}`,
|
||||
metadata,
|
||||
}
|
||||
},
|
||||
|
||||
@@ -11,3 +11,9 @@ Guidelines:
|
||||
- Provide 1-2 actions maximum
|
||||
- Make each action prompt self-contained so it can be injected as a synthetic user message
|
||||
- If you need a real answer from the user, use the `question` tool instead
|
||||
|
||||
When suggesting a code review:
|
||||
- Use `/local-review-uncommitted` as the action prompt for uncommitted working-tree changes
|
||||
- Use `/local-review` as the action prompt for committed branch-level changes
|
||||
- Only suggest review when the user's request appears fully addressed. Do not suggest it after every edit or partial implementation turn.
|
||||
- Do not repeat a review suggestion that was already dismissed in this conversation
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, test, spyOn } from "bun:test"
|
||||
import { Session } from "../../src/session"
|
||||
import { Command } from "../../src/command"
|
||||
import { Suggestion } from "../../src/suggestion"
|
||||
import { SuggestTool } from "../../src/tool/suggest"
|
||||
|
||||
@@ -28,19 +28,16 @@ const ctx = {
|
||||
|
||||
describe("tool.suggest", () => {
|
||||
let show: ReturnType<typeof spyOn>
|
||||
let updateMessage: ReturnType<typeof spyOn>
|
||||
let updatePart: ReturnType<typeof spyOn>
|
||||
let cmdGet: ReturnType<typeof spyOn>
|
||||
|
||||
beforeEach(() => {
|
||||
show = spyOn(Suggestion, "show")
|
||||
updateMessage = spyOn(Session, "updateMessage").mockResolvedValue({} as never)
|
||||
updatePart = spyOn(Session, "updatePart").mockResolvedValue({} as never)
|
||||
cmdGet = spyOn(Command, "get")
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
show.mockRestore()
|
||||
updateMessage.mockRestore()
|
||||
updatePart.mockRestore()
|
||||
cmdGet.mockRestore()
|
||||
})
|
||||
|
||||
test("returns dismissal result when suggestion is dismissed", async () => {
|
||||
@@ -60,13 +57,19 @@ describe("tool.suggest", () => {
|
||||
expect(result.metadata.dismissed).toBe(true)
|
||||
})
|
||||
|
||||
test("returns accepted action metadata when suggestion is accepted", async () => {
|
||||
test("resolves command template for slash-command action prompt", async () => {
|
||||
const tool = await SuggestTool.init()
|
||||
show.mockResolvedValueOnce({
|
||||
label: "Start review",
|
||||
description: "Run a local review now",
|
||||
prompt: "/local-review-uncommitted",
|
||||
})
|
||||
cmdGet.mockResolvedValueOnce({
|
||||
name: "local-review-uncommitted",
|
||||
description: "local review (uncommitted changes)",
|
||||
template: Promise.resolve("Review these uncommitted changes:\n\n## Files Changed\n..."),
|
||||
hints: [],
|
||||
})
|
||||
|
||||
const result = await tool.execute(
|
||||
{
|
||||
@@ -77,14 +80,83 @@ describe("tool.suggest", () => {
|
||||
)
|
||||
|
||||
expect(result.title).toBe("User accepted: Start review")
|
||||
expect(result.output).toContain("Continue with that request now")
|
||||
expect(result.output).toContain("Review these uncommitted changes:")
|
||||
expect(result.output).toContain("Carry out the following request now")
|
||||
expect(result.metadata.dismissed).toBe(false)
|
||||
expect(updateMessage).toHaveBeenCalledTimes(1)
|
||||
expect(updatePart).toHaveBeenCalledTimes(1)
|
||||
expect(result.metadata.accepted).toEqual({
|
||||
label: "Start review",
|
||||
description: "Run a local review now",
|
||||
prompt: "/local-review-uncommitted",
|
||||
})
|
||||
expect(cmdGet).toHaveBeenCalledWith("local-review-uncommitted")
|
||||
})
|
||||
|
||||
test("returns plain-text prompt directly for non-command actions", async () => {
|
||||
const tool = await SuggestTool.init()
|
||||
show.mockResolvedValueOnce({
|
||||
label: "Run tests",
|
||||
prompt: "Run the test suite and fix any failures",
|
||||
})
|
||||
|
||||
const result = await tool.execute(
|
||||
{
|
||||
suggest: "Tests might need running",
|
||||
actions: [{ label: "Run tests", prompt: "Run the test suite and fix any failures" }],
|
||||
},
|
||||
ctx as any,
|
||||
)
|
||||
|
||||
expect(result.title).toBe("User accepted: Run tests")
|
||||
expect(result.output).toContain("Run the test suite and fix any failures")
|
||||
expect(result.output).toContain("Carry out the following request now")
|
||||
expect(result.metadata.dismissed).toBe(false)
|
||||
expect(cmdGet).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test("falls back to raw prompt when command is not found", async () => {
|
||||
const tool = await SuggestTool.init()
|
||||
show.mockResolvedValueOnce({
|
||||
label: "Unknown cmd",
|
||||
prompt: "/nonexistent-command",
|
||||
})
|
||||
cmdGet.mockResolvedValueOnce(undefined)
|
||||
|
||||
const result = await tool.execute(
|
||||
{
|
||||
suggest: "Try this?",
|
||||
actions: [{ label: "Unknown cmd", prompt: "/nonexistent-command" }],
|
||||
},
|
||||
ctx as any,
|
||||
)
|
||||
|
||||
expect(result.title).toBe("User accepted: Unknown cmd")
|
||||
expect(result.output).toContain("/nonexistent-command")
|
||||
expect(result.metadata.dismissed).toBe(false)
|
||||
})
|
||||
|
||||
test("falls back to raw prompt when template resolution fails", async () => {
|
||||
const tool = await SuggestTool.init()
|
||||
show.mockResolvedValueOnce({
|
||||
label: "Start review",
|
||||
prompt: "/local-review-uncommitted",
|
||||
})
|
||||
cmdGet.mockResolvedValueOnce({
|
||||
name: "local-review-uncommitted",
|
||||
description: "local review (uncommitted changes)",
|
||||
template: Promise.reject(new Error("git not found")),
|
||||
hints: [],
|
||||
})
|
||||
|
||||
const result = await tool.execute(
|
||||
{
|
||||
suggest: "Run review?",
|
||||
actions: [{ label: "Start review", prompt: "/local-review-uncommitted" }],
|
||||
},
|
||||
ctx as any,
|
||||
)
|
||||
|
||||
expect(result.title).toBe("User accepted: Start review")
|
||||
expect(result.output).toContain("/local-review-uncommitted")
|
||||
expect(result.metadata.dismissed).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user