mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-15 09:25:28 +02:00
fix(pair-agent): consent before killing a healthy headless daemon
The pair-agent headed switch spawned 'connect --force-restart'
unconditionally — auto-killing a live headless daemon (open tabs, cookies,
logins) in direct contradiction of the iron rule it sits beside ('only an
explicit --force-restart may kill a live daemon'). The CLI now captures
daemon liveness BEFORE ensureServer (which can itself boot a fresh daemon)
and relaunches only when the user passed --force-restart to pair-agent;
otherwise it prints the tab count and continues against the existing daemon.
The /pair-agent skill gains a matching one-way-door consent question
(template half rides the wave's template block).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
565ca9b13b
commit
6955dfa348
+64
-21
@@ -159,6 +159,22 @@ export async function isServerHealthy(port: number, timeoutMs = 2000): Promise<b
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Best-effort tab count via GET /health (no auth, bounded). Returns null
|
||||||
|
* when the daemon doesn't answer in time or predates the `tabs` field —
|
||||||
|
* callers degrade to a countless phrasing, never block on this. */
|
||||||
|
async function fetchDaemonTabCount(port: number, timeoutMs = 2000): Promise<number | null> {
|
||||||
|
try {
|
||||||
|
const resp = await fetch(`http://127.0.0.1:${port}/health`, {
|
||||||
|
signal: AbortSignal.timeout(timeoutMs),
|
||||||
|
});
|
||||||
|
if (!resp.ok) return null;
|
||||||
|
const health = await resp.json() as any;
|
||||||
|
return typeof health.tabs === 'number' ? health.tabs : null;
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ─── Process Management ─────────────────────────────────────────
|
// ─── Process Management ─────────────────────────────────────────
|
||||||
async function killServer(pid: number): Promise<void> {
|
async function killServer(pid: number): Promise<void> {
|
||||||
if (!isProcessAlive(pid)) return;
|
if (!isProcessAlive(pid)) return;
|
||||||
@@ -1631,6 +1647,19 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
|
|||||||
commandArgs.push(stdin.trim());
|
commandArgs.push(stdin.trim());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #2219 IRON RULE (pair-agent leg): capture whether a LIVE daemon predates
|
||||||
|
// this invocation BEFORE ensureServer() can start a fresh one. pair-agent's
|
||||||
|
// headed switch below replaces the daemon via `connect --force-restart` —
|
||||||
|
// a kill that loses tabs/cookies/logins — so a PRE-EXISTING live daemon may
|
||||||
|
// only be replaced with the user's explicit --force-restart consent. A
|
||||||
|
// daemon that ensureServer just booted for this invocation holds no session
|
||||||
|
// state, so replacing it kills nothing the user had.
|
||||||
|
let pairAgentPreexistingDaemonAlive = false;
|
||||||
|
if (command === 'pair-agent') {
|
||||||
|
const preState = readState();
|
||||||
|
pairAgentPreexistingDaemonAlive = Boolean(preState?.pid && isProcessAlive(preState.pid));
|
||||||
|
}
|
||||||
|
|
||||||
let state = await ensureServer(globalFlags);
|
let state = await ensureServer(globalFlags);
|
||||||
|
|
||||||
// ─── Pair-Agent (post-server, pre-dispatch) ──────────────
|
// ─── Pair-Agent (post-server, pre-dispatch) ──────────────
|
||||||
@@ -1638,28 +1667,42 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
|
|||||||
// Ensure headed mode — the user should see the browser window
|
// Ensure headed mode — the user should see the browser window
|
||||||
// when sharing it with another agent. Feels safer, more impressive.
|
// when sharing it with another agent. Feels safer, more impressive.
|
||||||
if (state.mode !== 'headed' && !hasFlag(commandArgs, '--headless')) {
|
if (state.mode !== 'headed' && !hasFlag(commandArgs, '--headless')) {
|
||||||
console.log('[browse] Opening GStack Browser so you can see what the remote agent does...');
|
if (pairAgentPreexistingDaemonAlive && !globalFlags.forceRestart) {
|
||||||
// In compiled binaries, process.argv[1] is /$bunfs/... (virtual).
|
// #2219 IRON RULE: only an explicit --force-restart may kill a live
|
||||||
// Use process.execPath which is the real binary on disk.
|
// daemon. The headed switch is nice-to-have; the user's open tabs,
|
||||||
const browseBin = process.execPath;
|
// cookies, and logins are not. Continue against the live headless
|
||||||
// --force-restart: the headed switch is this command's explicit purpose
|
// daemon and tell the user how to opt into the headed relaunch.
|
||||||
// (the user asked to SEE the shared browser), and connect's #2219 guard
|
const tabCount = await fetchDaemonTabCount(state.port);
|
||||||
// would otherwise refuse to replace the healthy headless daemon.
|
const tabsPhrase = tabCount === null
|
||||||
const connectProc = Bun.spawn([browseBin, 'connect', '--force-restart'], {
|
? 'open tabs'
|
||||||
windowsHide: true,
|
: `${tabCount} tab${tabCount === 1 ? '' : 's'}`;
|
||||||
cwd: process.cwd(),
|
console.warn(`[browse] Live headless daemon has ${tabsPhrase}; continuing against it — pass --force-restart to relaunch headed, losing tabs/cookies.`);
|
||||||
stdio: ['ignore', 'inherit', 'inherit'],
|
|
||||||
// Disable parent-PID monitoring: pair-agent needs the server to outlive
|
|
||||||
// the connect subprocess. Setting to 0 tells the server not to self-terminate.
|
|
||||||
env: { ...process.env, BROWSE_PARENT_PID: '0' },
|
|
||||||
});
|
|
||||||
await connectProc.exited;
|
|
||||||
// Re-read state after headed mode switch
|
|
||||||
const newState = readState();
|
|
||||||
if (newState && await isServerHealthy(newState.port)) {
|
|
||||||
state = newState as ServerState;
|
|
||||||
} else {
|
} else {
|
||||||
console.warn('[browse] Could not switch to headed mode. Continuing headless.');
|
console.log('[browse] Opening GStack Browser so you can see what the remote agent does...');
|
||||||
|
// In compiled binaries, process.argv[1] is /$bunfs/... (virtual).
|
||||||
|
// Use process.execPath which is the real binary on disk.
|
||||||
|
const browseBin = process.execPath;
|
||||||
|
// --force-restart: reaching this branch means either no live daemon
|
||||||
|
// predated this invocation (nothing of the user's dies) or the user
|
||||||
|
// explicitly passed --force-restart to pair-agent (consent given).
|
||||||
|
// connect's #2219 guard would otherwise refuse to replace the
|
||||||
|
// healthy headless daemon ensureServer just returned.
|
||||||
|
const connectProc = Bun.spawn([browseBin, 'connect', '--force-restart'], {
|
||||||
|
windowsHide: true,
|
||||||
|
cwd: process.cwd(),
|
||||||
|
stdio: ['ignore', 'inherit', 'inherit'],
|
||||||
|
// Disable parent-PID monitoring: pair-agent needs the server to outlive
|
||||||
|
// the connect subprocess. Setting to 0 tells the server not to self-terminate.
|
||||||
|
env: { ...process.env, BROWSE_PARENT_PID: '0' },
|
||||||
|
});
|
||||||
|
await connectProc.exited;
|
||||||
|
// Re-read state after headed mode switch
|
||||||
|
const newState = readState();
|
||||||
|
if (newState && await isServerHealthy(newState.port)) {
|
||||||
|
state = newState as ServerState;
|
||||||
|
} else {
|
||||||
|
console.warn('[browse] Could not switch to headed mode. Continuing headless.');
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
await handlePairAgent(state, commandArgs);
|
await handlePairAgent(state, commandArgs);
|
||||||
|
|||||||
@@ -146,6 +146,44 @@ describe('#2219 iron rule (CLI integration)', () => {
|
|||||||
}
|
}
|
||||||
}, 30_000);
|
}, 30_000);
|
||||||
|
|
||||||
|
test('pair-agent over a live headless daemon WITHOUT --force-restart → daemon survives, notice printed, no headed relaunch', async () => {
|
||||||
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-iron-'));
|
||||||
|
const stateFile = path.join(tmpDir, 'browse.json');
|
||||||
|
const daemon = await startHealthyDaemon();
|
||||||
|
try {
|
||||||
|
pidChild = spawn('sleep', ['60'], { stdio: 'ignore' });
|
||||||
|
const daemonPid = pidChild.pid!;
|
||||||
|
const stateContent = {
|
||||||
|
pid: daemonPid,
|
||||||
|
port: daemon.port,
|
||||||
|
token: 'iron-rule-token',
|
||||||
|
startedAt: new Date().toISOString(),
|
||||||
|
serverPath: '',
|
||||||
|
mode: 'launched' as const,
|
||||||
|
};
|
||||||
|
fs.writeFileSync(stateFile, JSON.stringify(stateContent, null, 2));
|
||||||
|
|
||||||
|
// Exit code is NOT asserted: the fake daemon answers /pair with
|
||||||
|
// non-JSON so handlePairAgent fails later — the iron rule under test
|
||||||
|
// is everything that happens BEFORE that: no kill, no headed relaunch.
|
||||||
|
const result = await runCli(['pair-agent'], baseEnv(stateFile));
|
||||||
|
|
||||||
|
// The consent notice: live session named, opt-in flag named.
|
||||||
|
expect(result.stderr).toContain('continuing against it');
|
||||||
|
expect(result.stderr).toContain('--force-restart');
|
||||||
|
// No headed relaunch was attempted.
|
||||||
|
const combined = result.stdout + result.stderr;
|
||||||
|
expect(combined).not.toContain('Opening GStack Browser');
|
||||||
|
// THE IRON RULE: the daemon process was not killed.
|
||||||
|
expect(isProcessAlive(daemonPid)).toBe(true);
|
||||||
|
// And the state file was not clobbered.
|
||||||
|
expect(JSON.parse(fs.readFileSync(stateFile, 'utf-8'))).toEqual(stateContent);
|
||||||
|
} finally {
|
||||||
|
await daemon.close();
|
||||||
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
}, 30_000);
|
||||||
|
|
||||||
test('wedged-alive daemon + plain command → busy report + nonzero exit, NO kill', async () => {
|
test('wedged-alive daemon + plain command → busy report + nonzero exit, NO kill', async () => {
|
||||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-iron-'));
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-iron-'));
|
||||||
const stateFile = path.join(tmpDir, 'browse.json');
|
const stateFile = path.join(tmpDir, 'browse.json');
|
||||||
|
|||||||
@@ -122,3 +122,40 @@ describe('gate wiring — every tunnel activation point consults the guard', ()
|
|||||||
expect(SERVER_SRC).toContain("process.env.BROWSE_TUNNEL === '1' && !isPairAgentEnabled()");
|
expect(SERVER_SRC).toContain("process.env.BROWSE_TUNNEL === '1' && !isPairAgentEnabled()");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('pair-agent headed switch — #2219 iron-rule consent gate', () => {
|
||||||
|
// The behavioral leg (live daemon + no flag → notice, no kill) lives in
|
||||||
|
// busy-daemon-iron-rule.test.ts. These source pins cover the wiring the
|
||||||
|
// integration test can't exercise cheaply: the explicit-flag path and the
|
||||||
|
// capture-before-ensureServer ordering.
|
||||||
|
|
||||||
|
test('headed relaunch of a pre-existing live daemon is gated on explicit --force-restart', () => {
|
||||||
|
const gateAt = CLI_SRC.indexOf('if (pairAgentPreexistingDaemonAlive && !globalFlags.forceRestart) {');
|
||||||
|
expect(gateAt).toBeGreaterThan(-1);
|
||||||
|
// Refusal branch: notice printed, connect never spawned.
|
||||||
|
const elseAt = CLI_SRC.indexOf('} else {', gateAt);
|
||||||
|
expect(elseAt).toBeGreaterThan(gateAt);
|
||||||
|
const refusalBranch = CLI_SRC.slice(gateAt, elseAt);
|
||||||
|
expect(refusalBranch).toContain('continuing against it');
|
||||||
|
expect(refusalBranch).toContain('--force-restart to relaunch headed');
|
||||||
|
expect(refusalBranch).not.toContain('Bun.spawn');
|
||||||
|
// Consented branch (no pre-existing daemon OR explicit flag): the spawn
|
||||||
|
// of `connect --force-restart` lives here and ONLY here.
|
||||||
|
const branchEnd = CLI_SRC.indexOf('await handlePairAgent(state, commandArgs);', gateAt);
|
||||||
|
expect(branchEnd).toBeGreaterThan(elseAt);
|
||||||
|
const consentedBranch = CLI_SRC.slice(elseAt, branchEnd);
|
||||||
|
expect(consentedBranch).toContain("Bun.spawn([browseBin, 'connect', '--force-restart']");
|
||||||
|
// No second spawn site outside the gated block.
|
||||||
|
expect(CLI_SRC.indexOf("'connect', '--force-restart'")).toBe(CLI_SRC.lastIndexOf("'connect', '--force-restart'"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('pre-existing liveness is captured BEFORE ensureServer can boot a fresh daemon', () => {
|
||||||
|
// If the capture ran after ensureServer, a freshly-booted daemon would be
|
||||||
|
// indistinguishable from a session the user cares about — the gate would
|
||||||
|
// then refuse the headed switch even on a clean machine.
|
||||||
|
const captureAt = CLI_SRC.indexOf('pairAgentPreexistingDaemonAlive = Boolean(preState?.pid && isProcessAlive(preState.pid));');
|
||||||
|
const ensureAt = CLI_SRC.indexOf('let state = await ensureServer(globalFlags);');
|
||||||
|
expect(captureAt).toBeGreaterThan(-1);
|
||||||
|
expect(ensureAt).toBeGreaterThan(captureAt);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user