From 9f0ea25448a8e4ca3019052812452885a4058190 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Thu, 12 Feb 2026 04:03:01 +0700 Subject: [PATCH] fix(cursor): address PR #527 review feedback - Wire cursor command into main router (ccs.ts) following copilot pattern - Centralize default port (4242) and model (gpt-4.1) as constants - Remove duplicate types already in cursor-protobuf-schema from dev - Handle daemon exit code 0 before health check succeeds - Export PID helpers and model utils for testability - Add unit tests for cursor-daemon (PID file ops, isDaemonRunning) - Add unit tests for cursor-models (detectProvider, formatModelName, defaults) --- src/ccs.ts | 10 ++ src/commands/cursor-command.ts | 6 +- src/cursor/cursor-daemon.ts | 15 ++- src/cursor/cursor-models.ts | 14 ++- src/cursor/index.ts | 12 ++- tests/unit/cursor/cursor-daemon.test.ts | 123 ++++++++++++++++++++++++ tests/unit/cursor/cursor-models.test.ts | 82 ++++++++++++++++ 7 files changed, 250 insertions(+), 12 deletions(-) create mode 100644 tests/unit/cursor/cursor-daemon.test.ts create mode 100644 tests/unit/cursor/cursor-models.test.ts diff --git a/src/ccs.ts b/src/ccs.ts index 43d7eba2..720a37dd 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -532,6 +532,16 @@ async function main(): Promise { return; } + // Special case: cursor command (Cursor IDE integration) + // Only route to command handler for known subcommands, otherwise treat as profile + const CURSOR_SUBCOMMANDS = ['auth', 'status', 'models', 'start', 'stop', 'help', '--help', '-h']; + if (firstArg === 'cursor' && (args.length === 1 || CURSOR_SUBCOMMANDS.includes(args[1]))) { + // `ccs cursor ` - route to cursor command handler + const { handleCursorCommand } = await import('./commands/cursor-command'); + const exitCode = await handleCursorCommand(args.slice(1)); + process.exit(exitCode); + } + // Special case: copilot command (GitHub Copilot integration) // Only route to command handler for known subcommands, otherwise treat as profile const COPILOT_SUBCOMMANDS = [ diff --git a/src/commands/cursor-command.ts b/src/commands/cursor-command.ts index ecbb6602..aa4033f1 100644 --- a/src/commands/cursor-command.ts +++ b/src/commands/cursor-command.ts @@ -11,13 +11,15 @@ import { stopDaemon, getDaemonStatus, getAvailableModels, + DEFAULT_CURSOR_PORT, + DEFAULT_CURSOR_MODEL, } from '../cursor'; import { ok, fail, info, color } from '../utils/ui'; // Temporary default config until #521 adds cursor to unified config const DEFAULT_CURSOR_CONFIG = { - port: 4242, - model: 'gpt-4.1', + port: DEFAULT_CURSOR_PORT, + model: DEFAULT_CURSOR_MODEL, }; /** diff --git a/src/cursor/cursor-daemon.ts b/src/cursor/cursor-daemon.ts index 9fe6681c..1d96e74d 100644 --- a/src/cursor/cursor-daemon.ts +++ b/src/cursor/cursor-daemon.ts @@ -82,7 +82,7 @@ export async function getDaemonStatus(port: number): Promise /** * Read PID from file. */ -function getPidFromFile(): number | null { +export function getPidFromFile(): number | null { const pidFile = getPidFilePath(); try { if (fs.existsSync(pidFile)) { @@ -99,7 +99,7 @@ function getPidFromFile(): number | null { /** * Write PID to file. */ -function writePidToFile(pid: number): void { +export function writePidToFile(pid: number): void { const pidFile = getPidFilePath(); try { const dir = path.dirname(pidFile); @@ -115,7 +115,7 @@ function writePidToFile(pid: number): void { /** * Remove PID file. */ -function removePidFile(): void { +export function removePidFile(): void { const pidFile = getPidFilePath(); try { if (fs.existsSync(pidFile)) { @@ -208,8 +208,13 @@ export async function startDaemon( }); proc.on('exit', (code) => { - if (code !== 0 && code !== null) { - clearInterval(checkInterval); + clearInterval(checkInterval); + if (code === 0) { + resolve({ + success: false, + error: 'Daemon process exited unexpectedly with code 0', + }); + } else if (code !== null) { resolve({ success: false, error: `Daemon process exited with code ${code}`, diff --git a/src/cursor/cursor-models.ts b/src/cursor/cursor-models.ts index 4735e4c5..2a35baa7 100644 --- a/src/cursor/cursor-models.ts +++ b/src/cursor/cursor-models.ts @@ -8,6 +8,12 @@ import * as http from 'http'; import type { CursorModel } from './types'; +/** Default daemon port */ +export const DEFAULT_CURSOR_PORT = 4242; + +/** Default model ID */ +export const DEFAULT_CURSOR_MODEL = 'gpt-4.1'; + /** * Default models available through Cursor IDE. * Used as fallback when daemon is not reachable. @@ -96,7 +102,7 @@ export async function fetchModelsFromDaemon(port: number): Promise 0 ? models : DEFAULT_CURSOR_MODELS); } else { @@ -134,13 +140,13 @@ export async function getAvailableModels(port: number): Promise { * Uses gpt-4.1 as it's commonly available. */ export function getDefaultModel(): string { - return 'gpt-4.1'; + return DEFAULT_CURSOR_MODEL; } /** * Detect provider from model ID. */ -function detectProvider(modelId: string): string { +export function detectProvider(modelId: string): string { if (modelId.includes('claude')) return 'anthropic'; if (modelId.includes('gpt') || modelId.includes('o3')) return 'openai'; if (modelId.includes('gemini')) return 'google'; @@ -151,7 +157,7 @@ function detectProvider(modelId: string): string { /** * Format model ID to human-readable name. */ -function formatModelName(modelId: string): string { +export function formatModelName(modelId: string): string { // Find model in catalog for metadata const model = DEFAULT_CURSOR_MODELS.find((m) => m.id === modelId); if (model) { diff --git a/src/cursor/index.ts b/src/cursor/index.ts index 46832336..d589d799 100644 --- a/src/cursor/index.ts +++ b/src/cursor/index.ts @@ -11,11 +11,21 @@ export * from './types'; export { autoDetectTokens, saveCredentials, loadCredentials, checkAuthStatus } from './cursor-auth'; // Daemon -export { isDaemonRunning, getDaemonStatus, startDaemon, stopDaemon } from './cursor-daemon'; +export { + isDaemonRunning, + getDaemonStatus, + startDaemon, + stopDaemon, + getPidFromFile, + writePidToFile, + removePidFile, +} from './cursor-daemon'; // Models export { DEFAULT_CURSOR_MODELS, + DEFAULT_CURSOR_PORT, + DEFAULT_CURSOR_MODEL, fetchModelsFromDaemon, getAvailableModels, getDefaultModel, diff --git a/tests/unit/cursor/cursor-daemon.test.ts b/tests/unit/cursor/cursor-daemon.test.ts new file mode 100644 index 00000000..a4c374bb --- /dev/null +++ b/tests/unit/cursor/cursor-daemon.test.ts @@ -0,0 +1,123 @@ +/** + * Unit tests for Cursor daemon module + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { + getPidFromFile, + writePidToFile, + removePidFile, + isDaemonRunning, +} from '../../../src/cursor/cursor-daemon'; + +// Test isolation +let originalCcsHome: string | undefined; +let tempDir: string; + +beforeEach(() => { + originalCcsHome = process.env.CCS_HOME; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-daemon-test-')); + process.env.CCS_HOME = tempDir; +}); + +afterEach(() => { + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + + // Cleanup temp directory + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } +}); + +// CCS_HOME is set to tempDir; getCcsDir() appends '.ccs' to it +const ccsDir = () => path.join(tempDir, '.ccs'); + +describe('getPidFromFile', () => { + it('returns null when no PID file exists', () => { + expect(getPidFromFile()).toBeNull(); + }); + + it('returns PID when valid PID file exists', () => { + const cursorDir = path.join(ccsDir(), 'cursor'); + fs.mkdirSync(cursorDir, { recursive: true }); + fs.writeFileSync(path.join(cursorDir, 'daemon.pid'), '12345'); + + expect(getPidFromFile()).toBe(12345); + }); + + it('returns null when PID file contains invalid content', () => { + const cursorDir = path.join(ccsDir(), 'cursor'); + fs.mkdirSync(cursorDir, { recursive: true }); + fs.writeFileSync(path.join(cursorDir, 'daemon.pid'), 'not-a-number'); + + expect(getPidFromFile()).toBeNull(); + }); + + it('trims whitespace from PID file content', () => { + const cursorDir = path.join(ccsDir(), 'cursor'); + fs.mkdirSync(cursorDir, { recursive: true }); + fs.writeFileSync(path.join(cursorDir, 'daemon.pid'), ' 42 \n'); + + expect(getPidFromFile()).toBe(42); + }); +}); + +describe('writePidToFile', () => { + it('creates PID file with correct content', () => { + writePidToFile(12345); + + const pidFile = path.join(ccsDir(), 'cursor', 'daemon.pid'); + expect(fs.existsSync(pidFile)).toBe(true); + expect(fs.readFileSync(pidFile, 'utf8')).toBe('12345'); + }); + + it('creates cursor directory if it does not exist', () => { + const cursorDir = path.join(ccsDir(), 'cursor'); + expect(fs.existsSync(cursorDir)).toBe(false); + + writePidToFile(999); + + expect(fs.existsSync(cursorDir)).toBe(true); + }); + + it('overwrites existing PID file', () => { + writePidToFile(111); + writePidToFile(222); + + const pidFile = path.join(ccsDir(), 'cursor', 'daemon.pid'); + expect(fs.readFileSync(pidFile, 'utf8')).toBe('222'); + }); +}); + +describe('removePidFile', () => { + it('removes existing PID file', () => { + writePidToFile(12345); + const pidFile = path.join(ccsDir(), 'cursor', 'daemon.pid'); + expect(fs.existsSync(pidFile)).toBe(true); + + removePidFile(); + + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it('does not throw when PID file does not exist', () => { + expect(() => removePidFile()).not.toThrow(); + }); +}); + +describe('isDaemonRunning', () => { + it('returns false when no daemon is running on port', async () => { + // Use a port that should not have anything running + const result = await isDaemonRunning(19999); + expect(result).toBe(false); + }); +}); diff --git a/tests/unit/cursor/cursor-models.test.ts b/tests/unit/cursor/cursor-models.test.ts new file mode 100644 index 00000000..9cdd183b --- /dev/null +++ b/tests/unit/cursor/cursor-models.test.ts @@ -0,0 +1,82 @@ +/** + * Unit tests for Cursor models module + */ + +import { describe, it, expect } from 'bun:test'; +import { + DEFAULT_CURSOR_MODELS, + DEFAULT_CURSOR_PORT, + DEFAULT_CURSOR_MODEL, + getDefaultModel, + detectProvider, + formatModelName, +} from '../../../src/cursor/cursor-models'; + +describe('DEFAULT_CURSOR_MODELS', () => { + it('contains models from multiple providers', () => { + const providers = new Set(DEFAULT_CURSOR_MODELS.map((m) => m.provider)); + expect(providers.has('anthropic')).toBe(true); + expect(providers.has('openai')).toBe(true); + expect(providers.has('google')).toBe(true); + }); + + it('has exactly one default model', () => { + const defaults = DEFAULT_CURSOR_MODELS.filter((m) => m.isDefault); + expect(defaults).toHaveLength(1); + expect(defaults[0].id).toBe(DEFAULT_CURSOR_MODEL); + }); +}); + +describe('DEFAULT_CURSOR_PORT', () => { + it('is 4242', () => { + expect(DEFAULT_CURSOR_PORT).toBe(4242); + }); +}); + +describe('DEFAULT_CURSOR_MODEL', () => { + it('is gpt-4.1', () => { + expect(DEFAULT_CURSOR_MODEL).toBe('gpt-4.1'); + }); +}); + +describe('getDefaultModel', () => { + it('returns the default model constant', () => { + expect(getDefaultModel()).toBe(DEFAULT_CURSOR_MODEL); + }); +}); + +describe('detectProvider', () => { + it('detects anthropic models', () => { + expect(detectProvider('claude-sonnet-4')).toBe('anthropic'); + expect(detectProvider('claude-opus-4')).toBe('anthropic'); + }); + + it('detects openai models', () => { + expect(detectProvider('gpt-4.1')).toBe('openai'); + expect(detectProvider('gpt-5-mini')).toBe('openai'); + expect(detectProvider('o3-mini')).toBe('openai'); + }); + + it('detects google models', () => { + expect(detectProvider('gemini-2.5-pro')).toBe('google'); + }); + + it('detects cursor models', () => { + expect(detectProvider('cursor-small')).toBe('cursor'); + }); + + it('defaults to openai for unknown models', () => { + expect(detectProvider('unknown-model')).toBe('openai'); + }); +}); + +describe('formatModelName', () => { + it('returns catalog name for known models', () => { + expect(formatModelName('claude-sonnet-4')).toBe('Claude Sonnet 4'); + expect(formatModelName('gpt-4.1')).toBe('GPT-4.1'); + }); + + it('converts kebab-case to title case for unknown models', () => { + expect(formatModelName('my-custom-model')).toBe('My Custom Model'); + }); +});