From a4cbcadeafddba479c80deb94e757bda57920f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?d=20=F0=9F=94=B9?= <258577966+voidborne-d@users.noreply.github.com> Date: Tue, 14 Apr 2026 07:53:35 +0000 Subject: [PATCH] fix: keep cookie picker alive after cli exits Fixes garrytan/gstack#985 --- browse/src/cookie-picker-routes.ts | 17 ++++++++ browse/src/server.ts | 25 +++++++---- browse/test/cookie-picker-routes.test.ts | 53 +++++++++++++++++++++++- 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index a78741cc..f954c088 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -40,6 +40,23 @@ export function generatePickerCode(): string { return code; } +/** Return true while the picker still has a live code or session. */ +export function hasActivePicker(): boolean { + const now = Date.now(); + + for (const [code, expiry] of pendingCodes) { + if (expiry > now) return true; + pendingCodes.delete(code); + } + + for (const [session, expiry] of validSessions) { + if (expiry > now) return true; + validSessions.delete(session); + } + + return false; +} + /** Extract session ID from the gstack_picker cookie. */ function getSessionFromCookie(req: Request): string | null { const cookie = req.headers.get('cookie'); diff --git a/browse/src/server.ts b/browse/src/server.ts index 25fe25b3..3f2feb89 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -764,14 +764,18 @@ if (BROWSE_PARENT_PID > 0) { try { process.kill(BROWSE_PARENT_PID, 0); // signal 0 = existence check only, no signal sent } catch { - // Parent exited. Behavior depends on mode: - // - Normal (headless) mode: stay alive. Claude Code's Bash tool kills the - // parent shell between invocations, so the server must survive. The - // idle timeout (30 min) handles eventual cleanup. - // - Headed / tunnel mode: the idle timeout DOESN'T apply (see idleCheckInterval - // above — both modes early-return). If we ignored parent death here too, - // orphan daemons would accumulate forever after /pair-agent or - // /open-gstack-browser sessions end. Shutdown instead. + // Parent exited. Resolution order: + // 1. Active cookie picker (one-time code or session live)? Stay alive + // regardless of mode — tearing down the server mid-import leaves the + // picker UI with a stale "Failed to fetch" error. + // 2. Headed / tunnel mode? Shutdown. The idle timeout doesn't apply in + // these modes (see idleCheckInterval above — both early-return), so + // ignoring parent death here would leak orphan daemons after + // /pair-agent or /open-gstack-browser sessions. + // 3. Normal (headless) mode? Stay alive. Claude Code's Bash tool kills + // the parent shell between invocations. The idle timeout (30 min) + // handles eventual cleanup. + if (hasActivePicker()) return; const headed = browserManager.getConnectionMode() === 'headed'; if (headed || tunnelActive) { console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited in ${headed ? 'headed' : 'tunnel'} mode, shutting down`); @@ -785,6 +789,7 @@ if (BROWSE_PARENT_PID > 0) { } // ─── Command Sets (from commands.ts — single source of truth) ─── +import { hasActivePicker } from './cookie-picker-routes'; import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS } from './commands'; export { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS }; @@ -1250,6 +1255,10 @@ process.on('SIGINT', shutdown); // SIGTERM so external tooling (systemd, supervisord, CI) can shut cleanly // without waiting forever. Ctrl+C and /stop still work either way. process.on('SIGTERM', () => { + if (hasActivePicker()) { + console.log('[browse] Received SIGTERM but cookie picker is active, ignoring to avoid stranding the picker UI'); + return; + } const headed = browserManager.getConnectionMode() === 'headed'; if (headed || tunnelActive) { console.log(`[browse] Received SIGTERM in ${headed ? 'headed' : 'tunnel'} mode, shutting down`); diff --git a/browse/test/cookie-picker-routes.test.ts b/browse/test/cookie-picker-routes.test.ts index 50615608..c1934cd8 100644 --- a/browse/test/cookie-picker-routes.test.ts +++ b/browse/test/cookie-picker-routes.test.ts @@ -7,7 +7,7 @@ */ import { describe, test, expect } from 'bun:test'; -import { handleCookiePickerRoute, generatePickerCode } from '../src/cookie-picker-routes'; +import { handleCookiePickerRoute, generatePickerCode, hasActivePicker } from '../src/cookie-picker-routes'; // ─── Mock BrowserManager ────────────────────────────────────── @@ -284,6 +284,57 @@ describe('cookie-picker-routes', () => { }); }); + describe('active picker tracking', () => { + test('one-time codes keep the picker active until consumed', async () => { + const realNow = Date.now; + Date.now = () => realNow() + 3_700_000; + try { + expect(hasActivePicker()).toBe(false); // clears any stale state from prior tests + } finally { + Date.now = realNow; + } + + const { bm } = mockBrowserManager(); + const code = generatePickerCode(); + expect(hasActivePicker()).toBe(true); + + const res = await handleCookiePickerRoute( + makeUrl(`/cookie-picker?code=${code}`), + new Request('http://127.0.0.1:9470', { method: 'GET' }), + bm, + 'test-token', + ); + + expect(res.status).toBe(302); + expect(hasActivePicker()).toBe(true); // session is now active + }); + + test('picker becomes inactive after an invalid session probe clears expired state', async () => { + const { bm } = mockBrowserManager(); + const session = await getSessionCookie(bm, 'test-token'); + expect(hasActivePicker()).toBe(true); + + const realNow = Date.now; + Date.now = () => realNow() + 3_700_000; + try { + const res = await handleCookiePickerRoute( + makeUrl('/cookie-picker'), + new Request('http://127.0.0.1:9470', { + method: 'GET', + headers: { 'Cookie': `gstack_picker=${session}` }, + }), + bm, + 'test-token', + ); + + expect(res.status).toBe(403); + expect(hasActivePicker()).toBe(false); + } finally { + Date.now = realNow; + } + }); + }); + describe('session cookie auth', () => { test('valid session cookie grants HTML access', async () => { const { bm } = mockBrowserManager();