Files
gstack/browse/test/cli-supervisor.test.ts
T
Garry Tanandcapy-ai[bot] 5755601dac v1.91.248.0 fix(browse): direct mode launches Chromium with --proxy-server=direct:// so proxy env vars never win silently (#3440 item 8) (#3448)
* fix(browse): direct mode launches Chromium with --proxy-server=direct:// so proxy env vars never win silently (#3440 item 8)

BROWSE_PROXY_URL=direct only stopped gstack from configuring a proxy; the daemon
inherits the environment and the launch passed no proxy switch, so Chromium read
HTTPS_PROXY/HTTP_PROXY/ALL_PROXY itself and every goto still went through that
proxy while browse reported a direct connection. Every launch path now adds
--proxy-server=direct:// when no proxy is configured (chrome-headless-shell
ignores --no-proxy-server), the daemon logs the effective proxy mode on launch,
and BROWSER.md's proxy section says what Chromium actually gets.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Name the direct-connection switch once (DIRECT_PROXY_ARG in proxy-config.ts) so the
launch argument and the daemon log line cannot drift apart.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(browse): force direct:// only where Chromium would pick its own proxy; review fixes

Red team (ship Step 9.2): the first cut passed --proxy-server=direct:// on every
platform, but Chromium reads HTTPS_PROXY/HTTP_PROXY/ALL_PROXY itself only on
Linux. On macOS and Windows it follows the system proxy settings (PAC/WPAD),
which GStack Browser.app users behind a corporate proxy depend on, and
BROWSE_PROXY_URL cannot name a PAC. forcesDirect(env, platform) now gates the
switch: Linux always, elsewhere only with BROWSE_PROXY_URL=direct. The daemon
log says what is really ignored per platform, or that the system settings
apply.

Review pass 2: the CLI e2e asserts on the daemon-log slice its own invocation
wrote (the log is append-mode and shared across cases); connectProxy moves to
browse/test/helpers/connect-proxy.ts for both e2e files; the CHANGELOG Tests
bullets cover the generated cases. Adds the coverage-audit cases
(unparseable HTTPS_PROXY; extension, headed and handoff paths under Xvfb).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(browse): assert the direct switch on the launched Chromium command line per platform (#3440 item 8)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Ship review pass 3 (red team, GStack Minder's decision B, 2026-10-26): on macOS
and Windows an explicit BROWSE_PROXY_URL=direct is the only thing that makes the
daemon bypass the system proxy settings, so explicitDirect() puts it in the
daemon config hash, server.ts records it as proxyDirect in the state file, and
buildRestartEnv re-emits it on a crash auto-restart. A reused daemon started on
the system proxy is now refused (the refusal names BROWSE_PROXY_URL and offers
browse stop or unsetting it) instead of silently keeping the system proxy while
the CLI reports direct. Linux hashes as before. BROWSER.md says the daemon log
records the mode each time the daemon starts, not on every launch. The Linux
desktop (GNOME/KDE) system-proxy question is filed as #3450.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(browse): daemon-state and refusal e2e for the explicit direct opt-out; headed env and BROWSER.md contract checks (#3440 item 8)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Ship rerun review pass: proxy-direct-launch-args.test.ts joins the macOS
named-regressions job so its omission arm (no switch without the opt-out) runs
in CI; serverProxyEnv takes the platform so the state-file predicate is pinned
per platform instead of mirroring process.platform; buildHeadedServerEnv's
parameter type requires explicitDirect so a narrowed caller cannot drop the
opt-out silently; BROWSER.md's precedence item 6 and the reuse sentence say
what each platform does (a connect daemon gets the generic flags refusal);
CHANGELOG notes the one-time refusal against a pre-release daemon.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Ship adversarial pass: a crash restart re-emits BROWSE_PROXY_URL=direct from the
daemon state only when the invocation has no flags, so the env it hands the new
daemon never contradicts the config hash it also hands over; an explicit-direct
session is refused against a daemon with no recorded config hash, like --proxy
and --headed already were; the unparseable-variable hint says that the opt-out
also bypasses the system proxy settings on macOS and Windows; BROWSER.md notes
that a managed Chromium proxy policy still overrides the switch; CHANGELOG names
both files that joined the macOS named-regressions job.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(browse): read scheme-less proxy variables as http:// so the explicit direct launch keeps those sessions proxied (#3440 item 8)

A standard proxy variable without a scheme (curl-style host:port or
user:pass@host:port) was rejected by browse's parser with a "connects
directly" warning while Chromium on Linux read the variable itself, so the
session was proxied anyway. With the launch now explicit
(--proxy-server=direct://) that environment went genuinely direct. resolveProxy
now reads such values as http://, the way curl and Chromium's own environment
reader take them; --proxy and BROWSE_PROXY_URL still require the scheme, and a
value that does not parse even as http:// still warns and connects directly.

Also drops buildRestartEnv's persisted-state re-emit branch, which production
never reached (_globalFlags is set before any restart), names the four
BrowserManager launch sites in the CHANGELOG and excludes the cookie-import
worker, and carries the docs audit's one-line BROWSER.md project-tree note.

Decision: GStack Minder (2026-10-26), option A.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

The lenient scheme-less proxy read is now gated on the plain
[user:pass@]host[:port] shape (SCHEMELESS_PROXY_RE, trimmed): a scheme'd value
with surrounding whitespace, a one-slash typo and a bare number keep the
behaviour they had, and a credential that would spill past the userinfo
(?, #, /) is not rewritten, so nothing can land outside the redacted part.
Credentials that are not valid percent-encoding are a ProxyConfigError for
every source instead of a URIError crash. buildRestartEnv's docblock, BROWSER.md
and the CHANGELOG describe the parse (port 80 default, macOS/Windows now treat
such a value as a configured proxy).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

A backslash is excluded from the scheme-less proxy shape and parseProxyConfig
refuses any proxy URL with a path, query or fragment, for every source: WHATWG
parsing ends an http authority at a backslash, so a DOMAIN\user login or a /
in a password landed in the path, which the redactor does not mask, and was
printed in the clear on the start line, in the daemon log and in the state
file. Such values now warn (standard variables) or error (--proxy,
BROWSE_PROXY_URL) without echoing the credential. Docs updated.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(browse): keep a slash-less scheme spelling, refuse a bare ? or #, and tighten the proxy docs

A standard proxy variable whose userinfo opens with a known scheme
(https:bob:pw@host:3128, which the URL parser reads as https://) is no
longer taken as scheme-less credentials, so the protocol it names is kept.
parseProxyConfig refuses a literal ? or # anywhere in the value, not only
a non-empty query or fragment. The unparseable-variable test now puts the
credential in every looped value, so its no-echo assertion is real; new
cases pin the slash-less scheme spelling, the bare ?/#, an env-sourced
trailing slash, and that mixed credential sources stay a hard error for
the scheme-less spelling as for the http:// one (the cred-mixing refusal
predates this branch and its test on main pins it, so it is documented
rather than softened).

CHANGELOG and BROWSER.md: single backslashes inside code spans, "a path
other than a bare /", the socks5 bridge log lines in the effective-mode
list, and the character list applies anywhere in the value.

Decision: GStack Minder (2026-10-26), option B.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(browse): keep a basic-auth URL out of the scheme-guard comment so the pregate secret scan stays clean

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(browse): pin the mismatch remedy per platform so the Windows subset does not take process.platform

proxyMismatchLines defaulted to process.platform in the env-proxy-meets-direct-daemon case, so on the windows-free-tests lane the remedy read 'or unset HTTPS_PROXY' and the assertion failed. The case now passes 'linux' explicitly and pins the darwin/win32 remedy beside it. Test only; source unchanged since 1f6c85af.

Decision: GStack Minder (2026-10-26), option A.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(browse): the one-slash http:/host:3128 spelling was never a diagnostic; say what the parser does with it

The CHANGELOG sentence and the SCHEMELESS_PROXY_RE docblock claimed a one-slash scheme 'keeps its diagnostic'; the URL parser has always read it as http://host:3128/ and browse/test/proxy-env.test.ts:83 pins that. Wording only, found by the Step 14.5 docs audit on 1f6c85af.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(browse): percent-encoding advice per scheme, and state the credential-free-path narrowing and the new scheme-less cred-mix refusal

BROWSER.md said to percent-encode / \ ? # @ in http(s) credentials; ? and # are refused for every scheme, @ needs no encoding, and only the backslash is http(s)-specific. The CHANGELOG now states the narrowing that a credential-free standard-variable value with a path (http://proxy.corp:3128/pac) used to be read with the path dropped and now warns and goes direct, and that the cred-mix refusal is new for the scheme-less spelling. Wording only.

Decision: GStack Minder (2026-10-26), option A.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(browse): only host:port was Chromium's own reading; user:pass@host:port is newly proxied

Chromium's proxy spec parser (net/base/proxy_string_util.cc, ProxySchemeHostAndPortToProxyServer) returns an invalid server for any userinfo, so a scheme-less user:pass@host:port in HTTPS_PROXY was never honoured by Chromium on Linux and went direct; reading it as http:// follows curl and is new behaviour. BROWSER.md, the CHANGELOG, the resolveProxy docblock and the Linux e2e test comment now say so, and the scheme-less spill test is named for what it proves (those values never reach the path refusal; the http:// cases in the next test do). Wording only.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
2026-10-26 13:56:12 +00:00

195 lines
9.1 KiB
TypeScript

import { describe, test, expect } from 'bun:test';
import * as fs from 'fs';
import * as path from 'path';
import {
buildHeadedServerEnv,
runHeadedSupervisor,
SUPERVISOR_GUARD_WINDOW_MS,
type HeadedSupervisorDeps,
} from '../src/cli';
// v1.44 outer supervisor — static-grep invariants.
//
// Pre-v1.44 `$B connect` was fire-and-forget: spawn server detached, CLI
// exits, server runs unsupervised. If the server crashed, the user had to
// re-run `$B connect`. The opt-in supervisor (--supervise or
// BROWSE_SUPERVISE=1) keeps the CLI attached and respawns the server on
// unexpected exit, with the same crash-loop guard shape as the v1.44
// terminal-agent watchdog.
//
// The static tripwires below defend the wiring in main(): opt-in by default,
// signal handlers, env knobs. The behavioral block drives the extracted
// runHeadedSupervisor loop with injected clock, sleep, and process probes —
// the respawn path shipped broken (a block-scoped env) because only source
// text was checked.
const CLI_TS = path.resolve(import.meta.path, '..', '..', 'src', 'cli.ts');
describe('CLI outer supervisor (v1.44+)', () => {
test('1. supervisor is opt-in via --supervise flag or BROWSE_SUPERVISE env', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
expect(src).toContain("commandArgs.includes('--supervise')");
expect(src).toContain("process.env.BROWSE_SUPERVISE === '1'");
// Default path MUST still exit 0 promptly. The legacy contract is
// that every caller of `$B connect` (Claude Code Bash tool, scripts,
// CI) gets a prompt return.
expect(src).toMatch(/if \(!superviseRequested\) \{\s*process\.exit\(0\);\s*\}/);
});
test('2. SIGINT and SIGTERM trigger clean teardown', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
// Both signals must hit the teardown path or the user's Ctrl-C leaves
// an orphaned server (worse than no supervisor).
expect(src).toMatch(/process\.on\('SIGINT'.*teardownAndExit/);
expect(src).toMatch(/process\.on\('SIGTERM'.*teardownAndExit/);
// Teardown must signal the supervised server before exiting itself.
expect(src).toContain("safeKill(state.pid, 'SIGTERM')");
});
test('3. crash-loop guard with 5-in-5min rolling window', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
expect(src).toContain('SUPERVISOR_GUARD_WINDOW_MS = 5 * 60_000');
expect(src).toContain('SUPERVISOR_GUARD_MAX = 5');
// Window pruning: a long-lived daemon with sporadic crashes must NOT
// hit the guard (otherwise we punish the user for the supervisor doing
// its job).
expect(src).toMatch(/respawns\.shift\(\)/);
});
test('4. exponential backoff schedule, env-overridable', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
expect(src).toContain('GSTACK_SUPERVISOR_BACKOFF');
// Default schedule must include short waits at first (rapid recovery
// from transient crashes) and cap at a sensible long wait.
expect(src).toContain('1000,2000,4000,8000,30000');
});
test('5. tick interval is env-overridable for tests', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
expect(src).toContain('GSTACK_SUPERVISOR_TICK_MS');
});
test('6. respawned server gets a fresh terminal-agent too', () => {
const src = fs.readFileSync(CLI_TS, 'utf-8');
// After server respawn, the terminal-agent state is stale (old PID
// record points to a dead agent that exited with its parent). The
// supervisor must re-call spawnTerminalAgent or the PTY path stays
// broken even though the server is back up.
const block = sliceBetween(src, 'Supervisor mode:', '// ─── Headed Disconnect');
expect(block).toContain('spawnTerminalAgent({');
});
});
// A scripted world for runHeadedSupervisor: `alive` decides the PID probe per
// tick, sleep advances the injected clock, and every side effect is recorded.
function harness(opts: {
alive: (tick: number) => boolean;
startServer?: (call: number) => Promise<{ pid: number; port: number }>;
spawnTerminalAgent?: () => void;
tickMs?: number;
exitAfterSleeps?: number;
}) {
let clock = 1_000_000, sleeps = 0, tick = 0, exiting = false, starts = 0;
const calls = { startEnv: [] as Record<string, string>[], agents: [] as number[], log: [] as string[], warn: [] as string[], error: [] as string[] };
const deps: HeadedSupervisorDeps = {
env: buildHeadedServerEnv({ proxyUrl: 'socks5://127.0.0.1:9050', configHash: 'abc123', explicitDirect: false }),
tickMs: opts.tickMs ?? 30_000,
backoffMs: [1000, 2000, 4000, 8000, 30000],
daemonLog: '/state/browse-daemon.log',
readState: () => ({ pid: 4242 }),
isProcessAlive: () => opts.alive(tick++),
startServer: async (env) => {
calls.startEnv.push(env);
const call = starts++;
return opts.startServer ? opts.startServer(call) : { pid: 5000 + call, port: 34567 };
},
spawnTerminalAgent: (server) => { calls.agents.push(server.pid); opts.spawnTerminalAgent?.(); },
sleep: async (ms) => {
clock += ms; sleeps++;
if (opts.exitAfterSleeps !== undefined && sleeps >= opts.exitAfterSleeps) exiting = true;
},
now: () => clock,
isExiting: () => exiting,
log: (line) => calls.log.push(line),
warn: (line) => calls.warn.push(line),
error: (line) => calls.error.push(line),
};
return { deps, calls, stop: () => { exiting = true; } };
}
describe('runHeadedSupervisor (behavior)', () => {
test('a dead server is respawned with exactly the initial connect env, and its terminal agent too', async () => {
const h = harness({ alive: (t) => t !== 0, exitAfterSleeps: 4 });
expect(await runHeadedSupervisor(h.deps)).toBe('stopped');
expect(h.calls.startEnv).toHaveLength(1);
expect(h.calls.startEnv[0]).toEqual({
BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0',
BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_CONFIG_HASH: 'abc123',
});
expect(h.calls.startEnv[0]).toBe(h.deps.env);
expect(h.calls.agents).toEqual([5000]);
expect(h.calls.error).toEqual([]);
expect(h.calls.log.join('\n')).toContain('server respawned (PID 5000, port 34567)');
});
test('a failed respawn is logged with the daemon log path and counted toward the guard', async () => {
const h = harness({ alive: () => false, startServer: async () => { throw new Error('port 34567 busy'); } });
expect(await runHeadedSupervisor(h.deps)).toBe('gave_up');
const failures = h.calls.error.filter(line => line.includes('server respawn failed'));
expect(failures).toHaveLength(5);
expect(failures[0]).toBe('[browse] Supervisor: server respawn failed: port 34567 busy. Daemon log: /state/browse-daemon.log');
});
test('five crashes inside the window give up with the cause and the relaunch command', async () => {
const h = harness({ alive: () => false });
expect(await runHeadedSupervisor(h.deps)).toBe('gave_up');
expect(h.calls.startEnv).toHaveLength(5);
expect(h.calls.error.at(-1)).toBe(
'[browse] Supervisor: 5 server crashes in 300s, giving up. Crash reasons: /state/browse-daemon.log. Relaunch: $B connect --supervise',
);
});
test('crashes spread wider than the rolling window never trip the guard', async () => {
// One crash per tick with a tick longer than the window: every earlier
// respawn is pruned before the guard is checked.
const h = harness({ alive: (t) => t >= 12, tickMs: SUPERVISOR_GUARD_WINDOW_MS + 1, exitAfterSleeps: 30 });
expect(await runHeadedSupervisor(h.deps)).toBe('stopped');
expect(h.calls.startEnv).toHaveLength(12);
expect(h.calls.error).toEqual([]);
});
test('a terminal-agent failure after a successful respawn warns and keeps supervising', async () => {
const h = harness({ alive: (t) => t !== 0, spawnTerminalAgent: () => { throw new Error('no pty'); }, exitAfterSleeps: 4 });
expect(await runHeadedSupervisor(h.deps)).toBe('stopped');
expect(h.calls.warn.some(line => line === '[browse] Supervisor: terminal-agent respawn failed: no pty')).toBe(true);
expect(h.calls.error).toEqual([]);
});
test('an exit requested during backoff stops without starting a server', async () => {
// Sleep 1 is the tick, sleep 2 the backoff; exiting flips during backoff.
const h = harness({ alive: () => false, exitAfterSleeps: 2 });
expect(await runHeadedSupervisor(h.deps)).toBe('stopped');
expect(h.calls.startEnv).toEqual([]);
});
test('a live server is left alone', async () => {
const h = harness({ alive: () => true, exitAfterSleeps: 5 });
expect(await runHeadedSupervisor(h.deps)).toBe('stopped');
expect(h.calls.startEnv).toEqual([]);
});
});
describe('buildHeadedServerEnv', () => {
test('omits proxy and config hash when this invocation has none', () => {
expect(buildHeadedServerEnv({ proxyUrl: null, configHash: '', explicitDirect: false })).toEqual({ BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0' });
});
});
function sliceBetween(source: string, start: string, end: string): string {
const i = source.indexOf(start);
if (i === -1) throw new Error(`marker not found: ${start}`);
const j = source.indexOf(end, i + start.length);
if (j === -1) throw new Error(`end marker not found: ${end}`);
return source.slice(i, j);
}