Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
StreamDemon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 92 additions & 16 deletions browse/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
107 changes: 102 additions & 5 deletions browse/src/terminal-agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions browse/test/terminal-agent-integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down