From f9757083484a9cf7fd79524417b38818198aae02 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 20 Apr 2026 11:52:51 +0300 Subject: [PATCH 1/3] fix(vscode): restore review diff in repos with no remote --- .../kilo-vscode/src/agent-manager/GitOps.ts | 2 +- .../src/agent-manager/local-diff.ts | 19 ++++++++++-- packages/kilo-vscode/src/review-utils.ts | 2 +- .../tests/unit/git-stats-poller.test.ts | 29 +++++++++++++++++++ .../kilo-vscode/tests/unit/local-diff.test.ts | 22 ++++++++++++++ 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/GitOps.ts b/packages/kilo-vscode/src/agent-manager/GitOps.ts index f835cf3832c..0f0ff9a2048 100644 --- a/packages/kilo-vscode/src/agent-manager/GitOps.ts +++ b/packages/kilo-vscode/src/agent-manager/GitOps.ts @@ -205,7 +205,7 @@ export class GitOps { const remote = await this.resolveRemote(cwd, branch) const cacheKey = `default-branch:${cwd}:${remote}` const cached = this.getCached(cacheKey) - if (cached !== undefined) return cached + if (cached !== undefined) return cached === "" ? undefined : cached const head = await this.raw(["symbolic-ref", "--short", `refs/remotes/${remote}/HEAD`], cwd).catch(() => "") const result = head || undefined diff --git a/packages/kilo-vscode/src/agent-manager/local-diff.ts b/packages/kilo-vscode/src/agent-manager/local-diff.ts index 977b01de13d..92caacbc54f 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff.ts @@ -90,10 +90,25 @@ export function generatedLike(file: string): boolean { return false } +const BASE_CANDIDATES = ["main", "master", "dev", "develop"] + +async function resolveBase(git: GitOps, dir: string, base: string): Promise { + if (base && base !== "HEAD") { + const ok = await git.execGit(["rev-parse", "--verify", "--quiet", base], dir) + if (ok.code === 0) return base + } + for (const name of BASE_CANDIDATES) { + 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): Promise { - const result = await git.execGit(["merge-base", "HEAD", base], dir) + 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 }) + log?.("git merge-base failed", { code: result.code, stderr: result.stderr.trim(), dir, base, resolvedBase }) return undefined } return result.stdout.trim() diff --git a/packages/kilo-vscode/src/review-utils.ts b/packages/kilo-vscode/src/review-utils.ts index e5c56a3b287..4c5a18d03c2 100644 --- a/packages/kilo-vscode/src/review-utils.ts +++ b/packages/kilo-vscode/src/review-utils.ts @@ -35,7 +35,7 @@ export async function resolveLocalDiffTarget( const tracking = await gitOps.resolveTrackingBranch(root, branch) const fallback = tracking ? undefined : await gitOps.resolveDefaultBranch(root, branch) - const base = tracking ?? fallback ?? "HEAD" + const base = tracking || fallback || "HEAD" log(`Local diff: branch=${branch} tracking=${tracking ?? "none"} default=${fallback ?? "none"} base=${base}`) diff --git a/packages/kilo-vscode/tests/unit/git-stats-poller.test.ts b/packages/kilo-vscode/tests/unit/git-stats-poller.test.ts index 8b1844f7641..0afb8b9254d 100644 --- a/packages/kilo-vscode/tests/unit/git-stats-poller.test.ts +++ b/packages/kilo-vscode/tests/unit/git-stats-poller.test.ts @@ -53,6 +53,35 @@ function gitOps(handler: (args: string[], cwd: string) => Promise): GitO return new GitOps({ log: () => undefined, runGit: handler }) } +describe("GitOps", () => { + it("resolveDefaultBranch returns undefined on cache hit when there is no remote HEAD", async () => { + let calls = 0 + const git = new GitOps({ + log: () => undefined, + runGit: async (args) => { + calls++ + if (args[0] === "symbolic-ref") throw new Error("no remote HEAD") + if (args[0] === "branch" && args[1] === "--show-current") return "main" + if (args[0] === "config" && args[1] === "branch.main.remote") return "origin" + if (args[0] === "rev-parse" && args[3] === "@{upstream}") throw new Error("no upstream") + return "" + }, + }) + + // First call sets cache + const first = await git.resolveDefaultBranch("/test") + expect(first).toBeUndefined() + expect(calls).toBeGreaterThan(0) + + // Second call reads from cache + const beforeSecondCall = calls + const second = await git.resolveDefaultBranch("/test") + expect(second).toBeUndefined() + // Should not have made any new git calls for the exact same resolution + expect(calls).toBe(beforeSecondCall) + }) +}) + describe("GitStatsPoller", () => { it("does not overlap polling runs", async () => { let running = 0 diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index 0c95da09150..5df08a7755c 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -91,6 +91,28 @@ describe("generatedLike", () => { }) describe("diffSummary", () => { + it("uses candidate local branch fallback when base is empty string and on a feature branch", async () => { + await withRepo(async (dir) => { + runSync(dir, ["checkout", "-b", "feature"]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfeature\n") + runSync(dir, ["commit", "-am", "feature commit"]) + const result = await diffSummary(git(), dir, "") + const entry = result.find((e) => e.file === "seed.txt") + expect(entry?.status).toBe("modified") + }) + }) + + it("uses HEAD as fallback when no candidate branches exist and base is empty", async () => { + await withRepo(async (dir) => { + // Rename main to something else so it doesn't match candidates + runSync(dir, ["branch", "-m", "main", "other"]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nuncommitted\n") + const result = await diffSummary(git(), dir, "") + const entry = result.find((e) => e.file === "seed.txt") + expect(entry?.status).toBe("modified") + }) + }) + it("returns empty array when ancestor cannot be resolved", async () => { await withRepo(async (dir) => { const result = await diffSummary(git(), dir, "nonexistent-branch") From 12e35d9a742a93b5e9b73a32066cc312c7adbad5 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 20 Apr 2026 12:07:12 +0300 Subject: [PATCH 2/3] fix(vscode): honor explicit stale base in resolveBase --- packages/kilo-vscode/src/agent-manager/local-diff.ts | 8 ++++---- packages/kilo-vscode/tests/unit/local-diff.test.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/local-diff.ts b/packages/kilo-vscode/src/agent-manager/local-diff.ts index 92caacbc54f..091a9b7d60a 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff.ts @@ -93,10 +93,10 @@ export function generatedLike(file: string): boolean { const BASE_CANDIDATES = ["main", "master", "dev", "develop"] async function resolveBase(git: GitOps, dir: string, base: string): Promise { - if (base && base !== "HEAD") { - const ok = await git.execGit(["rev-parse", "--verify", "--quiet", base], dir) - if (ok.code === 0) return base - } + // 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) if (ok.code === 0) return name diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index 5df08a7755c..ed65c193370 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -120,6 +120,18 @@ describe("diffSummary", () => { }) }) + it("does not silently fall back to a candidate when an explicit base is stale", async () => { + await withRepo(async (dir) => { + // main exists locally, but caller provided a misspelled explicit base. + // We must NOT diff against main — merge-base should fail loudly. + runSync(dir, ["checkout", "-b", "feature"]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfeature\n") + runSync(dir, ["commit", "-am", "feature commit"]) + const result = await diffSummary(git(), dir, "typo-main") + expect(result).toEqual([]) + }) + }) + it("reports modified, added, and deleted tracked files", async () => { await withRepo(async (dir, base) => { // seed.txt is tracked on base. Modify it; add new.txt; delete seed.txt on HEAD. From edb1b9a5e494dc1da4bcb89b960e1a2e0df5a666 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 20 Apr 2026 14:18:11 +0300 Subject: [PATCH 3/3] fix(vscode): restore review revert-file action in repos with no remote Clicking 'revert file' in the Agent Manager review panel silently did nothing when the repo had no remote and no tracking branch. The diff view worked because it defensively searched for a local candidate branch, but the revert path ran merge-base against the placeholder HEAD and turned into a no-op for committed feature changes. --- .../src/agent-manager/local-diff.ts | 2 +- packages/kilo-vscode/src/review-utils.ts | 4 ++- .../kilo-vscode/tests/unit/local-diff.test.ts | 29 ++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/local-diff.ts b/packages/kilo-vscode/src/agent-manager/local-diff.ts index 091a9b7d60a..c58189dace2 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff.ts @@ -92,7 +92,7 @@ export function generatedLike(file: string): boolean { const BASE_CANDIDATES = ["main", "master", "dev", "develop"] -async function resolveBase(git: GitOps, dir: string, base: string): Promise { +export async function resolveBase(git: GitOps, dir: string, base: string): Promise { // 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. diff --git a/packages/kilo-vscode/src/review-utils.ts b/packages/kilo-vscode/src/review-utils.ts index 4c5a18d03c2..1e2a7c7927a 100644 --- a/packages/kilo-vscode/src/review-utils.ts +++ b/packages/kilo-vscode/src/review-utils.ts @@ -3,6 +3,7 @@ import * as vscode from "vscode" import { inspect } from "util" import type { SnapshotFileDiff } from "@kilocode/sdk/v2/client" import { GitOps } from "./agent-manager/GitOps" +import { resolveBase } from "./agent-manager/local-diff" export function appendOutput(channel: vscode.OutputChannel, prefix: string, ...args: unknown[]): void { const msg = args @@ -35,7 +36,8 @@ export async function resolveLocalDiffTarget( const tracking = await gitOps.resolveTrackingBranch(root, branch) const fallback = tracking ? undefined : await gitOps.resolveDefaultBranch(root, branch) - const base = tracking || fallback || "HEAD" + const raw = tracking || fallback || "HEAD" + const base = await resolveBase(gitOps, root, raw) log(`Local diff: branch=${branch} tracking=${tracking ?? "none"} default=${fallback ?? "none"} base=${base}`) diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index ed65c193370..fdcfefede1a 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -2,8 +2,9 @@ import { describe, it, expect } from "bun:test" import * as fs from "fs/promises" import * as os from "os" import * as path from "path" -import { diffSummary, diffFile, generatedLike, MAX_DETAIL_BYTES } from "../../src/agent-manager/local-diff" +import { diffSummary, diffFile, generatedLike, resolveBase, MAX_DETAIL_BYTES } from "../../src/agent-manager/local-diff" import { GitOps } from "../../src/agent-manager/GitOps" +import { resolveLocalDiffTarget } from "../../src/review-utils" function git(): GitOps { return new GitOps({ log: () => undefined }) @@ -99,6 +100,10 @@ describe("diffSummary", () => { const result = await diffSummary(git(), dir, "") const entry = result.find((e) => e.file === "seed.txt") expect(entry?.status).toBe("modified") + // Pin the export contract: resolveBase("HEAD") must resolve to a real + // candidate branch when one exists locally. If this regresses, the + // revert-file fix below also silently regresses. + expect(await resolveBase(git(), dir, "HEAD")).toBe("main") }) }) @@ -306,3 +311,25 @@ describe("diffFile", () => { }) }) }) + +describe("resolveLocalDiffTarget + revertFile", () => { + it("resolves a real candidate branch so revertFile actually restores the file when there is no remote", async () => { + await withRepo(async (dir) => { + // No remote; `main` exists locally with the seed commit. + runSync(dir, ["checkout", "-b", "feature"]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfeature\n") + runSync(dir, ["commit", "-am", "feature commit"]) + + const target = await resolveLocalDiffTarget(git(), () => undefined, dir) + expect(target).toBeDefined() + // Before the fix this was "HEAD", which made revertFile a no-op. + expect(target?.baseBranch).toBe("main") + + const revert = await git().revertFile(dir, target!.baseBranch, "seed.txt", "modified") + expect(revert.ok).toBe(true) + + const restored = await fs.readFile(path.join(dir, "seed.txt"), "utf-8") + expect(restored).toBe("seed\n") + }) + }) +})