From 02af8d5737d9c3db4172b094b1bfae13ccdccbb1 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Tue, 17 Feb 2026 03:38:27 +0700 Subject: [PATCH] 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 --- docs/metrics/maintainability-baseline.json | 6 ++-- src/ccs.ts | 36 +--------------------- src/commands/help-command.ts | 7 +++++ src/targets/claude-adapter.ts | 20 ++---------- src/targets/droid-adapter.ts | 20 ++---------- src/targets/droid-config-manager.ts | 14 +++++++-- src/utils/shell-executor.ts | 20 ++---------- src/utils/signal-forwarder.ts | 33 ++++++++++++++++++++ 8 files changed, 61 insertions(+), 95 deletions(-) create mode 100644 src/utils/signal-forwarder.ts diff --git a/docs/metrics/maintainability-baseline.json b/docs/metrics/maintainability-baseline.json index 967cd8dd..734ab29f 100644 --- a/docs/metrics/maintainability-baseline.json +++ b/docs/metrics/maintainability-baseline.json @@ -2,8 +2,8 @@ "sourceDirectory": "src", "largeFileThresholdLoc": 350, "typeScriptFileCount": 355, - "locInSrc": 71420, - "processExitReferenceCount": 188, - "synchronousFsApiReferenceCount": 880, + "locInSrc": 71353, + "processExitReferenceCount": 185, + "synchronousFsApiReferenceCount": 879, "largeFileCountOver350Loc": 56 } diff --git a/src/ccs.ts b/src/ccs.ts index e7e7dd74..b1df1571 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -725,7 +725,7 @@ async function main(): Promise { // 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 { } 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 { 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 { 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(); diff --git a/src/commands/help-command.ts b/src/commands/help-command.ts index bc84c08f..5971dfb5 100644 --- a/src/commands/help-command.ts +++ b/src/commands/help-command.ts @@ -312,6 +312,13 @@ Run ${color('ccs config', 'command')} for web dashboard`.trim(); ['ccsd [args]', 'Shorthand for: ccs --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`], diff --git a/src/targets/claude-adapter.ts b/src/targets/claude-adapter.ts index 2f61e4b6..2c2b7235 100644 --- a/src/targets/claude-adapter.ts +++ b/src/targets/claude-adapter.ts @@ -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(); diff --git a/src/targets/droid-adapter.ts b/src/targets/droid-adapter.ts index 0cbb6946..8368b0c7 100644 --- a/src/targets/droid-adapter.ts +++ b/src/targets/droid-adapter.ts @@ -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(); diff --git a/src/targets/droid-config-manager.ts b/src/targets/droid-config-manager.ts index f1255f66..47a73cc8 100644 --- a/src/targets/droid-config-manager.ts +++ b/src/targets/droid-config-manager.ts @@ -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> 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( diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index a672f97b..88b56fc3 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -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(); diff --git a/src/utils/signal-forwarder.ts b/src/utils/signal-forwarder.ts new file mode 100644 index 00000000..e2af8787 --- /dev/null +++ b/src/utils/signal-forwarder.ts @@ -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); + }; +}