From c941aa399a40eaf9e8779317a03b4f9e2edf1621 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 20 Aug 2026 13:09:08 +0200 Subject: [PATCH 1/5] fix(agent-manager): stabilize PR review comments --- .changeset/pr-comments-github-style.md | 2 + .../src/agent-manager/PRStatusPoller.ts | 11 +- .../src/agent-manager/pr-status-bridge.ts | 74 +++++--- .../src/agent-manager/pr/am-pr-utils.ts | 13 +- .../agent-manager/pr/pr-comment-context.ts | 66 +++++++ .../kilo-vscode/src/agent-manager/types.ts | 2 + .../tests/fixtures/pr-comments-render.tsx | 141 +++++++++++---- .../tests/unit/am-pr-status-bridge.test.ts | 44 ++++- .../tests/unit/pr-comment-context.test.ts | 108 +++++++++++ .../tests/unit/pr-status-merge.test.ts | 62 +++++++ .../tests/unit/review-comments-pr.test.ts | 104 ++++++++++- .../agent-manager/pr/PRCommentCard.tsx | 9 +- .../agent-manager/pr/PRComments.tsx | 171 +++++++++--------- .../webview-ui/agent-manager/pr/PRPanel.tsx | 86 ++++++++- .../agent-manager/pr/pr-comment-payload.ts | 143 ++++++++++++++- .../agent-manager/pr/pr-comment-state.ts | 70 +++++++ .../webview-ui/agent-manager/pr/pr-panel.css | 10 + .../webview-ui/agent-manager/pr/pr-types.ts | 2 + .../webview-ui/diff-viewer/PRCommentDiff.tsx | 23 ++- 19 files changed, 982 insertions(+), 159 deletions(-) create mode 100644 packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts create mode 100644 packages/kilo-vscode/tests/unit/pr-comment-context.test.ts create mode 100644 packages/kilo-vscode/tests/unit/pr-status-merge.test.ts create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/pr/pr-comment-state.ts diff --git a/.changeset/pr-comments-github-style.md b/.changeset/pr-comments-github-style.md index 5fe95f6a136..84504f6080e 100644 --- a/.changeset/pr-comments-github-style.md +++ b/.changeset/pr-comments-github-style.md @@ -3,3 +3,5 @@ --- Rework PR review comments in the Agent Manager PR panel: resolved threads now collapse into one-line rows in a Resolved group instead of being dimmed, each thread shows its replies, and every card has prominent Send to agent, Resolve, Copy, Open file, and Open on GitHub actions. A single button sends all unresolved comments to the agent, and comments arrive as structured review comments instead of pasted text. + +Every comment now shows the same amount of code, matching the GitHub snippet and continuing past the commented line with lines from the worktree, so a comment about what happens next is readable. Refreshing the PR no longer closes threads you opened, loses your scroll position, or makes comments disappear. diff --git a/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts b/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts index 528b8d3332c..bdd1473cbc6 100644 --- a/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts +++ b/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts @@ -14,6 +14,7 @@ import { parseReviewers, } from "./pr/am-pr-utils" import type { PRResult, GhThread, GhReviewRequest, GhReview } from "./pr/am-pr-types" +import { withContext } from "./pr/pr-comment-context" interface PRStatusPollerOptions { getWorktrees: () => Worktree[] @@ -482,10 +483,14 @@ export class PRStatusPoller { } } + /** + * Undefined on failure, never an empty thread list: the panel keeps the + * comments it already shows instead of collapsing the section mid-review. + */ private async fetchComments( prNumber: number, cwd: string, - ): Promise<{ total: number; unresolved: number; comments: PRComment[] }> { + ): Promise<{ total: number; unresolved: number; comments: PRComment[] } | undefined> { try { const repo = await this.getRepoInfo(cwd) const query = `query($owner: String!, $repo: String!, $number: Int!) { @@ -533,12 +538,12 @@ export class PRStatusPoller { ) const pr = JSON.parse(stdout)?.data?.repository?.pullRequest const threads = pr?.reviewThreads - const comments = parseComments((threads?.nodes ?? []) as GhThread[]) + const comments = await withContext(cwd, parseComments((threads?.nodes ?? []) as GhThread[])) const totalCount = threads?.totalCount ?? comments.length return { total: totalCount, unresolved: comments.filter((c) => !c.resolved).length, comments } } catch (err) { this.options.log("Failed to fetch PR comments:", err) - return { total: 0, unresolved: 0, comments: [] } + return undefined } } } diff --git a/packages/kilo-vscode/src/agent-manager/pr-status-bridge.ts b/packages/kilo-vscode/src/agent-manager/pr-status-bridge.ts index f4cd269eb37..fc67efa5fb2 100644 --- a/packages/kilo-vscode/src/agent-manager/pr-status-bridge.ts +++ b/packages/kilo-vscode/src/agent-manager/pr-status-bridge.ts @@ -10,7 +10,7 @@ import type { Disposable } from "./host" import type { Semaphore } from "./semaphore" import { PRStatusPoller } from "./PRStatusPoller" import { resolveComment, unresolveComment } from "./pr/PRActions" -import { ghErrorReason } from "./pr/am-pr-utils" +import { ghErrorReason, mergePRStatus } from "./pr/am-pr-utils" interface PRBridgeHost { getWorktrees(): Worktree[] @@ -32,6 +32,8 @@ interface PanelLike { export class PRStatusBridge { readonly poller: PRStatusPoller private readonly cache = new Map() + /** Branch each cached PR was found on, so a branch switch still clears it. */ + private readonly branches = new Map() private readonly host: PRBridgeHost private lastErrorNotified: "gh_missing" | "gh_auth" | "fetch_failed" | undefined @@ -137,11 +139,13 @@ export class PRStatusBridge { /** Remove cached status for a deleted worktree. */ remove(worktreeId: string): void { this.cache.delete(worktreeId) + this.branches.delete(worktreeId) } reset(): void { this.poller.stop() this.cache.clear() + this.branches.clear() this.lastErrorNotified = undefined } @@ -160,29 +164,57 @@ function bridgePollerOpts(bridge: PRStatusBridge, host: PRBridgeHost) { semaphore: host.semaphore, onStatus: (id: string, pr: PRStatus | null, err?: "gh_missing" | "gh_auth" | "fetch_failed") => { if (err) { - // Don't forward errors to the webview when we have prior PR data - // (in-memory cache or persisted prNumber) — that would overwrite - // the live badge with pr:null. Only forward when there's truly no - // prior data (first poll failed, nothing persisted). - if (!bridge["cache"].has(id) && !host.hasPersistedPR(id)) - host.postToWebview({ - type: "agentManager.prStatus", - worktreeId: id, - pr: null, - error: err, - } as AgentManagerOutMessage) - // Always forward auth/missing errors so the webview can show a toast, - // regardless of whether prior data exists. Deduplicate per error type - // so multiple failing worktrees don't produce multiple toasts. - if (err === "gh_auth" || err === "gh_missing") bridge.notifyError(err) + reportError(bridge, host, id, err) return } - const msg = { type: "agentManager.prStatus", worktreeId: id, pr, error: err } as AgentManagerOutMessage - bridge["cache"].set(id, msg) - bridge["lastErrorNotified"] = undefined - host.postToWebview(msg) - host.updateWorktreePR(id, pr?.number, pr?.url, pr?.state) + accept(bridge, host, id, pr) }, log: (...args: unknown[]) => host.log(...args), } } + +function reportError( + bridge: PRStatusBridge, + host: PRBridgeHost, + id: string, + err: "gh_missing" | "gh_auth" | "fetch_failed", +): void { + // Don't forward errors to the webview when we have prior PR data + // (in-memory cache or persisted prNumber) — that would overwrite + // the live badge with pr:null. Only forward when there's truly no + // prior data (first poll failed, nothing persisted). + if (!bridge["cache"].has(id) && !host.hasPersistedPR(id)) + host.postToWebview({ + type: "agentManager.prStatus", + worktreeId: id, + pr: null, + error: err, + } as AgentManagerOutMessage) + // Always forward auth/missing errors so the webview can show a toast, + // regardless of whether prior data exists. Deduplicate per error type + // so multiple failing worktrees don't produce multiple toasts. + if (err === "gh_auth" || err === "gh_missing") bridge.notifyError(err) +} + +function accept(bridge: PRStatusBridge, host: PRBridgeHost, id: string, pr: PRStatus | null): void { + const cached = bridge["cache"].get(id) + const prev = cached?.type === "agentManager.prStatus" ? cached.pr : null + const branch = host.getWorktrees().find((w: Worktree) => w.id === id)?.branch + // `gh` answers "no pull request" for a rate limit, a network blip, or an + // unresolvable fork ref exactly as it does for a branch that never had one. A + // PR cannot leave a branch, so on the same branch the known PR is kept: + // forwarding pr:null would unmount the panel and throw away the comment the + // user is reading. + if (!pr && prev && branch !== undefined && bridge["branches"].get(id) === branch) { + host.log(`PR status: keeping PR #${prev.number} for ${id}, empty result on ${branch}`) + return + } + const merged = pr && prev ? mergePRStatus(prev, pr) : pr + const msg = { type: "agentManager.prStatus", worktreeId: id, pr: merged } as AgentManagerOutMessage + bridge["cache"].set(id, msg) + if (pr && branch !== undefined) bridge["branches"].set(id, branch) + if (!pr) bridge["branches"].delete(id) + bridge["lastErrorNotified"] = undefined + host.postToWebview(msg) + host.updateWorktreePR(id, pr?.number, pr?.url, pr?.state) +} diff --git a/packages/kilo-vscode/src/agent-manager/pr/am-pr-utils.ts b/packages/kilo-vscode/src/agent-manager/pr/am-pr-utils.ts index 1eae226ad67..7fe96e36cbf 100644 --- a/packages/kilo-vscode/src/agent-manager/pr/am-pr-utils.ts +++ b/packages/kilo-vscode/src/agent-manager/pr/am-pr-utils.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto" -import type { CheckStatus, PRComment, PRReviewer, ReviewerState } from "../types" +import type { CheckStatus, PRComment, PRReviewer, PRStatus, ReviewerState } from "../types" import type { PRResult, GhThread, GhReviewRequest, GhReview } from "./am-pr-types" export function parsePRResult(json: string): PRResult | null { @@ -125,6 +125,17 @@ export function ghErrorReason(message: string): string { return (last ?? message.trim()).replace(/^gh:\s*/, "").slice(0, 200) } +/** + * Carry review threads across a status that has none. Only the selected worktree + * fetches comments, and that fetch can fail, so a plain replace would collapse + * the open comment list in the panel while the user is reading it. + */ +export function mergePRStatus(prev: PRStatus | undefined, next: PRStatus): PRStatus { + if (next.comments || !prev?.comments) return next + if (prev.number !== next.number) return next + return { ...next, comments: prev.comments } +} + /** * Signature of the comment threads, for poll deduplication. Thread and * unresolved counts alone hide edits and new replies, which the panel renders. diff --git a/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts b/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts new file mode 100644 index 00000000000..5a2d8af710e --- /dev/null +++ b/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts @@ -0,0 +1,66 @@ +/** + * GitHub truncates `diffHunk` at the commented line, so a review comment about + * what happens *after* that line has no code to point at. The worktree holds + * the file, so the lines below the comment are read from disk and attached to + * the thread. + */ +import { readFile, stat } from "node:fs/promises" +import path from "node:path" +import type { PRComment } from "../types" + +/** Lines of trailing context, matching the window the panel renders. */ +const AFTER = 3 +/** A generated or vendored file is not worth reading for three lines. */ +const SIZE = 2_000_000 +/** Bound the mtime cache so a long session cannot grow it without limit. */ +const CACHE = 200 + +const cache = new Map() + +export function clearContextCache(): void { + cache.clear() +} + +/** + * The last line of a hunk, without its diff marker. GitHub always ends the + * hunk at the commented line, so this is the text the comment refers to. + */ +function anchor(hunk: string): string | undefined { + const lines = hunk.split("\n").filter((line) => line.length > 0 && !line.startsWith("\\")) + const last = lines.at(-1) + return lines.length > 1 && last ? last.slice(1) : undefined +} + +async function lines(dir: string, file: string): Promise { + const full = path.join(dir, file) + const info = await stat(full).catch(() => undefined) + if (!info?.isFile() || info.size > SIZE) return undefined + const hit = cache.get(full) + if (hit && hit.mtime === info.mtimeMs) return hit.lines + const text = await readFile(full, "utf8").catch(() => undefined) + if (text === undefined) return undefined + const value = text.split("\n") + if (cache.size >= CACHE) cache.clear() + cache.set(full, { mtime: info.mtimeMs, lines: value }) + return value +} + +/** + * Attach trailing context to every thread whose file still matches its hunk. + * The anchor check is what keeps this honest: once the agent edits the file, + * the commented line no longer matches and the thread keeps hunk-only context + * instead of showing unrelated code. + */ +export async function withContext(dir: string, comments: PRComment[]): Promise { + return Promise.all( + comments.map(async (item) => { + if (!item.file || !item.line || !item.diffHunk || item.outdated) return item + const text = anchor(item.diffHunk) + if (text === undefined) return item + const source = await lines(dir, item.file) + if (!source || source[item.line - 1] !== text) return item + const after = source.slice(item.line, item.line + AFTER) + return after.length > 0 ? { ...item, after } : item + }), + ) +} diff --git a/packages/kilo-vscode/src/agent-manager/types.ts b/packages/kilo-vscode/src/agent-manager/types.ts index 9029cb63ae9..498b5664be1 100644 --- a/packages/kilo-vscode/src/agent-manager/types.ts +++ b/packages/kilo-vscode/src/agent-manager/types.ts @@ -77,6 +77,8 @@ export interface PRComment { outdated: boolean createdAt?: number diffHunk?: string + /** Lines after the commented line, read from the worktree: a hunk has none. */ + after?: string[] replies?: PRCommentReply[] } diff --git a/packages/kilo-vscode/tests/fixtures/pr-comments-render.tsx b/packages/kilo-vscode/tests/fixtures/pr-comments-render.tsx index bc971ee4465..e9bb18b8b11 100644 --- a/packages/kilo-vscode/tests/fixtures/pr-comments-render.tsx +++ b/packages/kilo-vscode/tests/fixtures/pr-comments-render.tsx @@ -39,6 +39,7 @@ const { MarkedProvider } = await import("@kilocode/kilo-ui/context/marked") const { VSCodeProvider } = await import("../../webview-ui/src/context/vscode") const { LanguageProvider } = await import("../../webview-ui/src/context/language") const { PRComments } = await import("../../webview-ui/agent-manager/pr/PRComments") +const { createSignal } = await import("solid-js") const root = document.createElement("div") const colors = document.createElement("style") @@ -50,6 +51,36 @@ const HUNK = '@@ -1 +1,14 @@\n+import { File as BaseFile, type FileProps } from "@opencode-ai/ui/file"\n+import type { JSX } from "solid-js"\n+import { createDefaultOptions } from "../pierre"\n+\n export * from "@opencode-ai/ui/file"\n+\n+export function File(props: FileProps) {\n+ const View = BaseFile as unknown as (props: FileProps) => JSX.Element\n+ if (props.mode === "text") return \n+\n+ // Keep inline file diffs on the same Pierre defaults as the dedicated viewer.\n+ const options = { ...createDefaultOptions(props.diffStyle), ...props } as FileProps\n' const sent: unknown[] = [] +const [comments, setComments] = createSignal({ + total: 2, + unresolved: 1, + comments: [ + { + id: "PRRC_open", + threadId: "PRRT_open", + author: "kilo-code-bot", + body: "comment body survives Pierre rendering", + file: "packages/kilo-ui/src/components/file.tsx", + line: 14, + resolved: false, + outdated: false, + diffHunk: HUNK, + // Read from the worktree by the extension: a hunk stops at the commented line. + after: [" return ", "}", ""], + replies: [{ author: "marius", body: "reply body is visible" }], + }, + { + id: "PRRC_done", + threadId: "PRRT_done", + author: "reviewer", + body: "settled discussion\n\nsecond paragraph only shows when expanded", + file: "packages/kilo-ui/src/components/other.tsx", + line: 3, + resolved: true, + outdated: false, + }, + ], +}) window.addEventListener("message", (ev: MessageEvent) => { if (ev.data?.type === "appendReviewComments") sent.push(ev.data) }) @@ -59,37 +90,7 @@ const dispose = render( - + @@ -103,15 +104,26 @@ await window.happyDOM.waitUntilComplete() const host = root.querySelector("diffs-container") const shadow = host?.shadowRoot const keyword = shadow?.querySelector('[data-content] span[style*="--syntax-keyword"]') -const string = shadow?.querySelector('[data-content] span[style*="--syntax-string"]') +const comment = shadow?.querySelector('[data-content] span[style*="--syntax-comment"]') +const code = shadow?.querySelectorAll("[data-content] [data-line]") assert.match(root.textContent ?? "", /comment body survives Pierre rendering/) assert.match(root.textContent ?? "", /reply body is visible/) assert.equal(root.querySelectorAll('[data-component="diff"]').length, 1) +// Four hunk lines ending at the commented line, like the GitHub comment +// snippet, then the worktree lines below it so a comment about what happens +// next is readable. No collapsed-context row counting the lines above them. +assert.equal(code?.length, 7) +assert.deepEqual( + [...(code ?? [])].map((node) => node.getAttribute("data-line-type")), + ["change-addition", "change-addition", "change-addition", "change-addition", "context", "context", "context"], +) +assert.match(shadow?.textContent ?? "", /return /) +assert.doesNotMatch(shadow?.textContent ?? "", /unmodified line/) assert.ok(keyword) -assert.ok(string) +assert.ok(comment) assert.match(keyword!.getAttribute("style") ?? "", /--syntax-keyword/) -assert.match(string!.getAttribute("style") ?? "", /--syntax-string/) -assert.notEqual(keyword!.getAttribute("style"), string!.getAttribute("style")) +assert.match(comment!.getAttribute("style") ?? "", /--syntax-comment/) +assert.notEqual(keyword!.getAttribute("style"), comment!.getAttribute("style")) // The resolved thread is hidden behind a collapsed group. assert.doesNotMatch(root.textContent ?? "", /settled discussion/) @@ -141,6 +153,15 @@ assert.ok(unresolve, "unresolve button is rendered") assert.equal((unresolve as HTMLButtonElement).disabled, false) assert.equal(unresolve!.getAttribute("data-disabled"), null) +// Polling replaces comment objects, but it must not reset the user's open thread. +setComments((prev) => ({ ...prev, comments: prev.comments.map((item) => ({ ...item })) })) +await window.happyDOM.waitUntilComplete() +const refreshedRow = [...root.querySelectorAll(".am-pr-comment-head")].find((node) => + /reviewer/.test(node.textContent ?? ""), +) +assert.equal(refreshedRow?.getAttribute("aria-expanded"), "true") +assert.match(root.textContent ?? "", /second paragraph only shows when expanded/) + // Send to agent hands the thread over as a structured review comment. const send = [...root.querySelectorAll('[data-component="button"]')].find((node) => /Send to agent/.test(node.textContent ?? ""), @@ -161,4 +182,54 @@ await window.happyDOM.waitUntilComplete() assert.equal(sent.length, 1) assert.equal((send as HTMLButtonElement).disabled, true) +// A poll that resolves the other thread regroups the list. Cards are keyed by +// thread, so the expanded card must not hand its state to its new neighbour. +setComments((prev) => ({ + ...prev, + unresolved: 0, + comments: prev.comments.map((item) => (item.threadId === "PRRT_open" ? { ...item, resolved: true } : item)), +})) +await window.happyDOM.waitUntilComplete() +const byThread = new Map( + [...root.querySelectorAll(".am-pr-comment[data-thread-id]")].map((node) => [ + node.getAttribute("data-thread-id"), + node, + ]), +) +assert.equal(byThread.size, 2) +assert.equal(byThread.get("PRRT_done")?.querySelector(".am-pr-comment-head")?.getAttribute("aria-expanded"), "true") +assert.equal(byThread.get("PRRT_open")?.querySelector(".am-pr-comment-head")?.getAttribute("aria-expanded"), "false") +assert.match(root.textContent ?? "", /second paragraph only shows when expanded/) + +// A remount must not lobotomize the panel. The extension can briefly report no +// PR, which tears these components down and builds them again. What the user +// opened, and what was already sent, are held per worktree outside the +// component, so both survive. dispose() +const second = document.createElement("div") +document.body.append(second) +const disposeSecond = render( + () => ( + + + + + + + + ), + second, +) +await window.happyDOM.waitUntilComplete() +const revived = new Map( + [...second.querySelectorAll(".am-pr-comment[data-thread-id]")].map((node) => [ + node.getAttribute("data-thread-id"), + node, + ]), +) +// The resolved group is still open, so both threads are reachable. +assert.equal(revived.size, 2) +assert.equal(revived.get("PRRT_done")?.querySelector(".am-pr-comment-head")?.getAttribute("aria-expanded"), "true") +assert.match(second.textContent ?? "", /second paragraph only shows when expanded/) +assert.match(second.textContent ?? "", /Sent/) +disposeSecond() diff --git a/packages/kilo-vscode/tests/unit/am-pr-status-bridge.test.ts b/packages/kilo-vscode/tests/unit/am-pr-status-bridge.test.ts index 0b03bfd8379..12d403fb704 100644 --- a/packages/kilo-vscode/tests/unit/am-pr-status-bridge.test.ts +++ b/packages/kilo-vscode/tests/unit/am-pr-status-bridge.test.ts @@ -23,7 +23,9 @@ const pr: PRStatus = { function harness(opts: { hasPersisted?: boolean } = {}) { const sent: AgentManagerOutMessage[] = [] - const worktrees: { id: string; path: string; prUrl?: string }[] = [{ id: "wt1", path: "/repo/wt1" }] + const worktrees: { id: string; path: string; branch: string; prUrl?: string }[] = [ + { id: "wt1", path: "/repo/wt1", branch: "feature" }, + ] const bridge = PRStatusBridge.create({ getWorktrees: () => worktrees as never, getWorkspaceRoot: () => "/repo", @@ -34,7 +36,7 @@ function harness(opts: { hasPersisted?: boolean } = {}) { log: () => {}, }) const onStatus = (bridge.poller as unknown as { options: { onStatus: (...a: unknown[]) => void } }).options.onStatus - return { bridge, sent, onStatus } + return { bridge, sent, onStatus, worktrees } } // --- error deduplication --- @@ -110,6 +112,44 @@ describe("PRStatusBridge onStatus", () => { const errorMsg = sent.find((m) => m.type === "agentManager.prError") expect(errorMsg).toEqual(expect.objectContaining({ error: "gh_missing" })) }) + + // A rate limit, a network blip, or an unresolvable fork ref all look like "no + // pull request". Forwarding that unmounts the panel and discards what the user + // has open, so a PR already found on this branch stays. + it("keeps a known PR when a poll finds no pull request on the same branch", () => { + const { bridge, sent, onStatus } = harness() + onStatus("wt1", pr) + sent.length = 0 + onStatus("wt1", null) + expect(sent).toHaveLength(0) + expect(bridge.snapshot().get("wt1")).toEqual(pr) + }) + + it("drops the PR once the worktree is on another branch", () => { + const { bridge, sent, onStatus, worktrees } = harness() + onStatus("wt1", pr) + sent.length = 0 + worktrees[0]!.branch = "other" + onStatus("wt1", null) + expect(sent).toEqual([expect.objectContaining({ type: "agentManager.prStatus", worktreeId: "wt1", pr: null })]) + expect(bridge.snapshot().has("wt1")).toBe(false) + }) + + it("forwards no pull request for a worktree that never had one", () => { + const { sent, onStatus } = harness() + onStatus("wt1", null) + expect(sent).toEqual([expect.objectContaining({ type: "agentManager.prStatus", worktreeId: "wt1", pr: null })]) + }) + + it("reports the PR again after a branch switch back", () => { + const { bridge, onStatus, worktrees } = harness() + onStatus("wt1", pr) + worktrees[0]!.branch = "other" + onStatus("wt1", null) + worktrees[0]!.branch = "feature" + onStatus("wt1", pr) + expect(bridge.snapshot().get("wt1")).toEqual(pr) + }) }) // --- replay --- diff --git a/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts b/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts new file mode 100644 index 00000000000..0e452042f5f --- /dev/null +++ b/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it, beforeEach } from "bun:test" +import { mkdtemp, mkdir, utimes, writeFile } from "node:fs/promises" +import path from "node:path" +import { tmpdir } from "node:os" +import type { PRComment } from "../../src/agent-manager/types" +import { clearContextCache, withContext } from "../../src/agent-manager/pr/pr-comment-context" + +const SOURCE = [ + "export function open(file: string) {", + " const event = new CustomEvent('kilo:open-file')", + " event.preventDefault()", + " return dispatch(event)", + "}", + "", +] + +// The hunk ends at line 3, the way GitHub truncates every diffHunk. +const HUNK = [ + "@@ -1,2 +1,3 @@", + " export function open(file: string) {", + "+ const event = new CustomEvent('kilo:open-file')", + "+ event.preventDefault()", +].join("\n") + +function thread(over: Partial = {}): PRComment { + return { + id: "PRRC_1", + threadId: "PRRT_1", + author: "kilo-code-bot", + body: "preventDefault runs before the file opens", + file: "src/open.ts", + line: 3, + resolved: false, + outdated: false, + diffHunk: HUNK, + ...over, + } +} + +async function repo(lines = SOURCE): Promise { + const dir = await mkdtemp(path.join(tmpdir(), "kilo-pr-context-")) + await mkdir(path.join(dir, "src")) + await write(dir, lines) + return dir +} + +async function write(dir: string, lines: string[], mtime = new Date(1_700_000_000_000)): Promise { + const file = path.join(dir, "src/open.ts") + await writeFile(file, lines.join("\n")) + await utimes(file, mtime, mtime) +} + +describe("withContext", () => { + beforeEach(() => { + clearContextCache() + }) + + it("continues the hunk with the lines below the commented line", async () => { + const dir = await repo() + const [item] = await withContext(dir, [thread()]) + + expect(item!.after).toEqual([" return dispatch(event)", "}", ""]) + }) + + it("stops at the end of the file", async () => { + const dir = await repo(SOURCE.slice(0, 4)) + const [item] = await withContext(dir, [thread()]) + + expect(item!.after).toEqual([" return dispatch(event)"]) + }) + + // The agent rewrites files while the review is open. Context read from a file + // that no longer matches the hunk would show code from the wrong place. + it("attaches nothing when the commented line no longer matches the file", async () => { + const dir = await repo(["export function open(file: string) {", " return dispatch(file)", "}", ""]) + const [item] = await withContext(dir, [thread()]) + + expect(item!.after).toBeUndefined() + }) + + it("re-reads a file once its mtime changes", async () => { + const dir = await repo() + await withContext(dir, [thread()]) + await write(dir, [...SOURCE.slice(0, 3), " return open(file)", "}", ""], new Date(1_700_000_600_000)) + const [item] = await withContext(dir, [thread()]) + + expect(item!.after).toEqual([" return open(file)", "}", ""]) + }) + + it("leaves outdated threads, missing files, and hunk-less threads untouched", async () => { + const dir = await repo() + const items = await withContext(dir, [ + thread({ threadId: "outdated", outdated: true }), + thread({ threadId: "gone", file: "src/missing.ts" }), + thread({ threadId: "bodyOnly", diffHunk: undefined }), + thread({ threadId: "noLine", line: undefined }), + ]) + + expect(items.map((item) => item.after)).toEqual([undefined, undefined, undefined, undefined]) + }) + + it("keeps every thread, in order, whatever the files say", async () => { + const dir = await repo() + const items = await withContext(dir, [thread({ threadId: "a" }), thread({ threadId: "b", outdated: true })]) + + expect(items.map((item) => item.threadId)).toEqual(["a", "b"]) + }) +}) diff --git a/packages/kilo-vscode/tests/unit/pr-status-merge.test.ts b/packages/kilo-vscode/tests/unit/pr-status-merge.test.ts new file mode 100644 index 00000000000..1b04fb5ef74 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/pr-status-merge.test.ts @@ -0,0 +1,62 @@ +/** + * PR status merging + * + * Only the selected worktree fetches review threads, and that GraphQL call can + * fail. A plain replace would blank the comment list in the open panel, so a + * status without threads keeps the ones already reported for the same PR. + */ +import { describe, it, expect } from "bun:test" +import { mergePRStatus } from "../../src/agent-manager/pr/am-pr-utils" +import type { PRStatus } from "../../src/agent-manager/types" + +function status(overrides: Partial = {}): PRStatus { + return { + number: 42, + title: "feat: add document inspector", + url: "https://github.com/org/repo/pull/42", + state: "open", + review: null, + checks: { status: "success", total: 1, passed: 1, failed: 0, pending: 0, checks: [] }, + additions: 1, + deletions: 0, + files: 1, + ...overrides, + } +} + +const threads = { + total: 2, + unresolved: 1, + comments: [ + { + id: "PRRC_1", + threadId: "PRRT_1", + author: "kilo-code-bot", + body: "guard this", + resolved: false, + outdated: false, + }, + ], +} + +describe("mergePRStatus", () => { + it("keeps the previous threads when a refresh reports none", () => { + const next = mergePRStatus(status({ comments: threads }), status({ title: "renamed" })) + + expect(next.title).toBe("renamed") + expect(next.comments).toEqual(threads) + }) + + it("prefers the threads reported by the refresh", () => { + const fresh = { total: 1, unresolved: 0, comments: [] } + expect(mergePRStatus(status({ comments: threads }), status({ comments: fresh })).comments).toEqual(fresh) + }) + + it("drops threads that belong to another pull request", () => { + expect(mergePRStatus(status({ comments: threads }), status({ number: 43 })).comments).toBeUndefined() + }) + + it("passes the first status through untouched", () => { + expect(mergePRStatus(undefined, status()).comments).toBeUndefined() + }) +}) diff --git a/packages/kilo-vscode/tests/unit/review-comments-pr.test.ts b/packages/kilo-vscode/tests/unit/review-comments-pr.test.ts index 1d8547c7fe4..6c9786ea87c 100644 --- a/packages/kilo-vscode/tests/unit/review-comments-pr.test.ts +++ b/packages/kilo-vscode/tests/unit/review-comments-pr.test.ts @@ -15,7 +15,13 @@ import { type PRReviewCommentData, type ReviewCommentData, } from "../../src/shared/review-comments" -import { githubUrl, prMarkdown, prPayload, preview } from "../../webview-ui/agent-manager/pr/pr-comment-payload" +import { + displayHunk, + githubUrl, + prMarkdown, + prPayload, + preview, +} from "../../webview-ui/agent-manager/pr/pr-comment-payload" import type { PRComment } from "../../webview-ui/agent-manager/pr/pr-types" function pr(overrides: Partial = {}): PRReviewCommentData { @@ -140,6 +146,102 @@ describe("prPayload", () => { expect(payload.diffHunk?.endsWith("line 79")).toBe(true) }) + it("crops a full-file hunk around the commented line", () => { + const hunk = ["@@ -1 +1,80 @@", ...Array.from({ length: 80 }, (_, i) => `+line ${i + 1}`)].join("\n") + const view = displayHunk(hunk, 70) + const lines = view.patch.split("\n") + + expect(lines).toHaveLength(8) + expect(lines[0]).toBe("@@ -1,0 +67,7 @@") + expect(lines[1]).toBe("+line 67") + expect(lines).toContain("+line 70") + expect(lines.at(-1)).toBe("+line 73") + expect(view.top).toBe(true) + expect(view.bottom).toBe(true) + }) + + // GitHub truncates diffHunk at the commented line, so hunk length says nothing + // about how much context a comment deserves. Every card renders one window: + // three lines, the commented line, then three more from the hunk when it has + // them and from the worktree when it does not. + it("renders the same window whatever the hunk length", () => { + const build = (count: number) => + [`@@ -0,0 +1,${count} @@`, ...Array.from({ length: count }, (_, i) => `+line ${i + 1}`)].join("\n") + const short = displayHunk(build(34), 34) + const long = displayHunk(build(172), 168) + + expect(short.lines.map((item) => item.text)).toEqual(["31", "32", "33", "34"].map((n) => `+line ${n}`)) + expect(long.lines.map((item) => item.text)).toEqual( + ["165", "166", "167", "168", "169", "170", "171"].map((n) => `+line ${n}`), + ) + expect(short.top).toBe(true) + expect(short.bottom).toBe(false) + expect(long.top).toBe(true) + expect(long.bottom).toBe(true) + }) + + // A hunk stops at the commented line, so a warning about what runs next has + // nothing to point at. The worktree lines continue the snippet as context. + it("continues the snippet with worktree lines below the commented line", () => { + const hunk = [ + "@@ -1,1 +1,3 @@", + " function open() {", + "+ const event = build()", + "+ event.preventDefault()", + ].join("\n") + const view = displayHunk(hunk, 3, [" return dispatch(event)", "}", "", "extra"]) + + expect(view.patch.split("\n")).toEqual([ + "@@ -1,4 +1,6 @@", + " function open() {", + "+ const event = build()", + "+ event.preventDefault()", + " return dispatch(event)", + " }", + " ", + ]) + expect(view.bottom).toBe(true) + }) + + it("keeps the GitHub window when the worktree has no matching context", () => { + const hunk = [ + "@@ -1,1 +1,3 @@", + " function open() {", + "+ const event = build()", + "+ event.preventDefault()", + ].join("\n") + const view = displayHunk(hunk, 3) + + expect(view.lines).toHaveLength(3) + expect(view.bottom).toBe(false) + }) + + it("gives the agent the worktree context too", () => { + const hunk = ["@@ -1,1 +1,2 @@", " function open() {", "+ event.preventDefault()"].join("\n") + const payload = prPayload(thread({ diffHunk: hunk, line: 2, after: [" return dispatch(event)", "}"] })) + + expect(payload.diffHunk?.split("\n")).toEqual([ + "@@ -1,3 +1,4 @@", + " function open() {", + "+ event.preventDefault()", + " return dispatch(event)", + " }", + "...", + ]) + }) + + it("gives the agent more of the hunk than the card renders", () => { + const hunk = ["@@ -0,0 +1,80 @@", ...Array.from({ length: 80 }, (_, i) => `+line ${i + 1}`)].join("\n") + const payload = prPayload(thread({ diffHunk: hunk, line: 40 })) + const lines = payload.diffHunk?.split("\n") ?? [] + + expect(lines[0]).toBe("@@ -0,0 +16,33 @@") + expect(lines[1]).toBe("...") + expect(lines).toContain("+line 40") + expect(lines.at(-1)).toBe("...") + expect(lines.length).toBeGreaterThan(displayHunk(hunk, 40).lines.length) + }) + it("caps a single-line hunk by characters", () => { const payload = prPayload(thread({ diffHunk: `@@ -1 +1 @@ ${"x".repeat(20_000)}` })) expect(payload.diffHunk!.length).toBeLessThan(9_000) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRCommentCard.tsx b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRCommentCard.tsx index 9927b9979f4..0088a3ac5e5 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRCommentCard.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRCommentCard.tsx @@ -35,7 +35,7 @@ export function PRCommentCard(props: Props) { } return ( -
+
- {card} + {card}
0}>
setDoneOpen((v) => !v)} + open={state().doneOpen} + onToggle={() => patch((prev) => ({ doneOpen: !prev.doneOpen }))} /> - +
- {card} + {card}
diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx index 850de08039a..d7a1facd451 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx @@ -1,5 +1,5 @@ /** @jsxImportSource solid-js */ -import { Component, Show } from "solid-js" +import { Component, Show, createEffect, createMemo, on, onCleanup } from "solid-js" import { IconButton } from "@kilocode/kilo-ui/icon-button" import { Tooltip } from "@kilocode/kilo-ui/tooltip" import type { WorktreeState } from "../../src/types/messages" @@ -10,6 +10,7 @@ import { PRReviewers } from "./PRReviewers" import { PRDescription } from "./PRDescription" import { PRChecks } from "./PRChecks" import { PRComments } from "./PRComments" +import { commentScroll, setCommentScroll } from "./pr-comment-state" import { PRSummary } from "./PRSummary" import "./pr-panel.css" @@ -26,11 +27,86 @@ interface PRPanelProps { export const PRPanel: Component = (props) => { let commentsRef: HTMLDivElement | undefined + let bodyRef: HTMLDivElement | undefined + let capture: number | undefined + let restore: number | undefined + + // A poll replaces the whole status, so the panel re-renders, and sometimes + // remounts, while the user reads. Anchoring on the topmost visible thread + // keeps that thread still even when a section above it grows, which a raw + // scrollTop cannot do. Both live outside the component so a remount restores + // the same position instead of jumping to the top. + const remember = () => { + if (!bodyRef) return + const top = bodyRef.getBoundingClientRect().top + for (const node of bodyRef.querySelectorAll("[data-thread-id]")) { + const box = node.getBoundingClientRect() + if (box.bottom <= top) continue + const id = node.dataset.threadId + if (id) setCommentScroll(props.worktreeId, bodyRef.scrollTop, { id, offset: box.top - top }) + return + } + setCommentScroll(props.worktreeId, bodyRef.scrollTop) + } + + const reposition = () => { + if (!bodyRef) return + const saved = commentScroll(props.worktreeId) + if (!saved) return + const anchor = saved.anchor + const node = anchor ? bodyRef.querySelector(`[data-thread-id="${anchor.id}"]`) : undefined + if (node && anchor) { + const delta = node.getBoundingClientRect().top - bodyRef.getBoundingClientRect().top - anchor.offset + if (Math.abs(delta) >= 1) bodyRef.scrollTop += delta + setCommentScroll(props.worktreeId, bodyRef.scrollTop, anchor) + return + } + const max = Math.max(0, bodyRef.scrollHeight - bodyRef.clientHeight) + bodyRef.scrollTop = Math.min(saved.scroll, max) + } + + // Two frames: the first lets Solid flush the new DOM, the second lets Pierre + // finish rendering the hunks that decide the final height. + const later = () => { + if (restore !== undefined) cancelAnimationFrame(restore) + restore = requestAnimationFrame(() => { + restore = requestAnimationFrame(() => { + restore = undefined + reposition() + }) + }) + } + + createEffect(on(() => props.worktreeId, later)) + createEffect(on(() => props.pr, later, { defer: true })) + + onCleanup(() => { + if (capture !== undefined) cancelAnimationFrame(capture) + if (restore !== undefined) cancelAnimationFrame(restore) + }) function jumpToComments() { commentsRef?.scrollIntoView({ behavior: "smooth", block: "start" }) } + function onScroll() { + if (capture !== undefined) return + capture = requestAnimationFrame(() => { + capture = undefined + remember() + }) + } + + // Only the selected worktree fetches threads, and that fetch can fail, so a + // refresh can arrive without comments. Dropping the section would discard the + // expanded card and the scroll position, so the last list for this PR stays. + const comments = createMemo<{ number: number; value: NonNullable } | undefined>((prev) => { + const next = props.pr.comments + if (next?.total) return { number: props.pr.number, value: next } + if (prev && prev.number === props.pr.number) return prev + return undefined + }) + return (
@@ -55,7 +131,7 @@ export const PRPanel: Component = (props) => {
-
+
0}> @@ -65,11 +141,11 @@ export const PRPanel: Component = (props) => { 0}> - - {(comments) => ( + + {(item) => (
max ? `${value.slice(0, max)}\n...` : value } -/** Keep the `@@` header and the tail: the commented line sits at the end. */ -function trim(value: string): string { +function parseHunk(value: string): { header: string; lines: HunkLine[] } | undefined { + const source = value.split("\n") + const match = source[0]?.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$/) + if (!match) return undefined + + let old = Number(match[1]) + let next = Number(match[3]) + const lines: HunkLine[] = [] + for (const [index, raw] of source.slice(1).entries()) { + if (raw === "" && index === source.length - 2) continue + if (raw.startsWith("\\")) continue + const text = raw === "" ? " " : raw + const mark = text[0] + if (mark !== " " && mark !== "+" && mark !== "-") return undefined + const item = { text, old, next } + lines.push(item) + if (mark !== "+") old++ + if (mark !== "-") next++ + } + + return { + header: `@@ -${match[1]}${match[2] ? `,${match[2]}` : ""} +${match[3]}${match[4] ? `,${match[4]}` : ""} @@${match[5]}`, + lines, + } +} + +function range(start: number, count: number): string { + return count === 1 ? `${start}` : `${start},${count}` +} + +function patch(header: string, lines: HunkLine[]): string { + const first = lines[0] + if (!first) return header + const oldCount = lines.filter((line) => line.text[0] !== "+").length + const nextCount = lines.filter((line) => line.text[0] !== "-").length + const suffix = header.slice(header.lastIndexOf("@@") + 2) + const title = `@@ -${range(first.old, oldCount)} +${range(first.next, nextCount)} @@${suffix}` + return `${title}\n${lines.map((line) => line.text).join("\n")}` +} + +/** Index of the line the comment was written against, or the end of the hunk. */ +function target(lines: HunkLine[], line?: number): number { + if (!line) return lines.length - 1 + const added = lines.findIndex((item) => item.next === line && item.text[0] === "+") + if (added >= 0) return added + const removed = lines.findIndex((item) => item.old === line && item.text[0] === "-") + if (removed >= 0) return removed + const kept = lines.findIndex((item) => item.next === line && item.text[0] === " ") + return kept >= 0 ? kept : lines.length - 1 +} + +/** + * Continue the hunk with lines read from the worktree. A hunk stops at the + * commented line, so a comment about what happens next has nothing to show + * without them, and they are unmodified code: context lines, never additions. + */ +function extend(lines: HunkLine[], after: string[], count: number): HunkLine[] { + const last = lines.at(-1) + if (!last || count === 0) return lines + const start = + last.text[0] === "-" + ? { old: last.old + 1, next: last.next } + : last.text[0] === "+" + ? { old: last.old, next: last.next + 1 } + : { old: last.old + 1, next: last.next + 1 } + return [ + ...lines, + ...after + .slice(0, count) + .map((text, index) => ({ text: ` ${text}`, old: start.old + index, next: start.next + index })), + ] +} + +/** Fixed-size window around the commented line, independent of hunk length. */ +function crop( + value: string, + line: number | undefined, + window: { before: number; after: number }, + after?: string[], +): HunkView { + const parsed = parseHunk(value) + if (!parsed || parsed.lines.length === 0) { + return { header: "", lines: [], patch: value, top: false, bottom: false } + } + const index = target(parsed.lines, line) + const start = Math.max(0, index - window.before) + const end = Math.min(parsed.lines.length, index + window.after + 1) + const room = window.after - (end - index - 1) + const tail = end === parsed.lines.length && after?.length && room > 0 ? after : [] + const lines = extend(parsed.lines.slice(start, end), tail, Math.min(room, tail.length)) + return { + header: parsed.header, + lines, + patch: patch(parsed.header, lines), + top: start > 0, + bottom: end < parsed.lines.length || tail.length > 0, + } +} + +/** Bounded context rendered inside a comment card. */ +export function displayHunk(value: string, line?: number, after?: string[]): HunkView { + return crop(value, line, VIEW, after) +} + +/** Keep the `@@` header and the comment context when formatting agent input. */ +function trim(value: string, line?: number, after?: string[]): string { + const view = crop(value, line, SEND, after) + if (view.lines.length === 0) return trimFallback(value) + if (!view.top && !view.bottom) return clip(value, HUNK_CHARS) + const body = view.patch.split("\n") + const lines = [body[0]!, ...(view.top ? ["..."] : []), ...body.slice(1), ...(view.bottom ? ["..."] : [])] + return clip(lines.join("\n"), HUNK_CHARS) +} + +/** Keep the `@@` header and the tail for malformed hunks without line metadata. */ +function trimFallback(value: string): string { const lines = value.split("\n") const cut = lines.length <= HUNK ? lines : [lines[0]!, "...", ...lines.slice(-HUNK)] return clip(cut.join("\n"), HUNK_CHARS) @@ -32,7 +169,7 @@ export function prPayload(comment: PRComment): PRReviewCommentData { body: clip(comment.body, BODY), file: comment.file, line: comment.line, - diffHunk: comment.diffHunk ? trim(comment.diffHunk) : undefined, + diffHunk: comment.diffHunk ? trim(comment.diffHunk, comment.line, comment.after) : undefined, outdated: comment.outdated || undefined, replies: replies.length > 0 ? replies : undefined, } diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-comment-state.ts b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-comment-state.ts new file mode 100644 index 00000000000..0d5ac79f959 --- /dev/null +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-comment-state.ts @@ -0,0 +1,70 @@ +/** + * PR panel view state, held outside the components that render it. + * + * Anything that briefly clears the PR status remounts the panel: a poll that + * cannot reach `gh`, a reselect, a side panel toggle. Component-local state + * dies with that remount, which closes threads the user opened and sends the + * scroll back to the top. Keying by worktree also means leaving a worktree and + * returning restores the threads that were open there. + */ +import { createSignal } from "solid-js" + +export interface CommentState { + /** threadId -> expansion override; the default follows resolved/outdated. */ + expanded: Record + /** threadId -> already handed to the agent. */ + sent: Record + /** threadId -> resolved state the user asked for, until a poll confirms it. */ + pending: Record + /** threadId -> message from a resolve that failed. */ + errors: Record + open: boolean + doneOpen: boolean +} + +export interface CommentAnchor { + id: string + offset: number +} + +const BLANK: CommentState = Object.freeze({ + expanded: {}, + sent: {}, + pending: {}, + errors: {}, + open: true, + doneOpen: false, +}) + +const [all, setAll] = createSignal>({}) + +export function commentState(worktree: string): CommentState { + return all()[worktree] ?? BLANK +} + +export function patchCommentState(worktree: string, patch: (prev: CommentState) => Partial): void { + setAll((prev) => { + const current = prev[worktree] ?? BLANK + return { ...prev, [worktree]: { ...current, ...patch(current) } } + }) +} + +export function omit(map: Record, id: string): Record { + const next = { ...map } + delete next[id] + return next +} + +/** + * Scroll position, deliberately not reactive: it is written on every scroll + * frame, and a signal would re-render every card that reads thread state. + */ +const positions = new Map() + +export function commentScroll(worktree: string): { scroll: number; anchor?: CommentAnchor } | undefined { + return positions.get(worktree) +} + +export function setCommentScroll(worktree: string, scroll: number, anchor?: CommentAnchor): void { + positions.set(worktree, { scroll, anchor }) +} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-panel.css b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-panel.css index 377347ff433..b34a25401b8 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-panel.css +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-panel.css @@ -364,6 +364,16 @@ margin: 6px 8px 0; } +.am-pr-diff-context-marker { + padding: 2px 8px; + color: var(--text-weak); + background: var(--vscode-textCodeBlock-background); + font-family: var(--font-mono, monospace); + font-size: var(--kilo-font-size-11); + line-height: 1.4; + text-align: center; +} + .am-pr-panel-description { line-height: 1.5; padding: 2px 0 4px; diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-types.ts b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-types.ts index 2e788e2985f..9b5c8359f8e 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-types.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/pr-types.ts @@ -31,6 +31,8 @@ export interface PRComment { outdated: boolean createdAt?: number diffHunk?: string + /** Lines after the commented line, read from the worktree: a hunk has none. */ + after?: string[] replies?: PRCommentReply[] } diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/PRCommentDiff.tsx b/packages/kilo-vscode/webview-ui/diff-viewer/PRCommentDiff.tsx index e2b7b324c8d..9042340996a 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/PRCommentDiff.tsx +++ b/packages/kilo-vscode/webview-ui/diff-viewer/PRCommentDiff.tsx @@ -1,15 +1,32 @@ import { Show, createMemo } from "solid-js" import { Diff } from "@kilocode/kilo-ui/diff" import { normalizeHunk } from "@kilocode/kilo-ui/session-diff" +import { displayHunk } from "../agent-manager/pr/pr-comment-payload" -export function PRCommentDiff(props: { file: string; hunk: string }) { - const view = createMemo(() => normalizeHunk(props.file, props.hunk)) +export function PRCommentDiff(props: { file: string; line?: number; hunk: string; after?: string[] }) { + const input = createMemo( + () => ({ file: props.file, line: props.line, hunk: props.hunk, after: (props.after ?? []).join("\n") }), + undefined, + { equals: (a, b) => a.file === b.file && a.line === b.line && a.hunk === b.hunk && a.after === b.after }, + ) + const view = createMemo(() => { + const data = input() + const hunk = displayHunk(data.hunk, data.line, data.after ? data.after.split("\n") : undefined) + const value = normalizeHunk(data.file, hunk.patch) + return value ? { hunk, value } : undefined + }) return ( {(value) => (
- + +
...
+ + + +
...
+
)} From a9e9570b6f085d9cdc839be574945f81e26347fd Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 20 Aug 2026 13:21:38 +0200 Subject: [PATCH 2/5] test(vscode): add PR comment context story data --- .../kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx index 90b79c3ec1e..82c3ea2782c 100644 --- a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx +++ b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx @@ -1507,6 +1507,7 @@ const prComments: NonNullable = { outdated: false, diffHunk: '@@ -39,7 +39,7 @@ export function execGhRead(args: string[]) {\n- return execWithShellEnv("gh", args, options)\n+ return execWithShellEnv("gh", args, { ...options, env: env(options) })', + after: [" return result", "}", ""], replies: [{ author: "hubot", body: "Agreed. A guard plus a log line is enough here." }], }, { From 3f770e66f7a9d0f67dd0b75a3627498a1cf58cc0 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 20 Aug 2026 14:22:41 +0200 Subject: [PATCH 3/5] fix(agent-manager): harden PR refresh state --- packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts | 2 +- .../src/agent-manager/pr/pr-comment-context.ts | 8 ++++++-- .../kilo-vscode/tests/unit/pr-comment-context.test.ts | 8 ++++++++ .../kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts b/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts index bdd1473cbc6..d49d360b921 100644 --- a/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts +++ b/packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts @@ -249,7 +249,7 @@ export class PRStatusPoller { const pr = await this.cachedFetchPR(wt.branch, wt.path) if (!pr || this.stale(generation)) { if (this.stale(generation)) return - const hash = `${worktreeId}:none` + const hash = `${worktreeId}:${wt.branch}:none` if (this.lastHash.get(worktreeId) === hash) return this.lastHash.set(worktreeId, hash) this.options.onStatus(worktreeId, null) diff --git a/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts b/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts index 5a2d8af710e..6862aaabebb 100644 --- a/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts +++ b/packages/kilo-vscode/src/agent-manager/pr/pr-comment-context.ts @@ -4,7 +4,7 @@ * the file, so the lines below the comment are read from disk and attached to * the thread. */ -import { readFile, stat } from "node:fs/promises" +import { readFile, realpath, stat } from "node:fs/promises" import path from "node:path" import type { PRComment } from "../types" @@ -32,7 +32,11 @@ function anchor(hunk: string): string | undefined { } async function lines(dir: string, file: string): Promise { - const full = path.join(dir, file) + const root = await realpath(dir).catch(() => undefined) + const full = await realpath(path.resolve(dir, file)).catch(() => undefined) + if (!root || !full) return undefined + const rel = path.relative(root, full) + if (!rel || rel.startsWith("..") || path.isAbsolute(rel)) return undefined const info = await stat(full).catch(() => undefined) if (!info?.isFile() || info.size > SIZE) return undefined const hit = cache.get(full) diff --git a/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts b/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts index 0e452042f5f..c815b2bdafd 100644 --- a/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts +++ b/packages/kilo-vscode/tests/unit/pr-comment-context.test.ts @@ -99,6 +99,14 @@ describe("withContext", () => { expect(items.map((item) => item.after)).toEqual([undefined, undefined, undefined, undefined]) }) + it("rejects a comment path outside the worktree", async () => { + const dir = await repo() + await writeFile(path.join(path.dirname(dir), "outside.ts"), SOURCE.join("\n")) + const [item] = await withContext(dir, [thread({ file: "../outside.ts" })]) + + expect(item!.after).toBeUndefined() + }) + it("keeps every thread, in order, whatever the files say", async () => { const dir = await repo() const items = await withContext(dir, [thread({ threadId: "a" }), thread({ threadId: "b", outdated: true })]) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx index d7a1facd451..35708e592f0 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/pr/PRPanel.tsx @@ -102,7 +102,7 @@ export const PRPanel: Component = (props) => { // expanded card and the scroll position, so the last list for this PR stays. const comments = createMemo<{ number: number; value: NonNullable } | undefined>((prev) => { const next = props.pr.comments - if (next?.total) return { number: props.pr.number, value: next } + if (next) return { number: props.pr.number, value: next } if (prev && prev.number === props.pr.number) return prev return undefined }) From 58c6d689c983da0085dac6a506754871838ce50b Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 20 Aug 2026 14:31:16 +0200 Subject: [PATCH 4/5] fix(agent-manager): start PR polling during hydration --- packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index 1a5cbd986e4..d9d6cc97efc 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -1435,7 +1435,8 @@ export class AgentManagerProvider implements Disposable { // already excludes worktrees in collapsed sections. this.syncPollerSkips() this.statsPoller.setEnabled(worktrees.length > 0 || this.panel !== undefined) - this.prBridge.poller.setEnabled(worktrees.length > 0) + // Start PR polling during state hydration so persisted badges get live status. + this.prBridge.poller.setEnabled(this.panel !== undefined) } /** Push empty state when the folder is not a git repo or has no folder open. */ From 45afcce46ed495925f42ab47c319304c181347e2 Mon Sep 17 00:00:00 2001 From: "kilo-maintainer[bot]" Date: Thu, 20 Aug 2026 12:59:06 +0000 Subject: [PATCH 5/5] chore: update kilo-vscode visual regression baselines --- .../agentmanager/pr-panel-comments-200-chromium-linux.png | 4 ++-- .../agentmanager/pr-panel-comments-chromium-linux.png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-200-chromium-linux.png b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-200-chromium-linux.png index 9529b8784ff..4eb665f771d 100644 --- a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-200-chromium-linux.png +++ b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-200-chromium-linux.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d83b6fdde1c87d7bdbd4fae163dea888c2c50a54f175ceef2405699f8f48443e -size 34823 +oid sha256:82b91b6fcb81a7500e37dfcc335d2977955cab4f786efa30f35880c09cd8987c +size 34732 diff --git a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-chromium-linux.png b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-chromium-linux.png index 0f0abe471cd..eec37606090 100644 --- a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-chromium-linux.png +++ b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/agentmanager/pr-panel-comments-chromium-linux.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:f8dbff32f3c1b73230e1a1c84c1a683235fbf56fff8a418e6d982603f263ead0 -size 38718 +oid sha256:a4c6debea9b97f733f17f60102fb6a3757ed46a5288796e701ee870b538ddb71 +size 38780