From af9c5f857dd60ce4766c5aecfbd3eac7d9e9684a Mon Sep 17 00:00:00 2001 From: StreamDemon Date: Mon, 27 Apr 2026 15:16:53 +0800 Subject: [PATCH] fix(browse): make sidebar Terminal pane work on Windows 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 /.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) --- browse/src/cli.ts | 108 +++++++++++++++--- browse/src/terminal-agent.ts | 107 ++++++++++++++++- .../test/terminal-agent-integration.test.ts | 12 +- bun.lock | 3 + package.json | 1 + 5 files changed, 207 insertions(+), 24 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 9c4881a25..c0eb6eb40 100644 --- 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 index 9ebc8cbbf..f834751fd 100644 --- 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((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 index cdcbe8de5..bc8bdabdc 100644 --- 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 { }); } -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 index 4fb0dfaef..b50ce5ed4 100644 --- a/bun.lock +++ b/bun.lock @@ -7,6 +7,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", @@ -199,6 +200,8 @@ "buffer-crc32": ["buffer-crc32@0.2.13", "", {}, "sha512-VO9Ht/+p3SN7SKWqcrgEzjGbRSJYTx+Q1pTQC0wrWqHx0vpJraQ6GtHx8tvcg1rlK1byhU5gccxgOgj7B0TDkQ=="], + "bun-pty": ["bun-pty@0.4.8", "", {}, "sha512-rO70Mrbr13+jxHHHu2YBkk2pNqrJE5cJn29WE++PUr+GFA0hq/VgtQPZANJ8dJo6d7XImvBk37Innt8GM7O28w=="], + "bytes": ["bytes@3.1.2", "", {}, "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg=="], "call-bind-apply-helpers": ["call-bind-apply-helpers@1.0.2", "", { "dependencies": { "es-errors": "^1.3.0", "function-bind": "^1.1.2" } }, "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ=="], diff --git a/package.json b/package.json index ba1b4f8f8..74d81f1b5 100644 --- 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",