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..c58189dace2 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"] + +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. + 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 + } + 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..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/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..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 }) @@ -91,6 +92,32 @@ 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") + // 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") + }) + }) + + 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") @@ -98,6 +125,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. @@ -272,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") + }) + }) +})