From 18d14185977e7b651fad2726d4a8a4455d423795 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Facu=5FVi=C3=B1as?= <facundovcanale@gmail.com>
Date: Wed, 11 Mar 2026 11:56:48 -0300
Subject: [PATCH] fix: bash/bg_shell hang and kill issues on Windows (#40)

- bg_shell killProcess: add Windows-specific taskkill /F /T /PID path
  with proper error handling (spawnSync with timeout, not stdio: "ignore")
- bg_shell startProcess: use getShellConfig() instead of hardcoded "bash",
  disable detached mode on Windows (process groups don't apply)
- GSD bash tool: wrap execute to inject 120s default timeout when the LLM
  omits the timeout parameter, preventing indefinite hangs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/resources/extensions/bg-shell/index.ts | 36 +++++++++++++++++-----
 src/resources/extensions/gsd/index.ts      | 26 ++++++++++++++--
 2 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/src/resources/extensions/bg-shell/index.ts b/src/resources/extensions/bg-shell/index.ts
index a83292951..83d227dcb 100644
--- a/src/resources/extensions/bg-shell/index.ts
+++ b/src/resources/extensions/bg-shell/index.ts
@@ -33,6 +33,7 @@ import {
 	truncateHead,
 	DEFAULT_MAX_BYTES,
 	DEFAULT_MAX_LINES,
+	getShellConfig,
 } from "@mariozechner/pi-coding-agent";
 import {
 	Text,
@@ -42,7 +43,7 @@ import {
 	Key,
 } from "@mariozechner/pi-tui";
 import { Type } from "@sinclair/typebox";
-import { spawn, type ChildProcess } from "node:child_process";
+import { spawn, spawnSync, type ChildProcess } from "node:child_process";
 import { createConnection } from "node:net";
 import { randomUUID } from "node:crypto";
 import { writeFileSync, readFileSync, existsSync, mkdirSync } from "node:fs";
@@ -551,11 +552,12 @@ function startProcess(opts: StartOptions): BgProcess {
 
 	const env = { ...process.env, ...(opts.env || {}) };
 
-	const proc = spawn("bash", ["-c", opts.command], {
+	const { shell, args: shellArgs } = getShellConfig();
+	const proc = spawn(shell, [...shellArgs, opts.command], {
 		cwd: opts.cwd,
 		stdio: ["pipe", "pipe", "pipe"],
 		env,
-		detached: true,
+		detached: process.platform !== "win32",
 	});
 
 	const bg: BgProcess = {
@@ -686,14 +688,32 @@ function killProcess(id: string, sig: NodeJS.Signals = "SIGTERM"): boolean {
 	if (!bg) return false;
 	if (!bg.alive) return true;
 	try {
-		if (bg.proc.pid) {
-			try {
-				process.kill(-bg.proc.pid, sig);
-			} catch {
+		if (process.platform === "win32") {
+			// Windows: use taskkill /F /T to force-kill the entire process tree.
+			// process.kill(-pid) (Unix process groups) does not work on Windows.
+			if (bg.proc.pid) {
+				const result = spawnSync("taskkill", ["/F", "/T", "/PID", String(bg.proc.pid)], {
+					timeout: 5000,
+					encoding: "utf-8",
+				});
+				if (result.status !== 0 && result.status !== 128) {
+					// taskkill failed — try the direct kill as fallback
+					bg.proc.kill(sig);
+				}
+			} else {
 				bg.proc.kill(sig);
 			}
 		} else {
-			bg.proc.kill(sig);
+			// Unix/macOS: kill the process group via negative PID
+			if (bg.proc.pid) {
+				try {
+					process.kill(-bg.proc.pid, sig);
+				} catch {
+					bg.proc.kill(sig);
+				}
+			} else {
+				bg.proc.kill(sig);
+			}
 		}
 		return true;
 	} catch {
diff --git a/src/resources/extensions/gsd/index.ts b/src/resources/extensions/gsd/index.ts
index 018843df1..caa3ed9bd 100644
--- a/src/resources/extensions/gsd/index.ts
+++ b/src/resources/extensions/gsd/index.ts
@@ -63,13 +63,35 @@ export default function (pi: ExtensionAPI) {
   registerGSDCommand(pi);
   registerWorktreeCommand(pi);
 
-  // ── Dynamic-cwd bash tool ──────────────────────────────────────────────
+  // ── Dynamic-cwd bash tool with default timeout ────────────────────────
   // The built-in bash tool captures cwd at startup. This replacement uses
   // a spawnHook to read process.cwd() dynamically so that process.chdir()
   // (used by /worktree switch) propagates to shell commands.
-  const dynamicBash = createBashTool(process.cwd(), {
+  //
+  // The upstream SDK's bash tool has no default timeout — if the LLM omits
+  // the timeout parameter, commands run indefinitely, causing hangs on
+  // Windows where process killing is unreliable (see #40). We wrap execute
+  // to inject a 120-second default when no timeout is provided.
+  const DEFAULT_BASH_TIMEOUT_SECS = 120;
+  const baseBash = createBashTool(process.cwd(), {
     spawnHook: (ctx) => ({ ...ctx, cwd: process.cwd() }),
   });
+  const dynamicBash = {
+    ...baseBash,
+    execute: async (
+      toolCallId: string,
+      params: { command: string; timeout?: number },
+      signal?: AbortSignal,
+      onUpdate?: any,
+      ctx?: any,
+    ) => {
+      const paramsWithTimeout = {
+        ...params,
+        timeout: params.timeout ?? DEFAULT_BASH_TIMEOUT_SECS,
+      };
+      return baseBash.execute(toolCallId, paramsWithTimeout, signal, onUpdate, ctx);
+    },
+  };
   pi.registerTool(dynamicBash as any);
 
   // ── session_start: render branded GSD header ───────────────────────────