mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 04:18:05 +00:00
fix(targets): harden adapter lifecycle and droid model edge cases
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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';
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<void>) | 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<void> {
|
||||
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<Map<string, DroidCustomModel>> {
|
||||
const result = new Map<string, DroidCustomModel>();
|
||||
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<num
|
||||
|
||||
const before = settings.customModels.length;
|
||||
settings.customModels = settings.customModels.filter((m) => {
|
||||
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;
|
||||
|
||||
@@ -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<void>;
|
||||
|
||||
/** 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;
|
||||
}
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
/**
|
||||
* Profile mode types used across routing and target adapters.
|
||||
*/
|
||||
export type ProfileType = 'settings' | 'account' | 'cliproxy' | 'copilot' | 'default';
|
||||
@@ -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.');
|
||||
|
||||
@@ -31,3 +31,47 @@ export function forwardSignals(child: ChildProcess): () => void {
|
||||
process.removeListener('SIGHUP', forwardSighup);
|
||||
};
|
||||
}
|
||||
|
||||
export type ChildProcessErrorHandler = (err: NodeJS.ErrnoException) => void | Promise<void>;
|
||||
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);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user