diff --git a/src/auth/profile-detector.ts b/src/auth/profile-detector.ts index 064f9360..23570249 100644 --- a/src/auth/profile-detector.ts +++ b/src/auth/profile-detector.ts @@ -25,8 +25,8 @@ import { getCcsDir } from '../utils/config-manager'; import type { CLIProxyProvider } from '../cliproxy/types'; import { CLIPROXY_PROVIDER_IDS, isCLIProxyProvider } from '../cliproxy/provider-capabilities'; import type { TargetType } from '../targets/target-adapter'; - -export type ProfileType = 'settings' | 'account' | 'cliproxy' | 'copilot' | 'default'; +import type { ProfileType } from '../types/profile'; +export type { ProfileType } from '../types/profile'; /** CLIProxy profile names (OAuth-based, zero config) */ export const CLIPROXY_PROFILES: readonly CLIProxyProvider[] = CLIPROXY_PROVIDER_IDS; diff --git a/src/targets/claude-adapter.ts b/src/targets/claude-adapter.ts index 2c2b7235..d7b60035 100644 --- a/src/targets/claude-adapter.ts +++ b/src/targets/claude-adapter.ts @@ -8,10 +8,11 @@ import { spawn, ChildProcess } from 'child_process'; import { TargetAdapter, TargetBinaryInfo, TargetCredentials, TargetType } from './target-adapter'; import { detectClaudeCli, getClaudeCliInfo } from '../utils/claude-detector'; +import type { ProfileType } from '../types/profile'; import { escapeShellArg, stripAnthropicEnv } from '../utils/shell-executor'; import { ErrorManager } from '../utils/error-manager'; import { getWebSearchHookEnv } from '../utils/websearch-manager'; -import { forwardSignals } from '../utils/signal-forwarder'; +import { wireChildProcessSignals } from '../utils/signal-forwarder'; export class ClaudeAdapter implements TargetAdapter { readonly type: TargetType = 'claude'; @@ -34,7 +35,7 @@ export class ClaudeAdapter implements TargetAdapter { return userArgs; } - buildEnv(creds: TargetCredentials, profileType: string): NodeJS.ProcessEnv { + buildEnv(creds: TargetCredentials, profileType: ProfileType): NodeJS.ProcessEnv { const webSearchEnv = getWebSearchHookEnv(); // For account/default profiles, strip ANTHROPIC_* from parent env to prevent @@ -100,16 +101,7 @@ export class ClaudeAdapter implements TargetAdapter { }); } - const cleanupSignalHandlers = forwardSignals(child); - - child.on('exit', (code, signal) => { - cleanupSignalHandlers(); - if (signal) process.kill(process.pid, signal as NodeJS.Signals); - else process.exit(code || 0); - }); - - child.on('error', async (err: NodeJS.ErrnoException) => { - cleanupSignalHandlers(); + wireChildProcessSignals(child, async (err: NodeJS.ErrnoException) => { if (err.code === 'EACCES') { console.error(`[X] Claude CLI is not executable: ${claudeCli}`); console.error(' Check file permissions and executable bit.'); @@ -133,7 +125,7 @@ export class ClaudeAdapter implements TargetAdapter { /** * Claude supports all CCS profile types. */ - supportsProfileType(_profileType: string): boolean { + supportsProfileType(_profileType: ProfileType): boolean { return true; } } diff --git a/src/targets/droid-adapter.ts b/src/targets/droid-adapter.ts index 8368b0c7..79004bd3 100644 --- a/src/targets/droid-adapter.ts +++ b/src/targets/droid-adapter.ts @@ -9,9 +9,10 @@ import { spawn, ChildProcess } from 'child_process'; import * as fs from 'fs'; import { TargetAdapter, TargetBinaryInfo, TargetCredentials, TargetType } from './target-adapter'; import { getDroidBinaryInfo, detectDroidCli, checkDroidVersion } from './droid-detector'; +import type { ProfileType } from '../types/profile'; import { upsertCcsModel } from './droid-config-manager'; import { escapeShellArg } from '../utils/shell-executor'; -import { forwardSignals } from '../utils/signal-forwarder'; +import { wireChildProcessSignals } from '../utils/signal-forwarder'; export class DroidAdapter implements TargetAdapter { readonly type: TargetType = 'droid'; @@ -57,7 +58,7 @@ export class DroidAdapter implements TargetAdapter { /** * Droid uses config file for credentials — minimal env needed. */ - buildEnv(_creds: TargetCredentials, _profileType: string): NodeJS.ProcessEnv { + buildEnv(_creds: TargetCredentials, _profileType: ProfileType): NodeJS.ProcessEnv { return { ...process.env }; } @@ -119,16 +120,7 @@ export class DroidAdapter implements TargetAdapter { }); } - const cleanupSignalHandlers = forwardSignals(child); - - child.on('exit', (code, signal) => { - cleanupSignalHandlers(); - if (signal) process.kill(process.pid, signal as NodeJS.Signals); - else process.exit(code || 0); - }); - - child.on('error', (err: NodeJS.ErrnoException) => { - cleanupSignalHandlers(); + wireChildProcessSignals(child, (err: NodeJS.ErrnoException) => { if (err.code === 'EACCES') { console.error(`[X] Droid CLI is not executable: ${droidPath}`); console.error(' Check file permissions and executable bit.'); @@ -153,7 +145,7 @@ export class DroidAdapter implements TargetAdapter { /** * Droid currently supports direct settings-based and default flows only. */ - supportsProfileType(profileType: string): boolean { + supportsProfileType(profileType: ProfileType): boolean { return profileType === 'settings' || profileType === 'default'; } } diff --git a/src/targets/droid-config-manager.ts b/src/targets/droid-config-manager.ts index 47a73cc8..348e345d 100644 --- a/src/targets/droid-config-manager.ts +++ b/src/targets/droid-config-manager.ts @@ -11,6 +11,7 @@ import * as os from 'os'; import * as lockfile from 'proper-lockfile'; const CCS_MODEL_PREFIX = 'ccs-'; +const CCS_DISPLAY_PREFIX = 'CCS '; /** Lock configuration for concurrent write safety */ const LOCK_STALE_MS = 10000; @@ -21,8 +22,12 @@ const LOCK_RETRY_MAX_MS = 1000; * Validate profile name to prevent filesystem/security issues. * Only alphanumeric, underscore, hyphen allowed. */ +function isValidProfileName(profile: string): boolean { + return !!profile && /^[a-zA-Z0-9_-]+$/.test(profile); +} + function validateProfileName(profile: string): void { - if (!profile || !/^[a-zA-Z0-9_-]+$/.test(profile)) { + if (!isValidProfileName(profile)) { throw new Error( `Invalid profile name "${profile}": must contain only alphanumeric characters, underscores, or hyphens` ); @@ -57,21 +62,39 @@ function isSupportedProvider(value: string): value is DroidCustomModel['provider return value === 'anthropic' || value === 'openai' || value === 'generic-chat-completion-api'; } -function asModelEntry(value: unknown): DroidCustomModelEntry | null { - if (!value || typeof value !== 'object') return null; +function isDroidCustomModelEntry(value: unknown): value is DroidCustomModelEntry { + if (!value || typeof value !== 'object') return false; const record = value as Record; - if ( - typeof record.displayName !== 'string' || - record.displayName.trim() === '' || - typeof record.model !== 'string' || - typeof record.baseUrl !== 'string' || - typeof record.apiKey !== 'string' || - typeof record.provider !== 'string' || - record.provider.trim() === '' - ) { - return null; + return ( + typeof record.displayName === 'string' && + record.displayName.trim() !== '' && + typeof record.model === 'string' && + typeof record.baseUrl === 'string' && + typeof record.apiKey === 'string' && + typeof record.provider === 'string' && + record.provider.trim() !== '' + ); +} + +function isManagedDisplayName(displayName: string): boolean { + return displayName.startsWith(CCS_DISPLAY_PREFIX) || displayName.startsWith(CCS_MODEL_PREFIX); +} + +function parseManagedProfile(displayName: string): string | null { + let profile: string | null = null; + + if (displayName.startsWith(CCS_DISPLAY_PREFIX)) { + profile = displayName.slice(CCS_DISPLAY_PREFIX.length).trim(); + } else if (displayName.startsWith(CCS_MODEL_PREFIX)) { + profile = displayName.slice(CCS_MODEL_PREFIX.length).trim(); } - return value as DroidCustomModelEntry; + + if (!profile || !isValidProfileName(profile)) return null; + return profile; +} + +function asModelEntry(value: unknown): DroidCustomModelEntry | null { + return isDroidCustomModelEntry(value) ? value : null; } function normalizeCustomModels(value: unknown): DroidCustomModelEntry[] { @@ -275,14 +298,6 @@ function writeDroidSettings(settings: DroidSettings): void { } } -/** - * Build the custom model alias from a CCS profile name. - * e.g., "gemini" → "ccs-gemini" - */ -function ccsAlias(profile: string): string { - return `${CCS_MODEL_PREFIX}${profile}`; -} - /** * Upsert a CCS-managed custom model entry. * Acquires file lock to prevent concurrent write races. @@ -293,20 +308,19 @@ export async function upsertCcsModel(profile: string, model: DroidCustomModel): let release: (() => Promise) | undefined; try { - release = await acquireFactoryLock(5); + release = await acquireFactoryLock(10); const settings = readDroidSettings(); settings.customModels = normalizeCustomModels(settings.customModels); - const alias = ccsAlias(profile); const entry: DroidCustomModelEntry = { ...model, displayName: `CCS ${profile}`, }; - // Find existing entry by checking displayName for CCS prefix match + // Find existing current or legacy entry for this profile. const idx = settings.customModels.findIndex( - (m) => m.displayName === `CCS ${profile}` || m.displayName === alias + (m) => parseManagedProfile(m.displayName) === profile ); if (idx >= 0) { @@ -338,7 +352,7 @@ export async function removeCcsModel(profile: string): Promise { settings.customModels = normalizeCustomModels(settings.customModels); settings.customModels = settings.customModels.filter( - (m) => m.displayName !== `CCS ${profile}` && m.displayName !== ccsAlias(profile) + (m) => parseManagedProfile(m.displayName) !== profile ); writeDroidSettings(settings); @@ -354,16 +368,14 @@ export async function listCcsModels(): Promise> { const result = new Map(); const settings = readDroidSettings(); for (const entry of normalizeCustomModels(settings.customModels)) { - if (entry.displayName?.startsWith('CCS ')) { - if (!isSupportedProvider(entry.provider)) { - continue; - } - const profile = entry.displayName.slice(4); // Remove "CCS " prefix - result.set(profile, { - ...entry, - provider: entry.provider, - }); - } + const profile = parseManagedProfile(entry.displayName); + if (!profile) continue; + if (!isSupportedProvider(entry.provider)) continue; + + result.set(profile, { + ...entry, + provider: entry.provider, + }); } return result; @@ -392,9 +404,13 @@ export async function pruneOrphanedModels(activeProfiles: string[]): Promise { - if (!m.displayName?.startsWith('CCS ')) return true; // Keep non-CCS entries - const profile = m.displayName.slice(4); - return activeProfiles.includes(profile); + const profile = parseManagedProfile(m.displayName); + if (profile) { + return activeProfiles.includes(profile); + } + + // Drop malformed managed entries; keep user-managed entries. + return !isManagedDisplayName(m.displayName); }); removed = before - settings.customModels.length; diff --git a/src/targets/target-adapter.ts b/src/targets/target-adapter.ts index edb6ff7d..522ee905 100644 --- a/src/targets/target-adapter.ts +++ b/src/targets/target-adapter.ts @@ -9,6 +9,8 @@ * Supported CLI target types. * 'claude' is the default; additional targets register via target-registry. */ +import type { ProfileType } from '../types/profile'; + export type TargetType = 'claude' | 'droid'; /** @@ -46,25 +48,46 @@ export interface TargetAdapter { readonly type: TargetType; readonly displayName: string; - /** Detect if the target CLI binary exists on system */ + /** + * Resolve the target CLI executable on the current machine. + * Return `null` when the binary is unavailable. + */ detectBinary(): TargetBinaryInfo | null; - /** Prepare credentials for delivery to target CLI */ + /** + * Prepare credential delivery for the target. + * Targets may write config files, mutate process state, or no-op. + * + * @throws Error when required credentials are missing or invalid. + */ prepareCredentials(creds: TargetCredentials): Promise; - /** Build spawn arguments for the target CLI */ + /** + * Build target-specific argument vector. + * `userArgs` are the arguments after CCS profile/flag parsing. + */ buildArgs(profile: string, userArgs: string[]): string[]; - /** Build environment variables for the target CLI */ - buildEnv(creds: TargetCredentials, profileType: string): NodeJS.ProcessEnv; + /** + * Build environment variables for process spawn. + * `profileType` allows targets to vary env behavior by CCS profile mode. + */ + buildEnv(creds: TargetCredentials, profileType: ProfileType): NodeJS.ProcessEnv; - /** Spawn the target CLI process (replaces current process flow) */ + /** + * Spawn and hand over execution to the target CLI process. + * Implementations are responsible for signal forwarding and exit propagation. + * + * @throws Error for unrecoverable launch failures (if not exiting directly). + */ exec( args: string[], env: NodeJS.ProcessEnv, options?: { cwd?: string; binaryInfo?: TargetBinaryInfo } ): void; - /** Check if a profile type is supported by this target */ - supportsProfileType(profileType: string): boolean; + /** + * Report whether this target can run a given CCS profile type. + */ + supportsProfileType(profileType: ProfileType): boolean; } diff --git a/src/types/index.ts b/src/types/index.ts index 4adba8b4..494883be 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -50,3 +50,6 @@ export type { // Utility types export { LogLevel } from './utils'; export type { ErrorCode, ColorName, TerminalInfo, Result } from './utils'; + +// Profile routing types +export type { ProfileType } from './profile'; diff --git a/src/types/profile.ts b/src/types/profile.ts new file mode 100644 index 00000000..75e3ff44 --- /dev/null +++ b/src/types/profile.ts @@ -0,0 +1,4 @@ +/** + * Profile mode types used across routing and target adapters. + */ +export type ProfileType = 'settings' | 'account' | 'cliproxy' | 'copilot' | 'default'; diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 88b56fc3..017cd709 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -7,7 +7,7 @@ import { spawn, spawnSync, ChildProcess } from 'child_process'; import { ErrorManager } from './error-manager'; import { getWebSearchHookEnv } from './websearch-manager'; -import { forwardSignals } from './signal-forwarder'; +import { wireChildProcessSignals } from './signal-forwarder'; /** * Strip ANTHROPIC_* env vars from an environment object. @@ -128,16 +128,7 @@ export function execClaude( }); } - const cleanupSignalHandlers = forwardSignals(child); - - child.on('exit', (code, signal) => { - cleanupSignalHandlers(); - if (signal) process.kill(process.pid, signal as NodeJS.Signals); - else process.exit(code || 0); - }); - - child.on('error', async (err: NodeJS.ErrnoException) => { - cleanupSignalHandlers(); + wireChildProcessSignals(child, async (err: NodeJS.ErrnoException) => { if (err.code === 'EACCES') { console.error(`[X] Claude CLI is not executable: ${claudeCli}`); console.error(' Check file permissions and executable bit.'); diff --git a/src/utils/signal-forwarder.ts b/src/utils/signal-forwarder.ts index e2af8787..174d56fd 100644 --- a/src/utils/signal-forwarder.ts +++ b/src/utils/signal-forwarder.ts @@ -31,3 +31,47 @@ export function forwardSignals(child: ChildProcess): () => void { process.removeListener('SIGHUP', forwardSighup); }; } + +export type ChildProcessErrorHandler = (err: NodeJS.ErrnoException) => void | Promise; +export type ChildProcessExitHandler = (code: number | null, signal: NodeJS.Signals | null) => void; + +function defaultExitHandler(code: number | null, signal: NodeJS.Signals | null): void { + if (signal) process.kill(process.pid, signal); + else process.exit(code || 0); +} + +/** + * Attach shared signal-forwarding lifecycle handlers to a child process. + * Ensures signal listeners are always cleaned up on child exit/error. + */ +export function wireChildProcessSignals( + child: ChildProcess, + onError: ChildProcessErrorHandler, + onExit: ChildProcessExitHandler = defaultExitHandler +): void { + const cleanupSignalHandlers = forwardSignals(child); + let settled = false; + + const settle = (): boolean => { + if (settled) return false; + settled = true; + cleanupSignalHandlers(); + return true; + }; + + child.on('exit', (code, signal) => { + if (!settle()) return; + onExit(code, signal); + }); + + child.on('error', async (err: NodeJS.ErrnoException) => { + if (!settle()) return; + try { + await onError(err); + } catch (handlerErr) { + const message = handlerErr instanceof Error ? handlerErr.message : String(handlerErr); + console.error(`[X] Failed to handle child process error: ${message}`); + process.exit(1); + } + }); +} diff --git a/tests/unit/targets/ccsd-alias-integration.test.ts b/tests/unit/targets/ccsd-alias-integration.test.ts new file mode 100644 index 00000000..ebc4fd69 --- /dev/null +++ b/tests/unit/targets/ccsd-alias-integration.test.ts @@ -0,0 +1,72 @@ +/** + * Integration-style test for Node argv alias behavior. + * + * This validates the runtime assumption used by target-resolver: + * when invoked via a `ccsd` symlink, Node preserves the invoked + * symlink path in process.argv[1]. + */ +import { describe, it, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { spawnSync } from 'child_process'; + +function probeArgvPath(aliasBasename: string): string { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-ccsd-alias-')); + const scriptPath = path.join(tmpDir, 'probe.js'); + const aliasPath = path.join(tmpDir, aliasBasename); + + try { + fs.writeFileSync(scriptPath, 'console.log(process.argv[1]);\n', { encoding: 'utf8' }); + fs.symlinkSync(scriptPath, aliasPath); + + const result = spawnSync('node', [aliasPath], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + + expect(result.status).toBe(0); + return result.stdout.trim(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +function probeArgvPathDirect(filename: string): string { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-ccsd-direct-')); + const scriptPath = path.join(tmpDir, filename); + + try { + fs.writeFileSync(scriptPath, 'console.log(process.argv[1]);\n', { encoding: 'utf8' }); + + const result = spawnSync('node', [scriptPath], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + + expect(result.status).toBe(0); + return result.stdout.trim(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +describe('ccsd alias integration', () => { + it('should preserve ccsd symlink basename in argv[1] under node', () => { + if (process.platform === 'win32') { + // Windows symlink creation requires elevated privileges/Developer Mode. + return; + } + + const argvPath = probeArgvPath('ccsd'); + expect(path.basename(argvPath)).toBe('ccsd'); + }); + + it('should preserve extension-style alias basenames for wrapper compatibility', () => { + const cmdArgvPath = probeArgvPathDirect('ccsd.cmd'); + const ps1ArgvPath = probeArgvPathDirect('ccsd.ps1'); + + expect(path.basename(cmdArgvPath)).toBe('ccsd.cmd'); + expect(path.basename(ps1ArgvPath)).toBe('ccsd.ps1'); + }); +}); diff --git a/tests/unit/targets/droid-config-manager.test.ts b/tests/unit/targets/droid-config-manager.test.ts index e2d63790..c1743d67 100644 --- a/tests/unit/targets/droid-config-manager.test.ts +++ b/tests/unit/targets/droid-config-manager.test.ts @@ -175,6 +175,60 @@ describe('droid-config-manager', () => { }) ).rejects.toThrow(/settings\.json\.tmp is a symlink/); }); + + it('should update legacy ccs- alias entry on upsert', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { + model: 'claude-opus-4-6', + displayName: 'ccs-gemini', + baseUrl: 'http://localhost:8317', + apiKey: 'old-key', + provider: 'anthropic', + }, + ], + }) + ); + + await upsertCcsModel('gemini', { + model: 'claude-opus-4-6', + displayName: 'CCS gemini', + baseUrl: 'http://localhost:8318', + apiKey: 'new-key', + provider: 'anthropic', + }); + + const settings = JSON.parse( + fs.readFileSync(path.join(factoryDir, 'settings.json'), 'utf8') + ); + expect(settings.customModels).toHaveLength(1); + expect(settings.customModels[0].displayName).toBe('CCS gemini'); + expect(settings.customModels[0].apiKey).toBe('new-key'); + expect(settings.customModels[0].baseUrl).toBe('http://localhost:8318'); + }); + + it('should reject symlinked settings file on write', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + + const realSettings = path.join(factoryDir, 'real-settings.json'); + fs.writeFileSync(realSettings, JSON.stringify({ customModels: [] })); + fs.symlinkSync(realSettings, path.join(factoryDir, 'settings.json')); + + await expect( + upsertCcsModel('gemini', { + model: 'claude-opus-4-6', + displayName: 'CCS gemini', + baseUrl: 'http://localhost:8317', + apiKey: 'dummy-key', + provider: 'anthropic', + }) + ).rejects.toThrow(/settings\.json is a symlink/); + }); }); describe('removeCcsModel', () => { @@ -213,6 +267,26 @@ describe('droid-config-manager', () => { expect(settings.customModels).toHaveLength(1); expect(settings.customModels[0].displayName).toBe('My GPT'); }); + + it('should remove legacy ccs- alias entries', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { model: 'opus', displayName: 'ccs-gemini', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'gpt-4o', displayName: 'My GPT', baseUrl: 'x', apiKey: 'y', provider: 'openai' }, + ], + }) + ); + + await removeCcsModel('gemini'); + + const settings = JSON.parse(fs.readFileSync(path.join(factoryDir, 'settings.json'), 'utf8')); + expect(settings.customModels).toHaveLength(1); + expect(settings.customModels[0].displayName).toBe('My GPT'); + }); }); describe('listCcsModels', () => { @@ -295,6 +369,60 @@ describe('droid-config-manager', () => { await expect(listCcsModels()).rejects.toThrow(/settings\.json is a symlink/); }); + + it('should include legacy ccs- alias entries', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { + model: 'opus', + displayName: 'ccs-gemini', + baseUrl: 'http://localhost:8317', + apiKey: 'dummy', + provider: 'anthropic', + }, + ], + }) + ); + + const models = await listCcsModels(); + expect(models.size).toBe(1); + expect(models.has('gemini')).toBe(true); + }); + + it('should ignore malformed managed display names', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { model: 'x', displayName: 'CCS ', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'x', displayName: 'ccs-', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'x', displayName: 'CCS ok', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + ], + }) + ); + + const models = await listCcsModels(); + expect(models.size).toBe(1); + expect(models.has('ok')).toBe(true); + }); + + it('should recover from corrupted JSON by backing up and returning empty models', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + const settingsPath = path.join(factoryDir, 'settings.json'); + + fs.writeFileSync(settingsPath, '{"customModels":[', 'utf8'); + + const models = await listCcsModels(); + expect(models.size).toBe(0); + expect(fs.existsSync(`${settingsPath}.bak`)).toBe(true); + }); }); describe('pruneOrphanedModels', () => { @@ -343,18 +471,59 @@ describe('droid-config-manager', () => { expect(settings.customModels).toHaveLength(1); expect(settings.customModels[0].displayName).toBe('My GPT'); }); + + it('should prune orphaned legacy ccs- alias entries', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { model: 'opus', displayName: 'ccs-gemini', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'sonnet', displayName: 'ccs-codex', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + ], + }) + ); + + const removed = await pruneOrphanedModels(['gemini']); + expect(removed).toBe(1); + + const models = await listCcsModels(); + expect(models.size).toBe(1); + expect(models.has('gemini')).toBe(true); + }); + + it('should prune malformed managed entries while preserving user models', async () => { + const factoryDir = path.join(tmpDir, '.factory'); + fs.mkdirSync(factoryDir, { recursive: true }); + fs.writeFileSync( + path.join(factoryDir, 'settings.json'), + JSON.stringify({ + customModels: [ + { model: 'x', displayName: 'CCS ', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'x', displayName: 'ccs-', baseUrl: 'x', apiKey: 'y', provider: 'anthropic' }, + { model: 'gpt-4o', displayName: 'My GPT', baseUrl: 'x', apiKey: 'y', provider: 'openai' }, + ], + }) + ); + + const removed = await pruneOrphanedModels([]); + expect(removed).toBe(2); + + const settings = JSON.parse(fs.readFileSync(path.join(factoryDir, 'settings.json'), 'utf8')); + expect(settings.customModels).toHaveLength(1); + expect(settings.customModels[0].displayName).toBe('My GPT'); + }); }); describe('concurrent writes', () => { - it('should handle concurrent upserts without data loss', async () => { - // Write in batches of 3 to simulate realistic concurrency - // (10 simultaneous locks exceeds retry budget) - const profiles = Array.from({ length: 9 }, (_, i) => `profile-${i}`); + it( + 'should handle concurrent upserts without data loss', + async () => { + const profiles = Array.from({ length: 10 }, (_, i) => `profile-${i}`); - for (let i = 0; i < profiles.length; i += 3) { - const batch = profiles.slice(i, i + 3); await Promise.all( - batch.map((p) => + profiles.map((p) => upsertCcsModel(p, { model: 'test-model', displayName: `CCS ${p}`, @@ -364,14 +533,15 @@ describe('droid-config-manager', () => { }) ) ); - } - const models = await listCcsModels(); - expect(models.size).toBe(9); + const models = await listCcsModels(); + expect(models.size).toBe(10); - for (const p of profiles) { - expect(models.has(p)).toBe(true); - } - }); + for (const p of profiles) { + expect(models.has(p)).toBe(true); + } + }, + 15000 + ); }); }); diff --git a/tests/unit/utils/signal-forwarder.test.ts b/tests/unit/utils/signal-forwarder.test.ts new file mode 100644 index 00000000..463453f8 --- /dev/null +++ b/tests/unit/utils/signal-forwarder.test.ts @@ -0,0 +1,148 @@ +import { describe, it, expect, jest } from 'bun:test'; +import { EventEmitter } from 'events'; +import type { ChildProcess } from 'child_process'; +import { forwardSignals, wireChildProcessSignals } from '../../../src/utils/signal-forwarder'; + +type MockChildProcess = EventEmitter & { + killed: boolean; + kill: ChildProcess['kill']; +}; + +function createMockChildProcess(): ChildProcess { + const child = new EventEmitter() as MockChildProcess; + child.killed = false; + child.kill = jest.fn(() => true) as ChildProcess['kill']; + return child as ChildProcess; +} + +function getSignalListenerCounts(): Record<'SIGINT' | 'SIGTERM' | 'SIGHUP', number> { + return { + SIGINT: process.listenerCount('SIGINT'), + SIGTERM: process.listenerCount('SIGTERM'), + SIGHUP: process.listenerCount('SIGHUP'), + }; +} + +describe('signal-forwarder', () => { + it('forwardSignals should register and cleanup listeners', () => { + const child = createMockChildProcess(); + const before = getSignalListenerCounts(); + + const cleanup = forwardSignals(child); + + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT + 1); + expect(process.listenerCount('SIGTERM')).toBe(before.SIGTERM + 1); + expect(process.listenerCount('SIGHUP')).toBe(before.SIGHUP + 1); + + cleanup(); + + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT); + expect(process.listenerCount('SIGTERM')).toBe(before.SIGTERM); + expect(process.listenerCount('SIGHUP')).toBe(before.SIGHUP); + }); + + it('wireChildProcessSignals should use default exit behavior for exit code', () => { + const child = createMockChildProcess(); + const before = getSignalListenerCounts(); + + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => undefined as never) as typeof process.exit); + const killSpy = jest + .spyOn(process, 'kill') + .mockImplementation((() => true) as typeof process.kill); + + try { + wireChildProcessSignals(child, () => {}); + child.emit('exit', 7, null); + + expect(exitSpy).toHaveBeenCalledWith(7); + expect(killSpy).not.toHaveBeenCalled(); + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT); + expect(process.listenerCount('SIGTERM')).toBe(before.SIGTERM); + expect(process.listenerCount('SIGHUP')).toBe(before.SIGHUP); + } finally { + exitSpy.mockRestore(); + killSpy.mockRestore(); + } + }); + + it('wireChildProcessSignals should use default exit behavior for signal', () => { + const child = createMockChildProcess(); + const before = getSignalListenerCounts(); + + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => undefined as never) as typeof process.exit); + const killSpy = jest + .spyOn(process, 'kill') + .mockImplementation((() => true) as typeof process.kill); + + try { + wireChildProcessSignals(child, () => {}); + child.emit('exit', null, 'SIGTERM'); + + expect(killSpy).toHaveBeenCalledWith(process.pid, 'SIGTERM'); + expect(exitSpy).not.toHaveBeenCalled(); + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT); + expect(process.listenerCount('SIGTERM')).toBe(before.SIGTERM); + expect(process.listenerCount('SIGHUP')).toBe(before.SIGHUP); + } finally { + exitSpy.mockRestore(); + killSpy.mockRestore(); + } + }); + + it('wireChildProcessSignals should invoke onError and cleanup listeners', async () => { + const child = createMockChildProcess(); + const before = getSignalListenerCounts(); + const onError = jest.fn(async () => {}); + const err = Object.assign(new Error('spawn failed'), { code: 'ENOENT' }) as NodeJS.ErrnoException; + + wireChildProcessSignals(child, onError); + child.emit('error', err); + await Promise.resolve(); + + expect(onError).toHaveBeenCalledWith(err); + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT); + expect(process.listenerCount('SIGTERM')).toBe(before.SIGTERM); + expect(process.listenerCount('SIGHUP')).toBe(before.SIGHUP); + }); + + it('wireChildProcessSignals should run only one terminal callback when error is followed by exit', async () => { + const child = createMockChildProcess(); + const onError = jest.fn(async () => {}); + const onExit = jest.fn(); + const err = Object.assign(new Error('spawn failed'), { code: 'ENOENT' }) as NodeJS.ErrnoException; + + wireChildProcessSignals(child, onError, onExit); + child.emit('error', err); + child.emit('exit', 1, null); + await Promise.resolve(); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onExit).not.toHaveBeenCalled(); + }); + + it('wireChildProcessSignals should exit with code 1 when onError throws', async () => { + const child = createMockChildProcess(); + const onError = jest.fn(async () => { + throw new Error('handler exploded'); + }); + const exitSpy = jest + .spyOn(process, 'exit') + .mockImplementation((() => undefined as never) as typeof process.exit); + + try { + const err = Object.assign(new Error('spawn failed'), { code: 'ENOENT' }) as NodeJS.ErrnoException; + wireChildProcessSignals(child, onError); + child.emit('error', err); + await Promise.resolve(); + + expect(onError).toHaveBeenCalledTimes(1); + expect(exitSpy).toHaveBeenCalledWith(1); + } finally { + exitSpy.mockRestore(); + } + }); +});