From a29743b0566406ceb536e61aec3de24e3a8b5acc Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 12 Mar 2026 22:35:20 +0800 Subject: [PATCH] fix: harden browse install and lifecycle checks (#4) Thanks @morluto --- browse/src/cli.ts | 46 +++++++++++++++++++----- browse/test/commands.test.ts | 68 ++++++++++++++++++++++++++++-------- setup | 29 ++++++++++++--- 3 files changed, 115 insertions(+), 28 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 8a7c4dca9fdf26f61ec185dc80327599eaf33359..931b85c10e89754c03490067f15f6073e757cad6 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -18,13 +18,39 @@ const BROWSE_PORT = process.env.CONDUCTOR_PORT : parseInt(process.env.BROWSE_PORT || '0', 10); const INSTANCE_SUFFIX = BROWSE_PORT ? `-${BROWSE_PORT}` : ''; const STATE_FILE = process.env.BROWSE_STATE_FILE || `/tmp/browse-server${INSTANCE_SUFFIX}.json`; -// When compiled, import.meta.dir is virtual. Use env var or well-known path. -const SERVER_SCRIPT = process.env.BROWSE_SERVER_SCRIPT - || (import.meta.dir.startsWith('/') && !import.meta.dir.includes('$bunfs') - ? path.resolve(import.meta.dir, 'server.ts') - : path.resolve(process.env.HOME || '/tmp', '.claude/skills/gstack/browse/src/server.ts')); const MAX_START_WAIT = 8000; // 8 seconds to start +export function resolveServerScript( + env: Record = process.env, + metaDir: string = import.meta.dir, + execPath: string = process.execPath +): string { + if (env.BROWSE_SERVER_SCRIPT) { + return env.BROWSE_SERVER_SCRIPT; + } + + // Dev mode: cli.ts runs directly from browse/src + if (metaDir.startsWith('/') && !metaDir.includes('$bunfs')) { + const direct = path.resolve(metaDir, 'server.ts'); + if (fs.existsSync(direct)) { + return direct; + } + } + + // Compiled binary: derive the source tree from browse/dist/browse + if (execPath) { + const adjacent = path.resolve(path.dirname(execPath), '..', 'src', 'server.ts'); + if (fs.existsSync(adjacent)) { + return adjacent; + } + } + + // Legacy fallback for user-level installs + return path.resolve(env.HOME || '/tmp', '.claude/skills/gstack/browse/src/server.ts'); +} + +const SERVER_SCRIPT = resolveServerScript(); + interface ServerState { pid: number; port: number; @@ -215,7 +241,9 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: await sendCommand(state, command, commandArgs); } -main().catch((err) => { - console.error(`[browse] ${err.message}`); - process.exit(1); -}); +if (import.meta.main) { + main().catch((err) => { + console.error(`[browse] ${err.message}`); + process.exit(1); + }); +} diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 151e943ab9db5a00fdc3c1ad5045ddc521b0edef..0d572bb6449243f3b00685d153c34a69601bcd0b 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -8,6 +8,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { startTestServer } from './test-server'; import { BrowserManager } from '../src/browser-manager'; +import { resolveServerScript } from '../src/cli'; import { handleReadCommand } from '../src/read-commands'; import { handleWriteCommand } from '../src/write-commands'; import { handleMetaCommand } from '../src/meta-commands'; @@ -420,33 +421,70 @@ describe('Status', () => { }); }); -// ─── CLI retry guard ──────────────────────────────────────────── +// ─── CLI server script resolution ─────────────────────────────── -describe('CLI retry guard', () => { - test('sendCommand aborts after repeated connection failures', async () => { - // Write a fake state file pointing to a port that refuses connections - const stateFile = '/tmp/browse-server.json'; - const origState = fs.existsSync(stateFile) ? fs.readFileSync(stateFile, 'utf-8') : null; +describe('CLI server script resolution', () => { + test('prefers adjacent browse/src/server.ts for compiled project installs', () => { + const root = fs.mkdtempSync('/tmp/gstack-cli-'); + const execPath = path.join(root, '.claude/skills/gstack/browse/dist/browse'); + const serverPath = path.join(root, '.claude/skills/gstack/browse/src/server.ts'); - fs.writeFileSync(stateFile, JSON.stringify({ port: 1, token: 'fake', pid: 999999 })); + fs.mkdirSync(path.dirname(execPath), { recursive: true }); + fs.mkdirSync(path.dirname(serverPath), { recursive: true }); + fs.writeFileSync(serverPath, '// test server\n'); + + const resolved = resolveServerScript( + { HOME: path.join(root, 'empty-home') }, + '$bunfs/root', + execPath + ); + + expect(resolved).toBe(serverPath); + + fs.rmSync(root, { recursive: true, force: true }); + }); +}); + +// ─── CLI lifecycle ────────────────────────────────────────────── + +describe('CLI lifecycle', () => { + test('dead state file triggers a clean restart', async () => { + const stateFile = `/tmp/browse-test-state-${Date.now()}.json`; + fs.writeFileSync(stateFile, JSON.stringify({ + port: 1, + token: 'fake', + pid: 999999, + })); const cliPath = path.resolve(__dirname, '../src/cli.ts'); - const result = await new Promise<{ code: number; stderr: string }>((resolve) => { + const result = await new Promise<{ code: number; stdout: string; stderr: string }>((resolve) => { const proc = spawn('bun', ['run', cliPath, 'status'], { timeout: 15000, - env: { ...process.env }, + env: { + ...process.env, + BROWSE_STATE_FILE: stateFile, + BROWSE_PORT_START: '9520', + }, }); + let stdout = ''; let stderr = ''; + proc.stdout.on('data', (d) => stdout += d.toString()); proc.stderr.on('data', (d) => stderr += d.toString()); - proc.on('close', (code) => resolve({ code: code ?? 1, stderr })); + proc.on('close', (code) => resolve({ code: code ?? 1, stdout, stderr })); }); - // Restore original state file - if (origState) fs.writeFileSync(stateFile, origState); - else if (fs.existsSync(stateFile)) fs.unlinkSync(stateFile); + let restartedPid: number | null = null; + if (fs.existsSync(stateFile)) { + restartedPid = JSON.parse(fs.readFileSync(stateFile, 'utf-8')).pid; + fs.unlinkSync(stateFile); + } + if (restartedPid) { + try { process.kill(restartedPid, 'SIGTERM'); } catch {} + } - // Should fail, not loop forever - expect(result.code).not.toBe(0); + expect(result.code).toBe(0); + expect(result.stdout).toContain('Status: healthy'); + expect(result.stderr).toContain('Starting server'); }, 20000); }); diff --git a/setup b/setup index 81ec2d669d1c84c281505adc8a271f81c167135b..73c6503e092a2ae50fc3e17bbad1a1ceaf695be7 100755 --- a/setup +++ b/setup @@ -4,11 +4,32 @@ set -e GSTACK_DIR="$(cd "$(dirname "$0")" && pwd)" SKILLS_DIR="$(dirname "$GSTACK_DIR")" +BROWSE_BIN="$GSTACK_DIR/browse/dist/browse" # 1. Build browse binary if needed -if [ ! -x "$GSTACK_DIR/browse/dist/browse" ]; then +NEEDS_BUILD=0 +if [ ! -x "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +elif [ -n "$(find "$GSTACK_DIR/browse/src" -type f -newer "$BROWSE_BIN" -print -quit 2>/dev/null)" ]; then + NEEDS_BUILD=1 +elif [ "$GSTACK_DIR/package.json" -nt "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +elif [ -f "$GSTACK_DIR/bun.lock" ] && [ "$GSTACK_DIR/bun.lock" -nt "$BROWSE_BIN" ]; then + NEEDS_BUILD=1 +fi + +if [ "$NEEDS_BUILD" -eq 1 ]; then echo "Building browse binary..." - cd "$GSTACK_DIR" && bun install && bun run build + ( + cd "$GSTACK_DIR" + bun install + bun run build + ) +fi + +if [ ! -x "$BROWSE_BIN" ]; then + echo "gstack setup failed: browse binary missing at $BROWSE_BIN" >&2 + exit 1 fi # 2. Only create skill symlinks if we're inside a .claude/skills directory @@ -30,12 +51,12 @@ if [ "$SKILLS_BASENAME" = "skills" ]; then done echo "gstack ready." - echo " browse: $GSTACK_DIR/browse/dist/browse" + echo " browse: $BROWSE_BIN" if [ ${#linked[@]} -gt 0 ]; then echo " linked skills: ${linked[*]}" fi else echo "gstack ready." - echo " browse: $GSTACK_DIR/browse/dist/browse" + echo " browse: $BROWSE_BIN" echo " (skipped skill symlinks — not inside .claude/skills/)" fi