diff --git a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/composite-webview/diff-summary-collapsed-chromium-linux.png b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/composite-webview/diff-summary-collapsed-chromium-linux.png index 0495921387..d341a7a2c7 100644 --- a/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/composite-webview/diff-summary-collapsed-chromium-linux.png +++ b/packages/kilo-docs/public/img/screenshot-tests/kilo-vscode/visual-regression/composite-webview/diff-summary-collapsed-chromium-linux.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:0d29bb3a9356cf8bec6bb0a9d6e4d0b59d147c1fa8adee7140c81a061eea407d -size 6930 +oid sha256:067a5c757dfa32d27955dd634b8dea52edef2141111a5eecee0161df36c0ed8b +size 7061 diff --git a/packages/kilo-vscode/src/KiloProvider.ts b/packages/kilo-vscode/src/KiloProvider.ts index 91d0831a57..56a6777ff8 100644 --- a/packages/kilo-vscode/src/KiloProvider.ts +++ b/packages/kilo-vscode/src/KiloProvider.ts @@ -608,8 +608,8 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper post: (msg) => this.postMessage(msg), openAgentManager: () => vscode.commands.executeCommand("kilo-code.new.agentManagerOpen"), openAdvancedWorktree: () => vscode.commands.executeCommand("kilo-code.new.agentManager.advancedWorktree"), - openChanges: (sessionId?: string) => - vscode.commands.executeCommand("kilo-code.new.showChanges", { sessionId }), + openChanges: (sessionId?: string, turnId?: string) => + vscode.commands.executeCommand("kilo-code.new.showChanges", { sessionId, turnId }), currentSessionId: this.currentSession?.id, createWorktree: async (baseBranch, branchName) => { await this.createWorktreeHandler?.(baseBranch, branchName) diff --git a/packages/kilo-vscode/src/diff/DiffViewerProvider.ts b/packages/kilo-vscode/src/diff/DiffViewerProvider.ts index 6bc6ba5470..417f2c65f4 100644 --- a/packages/kilo-vscode/src/diff/DiffViewerProvider.ts +++ b/packages/kilo-vscode/src/diff/DiffViewerProvider.ts @@ -5,6 +5,7 @@ import { getDiffMarkdownRender, setDiffMarkdownRender } from "../review-settings import { buildWebviewHtml, getWebviewFontSize } from "../utils" import { watchFontSizeConfig } from "../kilo-provider/font-size" import type { DiffSourceCatalog } from "./sources/catalog" +import { turnSourceId } from "./sources/turn" import type { PanelContext } from "./types" import { SourceController } from "./SourceController" @@ -63,12 +64,19 @@ export class DiffViewerProvider implements vscode.Disposable { * Entry point for the `kilo-code.new.showChanges` command. Composes the * PanelContext from the arg + injected session/workspace lookups so * callers don't have to know about it. + * + * When `turnId` is passed, opens the panel scoped to that single turn with + * the source picker hidden — the view becomes a static "diff of this turn" + * rather than the switchable workspace/session viewer. */ - openFromCommand(arg?: { sessionId?: string; initialSourceId?: string }): void { + openFromCommand(arg?: { sessionId?: string; turnId?: string; initialSourceId?: string }): void { + const sessionId = arg?.sessionId ?? this.sessionIdProvider() + const turnInitialSourceId = arg?.turnId && sessionId ? turnSourceId(sessionId, arg.turnId) : undefined this.openPanel({ workspaceRoot: getWorkspaceRoot(), - sessionId: arg?.sessionId ?? this.sessionIdProvider(), - initialSourceId: arg?.initialSourceId, + sessionId, + initialSourceId: turnInitialSourceId ?? arg?.initialSourceId, + hidePicker: !!turnInitialSourceId, }) } diff --git a/packages/kilo-vscode/src/diff/SourceController.ts b/packages/kilo-vscode/src/diff/SourceController.ts index 4433cab1dd..cd8d01a3bf 100644 --- a/packages/kilo-vscode/src/diff/SourceController.ts +++ b/packages/kilo-vscode/src/diff/SourceController.ts @@ -1,21 +1,29 @@ -import type * as vscode from "vscode" -import type { DiffSource, DiffSourceDescriptor, DiffSourceMessage, DiffSourcePost } from "./sources/types" +import { hashFileDiffs } from "./shared/hash" +import { DIFF_POLL_INTERVAL_MS } from "./polling" +import type { DiffSource, DiffSourceDescriptor } from "./sources/types" import type { PanelContext } from "./types" /** - * Owns the active DiffSource for a panel: builds it via the injected - * `build` function, runs initialFetch + start, and disposes it on swap - * or teardown. + * Owns the active DiffSource for a panel: builds it via the injected `build` + * function, runs an initial fetch, and then polls on a fixed interval with + * hash-dedup. Posts loading / diffs / notice messages to the webview, and + * disposes the source on swap or teardown. * - * Decoupled from the webview panel — receives neutral callbacks. Stale - * messages are filtered via an internal epoch counter that bumps on - * every stop/activate, so async posts from a disposed source are dropped. + * Sources are declarative — they only implement `fetch()` (and optionally + * `fetchFile` / `revert` / `dispose`). All lifecycle, polling, and message + * posting lives here so that concrete sources can be plain factory functions + * with closure state instead of classes with a `post`/`start`/`dispose` dance. + * + * Stale results are filtered via an internal epoch counter that bumps on + * every stop/activate, so in-flight fetches from a disposed source are + * dropped. */ export class SourceController { private ctx: PanelContext | undefined private activeId: string | undefined private active: DiffSource | undefined - private startDisposable: vscode.Disposable | undefined + private interval: ReturnType | undefined + private lastHash: string | undefined private epoch = 0 constructor( @@ -32,20 +40,20 @@ export class SourceController { return this.activeId } - /** Dispose the active source and bump the epoch so in-flight posts are dropped. */ + /** Dispose the active source and bump the epoch so in-flight fetches are dropped. */ stop(): void { this.epoch++ - this.startDisposable?.dispose() - this.startDisposable = undefined - this.active?.dispose() + this.stopPolling() + this.active?.dispose?.() this.active = undefined this.activeId = undefined + this.lastHash = undefined } /** - * Build, initial-fetch, and start source `id` in the current context. - * Internally disposes any previously active source. Throws if the - * catalog can't build the id — callers should catch and log. + * Build, initial-fetch, and start polling source `id` in the current context. + * Disposes any previously active source. Throws if the catalog can't build + * the id — callers should catch and log. */ async activate(id: string): Promise { const ctx = this.ctx @@ -67,19 +75,15 @@ export class SourceController { capabilities: source.descriptor.capabilities, }) - const sourcePost = this.guardedPost(epoch) - await source.initialFetch(sourcePost) - // Prevents source polling from starting after teardown or source swap. - if (this.epoch !== epoch || this.activeId !== id) { - if (this.active === source) source.dispose() - return - } - this.startDisposable = source.start?.(sourcePost) + const keepPolling = await this.runFetch(source, epoch, true) + // Prevents the polling interval from starting after teardown or swap. + if (this.epoch !== epoch || this.activeId !== id) return + if (keepPolling) this.startPolling(source, epoch) } async revertFile(file: string): Promise { const source = this.active - if (!source?.revertFile) { + if (!source?.revert) { this.post({ type: "diffViewer.revertFileResult", file, @@ -89,7 +93,8 @@ export class SourceController { return } - const result = await source.revertFile(file).catch((err) => { + const epoch = this.epoch + const result = await source.revert(file).catch((err) => { const message = err instanceof Error ? err.message : String(err) return { ok: false, message } }) @@ -99,22 +104,27 @@ export class SourceController { status: result.ok ? "success" : "error", message: result.message, }) + // Push fresh diffs immediately after a successful revert so the webview + // doesn't have to wait for the next polling tick. + if (result.ok && this.epoch === epoch && this.active === source) { + await this.runFetch(source, epoch, false) + } } /** * Lazy detail load for a single file. Forwards to the active source's - * `requestFile`. Posts `diff: null` when the source can't resolve the file + * `fetchFile`. Posts `diff: null` when the source can't resolve the file * or doesn't support per-file detail, so the webview can clear its * pending-loading indicator either way. */ async requestFile(file: string): Promise { const source = this.active const epoch = this.epoch - if (!source?.requestFile) { + if (!source?.fetchFile) { this.post({ type: "diffViewer.diffFile", file, diff: null }) return } - const diff = await source.requestFile(file).catch(() => null) + const diff = await source.fetchFile(file).catch(() => null) // Drop the response if the source has been disposed/swapped while we waited. if (this.epoch !== epoch) return this.post({ type: "diffViewer.diffFile", file, diff }) @@ -124,19 +134,57 @@ export class SourceController { this.stop() } - private guardedPost(epoch: number): DiffSourcePost { - return (msg: DiffSourceMessage) => { - // Drops stale messages from sources whose lifecycle epoch has ended. - if (this.epoch !== epoch) return - if (msg.type === "diffs") { - this.post({ type: "diffViewer.diffs", diffs: msg.diffs }) - } else if (msg.type === "loading") { - this.post({ type: "diffViewer.loading", loading: msg.loading }) - } else if (msg.type === "error") { + /** + * Run one fetch against the source and post results. Returns whether the + * controller should keep polling this source — false when the source + * requests a stop or the epoch has moved on. + */ + private async runFetch(source: DiffSource, epoch: number, initial: boolean): Promise { + if (initial) this.post({ type: "diffViewer.loading", loading: true }) + + try { + const result = await source.fetch() + if (this.epoch !== epoch) return false + + if (result.notice !== undefined) { + this.post({ type: "diffViewer.notice", notice: result.notice }) + } + + const hash = hashFileDiffs(result.diffs as never) + if (initial || hash !== this.lastHash) { + this.lastHash = hash + this.post({ type: "diffViewer.diffs", diffs: result.diffs }) + } + + return !result.stopPolling + } catch (err) { + if (this.epoch !== epoch) return false + // Errors are swallowed for the webview (it just needs the loading + // indicator cleared below), but we always log so initial-fetch + // failures leave a trace in the Extension Host output — previously + // they were silent and invisible in production. + console.log("[Kilo New] SourceController.fetch error", { initial, err }) + return true + } finally { + if (initial && this.epoch === epoch) { this.post({ type: "diffViewer.loading", loading: false }) - } else if (msg.type === "notice") { - this.post({ type: "diffViewer.notice", notice: msg.notice }) } } } + + private startPolling(source: DiffSource, epoch: number): void { + this.stopPolling() + this.interval = setInterval(async () => { + // Self-cancel when the tick reports the source is done + const keep = await this.runFetch(source, epoch, false) + if (!keep) this.stopPolling() + }, DIFF_POLL_INTERVAL_MS) + } + + private stopPolling(): void { + if (this.interval) { + clearInterval(this.interval) + this.interval = undefined + } + } } diff --git a/packages/kilo-vscode/src/diff/sources/catalog.ts b/packages/kilo-vscode/src/diff/sources/catalog.ts index d0d2976397..13c3027649 100644 --- a/packages/kilo-vscode/src/diff/sources/catalog.ts +++ b/packages/kilo-vscode/src/diff/sources/catalog.ts @@ -1,15 +1,16 @@ import type { KiloConnectionService } from "../../services/cli-backend" import type { PanelContext } from "../types" import type { DiffSource, DiffSourceDescriptor } from "./types" -import { WorktreeDiffSource, WORKSPACE_DESCRIPTOR, WORKSPACE_SOURCE_ID } from "./worktree" +import { createWorktreeDiffSource, WORKSPACE_DESCRIPTOR, WORKSPACE_SOURCE_ID } from "./worktree" import { SESSION_PREFIX, - SessionDiffSource, + createSessionDiffSource, sessionDescriptor, sessionSourceId, type SessionDiffFetch, type SnapshotEnabledCheck, } from "./session" +import { TURN_PREFIX, createTurnDiffSource, type TurnDiffFetch } from "./turn" /** * Enumerates and constructs diff sources for a PanelContext. @@ -21,6 +22,19 @@ export class DiffSourceCatalog { return data ?? [] } + /** + * Turn diffs are stored on the user message itself (`summary.diffs`), not + * on the session-level snapshot. The `/session/:id/diff` endpoint ignores + * its `messageID` param today, so we fetch the message directly instead. + */ + private readonly turnFetch: TurnDiffFetch = async ({ sessionID, messageID, directory }) => { + const client = this.connection.getClient() + const { data } = await client.session.message({ sessionID, messageID, directory }, { throwOnError: true }) + const info = data?.info + if (!info || info.role !== "user") return [] + return info.summary?.diffs ?? [] + } + private readonly checkSnapshotsEnabled: SnapshotEnabledCheck = async (directory) => { const client = this.connection.getClient() const { data } = await client.config.get({ directory }, { throwOnError: true }) @@ -31,6 +45,7 @@ export class DiffSourceCatalog { constructor(private readonly connection: KiloConnectionService) {} listAvailable(ctx: PanelContext): DiffSourceDescriptor[] { + if (ctx.hidePicker) return [] const out: DiffSourceDescriptor[] = [] if (ctx.workspaceRoot) out.push(WORKSPACE_DESCRIPTOR) if (ctx.sessionId) out.push(sessionDescriptor(ctx.sessionId)) @@ -45,12 +60,20 @@ export class DiffSourceCatalog { } build(id: string, ctx: PanelContext): DiffSource { - if (id === WORKSPACE_SOURCE_ID) return new WorktreeDiffSource(this.connection) + if (id === WORKSPACE_SOURCE_ID) return createWorktreeDiffSource(this.connection) + + if (id.startsWith(TURN_PREFIX)) { + const [sessionId, messageId] = id.slice(TURN_PREFIX.length).split(":") + if (!sessionId || !messageId) { + throw new Error(`DiffSourceCatalog.build: malformed turn id "${id}" (expected turn::)`) + } + return createTurnDiffSource(sessionId, messageId, this.turnFetch, ctx.workspaceRoot) + } if (id.startsWith(SESSION_PREFIX)) { const sessionId = id.slice(SESSION_PREFIX.length) if (!sessionId) throw new Error(`DiffSourceCatalog.build: empty session id in "${id}"`) - return new SessionDiffSource(sessionId, this.sessionFetch, ctx.workspaceRoot, this.checkSnapshotsEnabled) + return createSessionDiffSource(sessionId, this.sessionFetch, ctx.workspaceRoot, this.checkSnapshotsEnabled) } throw new Error(`DiffSourceCatalog.build: unknown source id "${id}"`) diff --git a/packages/kilo-vscode/src/diff/sources/session.ts b/packages/kilo-vscode/src/diff/sources/session.ts index b92125e0dd..cf10f0e2a2 100644 --- a/packages/kilo-vscode/src/diff/sources/session.ts +++ b/packages/kilo-vscode/src/diff/sources/session.ts @@ -1,10 +1,7 @@ -import * as vscode from "vscode" import type { SnapshotFileDiff } from "@kilocode/sdk/v2/client" -import type { DiffFile } from "../types" -import { hashFileDiffs } from "../shared/hash" -import { DIFF_POLL_INTERVAL_MS } from "../polling" -import type { DiffSource, DiffSourceDescriptor, DiffSourcePost } from "./types" import { normalize, text } from "@kilocode/kilo-ui/session-diff" +import type { DiffFile } from "../types" +import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "./types" export type SessionDiffFetch = (params: { sessionID: string; directory?: string }) => Promise @@ -26,109 +23,59 @@ export function sessionDescriptor(sessionId: string): DiffSourceDescriptor { } /** - * Diff for the current session. Initial fetch + 2.5s polling with hash dedup + * Diff for the current session. Returns file diffs from the SDK's session + * snapshot endpoint, or a `snapshots-disabled` notice if snapshotting is + * turned off in the workspace config (in which case the controller stops + * polling because repeated fetches can't surface new data). */ -export class SessionDiffSource implements DiffSource { - readonly descriptor: DiffSourceDescriptor +export function createSessionDiffSource( + sessionId: string, + fetch: SessionDiffFetch, + workspaceRoot?: string, + checkSnapshotsEnabled?: SnapshotEnabledCheck, +): DiffSource { + // Cached across fetches so subsequent polling ticks skip the config lookup. + let snapshotsDisabled = false - private lastHash: string | undefined - private interval: ReturnType | undefined - private disposed = false + return { + descriptor: sessionDescriptor(sessionId), - private snapshotsDisabled = false + async fetch(): Promise { + if (snapshotsDisabled) { + return { diffs: [], notice: "snapshots-disabled", stopPolling: true } + } - constructor( - private readonly sessionId: string, - private readonly fetch: SessionDiffFetch, - private readonly workspaceRoot?: string, - private readonly checkSnapshotsEnabled?: SnapshotEnabledCheck, - ) { - this.descriptor = sessionDescriptor(sessionId) - } - - async initialFetch(post: DiffSourcePost): Promise { - post({ type: "loading", loading: true }) - - try { - if (this.checkSnapshotsEnabled) { - const enabled = await this.checkSnapshotsEnabled(this.workspaceRoot) - if (this.disposed) return + if (checkSnapshotsEnabled) { + const enabled = await checkSnapshotsEnabled(workspaceRoot) if (!enabled) { - this.snapshotsDisabled = true - post({ type: "notice", notice: "snapshots-disabled" }) - post({ type: "diffs", diffs: [] }) - return + snapshotsDisabled = true + return { diffs: [], notice: "snapshots-disabled", stopPolling: true } } } - const diffs = await this.fetchDiffs() - if (this.disposed) return - this.lastHash = hashFileDiffs(diffs as never) - post({ type: "diffs", diffs }) - } catch (err) { - if (this.disposed) return - const message = err instanceof Error ? err.message : String(err) - post({ type: "error", message }) - } finally { - if (!this.disposed) post({ type: "loading", loading: false }) - } - } - - start(post: DiffSourcePost): vscode.Disposable { - this.stopPolling() - // Skip polling entirely when snapshots are disabled — nothing to fetch. - if (this.snapshotsDisabled) return new vscode.Disposable(() => {}) - this.interval = setInterval(() => { - void this.poll(post) - }, DIFF_POLL_INTERVAL_MS) - - return new vscode.Disposable(() => this.stopPolling()) - } - - dispose(): void { - this.disposed = true - this.stopPolling() - this.lastHash = undefined - } - - private async fetchDiffs(): Promise { - const raw = await this.fetch({ sessionID: this.sessionId, directory: this.workspaceRoot }) - return raw.map((r) => { - // Empty patch means binary or summarized (>256 KB) — normalize() can't - // parse it, so short-circuit to empty strings. - const view = r.patch === "" ? null : normalize(r) - return { - file: r.file, - before: view ? text(view, "deletions") : "", - after: view ? text(view, "additions") : "", - additions: r.additions, - deletions: r.deletions, - status: r.status, - tracked: true, - generatedLike: false, - summarized: r.patch === "", - } - }) - } - - private async poll(post: DiffSourcePost): Promise { - try { - const diffs = await this.fetchDiffs() - if (this.disposed) return - const hash = hashFileDiffs(diffs as never) - if (hash === this.lastHash) return - this.lastHash = hash - post({ type: "diffs", diffs }) - } catch (err) { - if (this.disposed) return - console.log("[Kilo New] SessionDiffSource.poll error", err) - } - } - - private stopPolling(): void { - if (this.interval) { - clearInterval(this.interval) - this.interval = undefined - } + const raw = await fetch({ sessionID: sessionId, directory: workspaceRoot }) + return { diffs: raw.map(toSessionDiffFile) } + }, + } +} + +/** + * Project a backend `SnapshotFileDiff` onto the `DiffFile` shape the viewer + * expects. Shared with `createTurnDiffSource` since both hit the same endpoint. + */ +export function toSessionDiffFile(raw: SnapshotFileDiff): DiffFile { + // Empty patch means binary or summarized (>256 KB) — normalize() can't + // parse it, so short-circuit to empty strings. + const view = raw.patch === "" ? null : normalize(raw) + return { + file: raw.file, + before: view ? text(view, "deletions") : "", + after: view ? text(view, "additions") : "", + additions: raw.additions, + deletions: raw.deletions, + status: raw.status, + tracked: true, + generatedLike: false, + summarized: raw.patch === "", } } diff --git a/packages/kilo-vscode/src/diff/sources/turn.ts b/packages/kilo-vscode/src/diff/sources/turn.ts new file mode 100644 index 0000000000..24b6b5e1bc --- /dev/null +++ b/packages/kilo-vscode/src/diff/sources/turn.ts @@ -0,0 +1,53 @@ +import type { SnapshotFileDiff } from "@kilocode/sdk/v2/client" +import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "./types" +import { toSessionDiffFile } from "./session" + +export const TURN_PREFIX = "turn:" + +export function turnSourceId(sessionId: string, messageId: string): string { + return `${TURN_PREFIX}${sessionId}:${messageId}` +} + +export function turnDescriptor(sessionId: string, messageId: string): DiffSourceDescriptor { + return { + id: turnSourceId(sessionId, messageId), + type: "turn", + // Group is irrelevant here because turn sources only open in hide-picker + // mode; the picker never renders them. Default to "Session" for cohesion. + group: "Session", + capabilities: { revert: false, comments: true }, + } +} + +/** + * Fetches the per-turn diffs attached to a user message. The session-level + * `/session/:id/diff` endpoint ignores `messageID`, so the per-turn view has + * to read from the message's own `summary.diffs`. + */ +export type TurnDiffFetch = (params: { + sessionID: string + messageID: string + directory?: string +}) => Promise + +/** + * Static diff for a single turn (the file changes attributed to one user + * message). Returns `stopPolling: true` so the controller runs `fetch` once + * and never schedules a polling tick — a completed turn's snapshot doesn't + * change. + */ +export function createTurnDiffSource( + sessionId: string, + messageId: string, + fetch: TurnDiffFetch, + workspaceRoot?: string, +): DiffSource { + return { + descriptor: turnDescriptor(sessionId, messageId), + + async fetch(): Promise { + const raw = await fetch({ sessionID: sessionId, messageID: messageId, directory: workspaceRoot }) + return { diffs: raw.map(toSessionDiffFile), stopPolling: true } + }, + } +} diff --git a/packages/kilo-vscode/src/diff/sources/types.ts b/packages/kilo-vscode/src/diff/sources/types.ts index 2ac5d01caf..6667de06fd 100644 --- a/packages/kilo-vscode/src/diff/sources/types.ts +++ b/packages/kilo-vscode/src/diff/sources/types.ts @@ -1,4 +1,3 @@ -import type * as vscode from "vscode" import type { DiffFile } from "../types" export interface DiffSourceCapabilities { @@ -7,10 +6,13 @@ export interface DiffSourceCapabilities { } /** - * Closed enum of diff source kinds. Drives i18n key composition: - * `diffViewer.source..label` and `diffViewer.source..tooltip`. + * Closed enum of diff source kinds. Drives i18n key composition for types + * that appear in the picker: `diffViewer.source..label` and + * `diffViewer.source..tooltip`. Types that are only ever shown in + * hide-picker mode (e.g. `turn`) don't need matching i18n entries because + * `DiffPickerHeader` never renders them. */ -export type DiffSourceType = "workspace" | "session" +export type DiffSourceType = "workspace" | "session" | "turn" export interface DiffSourceDescriptor { /** Unique within a panel context. E.g. "workspace", "session:". */ @@ -28,35 +30,36 @@ export interface DiffSourceDescriptor { */ export type DiffSourceNotice = "snapshots-disabled" -export type DiffSourceMessage = - | { type: "diffs"; diffs: DiffFile[] } - | { type: "loading"; loading: boolean } - | { type: "error"; message: string } - | { type: "notice"; notice: DiffSourceNotice | undefined } - -export type DiffSourcePost = (msg: DiffSourceMessage) => void +export interface DiffSourceFetch { + diffs: DiffFile[] + notice?: DiffSourceNotice + /** + * When true the controller stops polling the source after this fetch. + * Used for terminal states like snapshots-disabled, where repeat fetches + * can't surface new data. + */ + stopPolling?: boolean +} /** - * A DiffSource produces file diffs for a given context (local workspace, - * session changes, a turn, a git ref...). The SourceController owns one - * active source at a time and swaps between them on user request. + * A DiffSource is a plain data producer for a given context (local workspace, + * session changes, a turn, a git ref...). The SourceController owns one active + * source at a time, calls `fetch` on activation and on a polling tick, and + * forwards the results to the webview. */ export interface DiffSource { readonly descriptor: DiffSourceDescriptor - initialFetch(post: DiffSourcePost): Promise - - /** Start change detection (polling, SSE, watcher...). Dispose to stop. */ - start?(post: DiffSourcePost): vscode.Disposable - - revertFile?(file: string): Promise<{ ok: boolean; message: string }> + fetch(): Promise /** - * Lazy detail load for a single file, for sources that emit summarized entries - * (no `before`/`after` content) so the webview can fetch - * full content on demand. + * Lazy detail load for a single file, for sources that emit summarized + * entries (no `before`/`after` content) so the webview can fetch full + * content on demand. */ - requestFile?(file: string): Promise + fetchFile?(file: string): Promise - dispose(): void + revert?(file: string): Promise<{ ok: boolean; message: string }> + + dispose?(): void } diff --git a/packages/kilo-vscode/src/diff/sources/worktree.ts b/packages/kilo-vscode/src/diff/sources/worktree.ts index c31ceaf924..dc1f377900 100644 --- a/packages/kilo-vscode/src/diff/sources/worktree.ts +++ b/packages/kilo-vscode/src/diff/sources/worktree.ts @@ -4,12 +4,10 @@ import { GitOps } from "../../agent-manager/GitOps" import { diffSummary, diffFile } from "../../agent-manager/local-diff" import type { WorktreeDiffEntry } from "../../agent-manager/types" import { WorktreeDiffClient, type DiffTarget } from "../shared/client" -import { hashFileDiffs } from "../shared/hash" import { resolveLocalDiffTarget } from "../shared/target" -import { DIFF_POLL_INTERVAL_MS } from "../polling" import { appendOutput, getWorkspaceRoot } from "../../review-utils" import type { DiffFile } from "../types" -import type { DiffSource, DiffSourceDescriptor, DiffSourcePost } from "./types" +import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "./types" export const WORKSPACE_SOURCE_ID = "workspace" @@ -21,137 +19,74 @@ export const WORKSPACE_DESCRIPTOR: DiffSourceDescriptor = { } /** - * Diffs between the local working tree and the base branch. Polls a summary - * (one entry per changed file, no content) every {@link DIFF_POLL_INTERVAL_MS}, - * then loads `before`/`after`/`patch` per file on demand via {@link requestFile}. - * - * Mirrors the Agent Manager's `WorktreeDiffController` and runs entirely in - * the extension host (no `kilo serve` round-trip) + * Diffs between the local working tree and the base branch. Each fetch returns + * a summary (one entry per changed file, no content); the viewer loads + * `before`/`after` per file on demand via `fetchFile`. Runs entirely in the + * extension host — no `kilo serve` round-trip. */ -export class WorktreeDiffSource implements DiffSource { - readonly descriptor = WORKSPACE_DESCRIPTOR +export function createWorktreeDiffSource(connection: KiloConnectionService): DiffSource { + const output = vscode.window.createOutputChannel("Kilo Diff: Workspace") + const log = (...args: unknown[]) => appendOutput(output, "WorktreeDiffSource", ...args) + const git = new GitOps({ log }) - private readonly git: GitOps - private readonly output: vscode.OutputChannel - private target: DiffTarget | undefined - private lastHash: string | undefined - private interval: ReturnType | undefined - private post: DiffSourcePost | undefined + // Cached between fetches so repeated polling doesn't re-resolve the base + // branch every tick. Reset only on dispose (when the source is swapped out). + let target: DiffTarget | undefined - constructor(private readonly connection: KiloConnectionService) { - this.git = new GitOps({ log: (...args) => this.log(...args) }) - this.output = vscode.window.createOutputChannel("Kilo Diff: Workspace") + const resolveTarget = async (): Promise => { + if (target) return target + target = await resolveLocalDiffTarget(git, log, getWorkspaceRoot()) + return target } - async initialFetch(post: DiffSourcePost): Promise { - this.post = post - post({ type: "loading", loading: true }) + return { + descriptor: WORKSPACE_DESCRIPTOR, - const target = await this.resolveTarget() - if (!target) { - post({ type: "diffs", diffs: [] }) - post({ type: "loading", loading: false }) - return - } + async fetch(): Promise { + const current = await resolveTarget() + if (!current) return { diffs: [] } - this.target = target - await this.fetchAndPost(target, post, true) - post({ type: "loading", loading: false }) - } - - start(post: DiffSourcePost): vscode.Disposable { - this.post = post - this.stopPolling() - this.interval = setInterval(() => { - void this.poll(post) - }, DIFF_POLL_INTERVAL_MS) - - return new vscode.Disposable(() => this.stopPolling()) - } - - async revertFile(file: string): Promise<{ ok: boolean; message: string }> { - const target = this.target ?? (await this.resolveTarget()) - if (!target) { - return { ok: false, message: "Could not resolve diff target" } - } - - try { - const client = this.connection.getClient() - const diff = new WorktreeDiffClient(client, this.git, (...args) => this.log(...args)) - const result = await diff.revertFile(target, file) - if (result.ok && this.post) void this.poll(this.post) - return result - } catch (err) { - const message = err instanceof Error ? err.message : String(err) - this.log("Failed to revert file:", message) - return { ok: false, message } - } - } - - async requestFile(file: string): Promise { - if (!file) return null - const target = this.target ?? (await this.resolveTarget()) - if (!target) return null - this.target = target - - try { - const entry = await diffFile(this.git, target.directory, target.baseBranch, file, (...args) => this.log(...args)) - if (!entry) return null - return toDiffFile(entry) - } catch (err) { - this.log("Failed to fetch worktree diff file:", err) - return null - } - } - - dispose(): void { - this.stopPolling() - this.git.dispose() - this.output.dispose() - this.post = undefined - this.target = undefined - this.lastHash = undefined - } - - private async resolveTarget(): Promise { - return await resolveLocalDiffTarget(this.git, (...args) => this.log(...args), getWorkspaceRoot()) - } - - private async fetchAndPost(target: DiffTarget, post: DiffSourcePost, force: boolean): Promise { - try { - const entries = await diffSummary(this.git, target.directory, target.baseBranch, (...args) => this.log(...args)) + const entries = await diffSummary(git, current.directory, current.baseBranch, log) const diffs = entries.map(toDiffFile) - const hash = hashFileDiffs(diffs as never) - if (!force && hash === this.lastHash) return - this.lastHash = hash + log(`Diff: ${diffs.length} file(s)`) + return { diffs } + }, - this.log(`Diff: ${diffs.length} file(s)`) - post({ type: "diffs", diffs }) - } catch (err) { - const message = err instanceof Error ? err.message : String(err) - this.log("Failed to fetch diff:", message) - if (force) post({ type: "error", message }) - } - } + async fetchFile(file: string): Promise { + if (!file) return null + const current = await resolveTarget() + if (!current) return null - private async poll(post: DiffSourcePost): Promise { - const target = this.target - if (!target) { - await this.initialFetch(post) - return - } - await this.fetchAndPost(target, post, false) - } + try { + const entry = await diffFile(git, current.directory, current.baseBranch, file, log) + if (!entry) return null + return toDiffFile(entry) + } catch (err) { + log("Failed to fetch worktree diff file:", err) + return null + } + }, - private stopPolling(): void { - if (this.interval) { - clearInterval(this.interval) - this.interval = undefined - } - } + async revert(file: string): Promise<{ ok: boolean; message: string }> { + const current = await resolveTarget() + if (!current) return { ok: false, message: "Could not resolve diff target" } - private log(...args: unknown[]): void { - appendOutput(this.output, "WorktreeDiffSource", ...args) + try { + const client = connection.getClient() + const diff = new WorktreeDiffClient(client, git, log) + return await diff.revertFile(current, file) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + log("Failed to revert file:", message) + return { ok: false, message } + } + }, + + dispose(): void { + git.dispose() + output.dispose() + target = undefined + }, } } diff --git a/packages/kilo-vscode/src/diff/types.ts b/packages/kilo-vscode/src/diff/types.ts index 9850668212..c3260a413b 100644 --- a/packages/kilo-vscode/src/diff/types.ts +++ b/packages/kilo-vscode/src/diff/types.ts @@ -3,6 +3,11 @@ export interface PanelContext { sessionId?: string /** Overrides the computed default source on open. */ initialSourceId?: string + /** + * Hides the source picker header in the diff viewer. Used for panels that + * open in a fixed view (e.g. a specific turn's diff) + */ + hidePicker?: boolean } /** Mirrors `WorktreeFileDiff` in webview-ui/src/types/messages/agent-manager.ts. */ diff --git a/packages/kilo-vscode/src/extension.ts b/packages/kilo-vscode/src/extension.ts index 2ba3759cb5..8d01db1664 100644 --- a/packages/kilo-vscode/src/extension.ts +++ b/packages/kilo-vscode/src/extension.ts @@ -350,7 +350,7 @@ export function activate(context: vscode.ExtensionContext) { }), vscode.commands.registerCommand( "kilo-code.new.showChanges", - (arg?: { sessionId?: string; initialSourceId?: string }) => { + (arg?: { sessionId?: string; turnId?: string; initialSourceId?: string }) => { diffViewerProvider.openFromCommand(arg) }, ), diff --git a/packages/kilo-vscode/src/kilo-provider/sidebar-worktree.ts b/packages/kilo-vscode/src/kilo-provider/sidebar-worktree.ts index 84cccf373b..dc4d54fe5b 100644 --- a/packages/kilo-vscode/src/kilo-provider/sidebar-worktree.ts +++ b/packages/kilo-vscode/src/kilo-provider/sidebar-worktree.ts @@ -7,13 +7,14 @@ interface Msg { baseBranch?: string branchName?: string sessionId?: string + turnId?: string } interface Ctx { post: (msg: unknown) => void openAgentManager: () => Thenable openAdvancedWorktree: () => Thenable - openChanges: (sessionId?: string) => Thenable + openChanges: (sessionId?: string, turnId?: string) => Thenable currentSessionId?: string createWorktree?: (baseBranch?: string, branchName?: string) => Promise continueInWorktree?: ( @@ -54,7 +55,7 @@ export async function handleSidebarWorktreeMessage(message: Msg, ctx: Ctx) { } if (message.type === "openChanges") { - await ctx.openChanges(ctx.currentSessionId) + await ctx.openChanges(ctx.currentSessionId, message.turnId) return true } diff --git a/packages/kilo-vscode/tests/unit/diff-session-source.test.ts b/packages/kilo-vscode/tests/unit/diff-session-source.test.ts index 2fd01cf6aa..b1c2a37714 100644 --- a/packages/kilo-vscode/tests/unit/diff-session-source.test.ts +++ b/packages/kilo-vscode/tests/unit/diff-session-source.test.ts @@ -1,7 +1,11 @@ import { describe, it, expect } from "bun:test" import type { SnapshotFileDiff } from "@kilocode/sdk/v2/client" -import { SessionDiffSource, type SessionDiffFetch, type SnapshotEnabledCheck } from "../../src/diff/sources/session" -import type { DiffSourceMessage } from "../../src/diff/sources/types" +import { + createSessionDiffSource, + sessionDescriptor, + type SessionDiffFetch, + type SnapshotEnabledCheck, +} from "../../src/diff/sources/session" type FetchCall = { sessionID: string; directory?: string } @@ -15,11 +19,6 @@ function recording(result: SnapshotFileDiff[] | Error): { fetch: SessionDiffFetc return { fetch, calls } } -function collect(): { post: (msg: DiffSourceMessage) => void; messages: DiffSourceMessage[] } { - const messages: DiffSourceMessage[] = [] - return { post: (msg) => messages.push(msg), messages } -} - const modifiedPatch = [ "diff --git a/foo.ts b/foo.ts", "--- a/foo.ts", @@ -30,20 +29,15 @@ const modifiedPatch = [ "+new", ].join("\n") -describe("SessionDiffSource.initialFetch", () => { - it("posts loading/diffs/loading for an empty session", async () => { +describe("createSessionDiffSource.fetch", () => { + it("returns empty diffs for an empty session", async () => { const { fetch, calls } = recording([]) - const source = new SessionDiffSource("s1", fetch, "/repo") - const { post, messages } = collect() + const source = createSessionDiffSource("s1", fetch, "/repo") - await source.initialFetch(post) + const result = await source.fetch() expect(calls).toEqual([{ sessionID: "s1", directory: "/repo" }]) - expect(messages).toEqual([ - { type: "loading", loading: true }, - { type: "diffs", diffs: [] }, - { type: "loading", loading: false }, - ]) + expect(result).toEqual({ diffs: [] }) }) it("converts patches into before/after diffs", async () => { @@ -64,16 +58,13 @@ describe("SessionDiffSource.initialFetch", () => { }, ] const { fetch } = recording(raw) - const source = new SessionDiffSource("s2", fetch, "/repo") - const { post, messages } = collect() + const source = createSessionDiffSource("s2", fetch, "/repo") - await source.initialFetch(post) + const result = await source.fetch() - const diffsMsg = messages.find((m) => m.type === "diffs") - if (diffsMsg?.type !== "diffs") throw new Error("expected diffs message") - expect(diffsMsg.diffs).toHaveLength(2) + expect(result.diffs).toHaveLength(2) - const foo = diffsMsg.diffs[0]! + const foo = result.diffs[0]! expect(foo.file).toBe("foo.ts") expect(foo.before).toBe("keep\nold\n") expect(foo.after).toBe("keep\nnew\n") @@ -84,94 +75,80 @@ describe("SessionDiffSource.initialFetch", () => { expect(foo.generatedLike).toBe(false) expect(foo.summarized).toBe(false) - const big = diffsMsg.diffs[1]! + const big = result.diffs[1]! expect(big.summarized).toBe(true) expect(big.before).toBe("") expect(big.after).toBe("") }) - it("reports an error when the fetch throws", async () => { + it("propagates errors from the underlying fetch", async () => { const { fetch } = recording(new Error("network down")) - const source = new SessionDiffSource("s3", fetch) - const { post, messages } = collect() + const source = createSessionDiffSource("s3", fetch) - await source.initialFetch(post) - - expect(messages).toEqual([ - { type: "loading", loading: true }, - { type: "error", message: "network down" }, - { type: "loading", loading: false }, - ]) + await expect(source.fetch()).rejects.toThrow("network down") }) it("calls fetch without directory when workspaceRoot is not given", async () => { const { fetch, calls } = recording([]) - const source = new SessionDiffSource("s4", fetch) - const { post } = collect() + const source = createSessionDiffSource("s4", fetch) - await source.initialFetch(post) + await source.fetch() expect(calls).toEqual([{ sessionID: "s4", directory: undefined }]) }) }) -describe("SessionDiffSource lifecycle", () => { - it("dispose does not throw", () => { - const { fetch } = recording([]) - const source = new SessionDiffSource("s6", fetch) - source.dispose() - }) - - it("descriptor id encodes the session id", () => { - const { fetch } = recording([]) - const source = new SessionDiffSource("abc", fetch) +describe("createSessionDiffSource descriptor", () => { + it("encodes the session id in the descriptor", () => { + const source = createSessionDiffSource("abc", recording([]).fetch) expect(source.descriptor.id).toBe("session:abc") expect(source.descriptor.group).toBe("Session") expect(source.descriptor.capabilities).toEqual({ revert: false, comments: true }) }) - it("posts the snapshots-disabled notice and skips fetch when the check returns false", async () => { + it("exposes a stable descriptor helper", () => { + expect(sessionDescriptor("xyz").id).toBe("session:xyz") + }) +}) + +describe("createSessionDiffSource snapshot check", () => { + it("returns the snapshots-disabled notice and skips fetch when the check returns false", async () => { const { fetch, calls } = recording([ { file: "foo.ts", patch: modifiedPatch, additions: 1, deletions: 1, status: "modified" }, ]) const checkSnapshotsEnabled: SnapshotEnabledCheck = async () => false - const source = new SessionDiffSource("s-disabled", fetch, "/repo", checkSnapshotsEnabled) - const { post, messages } = collect() + const source = createSessionDiffSource("s-disabled", fetch, "/repo", checkSnapshotsEnabled) - await source.initialFetch(post) + const result = await source.fetch() expect(calls).toEqual([]) - expect(messages).toEqual([ - { type: "loading", loading: true }, - { type: "notice", notice: "snapshots-disabled" }, - { type: "diffs", diffs: [] }, - { type: "loading", loading: false }, - ]) + expect(result).toEqual({ diffs: [], notice: "snapshots-disabled", stopPolling: true }) + }) + + it("caches the disabled state so subsequent fetches skip the config lookup", async () => { + const { fetch } = recording([]) + let checks = 0 + const checkSnapshotsEnabled: SnapshotEnabledCheck = async () => { + checks++ + return false + } + const source = createSessionDiffSource("s-cache", fetch, "/repo", checkSnapshotsEnabled) + + await source.fetch() + await source.fetch() + + expect(checks).toBe(1) }) it("fetches normally when snapshots are enabled", async () => { const { fetch, calls } = recording([]) const checkSnapshotsEnabled: SnapshotEnabledCheck = async () => true - const source = new SessionDiffSource("s-enabled", fetch, "/repo", checkSnapshotsEnabled) - const { post, messages } = collect() + const source = createSessionDiffSource("s-enabled", fetch, "/repo", checkSnapshotsEnabled) - await source.initialFetch(post) + const result = await source.fetch() expect(calls).toEqual([{ sessionID: "s-enabled", directory: "/repo" }]) - expect(messages.some((m) => m.type === "notice")).toBe(false) - expect(messages.filter((m) => m.type === "diffs")).toHaveLength(1) - }) - - it("start() is a no-op when snapshots are disabled", async () => { - const { fetch } = recording([]) - const checkSnapshotsEnabled: SnapshotEnabledCheck = async () => false - const source = new SessionDiffSource("s-disabled-2", fetch, "/repo", checkSnapshotsEnabled) - const { post } = collect() - - await source.initialFetch(post) - const disposable = source.start(post) - expect(typeof disposable.dispose).toBe("function") - // Disposing must not throw even though no interval was scheduled. - disposable.dispose() + expect(result.notice).toBeUndefined() + expect(result.stopPolling).toBeUndefined() }) }) diff --git a/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts b/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts index cd37bb906f..49b3cc28dd 100644 --- a/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts +++ b/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts @@ -1,12 +1,12 @@ import { describe, it, expect } from "bun:test" import type { KiloConnectionService } from "../../src/services/cli-backend" import { DiffSourceCatalog } from "../../src/diff/sources/catalog" -import { SessionDiffSource, sessionDescriptor } from "../../src/diff/sources/session" -import { WORKSPACE_DESCRIPTOR, WorktreeDiffSource } from "../../src/diff/sources/worktree" +import { sessionDescriptor } from "../../src/diff/sources/session" +import { WORKSPACE_DESCRIPTOR } from "../../src/diff/sources/worktree" // Minimal stand-in for the connection service — the catalog only holds a -// reference and passes it to the source constructors, so we never exercise -// any of its methods in these tests. +// reference and passes it to the source factories, so we never exercise any +// of its methods in these tests. const connection = {} as unknown as KiloConnectionService function makeCatalog(): DiffSourceCatalog { @@ -33,6 +33,11 @@ describe("DiffSourceCatalog.listAvailable", () => { const out = makeCatalog().listAvailable({ workspaceRoot: undefined }) expect(out).toEqual([]) }) + + it("returns [] when hidePicker is set, regardless of workspace/session", () => { + const out = makeCatalog().listAvailable({ workspaceRoot: "/repo", sessionId: "s1", hidePicker: true }) + expect(out).toEqual([]) + }) }) describe("DiffSourceCatalog.defaultSourceId", () => { @@ -67,18 +72,34 @@ describe("DiffSourceCatalog.defaultSourceId", () => { }) describe("DiffSourceCatalog.build", () => { - it("builds a WorktreeDiffSource for 'workspace'", () => { + it("builds a workspace source for 'workspace'", () => { const src = makeCatalog().build("workspace", { workspaceRoot: "/repo" }) - expect(src).toBeInstanceOf(WorktreeDiffSource) expect(src.descriptor.id).toBe("workspace") - src.dispose() + expect(src.descriptor.type).toBe("workspace") + expect(src.revert).toBeDefined() + expect(src.fetchFile).toBeDefined() + src.dispose?.() }) - it("builds a SessionDiffSource for 'session:'", () => { + it("builds a session source for 'session:'", () => { const src = makeCatalog().build("session:s1", { workspaceRoot: "/repo", sessionId: "s1" }) - expect(src).toBeInstanceOf(SessionDiffSource) expect(src.descriptor.id).toBe("session:s1") - src.dispose() + expect(src.descriptor.type).toBe("session") + expect(src.revert).toBeUndefined() + src.dispose?.() + }) + + it("builds a turn source for 'turn::'", () => { + const src = makeCatalog().build("turn:sess:msg", { workspaceRoot: "/repo" }) + expect(src.descriptor.id).toBe("turn:sess:msg") + expect(src.descriptor.type).toBe("turn") + expect(src.revert).toBeUndefined() + src.dispose?.() + }) + + it("throws on a malformed turn id", () => { + expect(() => makeCatalog().build("turn:sess", { workspaceRoot: "/repo" })).toThrow(/malformed turn id/) + expect(() => makeCatalog().build("turn:", { workspaceRoot: "/repo" })).toThrow(/malformed turn id/) }) it("throws on an empty session id", () => { diff --git a/packages/kilo-vscode/tests/unit/diff-turn-source.test.ts b/packages/kilo-vscode/tests/unit/diff-turn-source.test.ts new file mode 100644 index 0000000000..55e5055d07 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/diff-turn-source.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect } from "bun:test" +import type { SnapshotFileDiff } from "@kilocode/sdk/v2/client" +import { + createTurnDiffSource, + turnDescriptor, + turnSourceId, + TURN_PREFIX, + type TurnDiffFetch, +} from "../../src/diff/sources/turn" + +type FetchCall = { sessionID: string; messageID: string; directory?: string } + +function recording(result: SnapshotFileDiff[] | Error): { fetch: TurnDiffFetch; calls: FetchCall[] } { + const calls: FetchCall[] = [] + const fetch: TurnDiffFetch = async (params) => { + calls.push(params) + if (result instanceof Error) throw result + return result + } + return { fetch, calls } +} + +const samplePatch = [ + "diff --git a/foo.ts b/foo.ts", + "--- a/foo.ts", + "+++ b/foo.ts", + "@@ -1,1 +1,1 @@", + "-old", + "+new", +].join("\n") + +describe("createTurnDiffSource.fetch", () => { + it("calls the fetch with sessionID + messageID + directory", async () => { + const { fetch, calls } = recording([]) + const source = createTurnDiffSource("sess", "msg", fetch, "/repo") + + await source.fetch() + + expect(calls).toEqual([{ sessionID: "sess", messageID: "msg", directory: "/repo" }]) + }) + + it("returns diffs with stopPolling=true so the controller skips polling", async () => { + const { fetch } = recording([ + { file: "foo.ts", patch: samplePatch, additions: 1, deletions: 1, status: "modified" }, + ]) + const source = createTurnDiffSource("sess", "msg", fetch) + + const result = await source.fetch() + + expect(result.stopPolling).toBe(true) + expect(result.notice).toBeUndefined() + expect(result.diffs).toHaveLength(1) + expect(result.diffs[0]!.file).toBe("foo.ts") + expect(result.diffs[0]!.before).toBe("old\n") + expect(result.diffs[0]!.after).toBe("new\n") + }) + + it("propagates underlying fetch errors", async () => { + const { fetch } = recording(new Error("backend unavailable")) + const source = createTurnDiffSource("sess", "msg", fetch) + + await expect(source.fetch()).rejects.toThrow("backend unavailable") + }) + + it("calls fetch without directory when workspaceRoot is not given", async () => { + const { fetch, calls } = recording([]) + const source = createTurnDiffSource("sess", "msg", fetch) + + await source.fetch() + + expect(calls).toEqual([{ sessionID: "sess", messageID: "msg", directory: undefined }]) + }) +}) + +describe("turn source descriptor + id helpers", () => { + it("encodes sessionId + messageId in the source id", () => { + expect(turnSourceId("abc", "42")).toBe(`${TURN_PREFIX}abc:42`) + }) + + it("produces a descriptor with type='turn' and no revert capability", () => { + const desc = turnDescriptor("abc", "42") + expect(desc.id).toBe("turn:abc:42") + expect(desc.type).toBe("turn") + expect(desc.capabilities).toEqual({ revert: false, comments: true }) + }) +}) diff --git a/packages/kilo-vscode/tests/unit/source-controller.test.ts b/packages/kilo-vscode/tests/unit/source-controller.test.ts index 509a7e5c19..e217bf927c 100644 --- a/packages/kilo-vscode/tests/unit/source-controller.test.ts +++ b/packages/kilo-vscode/tests/unit/source-controller.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect } from "bun:test" -import * as vscode from "vscode" import { SourceController } from "../../src/diff/SourceController" -import type { DiffSource, DiffSourceDescriptor, DiffSourcePost } from "../../src/diff/sources/types" +import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "../../src/diff/sources/types" const WORKSPACE_DESC: DiffSourceDescriptor = { id: "workspace", @@ -17,10 +16,6 @@ const SESSION_DESC: DiffSourceDescriptor = { capabilities: { revert: false, comments: true }, } -function disposable(onDispose: () => void = () => {}): vscode.Disposable { - return new vscode.Disposable(onDispose) -} - function make(sources: Record, descriptors?: DiffSourceDescriptor[]) { const posted: unknown[] = [] const controller = new SourceController( @@ -41,25 +36,21 @@ const byType = (posted: unknown[], type: string) => }) describe("SourceController.activate", () => { - it("builds, fetches, and starts the source", async () => { - let starts = 0 + it("builds, fetches, and posts available sources + capabilities + diffs", async () => { + let fetches = 0 const source: DiffSource = { descriptor: SESSION_DESC, - async initialFetch(post) { - post({ type: "diffs", diffs: [] }) + async fetch(): Promise { + fetches++ + return { diffs: [] } }, - start() { - starts++ - return disposable() - }, - dispose() {}, } const { controller, posted } = make({ "session:s1": source }, [WORKSPACE_DESC, SESSION_DESC]) controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) await controller.activate("session:s1") - expect(starts).toBe(1) + expect(fetches).toBe(1) expect(controller.currentId).toBe("session:s1") const available = byType(posted, "setAvailableSources") @@ -70,32 +61,53 @@ describe("SourceController.activate", () => { const caps = byType(posted, "diffViewer.capabilities") expect(caps).toHaveLength(1) expect(caps[0]!.capabilities).toEqual({ revert: false, comments: true }) + + const diffs = byType(posted, "diffViewer.diffs") + expect(diffs).toHaveLength(1) + expect(diffs[0]!.diffs).toEqual([]) + + const loading = byType(posted, "diffViewer.loading") + expect(loading.map((m) => m.loading)).toEqual([true, false]) + + // Clean up the polling interval scheduled by activate(). + controller.stop() + }) + + it("forwards notices from the source to the webview", async () => { + const source: DiffSource = { + descriptor: SESSION_DESC, + async fetch() { + return { diffs: [], notice: "snapshots-disabled", stopPolling: true } + }, + } + const { controller, posted } = make({ "session:s1": source }) + + controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) + await controller.activate("session:s1") + + const notices = byType(posted, "diffViewer.notice") + expect(notices).toHaveLength(1) + expect(notices[0]!.notice).toBe("snapshots-disabled") }) it("disposes the previous source when activating a new one", async () => { let workspaceDisposed = 0 - let workspaceSubscriptionDisposed = false const workspace: DiffSource = { descriptor: WORKSPACE_DESC, - async initialFetch() {}, - start() { - return disposable(() => { - workspaceSubscriptionDisposed = true - }) + async fetch() { + return { diffs: [] } }, dispose() { workspaceDisposed++ }, } - let sessionStarts = 0 + let sessionFetches = 0 const session: DiffSource = { descriptor: SESSION_DESC, - async initialFetch() {}, - start() { - sessionStarts++ - return disposable() + async fetch() { + sessionFetches++ + return { diffs: [] } }, - dispose() {}, } const { controller } = make({ workspace, "session:s1": session }) @@ -104,31 +116,28 @@ describe("SourceController.activate", () => { await controller.activate("session:s1") expect(workspaceDisposed).toBe(1) - expect(workspaceSubscriptionDisposed).toBe(true) - expect(sessionStarts).toBe(1) + expect(sessionFetches).toBe(1) expect(controller.currentId).toBe("session:s1") + + controller.stop() }) - it("does not start polling if the controller is stopped during initialFetch", async () => { + it("drops a fetch result and disposes the source when stopped mid-fetch", async () => { let release: () => void = () => {} - let fetched = 0 - let started = 0 + let fetches = 0 let disposed = 0 const session: DiffSource = { descriptor: SESSION_DESC, - async initialFetch() { - fetched++ + async fetch() { + fetches++ await new Promise((r) => (release = r)) - }, - start() { - started++ - return disposable() + return { diffs: [] } }, dispose() { disposed++ }, } - const { controller } = make({ "session:s1": session }) + const { controller, posted } = make({ "session:s1": session }) controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) const activation = controller.activate("session:s1") @@ -136,57 +145,51 @@ describe("SourceController.activate", () => { release() await activation - expect(fetched).toBe(1) - expect(started).toBe(0) + expect(fetches).toBe(1) expect(disposed).toBe(1) + // Fetch resolved after stop — its diffs must not leak to the webview. + expect(byType(posted, "diffViewer.diffs")).toEqual([]) }) - it("drops stale posts from a source that was swapped out", async () => { - let capturedPost: DiffSourcePost | undefined + it("drops a fetch result from a source that has been swapped out", async () => { + let release: () => void = () => {} const workspace: DiffSource = { descriptor: WORKSPACE_DESC, - async initialFetch(post) { - capturedPost = post + async fetch() { + await new Promise((r) => (release = r)) + return { diffs: [{ file: "stale.ts" } as never] } }, - start() { - return disposable() - }, - dispose() {}, } const session: DiffSource = { descriptor: SESSION_DESC, - async initialFetch(post) { - post({ type: "diffs", diffs: [] }) + async fetch() { + return { diffs: [] } }, - start() { - return disposable() - }, - dispose() {}, } const { controller, posted } = make({ workspace, "session:s1": session }) controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) - await controller.activate("workspace") + const first = controller.activate("workspace") await controller.activate("session:s1") + release() + await first - posted.length = 0 - capturedPost?.({ type: "diffs", diffs: [{ file: "stale.ts" } as never] }) + const diffs = byType(posted, "diffViewer.diffs") + // Only the session source's empty diffs should have been posted. + expect(diffs).toHaveLength(1) + expect(diffs[0]!.diffs).toEqual([]) - expect(byType(posted, "diffViewer.diffs")).toEqual([]) + controller.stop() }) }) describe("SourceController.stop", () => { - it("disposes the active source and its start subscription", async () => { + it("disposes the active source", async () => { let disposed = 0 - let subscriptionDisposed = false const session: DiffSource = { descriptor: SESSION_DESC, - async initialFetch() {}, - start() { - return disposable(() => { - subscriptionDisposed = true - }) + async fetch() { + return { diffs: [] } }, dispose() { disposed++ @@ -200,7 +203,12 @@ describe("SourceController.stop", () => { controller.stop() expect(disposed).toBe(1) - expect(subscriptionDisposed).toBe(true) + expect(controller.currentId).toBeUndefined() + }) + + it("is a no-op when no source is active", () => { + const { controller } = make({}) + controller.stop() expect(controller.currentId).toBeUndefined() }) }) @@ -209,11 +217,9 @@ describe("SourceController.revertFile", () => { it("posts error when the active source does not support revert", async () => { const session: DiffSource = { descriptor: SESSION_DESC, - async initialFetch() {}, - start() { - return disposable() + async fetch() { + return { diffs: [] } }, - dispose() {}, } const { controller, posted } = make({ "session:s1": session }) @@ -226,46 +232,51 @@ describe("SourceController.revertFile", () => { expect(results).toHaveLength(1) expect(results[0]!.status).toBe("error") expect(results[0]!.file).toBe("foo.ts") + + controller.stop() }) - it("posts success from a successful revert", async () => { - const calls: string[] = [] + it("posts success from a successful revert and triggers a fresh fetch", async () => { + const reverts: string[] = [] + let fetches = 0 const workspace: DiffSource = { descriptor: WORKSPACE_DESC, - async initialFetch() {}, - start() { - return disposable() + async fetch() { + fetches++ + return { diffs: [] } }, - async revertFile(file) { - calls.push(file) + async revert(file) { + reverts.push(file) return { ok: true, message: "Reverted" } }, - dispose() {}, } const { controller, posted } = make({ workspace }) controller.setContext({ workspaceRoot: "/repo" }) await controller.activate("workspace") + const fetchesAfterActivate = fetches posted.length = 0 await controller.revertFile("foo.ts") - expect(calls).toEqual(["foo.ts"]) + expect(reverts).toEqual(["foo.ts"]) const results = byType(posted, "diffViewer.revertFileResult") expect(results[0]!.status).toBe("success") expect(results[0]!.message).toBe("Reverted") + // Successful revert triggers an immediate re-fetch to push updated diffs. + expect(fetches).toBe(fetchesAfterActivate + 1) + + controller.stop() }) it("posts error when the revert implementation throws", async () => { const workspace: DiffSource = { descriptor: WORKSPACE_DESC, - async initialFetch() {}, - start() { - return disposable() + async fetch() { + return { diffs: [] } }, - async revertFile() { + async revert() { throw new Error("boom") }, - dispose() {}, } const { controller, posted } = make({ workspace }) @@ -277,5 +288,61 @@ describe("SourceController.revertFile", () => { const results = byType(posted, "diffViewer.revertFileResult") expect(results[0]!.status).toBe("error") expect(results[0]!.message).toBe("boom") + + controller.stop() + }) +}) + +describe("SourceController.requestFile", () => { + it("posts null when the source does not support per-file detail", async () => { + const session: DiffSource = { + descriptor: SESSION_DESC, + async fetch() { + return { diffs: [] } + }, + } + const { controller, posted } = make({ "session:s1": session }) + + controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) + await controller.activate("session:s1") + posted.length = 0 + await controller.requestFile("foo.ts") + + const files = byType(posted, "diffViewer.diffFile") + expect(files).toHaveLength(1) + expect(files[0]!.file).toBe("foo.ts") + expect(files[0]!.diff).toBeNull() + + controller.stop() + }) + + it("forwards the source's fetchFile result", async () => { + const detail = { + file: "foo.ts", + before: "a", + after: "b", + additions: 1, + deletions: 1, + } + const workspace: DiffSource = { + descriptor: WORKSPACE_DESC, + async fetch() { + return { diffs: [] } + }, + async fetchFile(file) { + return file === "foo.ts" ? detail : null + }, + } + const { controller, posted } = make({ workspace }) + + controller.setContext({ workspaceRoot: "/repo" }) + await controller.activate("workspace") + posted.length = 0 + await controller.requestFile("foo.ts") + + const files = byType(posted, "diffViewer.diffFile") + expect(files[0]!.diff).toEqual(detail) + + controller.stop() }) }) diff --git a/packages/kilo-vscode/webview-ui/src/components/chat/VscodeSessionTurn.tsx b/packages/kilo-vscode/webview-ui/src/components/chat/VscodeSessionTurn.tsx index 5bf289eb58..c4de848fa2 100644 --- a/packages/kilo-vscode/webview-ui/src/components/chat/VscodeSessionTurn.tsx +++ b/packages/kilo-vscode/webview-ui/src/components/chat/VscodeSessionTurn.tsx @@ -9,17 +9,11 @@ * - Simpler flat structure without overflow containers */ -import { Component, createMemo, For, Show, createSignal, createEffect, on } from "solid-js" -import { Dynamic } from "solid-js/web" +import { Component, createMemo, For, Show, createEffect } from "solid-js" import { UserMessageDisplay } from "@kilocode/kilo-ui/message-part" -import { Collapsible } from "@kilocode/kilo-ui/collapsible" -import { Accordion } from "@kilocode/kilo-ui/accordion" import { DiffChanges } from "@kilocode/kilo-ui/diff-changes" import { Icon } from "@kilocode/kilo-ui/icon" -import { StickyAccordionHeader } from "@kilocode/kilo-ui/sticky-accordion-header" import { useData } from "@kilocode/kilo-ui/context/data" -import { useFileComponent } from "@kilocode/kilo-ui/context/file" -import { contents } from "@kilocode/kilo-ui/session-diff" import { useI18n } from "@kilocode/kilo-ui/context/i18n" import { AssistantMessage } from "./AssistantMessage" import type { @@ -32,23 +26,12 @@ import { ErrorDisplay } from "./ErrorDisplay" import { useServer } from "../../context/server" import { useSession } from "../../context/session" import { useLanguage } from "../../context/language" +import { useVSCode } from "../../context/vscode" import { useFeedback } from "../../context/feedback" import { visibleError } from "../../context/session-errors" import type { ErrorDisplayProps } from "./ErrorDisplay" import type { Message as WebMessage } from "../../types/messages" -function getDirectory(path: string): string { - const sep = path.includes("/") ? "/" : "\\" - const idx = path.lastIndexOf(sep) - return idx === -1 ? "" : path.slice(0, idx + 1) -} - -function getFilename(path: string): string { - const sep = path.includes("/") ? "/" : "\\" - const idx = path.lastIndexOf(sep) - return idx === -1 ? path : path.slice(idx + 1) -} - export interface VscodeTurn { id: string user: WebMessage @@ -65,10 +48,10 @@ interface VscodeSessionTurnProps { export const VscodeSessionTurn: Component = (props) => { const data = useData() const i18n = useI18n() - const fileComponent = useFileComponent() const server = useServer() const session = useSession() const language = useLanguage() + const vscode = useVSCode() const feedback = useFeedback() const emptyParts: SDKPart[] = [] @@ -108,18 +91,7 @@ export const VscodeSessionTurn: Component = (props) => { .reverse() }) - const [open, setOpen] = createSignal(false) - const [expanded, setExpanded] = createSignal([]) - - createEffect( - on( - open, - (value, prev) => { - if (!value && prev) setExpanded([]) - }, - { defer: true }, - ), - ) + const openChanges = () => vscode.postMessage({ type: "openChanges", turnId: message().id }) // Copy part ID — the last text part from the last assistant message. // Synthetic parts (e.g. "Initializing snapshot…" from the slow-repo guard) @@ -205,104 +177,26 @@ export const VscodeSessionTurn: Component = (props) => { - {/* Diff summary — shown after completion */} + {/* Diff summary — shown after completion. Click opens the changes view. */} 0 && server.gitInstalled()}>
- - -
-
- {i18n.t("ui.sessionReview.change.modified")}{" "} - - {diffs().length} {i18n.t(diffs().length === 1 ? "ui.common.file.one" : "ui.common.file.other")} - -
- - -
-
-
-
- - -
- setExpanded(Array.isArray(value) ? value : value ? [value] : [])} - > - - {(diff) => { - const active = createMemo(() => expanded().includes(diff.file)) - const [visible, setVisible] = createSignal(false) - - createEffect( - on( - active, - (value) => { - if (!value) { - setVisible(false) - return - } - requestAnimationFrame(() => { - if (active()) setVisible(true) - }) - }, - { defer: true }, - ), - ) - - return ( - - - -
- - - - {`\u2066${getDirectory(diff.file)}\u2069`} - - - {getFilename(diff.file)} - -
- - - - - - -
-
-
-
- - -
- {(() => { - const view = diff.patch === "" ? { before: "", after: "" } : contents(diff) - return ( - - ) - })()} -
-
-
-
- ) - }} -
-
-
-
-
-
+
diff --git a/packages/kilo-vscode/webview-ui/src/stories/composite.stories.tsx b/packages/kilo-vscode/webview-ui/src/stories/composite.stories.tsx index fc4a65707f..30739bd331 100644 --- a/packages/kilo-vscode/webview-ui/src/stories/composite.stories.tsx +++ b/packages/kilo-vscode/webview-ui/src/stories/composite.stories.tsx @@ -1140,7 +1140,7 @@ export const McpToolExpanded: Story = { } // --------------------------------------------------------------------------- -// 19. Diff summary — "Modified N files" collapsed header +// 19. Diff summary — "Modified N files" banner (opens changes view on click) // --------------------------------------------------------------------------- const USER_MSG_ID = "user-msg-diff-001" @@ -1152,7 +1152,7 @@ const mockDiffs = [ ] export const DiffSummaryCollapsed: Story = { - name: "Diff Summary — Modified N files (collapsed)", + name: "Diff Summary — Modified N files", render: () => { const data = { ...defaultMockData, diff --git a/packages/kilo-vscode/webview-ui/src/styles/chat-layout.css b/packages/kilo-vscode/webview-ui/src/styles/chat-layout.css index 2afdedb0c9..5cbb811d05 100644 --- a/packages/kilo-vscode/webview-ui/src/styles/chat-layout.css +++ b/packages/kilo-vscode/webview-ui/src/styles/chat-layout.css @@ -213,8 +213,50 @@ width: 100%; } -.vscode-session-turn-diffs [data-slot="session-turn-diffs-title"] { - display: flex; - align-items: baseline; +.vscode-session-turn-diffs-trigger { + display: inline-flex; + align-items: center; gap: 6px; + padding: 0; + margin: 0; + background: transparent; + border: none; + color: inherit; + font: inherit; + text-align: left; + cursor: pointer; +} + +.vscode-session-turn-diffs-trigger [data-slot="session-turn-diffs-label"] { + color: var(--text-strong); + font-family: var(--font-family-sans); + font-size: var(--font-size-base); + font-weight: var(--font-weight-medium); + line-height: var(--line-height-large); +} + +.vscode-session-turn-diffs-trigger [data-slot="session-turn-diffs-chevron"] { + display: inline-flex; + align-items: center; + color: var(--icon-weaker); + opacity: 0; + transition: opacity 0.15s ease; +} + +.vscode-session-turn-diffs-trigger:hover [data-slot="session-turn-diffs-chevron"], +.vscode-session-turn-diffs-trigger:focus-visible [data-slot="session-turn-diffs-chevron"] { + opacity: 1; +} + +.vscode-session-turn-diffs-trigger [data-slot="session-turn-diffs-count"] { + color: var(--text-weak); + font-family: var(--font-family-sans); + font-size: var(--font-size-small); + line-height: var(--line-height-large); + font-variant-numeric: tabular-nums; +} + +.vscode-session-turn-diffs-trigger [data-slot="session-turn-diffs-meta"] { + display: inline-flex; + align-items: center; } diff --git a/packages/kilo-vscode/webview-ui/src/types/messages/webview-messages.ts b/packages/kilo-vscode/webview-ui/src/types/messages/webview-messages.ts index 1cbbb5fff6..74811c2959 100644 --- a/packages/kilo-vscode/webview-ui/src/types/messages/webview-messages.ts +++ b/packages/kilo-vscode/webview-ui/src/types/messages/webview-messages.ts @@ -740,6 +740,12 @@ export interface EnhancePromptRequest { // Open the standalone changes viewer tab from the sidebar export interface OpenChangesRequest { type: "openChanges" + /** + * When set, opens the viewer scoped to a single turn (identified by the + * user message ID). The source picker is hidden and polling is disabled + * for this mode. + */ + turnId?: string } // Open diff virtual (permission diff) in the lightweight diff virtual panel