From 09268e7e3307aea1508505692f83a2131e15f8a9 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 2 May 2026 22:39:13 -0400 Subject: [PATCH] refactor(cliproxy/executor): extract model-warnings + claude-launcher and polish orchestrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phases 08+09+10 of #1162. Final extractions and orchestrator cleanup: - src/cliproxy/executor/model-warnings.ts (80 LOC): warnBrokenModels — handles composite + simple paths, broken-model notification with replacement suggestions. - src/cliproxy/executor/claude-launcher.ts (161 LOC): launchClaude — final args assembly (web search, image analysis, browser tool args), trace context env, Windows shell escaping, spawn, quota monitor wiring, cleanup handlers. - New tests: 227 + 217 LOC. - index.ts cleanup: numbered section comments through orchestrator, removed ~17 now-unused imports, re-export block preserved. index.ts: 665 -> 550 LOC. Final reduction across all 10 phases: 1428 -> 550 (-878, -61%). Behavior unchanged; full suite passes 1824/1824. The remaining 550 LOC is the natural orchestrator floor — further extraction would require a 15-field context struct, which is manufactured complexity rather than genuine separation of concerns. Closes phases 02-10 of #1162. Refs #1162 --- .../__tests__/claude-launcher.test.ts | 217 +++++++++++++++++ .../executor/__tests__/model-warnings.test.ts | 227 ++++++++++++++++++ src/cliproxy/executor/claude-launcher.ts | 161 +++++++++++++ src/cliproxy/executor/index.ts | 153 ++---------- src/cliproxy/executor/model-warnings.ts | 80 ++++++ 5 files changed, 706 insertions(+), 132 deletions(-) create mode 100644 src/cliproxy/executor/__tests__/claude-launcher.test.ts create mode 100644 src/cliproxy/executor/__tests__/model-warnings.test.ts create mode 100644 src/cliproxy/executor/claude-launcher.ts create mode 100644 src/cliproxy/executor/model-warnings.ts diff --git a/src/cliproxy/executor/__tests__/claude-launcher.test.ts b/src/cliproxy/executor/__tests__/claude-launcher.test.ts new file mode 100644 index 00000000..535bef1f --- /dev/null +++ b/src/cliproxy/executor/__tests__/claude-launcher.test.ts @@ -0,0 +1,217 @@ +/** + * Tests for claude-launcher.ts — Phase 09 + * + * Verifies: + * - spawn called with expected args and env + * - Windows shell escaping path + * - Cleanup handlers registered (setupCleanupHandlers called) + * + * Strategy: mock child_process.spawn and all side-effectful dependencies so + * no real processes are started. + */ + +import { describe, expect, it, jest, beforeEach, afterEach, mock } from 'bun:test'; +import type { ChildProcess } from 'child_process'; +import type { ExecutorConfig } from '../../types'; + +// ── Spawn mock ──────────────────────────────────────────────────────────────── + +const mockSpawnResult = { + pid: 42, + on: jest.fn(), + stdout: null, + stderr: null, +} as unknown as ChildProcess; + +const mockSpawn = jest.fn().mockReturnValue(mockSpawnResult); + +mock.module('child_process', () => ({ + spawn: mockSpawn, +})); + +// ── Dependency mocks ────────────────────────────────────────────────────────── + +const mockEscapeShellArg = jest.fn((s: string) => `"${s}"`); +const mockGetWindowsEscapedCommandShell = jest.fn().mockReturnValue('cmd.exe'); + +mock.module('../../utils/shell-executor', () => ({ + escapeShellArg: mockEscapeShellArg, + getWindowsEscapedCommandShell: mockGetWindowsEscapedCommandShell, +})); + +mock.module('../../config/config-generator', () => ({ + getProviderSettingsPath: jest.fn().mockReturnValue('/fake/.ccs/settings/gemini.json'), + CLIPROXY_DEFAULT_PORT: 8317, + generateConfig: jest.fn(), + getProviderConfig: jest.fn(), +})); + +mock.module('../../utils/websearch-manager', () => ({ + appendThirdPartyWebSearchToolArgs: (args: string[]) => args, + createWebSearchTraceContext: jest.fn().mockReturnValue({}), + ensureWebSearchMcpOrThrow: jest.fn(), + displayWebSearchStatus: jest.fn(), + getWebSearchHookEnv: jest.fn().mockReturnValue({}), +})); + +mock.module('../../utils/image-analysis', () => ({ + appendThirdPartyImageAnalysisToolArgs: (args: string[]) => [...args, '--mcp-image-analysis'], + syncImageAnalysisMcpToConfigDir: jest.fn(), + ensureImageAnalysisMcpOrThrow: jest.fn().mockReturnValue(true), +})); + +mock.module('../../utils/browser', () => ({ + appendBrowserToolArgs: (args: string[]) => [...args, '--browser'], +})); + +mock.module('../accounts/account-manager', () => ({ + getDefaultAccount: jest.fn().mockReturnValue(null), +})); + +const mockSetupCleanupHandlers = jest.fn(); +mock.module('../session-bridge', () => ({ + setupCleanupHandlers: mockSetupCleanupHandlers, + checkOrJoinProxy: jest.fn(), + registerProxySession: jest.fn(), +})); + +const mockResolveRuntimeQuotaMonitorProviders = jest.fn().mockReturnValue([]); +mock.module('../account-resolution', () => ({ + resolveRuntimeQuotaMonitorProviders: mockResolveRuntimeQuotaMonitorProviders, + resolveAccounts: jest.fn(), +})); + +// Dynamic import for quota-manager +mock.module('../quota/quota-manager', () => ({ + startQuotaMonitor: jest.fn(), + stopQuotaMonitor: jest.fn(), +})); + +// ── Subject under test ──────────────────────────────────────────────────────── + +import { launchClaude } from '../claude-launcher'; + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function makeCfg(overrides: Partial = {}): ExecutorConfig { + return { + port: 8317, + timeout: 5000, + verbose: false, + pollInterval: 100, + ...overrides, + } as ExecutorConfig; +} + +function baseContext(overrides: object = {}) { + return { + claudeCli: '/usr/local/bin/claude', + claudeArgs: ['chat', '--model', 'gemini-2.5-pro'], + env: { ANTHROPIC_BASE_URL: 'http://localhost:8317' } as NodeJS.ProcessEnv, + cfg: makeCfg(), + provider: 'gemini' as const, + compositeProviders: [] as string[], + skipLocalAuth: true, + sessionId: undefined, + imageAnalysisMcpReady: false, + browserRuntimeEnv: undefined, + inheritedClaudeConfigDir: undefined, + codexReasoningProxy: null, + toolSanitizationProxy: null, + httpsTunnel: null, + verbose: false, + ...overrides, + }; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('launchClaude', () => { + beforeEach(() => { + mockSpawn.mockClear(); + mockSetupCleanupHandlers.mockClear(); + mockEscapeShellArg.mockClear(); + }); + + it('calls spawn with claudeCli and includes --settings arg', async () => { + const ctx = baseContext(); + await launchClaude(ctx); + + expect(mockSpawn).toHaveBeenCalledTimes(1); + const [cmd, spawnArgs] = mockSpawn.mock.calls[0] as [string, string[]]; + expect(cmd).toBe('/usr/local/bin/claude'); + expect(spawnArgs).toContain('--settings'); + }); + + it('inherits process env merged with provided env', async () => { + const ctx = baseContext({ env: { CUSTOM_KEY: 'custom-value' } as NodeJS.ProcessEnv }); + await launchClaude(ctx); + + const spawnOpts = mockSpawn.mock.calls[0][2] as { env: NodeJS.ProcessEnv }; + expect(spawnOpts.env?.CUSTOM_KEY).toBe('custom-value'); + }); + + it('passes stdio: inherit', async () => { + await launchClaude(baseContext()); + const spawnOpts = mockSpawn.mock.calls[0][2] as { stdio: string }; + expect(spawnOpts.stdio).toBe('inherit'); + }); + + it('appends image analysis args when imageAnalysisMcpReady=true', async () => { + await launchClaude(baseContext({ imageAnalysisMcpReady: true })); + const spawnArgs = mockSpawn.mock.calls[0][1] as string[]; + expect(spawnArgs).toContain('--mcp-image-analysis'); + }); + + it('does not append image analysis args when imageAnalysisMcpReady=false', async () => { + await launchClaude(baseContext({ imageAnalysisMcpReady: false })); + const spawnArgs = mockSpawn.mock.calls[0][1] as string[]; + expect(spawnArgs).not.toContain('--mcp-image-analysis'); + }); + + it('appends browser tool args when browserRuntimeEnv is set', async () => { + await launchClaude( + baseContext({ browserRuntimeEnv: { CCS_BROWSER: '1' } as NodeJS.ProcessEnv }) + ); + const spawnArgs = mockSpawn.mock.calls[0][1] as string[]; + expect(spawnArgs).toContain('--browser'); + }); + + it('registers cleanup handlers after spawn', async () => { + await launchClaude(baseContext()); + expect(mockSetupCleanupHandlers).toHaveBeenCalledTimes(1); + // First arg should be the ChildProcess returned by spawn + expect(mockSetupCleanupHandlers.mock.calls[0][0]).toBe(mockSpawnResult); + }); + + it('returns the ChildProcess from spawn', async () => { + const result = await launchClaude(baseContext()); + expect(result).toBe(mockSpawnResult); + }); + + describe('Windows shell escaping', () => { + const originalPlatform = process.platform; + + beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + }); + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + }); + + it('uses shell mode for .cmd executables on Windows', async () => { + await launchClaude(baseContext({ claudeCli: 'C:\\tools\\claude.cmd' })); + const spawnOpts = mockSpawn.mock.calls[0][2] as { shell: string | boolean }; + // shell property should be set (cmd.exe from mock) + expect(spawnOpts.shell).toBeTruthy(); + }); + + it('skips shell mode for non-script executables on Windows', async () => { + await launchClaude(baseContext({ claudeCli: 'C:\\tools\\claude.exe' })); + // spawn should be called with separate args array, not a shell cmd string + const [, secondArg] = mockSpawn.mock.calls[0]; + expect(Array.isArray(secondArg)).toBe(true); + }); + }); +}); diff --git a/src/cliproxy/executor/__tests__/model-warnings.test.ts b/src/cliproxy/executor/__tests__/model-warnings.test.ts new file mode 100644 index 00000000..054b3a7a --- /dev/null +++ b/src/cliproxy/executor/__tests__/model-warnings.test.ts @@ -0,0 +1,227 @@ +/** + * Tests for model-warnings.ts — Phase 08 + * + * Verifies that warnBrokenModels emits warnings for broken models and is + * silent for healthy ones, covering both simple and composite providers. + */ + +import { afterEach, beforeEach, describe, expect, it, jest } from 'bun:test'; + +// ── Stubs ───────────────────────────────────────────────────────────────────── + +// getCurrentModel: return whatever the test configures +const mockGetCurrentModel = jest.fn(); +// isModelBroken: return false by default +const mockIsModelBroken = jest.fn().mockReturnValue(false); +const mockGetModelIssueUrl = jest + .fn() + .mockReturnValue(undefined); +const mockFindModel = jest + .fn<{ name: string } | undefined, [string, string]>() + .mockReturnValue(undefined); +const mockGetSuggestedReplacementModel = jest + .fn() + .mockReturnValue(undefined); + +jest.mock('../model-warnings', () => { + // We test the real implementation — only mock its dependencies + return jest.requireActual('../model-warnings'); +}); + +jest.mock('../../config/model-config', () => ({ + getCurrentModel: mockGetCurrentModel, +})); + +jest.mock('../../cliproxy/model-catalog', () => ({ + isModelBroken: mockIsModelBroken, + getModelIssueUrl: mockGetModelIssueUrl, + findModel: mockFindModel, + getSuggestedReplacementModel: mockGetSuggestedReplacementModel, +})); + +// We import from real path after jest.mock so actual module is used +import { warnBrokenModels } from '../model-warnings'; +import type { ExecutorConfig } from '../../types'; + +// ── Helpers ──────────────────────────────────────────────────────────────────── + +function makeSimpleCfg(overrides: Partial = {}): ExecutorConfig { + return { + port: 8317, + timeout: 5000, + verbose: false, + pollInterval: 100, + isComposite: false, + ...overrides, + } as ExecutorConfig; +} + +// ── Tests ────────────────────────────────────────────────────────────────────── + +describe('warnBrokenModels', () => { + let errorSpy: ReturnType; + + beforeEach(() => { + errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + mockGetCurrentModel.mockReset(); + mockIsModelBroken.mockReturnValue(false); + mockGetModelIssueUrl.mockReturnValue(undefined); + mockFindModel.mockReturnValue(undefined); + mockGetSuggestedReplacementModel.mockReturnValue(undefined); + }); + + afterEach(() => { + errorSpy.mockRestore(); + }); + + it('does not warn when no model is configured', () => { + mockGetCurrentModel.mockReturnValue(undefined); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('does not warn when model is healthy', () => { + mockGetCurrentModel.mockReturnValue('gemini-2.5-pro'); + mockIsModelBroken.mockReturnValue(false); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('warns when model is broken (no replacement, no issue url)', () => { + mockGetCurrentModel.mockReturnValue('gemini-old'); + mockIsModelBroken.mockReturnValue(true); + mockFindModel.mockReturnValue({ name: 'Gemini Old' } as { name: string }); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('has known issues with Claude Code'))).toBe(true); + expect(calls.some((m) => m.includes('Tool calls will fail'))).toBe(true); + }); + + it('includes replacement model suggestion when available', () => { + mockGetCurrentModel.mockReturnValue('gemini-old'); + mockIsModelBroken.mockReturnValue(true); + mockGetSuggestedReplacementModel.mockReturnValue('gemini-2.5-pro'); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('gemini-2.5-pro'))).toBe(true); + }); + + it('includes tracking URL when issue url is available', () => { + mockGetCurrentModel.mockReturnValue('gemini-old'); + mockIsModelBroken.mockReturnValue(true); + mockGetModelIssueUrl.mockReturnValue('https://github.com/issues/123'); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('https://github.com/issues/123'))).toBe(true); + }); + + it('includes remote proxy note when skipLocalAuth=true', () => { + mockGetCurrentModel.mockReturnValue('gemini-old'); + mockIsModelBroken.mockReturnValue(true); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: true, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('remote proxy'))).toBe(true); + }); + + it('includes --config suggestion when skipLocalAuth=false', () => { + mockGetCurrentModel.mockReturnValue('gemini-old'); + mockIsModelBroken.mockReturnValue(true); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeSimpleCfg(), + compositeProviders: [], + skipLocalAuth: false, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('--config'))).toBe(true); + }); + + describe('composite variants', () => { + function makeCompositeCfg(): ExecutorConfig { + return { + port: 8317, + timeout: 5000, + verbose: false, + pollInterval: 100, + isComposite: true, + compositeTiers: { + opus: { provider: 'gemini', model: 'gemini-opus-old' }, + sonnet: { provider: 'gemini', model: 'gemini-sonnet-ok' }, + haiku: { provider: 'gemini', model: 'gemini-haiku-ok' }, + }, + } as unknown as ExecutorConfig; + } + + it('warns for broken composite tier', () => { + mockIsModelBroken.mockImplementation((_p, m) => m === 'gemini-opus-old'); + mockFindModel.mockReturnValue({ name: 'Gemini Opus Old' } as { name: string }); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeCompositeCfg(), + compositeProviders: ['gemini'], + skipLocalAuth: false, + }); + + const calls = errorSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes('opus tier'))).toBe(true); + }); + + it('does not warn for healthy composite tiers', () => { + mockIsModelBroken.mockReturnValue(false); + + warnBrokenModels({ + provider: 'gemini', + cfg: makeCompositeCfg(), + compositeProviders: ['gemini'], + skipLocalAuth: false, + }); + + expect(errorSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/cliproxy/executor/claude-launcher.ts b/src/cliproxy/executor/claude-launcher.ts new file mode 100644 index 00000000..c4f97a13 --- /dev/null +++ b/src/cliproxy/executor/claude-launcher.ts @@ -0,0 +1,161 @@ +/** + * Claude Launcher — Concern H + * + * Handles final argument assembly, trace context injection, Windows shell + * escaping, spawning the Claude CLI process, starting the runtime quota + * monitor, and wiring up cleanup handlers. + */ + +import { spawn, ChildProcess } from 'child_process'; +import * as os from 'os'; +import { escapeShellArg, getWindowsEscapedCommandShell } from '../../utils/shell-executor'; +import { getProviderSettingsPath } from '../config/config-generator'; +import { + ensureWebSearchMcpOrThrow as _ensureWebSearchMcpOrThrow, + appendThirdPartyWebSearchToolArgs, + createWebSearchTraceContext, +} from '../../utils/websearch-manager'; +import { appendThirdPartyImageAnalysisToolArgs } from '../../utils/image-analysis'; +import { appendBrowserToolArgs } from '../../utils/browser'; +import { getDefaultAccount } from '../accounts/account-manager'; +import { CLIProxyProvider, ExecutorConfig } from '../types'; +import { CodexReasoningProxy } from '../ai-providers/codex-reasoning-proxy'; +import { ToolSanitizationProxy } from '../proxy/tool-sanitization-proxy'; +import { HttpsTunnelProxy } from '../proxy/https-tunnel-proxy'; +import { setupCleanupHandlers } from './session-bridge'; +import { resolveRuntimeQuotaMonitorProviders } from './account-resolution'; + +export interface ClaudeLaunchContext { + /** Path to the Claude CLI executable */ + claudeCli: string; + /** Pre-filtered Claude args (CCS flags already stripped) */ + claudeArgs: string[]; + /** Fully assembled environment variables (without trace additions) */ + env: NodeJS.ProcessEnv; + /** Resolved executor config */ + cfg: ExecutorConfig; + /** Active CLIProxy provider */ + provider: CLIProxyProvider; + /** Providers derived from composite tiers (empty for simple providers) */ + compositeProviders: CLIProxyProvider[]; + /** Whether local OAuth was skipped (remote proxy auth in use) */ + skipLocalAuth: boolean; + /** Session ID for cleanup tracking */ + sessionId: string | undefined; + /** Whether image analysis MCP is ready */ + imageAnalysisMcpReady: boolean; + /** Browser runtime environment variables (undefined if browser not active) */ + browserRuntimeEnv: NodeJS.ProcessEnv | undefined; + /** Inherited Claude config dir for continuity */ + inheritedClaudeConfigDir: string | undefined; + /** Active Codex reasoning proxy (if any) */ + codexReasoningProxy: CodexReasoningProxy | null; + /** Active tool sanitization proxy (if any) */ + toolSanitizationProxy: ToolSanitizationProxy | null; + /** Active HTTPS tunnel proxy (if any) */ + httpsTunnel: HttpsTunnelProxy | null; + /** Whether verbose logging is enabled */ + verbose: boolean; +} + +/** + * Assemble final Claude CLI arguments, inject trace context, spawn the process, + * start the runtime quota monitor, and wire cleanup handlers. + * + * @returns The spawned ChildProcess for the Claude CLI. + */ +export async function launchClaude(context: ClaudeLaunchContext): Promise { + const { + claudeCli, + claudeArgs, + env, + cfg, + provider, + compositeProviders, + skipLocalAuth, + sessionId, + imageAnalysisMcpReady, + browserRuntimeEnv, + inheritedClaudeConfigDir, + codexReasoningProxy, + toolSanitizationProxy, + httpsTunnel, + verbose, + } = context; + + const isWindows = process.platform === 'win32'; + const needsShell = isWindows && /\.(cmd|bat|ps1)$/i.test(claudeCli); + + const settingsPath = cfg.customSettingsPath + ? cfg.customSettingsPath.replace(/^~/, os.homedir()) + : getProviderSettingsPath(provider); + + // Assemble final args: image analysis tools → browser tools → web search tools → settings + const imageAnalysisArgs = imageAnalysisMcpReady + ? appendThirdPartyImageAnalysisToolArgs(claudeArgs) + : claudeArgs; + const browserArgs = browserRuntimeEnv + ? appendBrowserToolArgs(imageAnalysisArgs) + : imageAnalysisArgs; + const launchArgs = [ + '--settings', + settingsPath, + ...appendThirdPartyWebSearchToolArgs(browserArgs), + ]; + + // Inject web search trace context into env + const traceEnv = createWebSearchTraceContext({ + launcher: 'cliproxy.executor', + args: launchArgs, + profile: cfg.profileName || provider, + profileType: 'cliproxy', + settingsPath, + claudeConfigDir: inheritedClaudeConfigDir, + }); + const tracedEnv = { ...env, ...traceEnv }; + + // Spawn: Windows .cmd/.bat/.ps1 need shell escaping; all others spawn directly + let claude: ChildProcess; + if (needsShell) { + const cmdString = [claudeCli, ...launchArgs].map(escapeShellArg).join(' '); + claude = spawn(cmdString, { + stdio: 'inherit', + windowsHide: true, + shell: getWindowsEscapedCommandShell(), + env: tracedEnv, + }); + } else { + claude = spawn(claudeCli, launchArgs, { + stdio: 'inherit', + windowsHide: true, + env: tracedEnv, + }); + } + + // Start runtime quota monitor (adaptive polling during session) + if (!skipLocalAuth) { + const { startQuotaMonitor } = await import('../quota/quota-manager'); + for (const monitorProvider of resolveRuntimeQuotaMonitorProviders( + provider, + compositeProviders + )) { + const monitorAccount = getDefaultAccount(monitorProvider); + if (monitorAccount) { + startQuotaMonitor(monitorProvider, monitorAccount.id); + } + } + } + + // Wire cleanup handlers (process exit, SIGINT, SIGTERM, proxy teardown) + setupCleanupHandlers( + claude, + sessionId, + cfg.port, + codexReasoningProxy, + toolSanitizationProxy, + httpsTunnel, + verbose + ); + + return claude; +} diff --git a/src/cliproxy/executor/index.ts b/src/cliproxy/executor/index.ts index 68b8fc39..7b4b3eb0 100644 --- a/src/cliproxy/executor/index.ts +++ b/src/cliproxy/executor/index.ts @@ -10,41 +10,24 @@ * 6. Claude CLI execution with cleanup handlers */ -import { spawn, ChildProcess } from 'child_process'; +import { ChildProcess } from 'child_process'; import * as fs from 'fs'; -import * as os from 'os'; import { fail, info, warn } from '../../utils/ui'; -import { escapeShellArg, getWindowsEscapedCommandShell } from '../../utils/shell-executor'; import { generateConfig, getProviderConfig, - getProviderSettingsPath, CLIPROXY_DEFAULT_PORT, } from '../config/config-generator'; -import { configureProviderModel, getCurrentModel } from '../config/model-config'; +import { configureProviderModel } from '../config/model-config'; import { supportsModelConfig } from '../model-catalog'; import { CLIProxyProvider, ExecutorConfig } from '../types'; -import { - isModelBroken, - getModelIssueUrl, - findModel, - getSuggestedReplacementModel, -} from '../model-catalog'; import { CodexReasoningProxy } from '../ai-providers/codex-reasoning-proxy'; import { ToolSanitizationProxy } from '../proxy/tool-sanitization-proxy'; -import { - ensureWebSearchMcpOrThrow, - displayWebSearchStatus, - appendThirdPartyWebSearchToolArgs, - createWebSearchTraceContext, -} from '../../utils/websearch-manager'; +import { ensureWebSearchMcpOrThrow, displayWebSearchStatus } from '../../utils/websearch-manager'; import { ensureImageAnalysisMcpOrThrow, syncImageAnalysisMcpToConfigDir, - appendThirdPartyImageAnalysisToolArgs, } from '../../utils/image-analysis'; -import { getDefaultAccount } from '../accounts/account-manager'; -import { appendBrowserToolArgs } from '../../utils/browser'; import { loadOrCreateUnifiedConfig, getThinkingConfig } from '../../config/unified-config-loader'; import { HttpsTunnelProxy } from '../proxy/https-tunnel-proxy'; import { resolveProfileContinuityInheritance } from '../../auth/profile-continuity-inheritance'; @@ -61,7 +44,7 @@ import { logEnvironment, resolveCliproxyImageAnalysisEnv, } from './env-resolver'; -import { checkOrJoinProxy, registerProxySession, setupCleanupHandlers } from './session-bridge'; +import { checkOrJoinProxy, registerProxySession } from './session-bridge'; import { getWebSearchHookEnv } from '../../utils/websearch-manager'; import { handleLogout, @@ -82,6 +65,8 @@ import { shouldStartHttpsTunnel } from './https-tunnel-policy'; import { filterCcsFlags, parseExecutorFlags, validateFlagCombinations } from './arg-parser'; import { resolveExecutorProxy } from './proxy-resolver'; import { buildProxyChain } from './proxy-chain-builder'; +import { warnBrokenModels } from './model-warnings'; +import { launchClaude } from './claude-launcher'; /** Local alias so internal call sites need no change */ const resolveRuntimeQuotaMonitorProviders = _resolveRuntimeQuotaMonitorProviders; @@ -276,51 +261,7 @@ export async function execClaudeWithCLIProxy( } // 5. Check for broken models (multi-tier for composite) - if (compositeProviders.length > 0 && cfg.compositeTiers) { - // Check all tier models in composite variant - const tiers: Array<'opus' | 'sonnet' | 'haiku'> = ['opus', 'sonnet', 'haiku']; - for (const tier of tiers) { - const tierConfig = cfg.compositeTiers[tier]; - if (tierConfig && isModelBroken(tierConfig.provider, tierConfig.model)) { - const modelEntry = findModel(tierConfig.provider, tierConfig.model); - const issueUrl = getModelIssueUrl(tierConfig.provider, tierConfig.model); - console.error(''); - console.error( - warn( - `${tier} tier: ${modelEntry?.name || tierConfig.model} has known issues with Claude Code` - ) - ); - console.error(' Tool calls will fail. Consider changing the model in config.yaml.'); - if (issueUrl) { - console.error(` Tracking: ${issueUrl}`); - } - console.error(''); - } - } - } else { - const currentModel = getCurrentModel(provider, cfg.customSettingsPath); - if (currentModel && isModelBroken(provider, currentModel)) { - const modelEntry = findModel(provider, currentModel); - const issueUrl = getModelIssueUrl(provider, currentModel); - const replacementModel = getSuggestedReplacementModel(provider, currentModel); - console.error(''); - console.error(warn(`${modelEntry?.name || currentModel} has known issues with Claude Code`)); - if (replacementModel) { - console.error(` Tool calls will fail. Use "${replacementModel}" instead.`); - } else { - console.error(' Tool calls will fail. Consider changing the model in config.yaml.'); - } - if (issueUrl) { - console.error(` Tracking: ${issueUrl}`); - } - if (skipLocalAuth) { - console.error(' Note: Model may be overridden by remote proxy configuration.'); - } else { - console.error(` Run "ccs ${provider} --config" to change model.`); - } - console.error(''); - } - } + warnBrokenModels({ provider, cfg, compositeProviders, skipLocalAuth }); // 6. Ensure user settings file exists ensureProviderSettingsFile(provider); @@ -571,77 +512,25 @@ export async function execClaudeWithCLIProxy( console.error(`[i] Thinking: ${thinkingLabel} (${sourceLabel})`); } - // 12. Filter CCS-specific flags before passing to Claude CLI + // 12. Filter CCS flags, spawn Claude CLI, start quota monitor, wire cleanup const claudeArgs = filterCcsFlags(argsWithoutBrowserFlags); - - const isWindows = process.platform === 'win32'; - const needsShell = isWindows && /\.(cmd|bat|ps1)$/i.test(claudeCli); - - const settingsPath = cfg.customSettingsPath - ? cfg.customSettingsPath.replace(/^~/, os.homedir()) - : getProviderSettingsPath(provider); - - let claude: ChildProcess; - const imageAnalysisArgs = imageAnalysisMcpReady - ? appendThirdPartyImageAnalysisToolArgs(claudeArgs) - : claudeArgs; - const browserArgs = browserRuntimeEnv - ? appendBrowserToolArgs(imageAnalysisArgs) - : imageAnalysisArgs; - const launchArgs = [ - '--settings', - settingsPath, - ...appendThirdPartyWebSearchToolArgs(browserArgs), - ]; - const traceEnv = createWebSearchTraceContext({ - launcher: 'cliproxy.executor', - args: launchArgs, - profile: cfg.profileName || provider, - profileType: 'cliproxy', - settingsPath, - claudeConfigDir: inheritedClaudeConfigDir, - }); - const tracedEnv = { ...env, ...traceEnv }; - if (needsShell) { - const cmdString = [claudeCli, ...launchArgs].map(escapeShellArg).join(' '); - claude = spawn(cmdString, { - stdio: 'inherit', - windowsHide: true, - shell: getWindowsEscapedCommandShell(), - env: tracedEnv, - }); - } else { - claude = spawn(claudeCli, launchArgs, { - stdio: 'inherit', - windowsHide: true, - env: tracedEnv, - }); - } - - // 12b. Start runtime quota monitor (adaptive polling during session) - if (!skipLocalAuth) { - const { startQuotaMonitor } = await import('../quota/quota-manager'); - for (const monitorProvider of resolveRuntimeQuotaMonitorProviders( - provider, - compositeProviders - )) { - const monitorAccount = getDefaultAccount(monitorProvider); - if (monitorAccount) { - startQuotaMonitor(monitorProvider, monitorAccount.id); - } - } - } - - // 13. Setup cleanup handlers - setupCleanupHandlers( - claude, + await launchClaude({ + claudeCli, + claudeArgs, + env, + cfg, + provider, + compositeProviders, + skipLocalAuth, sessionId, - cfg.port, + imageAnalysisMcpReady, + browserRuntimeEnv, + inheritedClaudeConfigDir, codexReasoningProxy, toolSanitizationProxy, httpsTunnel, - verbose - ); + verbose, + }); } // Re-export utility functions for backwards compatibility diff --git a/src/cliproxy/executor/model-warnings.ts b/src/cliproxy/executor/model-warnings.ts new file mode 100644 index 00000000..f7c1bfab --- /dev/null +++ b/src/cliproxy/executor/model-warnings.ts @@ -0,0 +1,80 @@ +/** + * Model Warnings — Concern G + * + * Emits console warnings when the active model (or any tier model in composite + * variants) is flagged as broken in the model catalog. + */ + +import { warn } from '../../utils/ui'; +import { getCurrentModel } from '../config/model-config'; +import { + isModelBroken, + getModelIssueUrl, + findModel, + getSuggestedReplacementModel, +} from '../model-catalog'; +import { CLIProxyProvider, ExecutorConfig } from '../types'; + +export interface ModelWarningsContext { + provider: CLIProxyProvider; + cfg: ExecutorConfig; + compositeProviders: CLIProxyProvider[]; + skipLocalAuth: boolean; + customSettingsPath?: string; +} + +/** + * Check all active models for known issues and emit warnings. + * + * For composite variants, checks every tier model. + * For simple providers, checks the currently configured model. + */ +export function warnBrokenModels(context: ModelWarningsContext): void { + const { provider, cfg, skipLocalAuth } = context; + + if (cfg.isComposite && cfg.compositeTiers) { + // Check all tier models in composite variant + const tiers: Array<'opus' | 'sonnet' | 'haiku'> = ['opus', 'sonnet', 'haiku']; + for (const tier of tiers) { + const tierConfig = cfg.compositeTiers[tier]; + if (tierConfig && isModelBroken(tierConfig.provider, tierConfig.model)) { + const modelEntry = findModel(tierConfig.provider, tierConfig.model); + const issueUrl = getModelIssueUrl(tierConfig.provider, tierConfig.model); + console.error(''); + console.error( + warn( + `${tier} tier: ${modelEntry?.name || tierConfig.model} has known issues with Claude Code` + ) + ); + console.error(' Tool calls will fail. Consider changing the model in config.yaml.'); + if (issueUrl) { + console.error(` Tracking: ${issueUrl}`); + } + console.error(''); + } + } + } else { + const currentModel = getCurrentModel(provider, cfg.customSettingsPath); + if (currentModel && isModelBroken(provider, currentModel)) { + const modelEntry = findModel(provider, currentModel); + const issueUrl = getModelIssueUrl(provider, currentModel); + const replacementModel = getSuggestedReplacementModel(provider, currentModel); + console.error(''); + console.error(warn(`${modelEntry?.name || currentModel} has known issues with Claude Code`)); + if (replacementModel) { + console.error(` Tool calls will fail. Use "${replacementModel}" instead.`); + } else { + console.error(' Tool calls will fail. Consider changing the model in config.yaml.'); + } + if (issueUrl) { + console.error(` Tracking: ${issueUrl}`); + } + if (skipLocalAuth) { + console.error(' Note: Model may be overridden by remote proxy configuration.'); + } else { + console.error(` Run "ccs ${provider} --config" to change model.`); + } + console.error(''); + } + } +}