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
This commit is contained in:
Affaan Mustafa
2026-02-12 15:57:20 -08:00
parent 76b271ab6b
commit 639c9aaca3
5 changed files with 51 additions and 44 deletions

View File

@@ -10,6 +10,39 @@ const vm = require('vm');
const HOOKS_FILE = path.join(__dirname, '../../hooks/hooks.json'); const HOOKS_FILE = path.join(__dirname, '../../hooks/hooks.json');
const VALID_EVENTS = ['PreToolUse', 'PostToolUse', 'PreCompact', 'SessionStart', 'SessionEnd', 'Stop', 'Notification', 'SubagentStop']; 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() { function validateHooks() {
if (!fs.existsSync(HOOKS_FILE)) { if (!fs.existsSync(HOOKS_FILE)) {
console.log('No hooks.json found, skipping validation'); console.log('No hooks.json found, skipping validation');
@@ -61,26 +94,9 @@ function validateHooks() {
} else { } else {
// Validate each hook entry // Validate each hook entry
for (let j = 0; j < matcher.hooks.length; j++) { for (let j = 0; j < matcher.hooks.length; j++) {
const hook = matcher.hooks[j]; if (validateHookEntry(matcher.hooks[j], `${eventType}[${i}].hooks[${j}]`)) {
if (!hook.type || typeof hook.type !== 'string') {
console.error(`ERROR: ${eventType}[${i}].hooks[${j}] missing or invalid 'type' field`);
hasErrors = true; 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++; totalMatchers++;
@@ -100,26 +116,9 @@ function validateHooks() {
} else { } else {
// Validate each hook entry // Validate each hook entry
for (let j = 0; j < hook.hooks.length; j++) { for (let j = 0; j < hook.hooks.length; j++) {
const h = hook.hooks[j]; if (validateHookEntry(hook.hooks[j], `Hook ${i}.hooks[${j}]`)) {
if (!h.type || typeof h.type !== 'string') {
console.error(`ERROR: Hook ${i}.hooks[${j}] missing or invalid 'type' field`);
hasErrors = true; 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++; totalMatchers++;

View File

@@ -68,7 +68,7 @@ export function getPackageManager(options?: GetPackageManagerOptions): PackageMa
/** /**
* Set the user's globally preferred package manager. * Set the user's globally preferred package manager.
* Saves to ~/.claude/package-manager.json. * 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 }; export function setPreferredPackageManager(pmName: PackageManagerName): { packageManager: string; setAt: string };

View File

@@ -246,7 +246,12 @@ function setPreferredPackageManager(pmName) {
const config = loadConfig() || {}; const config = loadConfig() || {};
config.packageManager = pmName; config.packageManager = pmName;
config.setAt = new Date().toISOString(); 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; return config;
} }

View File

@@ -97,7 +97,8 @@ export function parseSessionMetadata(content: string | null): SessionMetadata;
/** /**
* Calculate statistics for a session. * 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; export function getSessionStats(sessionPathOrContent: string): SessionStats;

View File

@@ -146,12 +146,14 @@ function parseSessionMetadata(content) {
*/ */
function getSessionStats(sessionPathOrContent) { function getSessionStats(sessionPathOrContent) {
// Accept pre-read content string to avoid redundant file reads. // Accept pre-read content string to avoid redundant file reads.
// If the argument looks like a file path (no newlines, ends with .tmp), // If the argument looks like a file path (no newlines, ends with .tmp,
// read from disk. Otherwise treat it as content. // starts with / on Unix or drive letter on Windows), read from disk.
const content = (typeof sessionPathOrContent === 'string' && // Otherwise treat it as content.
const looksLikePath = typeof sessionPathOrContent === 'string' &&
!sessionPathOrContent.includes('\n') && !sessionPathOrContent.includes('\n') &&
sessionPathOrContent.startsWith('/') && sessionPathOrContent.endsWith('.tmp') &&
sessionPathOrContent.endsWith('.tmp')) (sessionPathOrContent.startsWith('/') || /^[A-Za-z]:[/\\]/.test(sessionPathOrContent));
const content = looksLikePath
? getSessionContent(sessionPathOrContent) ? getSessionContent(sessionPathOrContent)
: sessionPathOrContent; : sessionPathOrContent;