From 21b18d0c4e7dbdf9e7070458ec5c5fb54ac6a410 Mon Sep 17 00:00:00 2001 From: kaitranntt Date: Sun, 25 Jan 2026 21:13:55 -0500 Subject: [PATCH] refactor(websearch): address PR review recommendations - Add comment in postuninstall.js explaining intentional os.homedir() - Fix comment in install-command.ts (doesn't touch global settings.json) - Consolidate duplicate getCcsHooksDir() - export from hook-config.ts - Add debug logging to silent catch blocks in profile-hook-injector.ts --- scripts/postuninstall.js | 2 ++ src/commands/install-command.ts | 2 +- src/utils/websearch/hook-config.ts | 2 +- src/utils/websearch/hook-installer.ts | 10 +--------- src/utils/websearch/profile-hook-injector.ts | 11 ++++++++--- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/scripts/postuninstall.js b/scripts/postuninstall.js index 4cd31d00..5058e4ae 100644 --- a/scripts/postuninstall.js +++ b/scripts/postuninstall.js @@ -12,6 +12,8 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +// Note: Uses os.homedir() directly because this script runs during npm uninstall, +// not in test context. CCS_HOME isolation is for src/ code only. const CCS_DIR = path.join(os.homedir(), '.ccs'); const HOOKS_DIR = path.join(CCS_DIR, 'hooks'); const MIGRATION_MARKER = path.join(CCS_DIR, '.hook-migrated'); diff --git a/src/commands/install-command.ts b/src/commands/install-command.ts index 8842dc0a..9959c735 100644 --- a/src/commands/install-command.ts +++ b/src/commands/install-command.ts @@ -35,7 +35,7 @@ export async function handleUninstallCommand(): Promise { let removed = 0; - // 1. Remove WebSearch hook (file + settings.json) + // 1. Remove WebSearch hook file + migration marker (does NOT touch global settings.json) const hookRemoved = uninstallWebSearchHook(); if (hookRemoved) { console.log(ok('Removed WebSearch hook')); diff --git a/src/utils/websearch/hook-config.ts b/src/utils/websearch/hook-config.ts index 00cc4391..72ca82e5 100644 --- a/src/utils/websearch/hook-config.ts +++ b/src/utils/websearch/hook-config.ts @@ -22,7 +22,7 @@ const WEBSEARCH_HOOK = 'websearch-transformer.cjs'; /** * Get CCS hooks directory (respects CCS_HOME for test isolation) */ -function getCcsHooksDir(): string { +export function getCcsHooksDir(): string { return path.join(getCcsDir(), 'hooks'); } diff --git a/src/utils/websearch/hook-installer.ts b/src/utils/websearch/hook-installer.ts index c54a3d61..21da5a5f 100644 --- a/src/utils/websearch/hook-installer.ts +++ b/src/utils/websearch/hook-installer.ts @@ -10,9 +10,8 @@ import * as fs from 'fs'; import * as path from 'path'; import { info, warn } from '../ui'; import { getWebSearchConfig } from '../../config/unified-config-loader'; -import { getHookPath, ensureHookConfig } from './hook-config'; +import { getHookPath, ensureHookConfig, getCcsHooksDir } from './hook-config'; import { removeMigrationMarker } from './profile-hook-injector'; -import { getCcsDir } from '../config-manager'; // Re-export from hook-config for backward compatibility export { getHookPath, getWebSearchHookConfig } from './hook-config'; @@ -20,13 +19,6 @@ export { getHookPath, getWebSearchHookConfig } from './hook-config'; // Hook file name const WEBSEARCH_HOOK = 'websearch-transformer.cjs'; -/** - * Get CCS hooks directory (respects CCS_HOME for test isolation) - */ -function getCcsHooksDir(): string { - return path.join(getCcsDir(), 'hooks'); -} - /** * Check if WebSearch hook is installed */ diff --git a/src/utils/websearch/profile-hook-injector.ts b/src/utils/websearch/profile-hook-injector.ts index 2f13e6aa..5bdc7ea5 100644 --- a/src/utils/websearch/profile-hook-injector.ts +++ b/src/utils/websearch/profile-hook-injector.ts @@ -209,7 +209,10 @@ function updateHookTimeoutIfNeeded( } return true; - } catch { + } catch (error) { + if (process.env.CCS_DEBUG) { + console.error(warn(`updateHookTimeoutIfNeeded failed: ${(error as Error).message}`)); + } return false; } } @@ -223,7 +226,9 @@ export function removeMigrationMarker(): void { if (fs.existsSync(markerPath)) { fs.unlinkSync(markerPath); } - } catch { - // Ignore errors + } catch (error) { + if (process.env.CCS_DEBUG) { + console.error(warn(`removeMigrationMarker failed: ${(error as Error).message}`)); + } } }