Merge pull request #560 from kaitranntt/fix/review-cursor-variant-safety

fix(cliproxy): address lifecycle port and thinking regressions
This commit is contained in:
Kai (Tam Nhu) Tran authored and GitHub committed 2026-02-14 10:39:05 +07:00
commit 9c24dd1f38
11 files changed
+391 -111

No files matched your search

+19 -16
View File
@@ -180,16 +180,17 @@ export function applyThinkingConfig(
// Get base model to check thinking support
const baseModel = result.ANTHROPIC_MODEL || '';
const normalizedBaseModel = normalizeModelForThinkingLookup(baseModel, provider);
if (!supportsThinking(provider, normalizedBaseModel)) {
// U2: Warn user if they explicitly provided --thinking but model doesn't support it
if (thinkingOverride !== undefined && shouldShowWarnings(thinkingConfig)) {
console.warn(
warn(
`Model ${baseModel || 'unknown'} (provider: ${provider}) does not support thinking budget. --thinking flag ignored.`
)
);
}
return result;
const baseModelSupportsThinking = supportsThinking(provider, normalizedBaseModel);
if (
!baseModelSupportsThinking &&
thinkingOverride !== undefined &&
shouldShowWarnings(thinkingConfig)
) {
console.warn(
warn(
`Model ${baseModel || 'unknown'} (provider: ${provider}) does not support thinking budget. --thinking flag ignored for default model.`
)
);
}
// Determine thinking value to use
@@ -215,12 +216,14 @@ export function applyThinkingConfig(
return result; // No thinking to apply
}
// Validate thinking value against model capabilities
const validation = validateThinking(provider, normalizedBaseModel, thinkingValue);
if (validation.warning && shouldShowWarnings(thinkingConfig)) {
console.warn(warn(validation.warning));
if (baseModelSupportsThinking) {
// Validate thinking value against default model capabilities.
const validation = validateThinking(provider, normalizedBaseModel, thinkingValue);
if (validation.warning && shouldShowWarnings(thinkingConfig)) {
console.warn(warn(validation.warning));
}
thinkingValue = validation.value;
}
thinkingValue = validation.value;
// If auto-detection resolves default tier to "off", skip the main model but still allow
// explicit per-tier thinking values for other tiers.
@@ -232,7 +235,7 @@ export function applyThinkingConfig(
return result; // No thinking to apply anywhere
}
// Otherwise, continue to process tiers with their own config (skip main model)
} else if (result.ANTHROPIC_MODEL) {
} else if (baseModelSupportsThinking && result.ANTHROPIC_MODEL) {
// Apply thinking suffix to main model (only if not off)
result.ANTHROPIC_MODEL = applyThinkingSuffixForProvider(
result.ANTHROPIC_MODEL,
@@ -41,15 +41,15 @@ export interface StartProxyResult {
/**
* Get current proxy status
*/
export function getProxyStatus(): ProxyStatusResult {
return getProxyStatusSession();
export function getProxyStatus(port?: number): ProxyStatusResult {
return getProxyStatusSession(port);
}
/**
* Stop the running CLIProxy instance
*/
export async function stopProxy(): Promise<StopProxyResult> {
return stopProxySession();
export async function stopProxy(port?: number): Promise<StopProxyResult> {
return stopProxySession(port);
}
/**
+4 -3
View File
@@ -17,6 +17,7 @@ import { isUnifiedMode } from '../../config/unified-config-loader';
import { deleteConfigForPort } from '../config-generator';
import { hasActiveSessions, deleteSessionLockForPort } from '../session-tracker';
import { warn } from '../../utils/ui';
import { getCcsDir } from '../../utils/config-manager';
import { validateCompositeTiers } from '../composite-validator';
import {
createSettingsFile,
@@ -24,7 +25,6 @@ import {
createCompositeSettingsFile,
deleteSettingsFile,
getRelativeSettingsPath,
getCompositeRelativeSettingsPath,
updateSettingsModel,
updateSettingsProviderAndModel,
} from './variant-settings';
@@ -359,7 +359,7 @@ export function createCompositeVariant(
type: 'composite',
default_tier: defaultTier,
tiers,
settings: getCompositeRelativeSettingsPath(name),
settings: settingsPath,
port,
};
saveCompositeVariantUnified(name, compositeConfig);
@@ -436,7 +436,8 @@ export function updateCompositeVariant(
}
// Preserve existing settings path when configured; otherwise use default path.
const settingsRef = existing.settings || getCompositeRelativeSettingsPath(name);
const settingsRef =
existing.settings || path.join(getCcsDir(), `composite-${name}.settings.json`);
// Create new settings file with updated config
const settingsPath = createCompositeSettingsFile(
@@ -11,14 +11,25 @@
import { initUI, header, color, dim, ok, warn, info } from '../../utils/ui';
import { getProxyStatus, startProxy, stopProxy } from '../../cliproxy/services';
import { detectRunningProxy } from '../../cliproxy/proxy-detector';
import { CLIPROXY_DEFAULT_PORT } from '../../cliproxy/config-generator';
import { CLIPROXY_DEFAULT_PORT, validatePort } from '../../cliproxy/config/port-manager';
import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader';
/**
* Resolve the local CLIProxy lifecycle port from unified config.
* Falls back to default port when unset/invalid.
*/
export function resolveLifecyclePort(): number {
const config = loadOrCreateUnifiedConfig();
return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT);
}
export async function handleStart(verbose = false): Promise<void> {
await initUI();
console.log(header('Start CLIProxy'));
console.log('');
const result = await startProxy(CLIPROXY_DEFAULT_PORT, verbose);
const port = resolveLifecyclePort();
const result = await startProxy(port, verbose);
if (result.started) {
if (result.alreadyRunning) {
console.log(info(`CLIProxy already running on port ${result.port}`));
@@ -40,7 +51,8 @@ export async function handleRestart(verbose = false): Promise<void> {
console.log(header('Restart CLIProxy'));
console.log('');
const stopResult = await stopProxy();
const port = resolveLifecyclePort();
const stopResult = await stopProxy(port);
if (stopResult.stopped) {
console.log(ok(`CLIProxy stopped (PID ${stopResult.pid})`));
} else if (stopResult.error === 'No active CLIProxy session found') {
@@ -50,7 +62,7 @@ export async function handleRestart(verbose = false): Promise<void> {
console.log(info('Attempting to start a fresh instance...'));
}
const startResult = await startProxy(CLIPROXY_DEFAULT_PORT, verbose);
const startResult = await startProxy(port, verbose);
if (startResult.started) {
if (startResult.alreadyRunning) {
console.log(info(`CLIProxy already running on port ${startResult.port}`));
@@ -69,7 +81,8 @@ export async function handleProxyStatus(): Promise<void> {
console.log(header('CLIProxy Status'));
console.log('');
const status = getProxyStatus();
const port = resolveLifecyclePort();
const status = getProxyStatus(port);
if (status.running) {
console.log(` Status: ${color('Running', 'success')}`);
console.log(` PID: ${status.pid}`);
@@ -82,11 +95,11 @@ export async function handleProxyStatus(): Promise<void> {
console.log(dim('To stop: ccs cliproxy stop'));
} else {
// Fallback: detect untracked/orphaned proxy process (e.g. detached session without lock file).
const detected = await detectRunningProxy();
const detected = await detectRunningProxy(port);
if (detected.running && detected.verified) {
console.log(` Status: ${color('Running', 'success')}`);
console.log(` PID: ${detected.pid ?? 'unknown'}`);
console.log(` Port: ${CLIPROXY_DEFAULT_PORT}`);
console.log(` Port: ${port}`);
console.log(` Sessions: ${detected.sessionCount || 0} active`);
if (!detected.sessionCount) {
console.log(dim(' Note: Detected running proxy without local session lock'));
@@ -107,7 +120,8 @@ export async function handleStop(): Promise<void> {
console.log(header('Stop CLIProxy'));
console.log('');
const result = await stopProxy();
const port = resolveLifecyclePort();
const result = await stopProxy(port);
if (result.stopped) {
console.log(ok(`CLIProxy stopped (PID ${result.pid})`));
if (result.sessionCount && result.sessionCount > 0) {
+49
View File
@@ -0,0 +1,49 @@
import * as fs from 'fs';
import * as path from 'path';
import { getCcsDir } from '../utils/config-manager';
function getCursorDir(): string {
return path.join(getCcsDir(), 'cursor');
}
function getPidFilePath(): string {
return path.join(getCursorDir(), 'daemon.pid');
}
export function getPidFromFile(): number | null {
const pidFile = getPidFilePath();
try {
if (fs.existsSync(pidFile)) {
const content = fs.readFileSync(pidFile, 'utf8').trim();
const pid = parseInt(content, 10);
return isNaN(pid) ? null : pid;
}
} catch {
// Ignore errors
}
return null;
}
export function writePidToFile(pid: number): void {
const pidFile = getPidFilePath();
try {
const dir = path.dirname(pidFile);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
fs.writeFileSync(pidFile, pid.toString(), { mode: 0o600 });
} catch {
// Ignore errors
}
}
export function removePidFile(): void {
const pidFile = getPidFilePath();
try {
if (fs.existsSync(pidFile)) {
fs.unlinkSync(pidFile);
}
} catch {
// Ignore errors
}
}
+40 -75
View File
@@ -10,22 +10,9 @@ import * as fs from 'fs';
import * as path from 'path';
import * as http from 'http';
import type { CursorDaemonConfig, CursorDaemonStatus } from './types';
import { getCcsDir } from '../utils/config-manager';
/**
* Get Cursor directory path.
*/
function getCursorDir(): string {
return path.join(getCcsDir(), 'cursor');
}
/**
* Get PID file path.
* Computed at runtime to respect CCS_HOME changes (e.g., in tests).
*/
function getPidFilePath(): string {
return path.join(getCursorDir(), 'daemon.pid');
}
import { getPidFromFile, writePidToFile, removePidFile } from './cursor-daemon-pid';
import { verifyDaemonOwnership } from './daemon-process-ownership';
export { getPidFromFile, writePidToFile, removePidFile } from './cursor-daemon-pid';
/**
* Resolve daemon entrypoint candidates for current runtime.
@@ -77,8 +64,26 @@ export async function isDaemonRunning(port: number): Promise<boolean> {
timeout: 3000,
},
(res) => {
res.resume(); // Drain response body
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;
}
try {
const payload = JSON.parse(body) as { service?: string };
resolve(payload.service === 'cursor-daemon');
} catch {
resolve(false);
}
});
}
);
@@ -109,53 +114,6 @@ export async function getDaemonStatus(port: number): Promise<CursorDaemonStatus>
};
}
/**
* Read PID from file.
*/
export function getPidFromFile(): number | null {
const pidFile = getPidFilePath();
try {
if (fs.existsSync(pidFile)) {
const content = fs.readFileSync(pidFile, 'utf8').trim();
const pid = parseInt(content, 10);
return isNaN(pid) ? null : pid;
}
} catch {
// Ignore errors
}
return null;
}
/**
* Write PID to file.
*/
export function writePidToFile(pid: number): void {
const pidFile = getPidFilePath();
try {
const dir = path.dirname(pidFile);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
fs.writeFileSync(pidFile, pid.toString(), { mode: 0o600 });
} catch {
// Ignore errors
}
}
/**
* Remove PID file.
*/
export function removePidFile(): void {
const pidFile = getPidFilePath();
try {
if (fs.existsSync(pidFile)) {
fs.unlinkSync(pidFile);
}
} catch {
// Ignore errors
}
}
/**
* Start the cursor daemon.
*
@@ -292,16 +250,23 @@ export async function stopDaemon(): Promise<{ success: boolean; error?: string }
}
try {
// Verify the PID belongs to our daemon before signaling
try {
const cmdline = fs.readFileSync(`/proc/${pid}/cmdline`, 'utf8');
if (!cmdline.includes('--ccs-daemon')) {
// PID was reused by an unrelated process
removePidFile();
return { success: true };
}
} catch {
// /proc not available (macOS/Windows) or process gone — proceed with kill
const ownership = verifyDaemonOwnership(pid);
if (ownership === 'not-running') {
removePidFile();
return { success: true };
}
if (ownership === 'not-owned') {
// PID was reused by an unrelated process.
removePidFile();
return { success: true };
}
if (ownership === 'unknown') {
return {
success: false,
error: `Refusing to stop PID ${pid}: unable to verify daemon ownership`,
};
}
// Send SIGTERM to the process
+75
View File
@@ -0,0 +1,75 @@
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 verifyDaemonOwnership(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';
}
const looksLikeCursorDaemon =
commandLine.includes('--ccs-daemon') && commandLine.includes('cursor-daemon-entry');
return looksLikeCursorDaemon ? 'owned' : 'not-owned';
}
+29 -5
View File
@@ -334,6 +334,32 @@ describe('applyThinkingConfig - composite variant integration', () => {
expect(result.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe('claude-sonnet-4-5-thinking(high)');
});
it('applies tier thinking even when default model does not support thinking', () => {
const envVars: NodeJS.ProcessEnv = {
ANTHROPIC_MODEL: 'gpt-4o-mini',
ANTHROPIC_DEFAULT_OPUS_MODEL: 'gpt-4o-mini',
ANTHROPIC_DEFAULT_SONNET_MODEL: 'claude-sonnet-4-5-thinking',
ANTHROPIC_DEFAULT_HAIKU_MODEL: 'claude-haiku-4-5-20251001',
};
const result = applyThinkingConfig(
envVars,
'codex' as CLIProxyProvider,
undefined,
{
sonnet: 'high',
},
{
sonnet: { provider: 'agy' as CLIProxyProvider },
}
);
// Default model provider/model do not support thinking.
expect(result.ANTHROPIC_MODEL).toBe('gpt-4o-mini');
// Supported mixed-provider tier must still receive its configured thinking value.
expect(result.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe('claude-sonnet-4-5-thinking(high)');
});
it('should handle numeric budgets in per-tier thinking', () => {
const envVars: NodeJS.ProcessEnv = {
ANTHROPIC_MODEL: 'claude-sonnet-4-5-thinking',
@@ -468,13 +494,11 @@ describe('applyThinkingConfig - composite variant integration', () => {
compositeTierThinking
);
// Base model check uses ANTHROPIC_MODEL with suffix — supportsThinking
// may not recognize suffixed model names, causing early return.
// All models preserved as-is.
// Existing suffix on main model should be preserved.
expect(result.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe('claude-sonnet-4-5-thinking(high)');
expect(result.ANTHROPIC_MODEL).toBe('claude-sonnet-4-5-thinking(high)');
// Opus stays unchanged because function returned early
expect(result.ANTHROPIC_DEFAULT_OPUS_MODEL).toBe('claude-opus-4-6-thinking');
// Other supported tiers still receive configured thinking.
expect(result.ANTHROPIC_DEFAULT_OPUS_MODEL).toBe('claude-opus-4-6-thinking(xhigh)');
});
it('should use codex effort suffix style when provider is codex', () => {
@@ -407,6 +407,26 @@ cliproxy:
expect(result.success).toBe(false);
expect(result.error).toContain("Invalid tier config for 'opus'");
});
it('stores composite settings path under active CCS_DIR', () => {
const result = createCompositeVariant({
name: 'scoped',
defaultTier: 'sonnet',
tiers: {
opus: { provider: 'gemini', model: 'gemini-2.5-pro' },
sonnet: { provider: 'agy', model: 'claude-sonnet-4-5-thinking' },
haiku: { provider: 'agy', model: 'claude-haiku-4-5-20251001' },
},
});
expect(result.success).toBe(true);
const variants = listVariantsFromConfig();
const expectedPath = path.join(tmpDir, 'composite-scoped.settings.json');
expect(variants.scoped.settings).toBe(expectedPath);
expect(result.settingsPath).toBe(expectedPath);
expect(fs.existsSync(expectedPath)).toBe(true);
});
});
describe('saveCompositeVariantUnified', () => {
@@ -0,0 +1,66 @@
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { resolveLifecyclePort } from '../../../src/commands/cliproxy/proxy-lifecycle-subcommand';
import { CLIPROXY_DEFAULT_PORT } from '../../../src/cliproxy/config/port-manager';
let tempDir: string;
let originalCcsDir: string | undefined;
function writeUnifiedConfig(localPort: number): void {
const configPath = path.join(tempDir, 'config.yaml');
const yaml = `version: 2
accounts: {}
profiles: {}
preferences:
theme: system
telemetry: false
auto_update: true
cliproxy:
oauth_accounts: {}
providers:
- gemini
- codex
- agy
variants: {}
cliproxy_server:
local:
port: ${localPort}
`;
fs.writeFileSync(configPath, yaml, 'utf8');
}
describe('resolveLifecyclePort', () => {
beforeEach(() => {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-proxy-lifecycle-'));
originalCcsDir = process.env.CCS_DIR;
process.env.CCS_DIR = tempDir;
});
afterEach(() => {
if (originalCcsDir !== undefined) {
process.env.CCS_DIR = originalCcsDir;
} else {
delete process.env.CCS_DIR;
}
if (tempDir && fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true, force: true });
}
});
it('uses configured cliproxy_server.local.port', () => {
writeUnifiedConfig(9456);
expect(resolveLifecyclePort()).toBe(9456);
});
it('falls back to default port when configured local port is invalid', () => {
writeUnifiedConfig(70000);
expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT);
});
it('falls back to default port when config file is missing', () => {
expect(resolveLifecyclePort()).toBe(CLIPROXY_DEFAULT_PORT);
});
});
+63
View File
@@ -6,6 +6,8 @@ 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 * as http from 'http';
import { spawn } from 'child_process';
import {
getPidFromFile,
writePidToFile,
@@ -180,6 +182,37 @@ describe('isDaemonRunning', () => {
const result = await isDaemonRunning(19999);
expect(result).toBe(false);
});
it('returns false when /health is 200 but service is not cursor-daemon', async () => {
const server = http.createServer((req, res) => {
if (req.url === '/health') {
res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ ok: true, service: 'not-cursor-daemon' }));
return;
}
res.writeHead(404);
res.end();
});
await new Promise<void>((resolve) => {
server.listen(0, '127.0.0.1', () => resolve());
});
try {
const address = server.address();
if (!address || typeof address === 'string') {
throw new Error('Unable to resolve test server port');
}
const result = await isDaemonRunning(address.port);
expect(result).toBe(false);
} finally {
await new Promise<void>((resolve) => {
server.close(() => resolve());
});
}
});
});
describe('getDaemonStatus', () => {
@@ -217,6 +250,36 @@ describe('stopDaemon', () => {
const pidFile = path.join(getTestCursorDir(), 'daemon.pid');
expect(fs.existsSync(pidFile)).toBe(false);
});
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');
}
writePidToFile(unrelatedPid);
try {
const result = await stopDaemon();
expect(result.success).toBe(true);
// Unrelated process should still be alive.
expect(() => process.kill(unrelatedPid, 0)).not.toThrow();
} finally {
try {
process.kill(unrelatedPid, 'SIGTERM');
} catch {
// Process already exited.
}
}
});
});
describe('handleCursorCommand', () => {