From 41995929cd9d0bb33726dca4875cc16cef162630 Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 25 Feb 2026 19:07:42 +0100 Subject: [PATCH] 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 --- .../src/agent-manager/AgentManagerProvider.ts | 33 +++++++++++++++++-- .../src/server/routes/experimental.ts | 26 +++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index 598c0584d43..779ad492cd6 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -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 { + // 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) diff --git a/packages/opencode/src/server/routes/experimental.ts b/packages/opencode/src/server/routes/experimental.ts index cb4a0a3d7ec..6560ff62ea0 100644 --- a/packages/opencode/src/server/routes/experimental.ts +++ b/packages/opencode/src/server/routes/experimental.ts @@ -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) }, )