mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
fix(vscode): dispose git processes on panel close to prevent orphans on Windows (#8572)
Add AbortController to GitOps that kills in-flight child processes when dispose() is called. Thread the abort signal through both simple-git and child_process.spawn so spawned git processes are terminated immediately on Windows (TerminateProcess) and Unix (SIGTERM). Wire gitOps.dispose() into all three owners: DiffViewerProvider, AgentManagerProvider, and KiloProvider.
This commit is contained in:
@@ -212,6 +212,7 @@ export class DiffViewerProvider implements vscode.Disposable {
|
||||
|
||||
public dispose(): void {
|
||||
this.stopDiffPolling()
|
||||
this.gitOps.dispose()
|
||||
this.panel?.dispose()
|
||||
this.outputChannel.dispose()
|
||||
}
|
||||
|
||||
@@ -171,6 +171,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
private pendingFollowup: Followup | null = null
|
||||
/** Worktree diff stats poller for the sidebar badge — reuses GitStatsPoller (local stats only) */
|
||||
private statsPoller: GitStatsPoller | null = null
|
||||
private statsGitOps: GitOps | null = null
|
||||
private cachedStats: unknown = null
|
||||
|
||||
/** Optional interceptor called before the standard message handler.
|
||||
@@ -3148,7 +3149,9 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
|
||||
private startStatsPolling(): void {
|
||||
this.statsPoller?.stop()
|
||||
this.statsGitOps?.dispose()
|
||||
const git = new GitOps({ log: () => {} })
|
||||
this.statsGitOps = git
|
||||
this.statsPoller = new GitStatsPoller({
|
||||
getWorktrees: () => [],
|
||||
getWorkspaceRoot: () => getWorkspaceRoot(),
|
||||
@@ -3176,6 +3179,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
*/
|
||||
dispose(): void {
|
||||
this.statsPoller?.stop()
|
||||
this.statsGitOps?.dispose()
|
||||
this.unsubscribeEvent?.()
|
||||
this.unsubscribeState?.()
|
||||
this.unsubscribeNotificationDismiss?.()
|
||||
|
||||
@@ -1918,6 +1918,7 @@ export class AgentManagerProvider implements Disposable {
|
||||
public dispose(): void {
|
||||
this.stopDiffPolling()
|
||||
this.statsPoller.stop()
|
||||
this.gitOps.dispose()
|
||||
this.prBridge.poller.stop()
|
||||
this.terminalManager.dispose()
|
||||
this.panel?.dispose()
|
||||
|
||||
@@ -62,19 +62,45 @@ export function nonInteractiveEnv(): NodeJS.ProcessEnv {
|
||||
export class GitOps {
|
||||
private readonly log: (...args: unknown[]) => void
|
||||
private readonly runGit: (args: string[], cwd: string) => Promise<string>
|
||||
private readonly controller = new AbortController()
|
||||
|
||||
get disposed(): boolean {
|
||||
return this.controller.signal.aborted
|
||||
}
|
||||
|
||||
constructor(options: GitOpsOptions) {
|
||||
this.log = options.log
|
||||
this.runGit =
|
||||
options.runGit ??
|
||||
((args, cwd) =>
|
||||
simpleGit(cwd)
|
||||
simpleGit(cwd, { abort: this.controller.signal })
|
||||
.raw(args)
|
||||
.then((out) => out.trim()))
|
||||
}
|
||||
|
||||
dispose(): void {
|
||||
if (!this.controller.signal.aborted) {
|
||||
this.controller.abort()
|
||||
}
|
||||
}
|
||||
|
||||
private raw(args: string[], cwd: string): Promise<string> {
|
||||
return this.runGit(args, cwd)
|
||||
const signal = this.controller.signal
|
||||
if (signal.aborted) return Promise.reject(new Error("GitOps disposed"))
|
||||
return new Promise<string>((resolve, reject) => {
|
||||
const onAbort = () => reject(new Error("GitOps disposed"))
|
||||
signal.addEventListener("abort", onAbort, { once: true })
|
||||
this.runGit(args, cwd).then(
|
||||
(value) => {
|
||||
signal.removeEventListener("abort", onAbort)
|
||||
resolve(value)
|
||||
},
|
||||
(err) => {
|
||||
signal.removeEventListener("abort", onAbort)
|
||||
reject(err)
|
||||
},
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/** Return the name of the currently checked-out branch, or `"HEAD"` if detached. */
|
||||
@@ -335,10 +361,14 @@ export class GitOps {
|
||||
}
|
||||
|
||||
private exec(args: string[], cwd: string, options?: ExecOptions): Promise<ExecResult> {
|
||||
if (this.controller.signal.aborted) {
|
||||
return Promise.resolve({ code: 1, stdout: "", stderr: "GitOps disposed" })
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
const child = spawn("git", args, {
|
||||
cwd,
|
||||
env: options?.env,
|
||||
signal: this.controller.signal,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
})
|
||||
|
||||
|
||||
@@ -8,6 +8,10 @@ function ops(handler: (args: string[], cwd: string) => Promise<string>): GitOps
|
||||
return new GitOps({ log: () => undefined, runGit: handler })
|
||||
}
|
||||
|
||||
function sleep(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms))
|
||||
}
|
||||
|
||||
function runGit(cwd: string, args: string[]): string {
|
||||
const result = Bun.spawnSync({
|
||||
cmd: ["git", ...args],
|
||||
@@ -475,4 +479,81 @@ describe("GitOps", () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("dispose", () => {
|
||||
it("aborts in-flight runGit calls quickly", async () => {
|
||||
let resolved = false
|
||||
const git = new GitOps({
|
||||
log: () => undefined,
|
||||
runGit: async () => {
|
||||
await sleep(5000)
|
||||
resolved = true
|
||||
return "should not reach"
|
||||
},
|
||||
})
|
||||
|
||||
const start = Date.now()
|
||||
const pending = git.currentBranch("/repo")
|
||||
git.dispose()
|
||||
await pending
|
||||
const elapsed = Date.now() - start
|
||||
expect(elapsed).toBeLessThan(500)
|
||||
expect(resolved).toBe(false)
|
||||
})
|
||||
|
||||
it("causes subsequent runGit calls to fail immediately", async () => {
|
||||
let called = false
|
||||
const git = new GitOps({
|
||||
log: () => undefined,
|
||||
runGit: async () => {
|
||||
called = true
|
||||
return "ok"
|
||||
},
|
||||
})
|
||||
git.dispose()
|
||||
|
||||
// currentBranch swallows errors — should return "" without calling runGit
|
||||
const result = await git.currentBranch("/repo")
|
||||
expect(result).toBe("")
|
||||
expect(called).toBe(false)
|
||||
})
|
||||
|
||||
it("reports disposed state", () => {
|
||||
const git = ops(async () => "ok")
|
||||
expect(git.disposed).toBe(false)
|
||||
git.dispose()
|
||||
expect(git.disposed).toBe(true)
|
||||
})
|
||||
|
||||
it("kills in-flight exec (spawn) processes", async () => {
|
||||
await withRepo(async (cwd) => {
|
||||
const git = new GitOps({ log: () => undefined })
|
||||
await fs.writeFile(nodePath.join(cwd, "a.txt"), "one\n", "utf8")
|
||||
runGit(cwd, ["add", "-A"])
|
||||
runGit(cwd, ["-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-m", "init"])
|
||||
await fs.writeFile(nodePath.join(cwd, "a.txt"), "two\n", "utf8")
|
||||
|
||||
const branch = runGit(cwd, ["branch", "--show-current"]) || "HEAD"
|
||||
const pending = git.buildWorktreePatch(cwd, branch)
|
||||
// Give spawn a moment to start, then dispose
|
||||
await sleep(10)
|
||||
git.dispose()
|
||||
|
||||
// Should either reject or return (but process should be killed)
|
||||
try {
|
||||
await pending
|
||||
} catch {
|
||||
// expected — aborted
|
||||
}
|
||||
expect(git.disposed).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
it("is safe to call multiple times", () => {
|
||||
const git = ops(async () => "ok")
|
||||
git.dispose()
|
||||
git.dispose()
|
||||
expect(git.disposed).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user