mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-17 17:51:27 +08:00
refactor: extract TabSession for per-tab state isolation (v0.15.16.0) (#873)
* plan: batch command endpoint + multi-tab parallel execution for GStack Browser * refactor: extract TabSession from BrowserManager for per-tab state Move per-tab state (refMap, lastSnapshot, frame) into a new TabSession class. BrowserManager delegates to the active TabSession via getActiveSession(). Zero behavior change — all existing tests pass. This is the foundation for the /batch endpoint: both /command and /batch will use the same handler functions with TabSession, eliminating shared state races during parallel tab execution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: update handler signatures to use TabSession Change handleReadCommand and handleSnapshot to take TabSession instead of BrowserManager. Change handleWriteCommand to take both TabSession (per-tab ops) and BrowserManager (global ops like viewport, headers, dialog). handleMetaCommand keeps BrowserManager for tab management. Tests use thin wrapper functions that bridge the old 3-arg call pattern to the new signatures via bm.getActiveSession(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add POST /batch endpoint for parallel multi-tab execution Execute multiple commands across tabs in a single HTTP request. Commands targeting different tabs run concurrently via Promise.allSettled. Commands targeting the same tab run sequentially within that group. Features: - Batch-safe command subset (text, goto, click, snapshot, screenshot, etc.) - newtab/closetab as special commands within batch - SSE streaming mode (stream: true) for partial results - Per-command error isolation (one tab failing doesn't abort the batch) - Max 50 commands per batch, soft batch-level timeout A 143-page crawl drops from ~45 min (serial HTTP) to ~5 min (20 tabs in parallel, batched commands). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add batch endpoint integration tests 10 tests covering: - Multi-tab parallel execution (goto + text on different tabs) - Same-tab sequential ordering - Per-command error isolation (one tab fails, others succeed) - Page-scoped refs (snapshot refs are per-session, not global) - Per-tab lastSnapshot (snapshot -D with independent baselines) - getSession/getActiveSession API - Batch-safe command subset validation - closeTab via page.close preserves at-least-one-page invariant - Parallel goto on 3 tabs simultaneously Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: harden codex-review E2E — extract SKILL.md section, bump maxTurns to 25 The test was copying the full 55KB/1075-line codex SKILL.md into the fixture, requiring 8 Read calls just to consume it and exhausting the 15-turn budget before reaching the actual codex review command. Now extracts only the review-relevant section (~6KB/148 lines), reducing Read calls from 8 to 1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: move batch endpoint plan into BROWSER.md as feature documentation The batch endpoint is implemented — document it as an actual feature in BROWSER.md (architecture, API shape, design decisions, usage pattern) and remove the standalone plan file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.15.16.0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: gstack <ship@gstack.dev> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -18,12 +18,12 @@
|
||||
import { chromium, type Browser, type BrowserContext, type BrowserContextOptions, type Page, type Locator, type Cookie } from 'playwright';
|
||||
import { addConsoleEntry, addNetworkEntry, addDialogEntry, networkBuffer, type DialogEntry } from './buffers';
|
||||
import { validateNavigationUrl } from './url-validation';
|
||||
import { TabSession, type RefEntry } from './tab-session';
|
||||
|
||||
export interface RefEntry {
|
||||
locator: Locator;
|
||||
role: string;
|
||||
name: string;
|
||||
}
|
||||
export type { RefEntry };
|
||||
|
||||
// Re-export TabSession for consumers
|
||||
export { TabSession };
|
||||
|
||||
export interface BrowserState {
|
||||
cookies: Cookie[];
|
||||
@@ -38,6 +38,7 @@ export class BrowserManager {
|
||||
private browser: Browser | null = null;
|
||||
private context: BrowserContext | null = null;
|
||||
private pages: Map<number, Page> = new Map();
|
||||
private tabSessions: Map<number, TabSession> = new Map();
|
||||
private activeTabId: number = 0;
|
||||
private nextTabId: number = 1;
|
||||
private extraHeaders: Record<string, string> = {};
|
||||
@@ -50,14 +51,7 @@ export class BrowserManager {
|
||||
// Maps tabId → clientId. Unowned tabs (not in this map) are root-only for writes.
|
||||
private tabOwnership: Map<number, string> = new Map();
|
||||
|
||||
// ─── Ref Map (snapshot → @e1, @e2, @c1, @c2, ...) ────────
|
||||
private refMap: Map<string, RefEntry> = new Map();
|
||||
|
||||
// ─── Snapshot Diffing ─────────────────────────────────────
|
||||
// NOT cleared on navigation — it's a text baseline for diffing
|
||||
private lastSnapshot: string | null = null;
|
||||
|
||||
// ─── Dialog Handling ──────────────────────────────────────
|
||||
// ─── Dialog Handling (global, not per-tab) ──────────────────
|
||||
private dialogAutoAccept: boolean = true;
|
||||
private dialogPromptText: string | null = null;
|
||||
|
||||
@@ -142,11 +136,11 @@ export class BrowserManager {
|
||||
* Get the ref map for external consumers (e.g., /refs endpoint).
|
||||
*/
|
||||
getRefMap(): Array<{ ref: string; role: string; name: string }> {
|
||||
const refs: Array<{ ref: string; role: string; name: string }> = [];
|
||||
for (const [ref, entry] of this.refMap) {
|
||||
refs.push({ ref, role: entry.role, name: entry.name });
|
||||
try {
|
||||
return this.getActiveSession().getRefEntries();
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
return refs;
|
||||
}
|
||||
|
||||
async launch() {
|
||||
@@ -220,7 +214,7 @@ export class BrowserManager {
|
||||
async launchHeaded(authToken?: string): Promise<void> {
|
||||
// Clear old state before repopulating
|
||||
this.pages.clear();
|
||||
this.refMap.clear();
|
||||
this.tabSessions.clear();
|
||||
this.nextTabId = 1;
|
||||
|
||||
// Find the gstack extension directory for auto-loading
|
||||
@@ -434,6 +428,7 @@ export class BrowserManager {
|
||||
this.context.on('page', (page) => {
|
||||
const id = this.nextTabId++;
|
||||
this.pages.set(id, page);
|
||||
this.tabSessions.set(id, new TabSession(page));
|
||||
this.activeTabId = id;
|
||||
this.wirePageEvents(page);
|
||||
// Inject indicator on the new tab
|
||||
@@ -447,6 +442,7 @@ export class BrowserManager {
|
||||
const page = existingPages[0];
|
||||
const id = this.nextTabId++;
|
||||
this.pages.set(id, page);
|
||||
this.tabSessions.set(id, new TabSession(page));
|
||||
this.activeTabId = id;
|
||||
this.wirePageEvents(page);
|
||||
// Inject indicator on restored page (addInitScript only fires on new navigations)
|
||||
@@ -521,6 +517,7 @@ export class BrowserManager {
|
||||
const page = await this.context.newPage();
|
||||
const id = this.nextTabId++;
|
||||
this.pages.set(id, page);
|
||||
this.tabSessions.set(id, new TabSession(page));
|
||||
this.activeTabId = id;
|
||||
|
||||
// Record tab ownership for multi-agent isolation
|
||||
@@ -545,6 +542,7 @@ export class BrowserManager {
|
||||
|
||||
await page.close();
|
||||
this.pages.delete(tabId);
|
||||
this.tabSessions.delete(tabId);
|
||||
this.tabOwnership.delete(tabId);
|
||||
|
||||
// Switch to another tab if we closed the active one
|
||||
@@ -560,9 +558,8 @@ export class BrowserManager {
|
||||
}
|
||||
|
||||
switchTab(id: number, opts?: { bringToFront?: boolean }): void {
|
||||
if (!this.pages.has(id)) throw new Error(`Tab ${id} not found`);
|
||||
if (!this.tabSessions.has(id)) throw new Error(`Tab ${id} not found`);
|
||||
this.activeTabId = id;
|
||||
this.activeFrame = null; // Frame context is per-tab
|
||||
// Only bring to front when explicitly requested (user-initiated tab switch).
|
||||
// Internal tab pinning (BROWSE_TAB) should NOT steal focus.
|
||||
if (opts?.bringToFront !== false) {
|
||||
@@ -592,7 +589,6 @@ export class BrowserManager {
|
||||
// Exact match — best case
|
||||
if (pageUrl === activeUrl && id !== this.activeTabId) {
|
||||
this.activeTabId = id;
|
||||
this.activeFrame = null;
|
||||
return;
|
||||
}
|
||||
// Fuzzy match — origin+pathname (handles query param / fragment differences)
|
||||
@@ -609,7 +605,6 @@ export class BrowserManager {
|
||||
// Fall back to fuzzy match
|
||||
if (fuzzyId !== null) {
|
||||
this.activeTabId = fuzzyId;
|
||||
this.activeFrame = null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -662,11 +657,24 @@ export class BrowserManager {
|
||||
return tabs;
|
||||
}
|
||||
|
||||
// ─── Page Access ───────────────────────────────────────────
|
||||
// ─── Session Access ────────────────────────────────────────
|
||||
/** Get the TabSession for the active tab. */
|
||||
getActiveSession(): TabSession {
|
||||
const session = this.tabSessions.get(this.activeTabId);
|
||||
if (!session) throw new Error('No active page. Use "browse goto <url>" first.');
|
||||
return session;
|
||||
}
|
||||
|
||||
/** Get a TabSession by tab ID. Used by /batch for parallel tab execution. */
|
||||
getSession(tabId: number): TabSession {
|
||||
const session = this.tabSessions.get(tabId);
|
||||
if (!session) throw new Error(`Tab ${tabId} not found`);
|
||||
return session;
|
||||
}
|
||||
|
||||
// ─── Page Access (delegates to active session) ─────────────
|
||||
getPage(): Page {
|
||||
const page = this.pages.get(this.activeTabId);
|
||||
if (!page) throw new Error('No active page. Use "browse goto <url>" first.');
|
||||
return page;
|
||||
return this.getActiveSession().page;
|
||||
}
|
||||
|
||||
getCurrentUrl(): string {
|
||||
@@ -677,60 +685,34 @@ export class BrowserManager {
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Ref Map ──────────────────────────────────────────────
|
||||
// ─── Ref Map (delegates to active session) ──────────────────
|
||||
setRefMap(refs: Map<string, RefEntry>) {
|
||||
this.refMap = refs;
|
||||
this.getActiveSession().setRefMap(refs);
|
||||
}
|
||||
|
||||
clearRefs() {
|
||||
this.refMap.clear();
|
||||
this.getActiveSession().clearRefs();
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a selector that may be a @ref (e.g., "@e3", "@c1") or a CSS selector.
|
||||
* Returns { locator } for refs or { selector } for CSS selectors.
|
||||
*/
|
||||
async resolveRef(selector: string): Promise<{ locator: Locator } | { selector: string }> {
|
||||
if (selector.startsWith('@e') || selector.startsWith('@c')) {
|
||||
const ref = selector.slice(1); // "e3" or "c1"
|
||||
const entry = this.refMap.get(ref);
|
||||
if (!entry) {
|
||||
throw new Error(
|
||||
`Ref ${selector} not found. Run 'snapshot' to get fresh refs.`
|
||||
);
|
||||
}
|
||||
const count = await entry.locator.count();
|
||||
if (count === 0) {
|
||||
throw new Error(
|
||||
`Ref ${selector} (${entry.role} "${entry.name}") is stale — element no longer exists. ` +
|
||||
`Run 'snapshot' for fresh refs.`
|
||||
);
|
||||
}
|
||||
return { locator: entry.locator };
|
||||
}
|
||||
return { selector };
|
||||
return this.getActiveSession().resolveRef(selector);
|
||||
}
|
||||
|
||||
/** Get the ARIA role for a ref selector, or null for CSS selectors / unknown refs. */
|
||||
getRefRole(selector: string): string | null {
|
||||
if (selector.startsWith('@e') || selector.startsWith('@c')) {
|
||||
const entry = this.refMap.get(selector.slice(1));
|
||||
return entry?.role ?? null;
|
||||
}
|
||||
return null;
|
||||
return this.getActiveSession().getRefRole(selector);
|
||||
}
|
||||
|
||||
getRefCount(): number {
|
||||
return this.refMap.size;
|
||||
return this.getActiveSession().getRefCount();
|
||||
}
|
||||
|
||||
// ─── Snapshot Diffing ─────────────────────────────────────
|
||||
// ─── Snapshot Diffing (delegates to active session) ─────────
|
||||
setLastSnapshot(text: string | null) {
|
||||
this.lastSnapshot = text;
|
||||
this.getActiveSession().setLastSnapshot(text);
|
||||
}
|
||||
|
||||
getLastSnapshot(): string | null {
|
||||
return this.lastSnapshot;
|
||||
return this.getActiveSession().getLastSnapshot();
|
||||
}
|
||||
|
||||
// ─── Dialog Control ───────────────────────────────────────
|
||||
@@ -782,30 +764,20 @@ export class BrowserManager {
|
||||
await page.close().catch(() => {});
|
||||
}
|
||||
this.pages.clear();
|
||||
this.clearRefs();
|
||||
this.tabSessions.clear();
|
||||
}
|
||||
|
||||
// ─── Frame context ─────────────────────────────────
|
||||
private activeFrame: import('playwright').Frame | null = null;
|
||||
|
||||
// ─── Frame context (delegates to active session) ────────────
|
||||
setFrame(frame: import('playwright').Frame | null): void {
|
||||
this.activeFrame = frame;
|
||||
this.getActiveSession().setFrame(frame);
|
||||
}
|
||||
|
||||
getFrame(): import('playwright').Frame | null {
|
||||
return this.activeFrame;
|
||||
return this.getActiveSession().getFrame();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the active frame if set, otherwise the current page.
|
||||
* Use this for operations that work on both Page and Frame (locator, evaluate, etc.).
|
||||
*/
|
||||
getActiveFrameOrPage(): import('playwright').Page | import('playwright').Frame {
|
||||
// Auto-recover from detached frames (iframe removed/navigated)
|
||||
if (this.activeFrame?.isDetached()) {
|
||||
this.activeFrame = null;
|
||||
}
|
||||
return this.activeFrame ?? this.getPage();
|
||||
return this.getActiveSession().getActiveFrameOrPage();
|
||||
}
|
||||
|
||||
// ─── State Save/Restore (shared by recreateContext + handoff) ─
|
||||
@@ -857,6 +829,7 @@ export class BrowserManager {
|
||||
const page = await this.context.newPage();
|
||||
const id = this.nextTabId++;
|
||||
this.pages.set(id, page);
|
||||
this.tabSessions.set(id, new TabSession(page));
|
||||
this.wirePageEvents(page);
|
||||
|
||||
if (saved.url) {
|
||||
@@ -924,6 +897,7 @@ export class BrowserManager {
|
||||
await page.close().catch(() => {});
|
||||
}
|
||||
this.pages.clear();
|
||||
this.tabSessions.clear();
|
||||
await this.context.close().catch(() => {});
|
||||
|
||||
// 3. Create new context with updated settings
|
||||
@@ -947,6 +921,7 @@ export class BrowserManager {
|
||||
// Fallback: create a clean context + blank tab
|
||||
try {
|
||||
this.pages.clear();
|
||||
this.tabSessions.clear();
|
||||
if (this.context) await this.context.close().catch(() => {});
|
||||
|
||||
const contextOptions: BrowserContextOptions = {
|
||||
@@ -1032,6 +1007,7 @@ export class BrowserManager {
|
||||
this.context = newContext;
|
||||
this.browser = newContext.browser();
|
||||
this.pages.clear();
|
||||
this.tabSessions.clear();
|
||||
this.connectionMode = 'headed';
|
||||
|
||||
if (Object.keys(this.extraHeaders).length > 0) {
|
||||
@@ -1074,9 +1050,13 @@ export class BrowserManager {
|
||||
* The meta-command handler calls handleSnapshot() after this.
|
||||
*/
|
||||
resume(): void {
|
||||
this.clearRefs();
|
||||
// Clear refs and frame on the active session
|
||||
try {
|
||||
const session = this.getActiveSession();
|
||||
session.clearRefs();
|
||||
session.setFrame(null);
|
||||
} catch {}
|
||||
this.resetFailures();
|
||||
this.activeFrame = null;
|
||||
}
|
||||
|
||||
getIsHeaded(): boolean {
|
||||
@@ -1101,11 +1081,12 @@ export class BrowserManager {
|
||||
|
||||
// ─── Console/Network/Dialog/Ref Wiring ────────────────────
|
||||
private wirePageEvents(page: Page) {
|
||||
// Track tab close — remove from pages map, switch to another tab
|
||||
// Track tab close — remove from pages and sessions maps, switch to another tab
|
||||
page.on('close', () => {
|
||||
for (const [id, p] of this.pages) {
|
||||
if (p === page) {
|
||||
this.pages.delete(id);
|
||||
this.tabSessions.delete(id);
|
||||
console.log(`[browse] Tab closed (id=${id}, remaining=${this.pages.size})`);
|
||||
// If the closed tab was active, switch to another
|
||||
if (this.activeTabId === id) {
|
||||
@@ -1121,8 +1102,13 @@ export class BrowserManager {
|
||||
// (lastSnapshot is NOT cleared — it's a text baseline for diffing)
|
||||
page.on('framenavigated', (frame) => {
|
||||
if (frame === page.mainFrame()) {
|
||||
this.clearRefs();
|
||||
this.activeFrame = null; // Navigation invalidates frame context
|
||||
// Find the TabSession for this page and clear its per-tab state
|
||||
for (const session of this.tabSessions.values()) {
|
||||
if (session.page === page) {
|
||||
session.onMainFrameNavigated();
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user