Skip to content

Commit cb9521d

Browse files
authored
fix(extension): per-workspace idle timeout for browser sessions (jackwener#1064)
* fix(extension): per-workspace idle timeout for browser sessions (jackwener#1058) The global 30s WINDOW_IDLE_TIMEOUT was too aggressive for interactive `opencli browser` commands where users type manually between invocations. - browser:*/operate:* workspaces now default to 10 min idle timeout - Adapter workspaces keep the existing 30s timeout - Support custom timeout via OPENCLI_BROWSER_TIMEOUT env var (seconds) or command-level idleTimeout parameter - Surface sessionExpired warning when a new window is created after the previous session timed out - Fix stale comment (said 120s, actual was 30s) Closes jackwener#1058 * fix: resolve sessionExpired double-delete race and timeout override lifecycle Addresses @codex-coder review blockers: 1. sessionExpired flag was never set because getAutomationWindow() consumed expiredWorkspaces before handleCommand() could check it. Fix: use .has() in getAutomationWindow, only .delete() in handleCommand. 2. workspaceTimeoutOverrides was never cleaned up — once set, it persisted until extension restart. Fix: clear override on idle timeout expiry, explicit close-window, and borrowed-session detach. Adds 5 tests covering: - browser:* uses 10min timeout (not 30s) - sessionExpired flag is set and consumed correctly - workspaceTimeoutOverrides cleared on idle expiry - workspaceTimeoutOverrides cleared on explicit close - idleTimeout from command applies to workspace override * refactor: remove sessionExpired warning per product decision @WAWQAQ decided session-expired warning is not needed. Remove expiredWorkspaces tracking, sessionExpired flag from protocol, and related CLI-side warning code. Keep per-workspace timeout and override lifecycle cleanup. * fix: clean up workspaceTimeoutOverrides on user-initiated window close The windows.onRemoved listener was missing workspaceTimeoutOverrides cleanup, causing stale overrides to persist across sessions when users manually close the automation window.
1 parent 025df31 commit cb9521d

8 files changed

Lines changed: 301 additions & 81 deletions

File tree

extension/dist/background.js

Lines changed: 142 additions & 68 deletions
Large diffs are not rendered by default.

extension/src/background.test.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,4 +332,108 @@ describe('background tab isolation', () => {
332332
expect(chrome.windows.remove).toHaveBeenCalledWith(1);
333333
expect(mod.__test__.getSession('site:notebooklm')).toBeNull();
334334
});
335+
336+
it('uses 10-minute timeout for browser:* workspaces', async () => {
337+
const { chrome } = createChromeMock();
338+
vi.useFakeTimers();
339+
vi.stubGlobal('chrome', chrome);
340+
341+
const mod = await import('./background');
342+
mod.__test__.setAutomationWindowId('browser:default', 1);
343+
344+
mod.__test__.resetWindowIdleTimer('browser:default');
345+
// After 30s (adapter timeout), session should still be alive
346+
await vi.advanceTimersByTimeAsync(30001);
347+
expect(mod.__test__.getSession('browser:default')).not.toBeNull();
348+
349+
// After 10 min total, session should be cleaned up
350+
await vi.advanceTimersByTimeAsync(600000 - 30001);
351+
expect(chrome.windows.remove).toHaveBeenCalledWith(1);
352+
expect(mod.__test__.getSession('browser:default')).toBeNull();
353+
});
354+
355+
it('clears workspaceTimeoutOverrides on idle expiry', async () => {
356+
const { chrome } = createChromeMock();
357+
vi.useFakeTimers();
358+
vi.stubGlobal('chrome', chrome);
359+
360+
const mod = await import('./background');
361+
mod.__test__.setAutomationWindowId('browser:test', 1);
362+
363+
// Set a custom timeout override
364+
mod.__test__.workspaceTimeoutOverrides.set('browser:test', 120_000);
365+
expect(mod.__test__.getIdleTimeout('browser:test')).toBe(120_000);
366+
367+
// Trigger idle timer with the custom timeout
368+
mod.__test__.resetWindowIdleTimer('browser:test');
369+
await vi.advanceTimersByTimeAsync(120001);
370+
371+
// Override should be cleaned up
372+
expect(mod.__test__.workspaceTimeoutOverrides.has('browser:test')).toBe(false);
373+
expect(mod.__test__.getSession('browser:test')).toBeNull();
374+
// Should fall back to default interactive timeout
375+
expect(mod.__test__.getIdleTimeout('browser:test')).toBe(600_000);
376+
});
377+
378+
it('clears workspaceTimeoutOverrides on explicit close', async () => {
379+
const { chrome } = createChromeMock();
380+
vi.stubGlobal('chrome', chrome);
381+
382+
const mod = await import('./background');
383+
mod.__test__.setAutomationWindowId('browser:close-test', 1);
384+
mod.__test__.workspaceTimeoutOverrides.set('browser:close-test', 300_000);
385+
386+
const result = await mod.__test__.handleCommand({
387+
id: 'close-1',
388+
action: 'close-window',
389+
workspace: 'browser:close-test',
390+
});
391+
392+
expect(result.ok).toBe(true);
393+
expect(mod.__test__.workspaceTimeoutOverrides.has('browser:close-test')).toBe(false);
394+
});
395+
396+
it('applies idleTimeout from command to workspace override', async () => {
397+
const { chrome } = createChromeMock();
398+
vi.stubGlobal('chrome', chrome);
399+
400+
const mod = await import('./background');
401+
mod.__test__.setAutomationWindowId('browser:custom', 1);
402+
403+
// Default for browser:* is 10 min
404+
expect(mod.__test__.getIdleTimeout('browser:custom')).toBe(600_000);
405+
406+
// Send a command with custom idleTimeout (in seconds)
407+
await mod.__test__.handleCommand({
408+
id: 'custom-1',
409+
action: 'sessions',
410+
workspace: 'browser:custom',
411+
idleTimeout: 120,
412+
});
413+
414+
// Override should now be 120s = 120000ms
415+
expect(mod.__test__.getIdleTimeout('browser:custom')).toBe(120_000);
416+
});
417+
418+
it('clears workspaceTimeoutOverrides when user manually closes automation window', async () => {
419+
const { chrome } = createChromeMock();
420+
vi.stubGlobal('chrome', chrome);
421+
422+
const mod = await import('./background');
423+
424+
// Set up a session with window ID 42 and a custom timeout override
425+
mod.__test__.setAutomationWindowId('browser:manual', 42);
426+
mod.__test__.workspaceTimeoutOverrides.set('browser:manual', 180_000);
427+
expect(mod.__test__.getIdleTimeout('browser:manual')).toBe(180_000);
428+
429+
// Simulate user closing the window — invoke the onRemoved listener
430+
const onRemovedListener = chrome.windows.onRemoved.addListener.mock.calls[0][0];
431+
await onRemovedListener(42);
432+
433+
// Session and override should both be cleaned up
434+
expect(mod.__test__.getSession('browser:manual')).toBeNull();
435+
expect(mod.__test__.workspaceTimeoutOverrides.has('browser:manual')).toBe(false);
436+
// Should fall back to default interactive timeout
437+
expect(mod.__test__.getIdleTimeout('browser:manual')).toBe(600_000);
438+
});
335439
});

extension/src/background.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ function scheduleReconnect(): void {
118118
// ─── Automation window isolation ─────────────────────────────────────
119119
// All opencli operations happen in a dedicated Chrome window so the
120120
// user's active browsing session is never touched.
121-
// The window auto-closes after 120s of idle (no commands).
121+
// The window auto-closes after a period of idle (no commands).
122+
// Interactive workspaces (browser:*, operate:*) get a longer timeout (10 min)
123+
// since users type commands manually; adapter workspaces keep a short 30s timeout.
122124

123125
type AutomationSession = {
124126
windowId: number;
@@ -129,7 +131,21 @@ type AutomationSession = {
129131
};
130132

131133
const automationSessions = new Map<string, AutomationSession>();
132-
const WINDOW_IDLE_TIMEOUT = 30000; // 30s — quick cleanup after command finishes
134+
const IDLE_TIMEOUT_DEFAULT = 30_000; // 30s — adapter-driven automation
135+
const IDLE_TIMEOUT_INTERACTIVE = 600_000; // 10min — human-paced browser:* / operate:*
136+
137+
/** Per-workspace custom timeout overrides set via command.idleTimeout */
138+
const workspaceTimeoutOverrides = new Map<string, number>();
139+
140+
function getIdleTimeout(workspace: string): number {
141+
const override = workspaceTimeoutOverrides.get(workspace);
142+
if (override !== undefined) return override;
143+
if (workspace.startsWith('browser:') || workspace.startsWith('operate:')) {
144+
return IDLE_TIMEOUT_INTERACTIVE;
145+
}
146+
return IDLE_TIMEOUT_DEFAULT;
147+
}
148+
133149
let windowFocused = false; // set per-command from daemon's OPENCLI_WINDOW_FOCUSED
134150

135151
function getWorkspaceKey(workspace?: string): string {
@@ -140,23 +156,26 @@ function resetWindowIdleTimer(workspace: string): void {
140156
const session = automationSessions.get(workspace);
141157
if (!session) return;
142158
if (session.idleTimer) clearTimeout(session.idleTimer);
143-
session.idleDeadlineAt = Date.now() + WINDOW_IDLE_TIMEOUT;
159+
const timeout = getIdleTimeout(workspace);
160+
session.idleDeadlineAt = Date.now() + timeout;
144161
session.idleTimer = setTimeout(async () => {
145162
const current = automationSessions.get(workspace);
146163
if (!current) return;
147164
if (!current.owned) {
148165
console.log(`[opencli] Borrowed workspace ${workspace} detached from window ${current.windowId} (idle timeout)`);
166+
workspaceTimeoutOverrides.delete(workspace);
149167
automationSessions.delete(workspace);
150168
return;
151169
}
152170
try {
153171
await chrome.windows.remove(current.windowId);
154-
console.log(`[opencli] Automation window ${current.windowId} (${workspace}) closed (idle timeout)`);
172+
console.log(`[opencli] Automation window ${current.windowId} (${workspace}) closed (idle timeout, ${timeout / 1000}s)`);
155173
} catch {
156174
// Already gone
157175
}
176+
workspaceTimeoutOverrides.delete(workspace);
158177
automationSessions.delete(workspace);
159-
}, WINDOW_IDLE_TIMEOUT);
178+
}, timeout);
160179
}
161180

162181
/** Get or create the dedicated automation window.
@@ -191,7 +210,7 @@ async function getAutomationWindow(workspace: string, initialUrl?: string): Prom
191210
const session: AutomationSession = {
192211
windowId: win.id!,
193212
idleTimer: null,
194-
idleDeadlineAt: Date.now() + WINDOW_IDLE_TIMEOUT,
213+
idleDeadlineAt: Date.now() + getIdleTimeout(workspace),
195214
owned: true,
196215
preferredTabId: null,
197216
};
@@ -229,6 +248,7 @@ chrome.windows.onRemoved.addListener(async (windowId) => {
229248
console.log(`[opencli] Automation window closed (${workspace})`);
230249
if (session.idleTimer) clearTimeout(session.idleTimer);
231250
automationSessions.delete(workspace);
251+
workspaceTimeoutOverrides.delete(workspace);
232252
}
233253
}
234254
});
@@ -280,6 +300,10 @@ chrome.runtime.onMessage.addListener((msg, _sender, sendResponse) => {
280300
async function handleCommand(cmd: Command): Promise<Result> {
281301
const workspace = getWorkspaceKey(cmd.workspace);
282302
windowFocused = cmd.windowFocused === true;
303+
// Apply custom idle timeout if specified in the command
304+
if (cmd.idleTimeout != null && cmd.idleTimeout > 0) {
305+
workspaceTimeoutOverrides.set(workspace, cmd.idleTimeout * 1000);
306+
}
283307
// Reset idle timer on every command (window stays alive while active)
284308
resetWindowIdleTimer(workspace);
285309
try {
@@ -387,7 +411,7 @@ function setWorkspaceSession(workspace: string, session: Omit<AutomationSession,
387411
automationSessions.set(workspace, {
388412
...session,
389413
idleTimer: null,
390-
idleDeadlineAt: Date.now() + WINDOW_IDLE_TIMEOUT,
414+
idleDeadlineAt: Date.now() + getIdleTimeout(workspace),
391415
});
392416
}
393417

@@ -781,6 +805,7 @@ async function handleCloseWindow(cmd: Command, workspace: string): Promise<Resul
781805
}
782806
}
783807
if (session.idleTimer) clearTimeout(session.idleTimer);
808+
workspaceTimeoutOverrides.delete(workspace);
784809
automationSessions.delete(workspace);
785810
}
786811
return { id: cmd.id, ok: true, data: { closed: true } };
@@ -886,6 +911,9 @@ export const __test__ = {
886911
handleBindCurrent,
887912
resolveTabId,
888913
resetWindowIdleTimer,
914+
handleCommand,
915+
getIdleTimeout,
916+
workspaceTimeoutOverrides,
889917
getSession: (workspace: string = 'default') => automationSessions.get(workspace) ?? null,
890918
getAutomationWindowId: (workspace: string = 'default') => automationSessions.get(workspace)?.windowId ?? null,
891919
setAutomationWindowId: (workspace: string, windowId: number | null) => {

extension/src/protocol.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ export interface Command {
6565
cdpParams?: Record<string, unknown>;
6666
/** When true, automation windows are created in the foreground (focused) */
6767
windowFocused?: boolean;
68+
/** Custom idle timeout in seconds for this workspace session. Overrides the default. */
69+
idleTimeout?: number;
6870
}
6971

7072
export interface Result {

src/browser/bridge.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export class BrowserBridge implements IBrowserFactory {
3030
return this._state;
3131
}
3232

33-
async connect(opts: { timeout?: number; workspace?: string } = {}): Promise<IPage> {
33+
async connect(opts: { timeout?: number; workspace?: string; idleTimeout?: number } = {}): Promise<IPage> {
3434
if (this._state === 'connected' && this._page) return this._page;
3535
if (this._state === 'connecting') throw new Error('Already connecting');
3636
if (this._state === 'closing') throw new Error('Session is closing');
@@ -40,7 +40,7 @@ export class BrowserBridge implements IBrowserFactory {
4040

4141
try {
4242
await this._ensureDaemon(opts.timeout);
43-
this._page = new Page(opts.workspace);
43+
this._page = new Page(opts.workspace, opts.idleTimeout);
4444
this._state = 'connected';
4545
return this._page;
4646
} catch (err) {

src/browser/daemon-client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ export interface DaemonCommand {
5050
cdpParams?: Record<string, unknown>;
5151
/** When true, automation windows are created in the foreground */
5252
windowFocused?: boolean;
53+
/** Custom idle timeout in seconds for this workspace session. Overrides the default. */
54+
idleTimeout?: number;
5355
}
5456

5557
export interface DaemonResult {

src/browser/page.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ function isUnsupportedNetworkCaptureError(err: unknown): boolean {
3030
* Page — implements IPage by talking to the daemon via HTTP.
3131
*/
3232
export class Page extends BasePage {
33-
constructor(private readonly workspace: string = 'default') {
33+
private readonly _idleTimeout: number | undefined;
34+
35+
constructor(private readonly workspace: string = 'default', idleTimeout?: number) {
3436
super();
37+
this._idleTimeout = idleTimeout;
3538
}
3639

3740
/** Active page identity (targetId), set after navigate and used in all subsequent commands */
@@ -40,15 +43,16 @@ export class Page extends BasePage {
4043
private _networkCaptureWarned = false;
4144

4245
/** Helper: spread workspace into command params */
43-
private _wsOpt(): { workspace: string } {
44-
return { workspace: this.workspace };
46+
private _wsOpt(): { workspace: string; idleTimeout?: number } {
47+
return { workspace: this.workspace, ...(this._idleTimeout != null && { idleTimeout: this._idleTimeout }) };
4548
}
4649

4750
/** Helper: spread workspace + page identity into command params */
4851
private _cmdOpts(): Record<string, unknown> {
4952
return {
5053
workspace: this.workspace,
5154
...(this._page !== undefined && { page: this._page }),
55+
...(this._idleTimeout != null && { idleTimeout: this._idleTimeout }),
5256
};
5357
}
5458

src/cli.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ const CLI_FILE = fileURLToPath(import.meta.url);
3131
async function getBrowserPage(): Promise<import('./types.js').IPage> {
3232
const { BrowserBridge } = await import('./browser/index.js');
3333
const bridge = new BrowserBridge();
34-
return bridge.connect({ timeout: 30, workspace: 'browser:default' });
34+
const envTimeout = process.env.OPENCLI_BROWSER_TIMEOUT;
35+
const idleTimeout = envTimeout ? parseInt(envTimeout, 10) : undefined;
36+
return bridge.connect({
37+
timeout: 30,
38+
workspace: 'browser:default',
39+
...(idleTimeout && idleTimeout > 0 && { idleTimeout }),
40+
});
3541
}
3642

3743
function parsePositiveIntOption(val: string | undefined, label: string, fallback: number): number {

0 commit comments

Comments
 (0)