mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-10-30 19:32:39 +08:00
Merge pull request #9204 from Kilo-Org/fix/agent-manager-local-diff-no-remote
fix(vscode): restore review diff in repos with no remote
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<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)
|
||||
if (ok.code === 0) return name
|
||||
}
|
||||
return "HEAD"
|
||||
}
|
||||
|
||||
async function ancestor(git: GitOps, dir: string, base: string, log?: Log): Promise<string | undefined> {
|
||||
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()
|
||||
|
||||
@@ -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}`)
|
||||
|
||||
|
||||
@@ -53,6 +53,35 @@ function gitOps(handler: (args: string[], cwd: string) => Promise<string>): 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
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user