From 0e6965d205c06667e9fe43205d8fadcfa4278aa7 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 28 Mar 2026 09:51:13 -0400 Subject: [PATCH] fix(websearch): harden settings-profile hook setup - fail enabled settings-profile create and launch flows when hook preparation is still unusable - refresh stale shared hook binaries without rewriting unchanged installs or failing benign races - add rollback and regression coverage for disabled, stale, invalid, copy, import, and launch paths --- src/api/services/profile-lifecycle-service.ts | 18 ++- src/api/services/profile-writer.ts | 39 +++++- src/ccs.ts | 8 +- src/cliproxy/services/variant-settings.ts | 48 ++++++- src/utils/websearch-manager.ts | 2 +- src/utils/websearch/hook-installer.ts | 47 ++++++- src/utils/websearch/index.ts | 2 +- src/utils/websearch/profile-hook-injector.ts | 64 ++++++--- .../api/profile-lifecycle-service.test.ts | 124 +++++++++++++++++- .../unit/api/profile-writer-anthropic.test.ts | 48 ++++++- .../settings-profile-websearch-launch.test.ts | 123 +++++++++++++++++ .../websearch/profile-hook-injector.test.ts | 72 +++++++++- 12 files changed, 549 insertions(+), 46 deletions(-) create mode 100644 tests/unit/targets/settings-profile-websearch-launch.test.ts diff --git a/src/api/services/profile-lifecycle-service.ts b/src/api/services/profile-lifecycle-service.ts index a3e69a7a..0265e614 100644 --- a/src/api/services/profile-lifecycle-service.ts +++ b/src/api/services/profile-lifecycle-service.ts @@ -9,7 +9,7 @@ import * as path from 'path'; import type { Config, Settings } from '../../types'; import type { TargetType } from '../../targets/target-adapter'; import { getCcsDir, getConfigPath, loadConfigSafe } from '../../utils/config-manager'; -import { ensureProfileHooks } from '../../utils/websearch/profile-hook-injector'; +import { ensureProfileHooksOrThrow } from '../../utils/websearch/profile-hook-injector'; import { isSensitiveKey } from '../../utils/sensitive-keys'; import { isReservedName } from '../../config/reserved-names'; import { isUnifiedMode, mutateUnifiedConfig } from '../../config/unified-config-loader'; @@ -216,8 +216,8 @@ export function registerApiProfileOrphans(options?: { } try { + ensureProfileHooksOrThrow(orphan.name); registerApiProfileInConfig(orphan.name, options?.target || 'claude', options?.force || false); - ensureProfileHooks(orphan.name); result.registered.push(orphan.name); } catch (error) { result.skipped.push({ name: orphan.name, reason: (error as Error).message }); @@ -264,7 +264,12 @@ export function copyApiProfile( : null; writeJsonObjectAtomically(destinationSettingsPath, sourceSettings); - ensureProfileHooks(destination); + try { + ensureProfileHooksOrThrow(destination); + } catch (hookError) { + rollbackSettingsFile(destinationSettingsPath, previousDestinationContent, destinationExisted); + throw hookError; + } try { registerApiProfileInConfig( destination, @@ -384,7 +389,12 @@ export function importApiProfileBundle( const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; writeJsonObjectAtomically(settingsPath, settings); - ensureProfileHooks(name); + try { + ensureProfileHooksOrThrow(name); + } catch (hookError) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw hookError; + } try { registerApiProfileInConfig(name, options?.target || bundleTarget || 'claude', options?.force); } catch (registrationError) { diff --git a/src/api/services/profile-writer.ts b/src/api/services/profile-writer.ts index a0e230de..912dc5b3 100644 --- a/src/api/services/profile-writer.ts +++ b/src/api/services/profile-writer.ts @@ -8,7 +8,7 @@ import { getCcsDir, getConfigPath, loadConfigSafe } from '../../utils/config-man import { expandPath } from '../../utils/helpers'; import { validateApiName } from './validation-service'; import { mutateUnifiedConfig, isUnifiedMode } from '../../config/unified-config-loader'; -import { ensureProfileHooks } from '../../utils/websearch/profile-hook-injector'; +import { ensureProfileHooksOrThrow } from '../../utils/websearch/profile-hook-injector'; import type { TargetType } from '../../targets/target-adapter'; import { resolveDroidProvider } from '../../targets/droid-provider'; import { isReservedName } from '../../config/reserved-names'; @@ -69,6 +69,21 @@ function getDeniedModelReason(baseUrl: string, models: ModelMapping): string | n return null; } +function rollbackSettingsFile( + filePath: string, + previousContent: string | null, + existedBefore: boolean +): void { + if (existedBefore && previousContent !== null) { + fs.writeFileSync(filePath, previousContent, 'utf8'); + return; + } + + if (fs.existsSync(filePath)) { + fs.unlinkSync(filePath); + } +} + /** Create settings.json file for API profile (legacy format) */ function createSettingsFile( name: string, @@ -105,11 +120,18 @@ function createSettingsFile( }, }; + const settingsExisted = fs.existsSync(settingsPath); + const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; fs.mkdirSync(ccsDir, { recursive: true }); fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2) + '\n', 'utf8'); - // Inject WebSearch hooks into profile settings - ensureProfileHooks(name); + try { + // Inject WebSearch hooks into profile settings + ensureProfileHooksOrThrow(name); + } catch (error) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw error; + } return settingsPath; } @@ -189,10 +211,17 @@ function createApiProfileUnified( fs.mkdirSync(ccsDir, { recursive: true }); } + const settingsExisted = fs.existsSync(settingsPath); + const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2) + '\n', 'utf8'); - // Inject WebSearch hooks into profile settings - ensureProfileHooks(name); + try { + // Inject WebSearch hooks into profile settings + ensureProfileHooksOrThrow(name); + } catch (error) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw error; + } mutateUnifiedConfig((config) => { config.profiles[name] = { diff --git a/src/ccs.ts b/src/ccs.ts index 4d273c2b..395f2748 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -27,7 +27,7 @@ import { ensureMcpWebSearch, displayWebSearchStatus, getWebSearchHookEnv, - ensureProfileHooks, + ensureProfileHooksOrThrow, } from './utils/websearch-manager'; import { getGlobalEnvConfig, getOfficialChannelsConfig } from './config/unified-config-loader'; import { ensureProfileHooks as ensureImageAnalyzerHooks } from './utils/hooks/image-analyzer-profile-hook-injector'; @@ -506,7 +506,7 @@ async function main(): Promise { if (profileInfo.type === 'cliproxy') { // CLIPROXY FLOW: OAuth-based profiles (gemini, codex, agy, qwen) or user-defined variants // Inject WebSearch hook into profile settings before launch - ensureProfileHooks(profileInfo.name); + ensureProfileHooksOrThrow(profileInfo.name); // Inject Image Analyzer hook into profile settings before launch ensureImageAnalyzerHooks(profileInfo.name); @@ -660,7 +660,7 @@ async function main(): Promise { } else if (profileInfo.type === 'copilot') { // COPILOT FLOW: GitHub Copilot subscription via copilot-api proxy // Inject WebSearch hook into profile settings before launch - ensureProfileHooks(profileInfo.name); + ensureProfileHooksOrThrow(profileInfo.name); // Inject Image Analyzer hook into profile settings before launch ensureImageAnalyzerHooks(profileInfo.name); @@ -693,7 +693,7 @@ async function main(): Promise { // Settings-based profiles (glm, glmt) are third-party providers // WebSearch is server-side tool - third-party providers have no access // Inject WebSearch hook into profile settings before launch - ensureProfileHooks(profileInfo.name); + ensureProfileHooksOrThrow(profileInfo.name); // Inject Image Analyzer hook into profile settings before launch ensureImageAnalyzerHooks(profileInfo.name); diff --git a/src/cliproxy/services/variant-settings.ts b/src/cliproxy/services/variant-settings.ts index 7c7d2038..e55bb866 100644 --- a/src/cliproxy/services/variant-settings.ts +++ b/src/cliproxy/services/variant-settings.ts @@ -14,7 +14,7 @@ import { expandPath } from '../../utils/helpers'; import { getClaudeEnvVars, CLIPROXY_DEFAULT_PORT } from '../config-generator'; import { CLIProxyProvider } from '../types'; import { CompositeTierConfig } from '../../config/unified-config-types'; -import { ensureProfileHooks } from '../../utils/websearch/profile-hook-injector'; +import { ensureProfileHooksOrThrow } from '../../utils/websearch/profile-hook-injector'; import { ensureProfileHooks as ensureImageAnalyzerHooks } from '../../utils/hooks/image-analyzer-profile-hook-injector'; import { getEffectiveApiKey } from '../auth-token-manager'; import { warn } from '../../utils/ui'; @@ -95,6 +95,21 @@ function writeSettings(filePath: string, settings: SettingsFile): void { fs.renameSync(tempPath, filePath); } +function rollbackSettingsFile( + filePath: string, + previousContent: string | null, + existedBefore: boolean +): void { + if (existedBefore && previousContent !== null) { + fs.writeFileSync(filePath, previousContent, 'utf8'); + return; + } + + if (fs.existsSync(filePath)) { + fs.unlinkSync(filePath); + } +} + /** * Get settings file path for a variant */ @@ -133,11 +148,18 @@ export function createSettingsFile( env: buildSettingsEnv(provider, model, port), }; + const settingsExisted = fs.existsSync(settingsPath); + const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; ensureDir(ccsDir); writeSettings(settingsPath, settings); - // Inject WebSearch hooks into variant settings - ensureProfileHooks(`${provider}-${name}`); + try { + // Inject WebSearch hooks into variant settings + ensureProfileHooksOrThrow(`${provider}-${name}`); + } catch (error) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw error; + } // Inject Image Analyzer hooks into variant settings ensureImageAnalyzerHooks(`${provider}-${name}`); @@ -161,11 +183,18 @@ export function createSettingsFileUnified( env: buildSettingsEnv(provider, model, port), }; + const settingsExisted = fs.existsSync(settingsPath); + const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; ensureDir(ccsDir); writeSettings(settingsPath, settings); - // Inject WebSearch hooks into variant settings - ensureProfileHooks(`${provider}-${name}`); + try { + // Inject WebSearch hooks into variant settings + ensureProfileHooksOrThrow(`${provider}-${name}`); + } catch (error) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw error; + } // Inject Image Analyzer hooks into variant settings ensureImageAnalyzerHooks(`${provider}-${name}`); @@ -252,12 +281,19 @@ export function createCompositeSettingsFile( } } + const settingsExisted = fs.existsSync(settingsPath); + const previousSettingsContent = settingsExisted ? fs.readFileSync(settingsPath, 'utf8') : null; ensureDir(settingsDir); writeSettings(settingsPath, settings); // Hook injectors target ~/.ccs/.settings.json; only run for default path. if (path.resolve(settingsPath) === path.resolve(defaultSettingsPath)) { - ensureProfileHooks(`composite-${name}`); + try { + ensureProfileHooksOrThrow(`composite-${name}`); + } catch (error) { + rollbackSettingsFile(settingsPath, previousSettingsContent, settingsExisted); + throw error; + } ensureImageAnalyzerHooks(`composite-${name}`); } diff --git a/src/utils/websearch-manager.ts b/src/utils/websearch-manager.ts index 861b8f8c..9c4a6dc7 100644 --- a/src/utils/websearch-manager.ts +++ b/src/utils/websearch-manager.ts @@ -63,7 +63,7 @@ export { } from './websearch/status'; // Re-export profile hook injection -export { ensureProfileHooks } from './websearch/profile-hook-injector'; +export { ensureProfileHooks, ensureProfileHooksOrThrow } from './websearch/profile-hook-injector'; // Import for local use import { clearGeminiCliCache, clearGrokCliCache, clearOpenCodeCliCache } from './websearch'; diff --git a/src/utils/websearch/hook-installer.ts b/src/utils/websearch/hook-installer.ts index 445ce132..51eff272 100644 --- a/src/utils/websearch/hook-installer.ts +++ b/src/utils/websearch/hook-installer.ts @@ -19,6 +19,21 @@ export { getHookPath, getWebSearchHookConfig } from './hook-config'; // Hook file name const WEBSEARCH_HOOK = 'websearch-transformer.cjs'; +function hasMatchingHookContents(sourcePath: string, destinationPath: string): boolean { + if (!fs.existsSync(destinationPath)) { + return false; + } + + const source = fs.readFileSync(sourcePath); + const destination = fs.readFileSync(destinationPath); + return source.equals(destination); +} + +function getTempHookPath(hookPath: string): string { + const uniqueSuffix = `${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + return `${hookPath}.${uniqueSuffix}.tmp`; +} + export function getMigrationMarkerPath(): string { return path.join(getCcsDir(), '.hook-migrated'); } @@ -94,9 +109,35 @@ export function installWebSearchHook(): boolean { return false; } - // Copy hook to ~/.ccs/hooks/ - fs.copyFileSync(sourcePath, hookPath); - fs.chmodSync(hookPath, 0o755); + // Avoid rewriting the shared hook binary when the bundled script is unchanged. + if (hasMatchingHookContents(sourcePath, hookPath)) { + return true; + } + + // Copy hook to ~/.ccs/hooks/ via a unique temp path so concurrent installers + // do not contend on the same file. + const tempHookPath = getTempHookPath(hookPath); + try { + fs.copyFileSync(sourcePath, tempHookPath); + fs.chmodSync(tempHookPath, 0o755); + + try { + fs.renameSync(tempHookPath, hookPath); + } catch (renameError) { + const errorCode = (renameError as NodeJS.ErrnoException).code; + if (errorCode !== 'EEXIST' && errorCode !== 'EPERM') { + throw renameError; + } + + fs.copyFileSync(tempHookPath, hookPath); + fs.chmodSync(hookPath, 0o755); + fs.unlinkSync(tempHookPath); + } + } finally { + if (fs.existsSync(tempHookPath)) { + fs.unlinkSync(tempHookPath); + } + } if (process.env.CCS_DEBUG) { console.error(info(`Installed WebSearch hook: ${hookPath}`)); diff --git a/src/utils/websearch/index.ts b/src/utils/websearch/index.ts index 84aeea99..83c240de 100644 --- a/src/utils/websearch/index.ts +++ b/src/utils/websearch/index.ts @@ -62,4 +62,4 @@ export { export { WEBSEARCH_API_KEY_PROVIDERS, getWebSearchApiKeyStates } from './provider-secrets'; // Profile Hook Injection -export { ensureProfileHooks } from './profile-hook-injector'; +export { ensureProfileHooks, ensureProfileHooksOrThrow } from './profile-hook-injector'; diff --git a/src/utils/websearch/profile-hook-injector.ts b/src/utils/websearch/profile-hook-injector.ts index 0437d071..cb98412d 100644 --- a/src/utils/websearch/profile-hook-injector.ts +++ b/src/utils/websearch/profile-hook-injector.ts @@ -20,6 +20,21 @@ import { getMigrationMarkerPath, installWebSearchHook } from './hook-installer'; // Valid profile name pattern (alphanumeric, dash, underscore only) const VALID_PROFILE_NAME = /^[a-zA-Z0-9_-]+$/; +function hasUsableHookBinary(): boolean { + try { + const hookPath = getHookPath(); + const stat = fs.statSync(hookPath); + if (!stat.isFile()) { + return false; + } + + fs.accessSync(hookPath, fs.constants.R_OK); + return true; + } catch { + return false; + } +} + /** * Check if CCS WebSearch hook exists in settings */ @@ -83,25 +98,8 @@ export function ensureProfileHooks(profileName: string): boolean { return false; } - // Keep the injected command target valid for all profile types, not just CLIProxy. - if (!installWebSearchHook() && !fs.existsSync(getHookPath())) { - if (process.env.CCS_DEBUG) { - console.error(warn('WebSearch hook binary is missing and could not be installed')); - } - return false; - } - - // One-time migration from global settings - migrateGlobalHook(); - // Get CCS directory (respects CCS_HOME for test isolation) const ccsDir = getCcsDir(); - - // Ensure CCS dir exists - if (!fs.existsSync(ccsDir)) { - fs.mkdirSync(ccsDir, { recursive: true, mode: 0o700 }); - } - const settingsPath = path.join(ccsDir, `${profileName}.settings.json`); // Read existing settings or create empty @@ -121,6 +119,25 @@ export function ensureProfileHooks(profileName: string): boolean { } } + // Keep the injected command target valid for all profile types, not just CLIProxy. + // The installer already skips byte-identical copies, so we can always attempt a refresh + // without rewriting unchanged hooks. Re-check existence after a failed install to tolerate + // concurrent first-run installs that may have completed in another process. + if (!installWebSearchHook() && !hasUsableHookBinary()) { + if (process.env.CCS_DEBUG) { + console.error(warn('WebSearch hook binary is missing and could not be installed')); + } + return false; + } + + // One-time migration from global settings + migrateGlobalHook(); + + // Ensure CCS dir exists before writing settings updates. + if (!fs.existsSync(ccsDir)) { + fs.mkdirSync(ccsDir, { recursive: true, mode: 0o700 }); + } + // Check if CCS hook already present if (hasCcsHook(settings)) { // Clean up any duplicates that may have accumulated (Windows path bug fix) @@ -176,6 +193,19 @@ export function ensureProfileHooks(profileName: string): boolean { } } +export function ensureProfileHooksOrThrow(profileName: string): void { + const wsConfig = getWebSearchConfig(); + if (!wsConfig.enabled) { + return; + } + + if (!ensureProfileHooks(profileName)) { + throw new Error( + `WebSearch is enabled, but CCS could not prepare the profile hook for "${profileName}".` + ); + } +} + /** * Update hook timeout if it differs from current config */ diff --git a/tests/unit/api/profile-lifecycle-service.test.ts b/tests/unit/api/profile-lifecycle-service.test.ts index 31dd1a83..3ed07c6e 100644 --- a/tests/unit/api/profile-lifecycle-service.test.ts +++ b/tests/unit/api/profile-lifecycle-service.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -9,7 +9,11 @@ import { importApiProfileBundle, registerApiProfileOrphans, } from '../../../src/api/services/profile-lifecycle-service'; -import { runWithScopedConfigDir, setGlobalConfigDir } from '../../../src/utils/config-manager'; +import { + loadConfigSafe, + runWithScopedConfigDir, + setGlobalConfigDir, +} from '../../../src/utils/config-manager'; describe('profile lifecycle service', () => { let tempHome = ''; @@ -37,6 +41,8 @@ describe('profile lifecycle service', () => { }); afterEach(() => { + mock.restore(); + if (originalCcsHome === undefined) { delete process.env.CCS_HOME; } else { @@ -122,6 +128,60 @@ describe('profile lifecycle service', () => { expect(result.skipped).toEqual([]); }); + it('does not register orphan profiles when WebSearch hook setup fails', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'extra.settings.json'), + JSON.stringify( + { env: { ANTHROPIC_BASE_URL: 'https://api.example.com', ANTHROPIC_AUTH_TOKEN: 'token' } }, + null, + 2 + ) + '\n' + ); + fs.writeFileSync(path.join(ccsDir, 'config.json'), JSON.stringify({ profiles: {} }, null, 2) + '\n'); + + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy failed'); + }); + + const result = await runInScopedCcsDir(() => registerApiProfileOrphans({ names: ['extra'] })); + const config = await runInScopedCcsDir(() => loadConfigSafe()); + + expect(copyFileSpy).toHaveBeenCalled(); + expect(result.registered).toEqual([]); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]?.reason).toContain('could not prepare the profile hook'); + expect(config.profiles.extra).toBeUndefined(); + }); + + it('keeps orphan registration non-fatal when WebSearch is disabled', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + fs.writeFileSync( + path.join(ccsDir, 'extra.settings.json'), + JSON.stringify( + { env: { ANTHROPIC_BASE_URL: 'https://api.example.com', ANTHROPIC_AUTH_TOKEN: 'token' } }, + null, + 2 + ) + '\n' + ); + fs.writeFileSync(path.join(ccsDir, 'config.json'), JSON.stringify({ profiles: {} }, null, 2) + '\n'); + fs.writeFileSync(path.join(ccsDir, 'config.yaml'), 'version: 12\nwebsearch:\n enabled: false\n', 'utf8'); + + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy should not run when WebSearch is disabled'); + }); + + const result = await runInScopedCcsDir(() => registerApiProfileOrphans({ names: ['extra'] })); + + expect(copyFileSpy).not.toHaveBeenCalled(); + expect(result.registered).toEqual(['extra']); + expect(result.skipped).toEqual([]); + }); + it('redacts all sensitive env values during export when includeSecrets=false', async () => { const ccsDir = path.join(tempHome, '.ccs'); fs.mkdirSync(ccsDir, { recursive: true }); @@ -160,6 +220,34 @@ describe('profile lifecycle service', () => { expect(result.error).toContain('Invalid source profile name'); }); + it('rolls back copied settings when WebSearch hook setup fails', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'config.json'), + JSON.stringify({ profiles: { source: '~/.ccs/source.settings.json' } }, null, 2) + '\n' + ); + fs.writeFileSync( + path.join(ccsDir, 'source.settings.json'), + JSON.stringify( + { env: { ANTHROPIC_BASE_URL: 'https://api.example.com', ANTHROPIC_AUTH_TOKEN: 'token' } }, + null, + 2 + ) + '\n' + ); + + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy failed'); + }); + + const result = await runInScopedCcsDir(() => copyApiProfile('source', 'copy-dest')); + + expect(result.success).toBe(false); + expect(result.error).toContain('could not prepare the profile hook'); + expect(copyFileSpy).toHaveBeenCalled(); + expect(fs.existsSync(path.join(ccsDir, 'copy-dest.settings.json'))).toBe(false); + }); + it('rejects import bundle with invalid profile target', async () => { const result = await runInScopedCcsDir(() => importApiProfileBundle({ @@ -179,6 +267,38 @@ describe('profile lifecycle service', () => { expect(result.error).toContain('Invalid bundle profile target'); }); + it('rolls back imported settings when WebSearch hook setup fails', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'config.json'), + JSON.stringify({ profiles: {} }, null, 2) + '\n' + ); + + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy failed'); + }); + + const result = await runInScopedCcsDir(() => + importApiProfileBundle({ + schemaVersion: 1, + exportedAt: new Date().toISOString(), + profile: { name: 'import-failure', target: 'claude' }, + settings: { + env: { + ANTHROPIC_BASE_URL: 'https://api.example.com', + ANTHROPIC_AUTH_TOKEN: 'token', + }, + }, + }) + ); + + expect(result.success).toBe(false); + expect(result.error).toContain('could not prepare the profile hook'); + expect(copyFileSpy).toHaveBeenCalled(); + expect(fs.existsSync(path.join(ccsDir, 'import-failure.settings.json'))).toBe(false); + }); + it('clears and warns for all redacted sensitive env keys on import', async () => { const ccsDir = path.join(tempHome, '.ccs'); fs.mkdirSync(ccsDir, { recursive: true }); diff --git a/tests/unit/api/profile-writer-anthropic.test.ts b/tests/unit/api/profile-writer-anthropic.test.ts index c354eb76..fe1d8973 100644 --- a/tests/unit/api/profile-writer-anthropic.test.ts +++ b/tests/unit/api/profile-writer-anthropic.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -15,6 +15,8 @@ describe('profile-writer Anthropic direct', () => { }); afterEach(() => { + mock.restore(); + if (originalCcsHome === undefined) { delete process.env.CCS_HOME; } else { @@ -101,4 +103,48 @@ describe('profile-writer Anthropic direct', () => { expect(settings.env.ANTHROPIC_AUTH_TOKEN).toBe('sk-or-testkey'); expect(settings.env.ANTHROPIC_API_KEY).toBe(''); }); + + it('rolls back the created settings file when WebSearch hook installation fails', () => { + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy failed'); + }); + + const result = createApiProfile( + 'hook-failure', + 'https://api.z.ai/api/anthropic', + 'ghp_testkey123', + { default: 'glm-5', opus: 'glm-5', sonnet: 'glm-5', haiku: 'glm-5' } + ); + + expect(result.success).toBe(false); + expect(result.error).toContain('could not prepare the profile hook'); + expect(copyFileSpy).toHaveBeenCalled(); + expect(fs.existsSync(path.join(tempHome, '.ccs', 'hook-failure.settings.json'))).toBe(false); + }); + + it('keeps profile creation non-fatal when WebSearch is disabled', () => { + fs.mkdirSync(path.join(tempHome, '.ccs'), { recursive: true }); + fs.writeFileSync( + path.join(tempHome, '.ccs', 'config.yaml'), + 'version: 12\nwebsearch:\n enabled: false\n', + 'utf8' + ); + + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { + throw new Error('copy should not run when WebSearch is disabled'); + }); + + const result = createApiProfile( + 'disabled-websearch', + 'https://api.z.ai/api/anthropic', + 'ghp_testkey123', + { default: 'glm-5', opus: 'glm-5', sonnet: 'glm-5', haiku: 'glm-5' } + ); + + expect(result.success).toBe(true); + expect(copyFileSpy).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(tempHome, '.ccs', 'disabled-websearch.settings.json'))).toBe( + true + ); + }); }); diff --git a/tests/unit/targets/settings-profile-websearch-launch.test.ts b/tests/unit/targets/settings-profile-websearch-launch.test.ts new file mode 100644 index 00000000..3753a218 --- /dev/null +++ b/tests/unit/targets/settings-profile-websearch-launch.test.ts @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +interface RunResult { + status: number | null; + stdout: string; + stderr: string; +} + +function runCcs(args: string[], env: NodeJS.ProcessEnv): RunResult { + const ccsEntry = path.join(process.cwd(), 'src', 'ccs.ts'); + const result = spawnSync(process.execPath, [ccsEntry, ...args], { + encoding: 'utf8', + env, + timeout: 20000, + }); + + return { + status: result.status, + stdout: result.stdout || '', + stderr: result.stderr || '', + }; +} + +describe('settings profile WebSearch launch', () => { + let tmpHome = ''; + let ccsDir = ''; + let settingsPath = ''; + let fakeClaudePath = ''; + let claudeArgsLogPath = ''; + let baseEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + if (process.platform === 'win32') { + return; + } + + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-websearch-launch-')); + ccsDir = path.join(tmpHome, '.ccs'); + settingsPath = path.join(ccsDir, 'glm.settings.json'); + fakeClaudePath = path.join(tmpHome, 'fake-claude.sh'); + claudeArgsLogPath = path.join(tmpHome, 'claude-args.txt'); + + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'config.json'), + JSON.stringify({ profiles: { glm: settingsPath } }, null, 2) + '\n' + ); + fs.writeFileSync( + settingsPath, + JSON.stringify( + { + env: { + ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic', + ANTHROPIC_AUTH_TOKEN: 'token', + ANTHROPIC_MODEL: 'glm-5', + }, + }, + null, + 2 + ) + '\n' + ); + + fs.writeFileSync( + fakeClaudePath, + `#!/bin/sh +printf "%s\n" "$@" > "${claudeArgsLogPath}" +exit 0 +`, + { encoding: 'utf8', mode: 0o755 } + ); + fs.chmodSync(fakeClaudePath, 0o755); + + baseEnv = { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_CLAUDE_PATH: fakeClaudePath, + CCS_DEBUG: '1', + }; + }); + + afterEach(() => { + if (process.platform === 'win32') { + return; + } + + fs.rmSync(tmpHome, { recursive: true, force: true }); + }); + + it('fails before Claude launch when an enabled WebSearch hook cannot be prepared', () => { + if (process.platform === 'win32') return; + + fs.writeFileSync(path.join(ccsDir, 'hooks'), 'not-a-directory', 'utf8'); + + const result = runCcs(['glm', 'smoke'], baseEnv); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('could not prepare the profile hook for "glm"'); + expect(fs.existsSync(claudeArgsLogPath)).toBe(false); + }); + + it('keeps launch non-fatal when WebSearch is disabled', () => { + if (process.platform === 'win32') return; + + fs.writeFileSync( + path.join(ccsDir, 'config.yaml'), + 'version: 12\nwebsearch:\n enabled: false\n', + 'utf8' + ); + fs.writeFileSync(path.join(ccsDir, 'hooks'), 'not-a-directory', 'utf8'); + + const result = runCcs(['glm', 'smoke'], baseEnv); + + expect(result.status).toBe(0); + expect(result.stderr).not.toContain('could not prepare the profile hook for "glm"'); + expect(fs.existsSync(claudeArgsLogPath)).toBe(true); + }); +}); diff --git a/tests/unit/utils/websearch/profile-hook-injector.test.ts b/tests/unit/utils/websearch/profile-hook-injector.test.ts index 60f3a2b8..2ab1faa1 100644 --- a/tests/unit/utils/websearch/profile-hook-injector.test.ts +++ b/tests/unit/utils/websearch/profile-hook-injector.test.ts @@ -27,6 +27,10 @@ describe('ensureProfileHooks', () => { return path.join(tempHome, '.ccs'); } + function getBundledHookContents(): string { + return fs.readFileSync(path.join(process.cwd(), 'lib', 'hooks', 'websearch-transformer.cjs'), 'utf8'); + } + afterEach(() => { mock.restore(); @@ -69,7 +73,7 @@ describe('ensureProfileHooks', () => { const hookPath = getHookPath(); fs.mkdirSync(path.dirname(hookPath), { recursive: true }); - fs.writeFileSync(hookPath, '// existing hook', 'utf8'); + fs.writeFileSync(hookPath, getBundledHookContents(), 'utf8'); const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => { throw new Error('copy skipped'); @@ -80,10 +84,74 @@ describe('ensureProfileHooks', () => { const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); expect(ensured).toBe(true); - expect(copyFileSpy).toHaveBeenCalled(); + expect(copyFileSpy).not.toHaveBeenCalled(); expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe(`node "${hookPath}"`); }); + it('does not rewrite the shared hook binary when it is already installed', () => { + setupTempHome(); + + expect(ensureProfileHooks('glm')).toBe(true); + + const firstMtime = fs.statSync(getHookPath()).mtimeMs; + const waitUntil = Date.now() + 25; + while (Date.now() < waitUntil) { + // Give the filesystem timestamp a chance to advance if a rewrite occurs. + } + + expect(ensureProfileHooks('glm')).toBe(true); + const secondMtime = fs.statSync(getHookPath()).mtimeMs; + + expect(secondMtime).toBe(firstMtime); + }); + + it('refreshes a stale shared hook binary when the bundled script has changed', () => { + setupTempHome(); + + const hookPath = getHookPath(); + fs.mkdirSync(path.dirname(hookPath), { recursive: true }); + fs.writeFileSync(hookPath, '// stale hook', 'utf8'); + + expect(ensureProfileHooks('glm')).toBe(true); + const installedHook = fs.readFileSync(hookPath, 'utf8'); + + expect(installedHook).not.toBe('// stale hook'); + expect(installedHook).toContain('CCS WebSearch Hook'); + }); + + it('succeeds when another process installs the hook during a failed local install', () => { + setupTempHome(); + + const hookPath = getHookPath(); + const originalCopyFileSync = fs.copyFileSync; + const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation((source, destination) => { + originalCopyFileSync(source, hookPath); + throw new Error(`simulated concurrent winner while copying to ${String(destination)}`); + }); + + const ensured = ensureProfileHooks('glm'); + const settingsPath = path.join(getCcsDir(), 'glm.settings.json'); + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + + expect(ensured).toBe(true); + expect(copyFileSpy).toHaveBeenCalled(); + expect(fs.existsSync(hookPath)).toBe(true); + expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe(`node "${hookPath}"`); + }); + + it('returns false when the hook path exists but is unusable', () => { + setupTempHome(); + + const hookPath = getHookPath(); + fs.mkdirSync(hookPath, { recursive: true }); + + const ensured = ensureProfileHooks('glm'); + + expect(ensured).toBe(false); + expect(fs.statSync(hookPath).isDirectory()).toBe(true); + expect(fs.existsSync(path.join(getCcsDir(), 'glm.settings.json'))).toBe(false); + }); + it('returns false for invalid profile names without creating files', () => { setupTempHome();