diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index 07ab5a2c2..e6b60befa 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -110,6 +110,7 @@ export async function handleCookiePickerRoute( req: Request, bm: BrowserManager, authToken?: string, + onCookieMutation?: () => void, ): Promise { const pathname = url.pathname; const port = parseInt(url.port, 10) || 9400; @@ -268,6 +269,7 @@ export async function handleCookiePickerRoute( // Add to Playwright context const page = bm.getActiveSession().getPage(); await page.context().addCookies(result.cookies); + onCookieMutation?.(); // Track what was imported for (const domain of Object.keys(result.domainCounts)) { @@ -305,6 +307,7 @@ export async function handleCookiePickerRoute( importedDomains.delete(domain); importedCounts.delete(domain); } + onCookieMutation?.(); console.log(`[cookie-picker] Removed cookies for ${domains.length} domains`); diff --git a/browse/src/server.ts b/browse/src/server.ts index a1c6ea765..7ecd66769 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -666,11 +666,24 @@ const idleCheckInterval = setInterval(idleCheckTick, 60_000); let autoCookieLockHeld = false; let autoCookieLastHash: string | null = null; let autoCookieDebounce: ReturnType | null = null; +let autoCookieCheckpointInFlight: Promise | null = null; async function autoCookieCheckpoint(): Promise { + while (autoCookieCheckpointInFlight) { + await autoCookieCheckpointInFlight; + } if (!autoCookieLockHeld) return; // never write without owning the lock - const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash); - autoCookieLastHash = res.hash; + const run = (async () => { + if (!autoCookieLockHeld) return; + const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash); + if (autoCookieLockHeld) autoCookieLastHash = res.hash; + })(); + autoCookieCheckpointInFlight = run; + try { + await run; + } finally { + if (autoCookieCheckpointInFlight === run) autoCookieCheckpointInFlight = null; + } } // Debounced checkpoint: coalesces bursts of mutating commands into one write a @@ -692,10 +705,14 @@ if (typeof autoCookieInterval.unref === 'function') autoCookieInterval.unref(); // Commands that can mutate cookies/browser state and thus warrant a checkpoint. const AUTO_COOKIE_MUTATING_COMMANDS = new Set([ - 'goto', 'click', 'fill', 'type', 'press', 'select', 'reload', 'back', 'forward', - 'chain', 'state', 'cookie-picker', 'set-cookie', + 'chain', 'state', 'newtab', 'tab-each', + 'js', 'eval', ]); +function shouldAutoCookieCheckpoint(command: string): boolean { + return WRITE_COMMANDS.has(command) || AUTO_COOKIE_MUTATING_COMMANDS.has(command); +} + // Test-only surface for server-factory.test.ts. Lets the dual-instance // idle-timer behavior be exercised deterministically without mutating // Date.now (which would interact with the leaked module-level setInterval). @@ -1006,14 +1023,7 @@ async function handleCommandInternalImpl( // so the trail records what the agent actually typed. const command = canonicalizeCommand(rawCommand); const isAliased = command !== rawCommand; - - // Auto-cookie checkpoint (opt-in): a mutating command may have changed - // cookies. Schedule a debounced checkpoint — the 1.5s debounce means the - // capture runs after the command has actually applied. No-op unless the lock - // is held. Skip nested chain subcommands: the outer chain already scheduled. - if ((opts?.chainDepth ?? 0) === 0 && AUTO_COOKIE_MUTATING_COMMANDS.has(command)) { - scheduleAutoCookieCheckpoint(); - } + const shouldCheckpointAutoCookies = (opts?.chainDepth ?? 0) === 0 && shouldAutoCookieCheckpoint(command); // ─── Recursion guard: reject nested chains ────────────────── if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) { @@ -1113,6 +1123,7 @@ async function handleCommandInternalImpl( // ─── newtab with ownership for scoped tokens ────────────── if (command === 'newtab' && tokenInfo && tokenInfo.clientId !== 'root') { const newId = await browserManager.newTab(args[0] || undefined, tokenInfo.clientId); + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); return { status: 200, json: true, result: JSON.stringify({ @@ -1240,6 +1251,11 @@ async function handleCommandInternalImpl( }; } + // Auto-cookie checkpoint (opt-in): schedule after the command attempt has + // actually run, so slow navigations/login redirects do not snapshot the + // pre-command cookie jar. No-op unless the lock is held. + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); + // ─── Centralized content wrapping (single location for all commands) ─── // Scoped tokens: content filter + enhanced envelope + datamarking // Root tokens: basic untrusted content wrapper (backward compat) @@ -1311,6 +1327,8 @@ async function handleCommandInternalImpl( } return { status: 200, result }; } catch (err: any) { + if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint(); + // Restore original active tab even on error if (savedTabId !== null) { try { browserManager.switchTab(savedTabId, { bringToFront: false }); } catch (restoreErr: any) { @@ -1803,7 +1821,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // Cookie picker routes — HTML page unauthenticated, data/action routes require auth if (url.pathname.startsWith('/cookie-picker')) { - return handleCookiePickerRoute(url, req, browserManager, authToken); + return handleCookiePickerRoute(url, req, browserManager, authToken, scheduleAutoCookieCheckpoint); } // Welcome page — served when GStack Browser launches in headed mode diff --git a/browse/test/auto-cookie-persist.test.ts b/browse/test/auto-cookie-persist.test.ts index 39e890e86..538ca6bad 100644 --- a/browse/test/auto-cookie-persist.test.ts +++ b/browse/test/auto-cookie-persist.test.ts @@ -34,6 +34,7 @@ import { writeSecureFileAtomic } from '../src/file-permissions'; const META = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8'); const BM = fs.readFileSync(path.join(import.meta.dir, '../src/browser-manager.ts'), 'utf-8'); +const PICKER = fs.readFileSync(path.join(import.meta.dir, '../src/cookie-picker-routes.ts'), 'utf-8'); function tmpConfig(): BrowseConfig { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-cookie-')); @@ -320,4 +321,22 @@ describe('wiring (static)', () => { expect(loadIdx).toBeGreaterThan(0); expect(newCtxIdx).toBeGreaterThan(loadIdx); // restore assigned into contextOptions before use }); + + test('debounced command checkpoint runs after dispatch and covers real mutators', () => { + expect(META).toContain('WRITE_COMMANDS.has(command)'); + expect(META).toContain("'js', 'eval'"); + expect(META).toContain("'chain', 'state', 'newtab', 'tab-each'"); + expect(META).not.toContain("'set-cookie'"); + const canonicalIdx = META.indexOf('const command = canonicalizeCommand(rawCommand);'); + const dispatchIdx = META.indexOf('if (READ_COMMANDS.has(command))', canonicalIdx); + const scheduleIdx = META.indexOf('// Auto-cookie checkpoint (opt-in): schedule after', dispatchIdx); + expect(dispatchIdx).toBeGreaterThan(canonicalIdx); + expect(scheduleIdx).toBeGreaterThan(dispatchIdx); + }); + + test('cookie picker imports/removals schedule auto-cookie persistence', () => { + expect(META).toContain('handleCookiePickerRoute(url, req, browserManager, authToken, scheduleAutoCookieCheckpoint)'); + expect(PICKER).toContain('onCookieMutation?: () => void'); + expect(PICKER.match(/onCookieMutation\?\.\(\)/g)?.length).toBe(2); + }); });