From 639c9aaca37542ff1ebdcbff2e7893122ba6a5fb Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Thu, 12 Feb 2026 15:57:20 -0800 Subject: [PATCH] fix: Windows path support, error handling, and dedup in validators - session-manager.js: fix getSessionStats path detection to handle Windows paths (C:\...) in addition to Unix paths (/) - package-manager.js: add try-catch to setPreferredPackageManager for consistent error handling with setProjectPackageManager - validate-hooks.js: extract duplicated hook entry validation into reusable validateHookEntry() helper - Update .d.ts JSDoc for both fixes --- scripts/ci/validate-hooks.js | 71 ++++++++++++++++---------------- scripts/lib/package-manager.d.ts | 2 +- scripts/lib/package-manager.js | 7 +++- scripts/lib/session-manager.d.ts | 3 +- scripts/lib/session-manager.js | 12 +++--- 5 files changed, 51 insertions(+), 44 deletions(-) diff --git a/scripts/ci/validate-hooks.js b/scripts/ci/validate-hooks.js index f91f9407..dc3f6f42 100644 --- a/scripts/ci/validate-hooks.js +++ b/scripts/ci/validate-hooks.js @@ -10,6 +10,39 @@ const vm = require('vm'); const HOOKS_FILE = path.join(__dirname, '../../hooks/hooks.json'); const VALID_EVENTS = ['PreToolUse', 'PostToolUse', 'PreCompact', 'SessionStart', 'SessionEnd', 'Stop', 'Notification', 'SubagentStop']; +/** + * Validate a single hook entry has required fields and valid inline JS + * @param {object} hook - Hook object with type and command fields + * @param {string} label - Label for error messages (e.g., "PreToolUse[0].hooks[1]") + * @returns {boolean} true if errors were found + */ +function validateHookEntry(hook, label) { + let hasErrors = false; + + if (!hook.type || typeof hook.type !== 'string') { + console.error(`ERROR: ${label} missing or invalid 'type' field`); + hasErrors = true; + } + + if (!hook.command || (typeof hook.command !== 'string' && !Array.isArray(hook.command))) { + console.error(`ERROR: ${label} missing or invalid 'command' field`); + hasErrors = true; + } else if (typeof hook.command === 'string') { + // Validate inline JS syntax in node -e commands + const nodeEMatch = hook.command.match(/^node -e "(.*)"$/s); + if (nodeEMatch) { + try { + new vm.Script(nodeEMatch[1].replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\t/g, '\t').replace(/\\\\/g, '\\')); + } catch (syntaxErr) { + console.error(`ERROR: ${label} has invalid inline JS: ${syntaxErr.message}`); + hasErrors = true; + } + } + } + + return hasErrors; +} + function validateHooks() { if (!fs.existsSync(HOOKS_FILE)) { console.log('No hooks.json found, skipping validation'); @@ -61,26 +94,9 @@ function validateHooks() { } else { // Validate each hook entry for (let j = 0; j < matcher.hooks.length; j++) { - const hook = matcher.hooks[j]; - if (!hook.type || typeof hook.type !== 'string') { - console.error(`ERROR: ${eventType}[${i}].hooks[${j}] missing or invalid 'type' field`); + if (validateHookEntry(matcher.hooks[j], `${eventType}[${i}].hooks[${j}]`)) { hasErrors = true; } - if (!hook.command || (typeof hook.command !== 'string' && !Array.isArray(hook.command))) { - console.error(`ERROR: ${eventType}[${i}].hooks[${j}] missing or invalid 'command' field`); - hasErrors = true; - } else if (typeof hook.command === 'string') { - // Validate inline JS syntax in node -e commands - const nodeEMatch = hook.command.match(/^node -e "(.*)"$/s); - if (nodeEMatch) { - try { - new vm.Script(nodeEMatch[1].replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\t/g, '\t').replace(/\\\\/g, '\\')); - } catch (syntaxErr) { - console.error(`ERROR: ${eventType}[${i}].hooks[${j}] has invalid inline JS: ${syntaxErr.message}`); - hasErrors = true; - } - } - } } } totalMatchers++; @@ -100,26 +116,9 @@ function validateHooks() { } else { // Validate each hook entry for (let j = 0; j < hook.hooks.length; j++) { - const h = hook.hooks[j]; - if (!h.type || typeof h.type !== 'string') { - console.error(`ERROR: Hook ${i}.hooks[${j}] missing or invalid 'type' field`); + if (validateHookEntry(hook.hooks[j], `Hook ${i}.hooks[${j}]`)) { hasErrors = true; } - if (!h.command || (typeof h.command !== 'string' && !Array.isArray(h.command))) { - console.error(`ERROR: Hook ${i}.hooks[${j}] missing or invalid 'command' field`); - hasErrors = true; - } else if (typeof h.command === 'string') { - // Validate inline JS syntax in node -e commands - const nodeEMatch = h.command.match(/^node -e "(.*)"$/s); - if (nodeEMatch) { - try { - new vm.Script(nodeEMatch[1].replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\t/g, '\t').replace(/\\\\/g, '\\')); - } catch (syntaxErr) { - console.error(`ERROR: Hook ${i}.hooks[${j}] has invalid inline JS: ${syntaxErr.message}`); - hasErrors = true; - } - } - } } } totalMatchers++; diff --git a/scripts/lib/package-manager.d.ts b/scripts/lib/package-manager.d.ts index ecfa78a6..90a9573f 100644 --- a/scripts/lib/package-manager.d.ts +++ b/scripts/lib/package-manager.d.ts @@ -68,7 +68,7 @@ export function getPackageManager(options?: GetPackageManagerOptions): PackageMa /** * Set the user's globally preferred package manager. * Saves to ~/.claude/package-manager.json. - * @throws If pmName is not a known package manager + * @throws If pmName is not a known package manager or if save fails */ export function setPreferredPackageManager(pmName: PackageManagerName): { packageManager: string; setAt: string }; diff --git a/scripts/lib/package-manager.js b/scripts/lib/package-manager.js index de4d1e43..f767282d 100644 --- a/scripts/lib/package-manager.js +++ b/scripts/lib/package-manager.js @@ -246,7 +246,12 @@ function setPreferredPackageManager(pmName) { const config = loadConfig() || {}; config.packageManager = pmName; config.setAt = new Date().toISOString(); - saveConfig(config); + + try { + saveConfig(config); + } catch (err) { + throw new Error(`Failed to save package manager preference: ${err.message}`); + } return config; } diff --git a/scripts/lib/session-manager.d.ts b/scripts/lib/session-manager.d.ts index 31e903b1..7fbbc695 100644 --- a/scripts/lib/session-manager.d.ts +++ b/scripts/lib/session-manager.d.ts @@ -97,7 +97,8 @@ export function parseSessionMetadata(content: string | null): SessionMetadata; /** * Calculate statistics for a session. - * Accepts either a file path (ending in .tmp) or pre-read content string. + * Accepts either a file path (absolute, ending in .tmp) or pre-read content string. + * Supports both Unix (/path/to/session.tmp) and Windows (C:\path\to\session.tmp) paths. */ export function getSessionStats(sessionPathOrContent: string): SessionStats; diff --git a/scripts/lib/session-manager.js b/scripts/lib/session-manager.js index e449030f..c3331b50 100644 --- a/scripts/lib/session-manager.js +++ b/scripts/lib/session-manager.js @@ -146,12 +146,14 @@ function parseSessionMetadata(content) { */ function getSessionStats(sessionPathOrContent) { // Accept pre-read content string to avoid redundant file reads. - // If the argument looks like a file path (no newlines, ends with .tmp), - // read from disk. Otherwise treat it as content. - const content = (typeof sessionPathOrContent === 'string' && + // If the argument looks like a file path (no newlines, ends with .tmp, + // starts with / on Unix or drive letter on Windows), read from disk. + // Otherwise treat it as content. + const looksLikePath = typeof sessionPathOrContent === 'string' && !sessionPathOrContent.includes('\n') && - sessionPathOrContent.startsWith('/') && - sessionPathOrContent.endsWith('.tmp')) + sessionPathOrContent.endsWith('.tmp') && + (sessionPathOrContent.startsWith('/') || /^[A-Za-z]:[/\\]/.test(sessionPathOrContent)); + const content = looksLikePath ? getSessionContent(sessionPathOrContent) : sessionPathOrContent;