Skip to content

Fix Browser Use runtime in codexui#126

Open
friuns2 wants to merge 5 commits intomainfrom
codex/fix-browser-use-runtime
Open

Fix Browser Use runtime in codexui#126
friuns2 wants to merge 5 commits intomainfrom
codex/fix-browser-use-runtime

Conversation

@friuns2
Copy link
Copy Markdown
Owner

@friuns2 friuns2 commented May 4, 2026

Summary

  • prefer the bundled Codex.app CLI on macOS so codexui uses the same app-server version as the desktop app
  • register bundled node_repl and start a session-scoped Browser Use backend for codexui chats
  • document and test the Browser Use runtime path

Tests

  • pnpm run build:cli
  • pnpm exec vitest run src/commandResolution.test.ts src/server/appServerRuntimeConfig.test.ts src/server/codexAppServerBridge.inlinePayload.test.ts src/server/codexAppServerBridge.authRefresh.test.ts
  • Live codexui RPC at http://127.0.0.1:4173: Browser Use opened https://example.com and returned {"title":"Example Domain","url":"https://example.com/"}

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Enable Browser Use runtime in codexui with bundled Codex.app integration

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Prefer bundled Codex.app CLI on macOS for consistent app-server versioning
• Register bundled node_repl MCP server when available on macOS
• Start session-scoped Browser Use backend for codexui chats
• Add comprehensive tests for command resolution and runtime configuration
Diagram
flowchart LR
  A["codexui RPC Request"] --> B["Detect turn/start or thread/start"]
  B --> C["ensureBrowserUseBackendForSession"]
  C --> D["Launch Playwright Browser"]
  D --> E["Create Unix Socket Server"]
  E --> F["Browser Use Client Connects"]
  F --> G["Handle CDP Commands"]
  G --> H["Return Browser Results"]
Loading

Grey Divider

File Changes

1. src/commandResolution.ts ✨ Enhancement +4/-1

Prioritize bundled Codex.app CLI on macOS

• Add MACOS_CODEX_APP_COMMAND constant pointing to bundled Codex.app CLI
• Update resolveCodexCommand() to prioritize bundled macOS command before PATH lookup
• Maintain CODEXUI_CODEX_COMMAND env var as highest-priority override

src/commandResolution.ts


2. src/commandResolution.test.ts 🧪 Tests +69/-0

Add command resolution tests for macOS bundled CLI

• Test that bundled Codex.app command is preferred on macOS when available
• Test that CODEXUI_CODEX_COMMAND env var overrides bundled command
• Mock filesystem, OS, and child_process for isolated testing

src/commandResolution.test.ts


3. src/server/appServerRuntimeConfig.ts ✨ Enhancement +14/-1

Register bundled node_repl MCP server on macOS

• Add MACOS_CODEX_APP_NODE_REPL_COMMAND constant for bundled node_repl
• Update buildAppServerArgs() to register bundled node_repl MCP server on macOS
• Include --disable-sandbox flag when node_repl is available

src/server/appServerRuntimeConfig.ts


View more (4)
4. src/server/appServerRuntimeConfig.test.ts 🧪 Tests +47/-0

Add app-server runtime configuration tests

• Test that bundled node_repl is added to app-server args on macOS
• Test that node_repl is not added on non-macOS platforms
• Mock filesystem and platform detection for isolated testing

src/server/appServerRuntimeConfig.test.ts


5. src/server/browserUseBackend.ts ✨ Enhancement +412/-0

Implement Browser Use backend with Playwright integration

• Implement session-scoped Browser Use backend using Unix sockets
• Launch Playwright browser and manage tabs with CDP session support
• Handle JSON-RPC messages for Browser Use protocol (createTab, getTabs, executeCdp, etc.)
• Patch Browser Use client to fallback to Unix socket when native transport unavailable
• Authorize socket peers using native module for security

src/server/browserUseBackend.ts


6. src/server/codexAppServerBridge.ts ✨ Enhancement +19/-6

Integrate Browser Use backend with RPC lifecycle

• Import ensureBrowserUseBackendForSession from browserUseBackend module
• Call ensureBrowserUseBackendForSession() on turn/start and thread/start RPC methods
• Use buildAppServerArgs() for app-server configuration instead of hardcoded args
• Remove hardcoded app-server args from class initialization

src/server/codexAppServerBridge.ts


7. tests.md 📝 Documentation +32/-0

Document Browser Use runtime manual testing procedure

• Add comprehensive manual test procedure for Browser Use plugin runtime
• Document prerequisites, test steps, and expected results
• Include theme switching validation and cleanup instructions

tests.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 4, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Hardcoded Browser Use path🐞 Bug ☼ Reliability
Description
ensureBrowserUseBackendForSession patches a Browser Use client script at a hardcoded local user path
(/Users/igor/...), so readFile/writeFile will throw on most machines and cause /codex-api/rpc
turn/start + thread/start to fail with 502 before the RPC is forwarded.
Code

src/server/browserUseBackend.ts[R69-133]

+const BROWSER_USE_SOCKET_DIR = '/tmp/codex-browser-use'
+const CODEX_BROWSER_USE_PEER_AUTHORIZATION =
+  '/Applications/Codex.app/Contents/Resources/native/browser-use-peer-authorization.node'
+const BROWSER_USE_CLIENT_PATH =
+  '/Users/igor/.codex/plugins/cache/openai-bundled/browser-use/0.1.0-alpha1/scripts/browser-client.mjs'
+const BROWSER_USE_NATIVE_CREATE_SOURCE =
+  'static async create(t){let n=eN();if(n!=null){let r=await n.createConnection(t),i=new e(r);return r.on("data",o=>i.handleData(o)),r.on("close",()=>{i.socket===r&&(i.socket=null)}),i}throw new Error(Q7())}'
+const BROWSER_USE_CODEXUI_CREATE_SOURCE =
+  'static async create(t){let n=eN();if(n!=null)try{let r=await n.createConnection(t),i=new e(r);return r.on("data",o=>i.handleData(o)),r.on("close",()=>{i.socket===r&&(i.socket=null)}),i}catch(r){if(!String(t).includes("codexui-"))throw r}try{let{createConnection:r}=await import("node:net"),i=r(t),o=new e(i);return await new Promise((s,a)=>{i.once("connect",s),i.once("error",a)}),i.on("data",s=>o.handleData(s)),i.on("close",()=>{o.socket===i&&(o.socket=null)}),o}catch(r){throw new Error(Q7())}}'
+const browserUseBackends = new Map<string, BrowserUseBackendRecord>()
+const require = createRequire(import.meta.url)
+let browserUseClientPatchPromise: Promise<void> | null = null
+
+export async function ensureBrowserUseBackendForSession(sessionId: string): Promise<void> {
+  const normalizedSessionId = sessionId.trim()
+  if (!normalizedSessionId || browserUseBackends.has(normalizedSessionId)) {
+    return
+  }
+
+  await ensureBrowserUseClientFallbackPatch()
+  await mkdir(BROWSER_USE_SOCKET_DIR, { recursive: true })
+  const socketPath = join(BROWSER_USE_SOCKET_DIR, `codexui-${process.pid}-${normalizedSessionId}.sock`)
+  await rm(socketPath, { force: true })
+
+  const backend: BrowserUseBackendRecord = {
+    server: createServer((socket) => handleConnection(backend, socket)),
+    socketPath,
+    browserPromise: launchBrowser(),
+    tabs: new Map(),
+    nextTabId: 1,
+    sessionId: normalizedSessionId,
+  }
+  browserUseBackends.set(normalizedSessionId, backend)
+
+  await new Promise<void>((resolve, reject) => {
+    const onError = (error: Error) => {
+      backend.server.off('listening', onListening)
+      reject(error)
+    }
+    const onListening = () => {
+      backend.server.off('error', onError)
+      resolve()
+    }
+    backend.server.once('error', onError)
+    backend.server.once('listening', onListening)
+    backend.server.listen(socketPath)
+  })
+}
+
+async function ensureBrowserUseClientFallbackPatch(): Promise<void> {
+  browserUseClientPatchPromise ??= (async () => {
+    const source = await readFile(BROWSER_USE_CLIENT_PATH, 'utf8')
+    if (source.includes(BROWSER_USE_CODEXUI_CREATE_SOURCE)) {
+      return
+    }
+    if (!source.includes(BROWSER_USE_NATIVE_CREATE_SOURCE)) {
+      throw new Error('Browser Use client transport shape changed; cannot install codexui fallback.')
+    }
+    await writeFile(
+      BROWSER_USE_CLIENT_PATH,
+      source.replace(BROWSER_USE_NATIVE_CREATE_SOURCE, BROWSER_USE_CODEXUI_CREATE_SOURCE),
+    )
+  })()
+  await browserUseClientPatchPromise
+}
Evidence
The backend init always calls ensureBrowserUseClientFallbackPatch, which unconditionally
reads/writes a fixed path under /Users/igor; the RPC middleware awaits backend init during
turn/start and thread/start, so any ENOENT/EACCES aborts the request path.

src/server/browserUseBackend.ts[69-133]
src/server/codexAppServerBridge.ts[5275-5294]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureBrowserUseClientFallbackPatch()` reads/writes a Browser Use client script using a hardcoded absolute path (`/Users/igor/...`). On other machines (or when the plugin isn’t installed), this throws and breaks `turn/start` / `thread/start` RPC calls.
### Issue Context
Backend initialization is awaited inside the `/codex-api/rpc` handler, so any exception prevents the downstream app-server RPC from being executed.
### Fix Focus Areas
- src/server/browserUseBackend.ts[69-133]
- src/server/codexAppServerBridge.ts[5275-5294]
### What to change
- Replace the hardcoded `BROWSER_USE_CLIENT_PATH` with a path resolved from Codex home/plugin config (or an explicit env var), and only attempt patching when the file exists.
- Make patching best-effort (catch ENOENT/EACCES and disable the fallback) so chats can proceed even without Browser Use.
- Avoid in-place edits when possible (write to a temp file + atomic rename, or prefer a runtime configuration hook over source rewriting).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unconditional backend init on start🐞 Bug ☼ Reliability
Description
The bridge eagerly awaits ensureBrowserUseBackendForSession on every turn/start and thread/start
request regardless of OS/plugin availability, so Browser Use setup failures or slow Playwright
launch can block core chat operations.
Code

src/server/codexAppServerBridge.ts[R5275-5294]

+        if (body.method === 'turn/start') {
+          const params = asRecord(body.params)
+          const threadId = typeof params?.threadId === 'string' ? params.threadId : ''
+          if (threadId) {
+            await ensureBrowserUseBackendForSession(threadId)
+          }
+        }
+
     const rpcResult = await appServer.rpc(body.method, body.params ?? null)
     const trimmedResult = trimThreadTurnsInRpcResult(body.method, rpcResult)
     const result = await sanitizeThreadTurnsInlinePayloads(body.method, trimmedResult)
+        if (body.method === 'thread/start') {
+          const rpcRecord = asRecord(result)
+          const rpcThread = asRecord(rpcRecord?.thread)
+          const threadId = typeof rpcThread?.id === 'string' ? rpcThread.id : ''
+          if (threadId) {
+            await ensureBrowserUseBackendForSession(threadId)
+          }
+        }
Evidence
The RPC handler triggers backend startup for both turn/start and thread/start;
ensureBrowserUseBackendForSession has no platform/feature gating and performs filesystem patching
plus browser/server startup before allowing the RPC to proceed.

src/server/codexAppServerBridge.ts[5275-5294]
src/server/browserUseBackend.ts[82-116]
src/server/browserUseBackend.ts[146-152]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `/codex-api/rpc` handler blocks `turn/start` and `thread/start` on Browser Use backend startup. This can break normal chats when Browser Use isn’t present/supported and can add significant latency due to Playwright launch.
### Issue Context
Backend startup includes patching plugin code, creating a unix socket server, and launching Playwright.
### Fix Focus Areas
- src/server/codexAppServerBridge.ts[5275-5294]
- src/server/browserUseBackend.ts[82-116]
- src/server/browserUseBackend.ts[146-152]
### What to change
- Add explicit platform/feature gating (e.g., only on darwin, only when Browser Use is enabled/detected).
- Wrap backend init in a best-effort try/catch so RPC can proceed when Browser Use initialization fails.
- Consider kicking backend init off asynchronously (don’t await) and/or only initializing when Browser Use is actually invoked, not on every `turn/start`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Malformed frames can crash🐞 Bug ⛨ Security
Description
parseFramedMessages calls JSON.parse on untrusted socket data without try/catch and without a
maximum frame size, allowing a client to crash the process or cause unbounded memory growth via a
huge advertised length.
Code

src/server/browserUseBackend.ts[R168-207]

+  socket.on('data', (chunk) => {
+    client.pendingData = Buffer.concat([client.pendingData, chunk])
+    const parsed = parseFramedMessages(client.pendingData)
+    client.pendingData = parsed.remainingData
+    for (const message of parsed.messages) {
+      void handleMessage(client, message)
+    }
+  })
+}
+
+function authorizeSocketPeer(socket: Socket): void {
+  try {
+    const fd = (socket as Socket & { _handle?: { fd?: number } })._handle?.fd
+    if (typeof fd !== 'number') {
+      return
+    }
+    const nativeModule = require(CODEX_BROWSER_USE_PEER_AUTHORIZATION) as {
+      authorizeSocketPeer?: (fd: number, allowUnsignedPeer: boolean) => unknown
+    }
+    nativeModule.authorizeSocketPeer?.(fd, false)
+  } catch {
+    socket.destroy()
+  }
+}
+
+function parseFramedMessages(data: Buffer): { messages: JsonRpcMessage[], remainingData: Buffer } {
+  const messages: JsonRpcMessage[] = []
+  let offset = 0
+  while (data.length - offset >= 4) {
+    const size = data.readUInt32LE(offset)
+    const end = offset + 4 + size
+    if (data.length < end) {
+      break
+    }
+    const text = data.subarray(offset + 4, end).toString('utf8')
+    messages.push(JSON.parse(text) as JsonRpcMessage)
+    offset = end
+  }
+  return { messages, remainingData: data.subarray(offset) }
+}
Evidence
Socket 'data' events call parseFramedMessages, which immediately JSON.parse()s each framed message;
any invalid JSON throws out of the event handler. The frame length is trusted, so the buffer can
grow indefinitely when a large size is advertised but not fully delivered.

src/server/browserUseBackend.ts[168-176]
src/server/browserUseBackend.ts[193-207]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Browser Use unix socket server can be crashed or DoS’ed because framed message parsing:
- trusts the 32-bit length field without bounds
- calls `JSON.parse()` without handling parse errors
### Issue Context
This code runs inside a socket `data` handler; thrown exceptions can terminate the Node process.
### Fix Focus Areas
- src/server/browserUseBackend.ts[168-176]
- src/server/browserUseBackend.ts[193-207]
### What to change
- Add a maximum frame size (e.g., 1–10MB) and destroy the socket if exceeded.
- Wrap `JSON.parse` in try/catch; on parse failure, destroy the socket (and/or ignore the frame) rather than throwing.
- Consider adding basic JSON-RPC shape validation before dispatching to handlers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. playwright loaded via import()📘 Rule violation ⚙ Maintainability
Description
The new Browser Use backend loads Playwright using a dynamic ESM import('playwright') rather than
the repo’s default CommonJS require('playwright'). This may break compatibility with environments
expecting CJS-style Playwright loading unless there is an explicit ESM requirement.
Code

src/server/browserUseBackend.ts[R146-151]

+async function launchBrowser(): Promise<PlaywrightBrowser> {
+  const dynamicImport = new Function('specifier', 'return import(specifier)') as (specifier: string) => Promise<{
+    chromium: { launch(options?: Record<string, unknown>): Promise<PlaywrightBrowser> }
+  }>
+  const { chromium } = await dynamicImport('playwright')
+  return await chromium.launch({ headless: false })
Evidence
PR Compliance ID 3 requires Playwright automation code to default to CommonJS
require('playwright') unless ESM is explicitly required. The added launchBrowser() uses a
dynamic ESM import to load playwright.

AGENTS.md
src/server/browserUseBackend.ts[146-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/server/browserUseBackend.ts` dynamically imports Playwright via `import('playwright')`, but the compliance checklist requires defaulting to CommonJS `require('playwright')` unless ESM is explicitly required.
## Issue Context
This code already uses `createRequire(import.meta.url)`, so it can likely load Playwright via `require('playwright')` while staying compatible with ESM modules.
## Fix Focus Areas
- src/server/browserUseBackend.ts[146-151]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Unsafe sessionId socket path🐞 Bug ⛨ Security
Description
The unix socket path embeds the raw session/thread id, so path separators or long values can escape
the intended directory via path.join normalization or exceed unix socket path limits, causing listen
failures or unexpected socket locations.
Code

src/server/browserUseBackend.ts[R82-92]

+export async function ensureBrowserUseBackendForSession(sessionId: string): Promise<void> {
+  const normalizedSessionId = sessionId.trim()
+  if (!normalizedSessionId || browserUseBackends.has(normalizedSessionId)) {
+    return
+  }
+
+  await ensureBrowserUseClientFallbackPatch()
+  await mkdir(BROWSER_USE_SOCKET_DIR, { recursive: true })
+  const socketPath = join(BROWSER_USE_SOCKET_DIR, `codexui-${process.pid}-${normalizedSessionId}.sock`)
+  await rm(socketPath, { force: true })
+
Evidence
Only trim() is applied to sessionId before using it as part of a filesystem path; path.join will
normalize '..' segments and very long ids can exceed AF_UNIX path length limits and break
server.listen.

src/server/browserUseBackend.ts[82-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The backend constructs a socket filename directly from `sessionId`. This can:
- create paths outside the intended socket directory (via `..` / path separators)
- exceed unix socket path length limits
### Issue Context
Socket creation happens during RPC handling; failures will break the chat flow.
### Fix Focus Areas
- src/server/browserUseBackend.ts[82-92]
### What to change
- Sanitize `sessionId` (e.g., allow only `[a-zA-Z0-9_-]`), and/or derive the filename from a hash of the sessionId.
- Enforce a maximum length.
- Optionally verify the final path is within `BROWSER_USE_SOCKET_DIR` after normalization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Backends not disposed🐞 Bug ☼ Reliability
Description
The PR introduces long-lived per-session servers and Playwright browsers, but the bridge dispose
path does not call closeBrowserUseBackends, leaking browser processes/resources and leaving stale
socket files across restarts.
Code

src/server/browserUseBackend.ts[R135-144]

+export async function closeBrowserUseBackends(): Promise<void> {
+  const backends = Array.from(browserUseBackends.values())
+  browserUseBackends.clear()
+  await Promise.allSettled(backends.map(async (backend) => {
+    await new Promise<void>((resolve) => backend.server.close(() => resolve()))
+    await rm(backend.socketPath, { force: true })
+    const browser = await backend.browserPromise.catch(() => null)
+    await browser?.close()
+  }))
+}
Evidence
A cleanup function exists, but middleware.dispose currently only disposes
terminalManager/backendQueueProcessor/appServer; there is no shutdown hook for the new Browser Use
backends in the bridge lifecycle.

src/server/browserUseBackend.ts[135-144]
src/server/codexAppServerBridge.ts[6590-6596]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Browser Use backends (net server + Playwright browser) are created per session but not closed when codexui shuts down/disposes the bridge.
### Issue Context
`httpServer.createServer()` delegates cleanup to `bridge.dispose()`, so failing to close these resources can leak processes and leave stale socket files.
### Fix Focus Areas
- src/server/browserUseBackend.ts[135-144]
- src/server/codexAppServerBridge.ts[6590-6596]
### What to change
- Import and call `closeBrowserUseBackends()` from `middleware.dispose()`.
- Optionally add process shutdown hooks (SIGINT/SIGTERM) in the server entrypoint to ensure cleanup on abrupt termination.
- Consider per-thread cleanup when a thread/session is deleted to avoid unbounded growth.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/server/browserUseBackend.ts
Comment thread src/server/codexAppServerBridge.ts
Comment thread src/server/browserUseBackend.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants