mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-28 19:11:03 +08:00
fix(vscode): use current remote default for diffs
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"kilo-code": patch
|
||||
---
|
||||
|
||||
Use the remote's current default branch for diff totals and new worktrees when local Git metadata still points to a retired trunk.
|
||||
@@ -0,0 +1,20 @@
|
||||
# Fix stale Git default branch diffs
|
||||
|
||||
## Goal
|
||||
|
||||
Keep branch diff totals and the diff view on the same comparison while preventing an old local `origin/HEAD` from selecting a retired trunk such as `master` after a repository moves to `main`.
|
||||
|
||||
## Approach
|
||||
|
||||
1. Update the existing shared `GitOps.resolveDefaultBranch` path instead of adding another diff calculator.
|
||||
2. Resolve the remote's current `HEAD` with a read-only, non-interactive `git ls-remote --symref` call, cache it, and use local `<remote>/HEAD` only when the remote cannot answer.
|
||||
3. Keep explicit worktree parent branches, configured upstreams, user base overrides, and repositories that still use `master` unchanged.
|
||||
4. Add focused tests for a stale local `origin/HEAD`, remote-unavailable fallback, and normal `master` repositories.
|
||||
|
||||
## Validation
|
||||
|
||||
1. Create a disposable repository whose remote moved from `master` to `main` while the clone retains `origin/HEAD -> origin/master`.
|
||||
2. Record the large pre-fix `master...HEAD` totals.
|
||||
3. Verify the shared resolver selects `origin/main` after the fix and that stats and file-list targets match.
|
||||
4. Verify a repository whose remote still advertises `master` continues to use `origin/master`.
|
||||
5. Run the focused unit tests, extension typecheck/lint, and the isolated VS Code self-test for the indicator-to-diff flow.
|
||||
@@ -43,6 +43,7 @@ interface ApplyPatchResult {
|
||||
interface ExecOptions {
|
||||
env?: NodeJS.ProcessEnv
|
||||
stdin?: string
|
||||
timeout?: number
|
||||
}
|
||||
|
||||
export interface ExecResult {
|
||||
@@ -126,6 +127,7 @@ export class GitOps {
|
||||
private executableCache: Promise<string> | undefined
|
||||
private readonly resolutionCache = new Map<string, { value: string; expires: number }>()
|
||||
private static readonly CACHE_TTL_MS = 60000
|
||||
private static readonly DEFAULT_BRANCH_CACHE_TTL_MS = 10 * 60_000
|
||||
private static readonly MAX_CACHE_SIZE = 100
|
||||
|
||||
get disposed(): boolean {
|
||||
@@ -165,7 +167,7 @@ export class GitOps {
|
||||
return undefined
|
||||
}
|
||||
|
||||
private setCached(key: string, value: string): void {
|
||||
private setCached(key: string, value: string, ttl = GitOps.CACHE_TTL_MS): void {
|
||||
if (this.resolutionCache.size >= GitOps.MAX_CACHE_SIZE) {
|
||||
let oldestKey: string | undefined
|
||||
let oldestExpiry = Infinity
|
||||
@@ -177,7 +179,7 @@ export class GitOps {
|
||||
}
|
||||
if (oldestKey) this.resolutionCache.delete(oldestKey)
|
||||
}
|
||||
this.resolutionCache.set(key, { value, expires: Date.now() + GitOps.CACHE_TTL_MS })
|
||||
this.resolutionCache.set(key, { value, expires: Date.now() + ttl })
|
||||
}
|
||||
|
||||
private raw(args: string[], cwd: string): Promise<string> {
|
||||
@@ -274,19 +276,32 @@ export class GitOps {
|
||||
return undefined
|
||||
}
|
||||
|
||||
/** Resolve the repo's default branch via <remote>/HEAD. */
|
||||
/** Resolve the repo's default branch from the remote, then local <remote>/HEAD. */
|
||||
async resolveDefaultBranch(cwd: string, branch?: string): Promise<string | undefined> {
|
||||
const remote = await this.resolveRemote(cwd, branch)
|
||||
const cacheKey = `default-branch:${cwd}:${remote}`
|
||||
const cached = this.getCached(cacheKey)
|
||||
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
|
||||
this.setCached(cacheKey, result ?? "")
|
||||
const advertised = await this.remoteHead(cwd, remote)
|
||||
const match = advertised.match(/^ref:\s+refs\/heads\/(.+)\s+HEAD$/m)
|
||||
const current = match?.[1] ? `${remote}/${match[1]}` : undefined
|
||||
const local = current
|
||||
? ""
|
||||
: await this.raw(["symbolic-ref", "--short", `refs/remotes/${remote}/HEAD`], cwd).catch(() => "")
|
||||
const result = current || local || undefined
|
||||
this.setCached(cacheKey, result ?? "", GitOps.DEFAULT_BRANCH_CACHE_TTL_MS)
|
||||
return result
|
||||
}
|
||||
|
||||
private async remoteHead(cwd: string, remote: string): Promise<string> {
|
||||
const args = ["ls-remote", "--symref", remote, "HEAD"]
|
||||
if (this.injected) return this.raw(args, cwd).catch(() => "")
|
||||
|
||||
const result = await this.exec(args, cwd, { env: nonInteractiveEnv(), timeout: 5000 })
|
||||
return result.code === 0 ? result.stdout.trim() : ""
|
||||
}
|
||||
|
||||
async hasRemoteRef(cwd: string, ref: string): Promise<boolean> {
|
||||
return this.raw(["rev-parse", "--verify", "--quiet", `refs/remotes/${ref}`], cwd)
|
||||
.then(() => true)
|
||||
@@ -641,6 +656,12 @@ export class GitOps {
|
||||
const err: Buffer[] = []
|
||||
let failure: string | undefined
|
||||
const abort = () => child.kill("SIGTERM")
|
||||
const timeout = options?.timeout
|
||||
? setTimeout(() => {
|
||||
failure = `Git command timed out after ${options.timeout}ms`
|
||||
child.kill("SIGTERM")
|
||||
}, options.timeout)
|
||||
: undefined
|
||||
|
||||
this.controller.signal.addEventListener("abort", abort, { once: true })
|
||||
child.stdout?.on("data", (chunk: Buffer) => out.push(chunk))
|
||||
@@ -650,6 +671,7 @@ export class GitOps {
|
||||
failure = error.message
|
||||
})
|
||||
child.on("close", (code) => {
|
||||
if (timeout) clearTimeout(timeout)
|
||||
this.controller.signal.removeEventListener("abort", abort)
|
||||
resolve({
|
||||
code: code ?? 1,
|
||||
|
||||
@@ -965,8 +965,18 @@ export class WorktreeManager {
|
||||
}
|
||||
|
||||
async defaultBranch(): Promise<string> {
|
||||
// 1. Try symbolic-ref against the resolved remote (not hardcoded "origin")
|
||||
const remote = await this.resolveRemote()
|
||||
|
||||
// 1. Prefer the shared resolver, which verifies the remote's current HEAD.
|
||||
if (this.ops && remote) {
|
||||
const ref = await this.ops.resolveDefaultBranch(this.root).catch((e) => {
|
||||
this.log(`defaultBranch: shared resolver failed: ${e}`)
|
||||
return undefined
|
||||
})
|
||||
if (ref?.startsWith(`${remote}/`)) return ref.slice(remote.length + 1)
|
||||
}
|
||||
|
||||
// 2. Try local symbolic-ref against the resolved remote (not hardcoded "origin")
|
||||
if (remote) {
|
||||
try {
|
||||
const head = await this.git.raw(["symbolic-ref", `refs/remotes/${remote}/HEAD`])
|
||||
@@ -978,7 +988,7 @@ export class WorktreeManager {
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Try current branch (if not detached)
|
||||
// 3. Try current branch (if not detached)
|
||||
try {
|
||||
const current = await this.currentBranch()
|
||||
if (current && current !== "HEAD") return current
|
||||
@@ -986,7 +996,7 @@ export class WorktreeManager {
|
||||
this.log(`defaultBranch: currentBranch failed: ${e}`)
|
||||
}
|
||||
|
||||
// 3. Try first local branch
|
||||
// 4. Try first local branch
|
||||
try {
|
||||
const branches = await this.git.branchLocal()
|
||||
if (branches.all.length > 0) return branches.all[0]
|
||||
|
||||
@@ -228,33 +228,68 @@ describe("GitOps", () => {
|
||||
})
|
||||
|
||||
describe("resolveDefaultBranch", () => {
|
||||
it("returns <remote>/HEAD symbolic ref", async () => {
|
||||
it("uses the remote's advertised HEAD instead of stale local metadata", async () => {
|
||||
const commands: string[][] = []
|
||||
const git = ops(async (args) => {
|
||||
commands.push(args)
|
||||
// resolveRemote: upstream is configured
|
||||
if (args[0] === "rev-parse" && args[3] === "@{upstream}") return "upstream/main"
|
||||
// symbolic-ref for upstream/HEAD
|
||||
if (args[0] === "symbolic-ref" && args[2] === "refs/remotes/upstream/HEAD") return "upstream/develop"
|
||||
if (args[0] === "ls-remote") return "ref: refs/heads/develop\tHEAD\nabc123\tHEAD"
|
||||
if (args[0] === "symbolic-ref" && args[2] === "refs/remotes/upstream/HEAD") return "upstream/master"
|
||||
return ""
|
||||
})
|
||||
expect(await git.resolveDefaultBranch("/repo", "feature")).toBe("upstream/develop")
|
||||
expect(commands.some((args) => args[0] === "symbolic-ref")).toBe(false)
|
||||
})
|
||||
|
||||
it("falls back to origin/HEAD when remote is origin", async () => {
|
||||
it("falls back to local origin/HEAD when the remote is unavailable", async () => {
|
||||
const git = ops(async (args) => {
|
||||
if (args[0] === "rev-parse" && args[3] === "@{upstream}") throw new Error("no upstream")
|
||||
if (args[0] === "config") throw new Error("no config")
|
||||
if (args[0] === "branch") return "feature"
|
||||
if (args[0] === "ls-remote") throw new Error("offline")
|
||||
if (args[0] === "symbolic-ref" && args[2] === "refs/remotes/origin/HEAD") return "origin/main"
|
||||
return ""
|
||||
})
|
||||
expect(await git.resolveDefaultBranch("/repo", "feature")).toBe("origin/main")
|
||||
})
|
||||
|
||||
it("keeps master when the remote still advertises master", async () => {
|
||||
const git = ops(async (args) => {
|
||||
if (args[0] === "rev-parse") throw new Error("no upstream")
|
||||
if (args[0] === "config") throw new Error("no config")
|
||||
if (args[0] === "branch") return "feature"
|
||||
if (args[0] === "ls-remote") return "ref: refs/heads/master\tHEAD\nabc123\tHEAD"
|
||||
return ""
|
||||
})
|
||||
|
||||
expect(await git.resolveDefaultBranch("/repo", "feature")).toBe("origin/master")
|
||||
})
|
||||
|
||||
it("caches the advertised remote HEAD", async () => {
|
||||
let calls = 0
|
||||
const git = ops(async (args) => {
|
||||
if (args[0] === "rev-parse") throw new Error("no upstream")
|
||||
if (args[0] === "config") throw new Error("no config")
|
||||
if (args[0] === "branch") return "feature"
|
||||
if (args[0] === "ls-remote") {
|
||||
calls++
|
||||
return "ref: refs/heads/main\tHEAD\nabc123\tHEAD"
|
||||
}
|
||||
return ""
|
||||
})
|
||||
|
||||
expect(await git.resolveDefaultBranch("/repo", "feature")).toBe("origin/main")
|
||||
expect(await git.resolveDefaultBranch("/repo", "feature")).toBe("origin/main")
|
||||
expect(calls).toBe(1)
|
||||
})
|
||||
|
||||
it("returns undefined when <remote>/HEAD is not set", async () => {
|
||||
const git = ops(async (args) => {
|
||||
if (args[0] === "rev-parse") throw new Error("no upstream")
|
||||
if (args[0] === "config") throw new Error("no config")
|
||||
if (args[0] === "branch") return ""
|
||||
if (args[0] === "ls-remote") throw new Error("no remote")
|
||||
if (args[0] === "symbolic-ref") throw new Error("no symbolic ref")
|
||||
return ""
|
||||
})
|
||||
|
||||
@@ -590,7 +590,7 @@ describe("GitStatsPoller", () => {
|
||||
expect(emitted.length).toBe(1)
|
||||
})
|
||||
|
||||
it("falls back to <remote>/HEAD when no upstream and no <remote>/<branch>", async () => {
|
||||
it("uses advertised remote HEAD when local <remote>/HEAD is stale", async () => {
|
||||
const emitted: Array<{
|
||||
branch: string
|
||||
files: number
|
||||
@@ -599,11 +599,15 @@ describe("GitStatsPoller", () => {
|
||||
ahead: number
|
||||
behind: number
|
||||
}> = []
|
||||
const bases: string[] = []
|
||||
|
||||
const poller = new GitStatsPoller({
|
||||
getWorktrees: () => [],
|
||||
getWorkspaceRoot: () => "/workspace",
|
||||
source: source(async () => diff(10, 4), "my-feature"),
|
||||
source: source(async (_dir, base) => {
|
||||
bases.push(base)
|
||||
return diff(10, 4)
|
||||
}, "my-feature"),
|
||||
onStats: () => undefined,
|
||||
onLocalStats: (stats) => emitted.push(stats),
|
||||
log: () => undefined,
|
||||
@@ -619,8 +623,9 @@ describe("GitStatsPoller", () => {
|
||||
// myfork/my-feature does not exist
|
||||
if (args[0] === "rev-parse" && args[1] === "--verify" && args[2] === "myfork/my-feature")
|
||||
throw new Error("no ref")
|
||||
// myfork/HEAD resolves to the default branch
|
||||
if (args[0] === "symbolic-ref" && args[2] === "refs/remotes/myfork/HEAD") return "myfork/develop"
|
||||
// The remote moved to develop, but this clone still records master.
|
||||
if (args[0] === "ls-remote") return "ref: refs/heads/develop\tHEAD\nabc123\tHEAD"
|
||||
if (args[0] === "symbolic-ref" && args[2] === "refs/remotes/myfork/HEAD") return "myfork/master"
|
||||
if (args[0] === "branch") return "my-feature"
|
||||
if (args[0] === "rev-list" && args[1] === "--left-right") return "0\t5"
|
||||
return ""
|
||||
@@ -632,6 +637,7 @@ describe("GitStatsPoller", () => {
|
||||
poller.stop()
|
||||
|
||||
expect(emitted[0]).toEqual({ branch: "my-feature", files: 1, additions: 10, deletions: 4, ahead: 5, behind: 0 })
|
||||
expect(bases[0]).toBe("myfork/develop")
|
||||
})
|
||||
|
||||
it("falls back to workingTreeStats when no tracking, no default branch, and no remote refs exist", async () => {
|
||||
|
||||
@@ -448,6 +448,40 @@ describe("diffFile", () => {
|
||||
})
|
||||
|
||||
describe("resolveLocalDiffTarget + revertFile", () => {
|
||||
it("uses the remote's current trunk when local origin/HEAD is stale", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "local-diff-stale-head-"))
|
||||
const remote = path.join(root, "remote.git")
|
||||
const dir = path.join(root, "clone")
|
||||
try {
|
||||
runSync(root, ["init", "--bare", "-b", "master", remote])
|
||||
runSync(root, ["clone", remote, dir])
|
||||
runSync(dir, ["config", "user.email", "test@example.com"])
|
||||
runSync(dir, ["config", "user.name", "Test"])
|
||||
await fs.writeFile(path.join(dir, "seed.txt"), "master\n")
|
||||
runSync(dir, ["add", "seed.txt"])
|
||||
runSync(dir, ["commit", "-m", "master seed"])
|
||||
runSync(dir, ["push", "-u", "origin", "master"])
|
||||
runSync(dir, ["checkout", "-b", "main"])
|
||||
await fs.writeFile(path.join(dir, "seed.txt"), "main\n")
|
||||
runSync(dir, ["commit", "-am", "move trunk to main"])
|
||||
runSync(dir, ["push", "-u", "origin", "main"])
|
||||
runSync(remote, ["symbolic-ref", "HEAD", "refs/heads/main"])
|
||||
runSync(dir, ["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/master"])
|
||||
runSync(dir, ["checkout", "-b", "feature"])
|
||||
await fs.writeFile(path.join(dir, "feature.txt"), "one line\n")
|
||||
|
||||
const target = await resolveLocalDiffTarget(git(), () => undefined, dir)
|
||||
|
||||
expect(target?.baseBranch).toBe("origin/main")
|
||||
expect(runSync(dir, ["symbolic-ref", "--short", "refs/remotes/origin/HEAD"])).toBe("origin/master")
|
||||
const entries = await diffSummary(git(), dir, target!.baseBranch)
|
||||
expect(entries).toHaveLength(1)
|
||||
expect(entries[0]).toMatchObject({ file: "feature.txt", additions: 1, deletions: 0 })
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
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.
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
versionedName,
|
||||
} from "../../src/agent-manager/branch-name"
|
||||
import { WorktreeStateManager } from "../../src/agent-manager/WorktreeStateManager"
|
||||
import { GitOps } from "../../src/agent-manager/GitOps"
|
||||
import type { PRInfo } from "../../src/agent-manager/git-import"
|
||||
import simpleGit from "simple-git"
|
||||
|
||||
@@ -46,9 +47,9 @@ async function createTempRepo(): Promise<string> {
|
||||
return dir
|
||||
}
|
||||
|
||||
function createManager(root: string): WorktreeManager {
|
||||
function createManager(root: string, ops?: GitOps): WorktreeManager {
|
||||
const logs: string[] = []
|
||||
return new WorktreeManager(root, (msg) => logs.push(msg))
|
||||
return new WorktreeManager(root, (msg) => logs.push(msg), ops)
|
||||
}
|
||||
|
||||
// Test-only helper to verify metadata writes keep the temp worktree checkout clean.
|
||||
@@ -1043,6 +1044,26 @@ describe("WorktreeManager.resolveStartPoint", () => {
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("WorktreeManager.resolveBaseBranch", () => {
|
||||
it("uses the shared remote default instead of stale local metadata", async () => {
|
||||
const { clone } = await createTempRepoWithOrigin()
|
||||
gitExec(["git", "-C", clone, "branch", "master"])
|
||||
gitExec(["git", "-C", clone, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/master"])
|
||||
const ops = new GitOps({
|
||||
log: () => undefined,
|
||||
runGit: async (args) => {
|
||||
if (args[0] === "rev-parse" && args[3] === "@{upstream}") return "origin/main"
|
||||
if (args[0] === "ls-remote") return "ref: refs/heads/main\tHEAD\nabc123\tHEAD"
|
||||
return ""
|
||||
},
|
||||
})
|
||||
const mgr = createManager(clone, ops)
|
||||
|
||||
expect(await mgr.resolveBaseBranch()).toEqual({ branch: "main", remote: "origin" })
|
||||
expect((await simpleGit(clone).raw(["symbolic-ref", "--short", "refs/remotes/origin/HEAD"])).trim()).toBe(
|
||||
"origin/master",
|
||||
)
|
||||
})
|
||||
|
||||
it("returns bare branch + remote when origin remote and tracking ref exist", async () => {
|
||||
const { clone } = await createTempRepoWithOrigin()
|
||||
const mgr = createManager(clone)
|
||||
|
||||
Reference in New Issue
Block a user