mirror of
https://github.com/garrytan/gstack.git
synced 2026-07-05 23:57:53 +02:00
feat(server): factory-export API surface + import.meta.main gate
Surfaces the embedder API gbrowser (phoenix) needs to consume gstack as a submodule, and gates module-load side effects so the file is safe to import without auto-starting a daemon. Changes to browse/src/server.ts: - AUTH_TOKEN now honors process.env.AUTH_TOKEN (trimmed) before falling back to crypto.randomUUID(). Whitespace-only values are rejected so the security boundary can't be silently weakened. - New exported types: ServerConfig and ServerHandle. ServerConfig documents the full factory contract (authToken, browsePort, idleTimeoutMs, config, browserManager, chromiumProfile, xvfb, proxyBridge, startTime, beforeRoute). ServerHandle documents the return shape (fetchLocal, fetchTunnel, shutdown, stopListeners). Caller-owned lifecycle annotations on xvfb and proxyBridge prevent double-close bugs from surprise ownership. - New exported function: resolveConfigFromEnv() builds a ServerConfig-shaped object from process.env for CLI use. Embedders construct their own ServerConfig explicitly. - start() is now exported. Embedders can call it with env vars set as a v1 escape hatch until full buildFetchHandler extraction lands. - Signal handlers (SIGINT, SIGTERM, Windows exit, uncaughtException, unhandledRejection) and the auto-kickoff at module bottom are now wrapped in `if (import.meta.main)`. CLI path is unchanged. Embedders register their own handlers. - shutdown() and emergencyCleanup() now call cleanSingletonLocks( resolveChromiumProfile()) instead of inline path+loop. Single implementation, defensive guard, honors per-workspace CHROMIUM_PROFILE. New tests: - browse/test/server-no-import-side-effects.test.ts: spawns a fresh Bun subprocess that imports server.ts, asserts no signal handlers registered, no state-dir populated. Guards the core refactor invariant from regression. - browse/test/server-factory.test.ts: 12 tests covering AUTH_TOKEN env behavior (honored, whitespace-rejected, trimmed), preserved exports (TUNNEL_COMMANDS, canDispatchOverTunnel), and ServerConfig/ServerHandle type compatibility. Deferred to follow-up PR: full buildFetchHandler extraction that hoists the 13 module-level mutables + helpers into a factory closure. Phoenix can ship v0.6.0.0 against the start()+env surface today; the cleaner factory comes next. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -0,0 +1,164 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
resolveConfigFromEnv,
|
||||
type ServerConfig,
|
||||
type ServerHandle,
|
||||
type Surface,
|
||||
} from '../src/server';
|
||||
import { TUNNEL_COMMANDS, canDispatchOverTunnel } from '../src/server';
|
||||
|
||||
/**
|
||||
* Tests for the factory-export API surface added so gbrowser (phoenix) can
|
||||
* consume gstack as a submodule. The full buildFetchHandler hybrid hoist is
|
||||
* deferred to a follow-up PR; this test file proves the type contract,
|
||||
* resolveConfigFromEnv behavior, and preserved exports.
|
||||
*/
|
||||
describe('server.ts factory API surface', () => {
|
||||
describe('resolveConfigFromEnv', () => {
|
||||
test('honors AUTH_TOKEN env var', () => {
|
||||
const orig = process.env.AUTH_TOKEN;
|
||||
process.env.AUTH_TOKEN = 'fixed-test-token-abc123';
|
||||
try {
|
||||
const cfg = resolveConfigFromEnv();
|
||||
expect(cfg.authToken).toBe('fixed-test-token-abc123');
|
||||
} finally {
|
||||
if (orig === undefined) delete process.env.AUTH_TOKEN;
|
||||
else process.env.AUTH_TOKEN = orig;
|
||||
}
|
||||
});
|
||||
|
||||
test('falls back to randomUUID when AUTH_TOKEN env is empty', () => {
|
||||
const orig = process.env.AUTH_TOKEN;
|
||||
process.env.AUTH_TOKEN = '';
|
||||
try {
|
||||
const cfg = resolveConfigFromEnv();
|
||||
// randomUUID returns a 36-char hex+dash string.
|
||||
expect(cfg.authToken).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/);
|
||||
} finally {
|
||||
if (orig === undefined) delete process.env.AUTH_TOKEN;
|
||||
else process.env.AUTH_TOKEN = orig;
|
||||
}
|
||||
});
|
||||
|
||||
test('falls back to randomUUID when AUTH_TOKEN is whitespace-only', () => {
|
||||
const orig = process.env.AUTH_TOKEN;
|
||||
process.env.AUTH_TOKEN = ' \t \n ';
|
||||
try {
|
||||
const cfg = resolveConfigFromEnv();
|
||||
expect(cfg.authToken).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/);
|
||||
expect(cfg.authToken.length).toBe(36);
|
||||
} finally {
|
||||
if (orig === undefined) delete process.env.AUTH_TOKEN;
|
||||
else process.env.AUTH_TOKEN = orig;
|
||||
}
|
||||
});
|
||||
|
||||
test('AUTH_TOKEN value is trimmed', () => {
|
||||
const orig = process.env.AUTH_TOKEN;
|
||||
process.env.AUTH_TOKEN = ' padded-token ';
|
||||
try {
|
||||
const cfg = resolveConfigFromEnv();
|
||||
expect(cfg.authToken).toBe('padded-token');
|
||||
} finally {
|
||||
if (orig === undefined) delete process.env.AUTH_TOKEN;
|
||||
else process.env.AUTH_TOKEN = orig;
|
||||
}
|
||||
});
|
||||
|
||||
test('reads BROWSE_PORT from env, defaults to 0', () => {
|
||||
const orig = process.env.BROWSE_PORT;
|
||||
process.env.BROWSE_PORT = '34567';
|
||||
try {
|
||||
expect(resolveConfigFromEnv().browsePort).toBe(34567);
|
||||
} finally {
|
||||
if (orig === undefined) delete process.env.BROWSE_PORT;
|
||||
else process.env.BROWSE_PORT = orig;
|
||||
}
|
||||
const origUnset = process.env.BROWSE_PORT;
|
||||
delete process.env.BROWSE_PORT;
|
||||
try {
|
||||
expect(resolveConfigFromEnv().browsePort).toBe(0);
|
||||
} finally {
|
||||
if (origUnset !== undefined) process.env.BROWSE_PORT = origUnset;
|
||||
}
|
||||
});
|
||||
|
||||
test('reads BROWSE_IDLE_TIMEOUT from env, defaults to 30 min (1800000ms)', () => {
|
||||
const orig = process.env.BROWSE_IDLE_TIMEOUT;
|
||||
delete process.env.BROWSE_IDLE_TIMEOUT;
|
||||
try {
|
||||
expect(resolveConfigFromEnv().idleTimeoutMs).toBe(1800000);
|
||||
} finally {
|
||||
if (orig !== undefined) process.env.BROWSE_IDLE_TIMEOUT = orig;
|
||||
}
|
||||
});
|
||||
|
||||
test('returns a populated config object with the expected shape', () => {
|
||||
const cfg = resolveConfigFromEnv();
|
||||
expect(cfg).toMatchObject({
|
||||
authToken: expect.any(String),
|
||||
browsePort: expect.any(Number),
|
||||
idleTimeoutMs: expect.any(Number),
|
||||
config: expect.objectContaining({
|
||||
stateDir: expect.any(String),
|
||||
stateFile: expect.any(String),
|
||||
auditLog: expect.any(String),
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('preserved exports', () => {
|
||||
test('TUNNEL_COMMANDS still exported and populated', () => {
|
||||
expect(TUNNEL_COMMANDS).toBeInstanceOf(Set);
|
||||
expect(TUNNEL_COMMANDS.size).toBeGreaterThan(0);
|
||||
expect(TUNNEL_COMMANDS.has('goto')).toBe(true);
|
||||
expect(TUNNEL_COMMANDS.has('click')).toBe(true);
|
||||
});
|
||||
|
||||
test('canDispatchOverTunnel still exported and functional', () => {
|
||||
expect(canDispatchOverTunnel('goto')).toBe(true);
|
||||
expect(canDispatchOverTunnel('shutdown')).toBe(false);
|
||||
expect(canDispatchOverTunnel(null)).toBe(false);
|
||||
expect(canDispatchOverTunnel(undefined)).toBe(false);
|
||||
expect(canDispatchOverTunnel('')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('type surface compiles', () => {
|
||||
// Compile-time shape checks. If these break, TypeScript fails to build
|
||||
// the test file — which is exactly the API-compat guarantee we want for
|
||||
// embedders depending on these types.
|
||||
test('Surface type accepts the two known values', () => {
|
||||
const local: Surface = 'local';
|
||||
const tunnel: Surface = 'tunnel';
|
||||
expect(local).toBe('local');
|
||||
expect(tunnel).toBe('tunnel');
|
||||
});
|
||||
|
||||
test('ServerConfig type accepts the documented minimum-required fields', () => {
|
||||
// This compiles only if ServerConfig accepts these field names + types.
|
||||
const minimalConfigShape = {
|
||||
authToken: 'tok',
|
||||
browsePort: 0,
|
||||
idleTimeoutMs: 1800000,
|
||||
config: { stateDir: '', stateFile: '', consoleLog: '', networkLog: '', dialogLog: '', auditLog: '', projectDir: '' },
|
||||
browserManager: {} as any,
|
||||
startTime: Date.now(),
|
||||
} satisfies Partial<ServerConfig>;
|
||||
expect(minimalConfigShape.authToken).toBe('tok');
|
||||
});
|
||||
|
||||
test('ServerHandle type exposes the documented surface', () => {
|
||||
// Compiles only if these property names exist on ServerHandle.
|
||||
type AssertHandleFields = ServerHandle extends {
|
||||
fetchLocal: any;
|
||||
fetchTunnel: any;
|
||||
shutdown: any;
|
||||
stopListeners: any;
|
||||
} ? true : false;
|
||||
const assertion: AssertHandleFields = true;
|
||||
expect(assertion).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,106 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import * as fs from 'fs';
|
||||
|
||||
/**
|
||||
* Guard the core refactor invariant: importing browse/src/server.ts must NOT
|
||||
* auto-start. Before this PR, the module called `start().catch(...)` at module
|
||||
* load time, which made the file impossible for embedders (gbrowser phoenix
|
||||
* overlay) to import without spawning a daemon. The fix wraps that kickoff in
|
||||
* `if (import.meta.main)` so the side effects only run when the module is the
|
||||
* process entry point.
|
||||
*
|
||||
* Approach: spawn a fresh Bun subprocess that imports the module and emits a
|
||||
* structured snapshot (initial vs post-import process state). Parent asserts
|
||||
* that no listeners were bound, no Bun.serve started, and no SIGINT handlers
|
||||
* were registered. The subprocess uses HOME=tmp + GSTACK_HOME=tmp so any
|
||||
* accidental state-dir write lands in a place we can verify is empty.
|
||||
*/
|
||||
describe('server.ts module import has no auto-start side effects', () => {
|
||||
test('importing server.ts does not bind Bun.serve, register signal handlers, or write state', async () => {
|
||||
const tmpHome = path.join(os.tmpdir(), `browse-no-sfx-${Date.now()}-${process.pid}`);
|
||||
fs.mkdirSync(tmpHome, { recursive: true });
|
||||
const tmpGstack = path.join(tmpHome, '.gstack');
|
||||
|
||||
const childScript = `
|
||||
const sigintBefore = process.listenerCount('SIGINT');
|
||||
const sigtermBefore = process.listenerCount('SIGTERM');
|
||||
const uncaughtBefore = process.listenerCount('uncaughtException');
|
||||
|
||||
// Snapshot any keys that look like our state path.
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
await import(${JSON.stringify(path.resolve(import.meta.dir, '../src/server.ts'))});
|
||||
|
||||
// After import, sleep a tick so any setTimeout(0)-style init can run.
|
||||
await new Promise(r => setTimeout(r, 50));
|
||||
|
||||
const sigintAfter = process.listenerCount('SIGINT');
|
||||
const sigtermAfter = process.listenerCount('SIGTERM');
|
||||
const uncaughtAfter = process.listenerCount('uncaughtException');
|
||||
|
||||
// Check that the gstack home directory wasn't populated as a side effect.
|
||||
let gstackPopulated = false;
|
||||
try {
|
||||
const entries = fs.readdirSync(${JSON.stringify(tmpGstack)});
|
||||
gstackPopulated = entries.length > 0;
|
||||
} catch {
|
||||
// Doesn't exist — that's the win we want.
|
||||
}
|
||||
|
||||
console.log(JSON.stringify({
|
||||
sigintBefore, sigintAfter,
|
||||
sigtermBefore, sigtermAfter,
|
||||
uncaughtBefore, uncaughtAfter,
|
||||
gstackPopulated,
|
||||
}));
|
||||
// Force exit so any background intervals don't keep this child alive
|
||||
// (the test framework would see a hang otherwise — which itself is a
|
||||
// signal that side effects DID run).
|
||||
process.exit(0);
|
||||
`;
|
||||
|
||||
const proc = Bun.spawn(['bun', '-e', childScript], {
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: tmpHome,
|
||||
GSTACK_HOME: tmpGstack,
|
||||
// Empty so the AUTH_TOKEN env path doesn't deterministically set a token.
|
||||
AUTH_TOKEN: '',
|
||||
// Force a stub state file so resolveConfig() at module load (if it
|
||||
// happens) won't crawl the host's real .gstack/.
|
||||
BROWSE_STATE_FILE: path.join(tmpGstack, 'browse.json'),
|
||||
},
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
});
|
||||
|
||||
const stdout = await new Response(proc.stdout).text();
|
||||
const stderr = await new Response(proc.stderr).text();
|
||||
await proc.exited;
|
||||
|
||||
// The last JSON line in stdout is our snapshot.
|
||||
const jsonLine = stdout.trim().split('\n').filter(l => l.startsWith('{')).pop();
|
||||
expect(jsonLine, `child stderr: ${stderr}`).toBeDefined();
|
||||
|
||||
const snapshot = JSON.parse(jsonLine!);
|
||||
|
||||
// No new signal handlers registered (gated on import.meta.main, which
|
||||
// is false in the subprocess because `bun -e` is the entry point).
|
||||
expect(snapshot.sigintAfter).toBe(snapshot.sigintBefore);
|
||||
expect(snapshot.sigtermAfter).toBe(snapshot.sigtermBefore);
|
||||
expect(snapshot.uncaughtAfter).toBe(snapshot.uncaughtBefore);
|
||||
|
||||
// gstack home should remain empty — initRegistry/initAuditLog/etc. side
|
||||
// effects from module load are acceptable (they happen at module level),
|
||||
// but only insofar as they don't bind listeners or write project state.
|
||||
// The presence/absence test here proves we didn't bind Bun.serve (which
|
||||
// would also try to write the state file).
|
||||
expect(snapshot.gstackPopulated).toBe(false);
|
||||
|
||||
// Cleanup
|
||||
try { fs.rmSync(tmpHome, { recursive: true, force: true }); } catch { /* best effort */ }
|
||||
}, 30_000);
|
||||
});
|
||||
Reference in New Issue
Block a user