Revert "Merge pull request #13449 from Kilo-Org/perf/agent-manager-bulk-diff-details"

This reverts commit f78d736e97, reversing
changes made to 7834f9308e.
This commit is contained in:
marius-kilocode
2026-08-26 11:20:49 +02:00
parent 472c301da2
commit 24cf297c80
27 changed files with 91 additions and 1188 deletions
@@ -1,5 +0,0 @@
---
"kilo-code": patch
---
Load Agent Manager worktree reviews faster by batching Git file details.
@@ -468,6 +468,9 @@ export class AgentManagerProvider implements Disposable {
return
}
// When the .kilocode → .kilo migration rewrote git worktree refs, nudge
// VS Code's git extension to re-discover them. Without this, worktrees
// won't appear in Source Control until the next VS Code restart.
if (init.refsFixed > 0) {
this.log(`Migration fixed ${init.refsFixed} git worktree ref(s), refreshing git`)
this.host.refreshGit()
@@ -478,6 +481,10 @@ export class AgentManagerProvider implements Disposable {
for (const s of state.getSessions()) this.panel?.sessions.trackSession(s.id)
this.pushState()
// Always list sessions, even when the state tracks none: the backend may
// still hold sessions for this project, and without the listing the
// sessionsLoaded message never reaches the webview, leaving the sidebar
// on skeletons forever.
this.panel?.sessions.refreshSessions()
// Recover any pending permission/question prompts that were missed during
@@ -834,11 +841,6 @@ export class AgentManagerProvider implements Disposable {
void this.diffs.requestFile(composeDiffId(m.sessionId, normalizeScope(m.scope), m.diffSessionId), m.file)
return null
}
if (m.type === "agentManager.requestWorktreeDiffFiles") {
const id = composeDiffId(m.sessionId, normalizeScope(m.scope), m.diffSessionId)
void this.diffs.requestFiles(m.projectId, id, m.files)
return null
}
if (m.type === "agentManager.applyWorktreeDiff") {
void this.diffs.apply(m.worktreeId, m.selectedFiles)
return null
@@ -599,11 +599,7 @@ export class GitOps {
return this.exec(args, cwd, options)
}
execGitBuffer(
args: string[],
cwd: string,
options?: { stdin?: string; signal?: AbortSignal },
): Promise<ExecBufferResult> {
execGitBuffer(args: string[], cwd: string, options?: { signal?: AbortSignal }): Promise<ExecBufferResult> {
return this.execBuffer(args, cwd, options)
}
@@ -613,40 +609,32 @@ export class GitOps {
}
private async execBuffer(args: string[], cwd: string, options?: ExecOptions): Promise<ExecBufferResult> {
if (this.controller.signal.aborted || options?.signal?.aborted) {
if (this.controller.signal.aborted) {
return { code: 1, stdout: Buffer.alloc(0), stderr: "GitOps disposed" }
}
const cmd = await this.executable(options?.signal).catch(() => undefined)
if (!cmd || this.controller.signal.aborted || options?.signal?.aborted) {
const cmd = await this.executable().catch(() => undefined)
if (!cmd || this.controller.signal.aborted) {
return { code: 1, stdout: Buffer.alloc(0), stderr: "GitOps disposed" }
}
const invoke = () => this.invoke(cmd, args, cwd, options)
return this.semaphore ? this.semaphore.run(invoke, options?.signal) : invoke()
}
private executable(cancel?: AbortSignal): Promise<string> {
private executable(): Promise<string> {
const signal = this.controller.signal
if (signal.aborted || cancel?.aborted) return Promise.reject(new Error("GitOps disposed"))
if (signal.aborted) return Promise.reject(new Error("GitOps disposed"))
return new Promise<string>((resolve, reject) => {
const clear = () => {
signal.removeEventListener("abort", abort)
cancel?.removeEventListener("abort", abort)
}
const abort = () => {
clear()
reject(new Error("GitOps disposed"))
}
signal.addEventListener("abort", abort, { once: true })
cancel?.addEventListener("abort", abort, { once: true })
const onAbort = () => reject(new Error("GitOps disposed"))
signal.addEventListener("abort", onAbort, { once: true })
const cache = (this.executableCache ??= Promise.resolve().then(() => this.binary()))
cache.then(
(value) => {
clear()
signal.removeEventListener("abort", onAbort)
resolve(value)
},
(err) => {
clear()
signal.removeEventListener("abort", onAbort)
if (this.executableCache === cache) this.executableCache = undefined
reject(err)
},
@@ -1,84 +0,0 @@
import type { GitOps } from "./GitOps"
type Entry = { file: string; status: string; tracked: boolean }
type Base = { id: string; bytes: number }
export function check(signal?: AbortSignal) {
if (signal?.aborted) throw new Error("Diff detail aborted")
}
export async function inspect(git: GitOps, dir: string, anc: string, metas: Entry[], signal?: AbortSignal) {
const items = metas.filter((meta) => meta.status !== "added")
const result = new Map<string, Base>()
if (items.length === 0) return result
const stdin = items.map((meta) => `${anc}:${meta.file}\n`).join("")
const output = await git.execGit(["cat-file", "--batch-check"], dir, { stdin, signal })
check(signal)
if (output.code !== 0) throw new Error("Could not inspect base files")
const lines = output.stdout.trimEnd().split("\n")
if (lines.length !== items.length) throw new Error("Incomplete base file metadata")
for (const [index, meta] of items.entries()) {
const [id, type, value] = lines[index]!.split(" ")
const bytes = Number(value)
if (!id || type !== "blob" || !Number.isSafeInteger(bytes) || bytes < 0) {
throw new Error(`Could not inspect base file for ${meta.file}`)
}
result.set(meta.file, { id, bytes })
}
return result
}
export async function blobs(git: GitOps, dir: string, metas: Entry[], base: Map<string, Base>, signal?: AbortSignal) {
const items = metas.filter((meta) => meta.status !== "added")
const result = new Map<string, Buffer>()
if (items.length === 0) return result
const stdin = items.map((meta) => `${base.get(meta.file)!.id}\n`).join("")
const output = await git.execGitBuffer(["cat-file", "--batch"], dir, { stdin, signal })
check(signal)
if (output.code !== 0) throw new Error("Could not read base files")
let offset = 0
for (const meta of items) {
const end = output.stdout.indexOf(10, offset)
if (end === -1) throw new Error(`Incomplete base file for ${meta.file}`)
const [id, type, value] = output.stdout.subarray(offset, end).toString("utf8").split(" ")
const size = Number(value)
const expected = base.get(meta.file)!
const next = end + 1 + size
if (id !== expected.id || type !== "blob" || size !== expected.bytes || output.stdout[next] !== 10) {
throw new Error(`Invalid base file for ${meta.file}`)
}
result.set(meta.file, output.stdout.subarray(end + 1, next))
offset = next + 1
}
return result
}
export async function patches(git: GitOps, dir: string, anc: string, metas: Entry[], signal?: AbortSignal) {
const items = metas.filter((meta) => meta.tracked)
const result = new Map<string, string>()
if (items.length === 0) return result
const output = await git.execGit(
[
"-c",
"core.quotepath=false",
"diff",
"--no-ext-diff",
"--no-renames",
anc,
"--",
...items.map((meta) => meta.file),
],
dir,
{ signal },
)
check(signal)
if (output.code !== 0) throw new Error("Could not create file diffs")
for (const patch of output.stdout.split(/(?=^diff --git )/m)) {
if (!patch) continue
const line = patch.slice(0, patch.indexOf("\n"))
const meta = items.find((item) => line === `diff --git a/${item.file} b/${item.file}`)
if (!meta) throw new Error("Could not match a file diff")
result.set(meta.file, patch)
}
return result
}
@@ -1,10 +1,8 @@
import * as fs from "fs/promises"
import { binaryFile } from "../diff/shared/binary"
import { imageMime, loadImage, readImageFile } from "../diff/shared/image"
import type { DiffBatch } from "../diff/types"
import { resolveInside } from "../diff/shared/path"
import type { GitOps } from "./GitOps"
import { blobs, check, inspect, patches } from "./local-diff-batch"
import type { WorktreeDiffEntry } from "./types"
type Status = "added" | "deleted" | "modified"
@@ -33,7 +31,6 @@ const MAX_UNTRACKED_BYTES = 1_000_000
* metadata preserved) so the webview can render counts without
* materializing the content. */
export const MAX_DETAIL_BYTES = 20_000_000
const MAX_BATCH_BYTES = 32 * 1024 * 1024
/**
* Local, Node.js-side replacement for the server's `WorktreeDiff.summary()` and
@@ -98,30 +95,21 @@ export function generatedLike(file: string): boolean {
const BASE_CANDIDATES = ["main", "master", "dev", "develop"]
export async function resolveBase(git: GitOps, dir: string, base: string, signal?: AbortSignal): Promise<string> {
export async function resolveBase(git: GitOps, dir: string, base: string): Promise<string> {
// If the caller gave an explicit base, honor it. Return it as-is so merge-base
// fails loudly on a stale/misspelled ref instead of silently diffing against
// an unrelated candidate branch.
if (base && base !== "HEAD") return base
for (const name of BASE_CANDIDATES) {
const ok = await git.execGit(["rev-parse", "--verify", "--quiet", `refs/heads/${name}`], dir, { signal })
check(signal)
const ok = await git.execGit(["rev-parse", "--verify", "--quiet", `refs/heads/${name}`], dir)
if (ok.code === 0) return name
}
return "HEAD"
}
async function ancestor(
git: GitOps,
dir: string,
base: string,
log?: Log,
signal?: AbortSignal,
): Promise<string | undefined> {
const resolvedBase = await resolveBase(git, dir, base, signal)
check(signal)
const result = await git.execGit(["merge-base", "HEAD", resolvedBase], dir, { signal })
check(signal)
async function ancestor(git: GitOps, dir: string, base: string, log?: Log): Promise<string | undefined> {
const resolvedBase = await resolveBase(git, dir, base)
const result = await git.execGit(["merge-base", "HEAD", resolvedBase], dir)
if (result.code !== 0) {
log?.("git merge-base failed", { code: result.code, stderr: result.stderr.trim(), dir, base, resolvedBase })
return undefined
@@ -147,11 +135,10 @@ function counts(value: string) {
return result
}
async function numstat(git: GitOps, dir: string, base: string, file?: string, signal?: AbortSignal) {
async function numstat(git: GitOps, dir: string, base: string, file?: string) {
const args = ["-c", "core.quotepath=false", "diff", "--numstat", "--no-renames", base]
if (file) args.push("--", file)
const result = await git.execGit(args, dir, { signal })
check(signal)
const result = await git.execGit(args, dir)
return counts(result.code === 0 ? result.stdout : "")
}
@@ -293,82 +280,6 @@ export async function diffSummary(git: GitOps, dir: string, base: string, log?:
return items.map(summarize)
}
function complete(
result: Map<string, WorktreeDiffEntry | null>,
metas: Meta[],
before: Map<string, Buffer>,
diffs: Map<string, string>,
after: Map<string, string>,
) {
for (const meta of metas) {
const value = after.get(meta.file)
const patch = meta.tracked ? diffs.get(meta.file) : buildUntrackedPatch(meta.file, value ?? "")
if (value === undefined || patch === undefined || (meta.status !== "added" && !before.has(meta.file))) {
result.set(meta.file, null)
continue
}
result.set(meta.file, {
...summarize(meta),
before: before.get(meta.file)?.toString("utf8") ?? "",
after: value,
patch,
additions: meta.status === "added" && meta.additions === 0 && !meta.tracked ? linesOf(value) : meta.additions,
summarized: false,
})
}
return result
}
async function collect(
git: GitOps,
dir: string,
anc: string,
metas: Meta[],
log?: Log,
signal?: AbortSignal,
): Promise<DiffBatch<WorktreeDiffEntry>> {
const entries = new Map<string, WorktreeDiffEntry | null>()
const deferred = new Set<string>()
if (metas.length === 0) return { entries, deferred }
const [base, sizes] = await Promise.all([
inspect(git, dir, anc, metas, signal),
Promise.all(
metas.map(async (meta) => [meta.file, meta.status === "deleted" ? 0 : await fileSize(dir, meta.file)] as const),
),
])
check(signal)
const working = new Map(sizes)
const active: Meta[] = []
let total = 0
for (const meta of metas) {
const before = base.get(meta.file)?.bytes ?? 0
const after = working.get(meta.file) ?? 0
if (before > MAX_DETAIL_BYTES || after > MAX_DETAIL_BYTES) {
log?.("diffFile: file too large for detail view, returning summarized entry", {
file: meta.file,
beforeBytes: before,
afterBytes: after,
cap: MAX_DETAIL_BYTES,
})
entries.set(meta.file, summarize(meta))
continue
}
if (total + before + after > MAX_BATCH_BYTES) {
deferred.add(meta.file)
continue
}
total += before + after
active.push(meta)
}
const [before, diffs, after] = await Promise.all([
blobs(git, dir, active, base, signal),
patches(git, dir, anc, active, signal),
Promise.all(active.map(async (meta) => [meta.file, await readAfter(dir, meta.file, meta.status)] as const)),
])
check(signal)
return { entries: complete(entries, active, before, diffs, new Map(after)), deferred }
}
export function createLocalDiff(git: GitOps, log?: Log) {
const states = new Map<string, { anc: string; metas: Map<string, Meta> }>()
const generations = new Map<string, number>()
@@ -400,152 +311,6 @@ export function createLocalDiff(git: GitOps, log?: Log) {
}
}
const identity = (dir: string, base: string, anc: string, meta: Meta) =>
`${dir}\0${base}\0${anc}\0${meta.file}\0${meta.tracked}\0${meta.status}\0${meta.additions}\0${meta.deletions}\0${meta.binary}\0${meta.stamp}`
const track = (id: string, meta: Meta, work: Promise<WorktreeDiffEntry>, signal?: AbortSignal) => {
pending.set(id, { signal, work })
work.then(
(value) => {
if (pending.get(id)?.work !== work) return
pending.delete(id)
if (
signal?.aborted ||
value.image?.before?.error === "unreadable" ||
value.image?.after?.error === "unreadable"
) {
return
}
remember(id, value, meta.stamp)
},
() => {
if (pending.get(id)?.work === work) pending.delete(id)
},
)
return work
}
const file = async (
dir: string,
base: string,
path: string,
signal?: AbortSignal,
): Promise<WorktreeDiffEntry | null> => {
const state = states.get(`${dir}\0${base}`)
if (!state) return diffFile(git, dir, base, path, log, signal)
const meta = state.metas.get(path)
if (!meta) return null
const id = identity(dir, base, state.anc, meta)
const cached = details.get(id)
if (cached) {
if (cached.stamp === meta.stamp) {
remember(id, cached.value, meta.stamp)
return cached.value
}
forget(id)
}
const current = pending.get(id)
if (current && current.signal === signal && !current.signal?.aborted) return current.work
return track(id, meta, materialize(git, dir, state.anc, meta, log, signal), signal)
}
const files = async (
dir: string,
base: string,
paths: readonly string[],
signal?: AbortSignal,
): Promise<DiffBatch<WorktreeDiffEntry>> => {
const result = new Map<string, WorktreeDiffEntry | null>()
const deferred = new Set<string>()
const key = `${dir}\0${base}`
const state = states.get(key)
if (!state) {
check(signal)
await Promise.all(paths.map(async (path) => result.set(path, await file(dir, base, path, signal))))
check(signal)
return { entries: result, deferred }
}
const waiting: Promise<void>[] = []
const fresh: Array<{ id: string; meta: Meta }> = []
const add = (path: string, work: Promise<WorktreeDiffEntry | null>) => {
waiting.push(
work.then(
(value) => void result.set(path, value),
() => {
check(signal)
if (!deferred.has(path)) result.set(path, null)
},
),
)
}
for (const path of new Set(paths)) {
const meta = state.metas.get(path)
if (!meta) {
result.set(path, null)
continue
}
const id = identity(dir, base, state.anc, meta)
const cached = details.get(id)
if (cached?.stamp === meta.stamp) {
remember(id, cached.value, meta.stamp)
result.set(path, cached.value)
continue
}
const current = pending.get(id)
if (current && current.signal === signal && !current.signal?.aborted) {
add(path, current.work)
continue
}
if (meta.binary || imageMime(path) || /[\r\n\t"\\]/.test(path)) {
add(path, file(dir, base, path, signal))
continue
}
fresh.push({ id, meta })
}
if (fresh.length > 0) {
const batch = collect(
git,
dir,
state.anc,
fresh.map((item) => item.meta),
log,
signal,
)
.then((values) => {
const current = states.get(key)
if (
!current ||
current.anc !== state.anc ||
fresh.some((item) => {
const meta = current.metas.get(item.meta.file)
return !meta || identity(dir, base, current.anc, meta) !== item.id
})
) {
throw new Error("Diff summary changed")
}
for (const path of values.deferred) deferred.add(path)
return values
})
.catch((err): DiffBatch<WorktreeDiffEntry> => {
check(signal)
log?.("Bulk diff detail failed, falling back to single-file requests", err)
for (const item of fresh) deferred.add(item.meta.file)
return { entries: new Map(), deferred }
})
for (const item of fresh) {
const work = batch.then((values) => {
const value = values.entries.get(item.meta.file)
if (!value) throw new Error(`Could not load diff for ${item.meta.file}`)
return value
})
add(item.meta.file, track(item.id, item.meta, work, signal))
}
}
await Promise.all(waiting)
check(signal)
return { entries: result, deferred }
}
return {
summary: async (dir: string, base: string): Promise<WorktreeDiffEntry[]> => {
const id = `${dir}\0${base}`
@@ -564,25 +329,46 @@ export function createLocalDiff(git: GitOps, log?: Log) {
if (states.size > 8) states.delete(states.keys().next().value!)
return items.map(summarize)
},
file,
files,
file: async (dir: string, base: string, file: string, signal?: AbortSignal): Promise<WorktreeDiffEntry | null> => {
const state = states.get(`${dir}\0${base}`)
if (!state) return diffFile(git, dir, base, file, log)
const meta = state.metas.get(file)
if (!meta) return null
const id = `${dir}\0${base}\0${state.anc}\0${file}\0${meta.tracked}\0${meta.status}\0${meta.additions}\0${meta.deletions}\0${meta.binary}\0${meta.stamp}`
const cached = details.get(id)
if (cached) {
if (cached.stamp === meta.stamp) {
remember(id, cached.value, meta.stamp)
return cached.value
}
forget(id)
}
const current = pending.get(id)
if (current && !current.signal?.aborted) return current.work
const work = materialize(git, dir, state.anc, meta, log, signal)
pending.set(id, { signal, work })
work.then(
(value) => {
if (pending.get(id)?.work !== work) return
pending.delete(id)
if (value.image?.before?.error === "unreadable" || value.image?.after?.error === "unreadable") return
remember(id, value, meta.stamp)
},
() => {
if (pending.get(id)?.work === work) pending.delete(id)
},
)
return work
},
}
}
async function detailMeta(
git: GitOps,
dir: string,
anc: string,
file: string,
signal?: AbortSignal,
): Promise<Meta | undefined> {
async function detailMeta(git: GitOps, dir: string, anc: string, file: string): Promise<Meta | undefined> {
const full = resolveInside(dir, file)
if (!full) return undefined
const tracked = await git.execGit(["ls-files", "--error-unmatch", "--", file], dir, { signal })
check(signal)
const tracked = await git.execGit(["ls-files", "--error-unmatch", "--", file], dir)
if (tracked.code !== 0) {
const untracked = await git.execGit(["ls-files", "--others", "--exclude-standard", "--", file], dir, { signal })
check(signal)
const untracked = await git.execGit(["ls-files", "--others", "--exclude-standard", "--", file], dir)
if (untracked.code !== 0 || !untracked.stdout.split("\n").includes(file)) return undefined
const exists = await fs.lstat(full).catch(() => undefined)
if (!exists) return undefined
@@ -602,9 +388,7 @@ async function detailMeta(
const nameStatus = await git.execGit(
["-c", "core.quotepath=false", "diff", "--name-status", "--no-renames", anc, "--", file],
dir,
{ signal },
)
check(signal)
if (nameStatus.code !== 0) return undefined
const line = nameStatus.stdout.trim().split("\n")[0]
if (!line) return undefined
@@ -613,7 +397,7 @@ async function detailMeta(
const pathPart = parts.slice(1).join("\t") || file
if (!code) return undefined
const counts = await numstat(git, dir, anc, file, signal)
const counts = await numstat(git, dir, anc, file)
const stat = counts.get(file) ?? counts.get(pathPart) ?? { additions: 0, deletions: 0, binary: false }
const status = statusFromCode(code)
return {
@@ -722,15 +506,12 @@ export async function diffFile(
base: string,
file: string,
log?: Log,
signal?: AbortSignal,
): Promise<WorktreeDiffEntry | null> {
check(signal)
const anc = await ancestor(git, dir, base, log, signal)
const anc = await ancestor(git, dir, base, log)
if (!anc) return null
const meta = await detailMeta(git, dir, anc, file, signal)
check(signal)
const meta = await detailMeta(git, dir, anc, file)
if (!meta) return null
return materialize(git, dir, anc, meta, log, signal)
return materialize(git, dir, anc, meta, log)
}
async function materialize(
@@ -771,15 +771,6 @@ interface RequestWorktreeDiffFileIn {
diffSessionId?: string
}
interface RequestWorktreeDiffFilesIn {
type: "agentManager.requestWorktreeDiffFiles"
projectId?: string
sessionId: string
files: string[]
scope?: string
diffSessionId?: string
}
interface StartDiffWatchIn {
type: "agentManager.startDiffWatch"
projectId?: string
@@ -1131,7 +1122,6 @@ export type AgentManagerInMessage =
| ImportFromPRIn
| RequestWorktreeDiffIn
| RequestWorktreeDiffFileIn
| RequestWorktreeDiffFilesIn
| ApplyWorktreeDiffIn
| StartDiffWatchIn
| StopDiffWatchIn
@@ -197,16 +197,6 @@ export class WorktreeDiffController {
await this.controller.requestFile(file)
}
public async requestFiles(project: string | undefined, id: string, files: readonly string[]): Promise<void> {
if (this.controller.currentId !== id || this.owner !== project || this.ctx.projectId?.() !== project) {
for (const file of files) {
this.ctx.post({ type: "agentManager.worktreeDiffFile", projectId: project, sessionId: id, file, diff: null })
}
return
}
await this.controller.requestFiles(files)
}
/** Resolve the base-branch choices for a context and push them to the webview. */
public async postBranches(id: string): Promise<void> {
const result = await this.branches(id).catch((err) => {
@@ -1,7 +1,8 @@
import { hashFileDiffs } from "./shared/hash"
import { DIFF_POLL_INTERVAL_MS } from "./polling"
import type { DiffSource, DiffSourceCapabilities, DiffSourceDescriptor, DiffSourceNotice } from "./sources/types"
import type { DiffBatch, DiffFile, PanelContext } from "./types"
import type { DiffFile } from "./types"
import type { PanelContext } from "./types"
type Messages = {
available?: (descriptors: DiffSourceDescriptor[], id: string) => unknown
@@ -198,43 +199,6 @@ export class SourceController {
this.send(this.messages.diffFile(source, file, diff))
}
async requestFiles(files: readonly string[]): Promise<void> {
const values = [...new Set(files.filter(Boolean))]
if (values.length === 0) return
const source = this.active
const epoch = this.epoch
if (!source || (!source.fetchFiles && !source.fetchFile)) {
for (const file of values) this.send(this.messages.diffFile(source, file, null))
return
}
await new Promise<void>((resolve) => setTimeout(resolve, 0))
if (this.epoch !== epoch || this.active !== source) {
for (const file of values) this.send(this.messages.diffFile(source, file, null))
return
}
const result: DiffBatch<DiffFile> = source.fetchFiles
? await source.fetchFiles(values).catch(() => ({ entries: new Map(), deferred: new Set(values) }))
: {
entries: new Map(
await Promise.all(
values.map(async (file) => [file, await source.fetchFile!(file).catch(() => null)] as const),
),
),
deferred: new Set(),
}
for (const file of values) {
if (this.epoch !== epoch || this.active !== source) {
this.send(this.messages.diffFile(source, file, null))
continue
}
const diff = result.deferred.has(file)
? await source.fetchFile?.(file).catch(() => null)
: result.entries.get(file)
const stale = this.epoch !== epoch || this.active !== source
this.send(this.messages.diffFile(source, file, stale ? null : (diff ?? null)))
}
}
dispose(): void {
this.stop()
}
@@ -4,7 +4,7 @@ import { GitOps } from "../../agent-manager/GitOps"
import { resolveLocalDiffTarget } from "../shared/target"
import { appendOutput, getWorkspaceRoot } from "../../review-utils"
import type { BranchListItem } from "../../agent-manager/git-import"
import type { DiffBatch, PanelContext } from "../types"
import type { PanelContext } from "../types"
import type { DiffSource, DiffSourceDescriptor } from "./types"
import { createWorktreeDiffSource, WORKSPACE_DESCRIPTOR, WORKSPACE_SOURCE_ID } from "./worktree"
import {
@@ -23,12 +23,6 @@ import type { WorktreeDiffEntry } from "../../agent-manager/types"
export interface LocalDiffSource {
summary: (dir: string, base: string) => Promise<WorktreeDiffEntry[]>
file: (dir: string, base: string, file: string, signal?: AbortSignal) => Promise<WorktreeDiffEntry | null>
files: (
dir: string,
base: string,
files: readonly string[],
signal?: AbortSignal,
) => Promise<DiffBatch<WorktreeDiffEntry>>
}
export interface WorkspaceBranchesResult {
@@ -113,7 +107,6 @@ export class DiffSourceCatalog implements vscode.Disposable {
baseBranch: ctx.baseBranch,
summary: this.local?.summary,
file: this.local?.file,
files: this.local?.files,
})
}
@@ -1,4 +1,4 @@
import type { DiffBatch, DiffFile } from "../types"
import type { DiffFile } from "../types"
export interface DiffSourceCapabilities {
revert: boolean
@@ -58,7 +58,6 @@ export interface DiffSource {
* content on demand.
*/
fetchFile?(file: string): Promise<DiffFile | null>
fetchFiles?(files: readonly string[]): Promise<DiffBatch<DiffFile>>
revert?(file: string): Promise<{ ok: boolean; message: string }>
@@ -5,7 +5,7 @@ import type { WorktreeDiffEntry } from "../../agent-manager/types"
import { WorktreeDiffReverter, type DiffTarget, type StatusResolver } from "../shared/reverter"
import { resolveLocalDiffTarget } from "../shared/target"
import { appendOutput, getWorkspaceRoot } from "../../review-utils"
import type { DiffBatch, DiffFile } from "../types"
import type { DiffFile } from "../types"
import type { DiffSource, DiffSourceDescriptor, DiffSourceFetch } from "./types"
export const WORKSPACE_SOURCE_ID = "workspace"
@@ -47,12 +47,6 @@ export interface WorktreeDiffSourceOptions {
log?: (...args: unknown[]) => void
summary?: (dir: string, base: string) => Promise<WorktreeDiffEntry[]>
file?: (dir: string, base: string, file: string, signal?: AbortSignal) => Promise<WorktreeDiffEntry | null>
files?: (
dir: string,
base: string,
files: readonly string[],
signal?: AbortSignal,
) => Promise<DiffBatch<WorktreeDiffEntry>>
}
/**
@@ -116,8 +110,6 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}):
return entry?.status
}
const bulk = opts.files
return {
descriptor: WORKSPACE_DESCRIPTOR,
@@ -141,7 +133,7 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}):
try {
const entry = opts.file
? await opts.file(current.directory, current.baseBranch, file, controller.signal)
: await diffFile(git, current.directory, current.baseBranch, file, log, controller.signal)
: await diffFile(git, current.directory, current.baseBranch, file, log)
if (!entry) return null
return toDiffFile(entry)
} catch (err) {
@@ -150,27 +142,6 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}):
}
},
...(bulk
? {
async fetchFiles(files: readonly string[]): Promise<DiffBatch<DiffFile>> {
const result = new Map<string, DiffFile | null>()
const current = await resolveTarget()
if (!current) return { entries: result, deferred: new Set() }
const batch = await bulk(current.directory, current.baseBranch, files, controller.signal).catch((err) => {
log("Failed to fetch worktree diff files:", err)
return undefined
})
if (!batch) return { entries: result, deferred: new Set(files) }
for (const file of files) {
if (batch.deferred.has(file)) continue
const entry = batch.entries.get(file)
result.set(file, entry ? toDiffFile(entry) : null)
}
return { entries: result, deferred: batch.deferred }
},
}
: {}),
async revert(file: string): Promise<{ ok: boolean; message: string }> {
const current = await resolveTarget()
if (!current) return { ok: false, message: "Could not resolve diff target" }
-5
View File
@@ -51,11 +51,6 @@ export interface DiffImage {
after?: DiffImageSide
}
export interface DiffBatch<T> {
entries: ReadonlyMap<string, T | null>
deferred: ReadonlySet<string>
}
/** Mirrors `WorktreeFileDiff` in webview-ui/src/types/messages/agent-manager.ts. */
export interface DiffFile {
file: string
@@ -539,7 +539,6 @@ describe("Agent Manager Provider — onMessage routing", () => {
"agentManager.showExistingLocalTerminal",
"agentManager.requestRepoInfo",
"agentManager.requestState",
"agentManager.requestWorktreeDiffFiles",
"agentManager.setTabOrder",
"agentManager.setDefaultBaseBranch",
"agentManager.terminal.create",
@@ -13,12 +13,8 @@ const diff = (file: string, additions = 1): WorktreeFileDiff => ({
interface Sent {
type: string
projectId?: string
sessionId?: string
diffSessionId?: string
scope?: string
file?: string
files?: string[]
}
// Only `postMessage` is exercised by the diff workflow, so a recording stub is
@@ -26,13 +22,10 @@ interface Sent {
const vscode = (sent: Sent[]) =>
({ postMessage: (msg: Sent) => sent.push(msg) }) as unknown as Parameters<typeof createWorktreeDiffs>[0]
const withDiffs = (
fn: (diffs: ReturnType<typeof createWorktreeDiffs>, sent: Sent[]) => void,
project: () => string | undefined = () => undefined,
) => {
const withDiffs = (fn: (diffs: ReturnType<typeof createWorktreeDiffs>, sent: Sent[]) => void) => {
createRoot((dispose) => {
const sent: Sent[] = []
fn(createWorktreeDiffs(vscode(sent), project), sent)
fn(createWorktreeDiffs(vscode(sent)), sent)
dispose()
})
}
@@ -97,43 +90,6 @@ describe("createWorktreeDiffs", () => {
})
})
it("requestDiffFiles marks unique files pending and reuses singular completion messages", () => {
withDiffs((diffs, sent) => {
diffs.requestDiffFiles("s1#branch", ["a.ts", "b.ts", "a.ts"])
expect(sent).toHaveLength(1)
expect(sent[0]?.type).toBe("agentManager.requestWorktreeDiffFiles")
expect(sent[0]?.files).toEqual(["a.ts", "b.ts"])
expect(diffs.diffFileLoadingFor(() => "s1#branch")).toEqual(new Set(["a.ts", "b.ts"]))
diffs.requestDiffFiles("s1#branch", ["a.ts"])
expect(sent).toHaveLength(1)
diffs.onWorktreeDiffFile({
type: "agentManager.worktreeDiffFile",
sessionId: "s1#branch",
file: "a.ts",
diff: diff("a.ts"),
})
expect(diffs.diffFileLoadingFor(() => "s1#branch")).toEqual(new Set(["b.ts"]))
})
})
it("keeps bulk requests qualified by project and session scope", () => {
withDiffs(
(diffs, sent) => {
diffs.requestDiffFiles("wt-1#session:ses-1", ["a.ts", "b.ts"])
expect(sent[0]).toMatchObject({
type: "agentManager.requestWorktreeDiffFiles",
projectId: "project-1",
sessionId: "wt-1",
scope: "session",
diffSessionId: "ses-1",
files: ["a.ts", "b.ts"],
})
expect(diffs.diffFileLoadingFor(() => "wt-1#session:ses-1")).toEqual(new Set(["a.ts", "b.ts"]))
},
() => "project-1",
)
})
it("refreshStaleDiffs requests only files not already loading", () => {
withDiffs((diffs, sent) => {
diffs.requestDiffFile("s1", "a.ts")
@@ -101,26 +101,6 @@ const SCRIPT = `
fail("did not request after existing loading state cleared " + JSON.stringify(blocked))
}
disposeBlocked()
const batches = []
const singles = []
const many = Array.from({ length: 18 }, (_, index) => ({ ...summary, file: "file-" + index + ".ts" }))
const disposeMany = createRoot((dispose) => {
createDiffRequests({
key: () => "review-many",
diffs: () => many,
open: () => many.map((item) => item.file),
loading: () => undefined,
send: () => (file) => singles.push(file),
batch: () => (files) => batches.push(files),
})
return dispose
})
await new Promise((resolve) => setTimeout(resolve, 0))
if (batches.length !== 2 || batches[0].length !== 16 || batches[1].length !== 2 || singles.length !== 0) {
fail("initial summarized diffs were not bounded and batched " + JSON.stringify({ batches, singles }))
}
disposeMany()
console.log("${PASS}")
`
@@ -78,7 +78,6 @@ describe("DiffSourceCatalog.build", () => {
expect(src.descriptor.type).toBe("workspace")
expect(src.revert).toBeDefined()
expect(src.fetchFile).toBeDefined()
expect(src.fetchFiles).toBeUndefined()
src.dispose?.()
})
@@ -57,20 +57,6 @@ describe("GitOps", () => {
})
})
it("passes stdin to binary Git commands without decoding their output", async () => {
await withRepo(async (cwd) => {
const git = new GitOps({ log: () => undefined, binary: async () => "git" })
const value = "before\u0000after"
const object = await git.execGit(["hash-object", "-w", "--stdin"], cwd, { stdin: value })
const result = await git.execGitBuffer(["cat-file", "--batch"], cwd, {
stdin: `${object.stdout.trim()}\n`,
})
expect(result.code).toBe(0)
expect(result.stdout.includes(Buffer.from(value))).toBe(true)
git.dispose()
})
})
it("does not hold a semaphore slot while resolving Git", async () => {
const semaphore = new Semaphore(1)
let resolve!: (value: string) => void
@@ -681,21 +667,6 @@ describe("GitOps", () => {
})
})
it("stops waiting for executable discovery when its request signal aborts", async () => {
let release!: (value: string) => void
const gate = new Promise<string>((resolve) => {
release = resolve
})
const git = new GitOps({ log: () => undefined, binary: () => gate })
const ctl = new AbortController()
const pending = git.execGit(["status"], "/repo", { signal: ctl.signal })
ctl.abort()
const result = await pending
expect(result.code).not.toBe(0)
release("git")
git.dispose()
})
it("kills an in-flight exec when its request signal aborts", async () => {
await withRepo(async (cwd) => {
const git = new GitOps({ log: () => undefined, binary: async () => process.execPath })
@@ -352,284 +352,6 @@ describe("diffFile", () => {
})
})
it("batches tracked and untracked details while preserving complete file contents", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "second file.txt"), "old\n")
runSync(dir, ["add", "second file.txt"])
runSync(dir, ["commit", "-m", "add second file"])
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n")
await fs.writeFile(path.join(dir, "second file.txt"), "old\nsecond\n")
await fs.writeFile(path.join(dir, "new.txt"), "new\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = await local.files(dir, base, ["seed.txt", "second file.txt", "new.txt"])
expect(result.entries.get("seed.txt")?.before).toBe("seed\n")
expect(result.entries.get("seed.txt")?.after).toBe("seed\nfirst\n")
expect(result.entries.get("seed.txt")?.patch).toContain("+first")
expect(result.entries.get("second file.txt")?.before).toBe("")
expect(result.entries.get("second file.txt")?.after).toBe("old\nsecond\n")
expect(result.entries.get("second file.txt")?.patch).toContain("+second")
expect(result.entries.get("new.txt")?.before).toBe("")
expect(result.entries.get("new.txt")?.patch).toContain("+new")
for (const item of ["seed.txt", "second file.txt", "new.txt"]) {
expect(result.entries.get(item)).toEqual(await diffFile(git(), dir, base, item))
}
expect(await local.file(dir, base, "seed.txt")).toBe(result.entries.get("seed.txt"))
})
})
it("maps combined patches and base blobs for modified paths with spaces", async () => {
await withRepo(async (dir, base) => {
const name = "nested folder/second file.ts"
await fs.mkdir(path.join(dir, "nested folder"))
await fs.writeFile(path.join(dir, name), "before\n")
runSync(dir, ["add", name])
runSync(dir, ["commit", "-m", "add nested source"])
runSync(dir, ["branch", "-f", base])
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nupdated\n")
await fs.writeFile(path.join(dir, name), "before\nafter\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = await local.files(dir, base, ["seed.txt", name])
expect(result.entries.get(name)?.before).toBe("before\n")
expect(result.entries.get(name)?.after).toBe("before\nafter\n")
expect(result.entries.get(name)?.patch).toContain(`diff --git a/${name} b/${name}`)
expect(result.entries.get("seed.txt")?.patch).not.toContain(name)
})
})
it("preserves separate hunks and content that resembles a Git file header", async () => {
await withRepo(async (dir, base) => {
const rows = Array.from({ length: 40 }, (_, index) => `line-${index}`)
await fs.writeFile(path.join(dir, "seed.txt"), `${rows.join("\n")}\n`)
runSync(dir, ["add", "seed.txt"])
runSync(dir, ["commit", "-m", "add long source file"])
runSync(dir, ["branch", "-f", base])
rows[2] = "diff --git a/fake.ts b/fake.ts"
rows[35] = "updated final hunk"
await fs.writeFile(path.join(dir, "seed.txt"), `${rows.join("\n")}\n`)
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = (await local.files(dir, base, ["seed.txt"])).entries.get("seed.txt")
expect(result).toEqual(await diffFile(git(), dir, base, "seed.txt"))
expect(result?.patch.match(/^@@ /gm)).toHaveLength(2)
expect(result?.patch).toContain("+diff --git a/fake.ts b/fake.ts")
})
})
it("keeps deleted files, images, and non-image binaries on their existing detail paths", async () => {
await withRepo(async (dir, base) => {
const before = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x01])
const after = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0xff])
await fs.writeFile(path.join(dir, "remove.txt"), "remove\n")
await fs.writeFile(path.join(dir, "banner.png"), before)
runSync(dir, ["add", "remove.txt", "banner.png"])
runSync(dir, ["commit", "-m", "add mixed files"])
runSync(dir, ["branch", "-f", base])
await fs.unlink(path.join(dir, "remove.txt"))
await fs.writeFile(path.join(dir, "banner.png"), after)
await fs.writeFile(path.join(dir, "tone.wav"), Buffer.from([0x52, 0x49, 0x46, 0x46, 0x00]))
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = await local.files(dir, base, ["remove.txt", "banner.png", "tone.wav", "missing.txt"])
expect(result.entries.get("remove.txt")?.before).toBe("remove\n")
expect(result.entries.get("remove.txt")?.after).toBe("")
expect(result.entries.get("remove.txt")?.patch).toContain("deleted file mode")
expect(result.entries.get("banner.png")?.image?.before?.data).toBe(before.toString("base64"))
expect(result.entries.get("banner.png")?.image?.after?.data).toBe(after.toString("base64"))
expect(result.entries.get("tone.wav")?.patch).toBe("")
expect(result.entries.get("missing.txt")).toBeNull()
})
})
it("does not materialize an oversized file in a bulk detail request", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "large.txt"), "a".repeat(MAX_DETAIL_BYTES + 1))
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = await local.files(dir, base, ["large.txt"])
expect(result.entries.get("large.txt")?.summarized).toBe(true)
expect(result.entries.get("large.txt")?.before).toBe("")
expect(result.entries.get("large.txt")?.after).toBe("")
expect(result.entries.get("large.txt")?.patch).toBe("")
})
})
it("defers files that exceed the bounded aggregate batch budget", async () => {
await withRepo(async (dir, base) => {
const size = 17_000_000
await fs.writeFile(path.join(dir, "first.txt"), "a".repeat(size))
await fs.writeFile(path.join(dir, "second.txt"), "b".repeat(size))
const local = createLocalDiff(git())
await local.summary(dir, base)
const result = await local.files(dir, base, ["first.txt", "second.txt"])
expect(result.entries.get("first.txt")?.after.length).toBe(size)
expect(result.entries.has("second.txt")).toBe(false)
expect(result.deferred.has("second.txt")).toBe(true)
expect((await local.file(dir, base, "second.txt"))?.after.length).toBe(size)
})
})
it("shares pending bulk detail with a concurrent single-file request", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nshared\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const batch = local.files(dir, base, ["seed.txt"])
const single = local.file(dir, base, "seed.txt")
const [many, one] = await Promise.all([batch, single])
expect(many.entries.get("seed.txt")).toBe(one)
})
})
it("does not share cancellation ownership across unrelated detail requests", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nindependent\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const first = new AbortController()
const second = new AbortController()
const batch = local.files(dir, base, ["seed.txt"], first.signal)
const single = local.file(dir, base, "seed.txt", second.signal)
first.abort()
await expect(batch).rejects.toThrow()
expect((await single)?.after).toBe("seed\nindependent\n")
})
})
it("invalidates cached bulk detail after the working-copy stamp changes", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const first = await local.files(dir, base, ["seed.txt"])
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nsecond value\n")
await local.summary(dir, base)
const second = await local.files(dir, base, ["seed.txt"])
expect(second.entries.get("seed.txt")).not.toBe(first.entries.get("seed.txt"))
expect(second.entries.get("seed.txt")?.after).toBe("seed\nsecond value\n")
})
})
it("keeps in-flight bulk detail when an unchanged summary refreshes", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nunchanged\n")
const ops = git()
const local = createLocalDiff(ops)
await local.summary(dir, base)
let release!: () => void
let started!: () => void
const gate = new Promise<void>((resolve) => {
release = resolve
})
const ready = new Promise<void>((resolve) => {
started = resolve
})
const original = ops.execGit.bind(ops)
ops.execGit = async (...args: Parameters<GitOps["execGit"]>) => {
if (args[0][0] === "cat-file" && args[0][1] === "--batch-check") {
started()
await gate
}
return original(...args)
}
const pending = local.files(dir, base, ["seed.txt"])
await ready
await local.summary(dir, base)
release()
const result = await pending
expect(result.entries.get("seed.txt")?.after).toBe("seed\nunchanged\n")
expect(result.deferred.size).toBe(0)
})
})
it("defers every requested file when bulk Git inspection fails", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfallback\n")
const ops = git()
const local = createLocalDiff(ops)
await local.summary(dir, base)
const original = ops.execGit.bind(ops)
ops.execGit = async (...args: Parameters<GitOps["execGit"]>) => {
if (args[0][0] === "cat-file" && args[0][1] === "--batch-check") {
return { code: 1, stdout: "", stderr: "batch inspection failed" }
}
return original(...args)
}
const result = await local.files(dir, base, ["seed.txt"])
expect(result.entries.has("seed.txt")).toBe(false)
expect(result.deferred.has("seed.txt")).toBe(true)
expect((await local.file(dir, base, "seed.txt"))?.after).toBe("seed\nfallback\n")
})
})
it("discards bulk detail when a newer summary replaces its snapshot", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n")
const ops = git()
const local = createLocalDiff(ops)
await local.summary(dir, base)
let release!: () => void
let started!: () => void
const gate = new Promise<void>((resolve) => {
release = resolve
})
const ready = new Promise<void>((resolve) => {
started = resolve
})
const original = ops.execGit.bind(ops)
ops.execGit = async (...args: Parameters<GitOps["execGit"]>) => {
if (args[0][0] === "cat-file" && args[0][1] === "--batch-check") {
started()
await gate
}
return original(...args)
}
const pending = local.files(dir, base, ["seed.txt"])
await ready
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nnewer summary\n")
await local.summary(dir, base)
release()
const stale = await pending
expect(stale.entries.has("seed.txt")).toBe(false)
expect(stale.deferred.has("seed.txt")).toBe(true)
expect((await local.file(dir, base, "seed.txt"))?.after).toBe("seed\nnewer summary\n")
})
})
it("cancels fallback detail requests before a summary snapshot exists", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nuncached\n")
const local = createLocalDiff(git())
const ctl = new AbortController()
const pending = local.files(dir, base, ["seed.txt"], ctl.signal)
ctl.abort()
await expect(pending).rejects.toThrow()
})
})
it("does not cache a batch that is aborted before Git completes", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\nbatched\n")
const local = createLocalDiff(git())
await local.summary(dir, base)
const ctl = new AbortController()
const pending = local.files(dir, base, ["seed.txt"], ctl.signal)
ctl.abort()
await expect(pending).rejects.toThrow()
expect((await local.file(dir, base, "seed.txt"))?.after).toBe("seed\nbatched\n")
})
})
it("reuses cached detail while the summary stamp is unchanged", async () => {
await withRepo(async (dir, base) => {
await fs.writeFile(path.join(dir, "seed.txt"), "seed\ncached\n")
@@ -411,195 +411,6 @@ describe("SourceController.requestFile", () => {
})
})
describe("SourceController.requestFiles", () => {
it("loads unique files in one source request and emits existing detail messages", async () => {
const calls: string[][] = []
const source: DiffSource = {
descriptor: WORKSPACE_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFiles(files) {
calls.push([...files])
return {
entries: new Map(
files.map((file) => [file, { file, before: "old", after: "new", additions: 1, deletions: 1 }]),
),
deferred: new Set(),
}
},
}
const { controller, posted } = make({ workspace: source })
controller.setContext({ workspaceRoot: "/repo" })
await controller.activate("workspace")
posted.length = 0
await controller.requestFiles(["one.ts", "two.ts", "one.ts"])
expect(calls).toEqual([["one.ts", "two.ts"]])
const messages = byType(posted, "diffViewer.diffFile")
expect(messages.map((message) => message.file)).toEqual(["one.ts", "two.ts"])
expect(messages.every((message) => message.diff !== null)).toBe(true)
controller.stop()
})
it("loads explicitly deferred bulk entries without retrying failed files", async () => {
const calls: string[] = []
const source: DiffSource = {
descriptor: WORKSPACE_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFiles(files) {
return {
entries: new Map([
[files[0]!, { file: files[0]!, before: "old", after: "new", additions: 1, deletions: 1 }],
["failed.ts", null],
]),
deferred: new Set(files.slice(1).filter((file) => file !== "failed.ts")),
}
},
async fetchFile(file) {
calls.push(file)
return { file, before: "old", after: "new", additions: 1, deletions: 1 }
},
}
const { controller, posted } = make({ workspace: source })
controller.setContext({ workspaceRoot: "/repo" })
await controller.activate("workspace")
posted.length = 0
await controller.requestFiles(["one.ts", "two.ts", "three.ts", "failed.ts"])
expect(calls).toEqual(["two.ts", "three.ts"])
const messages = byType(posted, "diffViewer.diffFile")
expect(messages.map((message) => message.file)).toEqual(["one.ts", "two.ts", "three.ts", "failed.ts"])
expect(messages.slice(0, 3).every((message) => message.diff !== null)).toBe(true)
expect(messages[3]!.diff).toBeNull()
controller.stop()
})
it("falls back to singular details when a bulk source fails", async () => {
const calls: string[] = []
const source: DiffSource = {
descriptor: WORKSPACE_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFiles() {
throw new Error("batch failed")
},
async fetchFile(file) {
calls.push(file)
return { file, before: "old", after: "new", additions: 1, deletions: 1 }
},
}
const { controller, posted } = make({ workspace: source })
controller.setContext({ workspaceRoot: "/repo" })
await controller.activate("workspace")
posted.length = 0
await controller.requestFiles(["one.ts", "two.ts"])
expect(calls).toEqual(["one.ts", "two.ts"])
expect(byType(posted, "diffViewer.diffFile").every((message) => message.diff !== null)).toBe(true)
controller.stop()
})
it("uses singular loading for sources without a bulk implementation", async () => {
const calls: string[] = []
const source: DiffSource = {
descriptor: SESSION_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFile(file) {
calls.push(file)
return { file, before: "old", after: "new", additions: 1, deletions: 1 }
},
}
const { controller, posted } = make({ "session:s1": source })
controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" })
await controller.activate("session:s1")
posted.length = 0
await controller.requestFiles(["one.ts", "two.ts"])
expect(calls).toEqual(["one.ts", "two.ts"])
expect(byType(posted, "diffViewer.diffFile")).toHaveLength(2)
controller.stop()
})
it("does not start queued bulk work after switching sources", async () => {
const calls: string[][] = []
const workspace: DiffSource = {
descriptor: WORKSPACE_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFiles(files) {
calls.push([...files])
return { entries: new Map(), deferred: new Set() }
},
}
const session: DiffSource = {
descriptor: SESSION_DESC,
async fetch() {
return { diffs: [] }
},
}
const { controller, posted } = make({ workspace, "session:s1": session })
controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" })
await controller.activate("workspace")
posted.length = 0
const request = controller.requestFiles(["one.ts", "two.ts"])
await controller.activate("session:s1")
await request
expect(calls).toEqual([])
const messages = byType(posted, "diffViewer.diffFile")
expect(messages.map((message) => message.file)).toEqual(["one.ts", "two.ts"])
expect(messages.every((message) => message.diff === null)).toBe(true)
controller.stop()
})
it("completes every pending file with null when the source stops", async () => {
let release!: () => void
let started!: () => void
const gate = new Promise<void>((resolve) => {
release = resolve
})
const ready = new Promise<void>((resolve) => {
started = resolve
})
const source: DiffSource = {
descriptor: WORKSPACE_DESC,
async fetch() {
return { diffs: [] }
},
async fetchFiles(files) {
started()
await gate
return {
entries: new Map(
files.map((file) => [file, { file, before: "old", after: "new", additions: 1, deletions: 1 }]),
),
deferred: new Set(),
}
},
}
const { controller, posted } = make({ workspace: source })
controller.setContext({ workspaceRoot: "/repo" })
await controller.activate("workspace")
posted.length = 0
const request = controller.requestFiles(["one.ts", "two.ts"])
await ready
controller.stop()
release()
await request
const messages = byType(posted, "diffViewer.diffFile")
expect(messages.map((message) => message.file)).toEqual(["one.ts", "two.ts"])
expect(messages.every((message) => message.diff === null)).toBe(true)
})
})
describe("SourceController.reactivate", () => {
it("rebuilds the active source via the build factory and refetches", async () => {
let builds = 0
@@ -10,9 +10,8 @@ import type { WorktreeStateManager } from "../../src/agent-manager/WorktreeState
// base branch the active source was (re)built with. The controller, scope
// resolution, and SourceController lifecycle under test are all real.
// Contexts are worktree ids (the sidebar selection), not session ids.
function make(onFetch?: (n: number) => Promise<void>, project?: () => string | undefined) {
function make(onFetch?: (n: number) => Promise<void>) {
const builds: { id: string; ctx: PanelContext }[] = []
const posted: unknown[] = []
let fetches = 0
const catalog = {
build: (id: string, ctx: PanelContext): DiffSource => {
@@ -40,11 +39,10 @@ function make(onFetch?: (n: number) => Promise<void>, project?: () => string | u
catalog,
git: {} as GitOps,
localDiffFile: async () => null,
post: (message) => posted.push(message),
post: () => {},
log: () => {},
projectId: project,
})
return { controller, builds, posted }
return { controller, builds }
}
const tick = () => new Promise((resolve) => setTimeout(resolve, 0))
@@ -57,77 +55,6 @@ async function waitFor(cond: () => boolean): Promise<void> {
throw new Error("waitFor timed out")
}
describe("WorktreeDiffController.requestFiles", () => {
it("completes every file when the requested composite scope is inactive", async () => {
const { controller, posted } = make()
await controller.requestFiles(undefined, "w1#branch", ["one.ts", "two.ts"])
expect(posted).toEqual([
{
type: "agentManager.worktreeDiffFile",
projectId: undefined,
sessionId: "w1#branch",
file: "one.ts",
diff: null,
},
{
type: "agentManager.worktreeDiffFile",
projectId: undefined,
sessionId: "w1#branch",
file: "two.ts",
diff: null,
},
])
controller.stop()
})
it("rejects a colliding composite scope owned by another project", async () => {
const { controller, builds, posted } = make(undefined, () => "project-a")
controller.start("w1#branch")
await waitFor(() => builds.length === 1)
posted.length = 0
await controller.requestFiles("project-b", "w1#branch", ["one.ts", "two.ts"])
expect(posted).toEqual([
{
type: "agentManager.worktreeDiffFile",
projectId: "project-b",
sessionId: "w1#branch",
file: "one.ts",
diff: null,
},
{
type: "agentManager.worktreeDiffFile",
projectId: "project-b",
sessionId: "w1#branch",
file: "two.ts",
diff: null,
},
])
controller.stop()
})
it("rejects requests when the active project changes after source activation", async () => {
let project = "project-a"
const { controller, builds, posted } = make(undefined, () => project)
controller.start("w1#branch")
await waitFor(() => builds.length === 1)
posted.length = 0
project = "project-b"
await controller.requestFiles("project-a", "w1#branch", ["one.ts"])
expect(posted).toEqual([
{
type: "agentManager.worktreeDiffFile",
projectId: "project-a",
sessionId: "w1#branch",
file: "one.ts",
diff: null,
},
])
controller.stop()
})
})
describe("WorktreeDiffController.setBase", () => {
it("rebuilds the active source against the overridden base branch", async () => {
const { controller, builds } = make()
@@ -586,6 +586,8 @@ const AgentManagerContent: Component = () => {
if (reviewActive()) closeReviewTab()
const opening = sidePanel() !== SidePanel.PR
setSidePanel((prev) => (prev === SidePanel.PR ? null : SidePanel.PR))
// Trigger an immediate refresh when opening so the panel shows fresh data
// rather than waiting for the next poll cycle
if (opening) {
const sel = selection()
if (sel && sel !== LOCAL)
@@ -1722,6 +1724,9 @@ const AgentManagerContent: Component = () => {
openReviewTab()
}
// Deferred close: flip signal immediately for instant UI feedback,
// the <Show> unmount triggers heavy FileDiff cleanup but the tab bar
// and chat view are already visible before that work runs.
const closeReviewTab = () => {
freezeTabs()
setReviewActive(false)
@@ -1744,10 +1749,9 @@ const AgentManagerContent: Component = () => {
return diffNotices()[diffDataKey(activeProjectId(), key)]
})
const requestDiffFile = (file: string | string[]) => {
const requestDiffFile = (file: string) => {
const id = diffScopeId()
if (!id) return
if (Array.isArray(file)) return diffs.requestDiffFiles(id, file)
diffs.requestDiffFile(id, file)
}
@@ -2652,7 +2656,6 @@ const AgentManagerContent: Component = () => {
: undefined
}
onRequestDiff={diffs.requestDiffFile}
onRequestDiffs={diffs.requestDiffFiles}
onOpenFile={(ctx, file, line) =>
vscode.postMessage({ type: "agentManager.openFile", sessionId: ctx, filePath: file, line })
}
@@ -2740,7 +2743,6 @@ const AgentManagerContent: Component = () => {
<FullScreenDiffView
diffs={reviewDiffs()}
loading={diffLoadingForCurrent()}
active={reviewActive()}
loadingFiles={diffFileLoadingForCurrent()}
sessionId={activeDiffSession()}
sessionKey={diffSessionKey()}
@@ -2758,7 +2760,6 @@ const AgentManagerContent: Component = () => {
markdownRender={markdown.render()}
onMarkdownRenderChange={markdown.update}
onRequestDiff={requestDiffFile}
onRequestDiffs={requestDiffFile}
onOpenFile={(file, line) => {
const id = diffCtx()
if (id) vscode.postMessage({ type: "agentManager.openFile", sessionId: id, filePath: file, line })
@@ -100,7 +100,6 @@ interface DiffPanelProps {
onClose: () => void
onExpand?: () => void
onRequestDiff?: (file: string) => void
onRequestDiffs?: (files: string[]) => void
onOpenFile?: (relativePath: string, line?: number) => void
onOpenDocument?: (relativePath: string) => void
onRevertFile?: (file: string) => void
@@ -291,7 +290,6 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
open,
loading: () => props.loadingFiles,
send: () => (props.active === false ? undefined : props.onRequestDiff),
batch: () => (props.active === false ? undefined : props.onRequestDiffs),
})
// --- CRUD ---
@@ -38,7 +38,6 @@ interface Props {
onClose: () => void
onExpand?: () => void
onRequestDiff: (key: string, file: string) => void
onRequestDiffs: (key: string, files: string[]) => void
onOpenFile: (ctx: string, file: string, line?: number) => void
onOpenDocument: (file: string) => void
onRevertFile: (key: string, ctx: string, file: string) => void
@@ -116,7 +115,6 @@ export const DiffPanelCache: Component<Props> = (props) => {
onClose={props.onClose}
onExpand={props.onExpand}
onRequestDiff={(file) => props.onRequestDiff(entry.key, file)}
onRequestDiffs={(files) => props.onRequestDiffs(entry.key, files)}
onOpenFile={(file, line) => props.onOpenFile(entry.ctx, file, line)}
onOpenDocument={props.onOpenDocument}
onRevertFile={(file) => props.onRevertFile(entry.key, entry.ctx, file)}
@@ -100,20 +100,6 @@ export function createWorktreeDiffs(
vscode.postMessage({ type: "agentManager.requestWorktreeDiffFile", projectId: project(), file, ...wireDiffId(id) })
}
const requestDiffFiles = (id: string, files: string[]) => {
const data = key(id)
const pending = diffFileLoading()[data] ?? {}
const next = [...new Set(files)].filter((file) => !pending[file])
if (next.length === 0) return
for (const file of next) setDiffFilePending(data, file, true)
vscode.postMessage({
type: "agentManager.requestWorktreeDiffFiles",
projectId: project(),
files: next,
...wireDiffId(id),
})
}
/** Files the backend flagged as stale in a merged update need a fresh fetch. */
const refreshStaleDiffs = (id: string, files: Set<string>, data = key(id), owner = project()) => {
const loading = diffFileLoading()[data] ?? {}
@@ -200,7 +186,6 @@ export function createWorktreeDiffs(
setDiffLoading: (loading: boolean) => setDiffLoadings(loading ? diffLoadings() : {}),
diffNotices,
requestDiffFile,
requestDiffFiles,
refreshStaleDiffs,
diffFileLoadingFor,
diffLoadingFor,
@@ -75,7 +75,6 @@ const DIFF_NOTICE_KEYS: Record<string, string> = {
interface FullScreenDiffViewProps {
diffs: WorktreeFileDiff[]
loading: boolean
active?: boolean
loadingFiles?: Set<string>
sessionId?: string
sessionKey?: string
@@ -91,7 +90,6 @@ interface FullScreenDiffViewProps {
markdownRender?: boolean
onMarkdownRenderChange?: (render: boolean) => void
onRequestDiff?: (file: string) => void
onRequestDiffs?: (files: string[]) => void
onOpenFile?: (relativePath: string, line?: number) => void
initialFile?: string
onRevertFile?: (file: string) => void
@@ -290,8 +288,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
diffs: () => props.diffs,
open,
loading: () => props.loadingFiles,
send: () => (props.active === false ? undefined : props.onRequestDiff),
batch: () => (props.active === false ? undefined : props.onRequestDiffs),
send: () => props.onRequestDiff,
})
// --- CRUD ---
@@ -9,11 +9,8 @@ interface DiffRequestOptions {
open: Accessor<string[]>
loading: Accessor<Set<string> | undefined>
send: Accessor<((file: string) => void) | undefined>
batch?: Accessor<((files: string[]) => void) | undefined>
}
const LIMIT = 16
export function createDiffRequests(opts: DiffRequestOptions) {
const requested = new Map<string, string>()
let active = false
@@ -28,23 +25,20 @@ export function createDiffRequests(opts: DiffRequestOptions) {
),
)
const eligible = (diff: WorktreeFileDiff) => {
if (opts.loading()?.has(diff.file)) return false
if (!isDiffExpandable(diff) || diff.summarized !== true) return false
return requested.get(diff.file) !== diffToken(diff)
}
const request = (diff: WorktreeFileDiff) => {
const send = opts.send()
if (!send || !eligible(diff)) return
requested.set(diff.file, diffToken(diff))
if (!send || opts.loading()?.has(diff.file)) return
if (!isDiffExpandable(diff) || diff.summarized !== true) return
const value = diffToken(diff)
if (requested.get(diff.file) === value) return
requested.set(diff.file, value)
send(diff.file)
}
createEffect(
on(
() => [opts.open(), opts.diffs(), opts.loading(), opts.send(), opts.batch?.()] as const,
([open, diffs, , , batch]) => {
() => [opts.open(), opts.diffs(), opts.loading(), opts.send()] as const,
([open, diffs]) => {
if (!opts.send()) {
requested.clear()
active = false
@@ -58,17 +52,10 @@ export function createDiffRequests(opts: DiffRequestOptions) {
for (const file of requested.keys()) {
if (!files.has(file)) requested.delete(file)
}
const next = open
.map((file) => diffs.find((item) => item.file === file))
.filter((diff): diff is WorktreeFileDiff => !!diff && diff.kind !== "image" && eligible(diff))
if (!batch || next.length < 2) {
for (const diff of next) request(diff)
return
}
for (let index = 0; index < next.length; index += LIMIT) {
const chunk = next.slice(index, index + LIMIT)
for (const diff of chunk) requested.set(diff.file, diffToken(diff))
batch(chunk.map((diff) => diff.file))
for (const file of open) {
const diff = diffs.find((item) => item.file === file)
if (!diff || diff.kind === "image") continue
request(diff)
}
},
),
@@ -1049,16 +1049,6 @@ export interface RequestWorktreeDiffFileMessage {
sessionId: string
file: string
scope?: string
diffSessionId?: string
}
export interface RequestWorktreeDiffFilesMessage {
type: "agentManager.requestWorktreeDiffFiles"
projectId?: string
sessionId: string
files: string[]
scope?: string
diffSessionId?: string
}
// Agent Manager: Start polling for live diff updates (webview → extension)
@@ -1067,7 +1057,6 @@ export interface StartDiffWatchMessage {
projectId?: string
sessionId: string
scope?: string
diffSessionId?: string
}
// Agent Manager: Stop polling for diff updates (webview → extension)
@@ -1662,7 +1651,6 @@ export type WebviewMessage =
| ImportFromPRRequest
| RequestWorktreeDiffMessage
| RequestWorktreeDiffFileMessage
| RequestWorktreeDiffFilesMessage
| StartDiffWatchMessage
| StopDiffWatchMessage
| RequestDiffBranchesMessage