mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
W4: /open-gstack-browser asks before replacing a live browse daemon
Step 0 probes with BROWSE_NO_AUTOSTART=1 $B status (no daemon is booted by the probe). No live daemon: clear Chromium profile locks and run plain $B connect, which reaps orphans and refuses to replace a busy daemon. Already headed: connect reports it. Live headless daemon: AskUserQuestion, and only an explicit yes runs $B connect --force-restart; spawned or headless sessions leave it running and print the stop line with the resolved browse binary. The hand-rolled kill / kill -9 of the recorded PID and the browse.json deletion are gone. Supersedes s10 M13. Tests: new open-gstack-browser-liveness.test.ts (no kill of the recorded PID, status probe before connect, --force-restart only after the yes branch, profile-lock cleanup kept, spawned/headless stop line). The state-extraction tripwire now covers only the port snippet, since the pid snippet is gone.
This commit is contained in:
1 parent
6fe767f0d7
commit
9cfc76d7b7
3 files changed
+133
-35
No files matched your search
@@ -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`
|
||||
|
||||
@@ -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'));
|
||||
});
|
||||
});
|
||||
@@ -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 });
|
||||
|
||||
Reference in new issue
Block a user