diff --git a/packages/playwright-core/src/tools/backend/browserBackend.ts b/packages/playwright-core/src/tools/backend/browserBackend.ts index b5036df30cc6d..1c7b3dbeebfca 100644 --- a/packages/playwright-core/src/tools/backend/browserBackend.ts +++ b/packages/playwright-core/src/tools/backend/browserBackend.ts @@ -82,7 +82,15 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement } async callTool(name: string, rawArguments: mcpServer.CallToolRequest['params']['arguments'] & { _meta?: Record } = {}, signal?: AbortSignal): Promise { - this._idleTimer?.poke(); + this._idleTimer?.callStarted(); + try { + return await this._callTool(name, rawArguments, signal); + } finally { + this._idleTimer?.callFinished(); + } + } + + private async _callTool(name: string, rawArguments: mcpServer.CallToolRequest['params']['arguments'] & { _meta?: Record }, signal?: AbortSignal): Promise { const json = !!rawArguments._meta?.json; const formatError = (message: string): mcpServer.CallToolResult => ({ content: [{ type: 'text' as const, text: json ? JSON.stringify({ isError: true, error: message }, null, 2) : `### Error\n${message}` }], diff --git a/packages/playwright-core/src/tools/backend/idleTimer.ts b/packages/playwright-core/src/tools/backend/idleTimer.ts index 641d296681db9..8f3fc44b629fe 100644 --- a/packages/playwright-core/src/tools/backend/idleTimer.ts +++ b/packages/playwright-core/src/tools/backend/idleTimer.ts @@ -19,20 +19,46 @@ export const defaultIdleTimeout = 60 * 60 * 1000; export class IdleTimer { private _timeout: number; private _onIdle: () => void; + private _running = 0; private _timer: NodeJS.Timeout | undefined; + private _disposed = false; constructor(timeout: number, onIdle: () => void) { this._timeout = timeout; this._onIdle = onIdle; } + callStarted() { + if (this._disposed) + return; + ++this._running; + this._clearTimer(); + } + + callFinished() { + if (this._running > 0) + --this._running; + if (this._disposed) + return; + if (this._running === 0) + this._timer = setTimeout(this._onIdle, this._timeout).unref(); + } + poke() { - this.dispose(); - this._timer = setTimeout(this._onIdle, this._timeout); + if (this._disposed) + return; + this._clearTimer(); + if (!this._running) + this._timer = setTimeout(this._onIdle, this._timeout).unref(); } - dispose() { + private _clearTimer() { clearTimeout(this._timer); this._timer = undefined; } + + dispose() { + this._disposed = true; + this._clearTimer(); + } } diff --git a/tests/mcp/idle-timeout.spec.ts b/tests/mcp/idle-timeout.spec.ts index a245d691e4638..d68a66f725208 100644 --- a/tests/mcp/idle-timeout.spec.ts +++ b/tests/mcp/idle-timeout.spec.ts @@ -92,3 +92,28 @@ test('cdp endpoint only disconnects on idle and reconnects to the same pages', a 'close browser': 1, }); }); + +test('does not close the browser while a tool call is running', async ({ startClient, server }) => { + const { client, stderr } = await startClient({ + args: ['--idle-timeout=500'], + env: { DEBUG: 'pw:mcp:test' }, + }); + + await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }); + + // The wait outlasts the idle timeout, which only starts once the call completes. + expect(await client.callTool({ + name: 'browser_wait_for', + arguments: { time: 1 }, + })).toHaveResponse({ + code: `await new Promise(f => setTimeout(f, 1 * 1000));`, + }); + + expect(formatLog(stderr())).toEqual({ + 'create browser (persistent)': 1, + 'create context': 1, + }); +});