refactor: fix 9 issues from round 2 code review

Bug fixes:
- #1 /logs?level=error returned 404 — use pathname for route matching
- #2 Duplicate initialization — added 'initialized' guard flag

Should fix:
- #4 Added screenshot() to IPage interface
- #5 Graceful shutdown rejects pending requests before exit
- #6 Use process.execPath instead of 'npx tsx' for faster daemon spawn

Cleanup:
- #7 Removed duplicate 'browser' keyword in package.json
- #8 Removed unused normalizeEvaluateSource import from browser.ts
- #9 Changed dynamic import to static import in intercept.ts
- #10 Added explicit throw at end of sendCommand for clarity

61 tests pass (4 test files). Extension: 10.55KB.

extension/src/background.ts

@@ -87,7 +87,11 @@ function scheduleReconnect(): void {
 
 // ─── Lifecycle events ────────────────────────────────────────────────
 
+let initialized = false;
+
 function initialize(): void {
+  if (initialized) return;
+  initialized = true;
   chrome.alarms.create('keepalive', { periodInMinutes: 0.4 }); // ~24 seconds
   cdp.registerListeners();
   connect();

package.json

@@ -32,8 +32,7 @@
     "cli",
     "browser",
     "web",
-    "ai",
-    "browser"
+    "ai"
   ],
   "author": "jackwener",
   "license": "Apache-2.0",

src/browser/daemon-client.ts

@@ -108,4 +108,6 @@ export async function sendCommand(
       throw err;
     }
   }
+  // Unreachable — the loop always returns or throws
+  throw new Error('sendCommand: max retries exhausted');
 }

src/browser/mcp.ts

@@ -78,11 +78,10 @@ export class PlaywrightMCP {
       console.error(`[opencli] Starting daemon (${isTs ? 'ts' : 'js'})...`);
     }
 
-    // Use tsx for .ts files, node for .js files
-    const runtime = isTs ? 'npx' : process.execPath;
-    const args = isTs ? ['tsx', daemonPath] : [daemonPath];
-
-    this._daemonProc = spawn(runtime, args, {
+    // Use the current runtime to spawn daemon — avoids slow npx resolution.
+    // If already running under tsx (dev), process.execPath is tsx's node.
+    // If running compiled (node dist/), process.execPath is node.
+    this._daemonProc = spawn(process.execPath, [daemonPath], {
       detached: true,
       stdio: 'ignore',
       env: { ...process.env },

src/daemon.ts

@@ -70,8 +70,9 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise
   if (req.method === 'OPTIONS') { res.writeHead(204); res.end(); return; }
 
   const url = req.url ?? '/';
+  const pathname = url.split('?')[0];
 
-  if (req.method === 'GET' && url === '/status') {
+  if (req.method === 'GET' && pathname === '/status') {
     jsonResponse(res, 200, {
       ok: true,
       extensionConnected: extensionWs?.readyState === WebSocket.OPEN,
@@ -80,8 +81,9 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise
     return;
   }
 
-  if (req.method === 'GET' && url === '/logs') {
-    const level = new URL(url, `http://localhost:${PORT}`).searchParams.get('level');
+  if (req.method === 'GET' && pathname === '/logs') {
+    const params = new URL(url, `http://localhost:${PORT}`).searchParams;
+    const level = params.get('level');
     const filtered = level
       ? logBuffer.filter(e => e.level === level)
       : logBuffer;
@@ -89,7 +91,7 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise
     return;
   }
 
-  if (req.method === 'DELETE' && url === '/logs') {
+  if (req.method === 'DELETE' && pathname === '/logs') {
     logBuffer.length = 0;
     jsonResponse(res, 200, { ok: true });
     return;
@@ -199,5 +201,17 @@ httpServer.on('error', (err: NodeJS.ErrnoException) => {
 });
 
 // Graceful shutdown
-process.on('SIGTERM', () => process.exit(0));
-process.on('SIGINT', () => process.exit(0));
+function shutdown(): void {
+  // Reject all pending requests so CLI doesn't hang
+  for (const [, p] of pending) {
+    clearTimeout(p.timer);
+    p.reject(new Error('Daemon shutting down'));
+  }
+  pending.clear();
+  if (extensionWs) extensionWs.close();
+  httpServer.close();
+  process.exit(0);
+}
+
+process.on('SIGTERM', shutdown);
+process.on('SIGINT', shutdown);

src/pipeline/steps/browser.ts

@@ -4,7 +4,7 @@
  */
 
 import type { IPage } from '../../types.js';
-import { render, normalizeEvaluateSource } from '../template.js';
+import { render } from '../template.js';
 
 export async function stepNavigate(page: IPage | null, params: any, data: any, args: Record<string, any>): Promise<any> {
   const url = render(params, { args, data });

src/pipeline/steps/intercept.ts

@@ -3,7 +3,7 @@
  */
 
 import type { IPage } from '../../types.js';
-import { render } from '../template.js';
+import { render, normalizeEvaluateSource } from '../template.js';
 import { generateInterceptorJs, generateReadInterceptedJs } from '../../interceptor.js';
 
 export async function stepIntercept(page: IPage | null, params: any, data: any, args: Record<string, any>): Promise<any> {
@@ -24,7 +24,6 @@ export async function stepIntercept(page: IPage | null, params: any, data: any,
     await page!.goto(String(url));
   } else if (trigger.startsWith('evaluate:')) {
     const js = trigger.slice('evaluate:'.length);
-    const { normalizeEvaluateSource } = await import('../template.js');
     await page!.evaluate(normalizeEvaluateSource(render(js, { args, data }) as string));
   } else if (trigger.startsWith('click:')) {
     const ref = render(trigger.slice('click:'.length), { args, data });

src/types.ts

@@ -23,4 +23,5 @@ export interface IPage {
   autoScroll(options?: { times?: number; delayMs?: number }): Promise<void>;
   installInterceptor(pattern: string): Promise<void>;
   getInterceptedRequests(): Promise<any[]>;
+  screenshot(options?: { format?: 'png' | 'jpeg'; quality?: number; fullPage?: boolean; path?: string }): Promise<string>;
 }