From 8a17410f9642911d3b74357d6bbfd16aecddad4e Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Thu, 16 Apr 2026 18:48:28 -0400 Subject: [PATCH] fix(browser): address platform and port review --- docs/browser-automation.md | 10 +++++ src/commands/browser-command.ts | 12 ++---- src/utils/browser/browser-settings.ts | 3 ++ src/utils/browser/browser-status.ts | 13 ++---- src/utils/browser/platform.ts | 9 ++++ .../unit/utils/browser/browser-status.test.ts | 41 +++++++++++++++++++ tests/unit/utils/browser/platform.test.ts | 17 ++++++++ 7 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 src/utils/browser/platform.ts create mode 100644 tests/unit/utils/browser/platform.test.ts diff --git a/docs/browser-automation.md b/docs/browser-automation.md index 87be8aaf..27ba99a6 100644 --- a/docs/browser-automation.md +++ b/docs/browser-automation.md @@ -90,6 +90,16 @@ CCS still supports environment-variable overrides for backward compatibility. If an override is active, Browser status surfaces should report that the current session is being managed externally by environment variables. +Override precedence is: + +1. `CCS_BROWSER_USER_DATA_DIR` +2. `CCS_BROWSER_PROFILE_DIR` +3. the persisted `browser.claude.user_data_dir` config value + +Config-backed Browser Attach always passes an explicit DevTools port to the runtime, even when the +effective value is the default `9222`. Metadata-based port discovery is preserved only for the +legacy `CCS_BROWSER_PROFILE_DIR` flow when `CCS_BROWSER_DEVTOOLS_PORT` is not set. + ## Managed Runtime Files - `~/.claude.json` -> CCS manages `mcpServers.ccs-browser` for Claude Browser Attach diff --git a/src/commands/browser-command.ts b/src/commands/browser-command.ts index d42a0654..d9c4ccf6 100644 --- a/src/commands/browser-command.ts +++ b/src/commands/browser-command.ts @@ -1,14 +1,9 @@ import { getBrowserStatus, type BrowserStatusPayload } from '../utils/browser'; +import { getNodePlatformKey } from '../utils/browser/platform'; import { color, dim, header, initUI, subheader } from '../utils/ui'; type HelpWriter = (line: string) => void; -function currentPlatform(): 'darwin' | 'linux' | 'win32' { - if (process.platform === 'darwin') return 'darwin'; - if (process.platform === 'win32') return 'win32'; - return 'linux'; -} - function summarizeBrowserHealth(status: BrowserStatusPayload): { label: 'ready' | 'partial' | 'action required'; exitCode: 0 | 1; @@ -63,9 +58,8 @@ function writeClaudeStatus( writeLine(` Detail: ${status.detail}`); writeLine(` Next step: ${status.nextStep}`); if (includeLaunchGuidance && status.enabled && status.state !== 'ready') { - writeLine( - ` Launch command (${currentPlatform()}): ${status.launchCommands[currentPlatform()]}` - ); + const platform = getNodePlatformKey(); + writeLine(` Launch command (${platform}): ${status.launchCommands[platform]}`); } writeLine(''); } diff --git a/src/utils/browser/browser-settings.ts b/src/utils/browser/browser-settings.ts index 2e0691e2..e1a9f6fe 100644 --- a/src/utils/browser/browser-settings.ts +++ b/src/utils/browser/browser-settings.ts @@ -74,6 +74,9 @@ export function getEffectiveClaudeBrowserAttachConfig( overrideActive: false, userDataDir: configUserDataDir, devtoolsPort: configPort, + // Config-backed browser attach always keeps an explicit port so launches + // stay aligned with Settings > Browser, even when the effective value is + // the default 9222. hasExplicitDevtoolsPort: true, }; } diff --git a/src/utils/browser/browser-status.ts b/src/utils/browser/browser-status.ts index 4af83b20..0ea469d2 100644 --- a/src/utils/browser/browser-status.ts +++ b/src/utils/browser/browser-status.ts @@ -2,6 +2,7 @@ import { getBrowserConfig } from '../../config/unified-config-loader'; import { getCodexBinaryInfo } from '../../targets/codex-detector'; import { type BrowserRuntimeEnv, resolveBrowserRuntimeEnv } from './chrome-reuse'; import { getBrowserMcpServerName, getBrowserMcpServerPath } from './mcp-installer'; +import { getNodePlatformKey } from './platform'; import { getEffectiveClaudeBrowserAttachConfig, getRecommendedBrowserUserDataDir, @@ -107,7 +108,7 @@ async function buildClaudeBrowserStatus( state: 'path_missing', title: 'Claude Browser Attach path is missing.', detail: message, - nextStep: `Create or choose a Chrome user-data directory, then launch Chrome with attach mode enabled. Example: ${launchCommands[platformKey()]}`, + nextStep: `Create or choose a Chrome user-data directory, then launch Chrome with attach mode enabled. Example: ${launchCommands[getNodePlatformKey()]}`, }; } @@ -117,7 +118,7 @@ async function buildClaudeBrowserStatus( state: 'browser_not_running', title: 'Claude Browser Attach could not find a running browser session.', detail: message, - nextStep: `Start Chrome with remote debugging and the configured user-data dir. Example: ${launchCommands[platformKey()]}`, + nextStep: `Start Chrome with remote debugging and the configured user-data dir. Example: ${launchCommands[getNodePlatformKey()]}`, }; } @@ -126,7 +127,7 @@ async function buildClaudeBrowserStatus( state: 'endpoint_unreachable', title: 'Claude Browser Attach could not reach the DevTools endpoint.', detail: message, - nextStep: `Restart the attach browser session or confirm the configured port. Example: ${launchCommands[platformKey()]}`, + nextStep: `Restart the attach browser session or confirm the configured port. Example: ${launchCommands[getNodePlatformKey()]}`, }; } } @@ -185,9 +186,3 @@ function buildLaunchCommands(userDataDir: string, devtoolsPort: number): Browser win32: `chrome.exe --remote-debugging-port=${devtoolsPort} --user-data-dir=${quotedPath}`, }; } - -function platformKey(): keyof BrowserLaunchCommands { - if (process.platform === 'darwin') return 'darwin'; - if (process.platform === 'win32') return 'win32'; - return 'linux'; -} diff --git a/src/utils/browser/platform.ts b/src/utils/browser/platform.ts new file mode 100644 index 00000000..9f747258 --- /dev/null +++ b/src/utils/browser/platform.ts @@ -0,0 +1,9 @@ +export type BrowserPlatformKey = 'darwin' | 'linux' | 'win32'; + +export function getNodePlatformKey( + platform: NodeJS.Platform = process.platform +): BrowserPlatformKey { + if (platform === 'darwin') return 'darwin'; + if (platform === 'win32') return 'win32'; + return 'linux'; +} diff --git a/tests/unit/utils/browser/browser-status.test.ts b/tests/unit/utils/browser/browser-status.test.ts index 1e047e5d..a15cd461 100644 --- a/tests/unit/utils/browser/browser-status.test.ts +++ b/tests/unit/utils/browser/browser-status.test.ts @@ -199,4 +199,45 @@ describe('browser status', () => { codexSpy.mockRestore(); } }); + + it('always forwards an explicit port for config-backed browser attach sessions', async () => { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: true, + user_data_dir: '/tmp/config-browser', + devtools_port: 9222, + }, + codex: { + enabled: true, + }, + }; + }); + + const runtimeSpy = spyOn(chromeReuse, 'resolveBrowserRuntimeEnv').mockResolvedValue({ + CCS_BROWSER_USER_DATA_DIR: '/tmp/config-browser', + CCS_BROWSER_DEVTOOLS_HOST: '127.0.0.1', + CCS_BROWSER_DEVTOOLS_PORT: '9222', + CCS_BROWSER_DEVTOOLS_HTTP_URL: 'http://127.0.0.1:9222', + CCS_BROWSER_DEVTOOLS_WS_URL: 'ws://127.0.0.1/devtools/browser/config', + }); + const codexSpy = spyOn(codexDetector, 'getCodexBinaryInfo').mockReturnValue({ + path: '/usr/local/bin/codex', + needsShell: false, + version: 'codex-cli 0.120.0', + features: ['config-overrides'], + }); + + try { + await getBrowserStatus(); + + expect(runtimeSpy.mock.calls[0]?.[0]).toEqual({ + profileDir: '/tmp/config-browser', + devtoolsPort: '9222', + }); + } finally { + runtimeSpy.mockRestore(); + codexSpy.mockRestore(); + } + }); }); diff --git a/tests/unit/utils/browser/platform.test.ts b/tests/unit/utils/browser/platform.test.ts new file mode 100644 index 00000000..2c6ab7a0 --- /dev/null +++ b/tests/unit/utils/browser/platform.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'bun:test'; +import { getNodePlatformKey } from '../../../../src/utils/browser/platform'; + +describe('browser platform helper', () => { + it('maps darwin explicitly', () => { + expect(getNodePlatformKey('darwin')).toBe('darwin'); + }); + + it('maps win32 explicitly', () => { + expect(getNodePlatformKey('win32')).toBe('win32'); + }); + + it('falls back to linux for other node platforms', () => { + expect(getNodePlatformKey('linux')).toBe('linux'); + expect(getNodePlatformKey('freebsd')).toBe('linux'); + }); +});