mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-15 20:20:09 +00:00
fix(targets): DRY signal handling, remove redundant guards, harden config manager
- Extract signal forwarding to reusable src/utils/signal-forwarder.ts - Update shell-executor, claude-adapter, droid-adapter to use shared utility - Remove 35 lines of redundant duplicate guards in src/ccs.ts - Harden droid-config-manager with lock constants and race-safe ensureFactoryDir - Update help-command with --target usage examples - Update maintainability metrics baseline
This commit is contained in:
@@ -2,8 +2,8 @@
|
||||
"sourceDirectory": "src",
|
||||
"largeFileThresholdLoc": 350,
|
||||
"typeScriptFileCount": 355,
|
||||
"locInSrc": 71420,
|
||||
"processExitReferenceCount": 188,
|
||||
"synchronousFsApiReferenceCount": 880,
|
||||
"locInSrc": 71353,
|
||||
"processExitReferenceCount": 185,
|
||||
"synchronousFsApiReferenceCount": 879,
|
||||
"largeFileCountOver350Loc": 56
|
||||
}
|
||||
|
||||
+1
-35
@@ -725,7 +725,7 @@ async function main(): Promise<void> {
|
||||
|
||||
// For non-claude targets, verify target binary exists once and pass it through.
|
||||
const targetBinaryInfo = targetAdapter?.detectBinary() ?? null;
|
||||
if (resolvedTarget !== 'claude' && (!targetAdapter || !targetBinaryInfo)) {
|
||||
if (resolvedTarget !== 'claude' && !targetBinaryInfo) {
|
||||
const displayName = targetAdapter?.displayName || resolvedTarget;
|
||||
console.error(fail(`${displayName} CLI not found.`));
|
||||
if (resolvedTarget === 'droid') {
|
||||
@@ -759,17 +759,6 @@ async function main(): Promise<void> {
|
||||
}
|
||||
|
||||
if (profileInfo.type === 'cliproxy') {
|
||||
// Guard: non-claude targets don't support CLIProxy flow yet
|
||||
if (resolvedTarget !== 'claude') {
|
||||
if (!targetAdapter?.supportsProfileType('cliproxy')) {
|
||||
console.error(
|
||||
fail(`${targetAdapter?.displayName || 'Target'} does not support CLIProxy profiles`)
|
||||
);
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// CLIPROXY FLOW: OAuth-based profiles (gemini, codex, agy, qwen) or user-defined variants
|
||||
// Inject WebSearch hook into profile settings before launch
|
||||
ensureProfileHooks(profileInfo.name);
|
||||
@@ -788,16 +777,6 @@ async function main(): Promise<void> {
|
||||
profileName: profileInfo.name,
|
||||
});
|
||||
} else if (profileInfo.type === 'copilot') {
|
||||
// Guard: non-claude targets don't support Copilot flow
|
||||
if (resolvedTarget !== 'claude') {
|
||||
if (!targetAdapter?.supportsProfileType('copilot')) {
|
||||
console.error(
|
||||
fail(`${targetAdapter?.displayName || 'Target'} does not support Copilot profiles`)
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// COPILOT FLOW: GitHub Copilot subscription via copilot-api proxy
|
||||
// Inject WebSearch hook into profile settings before launch
|
||||
ensureProfileHooks(profileInfo.name);
|
||||
@@ -943,19 +922,6 @@ async function main(): Promise<void> {
|
||||
execClaude(claudeCli, ['--settings', expandedSettingsPath, ...remainingArgs], envVars);
|
||||
}
|
||||
} else if (profileInfo.type === 'account') {
|
||||
// Guard: non-claude targets don't support account profiles
|
||||
if (resolvedTarget !== 'claude') {
|
||||
if (!targetAdapter?.supportsProfileType('account')) {
|
||||
console.error(
|
||||
fail(
|
||||
`${targetAdapter?.displayName || 'Target'} does not support account-based profiles`
|
||||
)
|
||||
);
|
||||
console.error(info('Use a settings-based profile with --target instead'));
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// NEW FLOW: Account-based profile (work, personal)
|
||||
// All platforms: Use instance isolation with CLAUDE_CONFIG_DIR
|
||||
const registry = new ProfileRegistry();
|
||||
|
||||
@@ -312,6 +312,13 @@ Run ${color('ccs config', 'command')} for web dashboard`.trim();
|
||||
['ccsd <profile> [args]', 'Shorthand for: ccs <profile> --target droid'],
|
||||
]);
|
||||
|
||||
// Multi-target examples
|
||||
printSubSection('Multi-Target', [
|
||||
['ccs glm --target droid', 'Run GLM profile on Droid CLI'],
|
||||
['ccsd glm', 'Same as above (alias)'],
|
||||
['ccs glm', 'Run GLM profile on Claude Code (default)'],
|
||||
]);
|
||||
|
||||
// Configuration
|
||||
printConfigSection('Configuration', [
|
||||
['Config File:', isUnifiedMode() ? `${dirDisplay}/config.yaml` : `${dirDisplay}/config.json`],
|
||||
|
||||
@@ -11,6 +11,7 @@ import { detectClaudeCli, getClaudeCliInfo } from '../utils/claude-detector';
|
||||
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';
|
||||
|
||||
export class ClaudeAdapter implements TargetAdapter {
|
||||
readonly type: TargetType = 'claude';
|
||||
@@ -99,24 +100,7 @@ export class ClaudeAdapter implements TargetAdapter {
|
||||
});
|
||||
}
|
||||
|
||||
const forwardSigInt = () => {
|
||||
if (!child.killed) child.kill('SIGINT');
|
||||
};
|
||||
const forwardSigTerm = () => {
|
||||
if (!child.killed) child.kill('SIGTERM');
|
||||
};
|
||||
const forwardSighup = () => {
|
||||
if (!child.killed) child.kill('SIGHUP');
|
||||
};
|
||||
process.on('SIGINT', forwardSigInt);
|
||||
process.on('SIGTERM', forwardSigTerm);
|
||||
process.on('SIGHUP', forwardSighup);
|
||||
|
||||
const cleanupSignalHandlers = () => {
|
||||
process.removeListener('SIGINT', forwardSigInt);
|
||||
process.removeListener('SIGTERM', forwardSigTerm);
|
||||
process.removeListener('SIGHUP', forwardSighup);
|
||||
};
|
||||
const cleanupSignalHandlers = forwardSignals(child);
|
||||
|
||||
child.on('exit', (code, signal) => {
|
||||
cleanupSignalHandlers();
|
||||
|
||||
@@ -11,6 +11,7 @@ import { TargetAdapter, TargetBinaryInfo, TargetCredentials, TargetType } from '
|
||||
import { getDroidBinaryInfo, detectDroidCli, checkDroidVersion } from './droid-detector';
|
||||
import { upsertCcsModel } from './droid-config-manager';
|
||||
import { escapeShellArg } from '../utils/shell-executor';
|
||||
import { forwardSignals } from '../utils/signal-forwarder';
|
||||
|
||||
export class DroidAdapter implements TargetAdapter {
|
||||
readonly type: TargetType = 'droid';
|
||||
@@ -118,24 +119,7 @@ export class DroidAdapter implements TargetAdapter {
|
||||
});
|
||||
}
|
||||
|
||||
const forwardSigInt = () => {
|
||||
if (!child.killed) child.kill('SIGINT');
|
||||
};
|
||||
const forwardSigTerm = () => {
|
||||
if (!child.killed) child.kill('SIGTERM');
|
||||
};
|
||||
const forwardSighup = () => {
|
||||
if (!child.killed) child.kill('SIGHUP');
|
||||
};
|
||||
process.on('SIGINT', forwardSigInt);
|
||||
process.on('SIGTERM', forwardSigTerm);
|
||||
process.on('SIGHUP', forwardSighup);
|
||||
|
||||
const cleanupSignalHandlers = () => {
|
||||
process.removeListener('SIGINT', forwardSigInt);
|
||||
process.removeListener('SIGTERM', forwardSigTerm);
|
||||
process.removeListener('SIGHUP', forwardSighup);
|
||||
};
|
||||
const cleanupSignalHandlers = forwardSignals(child);
|
||||
|
||||
child.on('exit', (code, signal) => {
|
||||
cleanupSignalHandlers();
|
||||
|
||||
@@ -12,6 +12,11 @@ import * as lockfile from 'proper-lockfile';
|
||||
|
||||
const CCS_MODEL_PREFIX = 'ccs-';
|
||||
|
||||
/** Lock configuration for concurrent write safety */
|
||||
const LOCK_STALE_MS = 10000;
|
||||
const LOCK_RETRY_MIN_MS = 200;
|
||||
const LOCK_RETRY_MAX_MS = 1000;
|
||||
|
||||
/**
|
||||
* Validate profile name to prevent filesystem/security issues.
|
||||
* Only alphanumeric, underscore, hyphen allowed.
|
||||
@@ -104,8 +109,11 @@ function getSettingsPath(): string {
|
||||
*/
|
||||
function ensureFactoryDir(): void {
|
||||
const dir = getFactoryDir();
|
||||
if (!fs.existsSync(dir)) {
|
||||
try {
|
||||
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
|
||||
} catch (err) {
|
||||
const error = err as NodeJS.ErrnoException;
|
||||
if (error.code !== 'EEXIST') throw error;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,8 +189,8 @@ async function acquireFactoryLock(retries: number): Promise<() => Promise<void>>
|
||||
const factoryDir = getFactoryDir();
|
||||
try {
|
||||
return await lockfile.lock(factoryDir, {
|
||||
stale: 10000,
|
||||
retries: { retries, minTimeout: 200, maxTimeout: 1000 },
|
||||
stale: LOCK_STALE_MS,
|
||||
retries: { retries, minTimeout: LOCK_RETRY_MIN_MS, maxTimeout: LOCK_RETRY_MAX_MS },
|
||||
});
|
||||
} catch (error) {
|
||||
throw new Error(
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
import { spawn, spawnSync, ChildProcess } from 'child_process';
|
||||
import { ErrorManager } from './error-manager';
|
||||
import { getWebSearchHookEnv } from './websearch-manager';
|
||||
import { forwardSignals } from './signal-forwarder';
|
||||
|
||||
/**
|
||||
* Strip ANTHROPIC_* env vars from an environment object.
|
||||
@@ -127,24 +128,7 @@ export function execClaude(
|
||||
});
|
||||
}
|
||||
|
||||
const forwardSigInt = () => {
|
||||
if (!child.killed) child.kill('SIGINT');
|
||||
};
|
||||
const forwardSigTerm = () => {
|
||||
if (!child.killed) child.kill('SIGTERM');
|
||||
};
|
||||
const forwardSighup = () => {
|
||||
if (!child.killed) child.kill('SIGHUP');
|
||||
};
|
||||
process.on('SIGINT', forwardSigInt);
|
||||
process.on('SIGTERM', forwardSigTerm);
|
||||
process.on('SIGHUP', forwardSighup);
|
||||
|
||||
const cleanupSignalHandlers = () => {
|
||||
process.removeListener('SIGINT', forwardSigInt);
|
||||
process.removeListener('SIGTERM', forwardSigTerm);
|
||||
process.removeListener('SIGHUP', forwardSighup);
|
||||
};
|
||||
const cleanupSignalHandlers = forwardSignals(child);
|
||||
|
||||
child.on('exit', (code, signal) => {
|
||||
cleanupSignalHandlers();
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* Signal Forwarder
|
||||
*
|
||||
* Shared utility for forwarding process signals to child processes
|
||||
* and cleaning up handlers on exit.
|
||||
*/
|
||||
import { ChildProcess } from 'child_process';
|
||||
|
||||
/**
|
||||
* Forward SIGINT, SIGTERM, SIGHUP to a child process.
|
||||
* Returns a cleanup function to remove the handlers.
|
||||
*/
|
||||
export function forwardSignals(child: ChildProcess): () => void {
|
||||
const forwardSigInt = () => {
|
||||
if (!child.killed) child.kill('SIGINT');
|
||||
};
|
||||
const forwardSigTerm = () => {
|
||||
if (!child.killed) child.kill('SIGTERM');
|
||||
};
|
||||
const forwardSighup = () => {
|
||||
if (!child.killed) child.kill('SIGHUP');
|
||||
};
|
||||
|
||||
process.on('SIGINT', forwardSigInt);
|
||||
process.on('SIGTERM', forwardSigTerm);
|
||||
process.on('SIGHUP', forwardSighup);
|
||||
|
||||
return () => {
|
||||
process.removeListener('SIGINT', forwardSigInt);
|
||||
process.removeListener('SIGTERM', forwardSigTerm);
|
||||
process.removeListener('SIGHUP', forwardSighup);
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user