Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pr-comments-from-changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": minor
---

Post inline comments to a checked-out GitHub pull request from Changes and Agent Manager diff views. The compact composer saves locally, sends to Kilo, or posts to GitHub with an explicit destination choice. Cmd/Ctrl+Enter saves a comment, and pressing it again in the review view sends all comments to Kilo without publishing to GitHub. The toolbar shows separate send-all-to-Kilo and send-all-to-GitHub actions, and only the Kilo action carries the keyboard shortcut. GitHub posting stops on the first error so unpublished comments are kept.
11 changes: 11 additions & 0 deletions packages/kilo-vscode/src/agent-manager/pr/review-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export class PRReviewActions {
if (!result.requestId) throw new Error("Missing request identity.")
const initial = this.host.context(message)
const context = { ...initial, pr: { ...initial.pr } }
await this.checkBranch(context)
if (message.type === "agentManager.loadPRFiles") {
const snapshot = await this.load(context, message)
this.host.post({ ...result, type: "agentManager.loadPRFilesResult", success: true, snapshot })
Expand Down Expand Up @@ -180,6 +181,13 @@ export class PRReviewActions {
}
}

private async checkBranch(context: PRReviewContext) {
if (!this.host.checkBranch) return
const branch = await this.host.checkBranch(context.directory)
if (!branch || branch === "HEAD" || branch !== context.branch)
throw new Error("Diff branch changed. Refresh and try again.")
}

private current(context: PRReviewContext, message: Record<string, unknown>) {
if (identity(this.host.context(message)) !== identity(context))
throw new Error("Pull request context changed. Reload the review.")
Expand All @@ -203,6 +211,7 @@ export class PRReviewActions {
}
}
const after = await metadata(context)
await this.checkBranch(context)
this.current(context, message)
if (
before.head !== after.head ||
Expand All @@ -229,6 +238,7 @@ export class PRReviewActions {
const snapshot = this.snapshot(context, message)
const { file, start, end, body } = selection(snapshot, message)
const fresh = await metadata(context)
await this.checkBranch(context)
this.current(context, message)
if (fresh.head !== snapshot.data.head || fresh.base !== snapshot.base)
throw new Error("Pull request changed. Reload the review before posting.")
Expand Down Expand Up @@ -263,6 +273,7 @@ export class PRReviewActions {
if (message.head !== snapshot.data.head)
throw new Error("Pull request changed. Reload the review before submitting.")
const fresh = await metadata(context)
await this.checkBranch(context)
this.current(context, message)
if (fresh.head !== snapshot.data.head || fresh.base !== snapshot.base)
throw new Error("Pull request changed. Reload the review before submitting.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ export interface PRReviewHost {
conflicts?: (context: PRReviewContext, base: string, head: string) => Promise<string[]>
getPRMergeMethod?: (repo: string) => PRMergeMethod | undefined
savePRMergeMethod?: (repo: string, method: PRMergeMethod) => Promise<void>
checkBranch?: (directory: string) => Promise<string>
}
51 changes: 49 additions & 2 deletions packages/kilo-vscode/src/diff/DiffViewerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import { addCommentReaction, isPRReactionContent, removeCommentReaction } from "
import type { PRStatus } from "../agent-manager/types"
import { ghErrorReason } from "../agent-manager/pr/am-pr-utils"
import { createDiffCommentActions } from "./comment-actions"
import { PRReviewActions } from "../agent-manager/pr/review-actions"
import type { PRReviewContext } from "../agent-manager/pr/review-context"
import { execWithShellEnv } from "../agent-manager/shell-env"

type CommentHandler = (comments: unknown[], autoSend: boolean) => void
type OpenArgs = {
Expand Down Expand Up @@ -120,6 +123,7 @@ export class DiffViewerProvider implements vscode.Disposable {
private baseBranchOverride: string | undefined
private target: CommentHandler | undefined
private readonly prPolling: ReturnType<typeof createDiffPRPolling>
private readonly reviews: PRReviewActions
private focusPending = false
private openGeneration = 0
private readonly identity = randomUUID()
Expand Down Expand Up @@ -149,6 +153,23 @@ export class DiffViewerProvider implements vscode.Disposable {
onStatus: () => this.sendComments(),
log: (...args) => this.log(...args),
})
this.reviews = new PRReviewActions({
context: (message) => this.reviewContext(message),
post: (message) => {
void this.panel?.webview.postMessage(message)
},
refresh: (review) => {
if (this.commentContext()?.token === review.projectId) this.prPolling.refresh()
},
dirtyFiles: () => [],
checkBranch: async (directory) => {
const result = await execWithShellEnv("git", ["rev-parse", "--abbrev-ref", "HEAD"], {
cwd: directory,
timeout: 5_000,
})
return result.stdout.trim()
},
})
}

setCommentHandler(handler: CommentHandler): void {
Expand Down Expand Up @@ -279,7 +300,7 @@ export class DiffViewerProvider implements vscode.Disposable {
}

private onMessage(msg: Record<string, unknown>): void {
if (this.actions.handle(msg)) return
if (this.actions.handle(msg) || this.reviews.handle(msg)) return
const handler = this.messageHandlers[msg.type as string]
handler?.(msg)
}
Expand Down Expand Up @@ -483,7 +504,14 @@ export class DiffViewerProvider implements vscode.Disposable {
const comments = selected && !match ? [...live, { ...selected, outdated: true }] : live
const ctx = this.commentContext()
const target = ctx
? { projectId: ctx.token, worktreeId: "diff", prNumber: ctx.pr.number, prUrl: ctx.pr.url }
? {
projectId: ctx.token,
worktreeId: "diff",
prNumber: ctx.pr.number,
prUrl: ctx.pr.url,
baseRefOid: ctx.pr.baseRefOid,
headRefOid: ctx.pr.headRefOid,
}
: undefined
void this.panel.webview.postMessage({
type: "diffViewer.prComments",
Expand Down Expand Up @@ -522,6 +550,25 @@ export class DiffViewerProvider implements vscode.Disposable {
}
}

private reviewContext(message: Record<string, unknown>): PRReviewContext {
const ctx = this.commentContext()
if (
!ctx ||
message.projectId !== ctx.token ||
message.worktreeId !== "diff" ||
message.prNumber !== ctx.pr.number ||
message.prUrl !== ctx.pr.url
)
throw new Error("Pull request context changed. Refresh and try again.")
return {
pr: ctx.pr,
directory: ctx.directory,
branch: ctx.branch,
worktreeId: "diff",
projectId: ctx.token,
}
}

private getHtml(webview: vscode.Webview): string {
return buildWebviewHtml(webview, {
scriptUri: webview.asWebviewUri(vscode.Uri.joinPath(this.extensionUri, "dist", "diff-viewer.js")),
Expand Down
2 changes: 2 additions & 0 deletions packages/kilo-vscode/src/shared/pr-comment-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export interface PRTarget {
worktreeId: string
prNumber: number
prUrl: string
baseRefOid?: string
headRefOid?: string
}

export interface PRFile {
Expand Down
5 changes: 4 additions & 1 deletion packages/kilo-vscode/src/shared/pr-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ export function parsePatch(patch: string, totals?: { additions: unknown; deletio
function hunks(patch: string, selection?: Range) {
const lines = patch.split("\n")
if (lines.at(-1) === "") lines.pop()
// Patches may include file headers (`diff --git`, `---`, `+++`) before the first hunk.
const start = lines.findIndex((line) => line.startsWith("@@"))
if (start < 0) return
const result: Range[] = []
const selected: string[] = []
let index = 0
let index = start
let added = 0
let removed = 0
let left = 0
Expand Down
14 changes: 7 additions & 7 deletions packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,18 +198,18 @@ test("preserves scroll while adding and editing a review comment", async ({ page
const line = target.locator('[data-line="1"]').last()
await line.hover()
await target.locator("[data-utility-button]").last().click()
await expect(target.locator(".am-annotation-textarea")).toBeVisible()
await target.locator(".am-annotation-textarea").fill("Keep this stable")
await expect(target.locator(".am-annotation-draft textarea")).toBeVisible()
await target.locator(".am-annotation-draft textarea").fill("Keep this stable")
const top = await target.evaluate((el) => el.getBoundingClientRect().top)
const before = await scroller.evaluate((el) => el.scrollTop)

await page.getByRole("button", { name: "Apply agent edit" }).click()
await expect(page.getByTestId("agent-edit-version")).toHaveText("after")
await expect(target.locator(".am-annotation-textarea")).toHaveValue("Keep this stable")
await expect(target.locator(".am-annotation-draft textarea")).toHaveValue("Keep this stable")
await expect.poll(async () => scroller.evaluate((el) => el.scrollTop)).toBeCloseTo(before, 0)
await expect.poll(async () => target.evaluate((el) => el.getBoundingClientRect().top)).toBeCloseTo(top, 0)

await target.getByRole("button", { name: "Comment" }).click()
await target.locator('[data-action="save"]').click()
await expect(target.getByText("Keep this stable")).toBeVisible()
const saved = await scroller.evaluate((el) => el.scrollTop)

Expand All @@ -229,15 +229,15 @@ for (const modifier of ["Meta", "Control"] as const) {
for (const text of ["First comment", "Second comment"]) {
await target.locator('[data-line="1"]').last().hover()
await target.locator("[data-utility-button]").last().click()
await target.locator(".am-annotation-textarea").fill(text)
await target.locator(".am-annotation-draft textarea").fill(text)
if (text === "First comment") {
await target.getByRole("button", { name: "Comment", exact: true }).click()
await target.locator('[data-action="save"]').click()
await expect(target.getByText(text, { exact: true })).toBeVisible()
}
}

await page.keyboard.press("Shift+Enter")
await expect(target.locator(".am-annotation-textarea")).toHaveValue("Second comment\n")
await expect(target.locator(".am-annotation-draft textarea")).toHaveValue("Second comment\n")

const result = await page.evaluate((modifier) => {
const sent: Array<{ comments: Array<{ comment: string }>; autoSend: boolean }> = []
Expand Down
141 changes: 141 additions & 0 deletions packages/kilo-vscode/tests/fixtures/diff-comment-form.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import assert from "node:assert/strict"
import { harness } from "./comment-harness"
import type { PRReviewRequest } from "../../src/shared/pr-comment-actions"

const { window, root, messages, node, button, input, type, last, respond, wait, mount } =
await harness<PRReviewRequest>()
const { PRCommentForm } = await import("../../webview-ui/agent-manager/pr/PRCommentForm")
const saved: string[] = []
const sent: string[] = []
let cancelled = 0
let completed = 0
const release = mount(() => (
<>
<div id="local">
<PRCommentForm
inline
action="diff"
worktreeId="diff-test"
file="example.ts"
side="RIGHT"
startLine={2}
endLine={2}
selectedText="return 1"
destination="local"
onSave={(body) => saved.push(body)}
onSendKilo={(body) => sent.push(body)}
onGithubSuccess={() => completed++}
onCancel={() => cancelled++}
onDestinationChange={() => {}}
/>
</div>
<div id="remote">
<PRCommentForm
inline
action="diff"
worktreeId="diff-test"
file="other.ts"
side="RIGHT"
startLine={5}
endLine={5}
selectedText="old line"
destination="github"
github={{
prNumber: 1,
prUrl: "https://github.com/example/fixture/pull/1",
snapshotId: "snapshot",
label: "GitHub #1",
closed: false,
}}
onSave={() => {}}
onSendKilo={() => {}}
onGithubSuccess={() => completed++}
onCancel={() => cancelled++}
onDestinationChange={() => {}}
/>
</div>
<div id="remote2">
<PRCommentForm
inline
action="diff"
worktreeId="diff-test"
file="other.ts"
side="RIGHT"
startLine={5}
endLine={5}
selectedText="old line"
destination="github"
github={{
prNumber: 2,
prUrl: "https://github.com/example/fixture/pull/2",
snapshotId: "snapshot-2",
label: "GitHub #2",
closed: false,
}}
onSave={() => {}}
onSendKilo={() => {}}
onGithubSuccess={() => completed++}
onCancel={() => cancelled++}
onDestinationChange={() => {}}
/>
</div>
</>
))
await wait()
const local = node("#local")
const remote = node("#remote")
const remote2 = node("#remote2")

// Local-only destination exposes Kilo actions, never the GitHub split button.
assert.equal(button("send-kilo", local).textContent, "Send to Kilo")
assert.equal(button("save", local).textContent, "Save")
assert.equal(button("cancel", local).textContent, "Cancel")
assert.equal(local.querySelector('[data-action="send-primary"]'), null, "no split button without a PR")
assert.equal(messages.length, 0)

type(local, "Keep this")
button("save", local).click()
assert.deepEqual(saved, ["Keep this"])
assert.equal(input(local).value, "", "saving clears the composer")
type(local, "Send this")
button("send-kilo", local).click()
assert.deepEqual(sent, ["Send this"])
assert.equal(input(local).value, "", "sending to Kilo clears the composer")

// Plain Enter sends to Kilo and never posts to GitHub.
type(local, "Keyboard send")
input(local).dispatchEvent(new window.KeyboardEvent("keydown", { key: "Enter", bubbles: true }))
assert.deepEqual(sent, ["Send this", "Keyboard send"])
assert.equal(messages.length, 0, "local actions never request a GitHub write")

// Cmd/Ctrl+Enter saves the comment locally instead of sending it.
type(local, "Keyboard save")
input(local).dispatchEvent(new window.KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }))
assert.deepEqual(saved, ["Keep this", "Keyboard save"], "Cmd+Enter saves the comment")
assert.deepEqual(sent, ["Send this", "Keyboard send"], "Cmd+Enter does not send to Kilo")
assert.equal(messages.length, 0, "Cmd+Enter never requests a GitHub write")

// The remembered GitHub destination drives the split primary label.
assert.equal(button("send-primary", remote).textContent, "Send to GitHub #1")
node('[aria-label="Choose destination"]', remote)

// Enter is not bound to the GitHub destination, so it cannot publish by accident.
type(remote, "Do not post")
input(remote).dispatchEvent(new window.KeyboardEvent("keydown", { key: "Enter", bubbles: true }))
assert.equal(messages.length, 0, "Enter never posts to GitHub")
assert.equal(input(remote2).value, "", "a draft is scoped to its own PR identity")

type(remote, "Post me")
button("send-primary", remote).click()
const request = last()
assert.equal(request.type, "agentManager.createReviewComment")
assert.equal(input(remote).disabled, true)
button("send-primary", remote).click()
assert.equal(messages.length, 1, "double submission cannot publish twice")
respond(request, {})
assert.equal(completed, 1)

button("cancel", local).click()
assert.equal(cancelled, 1)
release()
await window.happyDOM.close()
Loading
Loading