mirror of https://github.com/garrytan/gstack.git
fix(browse): checkpoint on real cookie mutators and after the command runs
The debounced auto-cookie checkpoint had two gaps. First, the trigger set was a hand-maintained list that missed the actual cookie-mutating commands (cookie, cookie-import, load-html) while listing a non-existent set-cookie. Drive the decision off WRITE_COMMANDS (the real mutator registry) plus a small extra set for js/eval and the meta commands (chain, state, newtab, tab-each) that can change the cookie jar. Second, the schedule call ran before dispatch, so a slow navigation or login redirect snapshotted the pre-command jar; move it to after the command attempt (success and error paths, plus the scoped-newtab early return) so the debounce captures the post-mutation state. Also wire cookie-picker import/remove: those flow through the /cookie-picker route, not /command, so they never scheduled a save. Pass scheduleAutoCookieCheckpoint as an onCookieMutation callback and fire it after addCookies / removal. Serialize checkpoints with an in-flight promise guard so an overlapping periodic + debounced + shutdown checkpoint can't race two concurrent saveAutoCookieState writes onto the same slot. The shutdown flush awaits any in-flight checkpoint first, then runs its own, so no save is lost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
53c2750cf1
commit
13ff147233
|
|
@ -110,6 +110,7 @@ export async function handleCookiePickerRoute(
|
||||||
req: Request,
|
req: Request,
|
||||||
bm: BrowserManager,
|
bm: BrowserManager,
|
||||||
authToken?: string,
|
authToken?: string,
|
||||||
|
onCookieMutation?: () => void,
|
||||||
): Promise<Response> {
|
): Promise<Response> {
|
||||||
const pathname = url.pathname;
|
const pathname = url.pathname;
|
||||||
const port = parseInt(url.port, 10) || 9400;
|
const port = parseInt(url.port, 10) || 9400;
|
||||||
|
|
@ -268,6 +269,7 @@ export async function handleCookiePickerRoute(
|
||||||
// Add to Playwright context
|
// Add to Playwright context
|
||||||
const page = bm.getActiveSession().getPage();
|
const page = bm.getActiveSession().getPage();
|
||||||
await page.context().addCookies(result.cookies);
|
await page.context().addCookies(result.cookies);
|
||||||
|
onCookieMutation?.();
|
||||||
|
|
||||||
// Track what was imported
|
// Track what was imported
|
||||||
for (const domain of Object.keys(result.domainCounts)) {
|
for (const domain of Object.keys(result.domainCounts)) {
|
||||||
|
|
@ -305,6 +307,7 @@ export async function handleCookiePickerRoute(
|
||||||
importedDomains.delete(domain);
|
importedDomains.delete(domain);
|
||||||
importedCounts.delete(domain);
|
importedCounts.delete(domain);
|
||||||
}
|
}
|
||||||
|
onCookieMutation?.();
|
||||||
|
|
||||||
console.log(`[cookie-picker] Removed cookies for ${domains.length} domains`);
|
console.log(`[cookie-picker] Removed cookies for ${domains.length} domains`);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -666,11 +666,24 @@ const idleCheckInterval = setInterval(idleCheckTick, 60_000);
|
||||||
let autoCookieLockHeld = false;
|
let autoCookieLockHeld = false;
|
||||||
let autoCookieLastHash: string | null = null;
|
let autoCookieLastHash: string | null = null;
|
||||||
let autoCookieDebounce: ReturnType<typeof setTimeout> | null = null;
|
let autoCookieDebounce: ReturnType<typeof setTimeout> | null = null;
|
||||||
|
let autoCookieCheckpointInFlight: Promise<void> | null = null;
|
||||||
|
|
||||||
async function autoCookieCheckpoint(): Promise<void> {
|
async function autoCookieCheckpoint(): Promise<void> {
|
||||||
|
while (autoCookieCheckpointInFlight) {
|
||||||
|
await autoCookieCheckpointInFlight;
|
||||||
|
}
|
||||||
if (!autoCookieLockHeld) return; // never write without owning the lock
|
if (!autoCookieLockHeld) return; // never write without owning the lock
|
||||||
const res = await saveAutoCookieState(config, activeBrowserManager.getContext(), autoCookieLastHash);
|
const run = (async () => {
|
||||||
autoCookieLastHash = res.hash;
|
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
|
// 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.
|
// Commands that can mutate cookies/browser state and thus warrant a checkpoint.
|
||||||
const AUTO_COOKIE_MUTATING_COMMANDS = new Set([
|
const AUTO_COOKIE_MUTATING_COMMANDS = new Set([
|
||||||
'goto', 'click', 'fill', 'type', 'press', 'select', 'reload', 'back', 'forward',
|
'chain', 'state', 'newtab', 'tab-each',
|
||||||
'chain', 'state', 'cookie-picker', 'set-cookie',
|
'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
|
// Test-only surface for server-factory.test.ts. Lets the dual-instance
|
||||||
// idle-timer behavior be exercised deterministically without mutating
|
// idle-timer behavior be exercised deterministically without mutating
|
||||||
// Date.now (which would interact with the leaked module-level setInterval).
|
// 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.
|
// so the trail records what the agent actually typed.
|
||||||
const command = canonicalizeCommand(rawCommand);
|
const command = canonicalizeCommand(rawCommand);
|
||||||
const isAliased = command !== rawCommand;
|
const isAliased = command !== rawCommand;
|
||||||
|
const shouldCheckpointAutoCookies = (opts?.chainDepth ?? 0) === 0 && shouldAutoCookieCheckpoint(command);
|
||||||
// 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();
|
|
||||||
}
|
|
||||||
|
|
||||||
// ─── Recursion guard: reject nested chains ──────────────────
|
// ─── Recursion guard: reject nested chains ──────────────────
|
||||||
if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) {
|
if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) {
|
||||||
|
|
@ -1113,6 +1123,7 @@ async function handleCommandInternalImpl(
|
||||||
// ─── newtab with ownership for scoped tokens ──────────────
|
// ─── newtab with ownership for scoped tokens ──────────────
|
||||||
if (command === 'newtab' && tokenInfo && tokenInfo.clientId !== 'root') {
|
if (command === 'newtab' && tokenInfo && tokenInfo.clientId !== 'root') {
|
||||||
const newId = await browserManager.newTab(args[0] || undefined, tokenInfo.clientId);
|
const newId = await browserManager.newTab(args[0] || undefined, tokenInfo.clientId);
|
||||||
|
if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint();
|
||||||
return {
|
return {
|
||||||
status: 200, json: true,
|
status: 200, json: true,
|
||||||
result: JSON.stringify({
|
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) ───
|
// ─── Centralized content wrapping (single location for all commands) ───
|
||||||
// Scoped tokens: content filter + enhanced envelope + datamarking
|
// Scoped tokens: content filter + enhanced envelope + datamarking
|
||||||
// Root tokens: basic untrusted content wrapper (backward compat)
|
// Root tokens: basic untrusted content wrapper (backward compat)
|
||||||
|
|
@ -1311,6 +1327,8 @@ async function handleCommandInternalImpl(
|
||||||
}
|
}
|
||||||
return { status: 200, result };
|
return { status: 200, result };
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
|
if (shouldCheckpointAutoCookies) scheduleAutoCookieCheckpoint();
|
||||||
|
|
||||||
// Restore original active tab even on error
|
// Restore original active tab even on error
|
||||||
if (savedTabId !== null) {
|
if (savedTabId !== null) {
|
||||||
try { browserManager.switchTab(savedTabId, { bringToFront: false }); } catch (restoreErr: any) {
|
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
|
// Cookie picker routes — HTML page unauthenticated, data/action routes require auth
|
||||||
if (url.pathname.startsWith('/cookie-picker')) {
|
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
|
// Welcome page — served when GStack Browser launches in headed mode
|
||||||
|
|
|
||||||
|
|
@ -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 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 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 {
|
function tmpConfig(): BrowseConfig {
|
||||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-cookie-'));
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-cookie-'));
|
||||||
|
|
@ -320,4 +321,22 @@ describe('wiring (static)', () => {
|
||||||
expect(loadIdx).toBeGreaterThan(0);
|
expect(loadIdx).toBeGreaterThan(0);
|
||||||
expect(newCtxIdx).toBeGreaterThan(loadIdx); // restore assigned into contextOptions before use
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue