v0.14.0 changeset (#534)

* v0.14.0 changeset

* fixes

* improvements

src/browser.test.ts

@@ -1,5 +1,5 @@
-import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
-import { BrowserManager } from './browser.js';
+import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest';
+import { BrowserManager, getDefaultTimeout } from './browser.js';
 import { executeCommand } from './actions.js';
 import { chromium } from 'playwright-core';
 
@@ -1196,3 +1196,56 @@ describe('BrowserManager', () => {
     });
   });
 });
+
+describe('getDefaultTimeout', () => {
+  const originalEnv = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...originalEnv };
+  });
+
+  it('should return 25000 when env var is not set', () => {
+    delete process.env.AGENT_BROWSER_DEFAULT_TIMEOUT;
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should return parsed value when env var is a valid positive integer', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '10000';
+    expect(getDefaultTimeout()).toBe(10000);
+  });
+
+  it('should return 25000 for negative values', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '-1';
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should return 25000 for zero', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '0';
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should return 25000 for values below 1000ms floor', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '500';
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should accept exactly 1000ms as the minimum', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '1000';
+    expect(getDefaultTimeout()).toBe(1000);
+  });
+
+  it('should return 25000 for non-numeric strings', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = 'abc';
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should return 25000 for empty string', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '';
+    expect(getDefaultTimeout()).toBe(25000);
+  });
+
+  it('should allow overriding above 25s for users who need longer timeouts', () => {
+    process.env.AGENT_BROWSER_DEFAULT_TIMEOUT = '60000';
+    expect(getDefaultTimeout()).toBe(60000);
+  });
+});

src/browser.ts

@@ -29,19 +29,21 @@ import {
 } from './state-utils.js';
 
 /**
- * Returns the default Playwright timeout for standard operations.
+ * Returns the default Playwright timeout in milliseconds for standard operations.
  * Can be overridden via the AGENT_BROWSER_DEFAULT_TIMEOUT environment variable.
+ * Default is 25s, which is below the CLI's 30s IPC read timeout to ensure
+ * Playwright errors are returned before the CLI gives up with EAGAIN.
  * CDP and recording contexts use a shorter fixed timeout (10s) and are not affected.
  */
-function getDefaultTimeout(): number {
+export function getDefaultTimeout(): number {
   const envValue = process.env.AGENT_BROWSER_DEFAULT_TIMEOUT;
   if (envValue) {
     const parsed = parseInt(envValue, 10);
-    if (!isNaN(parsed) && parsed > 0) {
+    if (!isNaN(parsed) && parsed >= 1000) {
       return parsed;
     }
   }
-  return 60000;
+  return 25000;
 }
 
 // Screencast frame data from CDP

src/daemon.test.ts

@@ -1,7 +1,9 @@
-import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import * as os from 'os';
 import * as path from 'path';
-import { getSocketDir } from './daemon.js';
+import * as net from 'net';
+import { EventEmitter } from 'events';
+import { getSocketDir, safeWrite } from './daemon.js';
 
 /**
  * HTTP request detection pattern used in daemon.ts to prevent cross-origin attacks.
@@ -94,3 +96,66 @@ describe('getSocketDir', () => {
     });
   });
 });
+
+function createMockSocket(opts: { destroyed?: boolean; writeReturns?: boolean } = {}) {
+  const emitter = new EventEmitter();
+  const socket = Object.assign(emitter, {
+    destroyed: opts.destroyed ?? false,
+    write: vi.fn().mockReturnValue(opts.writeReturns ?? true),
+    removeListener: emitter.removeListener.bind(emitter),
+  });
+  return socket as unknown as net.Socket;
+}
+
+describe('safeWrite', () => {
+  it('should resolve immediately when socket.write returns true', async () => {
+    const socket = createMockSocket({ writeReturns: true });
+    await safeWrite(socket, 'hello\n');
+    expect(socket.write).toHaveBeenCalledWith('hello\n');
+  });
+
+  it('should resolve immediately when socket is already destroyed', async () => {
+    const socket = createMockSocket({ destroyed: true });
+    await safeWrite(socket, 'hello\n');
+    expect(socket.write).not.toHaveBeenCalled();
+  });
+
+  it('should wait for drain event when socket.write returns false', async () => {
+    const socket = createMockSocket({ writeReturns: false });
+    const promise = safeWrite(socket, 'big payload');
+
+    // Simulate drain after a tick
+    setTimeout(() => socket.emit('drain'), 0);
+    await promise;
+
+    expect(socket.write).toHaveBeenCalledWith('big payload');
+  });
+
+  it('should reject on socket error while waiting for drain', async () => {
+    const socket = createMockSocket({ writeReturns: false });
+    const promise = safeWrite(socket, 'data');
+
+    setTimeout(() => socket.emit('error', new Error('connection reset')), 0);
+    await expect(promise).rejects.toThrow('connection reset');
+  });
+
+  it('should resolve on socket close while waiting for drain', async () => {
+    const socket = createMockSocket({ writeReturns: false });
+    const promise = safeWrite(socket, 'data');
+
+    setTimeout(() => socket.emit('close'), 0);
+    await promise;
+  });
+
+  it('should clean up listeners after drain resolves', async () => {
+    const socket = createMockSocket({ writeReturns: false });
+    const promise = safeWrite(socket, 'data');
+
+    setTimeout(() => socket.emit('drain'), 0);
+    await promise;
+
+    expect(socket.listenerCount('drain')).toBe(0);
+    expect(socket.listenerCount('error')).toBe(0);
+    expect(socket.listenerCount('close')).toBe(0);
+  });
+});

src/daemon.ts

@@ -26,7 +26,7 @@ type Manager = BrowserManager | IOSManager;
  * If the kernel buffer is full (socket.write returns false),
  * waits for the 'drain' event before resolving.
  */
-function safeWrite(socket: net.Socket, payload: string): Promise<void> {
+export function safeWrite(socket: net.Socket, payload: string): Promise<void> {
   return new Promise((resolve, reject) => {
     if (socket.destroyed) {
       resolve();
@@ -592,8 +592,13 @@ export async function startDaemon(options?: {
         commandQueue.push(line);
       }
 
-      processQueue().catch(() => {
-        // Socket write failures during queue processing are non-fatal
+      processQueue().catch((err) => {
+        // Socket write failures during queue processing are non-fatal;
+        // the client has likely disconnected.
+        console.warn('[warn] processQueue error:', err?.message ?? err);
+        if (process.env.AGENT_BROWSER_DEBUG === '1') {
+          console.error('[DEBUG] processQueue error (full):', err);
+        }
       });
     });