From 7bb9ceb4aff0d806ec19b3e97486a96d667834d2 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Fri, 13 Feb 2026 06:19:12 +0700 Subject: [PATCH] fix(cliproxy): keep variant updates atomic and persisted - require model when changing single-variant provider to avoid mixed config - rewrite provider-specific core env when provider changes while preserving custom fields - preserve custom composite settings path and return persisted composite PUT state --- src/cliproxy/services/variant-service.ts | 38 +++++- src/cliproxy/services/variant-settings.ts | 125 ++++++++++++++++++-- src/commands/cliproxy/variant-subcommand.ts | 9 +- src/web-server/routes/variant-routes.ts | 12 +- 4 files changed, 160 insertions(+), 24 deletions(-) diff --git a/src/cliproxy/services/variant-service.ts b/src/cliproxy/services/variant-service.ts index b9048d86..a2372961 100644 --- a/src/cliproxy/services/variant-service.ts +++ b/src/cliproxy/services/variant-service.ts @@ -26,6 +26,7 @@ import { getRelativeSettingsPath, getCompositeRelativeSettingsPath, updateSettingsModel, + updateSettingsProviderAndModel, } from './variant-settings'; import { VariantConfig, @@ -230,10 +231,30 @@ export function updateVariant(name: string, updates: UpdateVariantOptions): Vari return { success: false, error: 'Composite variant update not supported' }; } - // Update model in settings file if provided - if (updates.model !== undefined && existing.settings) { + const providerChanged = + updates.provider !== undefined && updates.provider !== existing.provider; + const hasModelUpdate = updates.model !== undefined && updates.model.trim().length > 0; + + if (providerChanged && !hasModelUpdate) { + return { + success: false, + error: 'Changing provider requires model update in the same request', + }; + } + + // Update settings file + if (existing.settings) { const settingsPath = existing.settings.replace(/^~/, os.homedir()); - updateSettingsModel(settingsPath, updates.model); + if (providerChanged) { + updateSettingsProviderAndModel( + settingsPath, + updates.provider as CLIProxyProfileName, + updates.model?.trim() || '', + existing.port + ); + } else if (updates.model !== undefined) { + updateSettingsModel(settingsPath, updates.model); + } } // Update config entry if provider or account changed @@ -272,7 +293,7 @@ export function updateVariant(name: string, updates: UpdateVariantOptions): Vari success: true, variant: { provider: updates.provider ?? existing.provider, - model: updates.model ?? existing.model, + model: updates.model?.trim() || existing.model, account: updates.account !== undefined ? updates.account : existing.account, port: existing.port, settings: existing.settings, @@ -414,12 +435,16 @@ export function updateCompositeVariant( } } + // Preserve existing settings path when configured; otherwise use default path. + const settingsRef = existing.settings || getCompositeRelativeSettingsPath(name); + // Create new settings file with updated config const settingsPath = createCompositeSettingsFile( name, mergedTiers, newDefaultTier, - existing.port + existing.port, + settingsRef ); // Save updated composite config to unified config @@ -427,7 +452,7 @@ export function updateCompositeVariant( type: 'composite', default_tier: newDefaultTier, tiers: mergedTiers, - settings: getCompositeRelativeSettingsPath(name), + settings: settingsRef, port: existing.port, }; saveCompositeVariantUnified(name, compositeConfig); @@ -441,6 +466,7 @@ export function updateCompositeVariant( default_tier: newDefaultTier, tiers: mergedTiers, port: existing.port, + settings: settingsRef, }, }; } catch (error) { diff --git a/src/cliproxy/services/variant-settings.ts b/src/cliproxy/services/variant-settings.ts index 7f37853d..211ff95c 100644 --- a/src/cliproxy/services/variant-settings.ts +++ b/src/cliproxy/services/variant-settings.ts @@ -21,6 +21,7 @@ import { warn } from '../../utils/ui'; /** Environment settings structure */ interface SettingsEnv { + [key: string]: string; ANTHROPIC_BASE_URL: string; ANTHROPIC_AUTH_TOKEN: string; ANTHROPIC_MODEL: string; @@ -30,7 +31,8 @@ interface SettingsEnv { } interface SettingsFile { - env: SettingsEnv; + env: Record; + [key: string]: unknown; } /** @@ -179,23 +181,64 @@ export function createCompositeSettingsFile( name: string, tiers: { opus: CompositeTierConfig; sonnet: CompositeTierConfig; haiku: CompositeTierConfig }, defaultTier: 'opus' | 'sonnet' | 'haiku', - port: number = CLIPROXY_DEFAULT_PORT + port: number = CLIPROXY_DEFAULT_PORT, + settingsPathOverride?: string ): string { const ccsDir = getCcsDir(); - const settingsPath = path.join(ccsDir, `composite-${name}.settings.json`); + const defaultSettingsPath = path.join(ccsDir, `composite-${name}.settings.json`); + const settingsPath = settingsPathOverride + ? (() => { + const expanded = expandPath(settingsPathOverride); + return path.isAbsolute(expanded) ? expanded : path.join(ccsDir, expanded); + })() + : defaultSettingsPath; + const settingsDir = path.dirname(settingsPath); - const settings: SettingsFile = { - env: buildCompositeSettingsEnv(tiers, defaultTier, port), - }; + const coreEnv = buildCompositeSettingsEnv(tiers, defaultTier, port); + let settings: SettingsFile = { env: coreEnv }; - ensureDir(ccsDir); + // Preserve non-core env vars and non-env fields (hooks/presets/etc.) when regenerating. + if (fs.existsSync(settingsPath)) { + try { + const content = fs.readFileSync(settingsPath, 'utf-8'); + const parsed = JSON.parse(content) as SettingsFile; + + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + const existingEnv = + parsed.env && typeof parsed.env === 'object' && !Array.isArray(parsed.env) + ? (parsed.env as Record) + : {}; + const { + ANTHROPIC_BASE_URL: _baseUrl, + ANTHROPIC_AUTH_TOKEN: _authToken, + ANTHROPIC_MODEL: _model, + ANTHROPIC_DEFAULT_OPUS_MODEL: _opus, + ANTHROPIC_DEFAULT_SONNET_MODEL: _sonnet, + ANTHROPIC_DEFAULT_HAIKU_MODEL: _haiku, + ...extraEnv + } = existingEnv; + + settings = { + ...parsed, + env: { + ...extraEnv, + ...coreEnv, + }, + }; + } + } catch { + // Invalid JSON — overwrite with a clean settings object. + } + } + + ensureDir(settingsDir); writeSettings(settingsPath, settings); - // Inject WebSearch hooks into variant settings - ensureProfileHooks(`composite-${name}`); - - // Inject Image Analyzer hooks into variant settings - ensureImageAnalyzerHooks(`composite-${name}`); + // Hook injectors target ~/.ccs/.settings.json; only run for default path. + if (path.resolve(settingsPath) === path.resolve(defaultSettingsPath)) { + ensureProfileHooks(`composite-${name}`); + ensureImageAnalyzerHooks(`composite-${name}`); + } return settingsPath; } @@ -258,3 +301,61 @@ export function updateSettingsModel(settingsPath: string, model: string): void { // Ignore errors - settings file may be invalid } } + +/** + * Update provider + model core env vars in an existing single-provider settings file. + * Preserves non-core env vars and top-level settings keys (hooks, presets, etc.). + */ +export function updateSettingsProviderAndModel( + settingsPath: string, + provider: CLIProxyProfileName, + model: string, + port: number = CLIPROXY_DEFAULT_PORT +): void { + const resolvedPath = expandPath(settingsPath); + const fileName = path.basename(resolvedPath); + if (fileName.startsWith('composite-')) { + console.log( + warn('Cannot update provider/model for composite variant. Edit config.yaml tiers directly.') + ); + return; + } + + const coreEnv = buildSettingsEnv(provider, model, port); + let settings: SettingsFile = { env: coreEnv }; + + if (fs.existsSync(resolvedPath)) { + try { + const content = fs.readFileSync(resolvedPath, 'utf8'); + const parsed = JSON.parse(content) as SettingsFile; + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + const existingEnv = + parsed.env && typeof parsed.env === 'object' && !Array.isArray(parsed.env) + ? (parsed.env as Record) + : {}; + const { + ANTHROPIC_BASE_URL: _baseUrl, + ANTHROPIC_AUTH_TOKEN: _authToken, + ANTHROPIC_MODEL: _model, + ANTHROPIC_DEFAULT_OPUS_MODEL: _opus, + ANTHROPIC_DEFAULT_SONNET_MODEL: _sonnet, + ANTHROPIC_DEFAULT_HAIKU_MODEL: _haiku, + ...extraEnv + } = existingEnv; + + settings = { + ...parsed, + env: { + ...extraEnv, + ...coreEnv, + }, + }; + } + } catch { + // Keep default and overwrite malformed file. + } + } + + ensureDir(path.dirname(resolvedPath)); + writeSettings(resolvedPath, settings); +} diff --git a/src/commands/cliproxy/variant-subcommand.ts b/src/commands/cliproxy/variant-subcommand.ts index 0c41ee3f..fa395445 100644 --- a/src/commands/cliproxy/variant-subcommand.ts +++ b/src/commands/cliproxy/variant-subcommand.ts @@ -522,7 +522,14 @@ export async function handleEdit( )) as CLIProxyProfileName; } - const changeModel = await InteractivePrompt.confirm('Change model?', { default: false }); + const providerChanged = !!(newProvider && newProvider !== variant.provider); + if (providerChanged) { + console.log(info('Provider changed. Model selection is required.')); + } + + const changeModel = providerChanged + ? true + : await InteractivePrompt.confirm('Change model?', { default: false }); let newModel = variant.model || ''; if (changeModel) { const providerForModel = newProvider || (variant.provider as CLIProxyProfileName); diff --git a/src/web-server/routes/variant-routes.ts b/src/web-server/routes/variant-routes.ts index 14aa4039..c21429ba 100644 --- a/src/web-server/routes/variant-routes.ts +++ b/src/web-server/routes/variant-routes.ts @@ -199,13 +199,14 @@ router.put('/:name', (req: Request, res: Response): void => { return; } + const persisted = result.variant; res.json({ name, type: 'composite', - default_tier, - tiers, - settings: result.variant?.settings, - port: result.variant?.port, + default_tier: persisted?.default_tier, + tiers: persisted?.tiers, + settings: persisted?.settings, + port: persisted?.port, updated: true, }); return; @@ -215,7 +216,8 @@ router.put('/:name', (req: Request, res: Response): void => { const result = updateVariant(name, { provider, account, model }); if (!result.success) { - res.status(404).json({ error: result.error }); + const status = result.error?.includes('not found') ? 404 : 400; + res.status(status).json({ error: result.error }); return; }