diff --git a/open-gstack-browser/SKILL.md.tmpl b/open-gstack-browser/SKILL.md.tmpl index 5ea932ea7..97713b651 100644 --- a/open-gstack-browser/SKILL.md.tmpl +++ b/open-gstack-browser/SKILL.md.tmpl @@ -30,36 +30,73 @@ anti-bot stealth, and custom branding. You see every action in real time. {{BROWSE_SETUP}} -## Step 0: Pre-flight cleanup +## Step 0: Check for a running browse daemon -Before connecting, kill any stale browse servers and clean up lock files that -may have persisted from a crash. This prevents "already connected" false -positives and Chromium profile lock conflicts. +A running browse daemon may hold open tabs, cookies and logged-in sessions, +and replacing it loses them. Probe without starting one +(`BROWSE_NO_AUTOSTART=1` keeps `status` from booting a daemon): ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)"; : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -# Kill any existing browse server -if [ -f "$(git rev-parse --show-toplevel 2>/dev/null)/.gstack/browse.json" ]; then - _OLD_PID=$(cat "$(git rev-parse --show-toplevel)/.gstack/browse.json" 2>/dev/null | grep -o '"pid":[[:space:]]*[0-9]*' | grep -o '[0-9]*') - [ -n "$_OLD_PID" ] && kill "$_OLD_PID" 2>/dev/null || true - sleep 1 - [ -n "$_OLD_PID" ] && kill -9 "$_OLD_PID" 2>/dev/null || true - rm -f "$(git rev-parse --show-toplevel)/.gstack/browse.json" -fi -# Clean Chromium profile locks (can persist after crashes) -_PROFILE_DIR="$GSTACK_STATE_ROOT/chromium-profile" -for _LF in SingletonLock SingletonSocket SingletonCookie; do - rm -f "$_PROFILE_DIR/$_LF" 2>/dev/null || true -done -echo "Pre-flight cleanup done" +_STATUS=$(BROWSE_NO_AUTOSTART=1 $B status 2>&1); _STATUS_RC=$? +printf '%s\n' "$_STATUS" | head -5 +if [ "$_STATUS_RC" -ne 0 ]; then echo "DAEMON: none" +elif printf '%s' "$_STATUS" | grep -q 'Mode: headed'; then echo "DAEMON: headed" +else echo "DAEMON: live"; fi ``` +- **`DAEMON: none`**: no daemon answered. Clear Chromium profile locks left + by a crash, then run Step 1's plain `$B connect`. The CLI reaps orphaned + Chromium and stale state itself, and it still refuses to replace a daemon + that is alive but too busy to answer; if it refuses, show its output and + stop. + + ```bash + eval "$(~/.claude/skills/gstack/bin/gstack-paths)"; : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" + _PROFILE_DIR="$GSTACK_STATE_ROOT/chromium-profile" + for _LF in SingletonLock SingletonSocket SingletonCookie; do + rm -f "$_PROFILE_DIR/$_LF" 2>/dev/null || true + done + ``` + +- **`DAEMON: headed`**: GStack Browser is already open. Step 1's plain + `$B connect` reports that; continue to Step 2. + +- **`DAEMON: live`**: a headless daemon is running. With `SESSION_KIND: + spawned` or `headless`, do not ask and do not replace it. Print this and + stop: + + ```bash + printf 'Live browse daemon left running. Run %s stop, then re-run /open-gstack-browser to replace it.\n' "$B" + ``` + + Otherwise AskUserQuestion. Replacing the daemon cannot be undone: + + > "A browse daemon is already running (tabs and logins may be active). + > Opening GStack Browser replaces it, and everything in that daemon is + > lost." + > + > Recommendation: B unless you are done with the running session. + + Options: + - A) Replace it (runs `$B connect --force-restart`; its tabs, cookies and logins are lost) + - B) Keep it running and stop here + + Only an explicit A runs Step 1 with `--force-restart`. On B, or a reply + that is not clearly A, print the "Live browse daemon left running" line + above and stop. + ## Step 1: Connect ```bash $B connect ``` +After an explicit A in Step 0 only: + +```bash +$B connect --force-restart +``` + This launches GStack Browser (rebranded Chromium) in headed mode with: - A visible window you can watch (not your regular Chrome — it stays untouched) - The gstack sidebar extension auto-loaded via `launchPersistentContext` diff --git a/test/open-gstack-browser-liveness.test.ts b/test/open-gstack-browser-liveness.test.ts new file mode 100644 index 000000000..c893c6ea0 --- /dev/null +++ b/test/open-gstack-browser-liveness.test.ts @@ -0,0 +1,68 @@ +/** + * /open-gstack-browser must not kill a live browse daemon on its own. + * + * Step 0 probes with `$B status` (without auto-starting a daemon). No live + * daemon → plain `$B connect`, which reaps orphans and refuses to replace a + * busy daemon itself. A live daemon → AskUserQuestion, and only an explicit + * yes runs `$B connect --force-restart`. Spawned or headless sessions leave it + * running and print how to replace it. The skill never kills the PID recorded + * in .gstack/browse.json by hand. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const TMPL = fs.readFileSync(path.join(ROOT, 'open-gstack-browser', 'SKILL.md.tmpl'), 'utf-8'); +// Printed with the resolved browse binary path in place of %s. +const STOP_LINE = 'Live browse daemon left running. Run %s stop, then re-run /open-gstack-browser to replace it.'; + +const section = (from: string, to: string) => { + const start = TMPL.indexOf(from); + const end = TMPL.indexOf(to, start + from.length); + expect(start).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + return TMPL.slice(start, end); +}; + +describe('/open-gstack-browser daemon liveness', () => { + test('no hand-rolled kill of the recorded daemon PID', () => { + expect(TMPL).not.toMatch(/\bkill\b[^\n]*(_OLD_PID|\$\w*PID)/); + expect(TMPL).not.toMatch(/kill -9/); + expect(TMPL).not.toMatch(/grep -o '"pid":/); + expect(TMPL).not.toMatch(/rm -f[^\n]*browse\.json/); + }); + + test('Step 0 probes with $B status without auto-starting a daemon, before any connect', () => { + const probeAt = TMPL.indexOf('BROWSE_NO_AUTOSTART=1 $B status'); + expect(probeAt).toBeGreaterThanOrEqual(0); + expect(probeAt).toBeLessThan(TMPL.indexOf('$B connect')); + }); + + test('--force-restart appears only in the explicit-yes branch', () => { + const live = section('`DAEMON: live`', '## Step 2'); + const ask = live.indexOf('AskUserQuestion'); + expect(ask).toBeGreaterThanOrEqual(0); + const occurrences = [...TMPL.matchAll(/--force-restart/g)].map((m) => m.index!); + expect(occurrences.length).toBeGreaterThan(0); + const liveStart = TMPL.indexOf('`DAEMON: live`'); + const askAbs = liveStart + ask; + for (const at of occurrences) expect(at).toBeGreaterThan(askAbs); + expect(live).toMatch(/only an explicit a runs[^\n]*--force-restart/i); + expect(TMPL).toMatch(/after an explicit a[^\n]*\n+```bash\n\$B connect --force-restart\n```/i); + }); + + test('the no-daemon branch runs plain connect and keeps the Chromium profile-lock cleanup', () => { + const none = section('`DAEMON: none`', '`DAEMON: headed`'); + expect(none).toContain('plain `$B connect`'); + expect(none).toContain('SingletonLock SingletonSocket SingletonCookie'); + expect(none).not.toContain('--force-restart'); + }); + + test('spawned or headless sessions leave the daemon running and print the resolved-binary stop line', () => { + const live = section('`DAEMON: live`', '## Step 1'); + expect(live).toMatch(/spawned[\s\S]{0,40}headless[\s\S]{0,40}do not ask/i); + expect(live).toContain(`printf '${STOP_LINE}\\n' "$B"`); + expect(live.indexOf(STOP_LINE)).toBeLessThan(live.indexOf('AskUserQuestion')); + }); +}); diff --git a/test/skill-browse-state-extraction.test.ts b/test/skill-browse-state-extraction.test.ts index 597e2a5fd..fcf254746 100644 --- a/test/skill-browse-state-extraction.test.ts +++ b/test/skill-browse-state-extraction.test.ts @@ -4,20 +4,14 @@ import * as os from 'os'; import * as path from 'path'; import { execFileSync } from 'child_process'; -// Tripwire for the pid/port extraction snippets in /open-gstack-browser. +// Tripwire for the port extraction snippet in /open-gstack-browser. // -// Step 0 (pre-flight cleanup) reads the stale daemon's pid out of -// .gstack/browse.json to kill it; Step 2 reads the port back to tell the user -// which one the Side Panel needs. Both used `grep -o '"pid":[0-9]*'`, which -// cannot match: every writer of that file in browse/src/server.ts serializes -// with `JSON.stringify(state, null, 2)`, so the real bytes are `"pid": 12060` -// — colon, SPACE, digits. -// -// The failure was silent in the worst way. `_OLD_PID` came back empty, the -// `kill` never ran, browse.json was deleted anyway, and the next `connect` -// died with "existing daemon has different config (proxy/headed mismatch)" -// — an error that points at proxy/headed flags, not at the cleanup that -// no-opped. Observed 2026-08-28 against a daemon left over from a reboot. +// Step 2 reads the port out of .gstack/browse.json to tell the user which one +// the Side Panel needs. A `grep -o '"port":[0-9]*'` pattern cannot match: +// every writer of that file in browse/src/server.ts serializes with +// `JSON.stringify(state, null, 2)`, so the real bytes are `"port": 34567` +// — colon, SPACE, digits. (The skill no longer reads the pid at all: the +// browse CLI owns daemon liveness and replacement.) // // So this test does not match strings; it RUNS the snippets the skill tells // the agent to run, against a state file written exactly the way the server @@ -38,7 +32,7 @@ function writeStateFile(dir: string, pid: number, port: number): string { } /** Pull the grep pipeline for `field` out of the skill prose and run it. */ -function extractViaSkill(source: string, field: 'pid' | 'port', stateFile: string): string { +function extractViaSkill(source: string, field: 'port', stateFile: string): string { const line = source .split('\n') .find((l) => l.includes(`grep -o '"${field}":`)); @@ -54,12 +48,11 @@ function extractViaSkill(source: string, field: 'pid' | 'port', stateFile: strin describe('/open-gstack-browser state-file extraction', () => { for (const [label, file] of [['generated', SKILL], ['template', TMPL]] as const) { - test(`${label}: pid and port survive the pretty-printed state file`, () => { + test(`${label}: the port survives the pretty-printed state file`, () => { const source = fs.readFileSync(file, 'utf-8'); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-state-')); try { const stateFile = writeStateFile(dir, 12060, 34567); - expect(extractViaSkill(source, 'pid', stateFile)).toBe('12060'); expect(extractViaSkill(source, 'port', stateFile)).toBe('34567'); } finally { fs.rmSync(dir, { recursive: true, force: true });