From ca78993e7612143b3193e3cec3f8976be909e2d6 Mon Sep 17 00:00:00 2001 From: kaitranntt Date: Tue, 23 Dec 2025 22:34:32 -0500 Subject: [PATCH] fix(config): improve edge case handling for config initialization - setup-command.ts: Add Ctrl+C handling (UserCancelledError), port validation (1-65535), protocol stripping from host, try-catch with user-friendly error messages - postinstall.js: Add ~/.ccs file check (not directory), wrap js-yaml require in try-catch with JSON fallback, validate config.json before migration, warn when both config files exist - recovery-manager.ts: Verify config is loadable (not just exists), add nested error handling for fallback write - ccs.ts: Make first-time install hint independent of recovery status (shows even when user manually created empty config.yaml) All edge cases identified by code review addressed. --- scripts/postinstall.js | 112 +++++++++++++++++++---------- src/ccs.ts | 19 ++--- src/commands/setup-command.ts | 69 ++++++++++++++++-- src/management/recovery-manager.ts | 36 +++++++--- 4 files changed, 173 insertions(+), 63 deletions(-) diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 0da1a18b..46c6bbf0 100755 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -93,7 +93,15 @@ function createConfigFiles() { const ccsDir = path.join(homedir, '.ccs'); // Create ~/.ccs/ directory if missing - if (!fs.existsSync(ccsDir)) { + if (fs.existsSync(ccsDir)) { + // Check if it's a file instead of directory (edge case) + const stats = fs.statSync(ccsDir); + if (!stats.isDirectory()) { + console.error('[X] ~/.ccs exists but is not a directory'); + console.error(' Remove or rename it: mv ~/.ccs ~/.ccs.bak'); + process.exit(1); + } + } else { fs.mkdirSync(ccsDir, { recursive: true, mode: 0o755 }); console.log('[OK] Created directory: ~/.ccs/'); } @@ -149,8 +157,22 @@ function createConfigFiles() { if (!fs.existsSync(configYamlPath)) { // Check for legacy config.json - autoMigrate() in ccs.ts will handle migration if (fs.existsSync(legacyConfigPath)) { - console.log('[OK] Legacy config.json found - will migrate to config.yaml on first run'); - } else { + // Validate legacy config.json before assuming migration will work + try { + const content = fs.readFileSync(legacyConfigPath, 'utf8'); + JSON.parse(content); + console.log('[OK] Legacy config.json found - will migrate to config.yaml on first run'); + } catch { + console.warn('[!] Legacy config.json is corrupted/invalid'); + console.warn(' Backup: mv ~/.ccs/config.json ~/.ccs/config.json.bak'); + console.warn(' Creating fresh config.yaml instead'); + // Fall through to create new config.yaml + fs.renameSync(legacyConfigPath, `${legacyConfigPath}.bak`); + } + } + + // Create config.yaml if it doesn't exist (and legacy wasn't valid) + if (!fs.existsSync(configYamlPath) && !fs.existsSync(legacyConfigPath)) { // Try to use unified config loader if dist is available try { const { saveUnifiedConfig } = require('../dist/config/unified-config-loader'); @@ -163,42 +185,58 @@ function createConfigFiles() { console.log('[OK] Created config: ~/.ccs/config.yaml'); } catch (loaderErr) { // Dist not built yet (fresh clone) - create minimal config.yaml manually - const yaml = require('js-yaml'); - const config = { - version: '2.0', - profiles: {}, - accounts: {}, - cliproxy: { - variants: {}, - oauth_accounts: {} - }, - cliproxy_server: { - local: { - port: 8317, - auto_start: true - } - } - }; - + // Wrap js-yaml require in try-catch in case it's not available + let yaml; try { - const yamlContent = yaml.dump(config, { - indent: 2, - lineWidth: -1, - noRefs: true, - sortKeys: false - }); - const tmpPath = `${configYamlPath}.tmp`; - fs.writeFileSync(tmpPath, yamlContent, 'utf8'); - fs.renameSync(tmpPath, configYamlPath); - console.log('[OK] Created config: ~/.ccs/config.yaml'); - } catch (yamlErr) { - // Final fallback: create legacy config.json - console.warn('[!] YAML write failed, creating legacy config.json'); + yaml = require('js-yaml'); + } catch { + // js-yaml not available - fallback to JSON + console.warn('[!] js-yaml not available, creating legacy config.json'); const fallbackConfig = { profiles: {} }; const tmpPath = `${legacyConfigPath}.tmp`; fs.writeFileSync(tmpPath, JSON.stringify(fallbackConfig, null, 2) + '\n', 'utf8'); fs.renameSync(tmpPath, legacyConfigPath); console.log('[OK] Created config: ~/.ccs/config.json (fallback)'); + yaml = null; + } + + if (yaml) { + const config = { + version: '2.0', + profiles: {}, + accounts: {}, + cliproxy: { + variants: {}, + oauth_accounts: {} + }, + cliproxy_server: { + local: { + port: 8317, + auto_start: true + } + } + }; + + try { + const yamlContent = yaml.dump(config, { + indent: 2, + lineWidth: -1, + noRefs: true, + sortKeys: false + }); + const tmpPath = `${configYamlPath}.tmp`; + fs.writeFileSync(tmpPath, yamlContent, 'utf8'); + fs.renameSync(tmpPath, configYamlPath); + console.log('[OK] Created config: ~/.ccs/config.yaml'); + } catch (yamlErr) { + // Final fallback: create legacy config.json + console.warn('[!] YAML write failed, creating legacy config.json'); + const fallbackConfig = { profiles: {} }; + const tmpPath = `${legacyConfigPath}.tmp`; + fs.writeFileSync(tmpPath, JSON.stringify(fallbackConfig, null, 2) + '\n', 'utf8'); + fs.renameSync(tmpPath, legacyConfigPath); + console.log('[OK] Created config: ~/.ccs/config.json (fallback)'); + } } } } @@ -206,10 +244,10 @@ function createConfigFiles() { console.log('[OK] Config exists: ~/.ccs/config.yaml (preserved)'); } - // Handle legacy config.json migrations (for users upgrading) - if (fs.existsSync(legacyConfigPath) && !fs.existsSync(configYamlPath)) { - // Migration will happen via autoMigrate() in ccs.ts on first run - console.log('[i] Legacy config.json will be migrated to config.yaml on first run'); + // Warn if both config files exist (user may want to clean up) + if (fs.existsSync(legacyConfigPath) && fs.existsSync(configYamlPath)) { + console.log('[!] Both config.yaml and config.json exist'); + console.log(' config.json will be ignored - consider removing it'); } // NOTE: GLM, GLMT, and Kimi profiles are NO LONGER auto-created during install diff --git a/src/ccs.ts b/src/ccs.ts index 25dd7171..ced3fc28 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -448,16 +448,17 @@ async function main(): Promise { if (recovered) { recovery.showRecoveryHints(); + } - // First-time install: offer setup wizard for interactive users - // Skip if headless, CI, or non-TTY environment - const { isFirstTimeInstall } = await import('./commands/setup-command'); - if (process.stdout.isTTY && !process.env['CI'] && isFirstTimeInstall()) { - console.log(''); - console.log(info('First-time install detected. Run `ccs setup` for guided configuration.')); - console.log(' Or use `ccs config` for the web dashboard.'); - console.log(''); - } + // First-time install: offer setup wizard for interactive users + // Check independently of recovery status (user may have empty config.yaml) + // Skip if headless, CI, or non-TTY environment + const { isFirstTimeInstall } = await import('./commands/setup-command'); + if (process.stdout.isTTY && !process.env['CI'] && isFirstTimeInstall()) { + console.log(''); + console.log(info('First-time install detected. Run `ccs setup` for guided configuration.')); + console.log(' Or use `ccs config` for the web dashboard.'); + console.log(''); } // Detect profile diff --git a/src/commands/setup-command.ts b/src/commands/setup-command.ts index ff867eaf..dc0c5801 100644 --- a/src/commands/setup-command.ts +++ b/src/commands/setup-command.ts @@ -16,11 +16,20 @@ import * as readline from 'readline'; import { initUI, header, ok, info, warn } from '../utils/ui'; import { loadOrCreateUnifiedConfig, + loadUnifiedConfig, saveUnifiedConfig, hasUnifiedConfig, } from '../config/unified-config-loader'; import { DEFAULT_CLIPROXY_SERVER_CONFIG } from '../config/unified-config-types'; +/** Custom error for user cancellation (Ctrl+C) */ +class UserCancelledError extends Error { + constructor() { + super('Setup cancelled by user'); + this.name = 'UserCancelledError'; + } +} + /** * Create readline interface for interactive prompts */ @@ -33,15 +42,23 @@ function createReadline(): readline.Interface { /** * Prompt user for input with optional default value + * Handles Ctrl+C gracefully by rejecting with UserCancelledError */ async function prompt( rl: readline.Interface, question: string, defaultValue?: string ): Promise { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const displayQuestion = defaultValue ? `${question} [${defaultValue}]: ` : `${question}: `; + + const onClose = () => { + reject(new UserCancelledError()); + }; + + rl.once('close', onClose); rl.question(displayQuestion, (answer) => { + rl.removeListener('close', onClose); resolve(answer.trim() || defaultValue || ''); }); }); @@ -102,8 +119,18 @@ export function isFirstTimeInstall(): boolean { return true; } - // Config exists - check if it's meaningfully configured - const config = loadOrCreateUnifiedConfig(); + // Try loading config directly to detect corruption + const loaded = loadUnifiedConfig(); + if (loaded === null) { + // Config exists but is corrupted/invalid - don't treat as first-time + // User should fix or delete the file, or use --force + console.log(warn('Warning: ~/.ccs/config.yaml exists but appears corrupted')); + console.log(info(' Run `ccs setup --force` to reset, or `ccs doctor` to diagnose')); + return false; + } + + // Config exists and is valid - check if it's meaningfully configured + const config = loaded; // Check for any meaningful configuration const hasProfiles = Object.keys(config.profiles || {}).length > 0; @@ -136,11 +163,15 @@ async function configureRemoteProxy(rl: readline.Interface): Promise<{ console.log(' Example: your-server.example.com'); console.log(''); - // Host - const host = await prompt(rl, 'Remote host (hostname or IP)'); + // Host - with protocol stripping + let host = await prompt(rl, 'Remote host (hostname or IP)'); if (!host) { throw new Error('Host is required for remote proxy mode'); } + // Strip protocol if user included it (common mistake) + host = host.replace(/^https?:\/\//, ''); + // Strip trailing slashes + host = host.replace(/\/+$/, ''); // Protocol const protocol = (await selectOption(rl, 'Protocol:', [ @@ -148,10 +179,19 @@ async function configureRemoteProxy(rl: readline.Interface): Promise<{ { label: 'HTTP', value: 'http', description: 'Unencrypted connection' }, ])) as 'http' | 'https'; - // Port (optional) + // Port (optional) - with validation const defaultPort = protocol === 'https' ? '443' : '80'; const portStr = await prompt(rl, `Port (leave empty for default ${defaultPort})`); - const port = portStr ? parseInt(portStr, 10) : undefined; + let port: number | undefined; + if (portStr) { + const parsed = parseInt(portStr, 10); + if (isNaN(parsed) || parsed < 1 || parsed > 65535 || !Number.isInteger(parsed)) { + console.log(warn(`Invalid port "${portStr}", using default: ${defaultPort}`)); + port = undefined; // Use default + } else { + port = parsed; + } + } // Auth token console.log(''); @@ -320,6 +360,21 @@ async function runSetupWizard(force: boolean = false): Promise { console.log(info('Configuration saved to: ~/.ccs/config.yaml')); console.log(''); + } catch (err) { + // Handle user cancellation gracefully + if (err instanceof UserCancelledError) { + console.log(''); + console.log(info('Setup cancelled.')); + console.log(' Run `ccs setup` when ready to configure.'); + console.log(''); + return; + } + // Handle other errors with user-friendly message + const message = err instanceof Error ? err.message : String(err); + console.log(''); + console.log(warn(`Setup failed: ${message}`)); + console.log(info(' Run `ccs setup` to try again.')); + console.log(''); } finally { rl.close(); } diff --git a/src/management/recovery-manager.ts b/src/management/recovery-manager.ts index b93621d3..ec2169a2 100644 --- a/src/management/recovery-manager.ts +++ b/src/management/recovery-manager.ts @@ -9,7 +9,11 @@ import * as path from 'path'; import * as os from 'os'; import { info } from '../utils/ui'; import { createEmptyUnifiedConfig, UNIFIED_CONFIG_VERSION } from '../config/unified-config-types'; -import { saveUnifiedConfig, hasUnifiedConfig } from '../config/unified-config-loader'; +import { + saveUnifiedConfig, + hasUnifiedConfig, + loadUnifiedConfig, +} from '../config/unified-config-loader'; /** * Get CCS home directory (respects CCS_HOME env for test isolation) @@ -55,9 +59,15 @@ class RecoveryManager { * This is the primary config format (YAML unified config) */ ensureConfigYaml(): boolean { - // Skip if config.yaml already exists + // Skip if config.yaml already exists AND is valid if (hasUnifiedConfig()) { - return false; + // Verify it's loadable (not corrupted) + const loaded = loadUnifiedConfig(); + if (loaded !== null) { + return false; // Config exists and is valid + } + // Config exists but is corrupted - will be recreated below + this.recovered.push('Detected corrupted ~/.ccs/config.yaml'); } // Check for legacy config.json - if exists, let autoMigrate handle it @@ -75,14 +85,20 @@ class RecoveryManager { saveUnifiedConfig(config); this.recovered.push('Created ~/.ccs/config.yaml'); return true; - } catch (_e) { + } catch (_saveErr) { // Fallback: create minimal config.json for backward compat - const fallbackConfig = { profiles: {} }; - const tmpPath = `${legacyConfigPath}.tmp`; - fs.writeFileSync(tmpPath, JSON.stringify(fallbackConfig, null, 2) + '\n', 'utf8'); - fs.renameSync(tmpPath, legacyConfigPath); - this.recovered.push('Created ~/.ccs/config.json (fallback)'); - return true; + try { + const fallbackConfig = { profiles: {} }; + const tmpPath = `${legacyConfigPath}.tmp`; + fs.writeFileSync(tmpPath, JSON.stringify(fallbackConfig, null, 2) + '\n', 'utf8'); + fs.renameSync(tmpPath, legacyConfigPath); + this.recovered.push('Created ~/.ccs/config.json (fallback)'); + return true; + } catch (_fallbackErr) { + // Both writes failed - log but don't crash + this.recovered.push('Failed to create config file (permission issue?)'); + return false; + } } }