test(browse): bun-version-agnostic hooks, hermetic env, cross-file state restore

- commands.test.ts: describe-scoped beforeAll(goto) hooks become
  beforeEach — bun 1.2.x runs describe-scoped beforeAll eagerly at file
  start in file order, so the three navigations clobbered each other and
  cascaded into 55 wrong-page / dead-session failures; beforeEach is
  correctly scoped on all bun versions.
- content-security.test.ts: restore the module-load urlBlocklistFilter
  after the content-filter describe — the module instance is shared
  across files, and leaving the registry empty broke
  security-integration.test.ts downstream.
- watchdog.test.ts: hardcoded ports 34901-34903 → BROWSE_PORT=0 (random),
  ending EADDRINUSE collisions with sibling worktree runs.
- gstack-update-check.test.ts: pin GSTACK_HOME/GSTACK_STATE_ROOT to the
  test's state dir so leaked env from other files can't redirect config.
- terminal-agent(.integration).test.ts: re-pin static-grep anchors to the
  current maybeSpawnPty/canDispatchOverTunnel source; feature-detected
  skip for Bun.spawn PTY support (absent on bun 1.2.x); success-path
  WebSocket handshake via raw TCP (fetch() never resolves 101 upgrades
  on bun 1.2.x).
- dual-listener.test.ts: update pinned literal to the args-aware
  canDispatchOverTunnel(body?.command, body?.args) form.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
t 2026-07-10 15:22:30 -07:00
parent 514e529408
commit 25f4e1a649
7 changed files with 135 additions and 37 deletions

View File

@ -5,7 +5,7 @@
* A real browse server is started and commands are sent via the CLI HTTP interface.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { describe, test, expect, beforeAll, beforeEach, afterAll } from 'bun:test';
import { startTestServer } from './test-server';
import { BrowserManager } from '../src/browser-manager';
import { resolveServerScript } from '../src/cli';
@ -138,7 +138,12 @@ describe('Navigation', () => {
// ─── Content Extraction ─────────────────────────────────────────
describe('Content extraction', () => {
beforeAll(async () => {
// beforeEach, NOT beforeAll: bun <1.3 runs every describe-scoped beforeAll
// eagerly at file start (in file order), so a beforeAll goto here is
// clobbered by later describes' beforeAll gotos before any test runs.
// beforeEach is scoped correctly on all bun versions, and a goto against
// the local fixture server costs only a few ms per test.
beforeEach(async () => {
await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm);
});
@ -196,7 +201,9 @@ describe('Content extraction', () => {
// ─── JavaScript / CSS / Attrs ───────────────────────────────────
describe('Inspection', () => {
beforeAll(async () => {
// beforeEach, NOT beforeAll — see 'Content extraction' note (bun <1.3
// runs describe-scoped beforeAll hooks eagerly at file start).
beforeEach(async () => {
await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm);
});
@ -1074,7 +1081,9 @@ describe('Dialog handling', () => {
// ─── Element State Checks (is) ─────────────────────────────────
describe('Element state checks', () => {
beforeAll(async () => {
// beforeEach, NOT beforeAll — see 'Content extraction' note (bun <1.3
// runs describe-scoped beforeAll hooks eagerly at file start).
beforeEach(async () => {
await handleWriteCommand('goto', [baseUrl + '/states.html'], bm);
});

View File

@ -124,6 +124,17 @@ describe('Content filter hooks', () => {
clearContentFilters();
});
// Restore module-load state when this describe is done. bun test runs
// every file in one process, so the content-security module instance is
// SHARED across test files — leaving the registry empty here breaks
// security-integration.test.ts, whose pipeline test pins that the
// built-in urlBlocklistFilter is registered (content-security.ts
// registers it at module load).
afterAll(() => {
clearContentFilters();
registerContentFilter(urlBlocklistFilter);
});
test('URL blocklist detects requestbin', () => {
const result = urlBlocklistFilter('', 'https://requestbin.com/r/abc', 'text');
expect(result.safe).toBe(false);

View File

@ -220,7 +220,9 @@ describe('/command tunnel command allowlist', () => {
'return handleCommand(body, tokenInfo)'
);
expect(commandBlock).toContain("surface === 'tunnel'");
expect(commandBlock).toContain('canDispatchOverTunnel(body?.command)');
// Args-aware since the --out (disk write) tunnel ban: the dispatch gate
// takes both the command and its args.
expect(commandBlock).toContain('canDispatchOverTunnel(body?.command, body?.args)');
expect(commandBlock).toContain('disallowed_command');
expect(commandBlock).toContain('is not allowed over the tunnel surface');
expect(commandBlock).toContain('status: 403');

View File

@ -22,6 +22,14 @@ function run(extraEnv: Record<string, string> = {}, args: string[] = []) {
...process.env,
GSTACK_DIR: gstackDir,
GSTACK_STATE_DIR: stateDir,
// gstack-config resolves its state dir as GSTACK_STATE_ROOT >
// GSTACK_HOME > GSTACK_STATE_DIR. Other test files in the same bun
// process set process.env.GSTACK_HOME mid-suite (and operator shells
// may export it too); a leaked value would win over GSTACK_STATE_DIR
// above and point gstack-config at the wrong config.yaml, making the
// update_check config tests order-dependent. Pin all three.
GSTACK_HOME: stateDir,
GSTACK_STATE_ROOT: stateDir,
GSTACK_REMOTE_URL: `file://${join(gstackDir, 'REMOTE_VERSION')}`,
...extraEnv,
},

View File

@ -22,6 +22,26 @@ import * as os from 'os';
const AGENT_SCRIPT = path.join(import.meta.dir, '../src/terminal-agent.ts');
const BASH = '/bin/bash';
// Bun.spawn PTY support (the `terminal` spawn option) is required for the
// agent to stream PTY output. On older Bun (< 1.3) the option is silently
// ignored — proc.terminal is undefined, terminal.write() no-ops, and the
// round-trip can never produce output. Feature-detect instead of version-
// sniffing, and SKIP (not fake-pass) the output-dependent test on runtimes
// that genuinely lack the capability. All auth/handshake/control-frame
// tests still run everywhere — they don't need a live PTY.
const BUN_HAS_PTY = (() => {
try {
const probe = (Bun as any).spawn(['/bin/sh', '-c', 'exit 0'], {
terminal: { rows: 2, cols: 2, data() {} },
});
const has = !!probe.terminal;
try { probe.kill(); } catch {}
return has;
} catch {
return false;
}
})();
let stateDir: string;
let agentProc: any;
let agentPort: number;
@ -128,7 +148,11 @@ describe('terminal-agent: /ws gates', () => {
});
describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () => {
test('binary writes go to PTY stdin, output streams back', async () => {
// Skipped when the Bun runtime lacks PTY spawn support (see BUN_HAS_PTY).
// Explicit 15s timeout: the test legitimately waits up to 5s for the WS
// open plus up to 5s for PTY output, which overflows bun's 5s default and
// leaves stale assertions that get mis-attributed to the NEXT test.
test.skipIf(!BUN_HAS_PTY)('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);
expect(granted.status).toBe(200);
@ -180,7 +204,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () =
try { ws.close(); } catch {}
// Give cleanup a moment.
await Bun.sleep(200);
});
}, 15000);
test('Sec-WebSocket-Protocol auth path: browser-style upgrade with token in protocol', async () => {
// This is the path the actual browser extension takes. Cross-port
@ -199,32 +223,57 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () =
const token = 'sec-protocol-token-must-be-at-least-seventeen-chars';
await grantToken(token);
// We exercise the protocol path by raw-handshaking via fetch+Upgrade,
// because Bun's test-client WebSocket constructor doesn't propagate
// We exercise the protocol path with a raw TCP handshake, because
// Bun's test-client WebSocket constructor doesn't propagate
// `protocols` cleanly when also passed `headers` (the constructor
// detects the third-arg form unreliably). Real browsers (Chromium)
// use the standard protocols arg fine — the server-side handler is
// identical either way, so this test still locks the load-bearing
// invariant: the agent accepts a token via Sec-WebSocket-Protocol
// and echoes the protocol back so a browser would accept the upgrade.
// detects the third-arg form unreliably), and fetch()+Upgrade headers
// never resolves for a 101 response on Bun < 1.3 (it only completes
// for ordinary statuses, so the success path hangs). Real browsers
// (Chromium) use the standard protocols arg fine — the server-side
// handler is identical either way, so this test still locks the
// load-bearing invariant: the agent accepts a token via
// Sec-WebSocket-Protocol and echoes the protocol back so a browser
// would accept the upgrade.
const handshakeKey = 'dGhlIHNhbXBsZSBub25jZQ==';
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, {
headers: {
'Connection': 'Upgrade',
'Upgrade': 'websocket',
'Sec-WebSocket-Version': '13',
'Sec-WebSocket-Key': handshakeKey,
'Sec-WebSocket-Protocol': `gstack-pty.${token}`,
'Origin': 'chrome-extension://test-extension-id',
},
const requestHead = [
'GET /ws HTTP/1.1',
`Host: 127.0.0.1:${agentPort}`,
'Connection: Upgrade',
'Upgrade: websocket',
'Sec-WebSocket-Version: 13',
`Sec-WebSocket-Key: ${handshakeKey}`,
`Sec-WebSocket-Protocol: gstack-pty.${token}`,
'Origin: chrome-extension://test-extension-id',
'',
'',
].join('\r\n');
const net = await import('node:net');
const responseHead = await new Promise<string>((resolve, reject) => {
const chunks: Buffer[] = [];
const socket = net.connect(agentPort, '127.0.0.1', () => socket.write(requestHead));
const finish = () => {
clearTimeout(timer);
socket.destroy();
resolve(Buffer.concat(chunks).toString('utf8'));
};
const timer = setTimeout(finish, 3000);
socket.on('data', (d: Buffer) => {
chunks.push(d);
// Response head complete once we see the blank line.
if (Buffer.concat(chunks).toString('utf8').includes('\r\n\r\n')) finish();
});
socket.on('error', (e: Error) => { clearTimeout(timer); reject(e); });
socket.on('close', finish);
});
// 101 Switching Protocols + protocol echoed back = browser would accept.
// 401/403/anything else = browser would close the connection immediately
// (the bug we hit in manual dogfood).
expect(resp.status).toBe(101);
expect(resp.headers.get('upgrade')?.toLowerCase()).toBe('websocket');
expect(resp.headers.get('sec-websocket-protocol')).toBe(`gstack-pty.${token}`);
const statusLine = responseHead.split('\r\n')[0] || '';
expect(statusLine).toContain('101');
expect(responseHead.toLowerCase()).toContain('upgrade: websocket');
expect(responseHead.toLowerCase()).toContain(`sec-websocket-protocol: gstack-pty.${token}`.toLowerCase());
});
test('Sec-WebSocket-Protocol auth: rejects unknown token even with valid Origin', async () => {

View File

@ -144,17 +144,33 @@ describe('Source-level guard: terminal-agent', () => {
test('lazy spawn: claude PTY is spawned in message handler, not on upgrade', () => {
// The whole point of lazy-spawn (codex finding #8) is that the WS
// upgrade itself does NOT call spawnClaude. Spawn happens on first
// message frame.
// upgrade itself does NOT spawn claude. Spawn happens on first
// message frame (binary input or the v1.44 explicit `start` frame),
// routed through the maybeSpawnPty helper, which is the only caller
// of spawnClaude.
const upgradeBlock = AGENT_SRC.slice(
AGENT_SRC.indexOf("if (url.pathname === '/ws')"),
AGENT_SRC.indexOf("websocket: {"),
);
expect(upgradeBlock).not.toContain('spawnClaude(');
expect(upgradeBlock).not.toContain('maybeSpawnPty(');
// Spawn must be invoked from the message handler (lazy on first byte).
const messageHandler = AGENT_SRC.slice(AGENT_SRC.indexOf('message(ws, raw)'));
expect(messageHandler).toContain('spawnClaude(');
expect(messageHandler).toContain('maybeSpawnPty(');
expect(messageHandler).toContain('!session.spawned');
// The open() upgrade handler must not spawn — it only creates the
// (spawned: false) session record or re-attaches a detached one.
const openBlock = AGENT_SRC.slice(
AGENT_SRC.indexOf('open(ws)'),
AGENT_SRC.indexOf('message(ws, raw)'),
);
expect(openBlock).not.toContain('spawnClaude(');
expect(openBlock).not.toContain('maybeSpawnPty(');
// And the helper itself is where spawnClaude actually happens, gated
// on session.spawned so it stays a single-shot lazy spawn.
const helperBlock = AGENT_SRC.slice(AGENT_SRC.indexOf('function maybeSpawnPty'));
expect(helperBlock).toContain('spawnClaude(');
expect(helperBlock).toContain('if (session.spawned) return true;');
});
test('process.on uncaughtException + unhandledRejection handlers exist', () => {

View File

@ -46,13 +46,19 @@ afterEach(async () => {
serverProc = null;
});
function spawnServer(env: Record<string, string>, port: number): Subprocess {
// No fixed BROWSE_PORT: these tests never dial the server's HTTP port —
// they only watch stdout markers and process liveness. BROWSE_PORT=0 lets
// the server pick a random free port (its default), so concurrent runs of
// this file (sibling worktrees, pr-test-env checkouts) and leftover
// servers from interrupted runs can't collide on a hardcoded port and
// kill the fresh spawn with EADDRINUSE before the 2s liveness check.
function spawnServer(env: Record<string, string>): Subprocess {
const stateFile = path.join(tmpDir, 'browse-state.json');
return spawn(['bun', 'run', SERVER_SCRIPT], {
env: {
...process.env,
BROWSE_STATE_FILE: stateFile,
BROWSE_PORT: String(port),
BROWSE_PORT: '0',
...env,
},
stdio: ['ignore', 'pipe', 'pipe'],
@ -97,7 +103,7 @@ async function readStdoutUntil(
describe('parent-process watchdog (v0.18.1.0)', () => {
test('BROWSE_PARENT_PID=0 disables the watchdog', async () => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'watchdog-pid0-'));
serverProc = spawnServer({ BROWSE_PARENT_PID: '0' }, 34901);
serverProc = spawnServer({ BROWSE_PARENT_PID: '0' });
const out = await readStdoutUntil(
serverProc,
@ -115,10 +121,7 @@ describe('parent-process watchdog (v0.18.1.0)', () => {
// Pass a bogus parent PID to prove BROWSE_HEADED takes precedence.
// If the server-side guard regresses, the watchdog would try to poll
// this PID and eventually fire on the "dead parent."
serverProc = spawnServer(
{ BROWSE_HEADED: '1', BROWSE_PARENT_PID: '999999' },
34902,
);
serverProc = spawnServer({ BROWSE_HEADED: '1', BROWSE_PARENT_PID: '999999' });
const out = await readStdoutUntil(
serverProc,
@ -137,7 +140,7 @@ describe('parent-process watchdog (v0.18.1.0)', () => {
const parentPid = parentProc.pid!;
// Default headless: no BROWSE_HEADED, real parent PID — watchdog active.
serverProc = spawnServer({ BROWSE_PARENT_PID: String(parentPid) }, 34903);
serverProc = spawnServer({ BROWSE_PARENT_PID: String(parentPid) });
const serverPid = serverProc.pid!;
// Give the server a moment to start and register the watchdog interval.