mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-11 03:13:12 +00:00
fix(browser): preserve profile-bound DevTools discovery (#1541)
Preserves the profile-bound DevTools port/discovery so browser launches resolve the correct per-profile endpoint.
This commit is contained in:
1 parent
473aa082f6
commit
ab6c59ba35
10 files changed
+122
-17
No files matched your search
@@ -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: () => ({}),
|
||||
}));
|
||||
|
||||
@@ -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<Pick<BrowserLaunchSetupResult, 'browserRuntimeEnv'>> {
|
||||
const browserConfig = getBrowserConfig();
|
||||
const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig);
|
||||
const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, {
|
||||
hasExplicitDevtoolsPort: hasExplicitClaudeBrowserDevtoolsPort(),
|
||||
});
|
||||
const claudeBrowserExposure = resolveBrowserExposure(
|
||||
{
|
||||
enabled: browserAttachConfig.enabled,
|
||||
|
||||
@@ -41,6 +41,7 @@ export {
|
||||
isDashboardAuthEnabled,
|
||||
getDashboardAuthConfig,
|
||||
getBrowserConfig,
|
||||
hasExplicitClaudeBrowserDevtoolsPort,
|
||||
getImageAnalysisConfig,
|
||||
getLoggingConfig,
|
||||
getCursorConfig,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -102,6 +102,7 @@ export {
|
||||
isDashboardAuthEnabled,
|
||||
getDashboardAuthConfig,
|
||||
getBrowserConfig,
|
||||
hasExplicitClaudeBrowserDevtoolsPort,
|
||||
getImageAnalysisConfig,
|
||||
getLoggingConfig,
|
||||
getCursorConfig,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<ClaudeBrowserStatus> {
|
||||
const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig);
|
||||
const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig, process.env, {
|
||||
hasExplicitDevtoolsPort,
|
||||
});
|
||||
const launchCommands = buildBrowserLaunchCommands(effective.userDataDir, effective.devtoolsPort);
|
||||
const base: Omit<ClaudeBrowserStatus, 'state' | 'title' | 'detail' | 'nextStep'> = {
|
||||
enabled: effective.enabled,
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in new issue
Block a user