From ab6c59ba354361d011e3004b9706a5ebe05ebf76 Mon Sep 17 00:00:00 2001 From: "Kai (Tam Nhu) Tran" <61256810+kaitranntt@users.noreply.github.com> Date: Mon, 15 Jun 2026 23:14:55 -0400 Subject: [PATCH] fix(browser): preserve profile-bound DevTools discovery (#1541) Preserves the profile-bound DevTools port/discovery so browser launches resolve the correct per-profile endpoint. --- .../__tests__/browser-launch-setup.test.ts | 5 ++ src/cliproxy/executor/browser-launch-setup.ts | 13 ++++- src/config/config-loader-facade.ts | 1 + src/config/loader/config-getters.ts | 23 ++++++++ src/config/unified-config-loader.ts | 1 + src/dispatcher/profile-resolver.ts | 11 +++- src/utils/browser/browser-settings.ts | 5 +- src/utils/browser/browser-setup.ts | 10 +++- src/utils/browser/browser-status.ts | 13 ++++- .../unit/utils/browser/browser-status.test.ts | 57 +++++++++++++++++-- 10 files changed, 122 insertions(+), 17 deletions(-) diff --git a/src/cliproxy/executor/__tests__/browser-launch-setup.test.ts b/src/cliproxy/executor/__tests__/browser-launch-setup.test.ts index befcb19b..e6c6fdd7 100644 --- a/src/cliproxy/executor/__tests__/browser-launch-setup.test.ts +++ b/src/cliproxy/executor/__tests__/browser-launch-setup.test.ts @@ -42,6 +42,7 @@ describe('resolveBrowserLaunchFlags — no browser flags', () => { })); mock.module('../../../config/config-loader-facade', () => ({ getBrowserConfig: () => makeBrowserConfig(false), + hasExplicitClaudeBrowserDevtoolsPort: () => false, loadOrCreateUnifiedConfig: () => ({}), getThinkingConfig: () => ({}), })); @@ -73,6 +74,7 @@ describe('resolveBrowserLaunchFlags — with browser-launch override', () => { })); mock.module('../../../config/config-loader-facade', () => ({ getBrowserConfig: () => makeBrowserConfig(true, 'auto'), + hasExplicitClaudeBrowserDevtoolsPort: () => false, loadOrCreateUnifiedConfig: () => ({}), getThinkingConfig: () => ({}), })); @@ -113,6 +115,7 @@ describe('resolveBrowserLaunchFlags — blocked override warning', () => { })); mock.module('../../../config/config-loader-facade', () => ({ getBrowserConfig: () => makeBrowserConfig(false, 'never'), + hasExplicitClaudeBrowserDevtoolsPort: () => false, loadOrCreateUnifiedConfig: () => ({}), getThinkingConfig: () => ({}), })); @@ -143,6 +146,7 @@ describe('resolveBrowserRuntime — attach disabled', () => { })); mock.module('../../../config/config-loader-facade', () => ({ getBrowserConfig: () => makeBrowserConfig(false), + hasExplicitClaudeBrowserDevtoolsPort: () => false, loadOrCreateUnifiedConfig: () => ({}), getThinkingConfig: () => ({}), })); @@ -173,6 +177,7 @@ describe('resolveBrowserRuntime — active runtime env', () => { })); mock.module('../../../config/config-loader-facade', () => ({ getBrowserConfig: () => makeBrowserConfig(true, 'always'), + hasExplicitClaudeBrowserDevtoolsPort: () => false, loadOrCreateUnifiedConfig: () => ({}), getThinkingConfig: () => ({}), })); diff --git a/src/cliproxy/executor/browser-launch-setup.ts b/src/cliproxy/executor/browser-launch-setup.ts index c33f3bdc..3dd787b6 100644 --- a/src/cliproxy/executor/browser-launch-setup.ts +++ b/src/cliproxy/executor/browser-launch-setup.ts @@ -20,7 +20,10 @@ import { resolveOptionalBrowserAttachRuntime, syncBrowserMcpToConfigDir, } from '../../utils/browser'; -import { getBrowserConfig } from '../../config/config-loader-facade'; +import { + getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, +} from '../../config/config-loader-facade'; export interface BrowserLaunchSetupResult { /** CLI override flag if --browser-launch / --no-browser-launch was passed */ @@ -57,7 +60,9 @@ export function resolveBrowserLaunchFlags(argsWithoutProxy: string[]): { } const browserConfig = getBrowserConfig(); - const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig); + const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, { + hasExplicitDevtoolsPort: hasExplicitClaudeBrowserDevtoolsPort(), + }); const claudeBrowserExposure = resolveBrowserExposure( { enabled: browserAttachConfig.enabled, @@ -85,7 +90,9 @@ export async function resolveBrowserRuntime( inheritedClaudeConfigDir: string | undefined ): Promise> { const browserConfig = getBrowserConfig(); - const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig); + const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, { + hasExplicitDevtoolsPort: hasExplicitClaudeBrowserDevtoolsPort(), + }); const claudeBrowserExposure = resolveBrowserExposure( { enabled: browserAttachConfig.enabled, diff --git a/src/config/config-loader-facade.ts b/src/config/config-loader-facade.ts index 5c187404..c97206c9 100644 --- a/src/config/config-loader-facade.ts +++ b/src/config/config-loader-facade.ts @@ -41,6 +41,7 @@ export { isDashboardAuthEnabled, getDashboardAuthConfig, getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, getImageAnalysisConfig, getLoggingConfig, getCursorConfig, diff --git a/src/config/loader/config-getters.ts b/src/config/loader/config-getters.ts index a37779a3..f8e77492 100644 --- a/src/config/loader/config-getters.ts +++ b/src/config/loader/config-getters.ts @@ -53,6 +53,13 @@ function getConfig(): import('../unified-config-types').UnifiedConfig { return loader.loadOrCreateUnifiedConfig(); } +function getPersistedConfig(): import('../unified-config-types').UnifiedConfig | null { + const loader = require('../unified-config-loader') as { + loadUnifiedConfig: () => import('../unified-config-types').UnifiedConfig | null; + }; + return loader.loadUnifiedConfig(); +} + // --------------------------------------------------------------------------- // GeminiWebSearchInfo interface // --------------------------------------------------------------------------- @@ -307,6 +314,22 @@ export function getBrowserConfig(): BrowserConfig { return canonicalizeBrowserConfig(config.browser); } +/** + * Return whether the persisted browser config explicitly defines + * claude.devtools_port. Canonicalized BrowserConfig values always contain a + * default port, so config-backed browser attach callers must use this raw + * persisted shape to decide whether the port should bypass profile discovery. + */ +export function hasExplicitClaudeBrowserDevtoolsPort(): boolean { + const claude = getPersistedConfig()?.browser?.claude; + if (!claude || !Object.prototype.hasOwnProperty.call(claude, 'devtools_port')) { + return false; + } + + const port = claude.devtools_port; + return Number.isFinite(port) && Math.floor(port as number) === port && port >= 1 && port <= 65535; +} + /** * Get image_analysis configuration. * Returns defaults if not configured. diff --git a/src/config/unified-config-loader.ts b/src/config/unified-config-loader.ts index c355d9c3..59fe94ea 100644 --- a/src/config/unified-config-loader.ts +++ b/src/config/unified-config-loader.ts @@ -102,6 +102,7 @@ export { isDashboardAuthEnabled, getDashboardAuthConfig, getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, getImageAnalysisConfig, getLoggingConfig, getCursorConfig, diff --git a/src/dispatcher/profile-resolver.ts b/src/dispatcher/profile-resolver.ts index 6a1dfb35..e1752221 100644 --- a/src/dispatcher/profile-resolver.ts +++ b/src/dispatcher/profile-resolver.ts @@ -18,7 +18,10 @@ import { loadSettings } from '../config/config-loader-facade'; import { expandPath } from '../utils/helpers'; import { fail, info, warn } from '../utils/ui'; import { ErrorManager } from '../utils/error-manager'; -import { getBrowserConfig } from '../config/config-loader-facade'; +import { + getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, +} from '../config/config-loader-facade'; import { getEffectiveClaudeBrowserAttachConfig, resolveBrowserExposure, @@ -291,7 +294,11 @@ export async function resolveProfileAndTarget( const targetBinaryInfo: TargetBinaryInfo | null = targetAdapter?.detectBinary() ?? null; const browserConfig = getBrowserConfig(); const claudeAttachConfig = - resolvedTarget === 'claude' ? getEffectiveClaudeBrowserAttachConfig(browserConfig) : undefined; + resolvedTarget === 'claude' + ? getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, { + hasExplicitDevtoolsPort: hasExplicitClaudeBrowserDevtoolsPort(), + }) + : undefined; const codexRuntimeConfigOverrides = resolveCodexRuntimeConfigOverrides( resolvedTarget, browserLaunchOverride diff --git a/src/utils/browser/browser-settings.ts b/src/utils/browser/browser-settings.ts index edc6b383..0f611ac9 100644 --- a/src/utils/browser/browser-settings.ts +++ b/src/utils/browser/browser-settings.ts @@ -225,12 +225,13 @@ export function getBrowserAttachOverride(env: NodeJS.ProcessEnv = process.env): export function getEffectiveClaudeBrowserAttachConfig( config: BrowserConfig, - env: NodeJS.ProcessEnv = process.env + env: NodeJS.ProcessEnv = process.env, + options: { hasExplicitDevtoolsPort?: boolean } = {} ): EffectiveClaudeBrowserAttachConfig { const override = getBrowserAttachOverride(env); const configUserDataDir = resolveBrowserUserDataDir(config.claude.user_data_dir) ?? getRecommendedBrowserUserDataDir(); - const configHasExplicitPort = config.claude.devtools_port !== undefined; + const configHasExplicitPort = options.hasExplicitDevtoolsPort ?? true; const configPort = normalizeDevtoolsPort(config.claude.devtools_port); const configEvalMode = config.claude.eval_mode ?? 'readonly'; const envEvalMode = parseBrowserEvalMode(env.CCS_BROWSER_EVAL_MODE); diff --git a/src/utils/browser/browser-setup.ts b/src/utils/browser/browser-setup.ts index b95345fc..afa99307 100644 --- a/src/utils/browser/browser-setup.ts +++ b/src/utils/browser/browser-setup.ts @@ -8,7 +8,11 @@ import { getRecommendedBrowserUserDataDir, isManagedClaudeBrowserAttachConfig, } from './browser-settings'; -import { getBrowserConfig, mutateConfig } from '../../config/config-loader-facade'; +import { + getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, + mutateConfig, +} from '../../config/config-loader-facade'; export interface BrowserSetupResult { configUpdated: boolean; @@ -41,7 +45,9 @@ export async function runBrowserSetup( const initialConfig = deps.getBrowserConfig(); const configUpdated = persistBrowserSetupConfig(deps, initialConfig); const persistedConfig = deps.getBrowserConfig(); - const effectiveConfig = getEffectiveClaudeBrowserAttachConfig(persistedConfig); + const effectiveConfig = getEffectiveClaudeBrowserAttachConfig(persistedConfig, process.env, { + hasExplicitDevtoolsPort: hasExplicitClaudeBrowserDevtoolsPort(), + }); const createdUserDataDir = isManagedClaudeBrowserAttachConfig(effectiveConfig) ? ensureManagedBrowserUserDataDir(effectiveConfig).createdProfileDir : false; diff --git a/src/utils/browser/browser-status.ts b/src/utils/browser/browser-status.ts index 48732454..69c94d31 100644 --- a/src/utils/browser/browser-status.ts +++ b/src/utils/browser/browser-status.ts @@ -19,7 +19,11 @@ import { getEffectiveClaudeBrowserAttachConfig, getRecommendedBrowserUserDataDir, } from './browser-settings'; -import { getBrowserConfig, loadUnifiedConfig } from '../../config/config-loader-facade'; +import { + getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, + loadUnifiedConfig, +} from '../../config/config-loader-facade'; export interface ClaudeBrowserStatus { enabled: boolean; @@ -124,9 +128,12 @@ export function getUserFacingBrowserConfig(): BrowserConfig { } async function buildClaudeBrowserStatus( - browserConfig = getUserFacingBrowserConfig() + browserConfig = getUserFacingBrowserConfig(), + hasExplicitDevtoolsPort = hasExplicitClaudeBrowserDevtoolsPort() ): Promise { - const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig); + const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, { + hasExplicitDevtoolsPort, + }); const launchCommands = buildBrowserLaunchCommands(effective.userDataDir, effective.devtoolsPort); const base: Omit = { enabled: effective.enabled, diff --git a/tests/unit/utils/browser/browser-status.test.ts b/tests/unit/utils/browser/browser-status.test.ts index 6033eecf..89d953ef 100644 --- a/tests/unit/utils/browser/browser-status.test.ts +++ b/tests/unit/utils/browser/browser-status.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { getBrowserConfig, + hasExplicitClaudeBrowserDevtoolsPort, mutateUnifiedConfig, saveUnifiedConfig, } from '../../../../src/config/unified-config-loader'; @@ -168,11 +169,11 @@ describe('browser status', () => { }; }); - const runtimeSpy = spyOn(chromeReuse, 'resolveBrowserRuntimeEnv').mockRejectedValue( - new Error( + const runtimeSpy = spyOn(chromeReuse, 'resolveBrowserRuntimeEnv').mockImplementation(() => { + throw new Error( `Chrome reuse metadata not found: ${join(tempHome, '.ccs', 'browser', 'chrome-user-data', 'DevToolsActivePort')}` - ) - ); + ); + }); const codexSpy = spyOn(codexDetector, 'getCodexBinaryInfo').mockReturnValue({ path: '/usr/local/bin/codex', needsShell: false, @@ -422,7 +423,51 @@ describe('browser status', () => { } }); - it('always forwards an explicit port for config-backed browser attach sessions', async () => { + it('preserves profile-bound port discovery when config omits DevTools port', async () => { + const config = createEmptyUnifiedConfig(); + config.browser = { + claude: { + enabled: true, + policy: 'auto', + user_data_dir: '/tmp/config-browser', + } as typeof config.browser.claude, + codex: { + enabled: true, + policy: 'auto', + } as typeof config.browser.codex, + }; + saveUnifiedConfig(config); + + expect(hasExplicitClaudeBrowserDevtoolsPort()).toBe(false); + + 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: '9333', + CCS_BROWSER_DEVTOOLS_HTTP_URL: 'http://127.0.0.1:9333', + 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: undefined, + }); + } finally { + runtimeSpy.mockRestore(); + codexSpy.mockRestore(); + } + }); + + it('forwards an explicit port for config-backed browser attach sessions', async () => { mutateUnifiedConfig((config) => { config.browser = { claude: { @@ -438,6 +483,8 @@ describe('browser status', () => { }; }); + expect(hasExplicitClaudeBrowserDevtoolsPort()).toBe(true); + const runtimeSpy = spyOn(chromeReuse, 'resolveBrowserRuntimeEnv').mockResolvedValue({ CCS_BROWSER_USER_DATA_DIR: '/tmp/config-browser', CCS_BROWSER_DEVTOOLS_HOST: '127.0.0.1',