fix: resolve diff viewer race condition and add diagnostics for Agent Manager (#6352)

* fix: resolve diff viewer race condition and add diagnostics for Agent Manager

onRequestWorktreeDiff now awaits stateReady before resolving the diff
target, preventing a race where startDiffWatch fires before state loads
from disk. Added diagnostic logging to resolveDiffTarget, diff polling,
and the server-side /experimental/worktree/diff endpoint so silent
failures become visible in the output channel.

* fix: guard stateReady await so rejection doesn't break diff polling

* docs: clarify why .catch() is required on stateReady await
This commit is contained in:
Marius
2026-02-25 18:07:42 +00:00
committed by GitHub
parent ea40081a56
commit 41995929cd
2 changed files with 54 additions and 5 deletions
@@ -1274,23 +1274,49 @@ export class AgentManagerProvider implements vscode.Disposable {
/** Resolve worktree path + parentBranch for a session, or undefined if not applicable. */
private resolveDiffTarget(sessionId: string): { directory: string; baseBranch: string } | undefined {
const state = this.getStateManager()
if (!state) return undefined
if (!state) {
this.log(`resolveDiffTarget: no state manager for session ${sessionId}`)
return undefined
}
const session = state.getSession(sessionId)
if (!session?.worktreeId) return undefined
if (!session) {
this.log(
`resolveDiffTarget: session ${sessionId} not found in state (${state.getSessions().length} total sessions)`,
)
return undefined
}
if (!session.worktreeId) {
this.log(`resolveDiffTarget: session ${sessionId} has no worktreeId (local session)`)
return undefined
}
const worktree = state.getWorktree(session.worktreeId)
if (!worktree) return undefined
if (!worktree) {
this.log(`resolveDiffTarget: worktree ${session.worktreeId} not found for session ${sessionId}`)
return undefined
}
return { directory: worktree.path, baseBranch: worktree.parentBranch }
}
/** One-shot diff fetch with loading indicators. Used by requestWorktreeDiff. */
private async onRequestWorktreeDiff(sessionId: string): Promise<void> {
// Ensure state is loaded before resolving diff target — avoids race where
// startDiffWatch arrives before initializeState() finishes loading state from disk.
// The .catch() is required: this method is called via `void` (fire-and-forget),
// so an uncaught rejection would become an unhandled promise rejection. On failure
// we log and fall through to resolveDiffTarget which logs the specific reason.
if (this.stateReady) {
await this.stateReady.catch((err) => this.log("stateReady rejected, continuing diff resolve:", err))
}
const target = this.resolveDiffTarget(sessionId)
if (!target) return
this.postToWebview({ type: "agentManager.worktreeDiffLoading", sessionId, loading: true })
try {
const client = this.connectionService.getHttpClient()
this.log(`Fetching worktree diff for session ${sessionId}: dir=${target.directory}, base=${target.baseBranch}`)
const diffs = await client.getWorktreeDiff(target.directory, target.baseBranch)
this.log(`Worktree diff returned ${diffs.length} file(s) for session ${sessionId}`)
const hash = diffs.map((d) => `${d.file}:${d.status}:${d.additions}:${d.deletions}:${d.after.length}`).join("|")
this.lastDiffHash = hash
@@ -1328,6 +1354,7 @@ export class AgentManagerProvider implements vscode.Disposable {
this.stopDiffPolling()
this.diffSessionId = sessionId
this.lastDiffHash = undefined
this.log(`Starting diff polling for session ${sessionId}`)
// Initial fetch with loading state
void this.onRequestWorktreeDiff(sessionId)
@@ -13,6 +13,7 @@ import { $ } from "bun" // kilocode_change
import path from "path" // kilocode_change
import { Snapshot } from "../../snapshot" // kilocode_change
import { Review } from "../../kilocode/review/review" // kilocode_change
import { Log } from "../../util/log" // kilocode_change
export const ExperimentalRoutes = lazy(() =>
new Hono()
@@ -208,12 +209,23 @@ export const ExperimentalRoutes = lazy(() =>
},
}),
async (c) => {
const log = Log.create({ service: "worktree-diff" })
const base = c.req.query("base") || (await Review.getBaseBranch())
const dir = Instance.directory
log.info("computing diff", { dir, base })
const mergeBaseResult = await $`git merge-base HEAD ${base}`.cwd(dir).quiet().nothrow()
if (mergeBaseResult.exitCode !== 0) return c.json([])
if (mergeBaseResult.exitCode !== 0) {
log.warn("git merge-base failed", {
exitCode: mergeBaseResult.exitCode,
stderr: mergeBaseResult.stderr.toString().trim(),
dir,
base,
})
return c.json([])
}
const ancestor = mergeBaseResult.stdout.toString().trim()
log.info("merge-base resolved", { ancestor: ancestor.slice(0, 12) })
const nameStatus = await $`git -c core.quotepath=false diff --name-status --no-renames ${ancestor}`
.cwd(dir)
@@ -285,7 +297,11 @@ export const ExperimentalRoutes = lazy(() =>
// viewer shows all working-tree changes, not just tracked ones.
const untrackedResult = await $`git ls-files --others --exclude-standard`.cwd(dir).quiet().nothrow()
if (untrackedResult.exitCode === 0) {
for (const file of untrackedResult.stdout.toString().trim().split("\n")) {
const untrackedFiles = untrackedResult.stdout.toString().trim()
if (untrackedFiles) {
log.info("untracked files found", { count: untrackedFiles.split("\n").length })
}
for (const file of untrackedFiles.split("\n")) {
if (!file || seen.has(file)) continue
const f = Bun.file(path.join(dir, file))
if (!(await f.exists())) continue
@@ -300,8 +316,14 @@ export const ExperimentalRoutes = lazy(() =>
status: "added",
})
}
} else {
log.warn("git ls-files failed", {
exitCode: untrackedResult.exitCode,
stderr: untrackedResult.stderr.toString().trim(),
})
}
log.info("diff complete", { totalFiles: diffs.length })
return c.json(diffs)
},
)