mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-14 00:49:00 +02:00
Merge origin/main (v1.65.0.0 fork port wave 2) into test-evals-ci-speedup
Second overlapping-wave merge; resolutions compose intent: - TEST_ROOTS: ours is the superset (main also wired ios-qa/daemon/test; ours additionally has ios-qa/scripts + browser-skills). package.json 'test' keeps routing through the canonical strict runner. - gbrainAvailable: main fixed the same load-flake with a strictly better mechanism (memoized stat-based PATH scan, no subprocess at all) — theirs supersedes this branch's memoized-exec probe. Main also made the query timeout env-overridable (GSTACK_BRAIN_TIMEOUT_MS). - Model defaults: adopted main's lib/eval-model.ts abstraction (one resolution point, env-overridable per kind) and applied decision D1a inside it: capture defaults to Sonnet (Opus opt-in via explicit arg or GSTACK_EVAL_MODEL_CAPTURE); test pins updated to follow. - Parent watchdog: main's rewrite (named parameterized tick, driven deterministically by its test via __testInternals__, plus handoff suppression semantics from session persistence) supersedes this branch's env-tunable interval; adopted their server + test wholesale. - windows-free-tests: ours (curated bun run test:windows) — main's hand-list grew by one more file, which the curated runner subsumes automatically; that drift is the reason for D11. - context-skills 0-for-26 fix: both waves made the IDENTICAL fix; kept this branch's comment (carries the receipts). - .gitignore: main's superset (also ignores Package.resolved — their never-commit call; untracked the copy this branch had committed). Verified: 239-test merge battery green, watchdog 8/8, eval-model 5/5, actionlint clean, eval:select works. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,11 +1,13 @@
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import { describe, test, expect, afterAll, setDefaultTimeout } from 'bun:test';
|
||||
import * as path from 'path';
|
||||
|
||||
// Every test here spawnSync's a `node` child; Windows CI cold-start (AV scan,
|
||||
// first-touch of node.exe) alone can blow bun's 5s default — observed 5,007ms
|
||||
// on a 50ms sleep test. Subprocess budget, not assertion looseness.
|
||||
setDefaultTimeout(20_000);
|
||||
|
||||
// Load the polyfill into a fresh object (don't clobber globalThis.Bun)
|
||||
const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs');
|
||||
// Forward slashes so the path survives interpolation into a JS string literal
|
||||
// on Windows, which is the platform this polyfill exists for.
|
||||
const requirePath = polyfillPath.replace(/\\/g, '/');
|
||||
|
||||
describe('bun-polyfill', () => {
|
||||
// We test the polyfill by requiring it in a subprocess under Node.js
|
||||
@@ -13,7 +15,7 @@ describe('bun-polyfill', () => {
|
||||
|
||||
test('Bun.sleep resolves after delay', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
(async () => {
|
||||
const start = Date.now();
|
||||
await Bun.sleep(50);
|
||||
@@ -27,7 +29,7 @@ describe('bun-polyfill', () => {
|
||||
|
||||
test('Bun.spawnSync runs a command and returns stdout', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
const r = Bun.spawnSync(['echo', 'hello'], { stdout: 'pipe' });
|
||||
console.log(r.stdout.toString().trim());
|
||||
console.log('exit:' + r.exitCode);
|
||||
@@ -39,7 +41,7 @@ describe('bun-polyfill', () => {
|
||||
|
||||
test('Bun.spawn launches a process with pid', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
const p = Bun.spawn(['echo', 'test'], { stdio: ['pipe', 'pipe', 'pipe'] });
|
||||
console.log(typeof p.pid === 'number' ? 'HAS_PID' : 'NO_PID');
|
||||
console.log(typeof p.kill === 'function' ? 'HAS_KILL' : 'NO_KILL');
|
||||
@@ -51,179 +53,9 @@ describe('bun-polyfill', () => {
|
||||
expect(lines[2]).toBe('HAS_UNREF');
|
||||
});
|
||||
|
||||
// Bun.spawn parity: `proc.exited` is a Promise resolving to the exit code.
|
||||
// The DPAPI helper and isBrowserRunning both `await proc.exited`; without
|
||||
// it the awaits resolve immediately to `undefined` and the caller reads
|
||||
// stdout before the child has produced it — surfacing as a silent failure.
|
||||
test('Bun.spawn exposes proc.exited that resolves to the exit code', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
const p = Bun.spawn(['node', '-e', 'process.exit(0)'], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
console.log(typeof p.exited === 'object' && typeof p.exited.then === 'function' ? 'IS_PROMISE' : 'NOT_PROMISE');
|
||||
console.log('exit:' + await p.exited);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
const lines = result.stdout.toString().trim().split('\n');
|
||||
expect(lines[0]).toBe('IS_PROMISE');
|
||||
expect(lines[1]).toBe('exit:0');
|
||||
});
|
||||
|
||||
test('Bun.spawn proc.exited reflects non-zero exit codes', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
const p = Bun.spawn(['node', '-e', 'process.exit(3)'], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
console.log('exit:' + await p.exited);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('exit:3');
|
||||
});
|
||||
|
||||
test('Bun.spawn proc.exited resolves before reading stdout (no race)', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
// Real-world pattern: write to stdout, then exit. Awaiting proc.exited
|
||||
// before reading must guarantee the bytes are flushed.
|
||||
const p = Bun.spawn(['node', '-e', 'process.stdout.write("ready"); process.exit(0)'], {
|
||||
stdio: ['ignore', 'pipe', 'ignore']
|
||||
});
|
||||
const code = await p.exited;
|
||||
const out = await new Response(p.stdout).text();
|
||||
console.log(out + ':' + code);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('ready:0');
|
||||
});
|
||||
|
||||
// Spawn-failure case: Node emits 'error' but not 'exit' when the binary
|
||||
// is missing, so listening only for 'exit' hangs `await proc.exited`
|
||||
// forever. The lifecycle promise must resolve on either event.
|
||||
test('Bun.spawn proc.exited resolves on spawn failure (missing binary)', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
const p = Bun.spawn(['this-binary-does-not-exist-zzz-' + Date.now()], {
|
||||
stdio: ['ignore', 'pipe', 'pipe']
|
||||
});
|
||||
const code = await Promise.race([
|
||||
p.exited,
|
||||
new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000))
|
||||
]).catch(() => 'TIMEOUT');
|
||||
console.log('exit:' + code);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
// Anything other than 'TIMEOUT' (and ideally a non-zero number) means the
|
||||
// lifecycle promise resolved on the spawn error.
|
||||
const out = result.stdout.toString().trim();
|
||||
expect(out).not.toBe('exit:TIMEOUT');
|
||||
expect(out).toMatch(/^exit:\d+$/);
|
||||
});
|
||||
|
||||
// GSTACK_SPAWN_MAX_BUFFER caps the drain so a runaway child can't OOM the
|
||||
// server. Past the cap, the pipe keeps flowing (child doesn't block) but
|
||||
// further bytes are dropped. Set a small cap, write more than that, assert
|
||||
// the captured stdout equals the cap and the child exits cleanly.
|
||||
test('Bun.spawn caps buffered output at GSTACK_SPAWN_MAX_BUFFER', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
process.env.GSTACK_SPAWN_MAX_BUFFER = '${1024}';
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
// Child writes 10 KB; cap is 1 KB; drained output should be exactly 1 KB
|
||||
// and exit should still resolve cleanly (child not back-pressured to death).
|
||||
const p = Bun.spawn(
|
||||
['node', '-e', 'process.stdout.write("y".repeat(10 * 1024)); process.exit(0)'],
|
||||
{ stdio: ['ignore', 'pipe', 'ignore'] }
|
||||
);
|
||||
const code = await Promise.race([
|
||||
p.exited,
|
||||
new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000))
|
||||
]).catch(() => 'TIMEOUT');
|
||||
const out = await new Response(p.stdout).text();
|
||||
console.log(out.length + ':' + code);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('1024:0');
|
||||
});
|
||||
|
||||
// Regression for the pipe-blocking case: if the child writes more than the
|
||||
// OS pipe buffer (~16-64 KB) and the polyfill doesn't drain eagerly, the
|
||||
// child blocks in write() and `exit` never fires. 1 MB is well past every
|
||||
// OS pipe buffer size. Pre-fix this test hangs forever; post-fix it returns
|
||||
// in <500ms. Bun's default per-test timeout is 5s — generous here.
|
||||
test('Bun.spawn drains large stdout so proc.exited still resolves', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
const ONE_MB = 1024 * 1024;
|
||||
// Exit in the write callback, not straight after write(): on modern
|
||||
// Node a pipe write past the OS buffer is async, and process.exit()
|
||||
// right after write() truncates at ~64 KB even with a live reader.
|
||||
// The callback only fires once the full MB is flushed — which still
|
||||
// requires the parent to drain, so the regression (no eager drain →
|
||||
// child blocks → timeout) is still caught.
|
||||
const p = Bun.spawn(
|
||||
['node', '-e', 'process.stdout.write("x".repeat(' + ONE_MB + '), () => process.exit(0))'],
|
||||
{ stdio: ['ignore', 'pipe', 'ignore'] }
|
||||
);
|
||||
const code = await Promise.race([
|
||||
p.exited,
|
||||
new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 10000))
|
||||
]).catch(e => 'TIMEOUT');
|
||||
const out = await new Response(p.stdout).text();
|
||||
console.log(out.length + ':' + code);
|
||||
})().catch((e) => { console.log('THREW:' + e.message); });
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('1048576:0');
|
||||
}, 15000);
|
||||
|
||||
// windowsHide is the one option where Node's default is the opposite of
|
||||
// Bun's: Node shows the child's console window, Bun hides it. Dropping it
|
||||
// in translation makes every spawned child pop a window on Windows, which
|
||||
// is the platform this whole file exists for. Both shims are covered.
|
||||
test('Bun.spawn defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('Bun.spawnSync defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawnSync;
|
||||
let seen;
|
||||
cp.spawnSync = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawnSync(['node', '-e', '']);
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('an explicit windowsHide:false is honored', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'], windowsHide: false });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:false');
|
||||
});
|
||||
|
||||
test('Bun.serve creates an HTTP server that responds', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${requirePath}');
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
const server = Bun.serve({
|
||||
port: 0, // Note: polyfill uses port directly, so we pick one
|
||||
hostname: '127.0.0.1',
|
||||
@@ -242,4 +74,48 @@ describe('bun-polyfill', () => {
|
||||
expect(lines[0]).toBe('HAS_STOP');
|
||||
expect(lines[1]).toBe('HAS_PORT');
|
||||
});
|
||||
|
||||
// windowsHide is the one option where Node's default is the opposite of
|
||||
// Bun's: Node shows the child's console window, Bun hides it. Dropping it
|
||||
// in translation makes every spawned child pop a window on Windows, which
|
||||
// is the platform this whole file exists for. Both shims are covered, and
|
||||
// an explicit windowsHide:false must survive forwarding (#2523 + #2539).
|
||||
test('Bun.spawn defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('Bun.spawnSync defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawnSync;
|
||||
let seen;
|
||||
cp.spawnSync = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
Bun.spawnSync(['node', '-e', '']);
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('an explicit windowsHide:false is honored', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require(${JSON.stringify(polyfillPath)});
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'], windowsHide: false });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:false');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* #1781 busy-vs-dead pinning test — the "recover from a busy daemon" leg of
|
||||
* the browser-lifecycle contract, previously untested.
|
||||
*
|
||||
* Wedges a fake daemon: /health answers healthy, but the FIRST POST /command
|
||||
* hard-destroys the socket (the CLI sees ECONNRESET — exactly what a
|
||||
* single-threaded daemon under beacon load looks like). The daemon "PID"
|
||||
* is a live sleep child.
|
||||
*
|
||||
* Contract under test (cli.ts sendCommand + probeHealthWithBackoff):
|
||||
* - CLI must NOT kill the live PID and must NOT restart the daemon
|
||||
* (a restart drops tab/cookie state — the original crash-loop bug).
|
||||
* - It probes /health, sees alive, and retries the SAME command against
|
||||
* the SAME daemon instance.
|
||||
*
|
||||
* Fails on pre-#1781 code (which killed + restarted on any conn error) and
|
||||
* on any regression that reorders the busy-probe before the alive check.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import { spawn, type ChildProcess } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import * as http from 'http';
|
||||
import { isProcessAlive } from '../src/error-handling';
|
||||
|
||||
const BOOT_ID = `boot-${Date.now()}`;
|
||||
|
||||
interface FakeDaemon {
|
||||
port: number;
|
||||
commandRequests: number;
|
||||
close: () => Promise<void>;
|
||||
}
|
||||
|
||||
/** /health healthy; first POST /command → socket destroy; then 200 + BOOT_ID. */
|
||||
async function startWedgedDaemon(): Promise<FakeDaemon> {
|
||||
const state = { commandRequests: 0 };
|
||||
const server = http.createServer((req, res) => {
|
||||
if (req.url === '/health') {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ status: 'healthy' }));
|
||||
return;
|
||||
}
|
||||
if (req.url === '/command' && req.method === 'POST') {
|
||||
state.commandRequests += 1;
|
||||
if (state.commandRequests === 1) {
|
||||
req.socket.destroy(); // wedged: connection dies mid-request
|
||||
return;
|
||||
}
|
||||
res.writeHead(200, { 'Content-Type': 'text/plain' });
|
||||
res.end(`RECOVERED ${BOOT_ID}`);
|
||||
return;
|
||||
}
|
||||
res.writeHead(404);
|
||||
res.end();
|
||||
});
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.once('error', reject);
|
||||
server.listen(0, '127.0.0.1', () => resolve());
|
||||
});
|
||||
const addr = server.address();
|
||||
if (!addr || typeof addr === 'string') throw new Error('fake daemon: bad address');
|
||||
return {
|
||||
port: addr.port,
|
||||
get commandRequests() { return state.commandRequests; },
|
||||
close: () => new Promise((r) => server.close(() => r())),
|
||||
};
|
||||
}
|
||||
|
||||
function runCli(args: string[], env: Record<string, string>, timeoutMs = 20_000):
|
||||
Promise<{ code: number; stdout: string; stderr: string }> {
|
||||
const cliPath = path.resolve(__dirname, '../src/cli.ts');
|
||||
return new Promise((resolve) => {
|
||||
const proc = spawn('bun', ['run', cliPath, ...args], { timeout: timeoutMs, env });
|
||||
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, stdout, stderr }));
|
||||
});
|
||||
}
|
||||
|
||||
let daemonPidChild: ChildProcess | null = null;
|
||||
afterAll(() => { daemonPidChild?.kill('SIGKILL'); });
|
||||
|
||||
describe('#1781 busy-daemon recovery (CLI integration)', () => {
|
||||
test('retries without kill; same daemon instance, state file untouched', async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-busy-'));
|
||||
const stateFile = path.join(tmpDir, 'browse.json');
|
||||
const daemon = await startWedgedDaemon();
|
||||
try {
|
||||
// A live process standing in for the daemon PID. If the CLI takes the
|
||||
// dead path it SIGTERMs this child — the aliveness assert catches it.
|
||||
daemonPidChild = spawn('sleep', ['60'], { stdio: 'ignore' });
|
||||
const daemonPid = daemonPidChild.pid!;
|
||||
|
||||
const stateContent = {
|
||||
pid: daemonPid,
|
||||
port: daemon.port,
|
||||
token: 'busy-test-token',
|
||||
startedAt: new Date().toISOString(),
|
||||
serverPath: '',
|
||||
mode: 'launched' as const,
|
||||
};
|
||||
fs.writeFileSync(stateFile, JSON.stringify(stateContent, null, 2));
|
||||
|
||||
const env: Record<string, string> = {};
|
||||
for (const [k, v] of Object.entries(process.env)) {
|
||||
if (v !== undefined) env[k] = v;
|
||||
}
|
||||
env.BROWSE_STATE_FILE = stateFile;
|
||||
|
||||
const result = await runCli(['status'], env);
|
||||
|
||||
// Recovered: retried the same command against the same daemon instance.
|
||||
expect(result.code).toBe(0);
|
||||
expect(result.stdout).toContain(`RECOVERED ${BOOT_ID}`);
|
||||
// The fork's CLI announces the busy retry on stderr; ours retries at the
|
||||
// probe layer without a message. Either is fine — the load-bearing
|
||||
// behavior is retry-without-kill, asserted below.
|
||||
expect(daemon.commandRequests).toBe(2); // wedged once, served once
|
||||
|
||||
// Never killed, never restarted — tab/cookie state intact.
|
||||
expect(result.stderr).not.toContain('Restarting');
|
||||
expect(isProcessAlive(daemonPid)).toBe(true);
|
||||
expect(JSON.parse(fs.readFileSync(stateFile, 'utf-8'))).toEqual(stateContent);
|
||||
} finally {
|
||||
// Cleanup must run even when an assertion throws — otherwise a failed
|
||||
// run leaks the wedged fake daemon and the tmp dir.
|
||||
await daemon.close();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
}, 30_000);
|
||||
});
|
||||
@@ -1,8 +1,22 @@
|
||||
/**
|
||||
* #1084 diagnostics — merged-design shape.
|
||||
*
|
||||
* Main's smell wave pinned a log-and-return-null acquireServerLock; this
|
||||
* branch keeps the typed ServerLockError + bounded-retry design (fully pinned
|
||||
* in server-lock-errors.test.ts). This file re-expresses the non-redundant
|
||||
* assertion intents from the wave's test against the kept design:
|
||||
* - unexpected open failures surface the REAL errno + lock path (typed
|
||||
* throw), never phantom "another process holds the lock" contention;
|
||||
* - holder-PID read failures surface errno + lock path the same way;
|
||||
* - genuine live contention stays SILENT (null return, no stderr noise).
|
||||
* Exact duplicates of server-lock-errors.test.ts coverage (stale-lock
|
||||
* reacquire, ENOENT self-heal, EACCES throw) are deliberately not repeated.
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { acquireServerLock } from '../src/cli';
|
||||
import { acquireServerLock, ServerLockError } from '../src/cli';
|
||||
|
||||
function withTempDir<T>(fn: (dir: string) => T): T {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-lock-'));
|
||||
@@ -27,14 +41,27 @@ function captureErrors<T>(fn: () => T): { result: T; messages: string[] } {
|
||||
}
|
||||
|
||||
describe('browse CLI server lock diagnostics (#1084)', () => {
|
||||
test('logs non-EEXIST open failures instead of reporting phantom lock contention', () => {
|
||||
test('unexpected open failures throw ServerLockError with the real errno — not phantom lock contention', () => {
|
||||
if (process.platform === 'win32') return; // ENOTDIR errno mapping differs on Windows
|
||||
withTempDir((dir) => {
|
||||
const lockPath = path.join(dir, 'missing-parent', 'browse.json.lock');
|
||||
const { result, messages } = captureErrors(() => acquireServerLock(lockPath));
|
||||
// A FILE where a directory is expected: open('wx') fails ENOTDIR — an
|
||||
// errno that is neither contention (EEXIST) nor the self-healing
|
||||
// missing-dir case (ENOENT). The old code's bare catch would have
|
||||
// reported "another process holds the lock" forever.
|
||||
const blocker = path.join(dir, 'blocker');
|
||||
fs.writeFileSync(blocker, 'not a directory\n');
|
||||
const lockPath = path.join(blocker, 'browse.json.lock');
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(messages.join('\n')).toContain('unexpected ENOENT while opening');
|
||||
expect(messages.join('\n')).toContain(lockPath);
|
||||
let thrown: any = null;
|
||||
try {
|
||||
acquireServerLock(lockPath);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
}
|
||||
expect(thrown).toBeInstanceOf(ServerLockError);
|
||||
expect(thrown.code).toBe('ENOTDIR');
|
||||
expect(thrown.message).toContain('E_SERVER_LOCK (ENOTDIR)');
|
||||
expect(thrown.message).toContain(lockPath);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -50,30 +77,25 @@ describe('browse CLI server lock diagnostics (#1084)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test('logs holder PID read failures with code and lock path', () => {
|
||||
test('holder PID read failures throw ServerLockError with code and lock path', () => {
|
||||
withTempDir((dir) => {
|
||||
// Lock path exists but is a DIRECTORY: open('wx') → EEXIST (looks like
|
||||
// contention), then the holder-PID read fails EISDIR. The kept design
|
||||
// surfaces that errno + path in a typed error instead of retrying or
|
||||
// reporting phantom contention.
|
||||
const lockPath = path.join(dir, 'browse.json.lock');
|
||||
fs.mkdirSync(lockPath);
|
||||
|
||||
const { result, messages } = captureErrors(() => acquireServerLock(lockPath));
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(messages.join('\n')).toContain('unexpected EISDIR while reading holder PID from');
|
||||
expect(messages.join('\n')).toContain(lockPath);
|
||||
});
|
||||
});
|
||||
|
||||
test('removes stale lock and reacquires it', () => {
|
||||
withTempDir((dir) => {
|
||||
const lockPath = path.join(dir, 'browse.json.lock');
|
||||
fs.writeFileSync(lockPath, 'not-a-pid\n');
|
||||
|
||||
const release = acquireServerLock(lockPath);
|
||||
|
||||
expect(release).toBeFunction();
|
||||
expect(fs.readFileSync(lockPath, 'utf-8').trim()).toBe(String(process.pid));
|
||||
release?.();
|
||||
expect(fs.existsSync(lockPath)).toBe(false);
|
||||
let thrown: any = null;
|
||||
try {
|
||||
acquireServerLock(lockPath);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
}
|
||||
expect(thrown).toBeInstanceOf(ServerLockError);
|
||||
expect(thrown.code).toBe('EISDIR');
|
||||
expect(thrown.message).toContain('E_SERVER_LOCK (EISDIR)');
|
||||
expect(thrown.message).toContain(lockPath);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,7 +16,7 @@ import { handleReadCommand as _handleReadCommand } from '../src/read-commands';
|
||||
import { handleWriteCommand as _handleWriteCommand } from '../src/write-commands';
|
||||
|
||||
const handleReadCommand = (cmd: string, args: string[], b: BrowserManager) =>
|
||||
_handleReadCommand(cmd, args, b.getActiveSession());
|
||||
_handleReadCommand(cmd, args, b.getActiveSession(), b);
|
||||
const handleWriteCommand = (cmd: string, args: string[], b: BrowserManager) =>
|
||||
_handleWriteCommand(cmd, args, b.getActiveSession(), b);
|
||||
import { generateCompareHtml } from '../../design/src/compare';
|
||||
|
||||
@@ -32,9 +32,13 @@ const CONTENT_SCRIPT_SENDER = { id: OWN_ID, url: 'https://evil.example/page', ta
|
||||
const FOREIGN_SENDER = { id: FOREIGN_ID, url: `chrome-extension://${FOREIGN_ID}/background.html` };
|
||||
const NO_URL_SENDER = { id: OWN_ID };
|
||||
|
||||
// 'sidebar-command' is no longer a message type at all — the chat-queue path
|
||||
// was ripped along with the /sidebar-command endpoint, so background.js now
|
||||
// rejects it pre-gate as an unknown type (no response, nothing to leak). It is
|
||||
// pinned separately below as a representative unknown type.
|
||||
const PRIVILEGED = [
|
||||
'getPort', 'setPort', 'getServerUrl', 'getToken', 'fetchRefs',
|
||||
'command', 'sidebar-command', 'getTabState',
|
||||
'command', 'getTabState',
|
||||
];
|
||||
// Content-script-originated flows that must keep working.
|
||||
const CONTENT_SCRIPT_TYPES = ['openSidePanel', 'elementPicked', 'pickerCancelled', 'inspectResult'];
|
||||
@@ -216,6 +220,16 @@ describe('background.js onMessage listener (behavioral)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('retired sidebar-command type is rejected pre-gate with no response and no leaks', () => {
|
||||
// Even from the most-trusted sender shape, a type outside ALLOWED_TYPES
|
||||
// never reaches a handler: no sendResponse, no token/port fields possible.
|
||||
for (const sender of [PAGE_SENDER, CONTENT_SCRIPT_SENDER, FOREIGN_SENDER, NO_URL_SENDER]) {
|
||||
const r = dispatch(listener, { type: 'sidebar-command', message: 'hi' }, sender);
|
||||
expect(r.responded).toBe(false);
|
||||
expectDenied(r);
|
||||
}
|
||||
});
|
||||
|
||||
test('denied setPort never persists the attacker port', () => {
|
||||
const before = calls.storageSet.length;
|
||||
const r = dispatch(listener, { type: 'setPort', port: 6666 }, CONTENT_SCRIPT_SENDER);
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
writeSecureFile,
|
||||
appendSecureFile,
|
||||
mkdirSecure,
|
||||
repairBrokenDacl,
|
||||
__resetWarnedForTests,
|
||||
} from '../src/file-permissions';
|
||||
|
||||
@@ -175,4 +176,35 @@ describe('mkdirSecure', () => {
|
||||
expect(fs.existsSync(path.join(tmpDir, 'a', 'b'))).toBe(true);
|
||||
expect(fs.existsSync(d)).toBe(true);
|
||||
});
|
||||
|
||||
test('created directory is listable by the creating process', () => {
|
||||
// #1605 contract: whatever ACL hardening happens, the client must be
|
||||
// able to read its own state dir immediately after creation.
|
||||
const d = path.join(tmpDir, 'state');
|
||||
mkdirSecure(d);
|
||||
fs.writeFileSync(path.join(d, 'browse.json'), '{}');
|
||||
expect(fs.readdirSync(d)).toContain('browse.json');
|
||||
});
|
||||
});
|
||||
|
||||
describe('repairBrokenDacl', () => {
|
||||
test('is a no-op on non-Windows platforms', () => {
|
||||
if (process.platform === 'win32') return;
|
||||
const d = path.join(tmpDir, 'dir');
|
||||
fs.mkdirSync(d);
|
||||
expect(() => repairBrokenDacl(d)).not.toThrow();
|
||||
});
|
||||
|
||||
test('on Windows, does not throw and directory stays listable', () => {
|
||||
if (process.platform !== 'win32') return;
|
||||
const d = path.join(tmpDir, 'dir');
|
||||
fs.mkdirSync(d);
|
||||
expect(() => repairBrokenDacl(d)).not.toThrow();
|
||||
expect(() => fs.readdirSync(d)).not.toThrow();
|
||||
});
|
||||
|
||||
test('on Windows, swallows icacls failure on a nonexistent path', () => {
|
||||
if (process.platform !== 'win32') return;
|
||||
expect(() => repairBrokenDacl(path.join(tmpDir, 'nonexistent'))).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,12 +15,20 @@ const SCRIPT = join(import.meta.dir, '..', '..', 'bin', 'gstack-config');
|
||||
let stateDir: string;
|
||||
|
||||
function run(args: string[] = [], extraEnv: Record<string, string> = {}) {
|
||||
// The script resolves its state dir as GSTACK_STATE_ROOT > GSTACK_HOME >
|
||||
// GSTACK_STATE_DIR > $HOME/.gstack. Strip the higher-precedence vars so a
|
||||
// stray value in the harness env (another test file's leftovers, operator
|
||||
// shell) can never outrank the per-test GSTACK_STATE_DIR isolation.
|
||||
const env: Record<string, string | undefined> = {
|
||||
...process.env,
|
||||
GSTACK_STATE_DIR: stateDir,
|
||||
};
|
||||
delete env.GSTACK_STATE_ROOT;
|
||||
delete env.GSTACK_HOME;
|
||||
Object.assign(env, extraEnv); // per-test overrides always win, deliberately
|
||||
|
||||
const result = Bun.spawnSync(['bash', SCRIPT, ...args], {
|
||||
env: {
|
||||
...process.env,
|
||||
GSTACK_STATE_DIR: stateDir,
|
||||
...extraEnv,
|
||||
},
|
||||
env,
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
});
|
||||
|
||||
@@ -17,14 +17,21 @@ let gstackDir: string;
|
||||
let stateDir: string;
|
||||
|
||||
function run(extraEnv: Record<string, string> = {}, args: string[] = []) {
|
||||
// gstack-config (which this script shells out to for update_check) resolves
|
||||
// state as GSTACK_STATE_ROOT > GSTACK_HOME > GSTACK_STATE_DIR > ~/.gstack.
|
||||
// Strip the higher-precedence vars so harness-env leftovers can never
|
||||
// outrank the per-test GSTACK_STATE_DIR isolation.
|
||||
const env: Record<string, string | undefined> = {
|
||||
...process.env,
|
||||
GSTACK_DIR: gstackDir,
|
||||
GSTACK_STATE_DIR: stateDir,
|
||||
GSTACK_REMOTE_URL: `file://${join(gstackDir, 'REMOTE_VERSION')}`,
|
||||
};
|
||||
delete env.GSTACK_STATE_ROOT;
|
||||
delete env.GSTACK_HOME;
|
||||
Object.assign(env, extraEnv); // per-test overrides always win, deliberately
|
||||
const result = Bun.spawnSync(['bash', SCRIPT, ...args], {
|
||||
env: {
|
||||
...process.env,
|
||||
GSTACK_DIR: gstackDir,
|
||||
GSTACK_STATE_DIR: stateDir,
|
||||
GSTACK_REMOTE_URL: `file://${join(gstackDir, 'REMOTE_VERSION')}`,
|
||||
...extraEnv,
|
||||
},
|
||||
env,
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
});
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
/**
|
||||
* Pair-agent opt-in gate.
|
||||
*
|
||||
* The remote pair-agent (ngrok tunnel) is OFF by default. All three activation
|
||||
* points — CLI auto-start, the /tunnel/start route, and the BROWSE_TUNNEL=1
|
||||
* startup path — route through the single `isPairAgentEnabled()` guard. This
|
||||
* test pins the guard's behavior (the root cause) plus a source-level tripwire
|
||||
* that each call site actually consults it.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterEach } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { isPairAgentEnabled } from '../src/config';
|
||||
|
||||
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
|
||||
const CLI_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/cli.ts'), 'utf-8');
|
||||
|
||||
const savedEnv = { GSTACK_HOME: process.env.GSTACK_HOME, GSTACK_PAIR_AGENT: process.env.GSTACK_PAIR_AGENT };
|
||||
const tmpHomes: string[] = [];
|
||||
|
||||
function tmpHomeWith(config: Record<string, string> | null): string {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-pair-'));
|
||||
tmpHomes.push(dir);
|
||||
if (config !== null) {
|
||||
// Canonical store: flat YAML lines, the shape bin/gstack-config writes.
|
||||
const yaml = Object.entries(config).map(([k, v]) => `${k}: ${v}`).join('\n') + '\n';
|
||||
fs.writeFileSync(path.join(dir, 'config.yaml'), yaml);
|
||||
}
|
||||
process.env.GSTACK_HOME = dir;
|
||||
delete process.env.GSTACK_PAIR_AGENT;
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const k of ['GSTACK_HOME', 'GSTACK_PAIR_AGENT'] as const) {
|
||||
if (savedEnv[k] === undefined) delete process.env[k];
|
||||
else process.env[k] = savedEnv[k];
|
||||
}
|
||||
while (tmpHomes.length) fs.rmSync(tmpHomes.pop()!, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('isPairAgentEnabled — fail-closed default', () => {
|
||||
test('OFF when no config store exists', () => {
|
||||
tmpHomeWith(null);
|
||||
expect(isPairAgentEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('OFF when config has no pair_agent key', () => {
|
||||
tmpHomeWith({ telemetry: 'off' });
|
||||
expect(isPairAgentEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('ON via the config.json fallback shape too', () => {
|
||||
const dir = tmpHomeWith(null);
|
||||
fs.writeFileSync(path.join(dir, 'config.json'), JSON.stringify({ pair_agent: 'on' }));
|
||||
expect(isPairAgentEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('OFF when pair_agent is explicitly "off"', () => {
|
||||
tmpHomeWith({ pair_agent: 'off' });
|
||||
expect(isPairAgentEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('ON only when pair_agent is exactly "on"', () => {
|
||||
tmpHomeWith({ pair_agent: 'on' });
|
||||
expect(isPairAgentEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('OFF when the store is malformed (fail-closed)', () => {
|
||||
const dir = tmpHomeWith(null);
|
||||
fs.writeFileSync(path.join(dir, 'config.yaml'), 'pair_agent: banana\n');
|
||||
fs.writeFileSync(path.join(dir, 'config.json'), '{ not json');
|
||||
expect(isPairAgentEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('env override wins: GSTACK_PAIR_AGENT=on forces ON even with config off', () => {
|
||||
tmpHomeWith({ pair_agent: 'off' });
|
||||
process.env.GSTACK_PAIR_AGENT = 'on';
|
||||
expect(isPairAgentEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('env override wins: GSTACK_PAIR_AGENT=off forces OFF even with config on', () => {
|
||||
tmpHomeWith({ pair_agent: 'on' });
|
||||
process.env.GSTACK_PAIR_AGENT = 'off';
|
||||
expect(isPairAgentEnabled()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('gate wiring — every tunnel activation point consults the guard', () => {
|
||||
test('CLI auto-start is gated (never auto-starts when disabled)', () => {
|
||||
// pairEnabled short-circuits the ngrok probe so the tunnel can't auto-start.
|
||||
expect(CLI_SRC).toContain('const pairEnabled = isPairAgentEnabled();');
|
||||
expect(CLI_SRC).toContain('const ngrokAvailable = pairEnabled && isNgrokAvailable();');
|
||||
});
|
||||
|
||||
test('CLI consent-off branch names the real remedy, never ngrok reinstall', () => {
|
||||
// When pair_agent is off but ngrok is installed+authed, telling the user
|
||||
// to `ngrok config add-authtoken` can never fix it — the gate is consent,
|
||||
// not tooling. The consent branch must carry the same remedy wording as
|
||||
// the /tunnel/start 403 body, and must not mention ngrok setup.
|
||||
const branchAt = CLI_SRC.indexOf('} else if (!pairEnabled) {');
|
||||
expect(branchAt).toBeGreaterThan(-1);
|
||||
const branchEnd = CLI_SRC.indexOf('} else {', branchAt);
|
||||
expect(branchEnd).toBeGreaterThan(branchAt);
|
||||
const branch = CLI_SRC.slice(branchAt, branchEnd);
|
||||
expect(branch).toContain('gstack-config set pair_agent on');
|
||||
expect(branch).toContain('/pair-agent');
|
||||
expect(branch).not.toContain('ngrok config add-authtoken');
|
||||
expect(branch).not.toContain('install ngrok');
|
||||
});
|
||||
|
||||
test('/tunnel/start refuses with the enable hint when disabled', () => {
|
||||
const startIdx = SERVER_SRC.indexOf("url.pathname === '/tunnel/start'");
|
||||
const block = SERVER_SRC.slice(startIdx, startIdx + 1200);
|
||||
expect(block).toContain('if (!isPairAgentEnabled())');
|
||||
expect(block).toContain('gstack-config set pair_agent on');
|
||||
});
|
||||
|
||||
test('BROWSE_TUNNEL=1 startup skips tunnel bind when disabled', () => {
|
||||
expect(SERVER_SRC).toContain("process.env.BROWSE_TUNNEL === '1' && !isPairAgentEnabled()");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,148 @@
|
||||
/**
|
||||
* Unit tests for the extracted poisoned-bundle self-heal probe (#2242).
|
||||
*
|
||||
* probePoisonedChromiumBundle() detects a Chromium bundle mutated by the
|
||||
* pre-v1.64 in-place rebrand (Info.plist contains "GStack Browser"),
|
||||
* removes it so `playwright install chromium` actually re-downloads, and
|
||||
* throws a typed PoisonedBundleError with the remediation command.
|
||||
*
|
||||
* Contracts pinned here:
|
||||
* - standard cache layout (chromium-<rev>/chrome-mac/<name>.app): the
|
||||
* WHOLE revision dir is removed, INSTALLATION_COMPLETE marker included
|
||||
* (leaving the marker makes the recommended re-fetch a no-op)
|
||||
* - non-cache layout: the .app + sibling install markers are removed,
|
||||
* nothing else
|
||||
* - clean bundle: untouched, no throw
|
||||
* - GSTACK_CHROMIUM_PATH bundles (custom/embedder) are NEVER deleted:
|
||||
* the probe refuses to act on that executable, and both call sites
|
||||
* (launchHeaded + handoff) only pass chromium.executablePath()
|
||||
* - the rethrow guard at the call sites is typed (instanceof), not a
|
||||
* fragile message-string match
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { probePoisonedChromiumBundle, PoisonedBundleError } from '../src/browser-manager';
|
||||
|
||||
const SRC = fs.readFileSync(
|
||||
path.join(import.meta.dir, '..', 'src', 'browser-manager.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
const POISONED_PLIST =
|
||||
'<plist><dict><key>CFBundleName</key><string>GStack Browser</string></dict></plist>';
|
||||
const CLEAN_PLIST =
|
||||
'<plist><dict><key>CFBundleName</key><string>Google Chrome for Testing</string></dict></plist>';
|
||||
|
||||
let tmpDir: string;
|
||||
let savedCustomPath: string | undefined;
|
||||
|
||||
/** Build <parentDir>/<name>.app with a plist and executable; return the executable path. */
|
||||
function makeApp(parentDir: string, plist: string): { appDir: string; exe: string } {
|
||||
const appDir = path.join(parentDir, 'Google Chrome for Testing.app');
|
||||
const macos = path.join(appDir, 'Contents', 'MacOS');
|
||||
fs.mkdirSync(macos, { recursive: true });
|
||||
fs.writeFileSync(path.join(appDir, 'Contents', 'Info.plist'), plist);
|
||||
const exe = path.join(macos, 'Google Chrome for Testing');
|
||||
fs.writeFileSync(exe, '#!/bin/sh\n', { mode: 0o755 });
|
||||
return { appDir, exe };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'poison-probe-'));
|
||||
savedCustomPath = process.env.GSTACK_CHROMIUM_PATH;
|
||||
delete process.env.GSTACK_CHROMIUM_PATH;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
if (savedCustomPath === undefined) delete process.env.GSTACK_CHROMIUM_PATH;
|
||||
else process.env.GSTACK_CHROMIUM_PATH = savedCustomPath;
|
||||
});
|
||||
|
||||
describe('probePoisonedChromiumBundle — poisoned cache bundle', () => {
|
||||
test('standard cache layout: whole chromium-<rev> dir removed (markers included), typed error thrown', () => {
|
||||
const revDir = path.join(tmpDir, 'ms-playwright', 'chromium-1234');
|
||||
const { exe } = makeApp(path.join(revDir, 'chrome-mac'), POISONED_PLIST);
|
||||
fs.writeFileSync(path.join(revDir, 'INSTALLATION_COMPLETE'), '');
|
||||
fs.writeFileSync(path.join(revDir, 'DEPENDENCIES_VALIDATED'), '');
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
probePoisonedChromiumBundle(exe);
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(PoisonedBundleError);
|
||||
// The message is the user's remediation — it must carry the command.
|
||||
expect((caught as Error).message).toContain('playwright install chromium');
|
||||
// Whole revision dir gone: leaving INSTALLATION_COMPLETE behind makes
|
||||
// `playwright install chromium` no-op ("is already downloaded") and the
|
||||
// remediation we just printed would do nothing.
|
||||
expect(fs.existsSync(revDir)).toBe(false);
|
||||
});
|
||||
|
||||
test('non-cache layout: only the .app + sibling install markers removed, neighbors survive', () => {
|
||||
const parentDir = path.join(tmpDir, 'custom-bundles');
|
||||
const { appDir, exe } = makeApp(parentDir, POISONED_PLIST);
|
||||
fs.writeFileSync(path.join(parentDir, 'INSTALLATION_COMPLETE'), '');
|
||||
fs.writeFileSync(path.join(parentDir, 'DEPENDENCIES_VALIDATED'), '');
|
||||
fs.writeFileSync(path.join(parentDir, 'unrelated.txt'), 'keep me');
|
||||
|
||||
expect(() => probePoisonedChromiumBundle(exe)).toThrow(PoisonedBundleError);
|
||||
expect(fs.existsSync(appDir)).toBe(false);
|
||||
expect(fs.existsSync(path.join(parentDir, 'INSTALLATION_COMPLETE'))).toBe(false);
|
||||
expect(fs.existsSync(path.join(parentDir, 'DEPENDENCIES_VALIDATED'))).toBe(false);
|
||||
// The parent dir itself and unrelated files are NOT swept.
|
||||
expect(fs.readFileSync(path.join(parentDir, 'unrelated.txt'), 'utf-8')).toBe('keep me');
|
||||
});
|
||||
});
|
||||
|
||||
describe('probePoisonedChromiumBundle — clean and missing bundles', () => {
|
||||
test('clean plist: untouched, no throw', () => {
|
||||
const revDir = path.join(tmpDir, 'ms-playwright', 'chromium-1234');
|
||||
const { appDir, exe } = makeApp(path.join(revDir, 'chrome-mac'), CLEAN_PLIST);
|
||||
fs.writeFileSync(path.join(revDir, 'INSTALLATION_COMPLETE'), '');
|
||||
|
||||
expect(() => probePoisonedChromiumBundle(exe)).not.toThrow();
|
||||
expect(fs.existsSync(path.join(appDir, 'Contents', 'Info.plist'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(revDir, 'INSTALLATION_COMPLETE'))).toBe(true);
|
||||
});
|
||||
|
||||
test('no plist at the probed path: no-op, no throw (bundle not installed yet)', () => {
|
||||
expect(() =>
|
||||
probePoisonedChromiumBundle(path.join(tmpDir, 'nope.app', 'Contents', 'MacOS', 'nope')),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('probePoisonedChromiumBundle — GSTACK_CHROMIUM_PATH is never deleted', () => {
|
||||
test('probe refuses to act on the GSTACK_CHROMIUM_PATH executable, even when poisoned', () => {
|
||||
// A custom/embedder bundle (GStack Browser.app wrapper) legitimately
|
||||
// contains "GStack Browser" in its plist — that is its branding, not
|
||||
// cache poison. Deleting it would destroy the embedder's product.
|
||||
const { appDir, exe } = makeApp(path.join(tmpDir, 'GStack Browser.app-parent'), POISONED_PLIST);
|
||||
process.env.GSTACK_CHROMIUM_PATH = exe;
|
||||
|
||||
expect(() => probePoisonedChromiumBundle(exe)).not.toThrow();
|
||||
expect(fs.existsSync(path.join(appDir, 'Contents', 'Info.plist'))).toBe(true);
|
||||
expect(fs.existsSync(exe)).toBe(true);
|
||||
});
|
||||
|
||||
test('caller contract: both headed launch paths probe chromium.executablePath() only', () => {
|
||||
// launchHeaded + handoff each call the probe with the Playwright-cache
|
||||
// path. No call site may ever pass the custom-bundle env var.
|
||||
const calls = SRC.match(/probePoisonedChromiumBundle\(chromium\.executablePath\(\)\)/g) || [];
|
||||
expect(calls.length).toBeGreaterThanOrEqual(2);
|
||||
expect(SRC).not.toMatch(/probePoisonedChromiumBundle\([^)]*GSTACK_CHROMIUM_PATH/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('typed rethrow guard at the call sites', () => {
|
||||
test('instanceof PoisonedBundleError, not message-string sniffing', () => {
|
||||
expect(SRC).not.toContain("includes('poisoned bundle')");
|
||||
expect(SRC).toMatch(/instanceof PoisonedBundleError/);
|
||||
});
|
||||
});
|
||||
@@ -54,10 +54,16 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
|
||||
expect(isProcessAlive(2147483646)).toBe(false);
|
||||
});
|
||||
|
||||
test('3. isProcessAlive spawns NO subprocess', () => {
|
||||
test('3. isProcessAlive spawns NO subprocess on POSIX (signal-0 path)', () => {
|
||||
// The heart of the bug: a liveness probe that forks is slow enough to
|
||||
// time out, and a timed-out probe silently answers "dead". Signal 0
|
||||
// cannot time out because it never leaves the process.
|
||||
//
|
||||
// Merged design note: on win32 the helper DOES keep a single hardened
|
||||
// tasklist probe (windowsHide, bounded timeout, quoted-CSV PID match)
|
||||
// because Bun's process.kill(pid, 0) throws ESRCH for live Windows PIDs
|
||||
// in compiled binaries. The POSIX path stays subprocess-free.
|
||||
if (process.platform === 'win32') return;
|
||||
const origSpawn = (Bun as any).spawn;
|
||||
const origSpawnSync = (Bun as any).spawnSync;
|
||||
const spawns: string[] = [];
|
||||
@@ -73,11 +79,16 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('4. no source file probes liveness via tasklist', () => {
|
||||
// Static tripwire: re-introducing a tasklist-based existence check
|
||||
// anywhere in src/ resurrects the false-negative class.
|
||||
test('4. no source file probes liveness via tasklist outside the central helper', () => {
|
||||
// Static tripwire: ad-hoc tasklist existence checks scattered across src/
|
||||
// resurrect the false-negative class (each call site re-invents the
|
||||
// timeout/parse handling and gets it subtly wrong). The ONE sanctioned
|
||||
// site is error-handling.ts's isProcessAlive win32 branch — centralized,
|
||||
// windowsHide, bounded timeout, quoted-CSV `"${pid}"` match. Every other
|
||||
// file must route through the helper.
|
||||
const offenders: string[] = [];
|
||||
for (const { file, content } of readAllSourceFiles()) {
|
||||
if (file === 'error-handling.ts') continue; // the canonical helper
|
||||
const code = stripComments(content);
|
||||
// `PID eq` is the existence-probe form specifically. Other tasklist
|
||||
// uses (e.g. IMAGENAME filters for browser detection) are unaffected.
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* #2242 bug 1 regression tripwire: never mutate the signed Chrome-for-Testing
|
||||
* bundle.
|
||||
*
|
||||
* The old launchHeaded() "rebrand" ran a global
|
||||
* `.replace(/Google Chrome for Testing/g, 'GStack Browser')` over the
|
||||
* bundle's Info.plist — which renamed CFBundleExecutable to a binary that
|
||||
* doesn't exist — and overwrote Resources/*.icns. Both writes broke the
|
||||
* codesign seal: GPU process exit_code=5, headed mode dead on macOS 26
|
||||
* (#2242, #2138, #2139).
|
||||
*
|
||||
* Static invariant (same pattern as cdp-session-cleanup.test.ts): the
|
||||
* browser lifecycle code must contain NO write into the Chromium .app
|
||||
* bundle. Branding lives in the wrapper .app / custom GBrowser build.
|
||||
* These assertions fail on the pre-fix code.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
const SRC = fs.readFileSync(
|
||||
path.join(import.meta.dir, '..', 'src', 'browser-manager.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
describe('#2242: signed Chromium bundle is never mutated', () => {
|
||||
test('no global Google-Chrome-for-Testing plist replace', () => {
|
||||
expect(SRC).not.toContain("replace(/Google Chrome for Testing/g");
|
||||
});
|
||||
|
||||
test('no Info.plist write into the Chromium bundle (reads allowed: self-heal probe)', () => {
|
||||
// The old code built `Info.plist` under the bundle's Contents dir and
|
||||
// wrote it back. Any reappearance of a WRITE is a regression. The
|
||||
// launch-time self-heal legitimately READS the plist to detect bundles
|
||||
// the old code already poisoned (EV4), so the path construction itself
|
||||
// is allowed — writes into it are not.
|
||||
expect(SRC).not.toMatch(/writeFileSync\(\s*chromePlist/);
|
||||
const plistWrites = SRC.match(/writeFileSync\([^)]*[Pp]list/g) || [];
|
||||
expect(plistWrites).toEqual([]);
|
||||
});
|
||||
|
||||
test('no icon overwrite into the Chromium bundle Resources dir', () => {
|
||||
expect(SRC).not.toMatch(/copyFileSync\([^)]*destIcon/);
|
||||
expect(SRC).not.toContain("CFBundleIconFile");
|
||||
});
|
||||
|
||||
test('the tombstone comment documenting why stays put', () => {
|
||||
// If someone deletes the explanation, the next contributor reintroduces
|
||||
// the mutation in good faith. Keep the why next to the where.
|
||||
expect(SRC).toContain('#2242');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,123 @@
|
||||
/**
|
||||
* acquireServerLock error honesty (#1084 regression).
|
||||
*
|
||||
* The old code wrapped fs.openSync(lockPath, 'wx') in a bare `catch {}` —
|
||||
* EVERY errno (EACCES, EIO, ENOSPC, ENOENT) fell into the "lock already
|
||||
* held" path and surfaced as "another instance is starting the server",
|
||||
* a phantom 15s contention timeout that masked the real filesystem error.
|
||||
*
|
||||
* New contract:
|
||||
* - EEXIST + live holder → null (real contention)
|
||||
* - EEXIST + dead holder → stale lock removed, acquired
|
||||
* - ENOENT (dir missing) → create dir, retry once, acquired
|
||||
* - anything else → ServerLockError with the real errno
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
// Default (CJS) export — its properties are mutable in Bun, unlike the frozen
|
||||
// `* as fs` namespace, and mutations propagate to cli.ts's own fs import.
|
||||
// Used only for the depth-cap livelock simulations below (restored in finally).
|
||||
import fsMutable from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { acquireServerLock, ServerLockError } from '../src/cli';
|
||||
|
||||
const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-lock-'));
|
||||
afterAll(() => {
|
||||
// Restore write perm so cleanup can delete the read-only dir.
|
||||
try { fs.chmodSync(path.join(tmpRoot, 'rodir'), 0o700); } catch {}
|
||||
fs.rmSync(tmpRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('acquireServerLock (#1084 error honesty)', () => {
|
||||
test('happy path: acquires and releases', () => {
|
||||
const lockPath = path.join(tmpRoot, 'happy.lock');
|
||||
const release = acquireServerLock(lockPath);
|
||||
expect(release).not.toBeNull();
|
||||
expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(process.pid));
|
||||
release!();
|
||||
expect(fs.existsSync(lockPath)).toBe(false);
|
||||
});
|
||||
|
||||
test('EACCES throws ServerLockError with the real errno — NOT phantom contention', () => {
|
||||
if (process.platform === 'win32' || process.getuid?.() === 0) return; // chmod semantics differ
|
||||
const rodir = path.join(tmpRoot, 'rodir');
|
||||
fs.mkdirSync(rodir, { recursive: true });
|
||||
fs.chmodSync(rodir, 0o500); // r-x: open('wx') inside fails EACCES
|
||||
const lockPath = path.join(rodir, 'browse.json.lock');
|
||||
let thrown: any = null;
|
||||
try {
|
||||
acquireServerLock(lockPath); // old code: returned null (phantom contention)
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
}
|
||||
expect(thrown).toBeInstanceOf(ServerLockError);
|
||||
expect(thrown.code).toBe('EACCES');
|
||||
expect(thrown.message).toContain('E_SERVER_LOCK (EACCES)');
|
||||
expect(thrown.message).toContain(lockPath);
|
||||
});
|
||||
|
||||
test('ENOENT (missing lock dir) creates the dir and acquires', () => {
|
||||
const lockPath = path.join(tmpRoot, 'newdir', 'browse.json.lock');
|
||||
// old code: openSync ENOENT → bare catch → readFileSync ENOENT → null
|
||||
const release = acquireServerLock(lockPath);
|
||||
expect(release).not.toBeNull();
|
||||
expect(fs.existsSync(lockPath)).toBe(true);
|
||||
release!();
|
||||
});
|
||||
|
||||
test('EEXIST + dead holder: removes stale lock and acquires', () => {
|
||||
const lockPath = path.join(tmpRoot, 'stale.lock');
|
||||
fs.writeFileSync(lockPath, '999999999\n'); // PID that cannot be alive
|
||||
const release = acquireServerLock(lockPath);
|
||||
expect(release).not.toBeNull();
|
||||
release!();
|
||||
});
|
||||
|
||||
test('EEXIST + live holder: returns null (real contention, no throw)', () => {
|
||||
const lockPath = path.join(tmpRoot, 'live.lock');
|
||||
fs.writeFileSync(lockPath, `${process.pid}\n`); // this test process is alive
|
||||
expect(acquireServerLock(lockPath)).toBeNull();
|
||||
fs.unlinkSync(lockPath);
|
||||
});
|
||||
|
||||
test('EEXIST + garbage lockfile content: NaN pid is treated as stale, lock acquired', () => {
|
||||
const lockPath = path.join(tmpRoot, 'garbage.lock');
|
||||
fs.writeFileSync(lockPath, 'not-a-pid\n'); // parseInt → NaN → falsy → stale path
|
||||
const release = acquireServerLock(lockPath);
|
||||
expect(release).not.toBeNull();
|
||||
// Our pid replaced the garbage — the stale lock was removed and re-acquired.
|
||||
expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(process.pid));
|
||||
release!();
|
||||
expect(fs.existsSync(lockPath)).toBe(false);
|
||||
});
|
||||
|
||||
// NOTE: the "stale lock that survives unlink" livelock variant is deliberately
|
||||
// not simulated here — the source removes locks through safeUnlink's own fs
|
||||
// binding, which a test-side fs monkey-patch cannot reliably intercept in Bun.
|
||||
// The depth cap itself is exercised by the vanish-race test below.
|
||||
|
||||
test('depth cap: holder that vanishes between open and read returns null after 5 retries', () => {
|
||||
// The EEXIST → readFileSync ENOENT race: the lock exists at openSync but
|
||||
// is gone by the read (holder released in between). Repeated forever
|
||||
// (open/release storm), the same depth cap must bound the retry loop.
|
||||
const lockPath = path.join(tmpRoot, 'vanish.lock');
|
||||
fs.writeFileSync(lockPath, `${process.pid}\n`);
|
||||
const origRead = fsMutable.readFileSync;
|
||||
try {
|
||||
(fsMutable as any).readFileSync = (p: fs.PathLike | number, ...rest: unknown[]) => {
|
||||
if (p === lockPath) {
|
||||
const e: NodeJS.ErrnoException = new Error('mock: lock vanished before read');
|
||||
e.code = 'ENOENT';
|
||||
throw e;
|
||||
}
|
||||
return (origRead as any)(p, ...rest);
|
||||
};
|
||||
expect(acquireServerLock(lockPath)).toBeNull();
|
||||
} finally {
|
||||
(fsMutable as any).readFileSync = origRead;
|
||||
fs.unlinkSync(lockPath);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,268 @@
|
||||
/**
|
||||
* Opt-in session-state persistence (#778, #2193, #1128, #1129).
|
||||
*
|
||||
* Pins the leg of the browser-lifecycle contract that had no coverage:
|
||||
* "shut down without losing live session state." Pre-fix, the headless
|
||||
* daemon used non-persistent chromium.launch() with zero storage
|
||||
* persistence — any crash or binary-version auto-restart silently lost all
|
||||
* auth. These tests fail on the old tree (module absent, no wiring).
|
||||
*
|
||||
* Suites:
|
||||
* 1. Pure serialize/deserialize/filter units (free, instant).
|
||||
* 2. Real-Chromium round-trip: cookie + localStorage survive a full
|
||||
* manager teardown + relaunch via persist/restore.
|
||||
* 3. Static wiring tripwire: server.ts restores at launch, snapshots at
|
||||
* shutdown, and the gate is BROWSE_PERSIST_STATE (default off).
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import {
|
||||
serializeSessionState, deserializeSessionState, filterSessionCookies,
|
||||
isSessionPersistEnabled, persistSessionState, restoreSessionState,
|
||||
} from '../src/session-persist';
|
||||
import type { BrowserState } from '../src/browser-manager';
|
||||
|
||||
const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-persist-'));
|
||||
afterAll(() => { fs.rmSync(tmpRoot, { recursive: true, force: true }); });
|
||||
|
||||
describe('session-persist units', () => {
|
||||
test('config gate: default off, exactly "1" enables', () => {
|
||||
expect(isSessionPersistEnabled({} as NodeJS.ProcessEnv)).toBe(false);
|
||||
expect(isSessionPersistEnabled({ BROWSE_PERSIST_STATE: '0' } as any)).toBe(false);
|
||||
expect(isSessionPersistEnabled({ BROWSE_PERSIST_STATE: 'true' } as any)).toBe(false);
|
||||
expect(isSessionPersistEnabled({ BROWSE_PERSIST_STATE: '1' } as any)).toBe(true);
|
||||
});
|
||||
|
||||
test('serialize strips loadedHtml/owner, keeps cookies + storage', () => {
|
||||
const state: BrowserState = {
|
||||
cookies: [{ name: 'sid', value: 'abc', domain: 'example.com', path: '/', expires: -1, httpOnly: false, secure: false, sameSite: 'Lax' } as any],
|
||||
pages: [{
|
||||
url: 'https://example.com/app',
|
||||
isActive: true,
|
||||
storage: { localStorage: { k: 'v' }, sessionStorage: {} },
|
||||
loadedHtml: '<script>evil</script>',
|
||||
loadedHtmlWaitUntil: 'load',
|
||||
owner: 'agent-1',
|
||||
}],
|
||||
};
|
||||
const raw = serializeSessionState(state);
|
||||
expect(raw).not.toContain('loadedHtml');
|
||||
expect(raw).not.toContain('evil');
|
||||
expect(raw).not.toContain('owner');
|
||||
const parsed = JSON.parse(raw);
|
||||
expect(parsed.version).toBe(1);
|
||||
expect(parsed.cookies[0].name).toBe('sid');
|
||||
expect(parsed.pages[0].storage.localStorage.k).toBe('v');
|
||||
});
|
||||
|
||||
test('deserialize rejects corrupt JSON, wrong version, missing arrays', () => {
|
||||
expect(deserializeSessionState('not json{')).toBeNull();
|
||||
expect(deserializeSessionState('{"version":99,"cookies":[],"pages":[]}')).toBeNull();
|
||||
expect(deserializeSessionState('{"version":1,"cookies":{}}')).toBeNull();
|
||||
});
|
||||
|
||||
test('deserialize strips loadedHtml/owner even if tampered onto disk', () => {
|
||||
const raw = JSON.stringify({
|
||||
version: 1,
|
||||
cookies: [],
|
||||
pages: [{ url: 'https://x.com', isActive: true, storage: null, loadedHtml: '<h1>x</h1>', owner: 'evil' }],
|
||||
});
|
||||
const state = deserializeSessionState(raw)!;
|
||||
expect((state.pages[0] as any).loadedHtml).toBeUndefined();
|
||||
expect((state.pages[0] as any).owner).toBeUndefined();
|
||||
});
|
||||
|
||||
test('cookie filter drops malformed + internal-network domains', () => {
|
||||
const kept = filterSessionCookies([
|
||||
{ name: 'ok', value: 'v', domain: 'example.com' },
|
||||
{ name: 'ok2', value: 'v', domain: '.example.com' }, // leading-dot public domain kept
|
||||
{ name: 'bad1', value: 'v', domain: 'localhost' },
|
||||
{ name: 'bad2', value: 'v', domain: '.corp.internal' },
|
||||
{ name: 'bad3', value: 'v', domain: '169.254.169.254' },
|
||||
{ name: 'bad4', value: 'v', domain: '169.254.1.2' }, // whole link-local block, not just metadata
|
||||
{ name: 'bad5', value: 'v', domain: '127.0.0.1' }, // IPv4 loopback literal
|
||||
{ name: 'bad6', value: 'v', domain: '.127.0.0.1' }, // leading-dot loopback variant
|
||||
{ name: 'bad7', value: 'v', domain: '::1' }, // IPv6 loopback
|
||||
{ name: 'bad8', value: 'v', domain: '[::1]' }, // bracketed IPv6 loopback
|
||||
{ name: 'bad9', value: 42, domain: 'example.com' },
|
||||
null,
|
||||
]);
|
||||
expect(kept.map((c: any) => c.name)).toEqual(['ok', 'ok2']);
|
||||
});
|
||||
|
||||
test('restoreSessionState: missing file → null, corrupt file → quarantined to .corrupt', async () => {
|
||||
const bmNeverCalled = { closeAllPages() { throw new Error('must not restore'); } } as any;
|
||||
expect(await restoreSessionState(bmNeverCalled, path.join(tmpRoot, 'nope.json'))).toBeNull();
|
||||
const corrupt = path.join(tmpRoot, 'corrupt.json');
|
||||
fs.writeFileSync(corrupt, '{oops');
|
||||
expect(await restoreSessionState(bmNeverCalled, corrupt)).toBeNull();
|
||||
expect(fs.existsSync(corrupt)).toBe(false); // moved aside, won't block every future launch
|
||||
expect(fs.existsSync(`${corrupt}.corrupt`)).toBe(true); // forensic artifact kept (R3)
|
||||
});
|
||||
|
||||
test('persistSessionState is a no-op in headed mode (profile owns state)', async () => {
|
||||
const file = path.join(tmpRoot, 'headed.json');
|
||||
const bm = {
|
||||
getConnectionMode: () => 'headed',
|
||||
saveState() { throw new Error('must not snapshot headed session'); },
|
||||
} as any;
|
||||
await persistSessionState(bm, file);
|
||||
expect(fs.existsSync(file)).toBe(false);
|
||||
});
|
||||
|
||||
test('persist writes atomically: no .tmp left behind, file parses', async () => {
|
||||
const file = path.join(tmpRoot, 'atomic.json');
|
||||
const state: BrowserState = {
|
||||
cookies: [{ name: 'sid', value: 'abc', domain: 'example.com' } as any],
|
||||
pages: [{ url: 'https://example.com', isActive: true, storage: null }],
|
||||
};
|
||||
const bm = { getConnectionMode: () => 'launched', saveState: async () => state } as any;
|
||||
await persistSessionState(bm, file);
|
||||
expect(fs.existsSync(`${file}.tmp`)).toBe(false); // staged copy renamed away
|
||||
const parsed = JSON.parse(fs.readFileSync(file, 'utf-8'));
|
||||
expect(parsed.cookies[0].name).toBe('sid');
|
||||
});
|
||||
|
||||
test('a failed snapshot write preserves the previous good snapshot', async () => {
|
||||
// chmod-based read-only dirs don't bind on Windows or when running as root.
|
||||
if (process.platform === 'win32' || process.getuid?.() === 0) return;
|
||||
const dir = path.join(tmpRoot, 'ro');
|
||||
fs.mkdirSync(dir);
|
||||
const file = path.join(dir, 'session-state.json');
|
||||
const goodState: BrowserState = {
|
||||
cookies: [],
|
||||
pages: [{ url: 'https://good.example', isActive: true, storage: null }],
|
||||
};
|
||||
const bm = { getConnectionMode: () => 'launched', saveState: async () => goodState } as any;
|
||||
await persistSessionState(bm, file);
|
||||
fs.chmodSync(dir, 0o500); // next .tmp write throws EACCES mid-persist
|
||||
try {
|
||||
await expect(persistSessionState(bm, file)).rejects.toThrow();
|
||||
// The crash-mid-write scenario the feature exists to survive: the
|
||||
// previous good snapshot is untouched and still parses.
|
||||
const parsed = JSON.parse(fs.readFileSync(file, 'utf-8'));
|
||||
expect(parsed.pages[0].url).toBe('https://good.example');
|
||||
} finally {
|
||||
fs.chmodSync(dir, 0o700);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('session-persist round-trip (real Chromium)', () => {
|
||||
test('cookie + localStorage + URL survive teardown → relaunch; loopback cookies dropped', async () => {
|
||||
const { BrowserManager } = await import('../src/browser-manager');
|
||||
const { startTestServer } = await import('./test-server');
|
||||
const { server, url } = startTestServer(0);
|
||||
const stateFile = path.join(tmpRoot, 'roundtrip.json');
|
||||
|
||||
const bm1 = new BrowserManager();
|
||||
await bm1.launch();
|
||||
try {
|
||||
const page = bm1.getPage();
|
||||
await page.goto(`${url}/basic.html`, { waitUntil: 'domcontentloaded' });
|
||||
// Real-site cookie: set on the context for a non-loopback domain (the
|
||||
// restore hygiene filter deliberately drops loopback/link-local
|
||||
// domains, so a 127.0.0.1 test-server cookie can't stand in for it).
|
||||
await page.context().addCookies([
|
||||
{ name: 'session_marker', value: 'alive-after-restart', domain: 'example.com', path: '/' },
|
||||
]);
|
||||
await page.evaluate(() => {
|
||||
document.cookie = 'loopback_marker=must-be-dropped; path=/'; // 127.0.0.1 host cookie
|
||||
localStorage.setItem('auth_marker', 'still-logged-in');
|
||||
});
|
||||
await persistSessionState(bm1, stateFile);
|
||||
} finally {
|
||||
await bm1.close();
|
||||
}
|
||||
|
||||
// File on disk is owner-only (cookies are secrets).
|
||||
if (process.platform !== 'win32') {
|
||||
expect(fs.statSync(stateFile).mode & 0o777).toBe(0o600);
|
||||
}
|
||||
|
||||
const bm2 = new BrowserManager();
|
||||
await bm2.launch();
|
||||
try {
|
||||
const restored = await restoreSessionState(bm2, stateFile);
|
||||
expect(restored).not.toBeNull();
|
||||
expect(restored!.pages.length).toBe(1); // counts derivable without a saveState() round-trip
|
||||
// Hygiene filter applied at restore: the real-site cookie survives,
|
||||
// the loopback cookie does not.
|
||||
expect(restored!.cookies.map((c: any) => c.name)).toEqual(['session_marker']);
|
||||
const page = bm2.getPage();
|
||||
expect(page.url()).toContain('/basic.html');
|
||||
const marker = await page.evaluate(() => ({
|
||||
cookie: document.cookie,
|
||||
auth: localStorage.getItem('auth_marker'),
|
||||
}));
|
||||
expect(marker.cookie).not.toContain('loopback_marker'); // dropped by isInternalCookieDomain
|
||||
expect(marker.auth).toBe('still-logged-in');
|
||||
const restoredCookies = await page.context().cookies('https://example.com');
|
||||
expect(restoredCookies.map((c) => `${c.name}=${c.value}`)).toContain('session_marker=alive-after-restart');
|
||||
} finally {
|
||||
await bm2.close();
|
||||
server.stop(true);
|
||||
}
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
describe('server wiring (static tripwire)', () => {
|
||||
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '..', 'src', 'server.ts'), 'utf-8');
|
||||
|
||||
test('start() restores and schedules interval snapshots behind the gate', () => {
|
||||
expect(SERVER_SRC).toContain('isSessionPersistEnabled()');
|
||||
expect(SERVER_SRC).toContain('restoreSessionState(browserManager');
|
||||
expect(SERVER_SRC).toContain('sessionPersistIntervalMs()');
|
||||
});
|
||||
|
||||
test('start() restores in the background AFTER the port binds (CLI readiness must not wait)', () => {
|
||||
// Restore re-creates tabs with up-to-15s goto timeouts; the CLI gives up
|
||||
// at 8s. A restore that runs before Bun.serve() makes every $B command
|
||||
// report "Server failed to start" on one slow saved URL.
|
||||
const serveAt = SERVER_SRC.indexOf('const server = Bun.serve(');
|
||||
const restoreAt = SERVER_SRC.indexOf('restoreSessionState(browserManager');
|
||||
expect(serveAt).toBeGreaterThan(-1);
|
||||
expect(restoreAt).toBeGreaterThan(serveAt);
|
||||
});
|
||||
|
||||
test('interval snapshots carry an in-flight guard (no overlapping persists)', () => {
|
||||
expect(SERVER_SRC).toContain('persistInFlight');
|
||||
});
|
||||
|
||||
test('interval ticks are gated on isShuttingDown (belt half of the shutdown ordering fix)', () => {
|
||||
// A tick that fires during browser teardown snapshots a degraded state
|
||||
// (zero tabs) over the good final snapshot. The handle-clear in shutdown()
|
||||
// is the suspenders; this gate is the belt for a tick already scheduled.
|
||||
const tickerAt = SERVER_SRC.indexOf('sessionPersistInterval = setInterval(');
|
||||
expect(tickerAt).toBeGreaterThan(-1);
|
||||
const tickerBlock = SERVER_SRC.slice(tickerAt, tickerAt + 500);
|
||||
expect(tickerBlock).toContain('if (isShuttingDown) return;');
|
||||
});
|
||||
|
||||
test('shutdown() clears the persist ticker BEFORE the final snapshot (suspenders half)', () => {
|
||||
const shutdownStart = SERVER_SRC.indexOf('async function shutdown(');
|
||||
const clearAt = SERVER_SRC.indexOf('clearInterval(sessionPersistInterval)', shutdownStart);
|
||||
const persistAt = SERVER_SRC.indexOf('persistSessionState(cfgBrowserManager', shutdownStart);
|
||||
expect(clearAt).toBeGreaterThan(shutdownStart);
|
||||
expect(persistAt).toBeGreaterThan(clearAt);
|
||||
});
|
||||
|
||||
test('shutdown() takes a final snapshot BEFORE closing the browser', () => {
|
||||
const shutdownStart = SERVER_SRC.indexOf('async function shutdown(');
|
||||
const persistAt = SERVER_SRC.indexOf('persistSessionState(cfgBrowserManager', shutdownStart);
|
||||
const closeAt = SERVER_SRC.indexOf('await cfgBrowserManager.close()', shutdownStart);
|
||||
expect(persistAt).toBeGreaterThan(shutdownStart);
|
||||
expect(closeAt).toBeGreaterThan(persistAt);
|
||||
});
|
||||
|
||||
test('shutdown() snapshot is deadlined — a wedged page.evaluate cannot hang shutdown', () => {
|
||||
const shutdownStart = SERVER_SRC.indexOf('async function shutdown(');
|
||||
const closeAt = SERVER_SRC.indexOf('await cfgBrowserManager.close()', shutdownStart);
|
||||
const raceAt = SERVER_SRC.indexOf('Promise.race', shutdownStart);
|
||||
expect(raceAt).toBeGreaterThan(shutdownStart);
|
||||
expect(raceAt).toBeLessThan(closeAt);
|
||||
});
|
||||
});
|
||||
@@ -14,7 +14,7 @@ import { handleMetaCommand } from '../src/meta-commands';
|
||||
import * as fs from 'fs';
|
||||
|
||||
const handleReadCommand = (cmd: string, args: string[], b: BrowserManager) =>
|
||||
_handleReadCommand(cmd, args, b.getActiveSession());
|
||||
_handleReadCommand(cmd, args, b.getActiveSession(), b);
|
||||
const handleWriteCommand = (cmd: string, args: string[], b: BrowserManager) =>
|
||||
_handleWriteCommand(cmd, args, b.getActiveSession(), b);
|
||||
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { handleMetaCommand } from '../src/meta-commands';
|
||||
|
||||
describe('server control acknowledgement ordering', () => {
|
||||
for (const [command, acknowledgement] of [
|
||||
['stop', 'Server stopped'],
|
||||
['restart', 'Restarting...'],
|
||||
] as const) {
|
||||
test(`${command} acknowledges before closing the listener`, async () => {
|
||||
let shutdownCalls = 0;
|
||||
const manager = { getActiveSession: () => ({}) } as any;
|
||||
|
||||
const result = await handleMetaCommand(command, [], manager, async () => {
|
||||
shutdownCalls += 1;
|
||||
});
|
||||
|
||||
expect(result).toBe(acknowledgement);
|
||||
expect(shutdownCalls).toBe(0);
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
expect(shutdownCalls).toBe(1);
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,175 @@
|
||||
/**
|
||||
* Telemetry consent tiers — the user-consent enforcement point.
|
||||
*
|
||||
* Telemetry is OPT-IN: it emits only when the user granted a tier through
|
||||
* the consent prompt (`telemetry: community` or `telemetry: anonymous` in
|
||||
* ~/.gstack/config.yaml). An absent key, an absent config file, an explicit
|
||||
* `off`, or any unrecognized value all mean DISABLED — the same default
|
||||
* bin/gstack-config's DEFAULTS table reports for an unset key, so a daemon
|
||||
* spawned outside a skill preamble (direct $B use, embedders) can never
|
||||
* emit while `gstack-config get telemetry` tells the user 'off'.
|
||||
*
|
||||
* The persistent tier reads through the shared flat-YAML helper in
|
||||
* config.ts (readGstackConfigYamlKey), same parser as the pair-agent gate.
|
||||
* Env tier: GSTACK_TELEMETRY_OFF=1 always disables; =0 is a harness-side
|
||||
* consent assertion that covers the no-config default only — it never
|
||||
* overrides an explicit `telemetry: off`.
|
||||
*
|
||||
* Harness mirrors pair-agent-optin-gate.test.ts: GSTACK_HOME → temp dir,
|
||||
* env saved/restored per test, cache reset via _resetTelemetryCache.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterEach } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { isTelemetryDisabled, logTelemetry, _resetTelemetryCache } from '../src/telemetry';
|
||||
|
||||
const savedEnv = {
|
||||
GSTACK_HOME: process.env.GSTACK_HOME,
|
||||
GSTACK_TELEMETRY_OFF: process.env.GSTACK_TELEMETRY_OFF,
|
||||
};
|
||||
const tmpHomes: string[] = [];
|
||||
|
||||
/** Fresh GSTACK_HOME with the given config.yaml body (null = no file). */
|
||||
function tmpHomeWith(configYaml: string | null): string {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-telemetry-optout-'));
|
||||
tmpHomes.push(dir);
|
||||
if (configYaml !== null) {
|
||||
fs.writeFileSync(path.join(dir, 'config.yaml'), configYaml);
|
||||
}
|
||||
process.env.GSTACK_HOME = dir;
|
||||
delete process.env.GSTACK_TELEMETRY_OFF;
|
||||
_resetTelemetryCache();
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const k of ['GSTACK_HOME', 'GSTACK_TELEMETRY_OFF'] as const) {
|
||||
if (savedEnv[k] === undefined) delete process.env[k];
|
||||
else process.env[k] = savedEnv[k]!;
|
||||
}
|
||||
_resetTelemetryCache();
|
||||
while (tmpHomes.length) fs.rmSync(tmpHomes.pop()!, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('telemetry persistent opt-out tier (config.yaml)', () => {
|
||||
test('DISABLED when config.yaml has plain `telemetry: off`', () => {
|
||||
tmpHomeWith('telemetry: off\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test("DISABLED when the value is single-quoted: telemetry: 'off'", () => {
|
||||
tmpHomeWith("telemetry: 'off'\n");
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('DISABLED when the value is double-quoted: telemetry: "off"', () => {
|
||||
tmpHomeWith('telemetry: "off"\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('DISABLED with a trailing comment: telemetry: off # user opted out', () => {
|
||||
tmpHomeWith('telemetry: off # user opted out\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('DISABLED when the key sits among other keys', () => {
|
||||
tmpHomeWith('pair_agent: off\ntelemetry: off\nskill_prefix: none\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('ENABLED when the user granted the `anonymous` tier', () => {
|
||||
tmpHomeWith('telemetry: anonymous\n');
|
||||
expect(isTelemetryDisabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('ENABLED when the user granted the `community` tier', () => {
|
||||
tmpHomeWith('telemetry: community\n');
|
||||
expect(isTelemetryDisabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('DISABLED when the key is absent — consent was never granted', () => {
|
||||
// bin/gstack-config's DEFAULTS table reports 'off' for an unset telemetry
|
||||
// key; the daemon must agree or direct-$B spawns emit while the user is
|
||||
// told telemetry is off (default-polarity split-brain).
|
||||
tmpHomeWith('pair_agent: on\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('DISABLED when config.yaml does not exist — fresh installs emit nothing', () => {
|
||||
tmpHomeWith(null);
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('DISABLED on an unrecognized tier value (fail-closed)', () => {
|
||||
tmpHomeWith('telemetry: banana\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('a commented-out consent line does not enable: `# telemetry: community`', () => {
|
||||
tmpHomeWith('# telemetry: community\n');
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('telemetry env tier + cache semantics', () => {
|
||||
test('GSTACK_TELEMETRY_OFF=1 disables even when config says anonymous', () => {
|
||||
tmpHomeWith('telemetry: anonymous\n');
|
||||
process.env.GSTACK_TELEMETRY_OFF = '1';
|
||||
_resetTelemetryCache();
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('GSTACK_TELEMETRY_OFF=0 never overrides an explicit `telemetry: off`', () => {
|
||||
// The =0 hint is a harness-side consent assertion for scratch homes with
|
||||
// no config store; a user's written opt-out always wins over it.
|
||||
tmpHomeWith('telemetry: off\n');
|
||||
process.env.GSTACK_TELEMETRY_OFF = '0';
|
||||
_resetTelemetryCache();
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
|
||||
test('GSTACK_TELEMETRY_OFF=0 enables when no config store exists (harness seam)', () => {
|
||||
tmpHomeWith(null);
|
||||
process.env.GSTACK_TELEMETRY_OFF = '0';
|
||||
_resetTelemetryCache();
|
||||
expect(isTelemetryDisabled()).toBe(false);
|
||||
});
|
||||
|
||||
test('verdict is cached per process; _resetTelemetryCache re-reads config', () => {
|
||||
const dir = tmpHomeWith('telemetry: anonymous\n');
|
||||
expect(isTelemetryDisabled()).toBe(false);
|
||||
// Opt out on disk mid-process: the cached verdict holds until reset.
|
||||
fs.writeFileSync(path.join(dir, 'config.yaml'), 'telemetry: off\n');
|
||||
expect(isTelemetryDisabled()).toBe(false);
|
||||
_resetTelemetryCache();
|
||||
expect(isTelemetryDisabled()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('enforcement: logTelemetry writes only with granted consent', () => {
|
||||
test('config-tier opt-out suppresses the JSONL append', async () => {
|
||||
const dir = tmpHomeWith('telemetry: off\n');
|
||||
logTelemetry({ event: 'domain_skill_fired', host: 'example.com' });
|
||||
// Fire-and-forget path: give any (incorrect) async append time to land.
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
expect(fs.existsSync(path.join(dir, 'analytics', 'browse-telemetry.jsonl'))).toBe(false);
|
||||
});
|
||||
|
||||
test('no consent ever recorded (absent key) suppresses the JSONL append', async () => {
|
||||
const dir = tmpHomeWith('pair_agent: on\n');
|
||||
logTelemetry({ event: 'domain_skill_fired', host: 'example.com' });
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
expect(fs.existsSync(path.join(dir, 'analytics', 'browse-telemetry.jsonl'))).toBe(false);
|
||||
});
|
||||
|
||||
test('granted `community` tier appends the event', async () => {
|
||||
const dir = tmpHomeWith('telemetry: community\n');
|
||||
logTelemetry({ event: 'domain_skill_fired', host: 'example.com' });
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
const file = path.join(dir, 'analytics', 'browse-telemetry.jsonl');
|
||||
expect(fs.existsSync(file)).toBe(true);
|
||||
expect(fs.readFileSync(file, 'utf-8')).toContain('domain_skill_fired');
|
||||
});
|
||||
});
|
||||
+181
-25
@@ -1,8 +1,12 @@
|
||||
import { describe, test, expect, afterEach } from 'bun:test';
|
||||
import { describe, test, expect, afterEach, beforeEach, mock } from 'bun:test';
|
||||
import { spawn, type Subprocess } from 'bun';
|
||||
import * as path from 'path';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as crypto from 'crypto';
|
||||
import { buildFetchHandler, __testInternals__, type ServerConfig } from '../src/server';
|
||||
import { __resetRegistry } from '../src/token-registry';
|
||||
import { resolveConfig } from '../src/config';
|
||||
|
||||
// End-to-end regression tests for the parent-process watchdog in server.ts.
|
||||
// The watchdog has layered behavior since v0.18.1.0 (#1025) and v0.18.2.0
|
||||
@@ -18,16 +22,13 @@ import * as os from 'os';
|
||||
// eventual cleanup.
|
||||
//
|
||||
// Tunnel mode coverage (parent dies → shutdown because idle timeout doesn't
|
||||
// apply) is not covered by an automated test here — tunnelActive is a runtime
|
||||
// variable set by /pair-agent's tunnel-create flow, not an env var, so faking
|
||||
// it would require invasive test-only hooks. The mode check is documented
|
||||
// inline at the watchdog and SIGTERM handlers, and would regress visibly for
|
||||
// /pair-agent users (server lingers after disconnect).
|
||||
// apply) is covered behaviorally in the in-process suite at the bottom of this
|
||||
// file: the tick is exported via __testInternals__.parentWatchdogTick (same
|
||||
// seam as idleCheckTick) and tunnelActive is simulated via setTunnelActive.
|
||||
//
|
||||
// Each test spawns the real server.ts. Tests 1 and 2 verify behavior via
|
||||
// stdout log line (fast). Test 3 shrinks the watchdog tick to 250ms via
|
||||
// BROWSE_WATCHDOG_INTERVAL_MS and waits for the stay-alive log line, then
|
||||
// confirms the server survived parent death (~1-2s instead of a 20s sleep).
|
||||
// stdout log line (fast). Test 3 waits for the watchdog poll cycle to confirm
|
||||
// the server REMAINS alive after parent death (slow — ~20s observation window).
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const SERVER_SCRIPT = path.join(ROOT, 'src', 'server.ts');
|
||||
@@ -138,26 +139,181 @@ describe('parent-process watchdog (v0.18.1.0)', () => {
|
||||
const parentPid = parentProc.pid!;
|
||||
|
||||
// Default headless: no BROWSE_HEADED, real parent PID — watchdog active.
|
||||
// 250ms tick (test-only knob) so this test doesn't wait out the
|
||||
// production 15s interval; the old version blind-slept 22s.
|
||||
serverProc = spawnServer(
|
||||
{ BROWSE_PARENT_PID: String(parentPid), BROWSE_WATCHDOG_INTERVAL_MS: '250' },
|
||||
34903,
|
||||
);
|
||||
serverProc = spawnServer({ BROWSE_PARENT_PID: String(parentPid) }, 34903);
|
||||
const serverPid = serverProc.pid!;
|
||||
|
||||
// Give the server a beat to register the watchdog interval.
|
||||
await Bun.sleep(500);
|
||||
// Give the server a moment to start and register the watchdog interval.
|
||||
await Bun.sleep(2000);
|
||||
expect(isProcessAlive(serverPid)).toBe(true);
|
||||
|
||||
// Kill the parent. Pre-#994 the server would shut down on the next tick.
|
||||
// Post-#994 it logs the parent exit and stays alive — wait for that log
|
||||
// line instead of sleeping past a fixed interval.
|
||||
// Kill the parent. The watchdog polls every 15s, so first tick after
|
||||
// parent death lands within ~15s. Pre-#994 the server would shutdown
|
||||
// here. Post-#994 the server logs the parent exit and stays alive.
|
||||
parentProc.kill('SIGKILL');
|
||||
const out = await readStdoutUntil(serverProc, 'server stays alive', 10_000);
|
||||
expect(out).toContain(
|
||||
`Parent process ${parentPid} exited (server stays alive, idle timeout will clean up)`,
|
||||
);
|
||||
|
||||
// Wait long enough for at least one watchdog tick (15s) plus margin.
|
||||
// Server should still be alive — that's the whole point of #994.
|
||||
await Bun.sleep(20_000);
|
||||
expect(isProcessAlive(serverPid)).toBe(true);
|
||||
}, 30_000);
|
||||
}, 45_000);
|
||||
});
|
||||
|
||||
// The three tests above all fix the mode via env at SPAWN time, so none of them
|
||||
// reaches the headed branch of the watchdog. That branch is only reachable by a
|
||||
// RUNTIME promotion, which `handoff` performs: it swaps in a headed context on a
|
||||
// running daemon without a restart, moving a daemon that legitimately registered
|
||||
// a watchdog onto the fatal side of the check. The parent is usually a
|
||||
// short-lived shell (Claude Code's Bash tool kills one after every invocation),
|
||||
// so the next poll shut the daemon down and discarded whatever the user had been
|
||||
// handed off to do — observed as repeated session loss mid-login.
|
||||
//
|
||||
// The fix must NOT clear the interval, though: the same tick is the
|
||||
// tunnel-orphan reaper (idle timeout is disabled in tunnel mode, so parent
|
||||
// death is the ONLY thing that reaps an internet-exposed daemon). Promotion
|
||||
// sets a suppress flag the tick re-reads each pass — "being headed" no longer
|
||||
// kills the daemon on parent death, but an active tunnel still does.
|
||||
//
|
||||
// Driving a real `handoff` needs a headed Chromium, which does not belong in the
|
||||
// free tier, so this pins the WIRING instead — the same static-tripwire approach
|
||||
// used by cdp-session-cleanup.test.ts and server-auth.test.ts. If either half of
|
||||
// the contract is dropped, the crash returns silently and these fail. The
|
||||
// behavioral halves (suppression + tunnel reaping) run in-process below.
|
||||
describe('headed parent-death shutdown is suppressed on runtime promotion', () => {
|
||||
const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), 'utf-8');
|
||||
|
||||
test('handoff() notifies the server that it promoted the daemon', () => {
|
||||
const src = read('src/browser-manager.ts');
|
||||
const promote = src.indexOf("this.connectionMode = 'headed';", src.indexOf('async handoff('));
|
||||
expect(promote).toBeGreaterThan(-1);
|
||||
// The notification must follow the promotion closely; a call left far away
|
||||
// (or removed) is the regression this guards.
|
||||
expect(src.slice(promote, promote + 800)).toContain('this.onHeadedPromotion?.()');
|
||||
});
|
||||
|
||||
test('the server binds that callback to the suppress-flag setter', () => {
|
||||
const src = read('src/server.ts');
|
||||
expect(src).toContain('function suppressHeadedParentShutdown()');
|
||||
// Bound on BOTH the module-level manager and any embedder-supplied one; the
|
||||
// watchdog reads activeBrowserManager, so binding only the default instance
|
||||
// leaves embedders (e.g. gbrowser) promoting silently.
|
||||
expect(src).toContain('browserManager.onHeadedPromotion = suppressHeadedParentShutdown');
|
||||
expect(src).toContain('cfgBrowserManager.onHeadedPromotion = suppressHeadedParentShutdown');
|
||||
});
|
||||
|
||||
test('promotion must NOT clear the interval — the tick doubles as the tunnel-orphan reaper', () => {
|
||||
const src = read('src/server.ts');
|
||||
// The original #2565 absorption cleared the ENTIRE interval on promotion.
|
||||
// Sequence handoff → resume → /pair-agent tunnel then left an
|
||||
// internet-exposed daemon that nothing reaps. The tick must stay
|
||||
// registered and re-check the suppress flag + tunnelActive every pass.
|
||||
expect(src).not.toContain('clearInterval(parentWatchdogTimer)');
|
||||
expect(src).toContain('setInterval(parentWatchdogTick');
|
||||
const tickStart = src.indexOf('function parentWatchdogTick(');
|
||||
expect(tickStart).toBeGreaterThan(-1);
|
||||
const tick = src.slice(tickStart, src.indexOf('\n}', tickStart));
|
||||
expect(tick).toContain('headedParentShutdownSuppressed');
|
||||
expect(tick).toContain('tunnelActive');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Behavioral: suppressed watchdog still reaps tunnel orphans ────────────
|
||||
//
|
||||
// In-process, via the same __testInternals__ seam server-factory.test.ts uses
|
||||
// for idleCheckTick. parentWatchdogTick(deadPid) simulates the 15s poll
|
||||
// discovering a dead parent; setTunnelActive simulates /pair-agent's
|
||||
// tunnel-create flow; suppressHeadedParentShutdown is exactly what the
|
||||
// handoff promotion callback invokes.
|
||||
function makeMinimalConfig(mode: 'launched' | 'headed', tmpDir: string): ServerConfig {
|
||||
const base = resolveConfig();
|
||||
return {
|
||||
authToken: 'watchdog-test-' + crypto.randomBytes(16).toString('hex'),
|
||||
browsePort: 34567,
|
||||
idleTimeoutMs: 1_800_000,
|
||||
// State paths pointed at a scratch dir so shutdown()'s cleanup can never
|
||||
// touch a real daemon's files on the machine running the tests.
|
||||
config: { ...base, stateFile: path.join(tmpDir, 'browse-state.json'), stateDir: tmpDir },
|
||||
browserManager: {
|
||||
getConnectionMode: () => mode,
|
||||
isWatching: () => false,
|
||||
stopWatch: () => {},
|
||||
close: async () => {},
|
||||
onDisconnect: null,
|
||||
} as any,
|
||||
startTime: Date.now(),
|
||||
// Skip terminal-agent teardown: identity files live under the REAL state
|
||||
// dir conventions and this suite must stay hermetic.
|
||||
ownsTerminalAgent: false,
|
||||
};
|
||||
}
|
||||
|
||||
describe('suppressed watchdog still reaps tunnel orphans (behavioral)', () => {
|
||||
// A PID above darwin/linux default pid_max: process.kill(pid, 0) throws
|
||||
// ESRCH, which the tick reads as "parent exited".
|
||||
const DEAD_PID = 999_999;
|
||||
let scratch: string;
|
||||
const savedChromiumProfile = process.env.CHROMIUM_PROFILE;
|
||||
|
||||
beforeEach(() => {
|
||||
scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'watchdog-tick-'));
|
||||
// shutdown() runs cleanSingletonLocks(resolveChromiumProfile()); point it
|
||||
// at scratch so the operator's real profile is never inspected.
|
||||
process.env.CHROMIUM_PROFILE = path.join(scratch, 'chromium-profile');
|
||||
__resetRegistry();
|
||||
__testInternals__.setTunnelActive(false);
|
||||
__testInternals__.setLastActivity(Date.now());
|
||||
__testInternals__.resetShutdownState();
|
||||
__testInternals__.resetParentWatchdogState();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedChromiumProfile === undefined) delete process.env.CHROMIUM_PROFILE;
|
||||
else process.env.CHROMIUM_PROFILE = savedChromiumProfile;
|
||||
__testInternals__.setTunnelActive(false);
|
||||
__testInternals__.resetShutdownState();
|
||||
__testInternals__.resetParentWatchdogState();
|
||||
try { fs.rmSync(scratch, { recursive: true, force: true }); } catch {}
|
||||
});
|
||||
|
||||
// Drain the fire-and-forget shutdown promise chain (flushBuffers + close)
|
||||
// the same way server-factory.test.ts does before asserting on exit.
|
||||
async function drainShutdown(): Promise<void> {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise<void>((r) => setImmediate(r));
|
||||
await new Promise<void>((r) => setImmediate(r));
|
||||
}
|
||||
|
||||
test('after promotion suppression, parent death does NOT shut down a headed daemon (#2565)', async () => {
|
||||
const exitMock = mock((_code?: number) => {});
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
buildFetchHandler(makeMinimalConfig('headed', scratch));
|
||||
__testInternals__.suppressHeadedParentShutdown(); // what handoff promotion triggers
|
||||
__testInternals__.parentWatchdogTick(DEAD_PID);
|
||||
await drainShutdown();
|
||||
expect(exitMock).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
(process as any).exit = originalExit;
|
||||
}
|
||||
});
|
||||
|
||||
test('CRITICAL: suppression active + tunnel live — parent death still shuts down', async () => {
|
||||
const exitMock = mock((_code?: number) => {});
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
buildFetchHandler(makeMinimalConfig('headed', scratch));
|
||||
__testInternals__.suppressHeadedParentShutdown();
|
||||
__testInternals__.setTunnelActive(true); // handoff → resume → /pair-agent tunnel
|
||||
__testInternals__.parentWatchdogTick(DEAD_PID);
|
||||
await drainShutdown();
|
||||
// The tick is the ONLY reaper for tunnel orphans (idle timeout is
|
||||
// disabled in tunnel mode). If this fails, an internet-exposed daemon
|
||||
// outlives its parent forever.
|
||||
expect(exitMock).toHaveBeenCalled();
|
||||
} finally {
|
||||
(process as any).exit = originalExit;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* Static tripwire for #1835: child spawns reachable on Windows must pass
|
||||
* windowsHide, or every daemon relaunch / taskkill / icacls / powershell
|
||||
* invocation flashes a black console window (and can steal focus).
|
||||
*
|
||||
* Source-level, same style as server-auth.test.ts / cdp-session-cleanup.test.ts:
|
||||
* cheap, deterministic, runs on every platform.
|
||||
*/
|
||||
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
const SRC = (f: string) => fs.readFileSync(path.join(import.meta.dir, '../src', f), 'utf-8');
|
||||
|
||||
/** Every occurrence of `needle` in `src` must have `windowsHide` within the
|
||||
* next `window` chars (the spawn's options object). */
|
||||
function expectHideNearEvery(src: string, needle: string, window = 400): void {
|
||||
let idx = src.indexOf(needle);
|
||||
expect(idx).toBeGreaterThanOrEqual(0);
|
||||
while (idx !== -1) {
|
||||
const slice = src.slice(idx, idx + window);
|
||||
expect(slice).toMatch(/windowsHide:\s*true/);
|
||||
idx = src.indexOf(needle, idx + needle.length);
|
||||
}
|
||||
}
|
||||
|
||||
describe('windowsHide on Windows-reachable spawns (#1835)', () => {
|
||||
test('daemon launch paths in cli.ts pass windowsHide', () => {
|
||||
const cli = SRC('cli.ts');
|
||||
// Installed path: node -e launcher — both the outer spawnSync and the
|
||||
// inner detached daemon spawn (inside the launcher code string).
|
||||
expect(cli).toContain('detached:true,windowsHide:true');
|
||||
expectHideNearEvery(cli, "'-e', launcherCode]");
|
||||
// Dev fallback: detached bun spawn.
|
||||
expectHideNearEvery(cli, "nodeSpawn('bun'");
|
||||
// taskkill (killServer).
|
||||
expectHideNearEvery(cli, "'taskkill'");
|
||||
});
|
||||
|
||||
test('Windows-only process probes pass windowsHide', () => {
|
||||
// tasklist in isProcessAlive — runs in polling loops.
|
||||
expectHideNearEvery(SRC('error-handling.ts'), "'tasklist'");
|
||||
// powershell DPAPI + tasklist in cookie import.
|
||||
const cookie = SRC('cookie-import-browser.ts');
|
||||
expectHideNearEvery(cookie, "'powershell'");
|
||||
expectHideNearEvery(cookie, "'tasklist'");
|
||||
});
|
||||
|
||||
test('icacls calls in file-permissions.ts pass windowsHide', () => {
|
||||
const perms = SRC('file-permissions.ts');
|
||||
expect((perms.match(/'icacls'/g) || []).length).toBeGreaterThanOrEqual(3);
|
||||
expectHideNearEvery(perms, "'icacls'");
|
||||
});
|
||||
|
||||
test('terminal-agent respawn in terminal-agent-control.ts passes windowsHide', () => {
|
||||
// The CLI cold-start + v1.44 watchdog respawn path. On Windows it runs
|
||||
// through the Node polyfill (dist/bun-polyfill.cjs) whose host default is
|
||||
// the opposite of Bun's — a visible console window on every watchdog
|
||||
// respawn is the symptom when the flag is dropped. Wider window: the
|
||||
// spawn's options object carries the full env wiring before the flag.
|
||||
expectHideNearEvery(SRC('terminal-agent-control.ts'), '(Bun as any).spawn(', 700);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user