diff --git a/src/ccs.ts b/src/ccs.ts index 99c57ca3..e858ab1d 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -30,7 +30,7 @@ import { getGlobalEnvConfig } from './config/unified-config-loader'; import { ensureProfileHooks as ensureImageAnalyzerHooks } from './utils/hooks/image-analyzer-profile-hook-injector'; import { getImageAnalysisHookEnv } from './utils/hooks'; import { fail, info, warn } from './utils/ui'; -import { COPILOT_SUBCOMMAND_TOKENS } from './copilot/constants'; +import { isCopilotSubcommandToken, isLikelyCopilotFlagAlias } from './copilot/constants'; // Import centralized error handling import { handleError, runCleanup } from './errors'; @@ -573,12 +573,19 @@ async function main(): Promise { } // Special case: copilot command (GitHub Copilot integration) - // Only route to command handler for known subcommands, otherwise treat as profile - if (firstArg === 'copilot' && args.length > 1 && COPILOT_SUBCOMMAND_TOKENS.includes(args[1])) { - // `ccs copilot ` - route to copilot command handler - const { handleCopilotCommand } = await import('./commands/copilot-command'); - const exitCode = await handleCopilotCommand(args.slice(1)); - process.exit(exitCode); + // Route known subcommands and likely mistyped aliases (`--statu`) to command handler. + // Non-command Claude args still pass through profile flow. + if (firstArg === 'copilot' && args.length > 1) { + const copilotToken = args[1]; + const shouldRouteToCopilotCommand = + isCopilotSubcommandToken(copilotToken) || + (args.length === 2 && isLikelyCopilotFlagAlias(copilotToken)); + + if (shouldRouteToCopilotCommand) { + const { handleCopilotCommand } = await import('./commands/copilot-command'); + const exitCode = await handleCopilotCommand(args.slice(1)); + process.exit(exitCode); + } } // First-time install: offer setup wizard for interactive users @@ -955,7 +962,8 @@ async function main(): Promise { const exitCode = await executeCopilotProfile( copilotConfig, remainingArgs, - continuityInheritance.claudeConfigDir + continuityInheritance.claudeConfigDir, + claudeCli ); process.exit(exitCode); } else if (profileInfo.type === 'settings') { diff --git a/src/commands/copilot-command.ts b/src/commands/copilot-command.ts index 960a7328..2bdb2b1d 100644 --- a/src/commands/copilot-command.ts +++ b/src/commands/copilot-command.ts @@ -107,7 +107,8 @@ async function handleAuth(): Promise { console.log(''); console.log('Next steps:'); console.log(' 1. Enable copilot: ccs copilot enable'); - console.log(' 2. Start daemon: npx copilot-api start'); + console.log(' 2. Start daemon: ccs copilot start'); + console.log(' (fallback: npx copilot-api start)'); console.log(' 3. Use copilot: ccs copilot'); return 0; } else { diff --git a/src/copilot/constants.ts b/src/copilot/constants.ts index c4c0b42f..cc594afd 100644 --- a/src/copilot/constants.ts +++ b/src/copilot/constants.ts @@ -41,3 +41,27 @@ export function normalizeCopilotSubcommand(token?: string): string | undefined { const alias = COPILOT_FLAG_ALIASES[token as keyof typeof COPILOT_FLAG_ALIASES]; return alias || token; } + +export function isCopilotSubcommandToken(token?: string): boolean { + return Boolean(token) && COPILOT_SUBCOMMAND_TOKENS.includes(token as string); +} + +/** + * Detect likely mistyped copilot flag aliases (e.g. `--statu`). + * This helps entrypoint routing show command help instead of falling through + * to profile execution for obvious copilot-command typos. + */ +export function isLikelyCopilotFlagAlias(token?: string): boolean { + if (!token || !token.startsWith('--') || token === '--') { + return false; + } + + const alias = token.slice(2).toLowerCase(); + if (!/^[a-z][a-z0-9-]*$/.test(alias)) { + return false; + } + + return COPILOT_SUBCOMMANDS.some( + (subcommand) => subcommand.startsWith(alias) || alias.startsWith(subcommand) + ); +} diff --git a/src/copilot/copilot-daemon.ts b/src/copilot/copilot-daemon.ts index 29116722..c8cc09fe 100644 --- a/src/copilot/copilot-daemon.ts +++ b/src/copilot/copilot-daemon.ts @@ -10,11 +10,28 @@ import * as fs from 'fs'; import * as path from 'path'; import * as http from 'http'; import { CopilotDaemonStatus } from './types'; -import { CopilotConfig } from '../config/unified-config-types'; +import { CopilotConfig, DEFAULT_COPILOT_CONFIG } from '../config/unified-config-types'; +import { loadOrCreateUnifiedConfig } from '../config/unified-config-loader'; import { getCopilotDir, getCopilotApiBinPath } from './copilot-package-manager'; import { verifyProcessOwnership } from '../cursor/daemon-process-ownership'; const DAEMON_HEALTH_MARKER = 'server running'; +const MIN_PORT = 1; +const MAX_PORT = 65535; + +function isValidPort(port: number): boolean { + return Number.isInteger(port) && port >= MIN_PORT && port <= MAX_PORT; +} + +function getConfiguredCopilotPort(): number { + try { + const config = loadOrCreateUnifiedConfig(); + const port = config.copilot?.port ?? DEFAULT_COPILOT_CONFIG.port; + return isValidPort(port) ? port : DEFAULT_COPILOT_CONFIG.port; + } catch { + return DEFAULT_COPILOT_CONFIG.port; + } +} function getPidFilePath(): string { return path.join(getCopilotDir(), 'daemon.pid'); @@ -25,6 +42,10 @@ function getPidFilePath(): string { * Uses 127.0.0.1 instead of localhost for more reliable local connections. */ export async function isDaemonRunning(port: number): Promise { + if (!isValidPort(port)) { + return false; + } + return new Promise((resolve) => { const req = http.request( { @@ -136,6 +157,13 @@ function removePidFile(): void { export async function startDaemon( config: CopilotConfig ): Promise<{ success: boolean; pid?: number; error?: string }> { + if (!isValidPort(config.port)) { + return { + success: false, + error: `Invalid Copilot daemon port ${config.port}. Expected integer between ${MIN_PORT} and ${MAX_PORT}.`, + }; + } + // Check if already running if (await isDaemonRunning(config.port)) { return { success: true, pid: getPidFromFile() ?? undefined }; @@ -250,6 +278,7 @@ export async function startDaemon( */ export async function stopDaemon(): Promise<{ success: boolean; error?: string }> { const pid = getPidFromFile(); + const configuredPort = getConfiguredCopilotPort(); if (!pid) { // No PID file, try to find by port @@ -260,8 +289,12 @@ export async function stopDaemon(): Promise<{ success: boolean; error?: string } try { const ownership = verifyProcessOwnership(pid, (commandLine) => { const lower = commandLine.toLowerCase(); + const hasCopilotApiBinary = /(^|[\\/\s])copilot-api(\.cmd|\.exe)?(\s|$)/.test(lower); + const hasStartCommand = /\bstart\b/.test(lower); + const hasExpectedPort = + lower.includes(`--port ${configuredPort}`) || lower.includes(`--port=${configuredPort}`); // copilot-api is launched as `... copilot-api start --port ` - return lower.includes('copilot-api') && lower.includes(' start'); + return hasCopilotApiBinary && hasStartCommand && hasExpectedPort; }); if (ownership === 'not-running') { removePidFile(); @@ -270,11 +303,23 @@ export async function stopDaemon(): Promise<{ success: boolean; error?: string } if (ownership === 'not-owned') { // PID was reused by an unrelated process. + // If daemon is still live on configured port, report failure (stop not completed). + if (await isDaemonRunning(configuredPort)) { + return { + success: false, + error: `Refusing to clear PID ${pid}: unrelated process owns PID and daemon is still responding on port ${configuredPort}`, + }; + } removePidFile(); return { success: true }; } if (ownership === 'unknown') { + // If daemon is not reachable, allow stale PID cleanup. + if (!(await isDaemonRunning(configuredPort))) { + removePidFile(); + return { success: true }; + } return { success: false, error: `Refusing to stop PID ${pid}: unable to verify daemon ownership`, diff --git a/src/copilot/copilot-executor.ts b/src/copilot/copilot-executor.ts index 8bfa3ea5..7225b5ed 100644 --- a/src/copilot/copilot-executor.ts +++ b/src/copilot/copilot-executor.ts @@ -76,7 +76,8 @@ export function generateCopilotEnv( export async function executeCopilotProfile( config: CopilotConfig, claudeArgs: string[], - claudeConfigDir?: string + claudeConfigDir?: string, + claudeCliPath: string = 'claude' ): Promise { // Ensure copilot-api is installed (auto-install if missing, auto-update if outdated) try { @@ -125,7 +126,9 @@ export async function executeCopilotProfile( } else { console.error(fail('copilot-api daemon is not running.')); console.error(''); - console.error('Start the daemon manually:'); + console.error('Start the daemon:'); + console.error(' ccs copilot start'); + console.error('Fallback manual command:'); console.error(` npx copilot-api start --port ${config.port}`); console.error(''); console.error('Or enable auto_start in config:'); @@ -158,7 +161,7 @@ export async function executeCopilotProfile( // Spawn Claude CLI return new Promise((resolve) => { - const proc = spawn('claude', claudeArgs, { + const proc = spawn(claudeCliPath, claudeArgs, { stdio: 'inherit', env, shell: process.platform === 'win32', diff --git a/src/web-server/routes/copilot-routes.ts b/src/web-server/routes/copilot-routes.ts index 3d4bae9e..31cf897c 100644 --- a/src/web-server/routes/copilot-routes.ts +++ b/src/web-server/routes/copilot-routes.ts @@ -27,6 +27,18 @@ const router = Router(); // Mount settings sub-routes router.use('/settings', copilotSettingsRoutes); +function parseRequiredModel(value: unknown): string | null { + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +function parseOptionalModel(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + /** * GET /api/copilot/status - Get Copilot status (auth + daemon + install info) */ @@ -75,31 +87,133 @@ router.get('/config', (_req: Request, res: Response): void => { router.put('/config', (req: Request, res: Response): void => { try { const updates = req.body; + if (!updates || typeof updates !== 'object' || Array.isArray(updates)) { + res.status(400).json({ error: 'Request body must be a JSON object' }); + return; + } + const payload = updates as Record; + + if ('port' in payload) { + if (typeof payload.port !== 'number' || !Number.isInteger(payload.port)) { + res.status(400).json({ error: 'port must be an integer' }); + return; + } + if (payload.port < 1 || payload.port > 65535) { + res.status(400).json({ error: 'port must be between 1 and 65535' }); + return; + } + } + + if ('enabled' in payload && typeof payload.enabled !== 'boolean') { + res.status(400).json({ error: 'enabled must be a boolean' }); + return; + } + + if ('auto_start' in payload && typeof payload.auto_start !== 'boolean') { + res.status(400).json({ error: 'auto_start must be a boolean' }); + return; + } + + if ('wait_on_limit' in payload && typeof payload.wait_on_limit !== 'boolean') { + res.status(400).json({ error: 'wait_on_limit must be a boolean' }); + return; + } + + if ( + 'account_type' in payload && + payload.account_type !== 'individual' && + payload.account_type !== 'business' && + payload.account_type !== 'enterprise' + ) { + res.status(400).json({ error: 'account_type must be individual, business, or enterprise' }); + return; + } + + if ('rate_limit' in payload) { + if (payload.rate_limit !== null) { + if (typeof payload.rate_limit !== 'number' || !Number.isInteger(payload.rate_limit)) { + res.status(400).json({ error: 'rate_limit must be an integer or null' }); + return; + } + if (payload.rate_limit < 0) { + res.status(400).json({ error: 'rate_limit must be >= 0 or null' }); + return; + } + } + } + + const normalizedModel = parseRequiredModel(payload.model); + if ('model' in payload && !normalizedModel) { + res.status(400).json({ error: 'model must be a non-empty string' }); + return; + } + + if ( + 'opus_model' in payload && + payload.opus_model !== undefined && + payload.opus_model !== null && + typeof payload.opus_model !== 'string' + ) { + res.status(400).json({ error: 'opus_model must be a string' }); + return; + } + + if ( + 'sonnet_model' in payload && + payload.sonnet_model !== undefined && + payload.sonnet_model !== null && + typeof payload.sonnet_model !== 'string' + ) { + res.status(400).json({ error: 'sonnet_model must be a string' }); + return; + } + + if ( + 'haiku_model' in payload && + payload.haiku_model !== undefined && + payload.haiku_model !== null && + typeof payload.haiku_model !== 'string' + ) { + res.status(400).json({ error: 'haiku_model must be a string' }); + return; + } + const config = loadOrCreateUnifiedConfig(); // Merge updates with existing config config.copilot = { - enabled: updates.enabled ?? config.copilot?.enabled ?? DEFAULT_COPILOT_CONFIG.enabled, + enabled: + (payload.enabled as boolean) ?? config.copilot?.enabled ?? DEFAULT_COPILOT_CONFIG.enabled, auto_start: - updates.auto_start ?? config.copilot?.auto_start ?? DEFAULT_COPILOT_CONFIG.auto_start, - port: updates.port ?? config.copilot?.port ?? DEFAULT_COPILOT_CONFIG.port, + (payload.auto_start as boolean) ?? + config.copilot?.auto_start ?? + DEFAULT_COPILOT_CONFIG.auto_start, + port: (payload.port as number) ?? config.copilot?.port ?? DEFAULT_COPILOT_CONFIG.port, account_type: - updates.account_type ?? config.copilot?.account_type ?? DEFAULT_COPILOT_CONFIG.account_type, + (payload.account_type as 'individual' | 'business' | 'enterprise') ?? + config.copilot?.account_type ?? + DEFAULT_COPILOT_CONFIG.account_type, rate_limit: - updates.rate_limit !== undefined - ? updates.rate_limit + payload.rate_limit !== undefined + ? (payload.rate_limit as number | null) : (config.copilot?.rate_limit ?? DEFAULT_COPILOT_CONFIG.rate_limit), wait_on_limit: - updates.wait_on_limit ?? + (payload.wait_on_limit as boolean) ?? config.copilot?.wait_on_limit ?? DEFAULT_COPILOT_CONFIG.wait_on_limit, - model: updates.model ?? config.copilot?.model ?? DEFAULT_COPILOT_CONFIG.model, + model: normalizedModel ?? config.copilot?.model ?? DEFAULT_COPILOT_CONFIG.model, opus_model: - updates.opus_model !== undefined ? updates.opus_model : config.copilot?.opus_model, + 'opus_model' in payload + ? parseOptionalModel(payload.opus_model) + : config.copilot?.opus_model, sonnet_model: - updates.sonnet_model !== undefined ? updates.sonnet_model : config.copilot?.sonnet_model, + 'sonnet_model' in payload + ? parseOptionalModel(payload.sonnet_model) + : config.copilot?.sonnet_model, haiku_model: - updates.haiku_model !== undefined ? updates.haiku_model : config.copilot?.haiku_model, + 'haiku_model' in payload + ? parseOptionalModel(payload.haiku_model) + : config.copilot?.haiku_model, }; saveUnifiedConfig(config); diff --git a/tests/unit/copilot/copilot-command-aliases.test.ts b/tests/unit/copilot/copilot-command-aliases.test.ts index 4cc4b7ae..cb8c19ca 100644 --- a/tests/unit/copilot/copilot-command-aliases.test.ts +++ b/tests/unit/copilot/copilot-command-aliases.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'bun:test'; import { COPILOT_SUBCOMMANDS, COPILOT_SUBCOMMAND_TOKENS, + isLikelyCopilotFlagAlias, normalizeCopilotSubcommand, } from '../../../src/copilot/constants'; @@ -23,6 +24,13 @@ describe('copilot command aliases', () => { expect(normalizeCopilotSubcommand('unknown')).toBe('unknown'); }); + it('detects likely mistyped command aliases for entrypoint routing', () => { + expect(isLikelyCopilotFlagAlias('--statu')).toBe(true); + expect(isLikelyCopilotFlagAlias('--enabl')).toBe(true); + expect(isLikelyCopilotFlagAlias('--print')).toBe(false); + expect(isLikelyCopilotFlagAlias('--')).toBe(false); + }); + it('exposes complete routing token list for ccs entrypoint', () => { for (const subcommand of COPILOT_SUBCOMMANDS) { expect(COPILOT_SUBCOMMAND_TOKENS).toContain(subcommand); diff --git a/tests/unit/copilot/copilot-daemon.test.ts b/tests/unit/copilot/copilot-daemon.test.ts index ad8e6b6f..fda794fd 100644 --- a/tests/unit/copilot/copilot-daemon.test.ts +++ b/tests/unit/copilot/copilot-daemon.test.ts @@ -4,7 +4,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { spawn } from 'child_process'; -import { isDaemonRunning, stopDaemon } from '../../../src/copilot/copilot-daemon'; +import { isDaemonRunning, startDaemon, stopDaemon } from '../../../src/copilot/copilot-daemon'; +import { DEFAULT_COPILOT_CONFIG } from '../../../src/config/unified-config-types'; import { getCcsDir } from '../../../src/utils/config-manager'; const activeServers: http.Server[] = []; @@ -58,7 +59,18 @@ async function createServer( return address.port; } +function writeCopilotPortConfig(port: number): void { + const configPath = path.join(getCcsDir(), 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, `version: 8\ncopilot:\n port: ${port}\n`); +} + describe('copilot daemon health detection', () => { + it('returns false for invalid port inputs', async () => { + expect(await isDaemonRunning(0)).toBe(false); + expect(await isDaemonRunning(65536)).toBe(false); + }); + it('returns false when no daemon is running on port', async () => { const running = await isDaemonRunning(19998); expect(running).toBe(false); @@ -103,10 +115,26 @@ describe('copilot daemon health detection', () => { const running = await isDaemonRunning(port); expect(running).toBe(false); }); + + it('fails fast when startDaemon is called with invalid port', async () => { + const result = await startDaemon({ + ...DEFAULT_COPILOT_CONFIG, + port: 70000, + }); + expect(result.success).toBe(false); + expect(result.error).toContain('Invalid Copilot daemon port'); + }); }); describe('copilot daemon stop safety', () => { - it('does not terminate unrelated process from stale PID file', async () => { + it('returns failure when stale PID points to unrelated process but daemon is still live', async () => { + const port = await createServer((_req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('Server running'); + }); + + writeCopilotPortConfig(port); + const unrelatedProcess = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000);'], { detached: true, stdio: 'ignore', @@ -125,9 +153,42 @@ describe('copilot daemon stop safety', () => { try { const result = await stopDaemon(); - if (!result.success) { - expect(result.error).toContain('unable to verify daemon ownership'); + expect(result.success).toBe(false); + expect(result.error).toContain('daemon is still responding on port'); + + // Unrelated process should still be alive. + expect(() => process.kill(unrelatedPid, 0)).not.toThrow(); + } finally { + try { + process.kill(unrelatedPid, 'SIGTERM'); + } catch { + // Process already exited. } + } + }); + + it('does not terminate unrelated process from stale PID file', async () => { + writeCopilotPortConfig(65534); + + const unrelatedProcess = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000);'], { + detached: true, + stdio: 'ignore', + }); + unrelatedProcess.unref(); + + const unrelatedPid = unrelatedProcess.pid; + expect(unrelatedPid).toBeDefined(); + if (!unrelatedPid) { + throw new Error('Failed to spawn unrelated process'); + } + + const pidFile = path.join(getCcsDir(), 'copilot', 'daemon.pid'); + fs.mkdirSync(path.dirname(pidFile), { recursive: true }); + fs.writeFileSync(pidFile, String(unrelatedPid)); + + try { + const result = await stopDaemon(); + expect(result.success).toBe(true); // Unrelated process should still be alive. expect(() => process.kill(unrelatedPid, 0)).not.toThrow();