From 7d02f55f9fba3fb81c0d1f1bfeb0183809de4d54 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 19 Apr 2026 23:35:21 -0400 Subject: [PATCH 1/3] feat(browser): add explicit runtime policy controls --- docs/browser-automation.md | 37 ++- docs/project-roadmap.md | 3 +- src/ccs.ts | 82 ++++-- src/cliproxy/executor/index.ts | 48 +++- src/commands/browser-command.ts | 234 +++++++++++++++++- src/commands/command-catalog.ts | 4 +- src/commands/completion-backend.ts | 5 +- src/config/unified-config-loader.ts | 7 + src/config/unified-config-types.ts | 8 + src/targets/target-resolver.ts | 16 ++ src/utils/browser/browser-policy.ts | 86 +++++++ src/utils/browser/browser-setup.ts | 2 + src/utils/browser/browser-status.ts | 7 + src/utils/browser/index.ts | 8 + src/web-server/routes/browser-routes.ts | 24 ++ tests/unit/commands/browser-command.test.ts | 51 +++- .../unit/commands/completion-backend.test.ts | 2 +- .../unit/commands/help-command-parity.test.ts | 2 + .../targets/codex-runtime-integration.test.ts | 110 ++++++++ .../default-profile-browser-launch.test.ts | 6 + .../settings-profile-browser-launch.test.ts | 6 + tests/unit/targets/target-resolver.test.ts | 12 +- .../unit/utils/browser/browser-policy.test.ts | 47 ++++ .../unit/utils/browser/browser-status.test.ts | 10 + tests/unit/web-server/browser-routes.test.ts | 28 +++ 25 files changed, 795 insertions(+), 50 deletions(-) create mode 100644 src/utils/browser/browser-policy.ts create mode 100644 tests/unit/utils/browser/browser-policy.test.ts diff --git a/docs/browser-automation.md b/docs/browser-automation.md index 0e0d1766..8e3e72bd 100644 --- a/docs/browser-automation.md +++ b/docs/browser-automation.md @@ -46,6 +46,10 @@ The Browser screen exposes two sections: - enable/disable CCS-managed browser tooling for Codex-target launches - review whether the detected Codex build supports managed browser overrides +Browser policy controls are CLI-first in this release. The dashboard remains the shared setup and +status surface, while `ccs browser policy` is the authoritative place to decide whether browser +tooling is auto-exposed or kept manual by default. + ### Via CLI ```bash @@ -53,10 +57,13 @@ ccs help browser ccs browser setup ccs browser status ccs browser doctor +ccs browser policy +ccs browser policy --all manual ``` Use `ccs browser setup` for the primary one-command setup path. Use `ccs browser status` for -the current state and `ccs browser doctor` for read-only troubleshooting guidance. +the current state, `ccs browser doctor` for read-only troubleshooting guidance, and +`ccs browser policy` to control default browser exposure. ### Via Config File @@ -66,17 +73,45 @@ Edit `~/.ccs/config.yaml`: browser: claude: enabled: false + policy: auto user_data_dir: "~/.ccs/browser/chrome-user-data" devtools_port: 9222 codex: enabled: true + policy: auto ``` Notes: +- `claude.policy` and `codex.policy` accept `auto` or `manual` - `claude.user_data_dir` is a **Chrome user-data directory**, not a display-name browser profile - `claude.devtools_port` is the expected remote debugging port for attach mode - `codex.enabled` controls whether CCS injects browser tooling into Codex-target launches +- `manual` keeps the lane configured but hidden until a launch explicitly opts in with `--browser` + +## Runtime Policy Controls + +CCS now separates **lane enablement** from **default exposure policy**: + +- `enabled: false` + - the lane is off +- `enabled: true` + `policy: auto` + - the lane is exposed automatically on matching launches +- `enabled: true` + `policy: manual` + - the lane stays configured, but CCS keeps browser tooling hidden unless the current launch uses + `--browser` + +One-run launch overrides: + +```bash +ccs browser policy --all manual +ccs glm --browser "inspect the page" +ccs glm --no-browser "summarize the docs" +ccs default --target codex --browser "use the browser tools for this run" +``` + +- `--browser` forces browser tooling on for the current launch when that lane is enabled +- `--no-browser` suppresses browser tooling for the current launch even when policy is `auto` ## Environment Variable Overrides diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index bed0ab31..c8457946 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -1,6 +1,6 @@ # CCS Project Roadmap -Last Updated: 2026-04-18 +Last Updated: 2026-04-19 Forward-looking roadmap documenting current priorities, GitHub issues, and future feature plans. @@ -41,6 +41,7 @@ All major modularization work is complete. The codebase evolved from monolithic ### Recent Fixes +- **2026-04-19**: **#1051** Browser tooling now has an explicit exposure policy instead of only coarse enablement toggles. CCS adds `browser..policy` (`auto` or `manual`) for both Claude Browser Attach and Codex Browser Tools, exposes CLI-first policy controls through `ccs browser policy`, `ccs browser enable`, and `ccs browser disable`, and adds one-run launch overrides `--browser` and `--no-browser` so users can force browser tooling on or off without editing saved config. - **2026-04-19**: **#1049** Browser setup now has a real remediation path instead of status/doctor-only guidance. CCS adds `ccs browser setup` as the primary one-command flow for Claude Browser Attach, shortens managed browser-path output to home-relative display paths where appropriate, and updates browser readiness guidance to point users at setup first while keeping browser doctor read-only by default. - **2026-04-18**: **#1038** Legacy OpenAI-compatible provider writes no longer self-destruct on the next `ccs cliproxy restart`. CCS now preserves AI-provider-managed top-level sections such as `openai-compatibility` during CLIProxy config regeneration, and the legacy `openai-compat` manager now rewrites only its own YAML section instead of dumping the whole file and stripping the generated version header. Regression coverage now proves the legacy helper keeps the generated header intact and that OpenAI-compatible connectors survive regeneration. - **2026-04-16**: **#1030** Browser automation is now a first-class CCS surface instead of an env-only/runtime-only feature. CCS adds `ccs help browser`, `ccs browser status`, and `ccs browser doctor`; a dedicated `Settings -> Browser` dashboard tab for Claude Browser Attach and Codex Browser Tools; a new `browser` section in `~/.ccs/config.yaml`; explicit readiness/next-step messaging for attach-mode Chrome sessions; and Codex UI guidance that marks the managed `ccs_browser` entry as CCS-owned and redirects browser setup away from the generic MCP editor. diff --git a/src/ccs.ts b/src/ccs.ts index 8a5371fa..9bed9be9 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -39,7 +39,10 @@ import { import { appendBrowserToolArgs, ensureBrowserMcpOrThrow, + getBlockedBrowserOverrideWarning, getEffectiveClaudeBrowserAttachConfig, + resolveBrowserExposure, + resolveBrowserLaunchFlagResolution, resolveOptionalBrowserAttachRuntime, syncBrowserMcpToConfigDir, } from './utils/browser'; @@ -83,6 +86,7 @@ import { maybeWarnAboutResumeLaneMismatch } from './auth/resume-lane-warning'; import { createLogger } from './services/logging'; import { buildCodexBrowserMcpOverrides } from './utils/browser-codex-overrides'; import type { ProfileDetectionResult } from './auth/profile-detector'; +import type { BrowserLaunchOverride } from './utils/browser'; // Import target adapter system import { @@ -137,11 +141,21 @@ const CODEX_RUNTIME_REASONING_LEVELS = new Set(['minimal', 'low', 'medium', 'hig const CODEX_NATIVE_PASSTHROUGH_FLAGS = new Set(['--help', '-h', '--version', '-v']); function resolveCodexRuntimeConfigOverrides( - target: ReturnType + target: ReturnType, + browserLaunchOverride: BrowserLaunchOverride | undefined ): string[] { - if (target !== 'codex' || !getBrowserConfig().codex.enabled) { + if (target !== 'codex') { return []; } + + const codexBrowserExposure = resolveBrowserExposure( + getBrowserConfig().codex, + browserLaunchOverride + ); + if (!codexBrowserExposure.exposeForLaunch) { + return []; + } + return buildCodexBrowserMcpOverrides(); } @@ -591,6 +605,17 @@ async function main(): Promise { const detector = new ProfileDetector(); try { + let browserLaunchOverride: BrowserLaunchOverride | undefined; + try { + const browserLaunchFlags = resolveBrowserLaunchFlagResolution(args); + browserLaunchOverride = browserLaunchFlags.override; + args = browserLaunchFlags.argsWithoutFlags; + } catch (error) { + console.error(fail((error as Error).message)); + process.exit(1); + return; + } + // Detect profile (strip --target flags before profile detection) const cleanArgs = stripTargetFlag(args); const { profile, remainingArgs } = detectProfile(cleanArgs); @@ -697,7 +722,38 @@ async function main(): Promise { // For non-claude targets, verify target binary exists once and pass it through. const targetBinaryInfo = targetAdapter?.detectBinary() ?? null; - const codexRuntimeConfigOverrides = resolveCodexRuntimeConfigOverrides(resolvedTarget); + const browserConfig = getBrowserConfig(); + const claudeAttachConfig = + resolvedTarget === 'claude' + ? getEffectiveClaudeBrowserAttachConfig(browserConfig) + : undefined; + const codexRuntimeConfigOverrides = resolveCodexRuntimeConfigOverrides( + resolvedTarget, + browserLaunchOverride + ); + const claudeBrowserExposure = + resolvedTarget === 'claude' + ? resolveBrowserExposure( + { + enabled: claudeAttachConfig?.enabled ?? browserConfig.claude.enabled, + policy: claudeAttachConfig?.overrideActive ? 'auto' : browserConfig.claude.policy, + }, + browserLaunchOverride + ) + : undefined; + const codexBrowserExposure = + resolvedTarget === 'codex' + ? resolveBrowserExposure(browserConfig.codex, browserLaunchOverride) + : undefined; + const blockedBrowserOverrideWarning = + resolvedTarget === 'claude' && claudeBrowserExposure + ? getBlockedBrowserOverrideWarning('Claude Browser Attach', claudeBrowserExposure) + : resolvedTarget === 'codex' && codexBrowserExposure + ? getBlockedBrowserOverrideWarning('Codex Browser Tools', codexBrowserExposure) + : undefined; + if (blockedBrowserOverrideWarning) { + console.error(warn(blockedBrowserOverrideWarning)); + } if (resolvedTarget !== 'claude' && !targetBinaryInfo) { const displayName = targetAdapter?.displayName || resolvedTarget; console.error(fail(`${displayName} CLI not found.`)); @@ -1057,13 +1113,11 @@ async function main(): Promise { // Settings-based profiles (glm, glmt) are third-party providers const imageAnalysisMcpReady = resolvedTarget === 'claude' ? ensureImageAnalysisMcpOrThrow() : true; - const browserAttachConfig = - resolvedTarget === 'claude' - ? getEffectiveClaudeBrowserAttachConfig(getBrowserConfig()) - : undefined; const browserAttachRuntime = - resolvedTarget === 'claude' && browserAttachConfig?.enabled - ? await resolveOptionalBrowserAttachRuntime(browserAttachConfig) + resolvedTarget === 'claude' && + claudeBrowserExposure?.exposeForLaunch && + claudeAttachConfig?.enabled + ? await resolveOptionalBrowserAttachRuntime(claudeAttachConfig) : undefined; const browserRuntimeEnv = browserAttachRuntime?.runtimeEnv; if (browserAttachRuntime?.warning) { @@ -1467,13 +1521,11 @@ async function main(): Promise { CCS_WEBSEARCH_SKIP: '1', CCS_IMAGE_ANALYSIS_SKIP: '1', }; - const browserAttachConfig = - resolvedTarget === 'claude' - ? getEffectiveClaudeBrowserAttachConfig(getBrowserConfig()) - : undefined; const browserAttachRuntime = - resolvedTarget === 'claude' && browserAttachConfig?.enabled - ? await resolveOptionalBrowserAttachRuntime(browserAttachConfig) + resolvedTarget === 'claude' && + claudeBrowserExposure?.exposeForLaunch && + claudeAttachConfig?.enabled + ? await resolveOptionalBrowserAttachRuntime(claudeAttachConfig) : undefined; const browserRuntimeEnv = browserAttachRuntime?.runtimeEnv; if (browserAttachRuntime?.warning) { diff --git a/src/cliproxy/executor/index.ts b/src/cliproxy/executor/index.ts index 3f6d1d11..98d0d32d 100644 --- a/src/cliproxy/executor/index.ts +++ b/src/cliproxy/executor/index.ts @@ -66,7 +66,10 @@ import { import { appendBrowserToolArgs, ensureBrowserMcpOrThrow, + getBlockedBrowserOverrideWarning, getEffectiveClaudeBrowserAttachConfig, + resolveBrowserExposure, + resolveBrowserLaunchFlagResolution, resolveOptionalBrowserAttachRuntime, syncBrowserMcpToConfigDir, } from '../../utils/browser'; @@ -245,6 +248,25 @@ export async function execClaudeWithCLIProxy( } : undefined, }); + const browserLaunchFlags = resolveBrowserLaunchFlagResolution(argsWithoutProxy); + const browserLaunchOverride = browserLaunchFlags.override; + const argsWithoutBrowserFlags = browserLaunchFlags.argsWithoutFlags; + const browserConfig = getBrowserConfig(); + const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig); + const claudeBrowserExposure = resolveBrowserExposure( + { + enabled: browserAttachConfig.enabled, + policy: browserAttachConfig.overrideActive ? 'auto' : browserConfig.claude.policy, + }, + browserLaunchOverride + ); + const blockedBrowserOverrideWarning = getBlockedBrowserOverrideWarning( + 'Claude Browser Attach', + claudeBrowserExposure + ); + if (blockedBrowserOverrideWarning) { + console.error(warn(blockedBrowserOverrideWarning)); + } // Port resolution and validation if (cfg.port && cfg.port !== CLIPROXY_DEFAULT_PORT) { @@ -264,10 +286,10 @@ export async function execClaudeWithCLIProxy( // Setup first-class CCS WebSearch runtime ensureWebSearchMcpOrThrow(); const imageAnalysisMcpReady = ensureImageAnalysisMcpOrThrow(); - const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(getBrowserConfig()); - const browserAttachRuntime = browserAttachConfig.enabled - ? await resolveOptionalBrowserAttachRuntime(browserAttachConfig) - : undefined; + const browserAttachRuntime = + browserAttachConfig.enabled && claudeBrowserExposure.exposeForLaunch + ? await resolveOptionalBrowserAttachRuntime(browserAttachConfig) + : undefined; const browserRuntimeEnv = browserAttachRuntime?.runtimeEnv; if (browserAttachRuntime?.warning) { process.stderr.write(`${warn(browserAttachRuntime.warning)}\n`); @@ -1260,7 +1282,7 @@ export async function execClaudeWithCLIProxy( '--settings', ...PROXY_CLI_FLAGS, ]; - const claudeArgs = argsWithoutProxy.filter((arg, idx) => { + const claudeArgs = argsWithoutBrowserFlags.filter((arg, idx) => { if (ccsFlags.includes(arg)) return false; if (arg.startsWith('--kiro-auth-method=')) return false; if (arg.startsWith('--kiro-idc-start-url=')) return false; @@ -1270,14 +1292,14 @@ export async function execClaudeWithCLIProxy( if (arg.startsWith('--effort=')) return false; if (arg.startsWith('--1m=') || arg.startsWith('--no-1m=')) return false; if ( - argsWithoutProxy[idx - 1] === '--use' || - argsWithoutProxy[idx - 1] === '--nickname' || - argsWithoutProxy[idx - 1] === '--kiro-auth-method' || - argsWithoutProxy[idx - 1] === '--kiro-idc-start-url' || - argsWithoutProxy[idx - 1] === '--kiro-idc-region' || - argsWithoutProxy[idx - 1] === '--kiro-idc-flow' || - argsWithoutProxy[idx - 1] === '--thinking' || - argsWithoutProxy[idx - 1] === '--effort' + argsWithoutBrowserFlags[idx - 1] === '--use' || + argsWithoutBrowserFlags[idx - 1] === '--nickname' || + argsWithoutBrowserFlags[idx - 1] === '--kiro-auth-method' || + argsWithoutBrowserFlags[idx - 1] === '--kiro-idc-start-url' || + argsWithoutBrowserFlags[idx - 1] === '--kiro-idc-region' || + argsWithoutBrowserFlags[idx - 1] === '--kiro-idc-flow' || + argsWithoutBrowserFlags[idx - 1] === '--thinking' || + argsWithoutBrowserFlags[idx - 1] === '--effort' ) return false; return true; diff --git a/src/commands/browser-command.ts b/src/commands/browser-command.ts index 249e65a8..2536c496 100644 --- a/src/commands/browser-command.ts +++ b/src/commands/browser-command.ts @@ -1,9 +1,12 @@ import * as browserUtils from '../utils/browser'; +import { getBrowserConfig, mutateUnifiedConfig } from '../config/unified-config-loader'; +import type { BrowserToolPolicy } from '../config/unified-config-types'; import { getCcsPathDisplay } from '../utils/config-manager'; import { getNodePlatformKey } from '../utils/browser/platform'; import { color, dim, header, initUI, subheader } from '../utils/ui'; type HelpWriter = (line: string) => void; +type BrowserLane = 'claude' | 'codex' | 'all'; function summarizeBrowserHealth(status: browserUtils.BrowserStatusPayload): { label: 'ready' | 'partial' | 'action required'; @@ -21,16 +24,40 @@ function summarizeBrowserHealth(status: browserUtils.BrowserStatusPayload): { return { label: 'ready', exitCode: 0 }; } +function isBrowserPolicy(value: string): value is BrowserToolPolicy { + return value === 'auto' || value === 'manual'; +} + +function parseBrowserLane(value: string | undefined): BrowserLane | undefined { + if (value === 'claude' || value === 'codex' || value === 'all') { + return value; + } + + return undefined; +} + function writeCommandTable(writeLine: HelpWriter): void { writeLine(subheader('Commands')); writeLine( - ` ${color('ccs browser setup', 'command')} Configure Claude Browser Attach and print the manual launch command` + ` ${color('ccs browser setup', 'command')} Configure Claude Browser Attach and print the manual launch command` ); writeLine( - ` ${color('ccs browser status', 'command')} Show Claude attach and Codex browser readiness` + ` ${color('ccs browser status', 'command')} Show Claude attach and Codex browser readiness` ); writeLine( - ` ${color('ccs browser doctor', 'command')} Explain what is missing and how to fix it` + ` ${color('ccs browser doctor', 'command')} Explain what is missing and how to fix it` + ); + writeLine( + ` ${color('ccs browser policy', 'command')} Show the saved browser exposure policy` + ); + writeLine( + ` ${color('ccs browser policy --all manual', 'command')} Keep browser tooling hidden unless a launch uses --browser` + ); + writeLine( + ` ${color('ccs browser enable ', 'command')} Turn a browser lane on` + ); + writeLine( + ` ${color('ccs browser disable ', 'command')} Turn a browser lane off` ); writeLine(''); } @@ -41,6 +68,19 @@ function writeIntro(writeLine: HelpWriter): void { ' Codex Browser Tools inject managed Playwright MCP overrides into Codex-target launches.' ); writeLine(''); + writeLine(subheader('Launch Overrides')); + writeLine( + ` ${color('--browser', 'command')} Force browser tooling on for the current launch when the lane is enabled` + ); + writeLine( + ` ${color('--no-browser', 'command')} Suppress browser tooling for the current launch even when policy is auto` + ); + writeLine(''); +} + +function writeLaunchPolicy(policy: BrowserToolPolicy, writeLine: HelpWriter): void { + writeLine(` Policy: ${browserUtils.describeBrowserPolicy(policy)}`); + writeLine(` Default launch behavior: ${browserUtils.describeDefaultBrowserExposure(policy)}`); } function writeClaudeStatus( @@ -56,6 +96,7 @@ function writeClaudeStatus( writeLine(subheader('Claude Browser Attach')); writeLine(` State: ${status.state}`); writeLine(` Enabled: ${status.enabled ? 'yes' : 'no'}`); + writeLaunchPolicy(status.policy, writeLine); writeLine(` Source: ${status.source}${status.overrideActive ? ' (env override active)' : ''}`); writeLine(` User data dir: ${userDataDirDisplay}`); writeLine(` DevTools port: ${status.devtoolsPort}`); @@ -80,6 +121,7 @@ function writeCodexStatus( writeLine(subheader('Codex Browser Tools')); writeLine(` State: ${status.state}`); writeLine(` Enabled: ${status.enabled ? 'yes' : 'no'}`); + writeLaunchPolicy(status.policy, writeLine); writeLine(` Managed server: ${status.serverName}`); writeLine(` Supports overrides: ${status.supportsConfigOverrides ? 'yes' : 'no'}`); writeLine(` Codex binary: ${status.binaryPath || 'not detected'}`); @@ -110,13 +152,144 @@ function writeSetupSummary( writeLine(''); } +function writePolicySummary(writeLine: HelpWriter): void { + const config = getBrowserConfig(); + + writeLine(header('ccs browser policy')); + writeLine(''); + writeIntro(writeLine); + writeLine(subheader('Claude Browser Attach')); + writeLine(` Enabled: ${config.claude.enabled ? 'yes' : 'no'}`); + writeLaunchPolicy(config.claude.policy, writeLine); + writeLine(''); + writeLine(subheader('Codex Browser Tools')); + writeLine(` Enabled: ${config.codex.enabled ? 'yes' : 'no'}`); + writeLaunchPolicy(config.codex.policy, writeLine); + writeLine(''); + writeLine(subheader('Examples')); + writeLine( + ` ${color('ccs browser policy --all manual', 'command')} ${dim('# keep browser tooling hidden until a launch opts in')}` + ); + writeLine( + ` ${color('ccs glm --browser "open the site"', 'command')} ${dim('# one-run browser opt-in')}` + ); + writeLine( + ` ${color('ccs glm --no-browser "summarize the docs"', 'command')} ${dim('# one-run browser opt-out')}` + ); + writeLine(''); +} + +function writeToggleSummary( + subcommand: 'enable' | 'disable', + lane: BrowserLane, + writeLine: HelpWriter +) { + const config = getBrowserConfig(); + const verb = subcommand === 'enable' ? 'enabled' : 'disabled'; + + writeLine(header(`ccs browser ${subcommand}`)); + writeLine(''); + writeLine(` Updated ${lane} browser lane${lane === 'all' ? 's' : ''}.`); + writeLine(` Browser lanes are now ${verb} as requested.`); + writeLine(''); + writeLine(subheader('Current State')); + writeLine(` Claude enabled: ${config.claude.enabled ? 'yes' : 'no'}`); + writeLine(` Claude policy: ${config.claude.policy}`); + writeLine(` Codex enabled: ${config.codex.enabled ? 'yes' : 'no'}`); + writeLine(` Codex policy: ${config.codex.policy}`); + writeLine(''); +} + +function updateBrowserPolicies(updates: { + claude?: BrowserToolPolicy; + codex?: BrowserToolPolicy; +}): void { + mutateUnifiedConfig((config) => { + const current = getBrowserConfig(); + config.browser = { + claude: { + enabled: current.claude.enabled, + policy: updates.claude ?? current.claude.policy, + user_data_dir: current.claude.user_data_dir, + devtools_port: current.claude.devtools_port, + }, + codex: { + enabled: current.codex.enabled, + policy: updates.codex ?? current.codex.policy, + }, + }; + }); +} + +function updateBrowserEnabled(subcommand: 'enable' | 'disable', lane: BrowserLane): void { + const nextEnabled = subcommand === 'enable'; + mutateUnifiedConfig((config) => { + const current = getBrowserConfig(); + config.browser = { + claude: { + enabled: lane === 'all' || lane === 'claude' ? nextEnabled : current.claude.enabled, + policy: current.claude.policy, + user_data_dir: current.claude.user_data_dir, + devtools_port: current.claude.devtools_port, + }, + codex: { + enabled: lane === 'all' || lane === 'codex' ? nextEnabled : current.codex.enabled, + policy: current.codex.policy, + }, + }; + }); +} + +function parsePolicyArgs(args: string[]): { + claude?: BrowserToolPolicy; + codex?: BrowserToolPolicy; + error?: string; +} { + let claude: BrowserToolPolicy | undefined; + let codex: BrowserToolPolicy | undefined; + + for (let index = 0; index < args.length; index += 2) { + const flag = args[index]; + const value = args[index + 1]; + + if (!flag) { + break; + } + + if (flag !== '--all' && flag !== '--claude' && flag !== '--codex') { + return { error: `Unknown browser policy argument: ${flag}` }; + } + + if (!value || value.startsWith('-')) { + return { error: `${flag} requires a value: auto or manual.` }; + } + + if (!isBrowserPolicy(value)) { + return { error: `${flag} must be one of: auto, manual.` }; + } + + if (flag === '--all' || flag === '--claude') { + claude = value; + } + if (flag === '--all' || flag === '--codex') { + codex = value; + } + } + + return { claude, codex }; +} + +function isHelpRequest(args: string[]): boolean { + return args.length === 0 || args[0] === 'help' || args.includes('--help') || args.includes('-h'); +} + export async function showBrowserHelp(writeLine: HelpWriter = console.log): Promise { await initUI(); writeLine(header('CCS Browser Help')); writeLine(''); writeIntro(writeLine); writeLine(subheader('Usage')); - writeLine(` ${color('ccs browser ', 'command')}`); + writeLine(` ${color('ccs browser ', 'command')}`); writeLine(` ${color('ccs help browser', 'command')}`); writeLine(''); writeCommandTable(writeLine); @@ -126,21 +299,23 @@ export async function showBrowserHelp(writeLine: HelpWriter = console.log): Prom writeLine(''); writeLine(subheader('Examples')); writeLine( - ` ${color('ccs browser setup', 'command')} ${dim('# Configure browser attach and print the manual launch command')}` + ` ${color('ccs browser setup', 'command')} ${dim('# configure browser attach and print the manual launch command')}` ); writeLine( - ` ${color('ccs browser doctor', 'command')} ${dim('# Detailed troubleshooting output')}` + ` ${color('ccs browser policy --all manual', 'command')} ${dim('# keep browser tooling hidden until a launch opts in')}` ); writeLine( - ` ${color('ccs config', 'command')} ${dim('# Open Settings > Browser in the dashboard')}` + ` ${color('ccs glm --browser "inspect app"', 'command')} ${dim('# one-run browser opt-in')}` + ); + writeLine( + ` ${color('ccs glm --no-browser "summarize app"', 'command')} ${dim('# one-run browser opt-out')}` + ); + writeLine( + ` ${color('ccs config', 'command')} ${dim('# open Settings > Browser in the dashboard')}` ); writeLine(''); } -function isHelpRequest(args: string[]): boolean { - return args.length === 0 || args[0] === 'help' || args.includes('--help') || args.includes('-h'); -} - export async function handleBrowserCommand( args: string[], writeLine: HelpWriter = console.log @@ -175,6 +350,41 @@ export async function handleBrowserCommand( return; } + if (subcommand === 'policy') { + const parsed = parsePolicyArgs(args.slice(1)); + await initUI(); + + if (parsed.error) { + writeLine(color(parsed.error, 'error')); + writeLine(''); + process.exitCode = 1; + return; + } + + if (parsed.claude || parsed.codex) { + updateBrowserPolicies(parsed); + } + + writePolicySummary(writeLine); + return; + } + + if (subcommand === 'enable' || subcommand === 'disable') { + const lane = parseBrowserLane(args[1]); + await initUI(); + + if (!lane || args.length > 2) { + writeLine(color(`Usage: ccs browser ${subcommand} `, 'error')); + writeLine(''); + process.exitCode = 1; + return; + } + + updateBrowserEnabled(subcommand, lane); + writeToggleSummary(subcommand, lane, writeLine); + return; + } + if (subcommand === 'doctor' && (args.includes('--fix') || args.includes('-f'))) { await initUI(); writeLine(color('`ccs browser doctor` is read-only.', 'error')); @@ -188,7 +398,7 @@ export async function handleBrowserCommand( await initUI(); writeLine(color(`Unknown browser subcommand: ${subcommand}`, 'error')); writeLine(''); - writeLine(` ${dim('Supported subcommands: setup, status, doctor')}`); + writeLine(` ${dim('Supported subcommands: setup, status, doctor, policy, enable, disable')}`); writeLine(''); process.exitCode = 1; return; diff --git a/src/commands/command-catalog.ts b/src/commands/command-catalog.ts index a3f9b666..6707867c 100644 --- a/src/commands/command-catalog.ts +++ b/src/commands/command-catalog.ts @@ -123,7 +123,7 @@ export const ROOT_COMMAND_CATALOG: readonly RootCommandEntry[] = [ }, { name: 'browser', - summary: 'Set up or inspect Claude Browser Attach and Codex Browser Tools readiness', + summary: 'Set up, inspect, and control Claude Browser Attach and Codex Browser Tools', group: 'runtime', visibility: 'public', }, @@ -320,7 +320,7 @@ export const COMMAND_FLAG_SUGGESTIONS: Readonly> = getLegacyTargetAliasEnvVars(); const RESERVED_BIN_NAMES = new Set(['ccs', ...Object.keys(BUILTIN_ARGV0_TARGET_MAP)]); +const INTERNAL_RUNTIME_ENTRY_BASENAMES = new Set([ + 'droid-runtime', + 'droid-runtime.ts', + 'droid-runtime.js', + 'codex-runtime', + 'codex-runtime.ts', + 'codex-runtime.js', + 'ccsxp-runtime', + 'ccsxp-runtime.ts', + 'ccsxp-runtime.js', +]); function addAliasToMap(map: Record, alias: string, target: TargetType): void { const normalizedAlias = alias.trim().toLowerCase(); @@ -102,6 +113,11 @@ function resolveEntrypointTarget(): TargetType | null { return null; } + const entryScript = path.basename(process.argv[1] || ''); + if (!INTERNAL_RUNTIME_ENTRY_BASENAMES.has(entryScript)) { + return null; + } + const normalizedTarget = rawTarget.trim().toLowerCase(); return isRuntimeTargetType(normalizedTarget) ? normalizedTarget : null; } diff --git a/src/utils/browser/browser-policy.ts b/src/utils/browser/browser-policy.ts new file mode 100644 index 00000000..eec51409 --- /dev/null +++ b/src/utils/browser/browser-policy.ts @@ -0,0 +1,86 @@ +import type { BrowserToolPolicy } from '../../config/unified-config-types'; + +export type BrowserLaunchOverride = 'force-enable' | 'force-disable'; + +export interface BrowserLaunchFlagResolution { + override?: BrowserLaunchOverride; + argsWithoutFlags: string[]; +} + +export interface ResolvedBrowserExposure { + enabled: boolean; + policy: BrowserToolPolicy; + override?: BrowserLaunchOverride; + exposeByDefault: boolean; + exposeForLaunch: boolean; +} + +const ENABLE_BROWSER_FLAG = '--browser'; +const DISABLE_BROWSER_FLAG = '--no-browser'; + +export function resolveBrowserLaunchFlagResolution(args: string[]): BrowserLaunchFlagResolution { + let override: BrowserLaunchOverride | undefined; + const argsWithoutFlags: string[] = []; + + for (const arg of args) { + if (arg === ENABLE_BROWSER_FLAG) { + if (override === 'force-disable') { + throw new Error('Use either `--browser` or `--no-browser`, not both.'); + } + override = 'force-enable'; + continue; + } + + if (arg === DISABLE_BROWSER_FLAG) { + if (override === 'force-enable') { + throw new Error('Use either `--browser` or `--no-browser`, not both.'); + } + override = 'force-disable'; + continue; + } + + argsWithoutFlags.push(arg); + } + + return { + override, + argsWithoutFlags, + }; +} + +export function resolveBrowserExposure( + config: { enabled: boolean; policy: BrowserToolPolicy }, + override?: BrowserLaunchOverride +): ResolvedBrowserExposure { + const exposeByDefault = config.enabled && config.policy === 'auto'; + const exposeForLaunch = + config.enabled && + (override === 'force-enable' || (override !== 'force-disable' && exposeByDefault)); + + return { + enabled: config.enabled, + policy: config.policy, + override, + exposeByDefault, + exposeForLaunch, + }; +} + +export function describeBrowserPolicy(policy: BrowserToolPolicy): string { + return policy === 'manual' ? 'manual' : 'auto'; +} + +export function describeDefaultBrowserExposure(policy: BrowserToolPolicy): string { + return policy === 'manual' ? 'hidden until `--browser`' : 'auto-exposed'; +} + +export function getBlockedBrowserOverrideWarning( + laneLabel: string, + exposure: ResolvedBrowserExposure +): string | undefined { + if (exposure.override === 'force-enable' && !exposure.enabled) { + return `Browser tooling was requested with \`--browser\`, but ${laneLabel} is disabled in CCS config.`; + } + + return undefined; +} diff --git a/src/utils/browser/browser-setup.ts b/src/utils/browser/browser-setup.ts index 69c038a5..1d23e738 100644 --- a/src/utils/browser/browser-setup.ts +++ b/src/utils/browser/browser-setup.ts @@ -88,11 +88,13 @@ function persistBrowserSetupConfig(deps: BrowserSetupDeps, currentConfig: Browse config.browser = { claude: { enabled: true, + policy: existingBrowser.claude.policy, user_data_dir: currentUserDataDir || getRecommendedBrowserUserDataDir(), devtools_port: currentConfig.claude.devtools_port, }, codex: { enabled: existingBrowser.codex.enabled, + policy: existingBrowser.codex.policy, }, }; }); diff --git a/src/utils/browser/browser-status.ts b/src/utils/browser/browser-status.ts index ee64ab02..537d5a1b 100644 --- a/src/utils/browser/browser-status.ts +++ b/src/utils/browser/browser-status.ts @@ -1,4 +1,5 @@ import * as path from 'path'; +import type { BrowserToolPolicy } from '../../config/unified-config-types'; import { getBrowserConfig } from '../../config/unified-config-loader'; import { getCcsPathDisplay } from '../config-manager'; import { getCodexBinaryInfo } from '../../targets/codex-detector'; @@ -17,6 +18,7 @@ import { export interface ClaudeBrowserStatus { enabled: boolean; + policy: BrowserToolPolicy; source: 'config' | 'CCS_BROWSER_USER_DATA_DIR' | 'CCS_BROWSER_PROFILE_DIR'; overrideActive: boolean; state: 'disabled' | 'path_missing' | 'browser_not_running' | 'endpoint_unreachable' | 'ready'; @@ -34,6 +36,7 @@ export interface ClaudeBrowserStatus { export interface CodexBrowserStatus { enabled: boolean; + policy: BrowserToolPolicy; state: 'disabled' | 'enabled' | 'unsupported_build'; title: string; detail: string; @@ -65,6 +68,7 @@ async function buildClaudeBrowserStatus( const managedBootstrap = ensureManagedBrowserUserDataDir(effective); const base: Omit = { enabled: effective.enabled, + policy: browserConfig.claude.policy, source: effective.source, overrideActive: effective.overrideActive, effectiveUserDataDir: effective.userDataDir, @@ -171,6 +175,7 @@ function buildCodexBrowserStatus(browserConfig = getBrowserConfig()): CodexBrows if (!browserConfig.codex.enabled) { return { enabled: false, + policy: browserConfig.codex.policy, state: 'disabled', title: 'Codex Browser Tools are disabled.', detail: 'CCS will not inject Playwright MCP browser tooling into Codex-target launches.', @@ -187,6 +192,7 @@ function buildCodexBrowserStatus(browserConfig = getBrowserConfig()): CodexBrows if (!binaryInfo || !supportsConfigOverrides) { return { enabled: true, + policy: browserConfig.codex.policy, state: 'unsupported_build', title: 'Codex Browser Tools need a Codex build with --config override support.', detail: binaryInfo @@ -202,6 +208,7 @@ function buildCodexBrowserStatus(browserConfig = getBrowserConfig()): CodexBrows return { enabled: true, + policy: browserConfig.codex.policy, state: 'enabled', title: 'Codex Browser Tools are enabled.', detail: 'CCS can inject the managed Playwright MCP overrides into Codex-target launches.', diff --git a/src/utils/browser/index.ts b/src/utils/browser/index.ts index 966c47ff..dbcf892f 100644 --- a/src/utils/browser/index.ts +++ b/src/utils/browser/index.ts @@ -16,6 +16,14 @@ export { } from './mcp-installer'; export { appendBrowserToolArgs } from './claude-tool-args'; +export { + describeBrowserPolicy, + describeDefaultBrowserExposure, + getBlockedBrowserOverrideWarning, + resolveBrowserExposure, + resolveBrowserLaunchFlagResolution, +} from './browser-policy'; +export type { BrowserLaunchOverride, BrowserLaunchFlagResolution } from './browser-policy'; export { buildBrowserLaunchCommands, diff --git a/src/web-server/routes/browser-routes.ts b/src/web-server/routes/browser-routes.ts index 14d8bdc3..6fb05425 100644 --- a/src/web-server/routes/browser-routes.ts +++ b/src/web-server/routes/browser-routes.ts @@ -10,14 +10,20 @@ const BROWSER_LOCAL_ACCESS_ERROR = interface BrowserRouteBody { claude?: { enabled?: boolean; + policy?: 'auto' | 'manual'; userDataDir?: string; devtoolsPort?: number; }; codex?: { enabled?: boolean; + policy?: 'auto' | 'manual'; }; } +function isValidBrowserPolicy(value: string): value is 'auto' | 'manual' { + return value === 'auto' || value === 'manual'; +} + function isValidDevtoolsPort(value: number): boolean { return Number.isInteger(value) && value >= 1 && value <= 65535; } @@ -73,6 +79,13 @@ router.put('/', async (req: Request, res: Response): Promise => { res.status(400).json({ error: 'Invalid value for claude.enabled. Must be a boolean.' }); return; } + if ( + claude?.policy !== undefined && + (typeof claude.policy !== 'string' || !isValidBrowserPolicy(claude.policy)) + ) { + res.status(400).json({ error: 'Invalid value for claude.policy. Must be auto or manual.' }); + return; + } if (claude?.userDataDir !== undefined && typeof claude.userDataDir !== 'string') { res.status(400).json({ error: 'Invalid value for claude.userDataDir. Must be a string.' }); return; @@ -90,6 +103,13 @@ router.put('/', async (req: Request, res: Response): Promise => { res.status(400).json({ error: 'Invalid value for codex.enabled. Must be a boolean.' }); return; } + if ( + codex?.policy !== undefined && + (typeof codex.policy !== 'string' || !isValidBrowserPolicy(codex.policy)) + ) { + res.status(400).json({ error: 'Invalid value for codex.policy. Must be auto or manual.' }); + return; + } try { const current = getBrowserConfig(); @@ -99,11 +119,13 @@ router.put('/', async (req: Request, res: Response): Promise => { config.browser = { claude: { enabled: claude?.enabled ?? current.claude.enabled, + policy: claude?.policy ?? current.claude.policy, user_data_dir: nextClaudeUserDataDir, devtools_port: claude?.devtoolsPort ?? current.claude.devtools_port, }, codex: { enabled: codex?.enabled ?? current.codex.enabled, + policy: codex?.policy ?? current.codex.policy, }, }; }); @@ -126,11 +148,13 @@ function toBrowserRouteConfig(config: ReturnType) { return { claude: { enabled: config.claude.enabled, + policy: config.claude.policy, userDataDir: config.claude.user_data_dir, devtoolsPort: config.claude.devtools_port, }, codex: { enabled: config.codex.enabled, + policy: config.codex.policy, }, }; } diff --git a/tests/unit/commands/browser-command.test.ts b/tests/unit/commands/browser-command.test.ts index f43dbfbe..809c9f6e 100644 --- a/tests/unit/commands/browser-command.test.ts +++ b/tests/unit/commands/browser-command.test.ts @@ -1,7 +1,11 @@ -import { afterEach, describe, expect, test, spyOn } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, test, spyOn } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import * as browserUtils from '../../../src/utils/browser'; import { handleBrowserCommand } from '../../../src/commands/browser-command'; +import { getBrowserConfig } from '../../../src/config/unified-config-loader'; function stripAnsi(input: string): string { return input.replace(/\u001b\[[0-9;]*m/g, ''); @@ -20,7 +24,22 @@ function currentPlatform(): 'darwin' | 'linux' | 'win32' { } describe('browser command', () => { + let tempHome = ''; + let originalCcsHome: string | undefined; + + beforeEach(() => { + tempHome = mkdtempSync(join(tmpdir(), 'ccs-browser-command-')); + originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tempHome; + }); + afterEach(() => { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + rmSync(tempHome, { recursive: true, force: true }); process.exitCode = 0; }); @@ -28,6 +47,7 @@ describe('browser command', () => { const statusSpy = spyOn(browserUtils, 'getBrowserStatus').mockResolvedValue({ claude: { enabled: true, + policy: 'auto', source: 'config', overrideActive: false, state: 'ready', @@ -54,6 +74,7 @@ describe('browser command', () => { }, codex: { enabled: true, + policy: 'auto', state: 'enabled', title: 'Codex Browser Tools are enabled.', detail: 'CCS can inject the managed Playwright MCP overrides.', @@ -75,6 +96,7 @@ describe('browser command', () => { ); expect(rendered.includes('Managed MCP: ccs-browser')).toBe(true); expect(rendered.includes('Managed server: ccs_browser')).toBe(true); + expect(rendered.includes('Policy: auto')).toBe(true); expect(rendered.includes('DevTools endpoint: http://127.0.0.1:9222')).toBe(true); } finally { statusSpy.mockRestore(); @@ -91,6 +113,7 @@ describe('browser command', () => { const statusSpy = spyOn(browserUtils, 'getBrowserStatus').mockResolvedValue({ claude: { enabled: true, + policy: 'manual', source: 'CCS_BROWSER_PROFILE_DIR', overrideActive: true, state: 'browser_not_running', @@ -106,6 +129,7 @@ describe('browser command', () => { }, codex: { enabled: true, + policy: 'manual', state: 'unsupported_build', title: 'Codex Browser Tools need a Codex build with --config override support.', detail: 'Detected Codex at /usr/local/bin/codex, but it does not advertise --config overrides.', @@ -121,6 +145,7 @@ describe('browser command', () => { const rendered = await renderLines(['doctor']); expect(rendered.includes('Result: action required')).toBe(true); + expect(rendered.includes('Default launch behavior: hidden until `--browser`')).toBe(true); expect(rendered.includes('Source: CCS_BROWSER_PROFILE_DIR (env override active)')).toBe( true ); @@ -140,6 +165,7 @@ describe('browser command', () => { const statusSpy = spyOn(browserUtils, 'getBrowserStatus').mockResolvedValue({ claude: { enabled: false, + policy: 'auto', source: 'config', overrideActive: false, state: 'disabled', @@ -162,6 +188,7 @@ describe('browser command', () => { }, codex: { enabled: true, + policy: 'auto', state: 'unsupported_build', title: 'Codex Browser Tools need a Codex build with --config override support.', detail: 'No Codex binary was detected, so CCS cannot confirm managed browser override support.', @@ -194,6 +221,7 @@ describe('browser command', () => { status: { claude: { enabled: true, + policy: 'auto', source: 'config', overrideActive: false, state: 'ready', @@ -220,6 +248,7 @@ describe('browser command', () => { }, codex: { enabled: true, + policy: 'auto', state: 'enabled', title: 'Codex Browser Tools are enabled.', detail: 'CCS can inject the managed Playwright MCP overrides.', @@ -255,10 +284,30 @@ describe('browser command', () => { expect(process.exitCode).toBe(1); }); + test('policy shows and updates the saved browser exposure mode', async () => { + const rendered = await renderLines(['policy', '--all', 'manual']); + + expect(rendered.includes('ccs browser policy')).toBe(true); + expect(rendered.includes('Default launch behavior: hidden until `--browser`')).toBe(true); + expect(getBrowserConfig().claude.policy).toBe('manual'); + expect(getBrowserConfig().codex.policy).toBe('manual'); + }); + + test('enable updates a single browser lane', async () => { + await renderLines(['disable', 'codex']); + expect(getBrowserConfig().codex.enabled).toBe(false); + + const rendered = await renderLines(['enable', 'codex']); + expect(rendered.includes('Updated codex browser lane.')).toBe(true); + expect(getBrowserConfig().codex.enabled).toBe(true); + }); + test('literal browser help still renders the help page', async () => { const rendered = await renderLines(['help']); expect(rendered.includes('CCS Browser Help')).toBe(true); expect(rendered.includes('ccs browser setup')).toBe(true); + expect(rendered.includes('ccs browser policy')).toBe(true); + expect(rendered.includes('--browser')).toBe(true); }); }); diff --git a/tests/unit/commands/completion-backend.test.ts b/tests/unit/commands/completion-backend.test.ts index ab0044c5..a86d9628 100644 --- a/tests/unit/commands/completion-backend.test.ts +++ b/tests/unit/commands/completion-backend.test.ts @@ -142,7 +142,7 @@ describe('completion backend', () => { test('suggests browser subcommands and fix/setup flags', () => { expect(suggestionValues(['browser'])).toEqual( - expect.arrayContaining(['setup', 'status', 'doctor']) + expect.arrayContaining(['setup', 'status', 'doctor', 'policy', 'enable', 'disable']) ); expect(suggestionValues(['browser', 'setup'])).toEqual(expect.arrayContaining(['--help', '-h'])); expect(suggestionValues(['browser', 'doctor'])).toEqual( diff --git a/tests/unit/commands/help-command-parity.test.ts b/tests/unit/commands/help-command-parity.test.ts index 9fd4fa17..100741df 100644 --- a/tests/unit/commands/help-command-parity.test.ts +++ b/tests/unit/commands/help-command-parity.test.ts @@ -81,6 +81,8 @@ describe('help command parity', () => { expect(rendered.includes('ccs browser setup')).toBe(true); expect(rendered.includes('ccs browser status')).toBe(true); expect(rendered.includes('ccs browser doctor')).toBe(true); + expect(rendered.includes('ccs browser policy')).toBe(true); + expect(rendered.includes('--browser')).toBe(true); }); test('completion topic documents install and verification paths', async () => { diff --git a/tests/unit/targets/codex-runtime-integration.test.ts b/tests/unit/targets/codex-runtime-integration.test.ts index bfdb1212..9219ec83 100644 --- a/tests/unit/targets/codex-runtime-integration.test.ts +++ b/tests/unit/targets/codex-runtime-integration.test.ts @@ -205,11 +205,13 @@ process.exit(0); config.browser = { claude: { enabled: false, + policy: 'auto', user_data_dir: '', devtools_port: 9222, }, codex: { enabled: false, + policy: 'auto', }, }; }); @@ -235,6 +237,114 @@ process.exit(0); } }); + it('keeps Codex browser MCP overrides off by default when policy is manual', () => { + if (process.platform === 'win32') return; + + const originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + + try { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: false, + policy: 'auto', + user_data_dir: '', + devtools_port: 9222, + }, + codex: { + enabled: true, + policy: 'manual', + }, + }; + }); + + const result = runCcs(['default', '--target', 'codex', 'fix failing tests'], { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CODEX_PATH: fakeCodexPath, + CCS_TEST_CODEX_ARGS_OUT: codexArgsLogPath, + }); + + expect(result.status).toBe(0); + const calls = readLoggedCodexCalls(codexArgsLogPath); + expect(calls).toEqual([['fix failing tests']]); + } finally { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + } + }); + + it('forces Codex browser MCP overrides on for one launch when --browser is passed', () => { + if (process.platform === 'win32') return; + + const originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + + try { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: false, + policy: 'auto', + user_data_dir: '', + devtools_port: 9222, + }, + codex: { + enabled: true, + policy: 'manual', + }, + }; + }); + + const result = runCcs(['default', '--target', 'codex', '--browser', 'fix failing tests'], { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CODEX_PATH: fakeCodexPath, + CCS_TEST_CODEX_ARGS_OUT: codexArgsLogPath, + }); + + expect(result.status).toBe(0); + const calls = readLoggedCodexCalls(codexArgsLogPath); + expect(calls[1]).toEqual( + expect.arrayContaining([ + 'mcp_servers.ccs_browser.enabled=true', + 'fix failing tests', + ]) + ); + } finally { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + } + }); + + it('suppresses Codex browser MCP overrides for one launch when --no-browser is passed', () => { + if (process.platform === 'win32') return; + + const result = runCcs(['default', '--target', 'codex', '--no-browser', 'fix failing tests'], { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CODEX_PATH: fakeCodexPath, + CCS_TEST_CODEX_ARGS_OUT: codexArgsLogPath, + }); + + expect(result.status).toBe(0); + const calls = readLoggedCodexCalls(codexArgsLogPath); + expect(calls).toEqual([['fix failing tests']]); + }); + it('keeps browser MCP runtime overrides when CCS_THINKING is ignored for native Codex default mode', () => { if (process.platform === 'win32') return; diff --git a/tests/unit/targets/default-profile-browser-launch.test.ts b/tests/unit/targets/default-profile-browser-launch.test.ts index f6c635c7..a5ceeddc 100644 --- a/tests/unit/targets/default-profile-browser-launch.test.ts +++ b/tests/unit/targets/default-profile-browser-launch.test.ts @@ -270,11 +270,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: 43123, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -321,11 +323,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: unreachablePort, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -396,11 +400,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: browserProfileDir, devtools_port: Number.parseInt(port, 10), }, codex: { enabled: true, + policy: 'auto', }, }; }); diff --git a/tests/unit/targets/settings-profile-browser-launch.test.ts b/tests/unit/targets/settings-profile-browser-launch.test.ts index 5444334e..ff23bac8 100644 --- a/tests/unit/targets/settings-profile-browser-launch.test.ts +++ b/tests/unit/targets/settings-profile-browser-launch.test.ts @@ -221,11 +221,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: 43123, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -273,11 +275,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: unreachablePort, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -354,11 +358,13 @@ server.listen(0, '127.0.0.1', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: browserProfileDir, devtools_port: Number.parseInt(port, 10), }, codex: { enabled: true, + policy: 'auto', }, }; }); diff --git a/tests/unit/targets/target-resolver.test.ts b/tests/unit/targets/target-resolver.test.ts index 68299433..d63c79d8 100644 --- a/tests/unit/targets/target-resolver.test.ts +++ b/tests/unit/targets/target-resolver.test.ts @@ -158,16 +158,22 @@ describe('resolveTargetType', () => { it('should detect internal entry target for dedicated package bin entrypoints', () => { process.env.CCS_INTERNAL_ENTRY_TARGET = 'droid'; - process.argv = ['node', 'ccs']; + process.argv = ['node', '/usr/local/lib/ccs/dist/bin/droid-runtime.js']; expect(resolveTargetType([])).toBe('droid'); }); it('should detect internal entry target for codex runtime bins', () => { process.env.CCS_INTERNAL_ENTRY_TARGET = 'codex'; - process.argv = ['node', 'ccs']; + process.argv = ['node', '/usr/local/lib/ccs/dist/bin/codex-runtime.js']; expect(resolveTargetType([])).toBe('codex'); }); + it('should ignore leaked internal entry target env on the main ccs entrypoint', () => { + process.env.CCS_INTERNAL_ENTRY_TARGET = 'codex'; + process.argv = ['node', 'ccs.js']; + expect(resolveTargetType([])).toBe('claude'); + }); + it('should normalize argv[0] and custom aliases case-insensitively', () => { process.env.CCS_DROID_ALIASES = 'DroidCaps'; process.argv = ['node', 'DROIDCAPS']; @@ -242,7 +248,7 @@ describe('resolveTargetType', () => { it('should prioritize internal entry target over profile config', () => { process.env.CCS_INTERNAL_ENTRY_TARGET = 'droid'; - process.argv = ['node', 'ccs']; + process.argv = ['node', '/usr/local/lib/ccs/dist/bin/droid-runtime.js']; expect(resolveTargetType([], { target: 'claude' })).toBe('droid'); }); diff --git a/tests/unit/utils/browser/browser-policy.test.ts b/tests/unit/utils/browser/browser-policy.test.ts new file mode 100644 index 00000000..cf2c1b8d --- /dev/null +++ b/tests/unit/utils/browser/browser-policy.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'bun:test'; +import { + resolveBrowserExposure, + resolveBrowserLaunchFlagResolution, +} from '../../../../src/utils/browser/browser-policy'; + +describe('browser policy', () => { + it('strips browser launch flags and records the override', () => { + expect(resolveBrowserLaunchFlagResolution(['glm', '--browser', 'check app'])).toEqual({ + override: 'force-enable', + argsWithoutFlags: ['glm', 'check app'], + }); + expect(resolveBrowserLaunchFlagResolution(['glm', '--no-browser', 'check app'])).toEqual({ + override: 'force-disable', + argsWithoutFlags: ['glm', 'check app'], + }); + }); + + it('rejects conflicting browser launch flags', () => { + expect(() => resolveBrowserLaunchFlagResolution(['--browser', '--no-browser'])).toThrow( + 'Use either `--browser` or `--no-browser`, not both.' + ); + }); + + it('resolves browser exposure from saved policy and one-run overrides', () => { + expect(resolveBrowserExposure({ enabled: true, policy: 'auto' })).toMatchObject({ + exposeByDefault: true, + exposeForLaunch: true, + }); + expect(resolveBrowserExposure({ enabled: true, policy: 'manual' })).toMatchObject({ + exposeByDefault: false, + exposeForLaunch: false, + }); + expect( + resolveBrowserExposure({ enabled: true, policy: 'manual' }, 'force-enable') + ).toMatchObject({ + exposeByDefault: false, + exposeForLaunch: true, + }); + expect( + resolveBrowserExposure({ enabled: true, policy: 'auto' }, 'force-disable') + ).toMatchObject({ + exposeByDefault: true, + exposeForLaunch: false, + }); + }); +}); diff --git a/tests/unit/utils/browser/browser-status.test.ts b/tests/unit/utils/browser/browser-status.test.ts index ceb64bd9..5ddceeaf 100644 --- a/tests/unit/utils/browser/browser-status.test.ts +++ b/tests/unit/utils/browser/browser-status.test.ts @@ -98,11 +98,13 @@ describe('browser status', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: 9222, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -138,11 +140,13 @@ describe('browser status', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '/config-browser', devtools_port: 9333, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -185,11 +189,13 @@ describe('browser status', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '', devtools_port: 9222, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -227,11 +233,13 @@ describe('browser status', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '/tmp/browser-profile', devtools_port: 9222, }, codex: { enabled: true, + policy: 'auto', }, }; }); @@ -294,11 +302,13 @@ describe('browser status', () => { config.browser = { claude: { enabled: true, + policy: 'auto', user_data_dir: '/tmp/config-browser', devtools_port: 9222, }, codex: { enabled: true, + policy: 'auto', }, }; }); diff --git a/tests/unit/web-server/browser-routes.test.ts b/tests/unit/web-server/browser-routes.test.ts index 2ce1c342..e54e264c 100644 --- a/tests/unit/web-server/browser-routes.test.ts +++ b/tests/unit/web-server/browser-routes.test.ts @@ -93,11 +93,13 @@ describe('browser routes', () => { expect(payload.config).toMatchObject({ claude: { enabled: false, + policy: 'auto', userDataDir: join(tempHome, '.ccs', 'browser', 'chrome-user-data'), devtoolsPort: 9222, }, codex: { enabled: true, + policy: 'auto', }, }); expect(payload.status.claude).toMatchObject({ @@ -117,11 +119,13 @@ describe('browser routes', () => { body: JSON.stringify({ claude: { enabled: true, + policy: 'manual', userDataDir: '/tmp/ccs-browser', devtoolsPort: 9333, }, codex: { enabled: false, + policy: 'manual', }, }), }); @@ -131,11 +135,13 @@ describe('browser routes', () => { expect(payload.browser.config).toMatchObject({ claude: { enabled: true, + policy: 'manual', userDataDir: '/tmp/ccs-browser', devtoolsPort: 9333, }, codex: { enabled: false, + policy: 'manual', }, }); @@ -143,11 +149,13 @@ describe('browser routes', () => { expect(config.browser).toMatchObject({ claude: { enabled: true, + policy: 'manual', user_data_dir: '/tmp/ccs-browser', devtools_port: 9333, }, codex: { enabled: false, + policy: 'manual', }, }); }); @@ -159,6 +167,7 @@ describe('browser routes', () => { body: JSON.stringify({ claude: { enabled: true, + policy: 'manual', userDataDir: '/tmp/ccs-browser-custom', devtoolsPort: 9333, }, @@ -181,6 +190,7 @@ describe('browser routes', () => { const payload = await resetResponse.json(); expect(payload.browser.config.claude).toMatchObject({ enabled: true, + policy: 'manual', userDataDir: join(tempHome, '.ccs', 'browser', 'chrome-user-data'), devtoolsPort: 9333, }); @@ -192,6 +202,7 @@ describe('browser routes', () => { expect(config.browser).toMatchObject({ claude: { enabled: true, + policy: 'manual', user_data_dir: join(tempHome, '.ccs', 'browser', 'chrome-user-data'), devtools_port: 9333, }, @@ -214,4 +225,21 @@ describe('browser routes', () => { error: 'Invalid value for claude.devtoolsPort. Must be an integer between 1 and 65535.', }); }); + + it('rejects invalid browser policy values at the route boundary', async () => { + const response = await fetch(`${baseUrl}/api/browser`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + codex: { + policy: 'always', + }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: 'Invalid value for codex.policy. Must be auto or manual.', + }); + }); }); From 039ed63a390da270fa5bb1685239508a21509d5c Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 20 Apr 2026 00:10:04 -0400 Subject: [PATCH 2/3] fix(browser): harden runtime policy edge cases --- docs/browser-automation.md | 3 + src/ccs.ts | 23 +++-- src/cliproxy/executor/index.ts | 17 +++- src/commands/command-catalog.ts | 2 + src/commands/completion-backend.ts | 15 ++++ src/utils/browser/browser-policy.ts | 9 +- src/utils/browser/browser-status.ts | 3 +- src/web-server/routes/browser-routes.ts | 46 ++++++++-- .../cliproxy/executor-option-value.test.ts | 47 +++++++++- .../unit/commands/completion-backend.test.ts | 12 +++ .../targets/codex-runtime-integration.test.ts | 18 ++++ .../default-profile-browser-launch.test.ts | 82 +++++++++++++++++ .../settings-profile-browser-launch.test.ts | 88 +++++++++++++++++++ .../unit/utils/browser/browser-policy.test.ts | 7 ++ .../unit/utils/browser/browser-status.test.ts | 1 + tests/unit/web-server/browser-routes.test.ts | 49 +++++++++++ 16 files changed, 394 insertions(+), 28 deletions(-) diff --git a/docs/browser-automation.md b/docs/browser-automation.md index 8e3e72bd..f4610195 100644 --- a/docs/browser-automation.md +++ b/docs/browser-automation.md @@ -126,6 +126,9 @@ 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. +The saved browser policy still controls default exposure. Env overrides change the effective attach +path/port for the current shell; they do not bypass `policy: manual`. + Override precedence is: 1. `CCS_BROWSER_USER_DATA_DIR` diff --git a/src/ccs.ts b/src/ccs.ts index 9bed9be9..1d8c5ad7 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -463,6 +463,16 @@ async function main(): Promise { } args = normalizeLegacyCursorArgs(args); + let browserLaunchOverride: BrowserLaunchOverride | undefined; + try { + const browserLaunchFlags = resolveBrowserLaunchFlagResolution(args); + browserLaunchOverride = browserLaunchFlags.override; + args = browserLaunchFlags.argsWithoutFlags; + } catch (error) { + console.error(fail((error as Error).message)); + process.exit(1); + return; + } cliLogger.info('command.start', 'CLI invocation started', { command: args[0] || 'default', @@ -605,17 +615,6 @@ async function main(): Promise { const detector = new ProfileDetector(); try { - let browserLaunchOverride: BrowserLaunchOverride | undefined; - try { - const browserLaunchFlags = resolveBrowserLaunchFlagResolution(args); - browserLaunchOverride = browserLaunchFlags.override; - args = browserLaunchFlags.argsWithoutFlags; - } catch (error) { - console.error(fail((error as Error).message)); - process.exit(1); - return; - } - // Detect profile (strip --target flags before profile detection) const cleanArgs = stripTargetFlag(args); const { profile, remainingArgs } = detectProfile(cleanArgs); @@ -736,7 +735,7 @@ async function main(): Promise { ? resolveBrowserExposure( { enabled: claudeAttachConfig?.enabled ?? browserConfig.claude.enabled, - policy: claudeAttachConfig?.overrideActive ? 'auto' : browserConfig.claude.policy, + policy: browserConfig.claude.policy, }, browserLaunchOverride ) diff --git a/src/cliproxy/executor/index.ts b/src/cliproxy/executor/index.ts index 98d0d32d..2f534ff2 100644 --- a/src/cliproxy/executor/index.ts +++ b/src/cliproxy/executor/index.ts @@ -65,6 +65,7 @@ import { } from '../../utils/image-analysis'; import { appendBrowserToolArgs, + type BrowserLaunchOverride, ensureBrowserMcpOrThrow, getBlockedBrowserOverrideWarning, getEffectiveClaudeBrowserAttachConfig, @@ -248,15 +249,23 @@ export async function execClaudeWithCLIProxy( } : undefined, }); - const browserLaunchFlags = resolveBrowserLaunchFlagResolution(argsWithoutProxy); - const browserLaunchOverride = browserLaunchFlags.override; - const argsWithoutBrowserFlags = browserLaunchFlags.argsWithoutFlags; + let browserLaunchOverride: BrowserLaunchOverride | undefined; + let argsWithoutBrowserFlags = argsWithoutProxy; + try { + const browserLaunchFlags = resolveBrowserLaunchFlagResolution(argsWithoutProxy); + browserLaunchOverride = browserLaunchFlags.override; + argsWithoutBrowserFlags = browserLaunchFlags.argsWithoutFlags; + } catch (error) { + console.error(fail((error as Error).message)); + process.exit(1); + return; + } const browserConfig = getBrowserConfig(); const browserAttachConfig = getEffectiveClaudeBrowserAttachConfig(browserConfig); const claudeBrowserExposure = resolveBrowserExposure( { enabled: browserAttachConfig.enabled, - policy: browserAttachConfig.overrideActive ? 'auto' : browserConfig.claude.policy, + policy: browserConfig.claude.policy, }, browserLaunchOverride ); diff --git a/src/commands/command-catalog.ts b/src/commands/command-catalog.ts index 6707867c..a1ad558f 100644 --- a/src/commands/command-catalog.ts +++ b/src/commands/command-catalog.ts @@ -305,6 +305,8 @@ export const PROVIDER_FLAGS = [ '--effort', '--1m', '--no-1m', + '--browser', + '--no-browser', '--logout', '--headless', '--port-forward', diff --git a/src/commands/completion-backend.ts b/src/commands/completion-backend.ts index 25e05eee..c968c720 100644 --- a/src/commands/completion-backend.ts +++ b/src/commands/completion-backend.ts @@ -84,6 +84,15 @@ function completeSubcommands( return uniqueStrings([...values, ...flags]).map((value) => suggestion(value)); } +function completeBrowserPolicyArgs(tokensBeforeCurrent: string[]): CompletionSuggestion[] { + const lastToken = tokensBeforeCurrent[tokensBeforeCurrent.length - 1]; + if (lastToken === '--all' || lastToken === '--claude' || lastToken === '--codex') { + return completeSubcommands(['auto', 'manual'], ['--help', '-h']); + } + + return completeSubcommands(['--all', '--claude', '--codex'], ['--help', '-h']); +} + function getSuggestionsForCommand(tokensBeforeCurrent: string[]): CompletionSuggestion[] { const [command, subcommand] = tokensBeforeCurrent; const lastToken = tokensBeforeCurrent[tokensBeforeCurrent.length - 1]; @@ -195,6 +204,12 @@ function getSuggestionsForCommand(tokensBeforeCurrent: string[]): CompletionSugg ['--help', '-h'] ); } + if (subcommand === 'enable' || subcommand === 'disable') { + return completeSubcommands(['claude', 'codex', 'all'], ['--help', '-h']); + } + if (subcommand === 'policy') { + return completeBrowserPolicyArgs(tokensBeforeCurrent); + } if (subcommand === 'setup' || subcommand === 'doctor') { return completeSubcommands([], ['--help', '-h']); } diff --git a/src/utils/browser/browser-policy.ts b/src/utils/browser/browser-policy.ts index eec51409..61a20196 100644 --- a/src/utils/browser/browser-policy.ts +++ b/src/utils/browser/browser-policy.ts @@ -22,7 +22,14 @@ export function resolveBrowserLaunchFlagResolution(args: string[]): BrowserLaunc let override: BrowserLaunchOverride | undefined; const argsWithoutFlags: string[] = []; - for (const arg of args) { + for (let index = 0; index < args.length; index++) { + const arg = args[index]; + + if (arg === '--') { + argsWithoutFlags.push(...args.slice(index)); + break; + } + if (arg === ENABLE_BROWSER_FLAG) { if (override === 'force-disable') { throw new Error('Use either `--browser` or `--no-browser`, not both.'); diff --git a/src/utils/browser/browser-status.ts b/src/utils/browser/browser-status.ts index 537d5a1b..c8adebfc 100644 --- a/src/utils/browser/browser-status.ts +++ b/src/utils/browser/browser-status.ts @@ -65,7 +65,6 @@ async function buildClaudeBrowserStatus( ): Promise { const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig); const launchCommands = buildBrowserLaunchCommands(effective.userDataDir, effective.devtoolsPort); - const managedBootstrap = ensureManagedBrowserUserDataDir(effective); const base: Omit = { enabled: effective.enabled, policy: browserConfig.claude.policy, @@ -90,6 +89,8 @@ async function buildClaudeBrowserStatus( }; } + const managedBootstrap = ensureManagedBrowserUserDataDir(effective); + if (managedBootstrap.createdProfileDir) { const managedMessage = describeManagedBrowserAttachNotReady( effective, diff --git a/src/web-server/routes/browser-routes.ts b/src/web-server/routes/browser-routes.ts index 6fb05425..e23e8910 100644 --- a/src/web-server/routes/browser-routes.ts +++ b/src/web-server/routes/browser-routes.ts @@ -20,6 +20,10 @@ interface BrowserRouteBody { }; } +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + function isValidBrowserPolicy(value: string): value is 'auto' | 'manual' { return value === 'auto' || value === 'manual'; } @@ -56,25 +60,49 @@ router.get('/status', async (_req: Request, res: Response): Promise => { }); router.put('/', async (req: Request, res: Response): Promise => { - if ( - req.body === null || - req.body === undefined || - typeof req.body !== 'object' || - Array.isArray(req.body) - ) { + if (!isPlainObject(req.body)) { res.status(400).json({ error: 'Invalid request body. Must be an object.' }); return; } - const { claude, codex } = req.body as BrowserRouteBody; - if (claude && (typeof claude !== 'object' || Array.isArray(claude))) { + const body = req.body as Record; + const rootKeys = Object.keys(body); + const unknownRootKeys = rootKeys.filter((key) => key !== 'claude' && key !== 'codex'); + if (unknownRootKeys.length > 0) { + res.status(400).json({ + error: `Unknown browser config field(s): ${unknownRootKeys.join(', ')}.`, + }); + return; + } + + const { claude, codex } = body as BrowserRouteBody; + if (Object.prototype.hasOwnProperty.call(body, 'claude') && !isPlainObject(claude)) { res.status(400).json({ error: 'Invalid value for claude. Must be an object.' }); return; } - if (codex && (typeof codex !== 'object' || Array.isArray(codex))) { + if (Object.prototype.hasOwnProperty.call(body, 'codex') && !isPlainObject(codex)) { res.status(400).json({ error: 'Invalid value for codex. Must be an object.' }); return; } + const unknownClaudeKeys = Object.keys(claude ?? {}).filter( + (key) => + key !== 'enabled' && key !== 'policy' && key !== 'userDataDir' && key !== 'devtoolsPort' + ); + if (unknownClaudeKeys.length > 0) { + res.status(400).json({ + error: `Unknown claude browser field(s): ${unknownClaudeKeys.join(', ')}.`, + }); + return; + } + const unknownCodexKeys = Object.keys(codex ?? {}).filter( + (key) => key !== 'enabled' && key !== 'policy' + ); + if (unknownCodexKeys.length > 0) { + res.status(400).json({ + error: `Unknown codex browser field(s): ${unknownCodexKeys.join(', ')}.`, + }); + return; + } if (claude?.enabled !== undefined && typeof claude.enabled !== 'boolean') { res.status(400).json({ error: 'Invalid value for claude.enabled. Must be a boolean.' }); return; diff --git a/tests/unit/cliproxy/executor-option-value.test.ts b/tests/unit/cliproxy/executor-option-value.test.ts index b6ff278e..d8259047 100644 --- a/tests/unit/cliproxy/executor-option-value.test.ts +++ b/tests/unit/cliproxy/executor-option-value.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, jest } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; import { + execClaudeWithCLIProxy, hasGitLabTokenLoginFlag, readOptionValue, } from '../../../src/cliproxy/executor/index'; @@ -40,3 +44,44 @@ describe('readOptionValue', () => { expect(hasGitLabTokenLoginFlag(['--gitlab-url', 'https://gitlab.example.com'])).toBe(false); }); }); + +describe('execClaudeWithCLIProxy browser flag validation', () => { + let tmpHome = ''; + let fakeClaudePath = ''; + let originalCcsHome: string | undefined; + + beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-cliproxy-executor-')); + fakeClaudePath = path.join(tmpHome, 'fake-claude.sh'); + fs.writeFileSync(fakeClaudePath, '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + fs.chmodSync(fakeClaudePath, 0o755); + originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + }); + + afterEach(() => { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + fs.rmSync(tmpHome, { recursive: true, force: true }); + }); + + it('exits cleanly when conflicting browser launch flags are provided', async () => { + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => undefined as never) as typeof process.exit); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + try { + await execClaudeWithCLIProxy(fakeClaudePath, 'gemini', ['--browser', '--no-browser'], {}); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith('[X] Use either `--browser` or `--no-browser`, not both.'); + } finally { + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); +}); diff --git a/tests/unit/commands/completion-backend.test.ts b/tests/unit/commands/completion-backend.test.ts index a86d9628..b683043e 100644 --- a/tests/unit/commands/completion-backend.test.ts +++ b/tests/unit/commands/completion-backend.test.ts @@ -148,6 +148,18 @@ describe('completion backend', () => { expect(suggestionValues(['browser', 'doctor'])).toEqual( expect.arrayContaining(['--help', '-h']) ); + expect(suggestionValues(['browser', 'enable'])).toEqual( + expect.arrayContaining(['claude', 'codex', 'all']) + ); + expect(suggestionValues(['browser', 'policy'])).toEqual( + expect.arrayContaining(['--all', '--claude', '--codex']) + ); + expect(suggestionValues(['browser', 'policy', '--all'])).toEqual( + expect.arrayContaining(['auto', 'manual']) + ); + expect(suggestionValues(['codex'])).toEqual( + expect.arrayContaining(['--browser', '--no-browser']) + ); }); test('treats cursor as a provider shortcut in completion', () => { diff --git a/tests/unit/targets/codex-runtime-integration.test.ts b/tests/unit/targets/codex-runtime-integration.test.ts index 9219ec83..40a70d0d 100644 --- a/tests/unit/targets/codex-runtime-integration.test.ts +++ b/tests/unit/targets/codex-runtime-integration.test.ts @@ -396,6 +396,24 @@ process.exit(0); }); } + it('strips browser launch flags before native codex passthrough diagnostics', () => { + if (process.platform === 'win32') return; + + const result = runCodexAlias(['--version', '--browser'], { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CODEX_PATH: fakeCodexPath, + CCS_TEST_CODEX_ARGS_OUT: codexArgsLogPath, + CCS_TEST_CODEX_VERSION: 'codex-cli 9.9.9-test', + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('codex-cli 9.9.9-test'); + expect(readLoggedCodexCalls(codexArgsLogPath)).toEqual([['--version']]); + }); + for (const helpFlag of ['--help', '-h']) { it(`passes ccsx ${helpFlag} straight through to the native Codex binary`, () => { if (process.platform === 'win32') return; diff --git a/tests/unit/targets/default-profile-browser-launch.test.ts b/tests/unit/targets/default-profile-browser-launch.test.ts index a5ceeddc..ae10e829 100644 --- a/tests/unit/targets/default-profile-browser-launch.test.ts +++ b/tests/unit/targets/default-profile-browser-launch.test.ts @@ -431,4 +431,86 @@ server.listen(0, '127.0.0.1', () => { } } }); + + it('keeps Claude browser attach hidden under manual policy until --browser is passed', async () => { + if (process.platform === 'win32') return; + + const mockServerScriptPath = path.join(tmpHome, 'mock-devtools-server-manual-default.js'); + const mockServerPortPath = path.join(tmpHome, 'mock-devtools-port-manual-default.txt'); + fs.writeFileSync( + mockServerScriptPath, + `const { createServer } = require('http'); +const fs = require('fs'); +const server = createServer((req, res) => { + if (req.url === '/json/version') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ Browser: 'Chrome/136.0.0.0', webSocketDebuggerUrl: 'ws://127.0.0.1/devtools/browser/manual-default-target' })); + return; + } + res.writeHead(404); + res.end('not found'); +}); +server.listen(0, '127.0.0.1', () => { + const address = server.address(); + fs.writeFileSync(${JSON.stringify(mockServerPortPath)}, String(address.port), 'utf8'); +}); +`, + 'utf8' + ); + + devtoolsServer = spawn(process.execPath, [mockServerScriptPath], { + stdio: 'ignore', + env: baseEnv, + }); + + const port = await waitForMockDevtoolsPort(mockServerPortPath); + await waitForDevtoolsVersionEndpoint(port); + + fs.mkdirSync(browserProfileDir, { recursive: true }); + fs.writeFileSync( + path.join(browserProfileDir, 'DevToolsActivePort'), + `${port}\n/devtools/browser/manual-default-target`, + 'utf8' + ); + + const originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + + try { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: true, + policy: 'manual', + user_data_dir: browserProfileDir, + devtools_port: Number.parseInt(port, 10), + }, + codex: { + enabled: true, + policy: 'auto', + }, + }; + }); + + const hiddenResult = runCcs(['default', 'smoke'], { + ...baseEnv, + }); + expect(hiddenResult.status).toBe(0); + expect(fs.readFileSync(claudeArgsLogPath, 'utf8')).not.toContain(BROWSER_PROMPT_SNIPPET); + expect(fs.readFileSync(claudeEnvLogPath, 'utf8')).not.toContain(browserProfileDir); + + const forcedResult = runCcs(['default', '--browser', 'smoke'], { + ...baseEnv, + }); + expect(forcedResult.status).toBe(0); + expect(fs.readFileSync(claudeArgsLogPath, 'utf8')).toContain(BROWSER_PROMPT_SNIPPET); + expect(fs.readFileSync(claudeEnvLogPath, 'utf8')).toContain(browserProfileDir); + } finally { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + } + }); }); diff --git a/tests/unit/targets/settings-profile-browser-launch.test.ts b/tests/unit/targets/settings-profile-browser-launch.test.ts index ff23bac8..bb3bb140 100644 --- a/tests/unit/targets/settings-profile-browser-launch.test.ts +++ b/tests/unit/targets/settings-profile-browser-launch.test.ts @@ -389,4 +389,92 @@ server.listen(0, '127.0.0.1', () => { } } }); + + it('keeps settings-profile Claude attach hidden under manual policy until --browser is passed', async () => { + if (process.platform === 'win32') return; + + const mockServerScriptPath = path.join(tmpHome, 'mock-devtools-server-manual-settings.js'); + const mockServerPortPath = path.join(tmpHome, 'mock-devtools-port-manual-settings.txt'); + fs.writeFileSync( + mockServerScriptPath, + `const { createServer } = require('http'); +const fs = require('fs'); +const server = createServer((req, res) => { + if (req.url === '/json/version') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ Browser: 'Chrome/136.0.0.0', webSocketDebuggerUrl: 'ws://127.0.0.1/devtools/browser/manual-settings-target' })); + return; + } + res.writeHead(404); + res.end('not found'); +}); +server.listen(0, '127.0.0.1', () => { + const address = server.address(); + fs.writeFileSync(${JSON.stringify(mockServerPortPath)}, String(address.port), 'utf8'); +}); +`, + 'utf8' + ); + + devtoolsServer = spawn(process.execPath, [mockServerScriptPath], { + stdio: 'ignore', + env: baseEnv, + }); + + const startDeadline = Date.now() + 5000; + while (!fs.existsSync(mockServerPortPath)) { + if (Date.now() > startDeadline) { + throw new Error('Timed out waiting for mock DevTools server to start'); + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + const port = fs.readFileSync(mockServerPortPath, 'utf8').trim(); + + fs.mkdirSync(browserProfileDir, { recursive: true }); + fs.writeFileSync( + path.join(browserProfileDir, 'DevToolsActivePort'), + `${port}\n/devtools/browser/manual-settings-target`, + 'utf8' + ); + + const originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + + try { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: true, + policy: 'manual', + user_data_dir: browserProfileDir, + devtools_port: Number.parseInt(port, 10), + }, + codex: { + enabled: true, + policy: 'auto', + }, + }; + }); + + const hiddenResult = runCcs(['glm', 'smoke'], { + ...baseEnv, + }); + expect(hiddenResult.status).toBe(0); + expect(fs.readFileSync(claudeArgsLogPath, 'utf8')).not.toContain(BROWSER_PROMPT_SNIPPET); + expect(fs.readFileSync(claudeEnvLogPath, 'utf8')).not.toContain(browserProfileDir); + + const forcedResult = runCcs(['glm', '--browser', 'smoke'], { + ...baseEnv, + }); + expect(forcedResult.status).toBe(0); + expect(fs.readFileSync(claudeArgsLogPath, 'utf8')).toContain(BROWSER_PROMPT_SNIPPET); + expect(fs.readFileSync(claudeEnvLogPath, 'utf8')).toContain(browserProfileDir); + } finally { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + } + }); }); diff --git a/tests/unit/utils/browser/browser-policy.test.ts b/tests/unit/utils/browser/browser-policy.test.ts index cf2c1b8d..03e35e19 100644 --- a/tests/unit/utils/browser/browser-policy.test.ts +++ b/tests/unit/utils/browser/browser-policy.test.ts @@ -22,6 +22,13 @@ describe('browser policy', () => { ); }); + it('stops parsing browser launch flags after the option terminator', () => { + expect(resolveBrowserLaunchFlagResolution(['glm', '--', '--browser', 'literal'])).toEqual({ + override: undefined, + argsWithoutFlags: ['glm', '--', '--browser', 'literal'], + }); + }); + it('resolves browser exposure from saved policy and one-run overrides', () => { expect(resolveBrowserExposure({ enabled: true, policy: 'auto' })).toMatchObject({ exposeByDefault: true, diff --git a/tests/unit/utils/browser/browser-status.test.ts b/tests/unit/utils/browser/browser-status.test.ts index 5ddceeaf..bda30626 100644 --- a/tests/unit/utils/browser/browser-status.test.ts +++ b/tests/unit/utils/browser/browser-status.test.ts @@ -88,6 +88,7 @@ describe('browser status', () => { serverName: 'ccs_browser', supportsConfigOverrides: true, }); + expect(existsSync(join(tempHome, '.ccs', 'browser', 'chrome-user-data'))).toBe(false); } finally { codexSpy.mockRestore(); } diff --git a/tests/unit/web-server/browser-routes.test.ts b/tests/unit/web-server/browser-routes.test.ts index e54e264c..11db1b5e 100644 --- a/tests/unit/web-server/browser-routes.test.ts +++ b/tests/unit/web-server/browser-routes.test.ts @@ -226,6 +226,38 @@ describe('browser routes', () => { }); }); + it('rejects null browser lane payloads instead of treating them as no-ops', async () => { + const response = await fetch(`${baseUrl}/api/browser`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + claude: null, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: 'Invalid value for claude. Must be an object.', + }); + }); + + it('rejects unknown browser config fields instead of silently ignoring them', async () => { + const response = await fetch(`${baseUrl}/api/browser`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + codxe: { + enabled: true, + }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: 'Unknown browser config field(s): codxe.', + }); + }); + it('rejects invalid browser policy values at the route boundary', async () => { const response = await fetch(`${baseUrl}/api/browser`, { method: 'PUT', @@ -242,4 +274,21 @@ describe('browser routes', () => { error: 'Invalid value for codex.policy. Must be auto or manual.', }); }); + + it('rejects unknown nested browser lane fields instead of silently ignoring them', async () => { + const response = await fetch(`${baseUrl}/api/browser`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + claude: { + userDatDir: '/tmp/typo', + }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: 'Unknown claude browser field(s): userDatDir.', + }); + }); }); From 6604357b2262e06502be49f4678063b887404ce2 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 20 Apr 2026 21:15:31 -0400 Subject: [PATCH 3/3] fix(browser): default browser tooling to manual opt-in --- docs/browser-automation.md | 26 +++-- docs/project-roadmap.md | 1 + src/commands/browser-command.ts | 19 +++- src/config/unified-config-loader.ts | 2 +- src/config/unified-config-types.ts | 11 +- src/utils/browser/browser-status.ts | 80 ++++++++++--- src/web-server/routes/browser-routes.ts | 12 +- tests/unit/commands/browser-command.test.ts | 42 ++++++- tests/unit/config/migration-manager.test.ts | 33 ++++++ .../targets/codex-runtime-integration.test.ts | 107 ++++++++++-------- .../codex-settings-bridge-launch.test.ts | 6 +- .../default-profile-browser-launch.test.ts | 14 +-- .../settings-profile-browser-launch.test.ts | 62 ++++++++-- tests/unit/unified-config.test.ts | 98 ++++++++++++++++ .../unit/utils/browser/browser-status.test.ts | 58 +++++++++- tests/unit/web-server/browser-routes.test.ts | 12 +- 16 files changed, 465 insertions(+), 118 deletions(-) diff --git a/docs/browser-automation.md b/docs/browser-automation.md index f4610195..a8ceb4ad 100644 --- a/docs/browser-automation.md +++ b/docs/browser-automation.md @@ -8,6 +8,8 @@ CCS provides browser automation through two separate runtime paths: - **Codex Browser Tools**: injects Playwright MCP tooling into Codex-target launches These are related, but they are not the same implementation and they do not promise a shared browser session. +On new installs, and on upgrades that do not already have explicit browser settings, both lanes +start **disabled** and **manual** so browser tooling is not auto-exposed until you opt in. ## How Browser Automation Works @@ -48,7 +50,8 @@ The Browser screen exposes two sections: Browser policy controls are CLI-first in this release. The dashboard remains the shared setup and status surface, while `ccs browser policy` is the authoritative place to decide whether browser -tooling is auto-exposed or kept manual by default. +tooling is auto-exposed or kept manual by default. Fresh installs, plus upgrades without an +existing browser section, surface both lanes as off/manual until you explicitly enable them. ### Via CLI @@ -63,7 +66,8 @@ ccs browser policy --all manual Use `ccs browser setup` for the primary one-command setup path. Use `ccs browser status` for the current state, `ccs browser doctor` for read-only troubleshooting guidance, and -`ccs browser policy` to control default browser exposure. +`ccs browser policy` to control default browser exposure. If you only want browser access for one +run, keep policy manual and add `--browser` to that launch. ### Via Config File @@ -73,12 +77,12 @@ Edit `~/.ccs/config.yaml`: browser: claude: enabled: false - policy: auto + policy: manual user_data_dir: "~/.ccs/browser/chrome-user-data" devtools_port: 9222 codex: - enabled: true - policy: auto + enabled: false + policy: manual ``` Notes: @@ -87,6 +91,7 @@ Notes: - `claude.user_data_dir` is a **Chrome user-data directory**, not a display-name browser profile - `claude.devtools_port` is the expected remote debugging port for attach mode - `codex.enabled` controls whether CCS injects browser tooling into Codex-target launches +- New installs, plus upgrades without saved browser settings, default both lanes to `enabled: false` and `policy: manual` - `manual` keeps the lane configured but hidden until a launch explicitly opts in with `--browser` ## Runtime Policy Controls @@ -94,7 +99,7 @@ Notes: CCS now separates **lane enablement** from **default exposure policy**: - `enabled: false` - - the lane is off + - the lane is off; this is the default for both lanes on new installs and upgrades without saved browser settings - `enabled: true` + `policy: auto` - the lane is exposed automatically on matching launches - `enabled: true` + `policy: manual` @@ -159,10 +164,11 @@ ccs browser setup That flow: 1. enables Claude Browser Attach in the saved CCS browser config -2. keeps the configured DevTools port normalized -3. creates the configured browser user-data directory if needed -4. prints the exact browser launch command for the current platform -5. re-checks readiness and reports the next step if Chrome still needs manual attention +2. leaves launch exposure under the saved policy, so `policy: manual` still requires `--browser` +3. keeps the configured DevTools port normalized +4. creates the configured browser user-data directory if needed +5. prints the exact browser launch command for the current platform +6. re-checks readiness and reports the next step if Chrome still needs manual attention ## Launching Chrome For Claude Attach diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index c8457946..a08866ec 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -41,6 +41,7 @@ All major modularization work is complete. The codebase evolved from monolithic ### Recent Fixes +- **2026-04-20**: **#1051** Browser automation now defaults safe-off for new installs and upgrades that do not already carry explicit browser settings. CCS changes both Claude Browser Attach and Codex Browser Tools to start with `enabled: false` and `policy: manual`, normalizes missing browser policies on upgrade back to `manual`, preserves explicit existing enablement, and updates status/help/docs so browser tooling is never implied to auto-expose unless users opt in. - **2026-04-19**: **#1051** Browser tooling now has an explicit exposure policy instead of only coarse enablement toggles. CCS adds `browser..policy` (`auto` or `manual`) for both Claude Browser Attach and Codex Browser Tools, exposes CLI-first policy controls through `ccs browser policy`, `ccs browser enable`, and `ccs browser disable`, and adds one-run launch overrides `--browser` and `--no-browser` so users can force browser tooling on or off without editing saved config. - **2026-04-19**: **#1049** Browser setup now has a real remediation path instead of status/doctor-only guidance. CCS adds `ccs browser setup` as the primary one-command flow for Claude Browser Attach, shortens managed browser-path output to home-relative display paths where appropriate, and updates browser readiness guidance to point users at setup first while keeping browser doctor read-only by default. - **2026-04-18**: **#1038** Legacy OpenAI-compatible provider writes no longer self-destruct on the next `ccs cliproxy restart`. CCS now preserves AI-provider-managed top-level sections such as `openai-compatibility` during CLIProxy config regeneration, and the legacy `openai-compat` manager now rewrites only its own YAML section instead of dumping the whole file and stripping the generated version header. Regression coverage now proves the legacy helper keeps the generated header intact and that OpenAI-compatible connectors survive regeneration. diff --git a/src/commands/browser-command.ts b/src/commands/browser-command.ts index 2536c496..3633f67e 100644 --- a/src/commands/browser-command.ts +++ b/src/commands/browser-command.ts @@ -48,13 +48,13 @@ function writeCommandTable(writeLine: HelpWriter): void { ` ${color('ccs browser doctor', 'command')} Explain what is missing and how to fix it` ); writeLine( - ` ${color('ccs browser policy', 'command')} Show the saved browser exposure policy` + ` ${color('ccs browser policy', 'command')} Show the saved browser exposure policy and safe defaults` ); writeLine( ` ${color('ccs browser policy --all manual', 'command')} Keep browser tooling hidden unless a launch uses --browser` ); writeLine( - ` ${color('ccs browser enable ', 'command')} Turn a browser lane on` + ` ${color('ccs browser enable ', 'command')} Turn a browser lane on without forcing auto-exposure` ); writeLine( ` ${color('ccs browser disable ', 'command')} Turn a browser lane off` @@ -67,6 +67,9 @@ function writeIntro(writeLine: HelpWriter): void { writeLine( ' Codex Browser Tools inject managed Playwright MCP overrides into Codex-target launches.' ); + writeLine( + ' New installs, plus upgrades without saved browser settings, keep both lanes off by default; enable a lane and use `--browser` when you want browser access.' + ); writeLine(''); writeLine(subheader('Launch Overrides')); writeLine( @@ -158,6 +161,10 @@ function writePolicySummary(writeLine: HelpWriter): void { writeLine(header('ccs browser policy')); writeLine(''); writeIntro(writeLine); + writeLine( + ' New installs and upgrades without saved browser settings: both lanes start disabled and manual.' + ); + writeLine(''); writeLine(subheader('Claude Browser Attach')); writeLine(` Enabled: ${config.claude.enabled ? 'yes' : 'no'}`); writeLaunchPolicy(config.claude.policy, writeLine); @@ -191,6 +198,11 @@ function writeToggleSummary( writeLine(''); writeLine(` Updated ${lane} browser lane${lane === 'all' ? 's' : ''}.`); writeLine(` Browser lanes are now ${verb} as requested.`); + if (subcommand === 'enable') { + writeLine( + ' Enabled lanes still respect policy, so browser access stays hidden until `--browser` while policy is manual.' + ); + } writeLine(''); writeLine(subheader('Current State')); writeLine(` Claude enabled: ${config.claude.enabled ? 'yes' : 'no'}`); @@ -296,6 +308,9 @@ export async function showBrowserHelp(writeLine: HelpWriter = console.log): Prom writeLine(subheader('What Each Lane Does')); writeLine(' Claude Browser Attach expects a Chrome user-data dir and remote debugging port.'); writeLine(' Codex Browser Tools depend on a Codex build that supports --config overrides.'); + writeLine( + ' New installs and upgrades without saved browser settings keep both lanes off by default, so enabling a lane does not auto-expose browser tooling unless policy is set to auto.' + ); writeLine(''); writeLine(subheader('Examples')); writeLine( diff --git a/src/config/unified-config-loader.ts b/src/config/unified-config-loader.ts index cd95335e..a3548fc6 100644 --- a/src/config/unified-config-loader.ts +++ b/src/config/unified-config-loader.ts @@ -73,7 +73,7 @@ function normalizeBrowserDevtoolsPort(value: number | undefined): number { } function normalizeBrowserPolicy(value: string | undefined): BrowserToolPolicy { - return value === 'manual' ? 'manual' : DEFAULT_BROWSER_CONFIG.claude.policy; + return value === 'auto' || value === 'manual' ? value : DEFAULT_BROWSER_CONFIG.claude.policy; } function canonicalizeBrowserConfig(config?: BrowserConfig): BrowserConfig { diff --git a/src/config/unified-config-types.ts b/src/config/unified-config-types.ts index 3ead57cf..6b294857 100644 --- a/src/config/unified-config-types.ts +++ b/src/config/unified-config-types.ts @@ -26,8 +26,9 @@ import { CLIPROXY_PROVIDER_IDS } from '../cliproxy/provider-capabilities'; * Version 10 = Exa + Tavily WebSearch backends * Version 11 = Discord Channels runtime auto-enable preferences * Version 12 = Official Channels multi-provider support (Telegram, Discord, iMessage) + * Version 13 = Browser automation defaults to safe manual/off exposure */ -export const UNIFIED_CONFIG_VERSION = 12; +export const UNIFIED_CONFIG_VERSION = 13; /** * Supported CLIProxy providers. @@ -834,7 +835,7 @@ export interface BrowserClaudeConfig { } export interface BrowserCodexConfig { - /** Enable Codex browser tooling injection (default: true) */ + /** Enable Codex browser tooling injection (default: false) */ enabled: boolean; /** Control whether Codex browser tooling is exposed automatically or only via --browser */ policy: BrowserToolPolicy; @@ -848,13 +849,13 @@ export interface BrowserConfig { export const DEFAULT_BROWSER_CONFIG: BrowserConfig = { claude: { enabled: false, - policy: 'auto', + policy: 'manual', user_data_dir: '', devtools_port: 9222, }, codex: { - enabled: true, - policy: 'auto', + enabled: false, + policy: 'manual', }, }; diff --git a/src/utils/browser/browser-status.ts b/src/utils/browser/browser-status.ts index c8adebfc..42b72222 100644 --- a/src/utils/browser/browser-status.ts +++ b/src/utils/browser/browser-status.ts @@ -1,6 +1,6 @@ import * as path from 'path'; -import type { BrowserToolPolicy } from '../../config/unified-config-types'; -import { getBrowserConfig } from '../../config/unified-config-loader'; +import type { BrowserConfig, BrowserToolPolicy } from '../../config/unified-config-types'; +import { getBrowserConfig, loadUnifiedConfig } from '../../config/unified-config-loader'; import { getCcsPathDisplay } from '../config-manager'; import { getCodexBinaryInfo } from '../../targets/codex-detector'; import { type BrowserRuntimeEnv, resolveBrowserRuntimeEnv } from './chrome-reuse'; @@ -53,15 +53,57 @@ export interface BrowserStatusPayload { } export async function getBrowserStatus(): Promise { - const browserConfig = getBrowserConfig(); + const browserConfig = getUserFacingBrowserConfig(); return { claude: await buildClaudeBrowserStatus(browserConfig), codex: buildCodexBrowserStatus(browserConfig), }; } +type PersistedBrowserConfig = { + claude?: Partial; + codex?: Partial; +}; + +function resolveSafeBrowserPolicy(policy: BrowserToolPolicy | undefined): BrowserToolPolicy { + return policy === 'auto' || policy === 'manual' ? policy : 'manual'; +} + +export function getUserFacingBrowserConfig(): BrowserConfig { + const canonical = getBrowserConfig(); + const persisted = loadUnifiedConfig()?.browser as PersistedBrowserConfig | undefined; + + if (!persisted) { + return { + claude: { + ...canonical.claude, + enabled: false, + policy: 'manual', + }, + codex: { + ...canonical.codex, + enabled: false, + policy: 'manual', + }, + }; + } + + return { + claude: { + ...canonical.claude, + enabled: persisted.claude?.enabled ?? false, + policy: resolveSafeBrowserPolicy(persisted.claude?.policy), + }, + codex: { + ...canonical.codex, + enabled: persisted.codex?.enabled ?? false, + policy: resolveSafeBrowserPolicy(persisted.codex?.policy), + }, + }; +} + async function buildClaudeBrowserStatus( - browserConfig = getBrowserConfig() + browserConfig = getUserFacingBrowserConfig() ): Promise { const effective = getEffectiveClaudeBrowserAttachConfig(browserConfig); const launchCommands = buildBrowserLaunchCommands(effective.userDataDir, effective.devtoolsPort); @@ -84,8 +126,8 @@ async function buildClaudeBrowserStatus( state: 'disabled', title: 'Claude Browser Attach is disabled.', detail: - 'CCS will not provision the managed browser MCP runtime for Claude launches until this lane is enabled.', - nextStep: `Enable Claude Browser Attach in Settings > Browser or in ${getCcsPathDisplay('config.yaml')}, then run \`ccs browser setup\`.`, + 'CCS keeps Claude Browser Attach off by default and will not provision the managed browser MCP runtime until this lane is enabled.', + nextStep: `Enable Claude Browser Attach in Settings > Browser or in ${getCcsPathDisplay('config.yaml')}, then run \`ccs browser setup\` when you are ready to opt in.`, }; } @@ -122,8 +164,13 @@ async function buildClaudeBrowserStatus( state: 'ready', title: 'Claude Browser Attach is ready.', detail: - 'CCS can reach the configured Chrome DevTools endpoint for the current attach session.', - nextStep: 'Launch a Claude-target CCS session to use the managed browser MCP runtime.', + browserConfig.claude.policy === 'manual' + ? 'CCS can reach the configured Chrome DevTools endpoint, and the lane stays hidden until a launch uses `--browser`.' + : 'CCS can reach the configured Chrome DevTools endpoint for the current attach session.', + nextStep: + browserConfig.claude.policy === 'manual' + ? 'Launch a Claude-target CCS session with `--browser` to use the managed browser MCP runtime.' + : 'Launch a Claude-target CCS session to use the managed browser MCP runtime.', runtimeEnv, }; } catch (error) { @@ -172,16 +219,17 @@ async function buildClaudeBrowserStatus( } } -function buildCodexBrowserStatus(browserConfig = getBrowserConfig()): CodexBrowserStatus { +function buildCodexBrowserStatus(browserConfig = getUserFacingBrowserConfig()): CodexBrowserStatus { if (!browserConfig.codex.enabled) { return { enabled: false, policy: browserConfig.codex.policy, state: 'disabled', title: 'Codex Browser Tools are disabled.', - detail: 'CCS will not inject Playwright MCP browser tooling into Codex-target launches.', + detail: + 'CCS keeps Codex Browser Tools off by default and will not inject Playwright MCP browser tooling until this lane is enabled.', nextStep: - 'Enable Codex Browser Tools in Settings > Browser to restore the managed Codex browser path.', + 'Enable Codex Browser Tools in Settings > Browser when you want browser access on Codex-target launches.', serverName: 'ccs_browser', supportsConfigOverrides: false, binaryPath: null, @@ -212,8 +260,14 @@ function buildCodexBrowserStatus(browserConfig = getBrowserConfig()): CodexBrows policy: browserConfig.codex.policy, state: 'enabled', title: 'Codex Browser Tools are enabled.', - detail: 'CCS can inject the managed Playwright MCP overrides into Codex-target launches.', - nextStep: 'Use a Codex-target CCS launch to access browser tools.', + detail: + browserConfig.codex.policy === 'manual' + ? 'CCS can inject the managed Playwright MCP overrides when a Codex-target launch opts in with `--browser`.' + : 'CCS can inject the managed Playwright MCP overrides into Codex-target launches.', + nextStep: + browserConfig.codex.policy === 'manual' + ? 'Use `--browser` on a Codex-target CCS launch to access browser tools.' + : 'Use a Codex-target CCS launch to access browser tools.', serverName: 'ccs_browser', supportsConfigOverrides, binaryPath: binaryInfo.path, diff --git a/src/web-server/routes/browser-routes.ts b/src/web-server/routes/browser-routes.ts index e23e8910..215960fb 100644 --- a/src/web-server/routes/browser-routes.ts +++ b/src/web-server/routes/browser-routes.ts @@ -1,6 +1,8 @@ import { Router, type Request, type Response } from 'express'; -import { getBrowserConfig, mutateUnifiedConfig } from '../../config/unified-config-loader'; +import { mutateUnifiedConfig } from '../../config/unified-config-loader'; +import type { BrowserConfig } from '../../config/unified-config-types'; import { getBrowserStatus } from '../../utils/browser'; +import { getUserFacingBrowserConfig } from '../../utils/browser/browser-status'; import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); @@ -40,7 +42,7 @@ router.use((req: Request, res: Response, next) => { router.get('/', async (_req: Request, res: Response): Promise => { try { - const config = getBrowserConfig(); + const config = getUserFacingBrowserConfig(); const status = await getBrowserStatus(); res.json({ config: toBrowserRouteConfig(config), @@ -140,7 +142,7 @@ router.put('/', async (req: Request, res: Response): Promise => { } try { - const current = getBrowserConfig(); + const current = getUserFacingBrowserConfig(); const nextClaudeUserDataDir = claude?.userDataDir === undefined ? current.claude.user_data_dir : claude.userDataDir.trim(); mutateUnifiedConfig((config) => { @@ -158,7 +160,7 @@ router.put('/', async (req: Request, res: Response): Promise => { }; }); - const config = getBrowserConfig(); + const config = getUserFacingBrowserConfig(); const status = await getBrowserStatus(); res.json({ success: true, @@ -172,7 +174,7 @@ router.put('/', async (req: Request, res: Response): Promise => { } }); -function toBrowserRouteConfig(config: ReturnType) { +function toBrowserRouteConfig(config: BrowserConfig) { return { claude: { enabled: config.claude.enabled, diff --git a/tests/unit/commands/browser-command.test.ts b/tests/unit/commands/browser-command.test.ts index 809c9f6e..df83e075 100644 --- a/tests/unit/commands/browser-command.test.ts +++ b/tests/unit/commands/browser-command.test.ts @@ -47,7 +47,7 @@ describe('browser command', () => { const statusSpy = spyOn(browserUtils, 'getBrowserStatus').mockResolvedValue({ claude: { enabled: true, - policy: 'auto', + policy: 'manual', source: 'config', overrideActive: false, state: 'ready', @@ -74,7 +74,7 @@ describe('browser command', () => { }, codex: { enabled: true, - policy: 'auto', + policy: 'manual', state: 'enabled', title: 'Codex Browser Tools are enabled.', detail: 'CCS can inject the managed Playwright MCP overrides.', @@ -94,9 +94,15 @@ describe('browser command', () => { expect(rendered.includes('Codex Browser Tools inject managed Playwright MCP overrides')).toBe( true ); + expect( + rendered.includes( + 'New installs, plus upgrades without saved browser settings, keep both lanes off by default' + ) + ).toBe(true); expect(rendered.includes('Managed MCP: ccs-browser')).toBe(true); expect(rendered.includes('Managed server: ccs_browser')).toBe(true); - expect(rendered.includes('Policy: auto')).toBe(true); + expect(rendered.includes('Policy: manual')).toBe(true); + expect(rendered.includes('Default launch behavior: hidden until `--browser`')).toBe(true); expect(rendered.includes('DevTools endpoint: http://127.0.0.1:9222')).toBe(true); } finally { statusSpy.mockRestore(); @@ -288,18 +294,37 @@ describe('browser command', () => { const rendered = await renderLines(['policy', '--all', 'manual']); expect(rendered.includes('ccs browser policy')).toBe(true); + expect( + rendered.includes( + 'New installs and upgrades without saved browser settings: both lanes start disabled and manual.' + ) + ).toBe(true); expect(rendered.includes('Default launch behavior: hidden until `--browser`')).toBe(true); expect(getBrowserConfig().claude.policy).toBe('manual'); expect(getBrowserConfig().codex.policy).toBe('manual'); }); - test('enable updates a single browser lane', async () => { - await renderLines(['disable', 'codex']); - expect(getBrowserConfig().codex.enabled).toBe(false); + test('policy shows safe default-off browser settings on a fresh install', async () => { + const rendered = await renderLines(['policy']); + expect(rendered.includes('ccs browser policy')).toBe(true); + expect( + rendered.includes( + 'New installs and upgrades without saved browser settings: both lanes start disabled and manual.' + ) + ).toBe(true); + expect(rendered.includes('Claude Browser Attach')).toBe(true); + expect(rendered.includes('Codex Browser Tools')).toBe(true); + expect(rendered.includes('Enabled: no')).toBe(true); + expect(rendered.includes('Policy: manual')).toBe(true); + expect(rendered.includes('Default launch behavior: hidden until `--browser`')).toBe(true); + }); + + test('enable updates a single browser lane', async () => { const rendered = await renderLines(['enable', 'codex']); expect(rendered.includes('Updated codex browser lane.')).toBe(true); expect(getBrowserConfig().codex.enabled).toBe(true); + expect(getBrowserConfig().codex.policy).toBe('manual'); }); test('literal browser help still renders the help page', async () => { @@ -309,5 +334,10 @@ describe('browser command', () => { expect(rendered.includes('ccs browser setup')).toBe(true); expect(rendered.includes('ccs browser policy')).toBe(true); expect(rendered.includes('--browser')).toBe(true); + expect( + rendered.includes( + 'New installs and upgrades without saved browser settings keep both lanes off by default' + ) + ).toBe(true); }); }); diff --git a/tests/unit/config/migration-manager.test.ts b/tests/unit/config/migration-manager.test.ts index 92a8ec3a..90a3d460 100644 --- a/tests/unit/config/migration-manager.test.ts +++ b/tests/unit/config/migration-manager.test.ts @@ -177,6 +177,39 @@ describe('migration-manager legacy kimi compatibility', () => { expect(unified?.accounts.personal.continuity_mode).toBeUndefined(); }); + it('applies safe browser defaults when migrating legacy config files', async () => { + fs.writeFileSync( + path.join(ccsDir, 'config.json'), + JSON.stringify( + { + profiles: { + glm: '~/.ccs/glm.settings.json', + }, + }, + null, + 2 + ) + ); + fs.writeFileSync(path.join(ccsDir, 'glm.settings.json'), JSON.stringify({ env: {} })); + + const result = await migrate(false); + expect(result.success).toBe(true); + + const unified = loadUnifiedConfig(); + expect(unified?.browser).toEqual({ + claude: { + enabled: false, + policy: 'manual', + user_data_dir: '', + devtools_port: 9222, + }, + codex: { + enabled: false, + policy: 'manual', + }, + }); + }); + it('normalizes valid legacy shared groups and drops invalid ones during migration', async () => { fs.writeFileSync( path.join(ccsDir, 'profiles.json'), diff --git a/tests/unit/targets/codex-runtime-integration.test.ts b/tests/unit/targets/codex-runtime-integration.test.ts index 40a70d0d..833332d7 100644 --- a/tests/unit/targets/codex-runtime-integration.test.ts +++ b/tests/unit/targets/codex-runtime-integration.test.ts @@ -163,7 +163,7 @@ process.exit(0); fs.rmSync(tmpHome, { recursive: true, force: true }); }); - it('injects browser MCP runtime overrides for native Codex default launches', () => { + it('keeps browser MCP runtime overrides off for untouched Codex launches', () => { if (process.platform === 'win32') return; const result = runCcs(['default', '--target', 'codex', 'fix failing tests'], { @@ -178,20 +178,64 @@ process.exit(0); expect(result.status).toBe(0); const calls = readLoggedCodexCalls(codexArgsLogPath); - expect(calls).toEqual([ - ['-c', 'model="gpt-5"', '--version'], - [ - '-c', - `mcp_servers.ccs_browser.command=${JSON.stringify(process.platform === 'win32' ? 'npx.cmd' : 'npx')}`, - '-c', - `mcp_servers.ccs_browser.args=${JSON.stringify(['-y', '@playwright/mcp@0.0.70'])}`, - '-c', - 'mcp_servers.ccs_browser.enabled=true', - '-c', - 'mcp_servers.ccs_browser.tool_timeout_sec=30', - 'fix failing tests', - ], - ]); + expect(calls).toEqual([['fix failing tests']]); + }); + + it('injects browser MCP runtime overrides when Codex browser policy is explicitly auto-enabled', () => { + if (process.platform === 'win32') return; + + const originalCcsHome = process.env.CCS_HOME; + process.env.CCS_HOME = tmpHome; + + try { + mutateUnifiedConfig((config) => { + config.browser = { + claude: { + enabled: false, + policy: 'manual', + user_data_dir: '', + devtools_port: 9222, + }, + codex: { + enabled: true, + policy: 'auto', + }, + }; + }); + + const result = runCcs(['default', '--target', 'codex', 'fix failing tests'], { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CODEX_PATH: fakeCodexPath, + CCS_TEST_CODEX_ARGS_OUT: codexArgsLogPath, + CCS_THINKING: '8192', + }); + + expect(result.status).toBe(0); + const calls = readLoggedCodexCalls(codexArgsLogPath); + expect(calls).toEqual([ + ['-c', 'model="gpt-5"', '--version'], + [ + '-c', + `mcp_servers.ccs_browser.command=${JSON.stringify(process.platform === 'win32' ? 'npx.cmd' : 'npx')}`, + '-c', + `mcp_servers.ccs_browser.args=${JSON.stringify(['-y', '@playwright/mcp@0.0.70'])}`, + '-c', + 'mcp_servers.ccs_browser.enabled=true', + '-c', + 'mcp_servers.ccs_browser.tool_timeout_sec=30', + 'fix failing tests', + ], + ]); + } finally { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + } }); it('skips Codex browser MCP overrides when browser tooling is disabled in config', () => { @@ -345,7 +389,7 @@ process.exit(0); expect(calls).toEqual([['fix failing tests']]); }); - it('keeps browser MCP runtime overrides when CCS_THINKING is ignored for native Codex default mode', () => { + it('keeps browser MCP runtime overrides off when CCS_THINKING is ignored for native Codex default mode', () => { if (process.platform === 'win32') return; const result = runCcs(['default', '--target', 'codex', 'fix failing tests'], { @@ -360,20 +404,7 @@ process.exit(0); expect(result.status).toBe(0); const calls = readLoggedCodexCalls(codexArgsLogPath); - expect(calls).toEqual([ - ['-c', 'model="gpt-5"', '--version'], - [ - '-c', - `mcp_servers.ccs_browser.command=${JSON.stringify(process.platform === 'win32' ? 'npx.cmd' : 'npx')}`, - '-c', - `mcp_servers.ccs_browser.args=${JSON.stringify(['-y', '@playwright/mcp@0.0.70'])}`, - '-c', - 'mcp_servers.ccs_browser.enabled=true', - '-c', - 'mcp_servers.ccs_browser.tool_timeout_sec=30', - 'fix failing tests', - ], - ]); + expect(calls).toEqual([['fix failing tests']]); }); for (const versionFlag of ['--version', '-v']) { @@ -488,14 +519,6 @@ process.exit(0); expect(readLoggedCodexCalls(codexArgsLogPath)).toEqual([ ['-c', 'model="gpt-5"', '--version'], [ - '-c', - 'mcp_servers.ccs_browser.command="npx"', - '-c', - 'mcp_servers.ccs_browser.args=["-y","@playwright/mcp@0.0.70"]', - '-c', - 'mcp_servers.ccs_browser.enabled=true', - '-c', - 'mcp_servers.ccs_browser.tool_timeout_sec=30', '-c', 'model_reasoning_effort="high"', 'fix failing tests', @@ -690,14 +713,6 @@ process.exit(0); expect(readLoggedCodexCalls(codexArgsLogPath)).toEqual([ ['-c', 'model="gpt-5"', '--version'], [ - '-c', - 'mcp_servers.ccs_browser.command="npx"', - '-c', - 'mcp_servers.ccs_browser.args=["-y","@playwright/mcp@0.0.70"]', - '-c', - 'mcp_servers.ccs_browser.enabled=true', - '-c', - 'mcp_servers.ccs_browser.tool_timeout_sec=30', '-c', 'model_reasoning_effort="high"', 'fix failing tests', diff --git a/tests/unit/targets/codex-settings-bridge-launch.test.ts b/tests/unit/targets/codex-settings-bridge-launch.test.ts index edec0748..6bf1c75c 100644 --- a/tests/unit/targets/codex-settings-bridge-launch.test.ts +++ b/tests/unit/targets/codex-settings-bridge-launch.test.ts @@ -114,7 +114,7 @@ exit 0 fs.rmSync(tmpHome, { recursive: true, force: true }); }); - it('launches Codex bridge settings profiles and injects runtime overrides', () => { + it('launches Codex bridge settings profiles without browser overrides unless the lane is enabled', () => { if (process.platform === 'win32') return; const result = runCcs(['codex-api', '--target', 'codex', '--effort', 'high', 'smoke'], baseEnv); @@ -126,8 +126,8 @@ exit 0 expect(argsLog).toContain('model_provider="ccs_runtime"'); expect(argsLog).toContain('model_providers.ccs_runtime.base_url="http://127.0.0.1:8317/api/provider/codex"'); expect(argsLog).toContain('model_reasoning_effort="high"'); - expect(argsLog).toContain('mcp_servers.ccs_browser.command='); - expect(argsLog).toContain('mcp_servers.ccs_browser.args=["-y","@playwright/mcp@0.0.70"]'); + expect(argsLog).not.toContain('mcp_servers.ccs_browser.command='); + expect(argsLog).not.toContain('mcp_servers.ccs_browser.args=["-y","@playwright/mcp@0.0.70"]'); expect(argsLog).toContain('smoke'); expect(fs.readFileSync(codexEnvLogPath, 'utf8')).toBe('bridge-token'); }); diff --git a/tests/unit/targets/default-profile-browser-launch.test.ts b/tests/unit/targets/default-profile-browser-launch.test.ts index ae10e829..be81261b 100644 --- a/tests/unit/targets/default-profile-browser-launch.test.ts +++ b/tests/unit/targets/default-profile-browser-launch.test.ts @@ -198,7 +198,7 @@ exit 0 expect(launchedEnv).not.toContain('devtools/browser/stale-default'); }); - it('passes browser runtime env through default Claude launches when reuse is configured', async () => { + it('does not auto-enable browser runtime for default Claude launches from env overrides alone', async () => { if (process.platform === 'win32') return; const mockServerScriptPath = path.join(tmpHome, 'mock-devtools-server.js'); @@ -249,14 +249,14 @@ server.listen(0, '127.0.0.1', () => { expect(result.stderr).not.toContain('Chrome reuse metadata not found'); expect(result.status).toBe(0); const launchedArgs = fs.readFileSync(claudeArgsLogPath, 'utf8'); - expect(launchedArgs).toContain('--append-system-prompt'); - expect(launchedArgs).toContain(BROWSER_PROMPT_SNIPPET); + expect(launchedArgs).not.toContain('--append-system-prompt'); + expect(launchedArgs).not.toContain(BROWSER_PROMPT_SNIPPET); const launchedEnv = fs.readFileSync(claudeEnvLogPath, 'utf8'); - expect(launchedEnv).toContain(`userDataDir=${browserProfileDir}`); - expect(launchedEnv).toContain(`port=${port}`); - expect(launchedEnv).toContain(`httpUrl=http://127.0.0.1:${port}`); - expect(launchedEnv).toContain('wsUrl=ws://127.0.0.1/devtools/browser/default-target'); + expect(launchedEnv).not.toContain(`userDataDir=${browserProfileDir}`); + expect(launchedEnv).not.toContain(`port=${port}`); + expect(launchedEnv).not.toContain(`httpUrl=http://127.0.0.1:${port}`); + expect(launchedEnv).not.toContain('wsUrl=ws://127.0.0.1/devtools/browser/default-target'); }); it('skips managed browser attach when the default CCS browser profile directory is missing', () => { diff --git a/tests/unit/targets/settings-profile-browser-launch.test.ts b/tests/unit/targets/settings-profile-browser-launch.test.ts index bb3bb140..1a343f62 100644 --- a/tests/unit/targets/settings-profile-browser-launch.test.ts +++ b/tests/unit/targets/settings-profile-browser-launch.test.ts @@ -1,4 +1,5 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, setDefaultTimeout } from 'bun:test'; +import { request as httpRequest } from 'http'; import { spawn, spawnSync, type ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; @@ -6,6 +7,7 @@ import * as path from 'path'; import { mutateUnifiedConfig } from '../../../src/config/unified-config-loader'; const BROWSER_PROMPT_SNIPPET = 'prefer the CCS MCP Browser tool'; +setDefaultTimeout(30000); interface RunResult { status: number | null; @@ -40,6 +42,40 @@ function reserveClosedPort(): number { return port; } +async function waitForDevtoolsVersionEndpoint(port: string, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + + while (Date.now() <= deadline) { + try { + await new Promise((resolve, reject) => { + const req = httpRequest( + { + hostname: '127.0.0.1', + port: Number.parseInt(port, 10), + path: '/json/version', + method: 'GET', + }, + (res) => { + res.resume(); + if (res.statusCode === 200) { + resolve(); + return; + } + reject(new Error(`Unexpected status: ${res.statusCode ?? 'unknown'}`)); + } + ); + req.on('error', reject); + req.end(); + }); + return; + } catch { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + } + + throw new Error('Timed out waiting for mock DevTools endpoint to become ready'); +} + describe('settings profile browser launch', () => { let tmpHome = ''; let ccsDir = ''; @@ -128,7 +164,7 @@ exit 0 fs.rmSync(tmpHome, { recursive: true, force: true }); }); - it('fails before Claude launch when browser reuse cannot resolve DevToolsActivePort', () => { + it('does not block settings-profile launches when browser reuse cannot resolve DevToolsActivePort and the lane stays default-off', () => { if (process.platform === 'win32') return; fs.mkdirSync(browserProfileDir, { recursive: true }); @@ -138,12 +174,12 @@ exit 0 CCS_BROWSER_PROFILE_DIR: browserProfileDir, }); - expect(result.status).toBe(1); - expect(result.stderr).toContain('DevToolsActivePort'); - expect(fs.existsSync(claudeArgsLogPath)).toBe(false); + expect(result.status).toBe(0); + expect(result.stderr).not.toContain('DevToolsActivePort'); + expect(fs.existsSync(claudeArgsLogPath)).toBe(true); }); - it('passes browser reuse env and steering prompt into the Claude launch', async () => { + it('does not auto-enable browser reuse for settings-profile launches from env overrides alone', async () => { if (process.platform === 'win32') return; const mockServerScriptPath = path.join(tmpHome, 'mock-devtools-server.js'); @@ -182,6 +218,7 @@ server.listen(0, '127.0.0.1', () => { await new Promise((resolve) => setTimeout(resolve, 25)); } const port = fs.readFileSync(mockServerPortPath, 'utf8').trim(); + await waitForDevtoolsVersionEndpoint(port); fs.mkdirSync(browserProfileDir, { recursive: true }); fs.writeFileSync( @@ -200,14 +237,13 @@ server.listen(0, '127.0.0.1', () => { expect(result.stderr).not.toContain('Chrome reuse metadata not found'); expect(result.status).toBe(0); const launchedArgs = fs.readFileSync(claudeArgsLogPath, 'utf8'); - expect(launchedArgs).toContain('--append-system-prompt'); - expect(launchedArgs).toContain(BROWSER_PROMPT_SNIPPET); + expect(launchedArgs).not.toContain(BROWSER_PROMPT_SNIPPET); const launchedEnv = fs.readFileSync(claudeEnvLogPath, 'utf8'); - expect(launchedEnv).toContain(`userDataDir=${browserProfileDir}`); - expect(launchedEnv).toContain(`port=${port}`); - expect(launchedEnv).toContain(`httpUrl=http://127.0.0.1:${port}`); - expect(launchedEnv).toContain('wsUrl=ws://127.0.0.1/devtools/browser/browser-target'); + expect(launchedEnv).not.toContain(`userDataDir=${browserProfileDir}`); + expect(launchedEnv).not.toContain(`port=${port}`); + expect(launchedEnv).not.toContain(`httpUrl=http://127.0.0.1:${port}`); + expect(launchedEnv).not.toContain('wsUrl=ws://127.0.0.1/devtools/browser/browser-target'); }); it('skips managed browser attach for settings-profile launches when the default CCS browser profile directory is missing', () => { @@ -346,6 +382,7 @@ server.listen(0, '127.0.0.1', () => { await new Promise((resolve) => setTimeout(resolve, 25)); } const port = fs.readFileSync(mockServerPortPath, 'utf8').trim(); + await waitForDevtoolsVersionEndpoint(port); fs.mkdirSync(browserProfileDir, { recursive: true }); fs.writeFileSync( @@ -429,6 +466,7 @@ server.listen(0, '127.0.0.1', () => { await new Promise((resolve) => setTimeout(resolve, 25)); } const port = fs.readFileSync(mockServerPortPath, 'utf8').trim(); + await waitForDevtoolsVersionEndpoint(port); fs.mkdirSync(browserProfileDir, { recursive: true }); fs.writeFileSync( diff --git a/tests/unit/unified-config.test.ts b/tests/unit/unified-config.test.ts index 13c5661c..65c037d5 100644 --- a/tests/unit/unified-config.test.ts +++ b/tests/unit/unified-config.test.ts @@ -117,6 +117,22 @@ describe('unified-config-types', () => { expect(config.cliproxy.providers).toContain('gemini'); expect(config.cliproxy.providers).toContain('codex'); }); + + it('should default browser automation to disabled and manual for both lanes', () => { + const config = createEmptyUnifiedConfig(); + expect(config.browser).toEqual({ + claude: { + enabled: false, + policy: 'manual', + user_data_dir: '', + devtools_port: 9222, + }, + codex: { + enabled: false, + policy: 'manual', + }, + }); + }); }); describe('isUnifiedConfig', () => { @@ -337,3 +353,85 @@ describe('official-channels-config', () => { } }); }); + +describe('browser-config', () => { + it('fills in safe browser defaults when the browser section is missing on older configs', () => { + const originalCcsHome = process.env.CCS_HOME; + const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-browser-defaults-home-')); + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + ['version: 12', 'websearch:', ' enabled: false', ''].join('\n') + ); + + process.env.CCS_HOME = tempHome; + try { + const config = loadOrCreateUnifiedConfig(); + expect(config.browser).toMatchObject({ + claude: { + enabled: false, + policy: 'manual', + }, + codex: { + enabled: false, + policy: 'manual', + }, + }); + } finally { + if (originalCcsHome === undefined) { + delete process.env.CCS_HOME; + } else { + process.env.CCS_HOME = originalCcsHome; + } + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it('preserves explicit browser enablement while defaulting missing policies to manual', () => { + const originalCcsHome = process.env.CCS_HOME; + const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-browser-policy-home-')); + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + [ + 'version: 12', + 'browser:', + ' claude:', + ' enabled: true', + ' user_data_dir: "/tmp/claude-browser"', + ' devtools_port: 9333', + ' codex:', + ' enabled: true', + '', + ].join('\n') + ); + + process.env.CCS_HOME = tempHome; + try { + const config = loadOrCreateUnifiedConfig(); + expect(config.browser).toMatchObject({ + claude: { + enabled: true, + policy: 'manual', + user_data_dir: '/tmp/claude-browser', + devtools_port: 9333, + }, + codex: { + enabled: true, + policy: 'manual', + }, + }); + } finally { + if (originalCcsHome === undefined) { + delete process.env.CCS_HOME; + } else { + process.env.CCS_HOME = originalCcsHome; + } + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/unit/utils/browser/browser-status.test.ts b/tests/unit/utils/browser/browser-status.test.ts index bda30626..abaeaaff 100644 --- a/tests/unit/utils/browser/browser-status.test.ts +++ b/tests/unit/utils/browser/browser-status.test.ts @@ -5,7 +5,9 @@ import { join } from 'node:path'; import { getBrowserConfig, mutateUnifiedConfig, + saveUnifiedConfig, } from '../../../../src/config/unified-config-loader'; +import { createEmptyUnifiedConfig } from '../../../../src/config/unified-config-types'; import * as chromeReuse from '../../../../src/utils/browser/chrome-reuse'; import { getBrowserStatus } from '../../../../src/utils/browser/browser-status'; import { @@ -62,7 +64,7 @@ describe('browser status', () => { rmSync(tempHome, { recursive: true, force: true }); }); - it('returns a disabled Claude lane with the recommended managed user-data dir by default', async () => { + it('returns disabled/manual browser lanes with the recommended managed user-data dir by default', async () => { const codexSpy = spyOn(codexDetector, 'getCodexBinaryInfo').mockReturnValue({ path: '/usr/local/bin/codex', needsShell: false, @@ -76,24 +78,72 @@ describe('browser status', () => { expect(status.claude).toMatchObject({ enabled: false, state: 'disabled', + policy: 'manual', source: 'config', effectiveUserDataDir: join(tempHome, '.ccs', 'browser', 'chrome-user-data'), devtoolsPort: 9222, managedMcpServerName: 'ccs-browser', }); expect(status.claude.launchCommands.linux).toContain('--remote-debugging-port=9222'); + expect(status.claude.detail).toContain('off by default'); expect(status.codex).toMatchObject({ - enabled: true, - state: 'enabled', + enabled: false, + policy: 'manual', + state: 'disabled', serverName: 'ccs_browser', - supportsConfigOverrides: true, + supportsConfigOverrides: false, }); + expect(status.codex.detail).toContain('off by default'); expect(existsSync(join(tempHome, '.ccs', 'browser', 'chrome-user-data'))).toBe(false); } finally { codexSpy.mockRestore(); } }); + it('resolves missing saved browser policies to manual while preserving explicit enabled values', async () => { + const config = createEmptyUnifiedConfig(); + config.browser = { + claude: { + enabled: true, + user_data_dir: '/tmp/explicit-claude', + devtools_port: 9333, + } as typeof config.browser.claude, + codex: { + enabled: true, + } as typeof config.browser.codex, + }; + saveUnifiedConfig(config); + + const runtimeSpy = spyOn(chromeReuse, 'resolveBrowserRuntimeEnv').mockRejectedValue( + new Error('Chrome reuse metadata not found: /tmp/explicit-claude/DevToolsActivePort') + ); + const codexSpy = spyOn(codexDetector, 'getCodexBinaryInfo').mockReturnValue({ + path: '/usr/local/bin/codex', + needsShell: false, + version: 'codex-cli 0.120.0', + features: ['config-overrides'], + }); + + try { + const status = await getBrowserStatus(); + + expect(status.claude).toMatchObject({ + enabled: true, + policy: 'manual', + effectiveUserDataDir: '/tmp/explicit-claude', + devtoolsPort: 9333, + }); + expect(status.codex).toMatchObject({ + enabled: true, + policy: 'manual', + state: 'enabled', + }); + } finally { + runtimeSpy.mockRestore(); + codexSpy.mockRestore(); + } + }); + it('bootstraps the managed default browser profile dir before reporting attach readiness', async () => { mutateUnifiedConfig((config) => { config.browser = { diff --git a/tests/unit/web-server/browser-routes.test.ts b/tests/unit/web-server/browser-routes.test.ts index 11db1b5e..d8a4e1fd 100644 --- a/tests/unit/web-server/browser-routes.test.ts +++ b/tests/unit/web-server/browser-routes.test.ts @@ -93,23 +93,27 @@ describe('browser routes', () => { expect(payload.config).toMatchObject({ claude: { enabled: false, - policy: 'auto', + policy: 'manual', userDataDir: join(tempHome, '.ccs', 'browser', 'chrome-user-data'), devtoolsPort: 9222, }, codex: { - enabled: true, - policy: 'auto', + enabled: false, + policy: 'manual', }, }); expect(payload.status.claude).toMatchObject({ state: 'disabled', + policy: 'manual', managedMcpServerName: 'ccs-browser', }); expect(payload.status.codex).toMatchObject({ - enabled: true, + enabled: false, + state: 'disabled', + policy: 'manual', serverName: 'ccs_browser', }); + expect(payload.status.codex.detail).toContain('off by default'); }); it('updates the saved browser config through the dashboard route', async () => {