fix(browse): make sidebar Terminal pane work on Windows

StreamDemon · claude · StreamDemon · commit 61a6336cf3cd · 2026-04-27T15:16:53.000+08:00
Two Windows-only blockers in the side-panel PTY: 1. terminal-agent.ts spawnClaude() used Bun.spawn(..., {terminal:}), which is POSIX-only by design (oven-sh/bun#25565). Platform-gated to bun-pty (Rust portable-pty via Bun FFI) on Windows; POSIX path unchanged. Wrapped behind a Bun-Subprocess-shaped adapter so disposeSession, the message handler, and source-grep tests see one uniform shape across platforms. 2. cli.ts spawned the agent via Bun.spawn(['bun','run',...]) from inside compiled browse.exe; the child died before writing its state files. pkill-based cleanup also doesn't exist on Windows. Mirrors the existing Windows-detach pattern at cli.ts:221-232: Bun.spawnSync( ['node','-e',launcherCode]) wrapping child_process.spawn().unref(). Cleanup uses taskkill /PID with /FI IMAGENAME filter so recycled PIDs can't be killed by accident. Spawned-child stdio is redirected to <projectDir>/.gstack/terminal-agent.log so startup failures (port collision, missing claude, bun-pty load fail) are diagnosable instead of presenting as silent 503s in the sidebar. Closes #1221. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diff --git a/browse/src/cli.ts b/browse/src/cli.ts
@@ -896,23 +896,99 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
         if (fs.existsSync(termAgentScript)) {
           // Kill old terminal-agents so a stale port file can't trick the
           // server into routing /pty-session at a dead listener.
-          try {
-            const { spawnSync } = require('child_process');
-            spawnSync('pkill', ['-f', 'terminal-agent\\.ts'], { stdio: 'ignore', timeout: 3000 });
-          } catch (err: any) {
-            if (err?.code !== 'ENOENT') throw err;
+          if (IS_WINDOWS) {
+            // Windows: read the PID file the previous agent wrote, target it
+            // explicitly with taskkill /PID. Don't use /IM bun.exe — that
+            // would kill every Bun process on the machine. Add /FI IMAGENAME
+            // filter so a PID that was recycled to a non-bun process gets
+            // skipped (the previous agent may have been hard-killed and left
+            // a stale PID file). Number.isFinite check distinguishes "first
+            // run" from "corrupted PID file" — log a warning for the latter.
+            const pidFile = path.join(path.dirname(config.stateFile), 'terminal-agent.pid');
+            try {
+              const raw = fs.readFileSync(pidFile, 'utf-8');
+              const oldPid = parseInt(raw, 10);
+              if (Number.isFinite(oldPid) && oldPid > 0) {
+                Bun.spawnSync(['taskkill', '/PID', String(oldPid), '/T', '/F', '/FI', 'IMAGENAME eq bun.exe'], {
+                  stdout: 'pipe', stderr: 'pipe', timeout: 5000,
+                });
+              } else {
+                console.warn(`[browse] terminal-agent.pid contained non-numeric content (${JSON.stringify(raw.slice(0, 32))}); skipping kill`);
+              }
+            } catch (err: any) {
+              if (err?.code !== 'ENOENT') throw err;
+            }
+          } else {
+            try {
+              const { spawnSync } = require('child_process');
+              spawnSync('pkill', ['-f', 'terminal-agent\\.ts'], { stdio: 'ignore', timeout: 3000 });
+            } catch (err: any) {
+              if (err?.code !== 'ENOENT') throw err;
+            }
+          }
+
+          if (IS_WINDOWS) {
+            // Windows: Bun.spawn(['bun','run',...]).unref() doesn't truly
+            // detach when invoked from inside the compiled browse.exe — the
+            // child dies before it can write its state files (terminal-port,
+            // terminal-agent.pid). Mirror the proven pattern at cli.ts:221-232:
+            // stringified child_process.spawn launched via `node -e`, with
+            // detached: true and .unref() so the agent outlives this process.
+            //
+            // Resolve the real bun.exe — npm-installed `bun` is a POSIX shell
+            // script Windows can't execute (the bug #1 npm-shim issue). Don't
+            // fall back to bare 'bun.exe' on PATH — it'll resolve to that same
+            // shim. process.execPath returns the compiled browse.exe (which
+            // can't be re-invoked as `bun run`), so it's not a valid fallback
+            // either. If neither candidate exists, fail loud with install
+            // instructions and skip the spawn (chat degrades gracefully).
+            const bunCandidates = [
+              process.env.BUN_INSTALL && `${process.env.BUN_INSTALL}\\bin\\bun.exe`,
+              process.env.USERPROFILE && `${process.env.USERPROFILE}\\.bun\\bin\\bun.exe`,
+            ].filter(Boolean) as string[];
+            const bunExe = bunCandidates.find(c => fs.existsSync(c));
+            if (!bunExe) {
+              console.error('[browse] Could not find bun.exe (checked BUN_INSTALL\\bin and %USERPROFILE%\\.bun\\bin).');
+              console.error('[browse] Terminal agent will not start. Install official Bun: irm bun.com/install.ps1 | iex');
+            } else {
+              // Redirect the detached child's stdout+stderr to a log file so
+              // startup failures (port collision, claude-not-found, bun-pty
+              // load failure per terminal-agent.ts:33, etc.) are diagnosable.
+              // Without this, the user sees only "Terminal agent started" +
+              // 503s in the sidebar with no breadcrumb. Parent's stderr is
+              // 'inherit' so synchronous launcher errors (e.g. node missing)
+              // surface immediately to the user's console.
+              const logPath = path.join(path.dirname(config.stateFile), 'terminal-agent.log');
+              const extraEnvStr = JSON.stringify({
+                BROWSE_STATE_FILE: config.stateFile,
+                BROWSE_SERVER_PORT: String(newState.port),
+              });
+              const launcherCode =
+                `const{spawn}=require('child_process');` +
+                `const fs=require('fs');` +
+                `const out=fs.openSync(${JSON.stringify(logPath)},'a');` +
+                `spawn(${JSON.stringify(bunExe)},['run',${JSON.stringify(termAgentScript)}],` +
+                `{detached:true,stdio:['ignore',out,out],windowsHide:true,` +
+                `cwd:${JSON.stringify(config.projectDir)},` +
+                `env:Object.assign({},process.env,${extraEnvStr})}).unref()`;
+              Bun.spawnSync(['node', '-e', launcherCode], {
+                stdio: ['ignore', 'ignore', 'inherit'],
+              });
+              console.log(`[browse] Terminal agent started (detached); logs: ${logPath}`);
+            }
+          } else {
+            const termProc = Bun.spawn(['bun', 'run', termAgentScript], {
+              cwd: config.projectDir,
+              env: {
+                ...process.env,
+                BROWSE_STATE_FILE: config.stateFile,
+                BROWSE_SERVER_PORT: String(newState.port),
+              },
+              stdio: ['ignore', 'ignore', 'ignore'],
+            });
+            termProc.unref();
+            console.log(`[browse] Terminal agent started (PID: ${termProc.pid})`);
           }
-          const termProc = Bun.spawn(['bun', 'run', termAgentScript], {
-            cwd: config.projectDir,
-            env: {
-              ...process.env,
-              BROWSE_STATE_FILE: config.stateFile,
-              BROWSE_SERVER_PORT: String(newState.port),
-            },
-            stdio: ['ignore', 'ignore', 'ignore'],
-          });
-          termProc.unref();
-          console.log(`[browse] Terminal agent started (PID: ${termProc.pid})`);
         }
       } catch (err: any) {
         // Non-fatal: chat still works without the terminal agent.
diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts
@@ -23,10 +23,31 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import * as crypto from 'crypto';
-import { safeUnlink } from './error-handling';
+import { safeUnlink, safeUnlinkQuiet } from './error-handling';
+import { IS_WINDOWS } from './platform';
+
+// bun-pty is loaded only on Windows because Bun's `terminal:` spawn option
+// is POSIX-only (upstream oven-sh/bun#25565). The agent runs as
+// `bun run terminal-agent.ts` (not compiled), so node_modules resolution
+// works at runtime; macOS/Linux skip the dependency entirely. If load
+// fails (missing dep, native binding can't dlopen — e.g. AV quarantine,
+// MSVC runtime missing), exit loudly with actionable instructions so the
+// failure isn't invisible behind cli.ts's "Terminal agent started" message.
+let ptySpawn: any = null;
+if (IS_WINDOWS) {
+  try {
+    ptySpawn = (await import('bun-pty')).spawn;
+  } catch (err: any) {
+    console.error(`[terminal-agent] FATAL: bun-pty failed to load on Windows: ${err?.message || err}`);
+    console.error(`[terminal-agent] Run \`bun install\` in the gstack directory.`);
+    console.error(`[terminal-agent] If the problem persists, see https://github.com/oven-sh/bun/issues/25565`);
+    process.exit(2);
+  }
+}
 
 const STATE_FILE = process.env.BROWSE_STATE_FILE || path.join(process.env.HOME || '/tmp', '.gstack', 'browse.json');
 const PORT_FILE = path.join(path.dirname(STATE_FILE), 'terminal-port');
+const PID_FILE = path.join(path.dirname(STATE_FILE), 'terminal-agent.pid');
 const BROWSE_SERVER_PORT = parseInt(process.env.BROWSE_SERVER_PORT || '0', 10);
 const EXTENSION_ID = process.env.BROWSE_EXTENSION_ID || ''; // optional: tighten Origin check
 const INTERNAL_TOKEN = crypto.randomBytes(32).toString('base64url'); // shared with parent server via env at spawn
@@ -74,6 +95,17 @@ function findClaude(): string | null {
     `${process.env.HOME}/.bun/bin/claude`,
     `${process.env.HOME}/.npm-global/bin/claude`,
   ];
+  if (IS_WINDOWS) {
+    // HOME is often unset on Windows; use APPDATA / LOCALAPPDATA / USERPROFILE
+    // explicitly. Cover the npm shim (claude.cmd), the official Anthropic
+    // installer, and `bun install -g`.
+    if (process.env.APPDATA) candidates.push(`${process.env.APPDATA}\\npm\\claude.cmd`);
+    if (process.env.LOCALAPPDATA) {
+      candidates.push(`${process.env.LOCALAPPDATA}\\AnthropicClaude\\claude.exe`);
+      candidates.push(`${process.env.LOCALAPPDATA}\\Programs\\Anthropic\\claude\\claude.exe`);
+    }
+    if (process.env.USERPROFILE) candidates.push(`${process.env.USERPROFILE}\\.bun\\bin\\claude.exe`);
+  }
   for (const c of candidates) {
     try { fs.accessSync(c, fs.constants.X_OK); return c; } catch {}
   }
@@ -167,6 +199,54 @@ function spawnClaude(cols: number, rows: number, onData: (chunk: Buffer) => void
   const stateDir = path.dirname(STATE_FILE);
   const tabHint = buildTabAwarenessHint(stateDir);
 
+  if (IS_WINDOWS) {
+    // ConPTY-backed PTY via bun-pty (Rust portable-pty + Bun FFI). Returns a
+    // wrapper that mimics Bun's Subprocess shape — { pid, terminal: { write,
+    // resize, close }, kill, killed, exited } — so disposeSession and the
+    // message handler use one set of optional-chain accessors across platforms.
+    const pty = ptySpawn(claudePath, ['--append-system-prompt', tabHint], {
+      name: 'xterm-256color',
+      cols,
+      rows,
+      env,
+    });
+    pty.onData((s: string) => onData(Buffer.from(s, 'utf-8')));
+    let killed = false;
+    const exited = new Promise<number>((resolve) => {
+      pty.onExit(({ exitCode }: any) => {
+        killed = true;
+        resolve(exitCode ?? 0);
+      });
+    });
+    return {
+      pid: pty.pid,
+      get killed() { return killed; },
+      terminal: {
+        // Don't swallow write errors here — the message handler at the call
+        // site already wraps in try/catch and logs via console.error. Letting
+        // throws propagate keeps Win/POSIX error visibility symmetric.
+        write: (buf: Buffer | string) => {
+          pty.write(typeof buf === 'string' ? buf : buf.toString('utf-8'));
+        },
+        resize: (c: number, r: number) => {
+          try { pty.resize(c, r); } catch (err) {
+            console.error('[terminal-agent] pty.resize failed:', err);
+          }
+        },
+        close: () => { /* bun-pty has no terminal.close; kill() handles teardown */ },
+      },
+      kill: (signal?: string) => {
+        try {
+          pty.kill(signal);
+          killed = true;
+        } catch (err) {
+          console.error('[terminal-agent] pty.kill failed:', err);
+        }
+      },
+      exited,
+    };
+  }
+
   const proc = (Bun as any).spawn([claudePath, '--append-system-prompt', tabHint], {
     terminal: {
       rows,
@@ -529,16 +609,33 @@ function main() {
   fs.writeFileSync(tmp, String(port), { mode: 0o600 });
   fs.renameSync(tmp, PORT_FILE);
 
+  // Write PID file atomically. cli.ts reads this on disconnect/cleanup so
+  // it can target this exact agent with taskkill /PID on Windows (where
+  // pkill -f doesn't exist and /IM bun.exe would kill every Bun process).
+  const pidTmp = `${PID_FILE}.tmp-${process.pid}`;
+  fs.writeFileSync(pidTmp, String(process.pid), { mode: 0o600 });
+  fs.renameSync(pidTmp, PID_FILE);
+
   // Hand the parent the internal token so it can call /internal/grant.
   // Parent learns INTERNAL_TOKEN via env (TERMINAL_AGENT_INTERNAL_TOKEN below).
   // We just print it on stdout for the supervising process to pick up if it's
   // not already in env. Defense against env races at spawn time.
   console.log(`[terminal-agent] listening on 127.0.0.1:${port} pid=${process.pid}`);
 
-  // Cleanup port file on exit.
-  const cleanup = () => { safeUnlink(PORT_FILE); process.exit(0); };
-  process.on('SIGTERM', cleanup);
-  process.on('SIGINT', cleanup);
+  // Cleanup state files on exit. `safeUnlinkQuiet` because per CLAUDE.md
+  // shutdown paths must swallow all errors so a single throw doesn't skip
+  // subsequent unlinks. Register on `exit` (sync handler — no process.exit
+  // inside!) so hard-killed agents (Windows taskkill /T /F skips signal
+  // handlers) still clean up state files. SIGTERM/SIGINT call cleanup
+  // explicitly because process.exit(0) won't fire 'exit' synchronously
+  // before the handler returns; the explicit calls cover the graceful path.
+  const cleanup = () => {
+    safeUnlinkQuiet(PORT_FILE);
+    safeUnlinkQuiet(PID_FILE);
+  };
+  process.on('exit', cleanup);
+  process.on('SIGTERM', () => { cleanup(); process.exit(0); });
+  process.on('SIGINT', () => { cleanup(); process.exit(0); });
 }
 
 // Export the internal token so cli.ts can pass the SAME value to the parent
diff --git a/browse/test/terminal-agent-integration.test.ts b/browse/test/terminal-agent-integration.test.ts
@@ -18,10 +18,15 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
+import { IS_WINDOWS } from '../src/platform';
 
 const AGENT_SCRIPT = path.join(import.meta.dir, '../src/terminal-agent.ts');
 const BASH = '/bin/bash';
 
+// /bin/bash isn't available on Windows; the agent's PTY layer is also a
+// different path (bun-pty vs Bun.spawn({terminal:})). Skip this whole suite
+// on Windows — CI is Ubuntu-only so it doesn't change coverage.
+
 let stateDir: string;
 let agentProc: any;
 let agentPort: number;
@@ -50,6 +55,7 @@ function readTokenFile(): string {
 }
 
 beforeAll(() => {
+  if (IS_WINDOWS) return;
   stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-term-'));
   const stateFile = path.join(stateDir, 'browse.json');
   // browse.json must exist so the agent's readBrowseToken doesn't throw.
@@ -83,7 +89,7 @@ async function grantToken(token: string): Promise<Response> {
   });
 }
 
-describe('terminal-agent: /internal/grant', () => {
+describe.skipIf(IS_WINDOWS)('terminal-agent: /internal/grant', () => {
   test('accepts grants signed with the internal token', async () => {
     const resp = await grantToken('test-cookie-token-very-long-yes');
     expect(resp.status).toBe(200);
@@ -102,7 +108,7 @@ describe('terminal-agent: /internal/grant', () => {
   });
 });
 
-describe('terminal-agent: /ws gates', () => {
+describe.skipIf(IS_WINDOWS)('terminal-agent: /ws gates', () => {
   test('rejects upgrade attempts without an extension Origin', async () => {
     const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`);
     expect(resp.status).toBe(403);
@@ -127,7 +133,7 @@ describe('terminal-agent: /ws gates', () => {
   });
 });
 
-describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () => {
+describe.skipIf(IS_WINDOWS)('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () => {
   test('binary writes go to PTY stdin, output streams back', async () => {
     const cookie = 'rt-token-must-be-at-least-seventeen-chars-long';
     const granted = await grantToken(cookie);
diff --git a/bun.lock b/bun.lock
diff --git a/package.json b/package.json
@@ -43,6 +43,7 @@
   "dependencies": {
     "@huggingface/transformers": "^4.1.0",
     "@ngrok/ngrok": "^1.7.0",
+    "bun-pty": "^0.4.8",
     "diff": "^7.0.0",
     "marked": "^18.0.2",
     "playwright": "^1.58.2",
