From 60c91b51132d7a4bdfd7da886f52bf3a205d2a86 Mon Sep 17 00:00:00 2001 From: Marius Date: Wed, 8 Apr 2026 12:59:33 +0200 Subject: [PATCH] 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. --- .../kilo-vscode/src/DiffViewerProvider.ts | 1 + packages/kilo-vscode/src/KiloProvider.ts | 4 + .../src/agent-manager/AgentManagerProvider.ts | 1 + .../kilo-vscode/src/agent-manager/GitOps.ts | 34 +++++++- .../kilo-vscode/tests/unit/git-ops.test.ts | 81 +++++++++++++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/packages/kilo-vscode/src/DiffViewerProvider.ts b/packages/kilo-vscode/src/DiffViewerProvider.ts index f9f143f7467..67466a0fa8a 100644 --- a/packages/kilo-vscode/src/DiffViewerProvider.ts +++ b/packages/kilo-vscode/src/DiffViewerProvider.ts @@ -212,6 +212,7 @@ export class DiffViewerProvider implements vscode.Disposable { public dispose(): void { this.stopDiffPolling() + this.gitOps.dispose() this.panel?.dispose() this.outputChannel.dispose() } diff --git a/packages/kilo-vscode/src/KiloProvider.ts b/packages/kilo-vscode/src/KiloProvider.ts index 481284eec01..6888caf5d19 100644 --- a/packages/kilo-vscode/src/KiloProvider.ts +++ b/packages/kilo-vscode/src/KiloProvider.ts @@ -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?.() diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index 3554990404d..8ebb82f9a89 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -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() diff --git a/packages/kilo-vscode/src/agent-manager/GitOps.ts b/packages/kilo-vscode/src/agent-manager/GitOps.ts index dcd74e95987..6c43f64d1e4 100644 --- a/packages/kilo-vscode/src/agent-manager/GitOps.ts +++ b/packages/kilo-vscode/src/agent-manager/GitOps.ts @@ -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 + 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 { - return this.runGit(args, cwd) + const signal = this.controller.signal + if (signal.aborted) return Promise.reject(new Error("GitOps disposed")) + return new Promise((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 { + 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"], }) diff --git a/packages/kilo-vscode/tests/unit/git-ops.test.ts b/packages/kilo-vscode/tests/unit/git-ops.test.ts index eff6a667259..64f0b78c85a 100644 --- a/packages/kilo-vscode/tests/unit/git-ops.test.ts +++ b/packages/kilo-vscode/tests/unit/git-ops.test.ts @@ -8,6 +8,10 @@ function ops(handler: (args: string[], cwd: string) => Promise): GitOps return new GitOps({ log: () => undefined, runGit: handler }) } +function sleep(ms: number): Promise { + 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) + }) + }) })