From 4b1cda25d945e6482be68804907177a8ad38489e Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 4 Mar 2026 16:13:44 +0700 Subject: [PATCH 1/5] fix(copilot): improve daemon liveness and flag aliases --- src/ccs.ts | 8 +++ src/commands/copilot-command.ts | 13 +++- src/copilot/copilot-daemon.ts | 73 +++++++++++++++++++---- tests/unit/copilot/copilot-daemon.test.ts | 71 ++++++++++++++++++++++ 4 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 tests/unit/copilot/copilot-daemon.test.ts diff --git a/src/ccs.ts b/src/ccs.ts index 49e5c898..8790e839 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -582,6 +582,14 @@ async function main(): Promise { 'stop', 'enable', 'disable', + '--auth', + '--status', + '--models', + '--usage', + '--start', + '--stop', + '--enable', + '--disable', 'help', '--help', '-h', diff --git a/src/commands/copilot-command.ts b/src/commands/copilot-command.ts index 7ce38232..7184a120 100644 --- a/src/commands/copilot-command.ts +++ b/src/commands/copilot-command.ts @@ -21,7 +21,18 @@ import { ok, fail, info, color } from '../utils/ui'; * Handle copilot subcommand. */ export async function handleCopilotCommand(args: string[]): Promise { - const subcommand = args[0]; + const subcommandAliasMap: Record = { + '--auth': 'auth', + '--status': 'status', + '--models': 'models', + '--usage': 'usage', + '--start': 'start', + '--stop': 'stop', + '--enable': 'enable', + '--disable': 'disable', + }; + const rawSubcommand = args[0]; + const subcommand = (rawSubcommand && subcommandAliasMap[rawSubcommand]) || rawSubcommand; switch (subcommand) { case 'auth': diff --git a/src/copilot/copilot-daemon.ts b/src/copilot/copilot-daemon.ts index 36d492f7..dc6cbbef 100644 --- a/src/copilot/copilot-daemon.ts +++ b/src/copilot/copilot-daemon.ts @@ -25,12 +25,26 @@ export async function isDaemonRunning(port: number): Promise { { hostname: '127.0.0.1', port, - path: '/usage', + path: '/', method: 'GET', timeout: 3000, }, (res) => { - resolve(res.statusCode === 200); + let body = ''; + res.setEncoding('utf8'); + + res.on('data', (chunk) => { + body += chunk; + }); + + res.on('end', () => { + if (res.statusCode !== 200) { + resolve(false); + return; + } + + resolve(body.trim().toLowerCase().includes('server running')); + }); } ); @@ -137,6 +151,20 @@ export async function startDaemon( return new Promise((resolve) => { let proc: ChildProcess; + let resolved = false; + let checkTimeout: NodeJS.Timeout | null = null; + + const safeResolve = (result: { success: boolean; pid?: number; error?: string }) => { + if (resolved) return; + resolved = true; + if (checkTimeout) { + clearTimeout(checkTimeout); + } + if (!result.success) { + removePidFile(); + } + resolve(result); + }; try { proc = spawn(binPath, args, { @@ -155,30 +183,53 @@ export async function startDaemon( // Wait for daemon to be ready (poll for up to 30 seconds) let attempts = 0; const maxAttempts = 30; - const checkInterval = setInterval(async () => { + const pollHealth = async () => { + if (resolved) return; attempts++; if (await isDaemonRunning(config.port)) { - clearInterval(checkInterval); - resolve({ success: true, pid: proc.pid }); + safeResolve({ success: true, pid: proc.pid }); } else if (attempts >= maxAttempts) { - clearInterval(checkInterval); - resolve({ + if (proc.pid) { + try { + process.kill(proc.pid, 'SIGTERM'); + } catch { + // Already exited + } + } + safeResolve({ success: false, error: 'Daemon did not start within 30 seconds', }); + } else { + checkTimeout = setTimeout(pollHealth, 1000); } - }, 1000); + }; + checkTimeout = setTimeout(pollHealth, 1000); proc.on('error', (err) => { - clearInterval(checkInterval); - resolve({ + safeResolve({ success: false, error: `Failed to start daemon: ${err.message}`, }); }); + + proc.on('exit', (code, signal) => { + if (code === null) { + safeResolve({ + success: false, + error: `Daemon process was killed by signal ${signal}`, + }); + return; + } + + safeResolve({ + success: false, + error: `Daemon process exited with code ${code}`, + }); + }); } catch (err) { - resolve({ + safeResolve({ success: false, error: `Failed to spawn daemon: ${(err as Error).message}`, }); diff --git a/tests/unit/copilot/copilot-daemon.test.ts b/tests/unit/copilot/copilot-daemon.test.ts new file mode 100644 index 00000000..9de0c02e --- /dev/null +++ b/tests/unit/copilot/copilot-daemon.test.ts @@ -0,0 +1,71 @@ +import { afterEach, describe, expect, it } from 'bun:test'; +import * as http from 'http'; +import { isDaemonRunning } from '../../../src/copilot/copilot-daemon'; + +const activeServers: http.Server[] = []; + +afterEach(async () => { + await Promise.all( + activeServers.splice(0).map( + (server) => + new Promise((resolve) => { + server.close(() => resolve()); + }) + ) + ); +}); + +async function createServer( + handler: (req: http.IncomingMessage, res: http.ServerResponse) => void +): Promise { + const server = http.createServer(handler); + activeServers.push(server); + + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', () => resolve()); + }); + + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Unable to resolve server port'); + } + + return address.port; +} + +describe('copilot daemon health detection', () => { + it('returns false when no daemon is running on port', async () => { + const running = await isDaemonRunning(19998); + expect(running).toBe(false); + }); + + it('returns true when daemon root endpoint confirms server is running', async () => { + const port = await createServer((_req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('Server running'); + }); + + const running = await isDaemonRunning(port); + expect(running).toBe(true); + }); + + it('returns false when root endpoint returns 200 but unexpected body', async () => { + const port = await createServer((_req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('ok'); + }); + + const running = await isDaemonRunning(port); + expect(running).toBe(false); + }); + + it('returns false when root endpoint is non-200', async () => { + const port = await createServer((_req, res) => { + res.writeHead(503, { 'Content-Type': 'text/plain' }); + res.end('unavailable'); + }); + + const running = await isDaemonRunning(port); + expect(running).toBe(false); + }); +}); From f4678d639772699d9f3504dcd05bc9e311f67527 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 4 Mar 2026 16:37:23 +0700 Subject: [PATCH 2/5] fix(copilot): sync alias UX and harden daemon stop safety --- src/ccs.ts | 24 +----- src/commands/copilot-command.ts | 19 ++--- src/copilot/constants.ts | 43 +++++++++++ src/copilot/copilot-daemon.ts | 43 +++++++++-- src/copilot/copilot-executor.ts | 2 +- src/copilot/daemon-process-ownership.ts | 76 +++++++++++++++++++ .../copilot/copilot-command-aliases.test.ts | 35 +++++++++ tests/unit/copilot/copilot-daemon.test.ts | 75 +++++++++++++++++- 8 files changed, 272 insertions(+), 45 deletions(-) create mode 100644 src/copilot/constants.ts create mode 100644 src/copilot/daemon-process-ownership.ts create mode 100644 tests/unit/copilot/copilot-command-aliases.test.ts diff --git a/src/ccs.ts b/src/ccs.ts index 8790e839..99c57ca3 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -30,6 +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 centralized error handling import { handleError, runCleanup } from './errors'; @@ -573,28 +574,7 @@ async function main(): Promise { // Special case: copilot command (GitHub Copilot integration) // Only route to command handler for known subcommands, otherwise treat as profile - const COPILOT_SUBCOMMANDS = [ - 'auth', - 'status', - 'models', - 'usage', - 'start', - 'stop', - 'enable', - 'disable', - '--auth', - '--status', - '--models', - '--usage', - '--start', - '--stop', - '--enable', - '--disable', - 'help', - '--help', - '-h', - ]; - if (firstArg === 'copilot' && args.length > 1 && COPILOT_SUBCOMMANDS.includes(args[1])) { + 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)); diff --git a/src/commands/copilot-command.ts b/src/commands/copilot-command.ts index 7184a120..960a7328 100644 --- a/src/commands/copilot-command.ts +++ b/src/commands/copilot-command.ts @@ -16,23 +16,13 @@ import { import { loadOrCreateUnifiedConfig, saveUnifiedConfig } from '../config/unified-config-loader'; import { DEFAULT_COPILOT_CONFIG } from '../config/unified-config-types'; import { ok, fail, info, color } from '../utils/ui'; +import { normalizeCopilotSubcommand } from '../copilot/constants'; /** * Handle copilot subcommand. */ export async function handleCopilotCommand(args: string[]): Promise { - const subcommandAliasMap: Record = { - '--auth': 'auth', - '--status': 'status', - '--models': 'models', - '--usage': 'usage', - '--start': 'start', - '--stop': 'stop', - '--enable': 'enable', - '--disable': 'disable', - }; - const rawSubcommand = args[0]; - const subcommand = (rawSubcommand && subcommandAliasMap[rawSubcommand]) || rawSubcommand; + const subcommand = normalizeCopilotSubcommand(args[0]); switch (subcommand) { case 'auth': @@ -88,6 +78,11 @@ function handleHelp(): number { console.log(' 3. ccs copilot start # Start daemon'); console.log(' 4. ccs copilot usage # Check quota usage'); console.log(''); + console.log('Flag aliases:'); + console.log( + ' ccs copilot --auth | --status | --models | --usage | --start | --stop | --enable | --disable' + ); + console.log(''); console.log('Or use the web UI: ccs config → Copilot tab'); console.log(''); return 0; diff --git a/src/copilot/constants.ts b/src/copilot/constants.ts new file mode 100644 index 00000000..c4c0b42f --- /dev/null +++ b/src/copilot/constants.ts @@ -0,0 +1,43 @@ +/** + * Shared Copilot command tokens and aliases. + * Keep all copilot subcommand routing in one place to avoid drift. + */ + +export const COPILOT_SUBCOMMANDS = [ + 'auth', + 'status', + 'models', + 'usage', + 'start', + 'stop', + 'enable', + 'disable', +] as const; + +export type CopilotSubcommand = (typeof COPILOT_SUBCOMMANDS)[number]; + +export const COPILOT_FLAG_ALIASES: Readonly> = + Object.freeze({ + '--auth': 'auth', + '--status': 'status', + '--models': 'models', + '--usage': 'usage', + '--start': 'start', + '--stop': 'stop', + '--enable': 'enable', + '--disable': 'disable', + }); + +export const COPILOT_SUBCOMMAND_TOKENS = Object.freeze([ + ...COPILOT_SUBCOMMANDS, + ...Object.keys(COPILOT_FLAG_ALIASES), + 'help', + '--help', + '-h', +]); + +export function normalizeCopilotSubcommand(token?: string): string | undefined { + if (!token) return token; + const alias = COPILOT_FLAG_ALIASES[token as keyof typeof COPILOT_FLAG_ALIASES]; + return alias || token; +} diff --git a/src/copilot/copilot-daemon.ts b/src/copilot/copilot-daemon.ts index dc6cbbef..2dae17be 100644 --- a/src/copilot/copilot-daemon.ts +++ b/src/copilot/copilot-daemon.ts @@ -12,8 +12,13 @@ import * as http from 'http'; import { CopilotDaemonStatus } from './types'; import { CopilotConfig } from '../config/unified-config-types'; import { getCopilotDir, getCopilotApiBinPath } from './copilot-package-manager'; +import { verifyCopilotDaemonOwnership } from './daemon-process-ownership'; -const PID_FILE = path.join(getCopilotDir(), 'daemon.pid'); +const DAEMON_HEALTH_MARKER = 'server running'; + +function getPidFilePath(): string { + return path.join(getCopilotDir(), 'daemon.pid'); +} /** * Check if copilot-api daemon is running on the specified port. @@ -43,7 +48,7 @@ export async function isDaemonRunning(port: number): Promise { return; } - resolve(body.trim().toLowerCase().includes('server running')); + resolve(body.trim().toLowerCase().includes(DAEMON_HEALTH_MARKER)); }); } ); @@ -79,9 +84,10 @@ export async function getDaemonStatus(port: number): Promise` + const lower = commandLine.toLowerCase(); + const looksLikeCopilotDaemon = lower.includes('copilot-api') && lower.includes(' start'); + + return looksLikeCopilotDaemon ? 'owned' : 'not-owned'; +} diff --git a/tests/unit/copilot/copilot-command-aliases.test.ts b/tests/unit/copilot/copilot-command-aliases.test.ts new file mode 100644 index 00000000..4cc4b7ae --- /dev/null +++ b/tests/unit/copilot/copilot-command-aliases.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'bun:test'; +import { + COPILOT_SUBCOMMANDS, + COPILOT_SUBCOMMAND_TOKENS, + normalizeCopilotSubcommand, +} from '../../../src/copilot/constants'; + +describe('copilot command aliases', () => { + it('normalizes all supported flag aliases', () => { + for (const subcommand of COPILOT_SUBCOMMANDS) { + expect(normalizeCopilotSubcommand(`--${subcommand}`)).toBe(subcommand); + } + }); + + it('keeps canonical subcommands unchanged', () => { + for (const subcommand of COPILOT_SUBCOMMANDS) { + expect(normalizeCopilotSubcommand(subcommand)).toBe(subcommand); + } + }); + + it('returns unknown tokens unchanged', () => { + expect(normalizeCopilotSubcommand('--unknown')).toBe('--unknown'); + expect(normalizeCopilotSubcommand('unknown')).toBe('unknown'); + }); + + it('exposes complete routing token list for ccs entrypoint', () => { + for (const subcommand of COPILOT_SUBCOMMANDS) { + expect(COPILOT_SUBCOMMAND_TOKENS).toContain(subcommand); + expect(COPILOT_SUBCOMMAND_TOKENS).toContain(`--${subcommand}`); + } + expect(COPILOT_SUBCOMMAND_TOKENS).toContain('help'); + expect(COPILOT_SUBCOMMAND_TOKENS).toContain('--help'); + expect(COPILOT_SUBCOMMAND_TOKENS).toContain('-h'); + }); +}); diff --git a/tests/unit/copilot/copilot-daemon.test.ts b/tests/unit/copilot/copilot-daemon.test.ts index 9de0c02e..ad8e6b6f 100644 --- a/tests/unit/copilot/copilot-daemon.test.ts +++ b/tests/unit/copilot/copilot-daemon.test.ts @@ -1,8 +1,21 @@ -import { afterEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; import * as http from 'http'; -import { isDaemonRunning } from '../../../src/copilot/copilot-daemon'; +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 { getCcsDir } from '../../../src/utils/config-manager'; const activeServers: http.Server[] = []; +let originalCcsHome: string | undefined; +let tempDir: string; + +beforeEach(() => { + originalCcsHome = process.env.CCS_HOME; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-copilot-daemon-test-')); + process.env.CCS_HOME = tempDir; +}); afterEach(async () => { await Promise.all( @@ -13,6 +26,18 @@ afterEach(async () => { }) ) ); + + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } }); async function createServer( @@ -59,6 +84,16 @@ describe('copilot daemon health detection', () => { expect(running).toBe(false); }); + it('returns false when root endpoint returns 200 with empty body', async () => { + const port = await createServer((_req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end(''); + }); + + const running = await isDaemonRunning(port); + expect(running).toBe(false); + }); + it('returns false when root endpoint is non-200', async () => { const port = await createServer((_req, res) => { res.writeHead(503, { 'Content-Type': 'text/plain' }); @@ -69,3 +104,39 @@ describe('copilot daemon health detection', () => { expect(running).toBe(false); }); }); + +describe('copilot daemon stop safety', () => { + it('does not terminate unrelated process from stale PID file', async () => { + 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(); + if (!result.success) { + expect(result.error).toContain('unable to verify daemon ownership'); + } + + // Unrelated process should still be alive. + expect(() => process.kill(unrelatedPid, 0)).not.toThrow(); + } finally { + try { + process.kill(unrelatedPid, 'SIGTERM'); + } catch { + // Process already exited. + } + } + }); +}); From 5ad2416a863e8190351a1ee42e00646093e3e70f Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 4 Mar 2026 16:42:59 +0700 Subject: [PATCH 3/5] refactor(daemon): share process ownership guard for safe shutdown --- src/copilot/copilot-daemon.ts | 8 ++- src/copilot/daemon-process-ownership.ts | 76 ------------------------- src/cursor/daemon-process-ownership.ts | 16 ++++-- 3 files changed, 18 insertions(+), 82 deletions(-) delete mode 100644 src/copilot/daemon-process-ownership.ts diff --git a/src/copilot/copilot-daemon.ts b/src/copilot/copilot-daemon.ts index 2dae17be..29116722 100644 --- a/src/copilot/copilot-daemon.ts +++ b/src/copilot/copilot-daemon.ts @@ -12,7 +12,7 @@ import * as http from 'http'; import { CopilotDaemonStatus } from './types'; import { CopilotConfig } from '../config/unified-config-types'; import { getCopilotDir, getCopilotApiBinPath } from './copilot-package-manager'; -import { verifyCopilotDaemonOwnership } from './daemon-process-ownership'; +import { verifyProcessOwnership } from '../cursor/daemon-process-ownership'; const DAEMON_HEALTH_MARKER = 'server running'; @@ -258,7 +258,11 @@ export async function stopDaemon(): Promise<{ success: boolean; error?: string } } try { - const ownership = verifyCopilotDaemonOwnership(pid); + const ownership = verifyProcessOwnership(pid, (commandLine) => { + const lower = commandLine.toLowerCase(); + // copilot-api is launched as `... copilot-api start --port ` + return lower.includes('copilot-api') && lower.includes(' start'); + }); if (ownership === 'not-running') { removePidFile(); return { success: true }; diff --git a/src/copilot/daemon-process-ownership.ts b/src/copilot/daemon-process-ownership.ts deleted file mode 100644 index 0d80f8d8..00000000 --- a/src/copilot/daemon-process-ownership.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { spawnSync } from 'child_process'; -import * as fs from 'fs'; - -export type DaemonOwnershipStatus = 'owned' | 'not-owned' | 'not-running' | 'unknown'; - -function getProcessCommandLine(pid: number): string | null { - if (process.platform === 'linux') { - try { - // /proc cmdline uses null separators between arguments. - return fs.readFileSync(`/proc/${pid}/cmdline`, 'utf8').replace(/\0/g, ' ').trim(); - } catch { - return null; - } - } - - if (process.platform === 'darwin') { - try { - const result = spawnSync('ps', ['-p', String(pid), '-o', 'command='], { - encoding: 'utf8', - }); - if (result.error || result.status !== 0) { - return null; - } - return result.stdout.trim(); - } catch { - return null; - } - } - - if (process.platform === 'win32') { - const command = `(Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}" | Select-Object -ExpandProperty CommandLine)`; - const shells = ['powershell.exe', 'powershell', 'pwsh.exe', 'pwsh']; - for (const shell of shells) { - try { - const result = spawnSync(shell, ['-NoProfile', '-Command', command], { - encoding: 'utf8', - }); - if (result.error) { - continue; - } - if (result.status !== 0) { - return null; - } - return result.stdout.trim(); - } catch { - // Try next shell candidate - } - } - return null; - } - - return null; -} - -export function verifyCopilotDaemonOwnership(pid: number): DaemonOwnershipStatus { - try { - process.kill(pid, 0); - } catch (err) { - const error = err as NodeJS.ErrnoException; - if (error.code === 'ESRCH') { - return 'not-running'; - } - return 'unknown'; - } - - const commandLine = getProcessCommandLine(pid); - if (!commandLine) { - return 'unknown'; - } - - // copilot-api is launched as `... copilot-api start --port ` - const lower = commandLine.toLowerCase(); - const looksLikeCopilotDaemon = lower.includes('copilot-api') && lower.includes(' start'); - - return looksLikeCopilotDaemon ? 'owned' : 'not-owned'; -} diff --git a/src/cursor/daemon-process-ownership.ts b/src/cursor/daemon-process-ownership.ts index c9c0eddf..81ac9384 100644 --- a/src/cursor/daemon-process-ownership.ts +++ b/src/cursor/daemon-process-ownership.ts @@ -52,7 +52,10 @@ function getProcessCommandLine(pid: number): string | null { return null; } -export function verifyDaemonOwnership(pid: number): DaemonOwnershipStatus { +export function verifyProcessOwnership( + pid: number, + ownershipMatcher: (commandLine: string) => boolean +): DaemonOwnershipStatus { try { process.kill(pid, 0); } catch (err) { @@ -68,8 +71,13 @@ export function verifyDaemonOwnership(pid: number): DaemonOwnershipStatus { return 'unknown'; } - const looksLikeCursorDaemon = - commandLine.includes('--ccs-daemon') && commandLine.includes('cursor-daemon-entry'); + return ownershipMatcher(commandLine) ? 'owned' : 'not-owned'; +} - return looksLikeCursorDaemon ? 'owned' : 'not-owned'; +export function verifyDaemonOwnership(pid: number): DaemonOwnershipStatus { + return verifyProcessOwnership( + pid, + (commandLine) => + commandLine.includes('--ccs-daemon') && commandLine.includes('cursor-daemon-entry') + ); } From 1fd128e50feb3fe779ac30e8706c722024e77543 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 4 Mar 2026 17:05:35 +0700 Subject: [PATCH 4/5] fix(copilot): harden daemon lifecycle and validate config updates --- src/ccs.ts | 24 ++-- src/commands/copilot-command.ts | 3 +- src/copilot/constants.ts | 24 ++++ src/copilot/copilot-daemon.ts | 49 ++++++- src/copilot/copilot-executor.ts | 9 +- src/web-server/routes/copilot-routes.ts | 136 ++++++++++++++++-- .../copilot/copilot-command-aliases.test.ts | 8 ++ tests/unit/copilot/copilot-daemon.test.ts | 69 ++++++++- 8 files changed, 293 insertions(+), 29 deletions(-) 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(); From 930d66fc0d31468ecc53fc50bd6db81aaed1fb7b Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 4 Mar 2026 17:18:44 +0700 Subject: [PATCH 5/5] fix(copilot): refine ownership checks and command error handling --- src/ccs.ts | 9 +++------ src/commands/copilot-command.ts | 3 ++- src/copilot/constants.ts | 20 ------------------- src/copilot/copilot-daemon.ts | 7 +++---- src/web-server/routes/copilot-routes.ts | 19 ++++++++++++++++++ .../copilot/copilot-command-aliases.test.ts | 8 -------- 6 files changed, 27 insertions(+), 39 deletions(-) diff --git a/src/ccs.ts b/src/ccs.ts index e858ab1d..25c27c11 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 { isCopilotSubcommandToken, isLikelyCopilotFlagAlias } from './copilot/constants'; +import { isCopilotSubcommandToken } from './copilot/constants'; // Import centralized error handling import { handleError, runCleanup } from './errors'; @@ -573,13 +573,10 @@ async function main(): Promise { } // Special case: copilot command (GitHub Copilot integration) - // Route known subcommands and likely mistyped aliases (`--statu`) to command handler. - // Non-command Claude args still pass through profile flow. + // Route known subcommands to command handler, keep all other args as profile passthrough. if (firstArg === 'copilot' && args.length > 1) { const copilotToken = args[1]; - const shouldRouteToCopilotCommand = - isCopilotSubcommandToken(copilotToken) || - (args.length === 2 && isLikelyCopilotFlagAlias(copilotToken)); + const shouldRouteToCopilotCommand = isCopilotSubcommandToken(copilotToken); if (shouldRouteToCopilotCommand) { const { handleCopilotCommand } = await import('./commands/copilot-command'); diff --git a/src/commands/copilot-command.ts b/src/commands/copilot-command.ts index 2bdb2b1d..7b6620ee 100644 --- a/src/commands/copilot-command.ts +++ b/src/commands/copilot-command.ts @@ -49,7 +49,8 @@ export async function handleCopilotCommand(args: string[]): Promise { default: console.error(fail(`Unknown subcommand: ${subcommand}`)); console.error(''); - return handleHelp(); + handleHelp(); + return 1; } } diff --git a/src/copilot/constants.ts b/src/copilot/constants.ts index cc594afd..c5bb8b07 100644 --- a/src/copilot/constants.ts +++ b/src/copilot/constants.ts @@ -45,23 +45,3 @@ export function normalizeCopilotSubcommand(token?: string): string | undefined { 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 c8cc09fe..4ec2d75b 100644 --- a/src/copilot/copilot-daemon.ts +++ b/src/copilot/copilot-daemon.ts @@ -289,12 +289,11 @@ 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 hasCopilotApiBinary = /copilot-api(\.cmd|\.exe)?/.test(lower); const hasStartCommand = /\bstart\b/.test(lower); - const hasExpectedPort = - lower.includes(`--port ${configuredPort}`) || lower.includes(`--port=${configuredPort}`); + const hasPortArgument = /--port(?:\s+|=)\d+\b/.test(lower); // copilot-api is launched as `... copilot-api start --port ` - return hasCopilotApiBinary && hasStartCommand && hasExpectedPort; + return hasCopilotApiBinary && hasStartCommand && hasPortArgument; }); if (ownership === 'not-running') { removePidFile(); diff --git a/src/web-server/routes/copilot-routes.ts b/src/web-server/routes/copilot-routes.ts index 31cf897c..625c160c 100644 --- a/src/web-server/routes/copilot-routes.ts +++ b/src/web-server/routes/copilot-routes.ts @@ -92,6 +92,25 @@ router.put('/config', (req: Request, res: Response): void => { return; } const payload = updates as Record; + const allowedKeys = new Set([ + 'enabled', + 'auto_start', + 'port', + 'account_type', + 'rate_limit', + 'wait_on_limit', + 'model', + 'opus_model', + 'sonnet_model', + 'haiku_model', + ]); + const unknownKeys = Object.keys(payload).filter((key) => !allowedKeys.has(key)); + if (unknownKeys.length > 0) { + res.status(400).json({ + error: `Unknown copilot config field(s): ${unknownKeys.join(', ')}`, + }); + return; + } if ('port' in payload) { if (typeof payload.port !== 'number' || !Number.isInteger(payload.port)) { diff --git a/tests/unit/copilot/copilot-command-aliases.test.ts b/tests/unit/copilot/copilot-command-aliases.test.ts index cb8c19ca..4cc4b7ae 100644 --- a/tests/unit/copilot/copilot-command-aliases.test.ts +++ b/tests/unit/copilot/copilot-command-aliases.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from 'bun:test'; import { COPILOT_SUBCOMMANDS, COPILOT_SUBCOMMAND_TOKENS, - isLikelyCopilotFlagAlias, normalizeCopilotSubcommand, } from '../../../src/copilot/constants'; @@ -24,13 +23,6 @@ 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);