mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-17 06:17:09 +00:00
fix(websearch): avoid partial settings-hook migration
- verify the hook binary before migrating global WebSearch hook state - centralize migration-marker helpers in hook-installer exports - expand regression coverage for existing-hook, disabled, invalid-name, and failed-install paths
This commit is contained in:
@@ -46,6 +46,7 @@ export {
|
||||
hasWebSearchHook,
|
||||
getWebSearchHookConfig,
|
||||
installWebSearchHook,
|
||||
removeMigrationMarker,
|
||||
uninstallWebSearchHook,
|
||||
} from './websearch/hook-installer';
|
||||
|
||||
@@ -62,7 +63,7 @@ export {
|
||||
} from './websearch/status';
|
||||
|
||||
// Re-export profile hook injection
|
||||
export { ensureProfileHooks, removeMigrationMarker } from './websearch/profile-hook-injector';
|
||||
export { ensureProfileHooks } from './websearch/profile-hook-injector';
|
||||
|
||||
// Import for local use
|
||||
import { clearGeminiCliCache, clearGrokCliCache, clearOpenCodeCliCache } from './websearch';
|
||||
|
||||
@@ -19,7 +19,7 @@ export { getHookPath, getWebSearchHookConfig } from './hook-config';
|
||||
// Hook file name
|
||||
const WEBSEARCH_HOOK = 'websearch-transformer.cjs';
|
||||
|
||||
function getMigrationMarkerPath(): string {
|
||||
export function getMigrationMarkerPath(): string {
|
||||
return path.join(getCcsDir(), '.hook-migrated');
|
||||
}
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@ export {
|
||||
hasWebSearchHook,
|
||||
getWebSearchHookConfig,
|
||||
installWebSearchHook,
|
||||
removeMigrationMarker,
|
||||
uninstallWebSearchHook,
|
||||
} from './hook-installer';
|
||||
|
||||
@@ -61,4 +62,4 @@ export {
|
||||
export { WEBSEARCH_API_KEY_PROVIDERS, getWebSearchApiKeyStates } from './provider-secrets';
|
||||
|
||||
// Profile Hook Injection
|
||||
export { ensureProfileHooks, removeMigrationMarker } from './profile-hook-injector';
|
||||
export { ensureProfileHooks } from './profile-hook-injector';
|
||||
|
||||
@@ -15,18 +15,11 @@ import { getWebSearchConfig } from '../../config/unified-config-loader';
|
||||
import { removeHookConfig } from './hook-config';
|
||||
import { getCcsDir } from '../config-manager';
|
||||
import { isCcsWebSearchHook, deduplicateCcsHooks } from './hook-utils';
|
||||
import { installWebSearchHook } from './hook-installer';
|
||||
import { getMigrationMarkerPath, installWebSearchHook } from './hook-installer';
|
||||
|
||||
// Valid profile name pattern (alphanumeric, dash, underscore only)
|
||||
const VALID_PROFILE_NAME = /^[a-zA-Z0-9_-]+$/;
|
||||
|
||||
/**
|
||||
* Get migration marker path (respects CCS_HOME for test isolation)
|
||||
*/
|
||||
function getMigrationMarkerPath(): string {
|
||||
return path.join(getCcsDir(), '.hook-migrated');
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if CCS WebSearch hook exists in settings
|
||||
*/
|
||||
@@ -90,6 +83,14 @@ 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();
|
||||
|
||||
@@ -101,14 +102,6 @@ export function ensureProfileHooks(profileName: string): boolean {
|
||||
fs.mkdirSync(ccsDir, { recursive: true, mode: 0o700 });
|
||||
}
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
const settingsPath = path.join(ccsDir, `${profileName}.settings.json`);
|
||||
|
||||
// Read existing settings or create empty
|
||||
@@ -243,19 +236,3 @@ function updateHookTimeoutIfNeeded(
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove migration marker (called during uninstall)
|
||||
*/
|
||||
export function removeMigrationMarker(): void {
|
||||
try {
|
||||
const markerPath = getMigrationMarkerPath();
|
||||
if (fs.existsSync(markerPath)) {
|
||||
fs.unlinkSync(markerPath);
|
||||
}
|
||||
} catch (error) {
|
||||
if (process.env.CCS_DEBUG) {
|
||||
console.error(warn(`removeMigrationMarker failed: ${(error as Error).message}`));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,33 +1,58 @@
|
||||
import { afterEach, describe, expect, it } from 'bun:test';
|
||||
import { afterEach, describe, expect, it, mock, spyOn } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { ensureProfileHooks } from '../../../../src/utils/websearch/profile-hook-injector';
|
||||
import { getHookPath } from '../../../../src/utils/websearch/hook-config';
|
||||
import { getMigrationMarkerPath } from '../../../../src/utils/websearch/hook-installer';
|
||||
|
||||
describe('ensureProfileHooks', () => {
|
||||
let tempHome: string | undefined;
|
||||
let originalCcsHome: string | undefined;
|
||||
let originalClaudeConfigDir: string | undefined;
|
||||
|
||||
function setupTempHome(): string {
|
||||
tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-profile-hook-test-'));
|
||||
originalCcsHome = process.env.CCS_HOME;
|
||||
originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR;
|
||||
process.env.CCS_HOME = tempHome;
|
||||
delete process.env.CLAUDE_CONFIG_DIR;
|
||||
return tempHome;
|
||||
}
|
||||
|
||||
function getCcsDir(): string {
|
||||
if (!tempHome) {
|
||||
throw new Error('tempHome not initialized');
|
||||
}
|
||||
return path.join(tempHome, '.ccs');
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
mock.restore();
|
||||
|
||||
if (originalCcsHome !== undefined) {
|
||||
process.env.CCS_HOME = originalCcsHome;
|
||||
} else {
|
||||
delete process.env.CCS_HOME;
|
||||
}
|
||||
|
||||
if (originalClaudeConfigDir !== undefined) {
|
||||
process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir;
|
||||
} else {
|
||||
delete process.env.CLAUDE_CONFIG_DIR;
|
||||
}
|
||||
|
||||
if (tempHome && fs.existsSync(tempHome)) {
|
||||
fs.rmSync(tempHome, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
tempHome = undefined;
|
||||
originalCcsHome = undefined;
|
||||
originalClaudeConfigDir = undefined;
|
||||
});
|
||||
|
||||
it('installs the hook binary before writing the profile hook command', () => {
|
||||
tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-profile-hook-test-'));
|
||||
originalCcsHome = process.env.CCS_HOME;
|
||||
process.env.CCS_HOME = tempHome;
|
||||
setupTempHome();
|
||||
|
||||
const ensured = ensureProfileHooks('glm');
|
||||
const hookPath = getHookPath();
|
||||
@@ -38,4 +63,88 @@ describe('ensureProfileHooks', () => {
|
||||
expect(fs.existsSync(hookPath)).toBe(true);
|
||||
expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe(`node "${hookPath}"`);
|
||||
});
|
||||
|
||||
it('succeeds when the hook already exists on disk and installation is effectively a no-op', () => {
|
||||
setupTempHome();
|
||||
|
||||
const hookPath = getHookPath();
|
||||
fs.mkdirSync(path.dirname(hookPath), { recursive: true });
|
||||
fs.writeFileSync(hookPath, '// existing hook', 'utf8');
|
||||
|
||||
const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => {
|
||||
throw new Error('copy skipped');
|
||||
});
|
||||
|
||||
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(settings.hooks.PreToolUse[0].hooks[0].command).toBe(`node "${hookPath}"`);
|
||||
});
|
||||
|
||||
it('returns false for invalid profile names without creating files', () => {
|
||||
setupTempHome();
|
||||
|
||||
const ensured = ensureProfileHooks('../glm');
|
||||
|
||||
expect(ensured).toBe(false);
|
||||
expect(fs.existsSync(getCcsDir())).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when WebSearch is disabled without creating files', () => {
|
||||
setupTempHome();
|
||||
|
||||
fs.mkdirSync(getCcsDir(), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(getCcsDir(), 'config.yaml'),
|
||||
'version: 12\nwebsearch:\n enabled: false\n',
|
||||
'utf8'
|
||||
);
|
||||
|
||||
const ensured = ensureProfileHooks('glm');
|
||||
|
||||
expect(ensured).toBe(false);
|
||||
expect(fs.existsSync(getHookPath())).toBe(false);
|
||||
expect(fs.existsSync(path.join(getCcsDir(), 'glm.settings.json'))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when hook installation fails and no hook exists on disk', () => {
|
||||
setupTempHome();
|
||||
|
||||
const claudeSettingsPath = path.join(tempHome, '.claude', 'settings.json');
|
||||
fs.mkdirSync(path.dirname(claudeSettingsPath), { recursive: true });
|
||||
const globalSettings = {
|
||||
hooks: {
|
||||
PreToolUse: [
|
||||
{
|
||||
matcher: 'WebSearch',
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: `node "${getHookPath()}"`,
|
||||
timeout: 90,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
fs.writeFileSync(claudeSettingsPath, JSON.stringify(globalSettings, null, 2), 'utf8');
|
||||
|
||||
const copyFileSpy = spyOn(fs, 'copyFileSync').mockImplementation(() => {
|
||||
throw new Error('copy failed');
|
||||
});
|
||||
|
||||
const ensured = ensureProfileHooks('glm');
|
||||
const persistedGlobalSettings = JSON.parse(fs.readFileSync(claudeSettingsPath, 'utf8'));
|
||||
|
||||
expect(ensured).toBe(false);
|
||||
expect(copyFileSpy).toHaveBeenCalled();
|
||||
expect(fs.existsSync(getHookPath())).toBe(false);
|
||||
expect(fs.existsSync(getMigrationMarkerPath())).toBe(false);
|
||||
expect(fs.existsSync(path.join(getCcsDir(), 'glm.settings.json'))).toBe(false);
|
||||
expect(persistedGlobalSettings).toEqual(globalSettings);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user