From 9739fed6ff2d7890a4f8597513f2f90abeb83714 Mon Sep 17 00:00:00 2001 From: Reagan Hsu Date: Sat, 9 May 2026 14:09:45 -0700 Subject: [PATCH] Prevent reruns from inheriting stale browser harness state Rerun keeps the app session id while replacing the assigned CDP target. The harness REPL was keyed only by app session id, so reruns could reuse a long-lived browser-harness-js process that still held CDP state for the prior target. Scope the deterministic REPL port by target id as well as session id, and make the stock REPL reconnect if an existing CDP socket cannot attach to the assigned target. Constraint: Rerun reuses the app session id but creates a replacement browser target Rejected: Kill all harness REPL processes on rerun | broader lifecycle churn and could affect unrelated active sessions Confidence: high Scope-risk: narrow Directive: Keep Browser Harness JS REPL identity aligned with the assigned target when target lifecycle changes Tested: yarn test --run tests/unit/hl/browserHarnessEnv.test.ts Tested: yarn test --run tests/unit/hl Tested: yarn typecheck Tested: yarn lint Tested: yarn test Not-tested: Live Electron rerun from this worktree --- app/src/main/hl/engines/browserHarnessEnv.ts | 6 +-- .../hl/stock/browser-harness-js/sdk/repl.ts | 7 +++- .../stock/browser-harness-js/sdk/session.ts | 19 +++++++-- app/tests/unit/hl/browserHarnessEnv.test.ts | 39 +++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 app/tests/unit/hl/browserHarnessEnv.test.ts diff --git a/app/src/main/hl/engines/browserHarnessEnv.ts b/app/src/main/hl/engines/browserHarnessEnv.ts index 237fe425..25c6de00 100644 --- a/app/src/main/hl/engines/browserHarnessEnv.ts +++ b/app/src/main/hl/engines/browserHarnessEnv.ts @@ -2,15 +2,15 @@ import { createHash } from 'node:crypto'; import path from 'node:path'; import type { SpawnContext } from './types'; -export function browserHarnessReplPort(sessionId: string): string { - const n = createHash('sha256').update(sessionId).digest().readUInt16BE(0); +export function browserHarnessReplPort(sessionId: string, targetId = ''): string { + const n = createHash('sha256').update(`${sessionId}:${targetId}`).digest().readUInt16BE(0); return String(18_000 + (n % 20_000)); } export function applyBrowserHarnessEnv(ctx: SpawnContext, env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const sdkDir = path.join(ctx.harnessDir, 'browser-harness-js', 'sdk'); env.PATH = env.PATH ? `${sdkDir}${path.delimiter}${env.PATH}` : sdkDir; - env.CDP_REPL_PORT = env.CDP_REPL_PORT ?? browserHarnessReplPort(ctx.sessionId); + env.CDP_REPL_PORT = env.CDP_REPL_PORT ?? browserHarnessReplPort(ctx.sessionId, ctx.targetId); env.CDP_REPL_LOG = env.CDP_REPL_LOG ?? path.join(ctx.harnessDir, `browser-harness-js-${ctx.sessionId}.log`); env.BU_SESSION_ID = ctx.sessionId; return env; diff --git a/app/src/main/hl/stock/browser-harness-js/sdk/repl.ts b/app/src/main/hl/stock/browser-harness-js/sdk/repl.ts index 40334ca2..4504a738 100644 --- a/app/src/main/hl/stock/browser-harness-js/sdk/repl.ts +++ b/app/src/main/hl/stock/browser-harness-js/sdk/repl.ts @@ -34,7 +34,12 @@ async function connectToAssignedTarget(): Promise<{ targetId: string; port: numb if (!session.isConnected()) { await session.connect({ port, targetId }); } else { - await session.use(targetId).catch(() => {}); + try { + await session.use(targetId); + } catch { + session.close(); + await session.connect({ port, targetId }); + } } await Promise.all([ diff --git a/app/src/main/hl/stock/browser-harness-js/sdk/session.ts b/app/src/main/hl/stock/browser-harness-js/sdk/session.ts index 4fc530ef..825bae10 100644 --- a/app/src/main/hl/stock/browser-harness-js/sdk/session.ts +++ b/app/src/main/hl/stock/browser-harness-js/sdk/session.ts @@ -124,10 +124,16 @@ export class Session implements Transport { const timer = setTimeout(() => finish(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs); ws.addEventListener('open', () => finish()); ws.addEventListener('error', (e) => finish(new Error(`WS error: ${(e as any)?.message ?? 'connect failed (likely 403, permission not granted, or port closed)'}`))); - ws.addEventListener('message', (e) => this.onMessage(String(e.data))); + ws.addEventListener('message', (e) => { + if (this.ws === ws) this.onMessage(String(e.data)); + }); ws.addEventListener('close', () => { - for (const [, p] of this.pending) p.reject(new Error('CDP socket closed')); - this.pending.clear(); + if (this.ws === ws) { + this.ws = undefined; + this.activeSessionId = undefined; + for (const [, p] of this.pending) p.reject(new Error('CDP socket closed')); + this.pending.clear(); + } finish(new Error('WS closed before open (likely 403 or port closed)')); }); this.ws = ws; @@ -139,7 +145,12 @@ export class Session implements Transport { } close(): void { - this.ws?.close(); + const ws = this.ws; + this.ws = undefined; + this.activeSessionId = undefined; + for (const [, p] of this.pending) p.reject(new Error('CDP socket closed')); + this.pending.clear(); + ws?.close(); } /** diff --git a/app/tests/unit/hl/browserHarnessEnv.test.ts b/app/tests/unit/hl/browserHarnessEnv.test.ts new file mode 100644 index 00000000..cd409360 --- /dev/null +++ b/app/tests/unit/hl/browserHarnessEnv.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest'; +import { applyBrowserHarnessEnv, browserHarnessReplPort } from '../../../src/main/hl/engines/browserHarnessEnv'; +import type { SpawnContext } from '../../../src/main/hl/engines/types'; + +function spawnContext(targetId: string): SpawnContext { + return { + prompt: 'Open example.com', + harnessDir: '/tmp/harness', + sessionId: 'session-123', + targetId, + cdpPort: 9222, + attachmentRefs: [], + }; +} + +describe('browser harness environment', () => { + it('scopes the REPL port to the assigned target as well as the app session', () => { + const firstTarget = browserHarnessReplPort('session-123', 'target-a'); + const secondTarget = browserHarnessReplPort('session-123', 'target-b'); + + expect(browserHarnessReplPort('session-123', 'target-a')).toBe(firstTarget); + expect(secondTarget).not.toBe(firstTarget); + }); + + it('gives reruns with a replacement browser target a fresh REPL port', () => { + const firstEnv = applyBrowserHarnessEnv(spawnContext('old-target'), {}); + const rerunEnv = applyBrowserHarnessEnv(spawnContext('new-target'), {}); + + expect(firstEnv.CDP_REPL_PORT).toBe(browserHarnessReplPort('session-123', 'old-target')); + expect(rerunEnv.CDP_REPL_PORT).toBe(browserHarnessReplPort('session-123', 'new-target')); + expect(rerunEnv.CDP_REPL_PORT).not.toBe(firstEnv.CDP_REPL_PORT); + }); + + it('preserves an explicit REPL port override', () => { + const env = applyBrowserHarnessEnv(spawnContext('target-a'), { CDP_REPL_PORT: '9876' }); + + expect(env.CDP_REPL_PORT).toBe('9876'); + }); +});