fix: improve error handling, fix bugs, and optimize core libraries

utils.js:
- Fix countInFile: enforce global flag on regex to prevent silent
  under-counting (match() without /g returns only first match)
- Add 5s timeout to readStdinJson to prevent hooks hanging forever
- Handle EEXIST race condition in ensureDir
- Pre-compile regex patterns in getGitModifiedFiles to avoid N*M
  compilations and catch invalid patterns before filtering
- Add JSDoc documentation to all improved functions

session-manager.js:
- Fix getSessionById triple file read: pass pre-read content to
  getSessionStats instead of re-reading from disk
- Allow getSessionStats to accept content string directly

session-aliases.js:
- Wrap temp file cleanup in try/catch to prevent cascading errors

check-console-log.js:
- Refactor to use shared utils (isGitRepo, getGitModifiedFiles, log)
  instead of raw execSync calls
- Add exclusion patterns for test files, config files, and scripts/
  where console.log is intentional

session-end.js:
- Log count of skipped unparseable transcript lines for diagnostics

suggest-compact.js:
- Guard against NaN from corrupted counter files

package-manager.js:
- Remove dead fallbackOrder parameter (unused after #162 fix)
This commit is contained in:
Affaan Mustafa
2026-02-12 07:06:53 -08:00
parent b2285e870a
commit 18c5a76a96
7 changed files with 134 additions and 54 deletions

View File

@@ -3,56 +3,60 @@
/** /**
* Stop Hook: Check for console.log statements in modified files * Stop Hook: Check for console.log statements in modified files
* *
* This hook runs after each response and checks if any modified * Cross-platform (Windows, macOS, Linux)
* JavaScript/TypeScript files contain console.log statements. *
* It provides warnings to help developers remember to remove * Runs after each response and checks if any modified JavaScript/TypeScript
* debug statements before committing. * files contain console.log statements. Provides warnings to help developers
* remember to remove debug statements before committing.
*
* Exclusions: test files, config files, and scripts/ directory (where
* console.log is often intentional).
*/ */
const { execSync } = require('child_process');
const fs = require('fs'); const fs = require('fs');
const { isGitRepo, getGitModifiedFiles, log } = require('../lib/utils');
// Files where console.log is expected and should not trigger warnings
const EXCLUDED_PATTERNS = [
/\.test\.[jt]sx?$/,
/\.spec\.[jt]sx?$/,
/\.config\.[jt]s$/,
/scripts\//,
/__tests__\//,
/__mocks__\//,
];
let data = ''; let data = '';
// Read stdin
process.stdin.on('data', chunk => { process.stdin.on('data', chunk => {
data += chunk; data += chunk;
}); });
process.stdin.on('end', () => { process.stdin.on('end', () => {
try { try {
// Check if we're in a git repository if (!isGitRepo()) {
try {
execSync('git rev-parse --git-dir', { stdio: 'pipe' });
} catch {
// Not in a git repo, just pass through the data
console.log(data); console.log(data);
process.exit(0); process.exit(0);
} }
// Get list of modified files const files = getGitModifiedFiles(['\\.tsx?$', '\\.jsx?$'])
const files = execSync('git diff --name-only HEAD', { .filter(f => fs.existsSync(f))
encoding: 'utf8', .filter(f => !EXCLUDED_PATTERNS.some(pattern => pattern.test(f)));
stdio: ['pipe', 'pipe', 'pipe']
})
.split('\n')
.filter(f => /\.(ts|tsx|js|jsx)$/.test(f) && fs.existsSync(f));
let hasConsole = false; let hasConsole = false;
// Check each file for console.log
for (const file of files) { for (const file of files) {
const content = fs.readFileSync(file, 'utf8'); const content = fs.readFileSync(file, 'utf8');
if (content.includes('console.log')) { if (content.includes('console.log')) {
console.error(`[Hook] WARNING: console.log found in ${file}`); log(`[Hook] WARNING: console.log found in ${file}`);
hasConsole = true; hasConsole = true;
} }
} }
if (hasConsole) { if (hasConsole) {
console.error('[Hook] Remove console.log statements before committing'); log('[Hook] Remove console.log statements before committing');
} }
} catch (_error) { } catch {
// Silently ignore errors (git might not be available, etc.) // Silently ignore errors (git might not be available, etc.)
} }

View File

@@ -38,6 +38,7 @@ function extractSessionSummary(transcriptPath) {
const userMessages = []; const userMessages = [];
const toolsUsed = new Set(); const toolsUsed = new Set();
const filesModified = new Set(); const filesModified = new Set();
let parseErrors = 0;
for (const line of lines) { for (const line of lines) {
try { try {
@@ -66,10 +67,14 @@ function extractSessionSummary(transcriptPath) {
} }
} }
} catch { } catch {
// Skip unparseable lines parseErrors++;
} }
} }
if (parseErrors > 0) {
log(`[SessionEnd] Skipped ${parseErrors}/${lines.length} unparseable transcript lines`);
}
if (userMessages.length === 0) return null; if (userMessages.length === 0) return null;
return { return {

View File

@@ -23,9 +23,9 @@ const {
async function main() { async function main() {
// Track tool call count (increment in a temp file) // Track tool call count (increment in a temp file)
// Use a session-specific counter file based on PID from parent process // Use a session-specific counter file based on session ID from environment
// or session ID from environment // or parent PID as fallback
const sessionId = process.env.CLAUDE_SESSION_ID || process.ppid || 'default'; const sessionId = process.env.CLAUDE_SESSION_ID || String(process.ppid) || 'default';
const counterFile = path.join(getTempDir(), `claude-tool-count-${sessionId}`); const counterFile = path.join(getTempDir(), `claude-tool-count-${sessionId}`);
const threshold = parseInt(process.env.COMPACT_THRESHOLD || '50', 10); const threshold = parseInt(process.env.COMPACT_THRESHOLD || '50', 10);
@@ -34,7 +34,9 @@ async function main() {
// Read existing count or start at 1 // Read existing count or start at 1
const existing = readFile(counterFile); const existing = readFile(counterFile);
if (existing) { if (existing) {
count = parseInt(existing.trim(), 10) + 1; const parsed = parseInt(existing.trim(), 10);
// Guard against NaN from corrupted counter file
count = Number.isFinite(parsed) ? parsed + 1 : 1;
} }
// Save updated count // Save updated count

View File

@@ -156,11 +156,12 @@ function getAvailablePackageManagers() {
* 5. Global user preference (in ~/.claude/package-manager.json) * 5. Global user preference (in ~/.claude/package-manager.json)
* 6. Default to npm (no child processes spawned) * 6. Default to npm (no child processes spawned)
* *
* @param {object} options - { projectDir, fallbackOrder } * @param {object} options - Options
* @param {string} options.projectDir - Project directory to detect from (default: cwd)
* @returns {object} - { name, config, source } * @returns {object} - { name, config, source }
*/ */
function getPackageManager(options = {}) { function getPackageManager(options = {}) {
const { projectDir = process.cwd(), fallbackOrder = DETECTION_PRIORITY } = options; const { projectDir = process.cwd() } = options;
// 1. Check environment variable // 1. Check environment variable
const envPm = process.env.CLAUDE_PACKAGE_MANAGER; const envPm = process.env.CLAUDE_PACKAGE_MANAGER;

View File

@@ -135,9 +135,13 @@ function saveAliases(aliases) {
} }
} }
// Clean up temp file // Clean up temp file (best-effort)
if (fs.existsSync(tempPath)) { try {
fs.unlinkSync(tempPath); if (fs.existsSync(tempPath)) {
fs.unlinkSync(tempPath);
}
} catch {
// Non-critical: temp file will be overwritten on next save
} }
return false; return false;

View File

@@ -143,11 +143,21 @@ function parseSessionMetadata(content) {
/** /**
* Calculate statistics for a session * Calculate statistics for a session
* @param {string} sessionPath - Full path to session file * @param {string} sessionPathOrContent - Full path to session file, OR
* the pre-read content string (to avoid redundant disk reads when
* the caller already has the content loaded).
* @returns {object} Statistics object * @returns {object} Statistics object
*/ */
function getSessionStats(sessionPath) { function getSessionStats(sessionPathOrContent) {
const content = getSessionContent(sessionPath); // 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' &&
!sessionPathOrContent.includes('\n') &&
sessionPathOrContent.endsWith('.tmp'))
? getSessionContent(sessionPathOrContent)
: sessionPathOrContent;
const metadata = parseSessionMetadata(content); const metadata = parseSessionMetadata(content);
return { return {
@@ -281,7 +291,8 @@ function getSessionById(sessionId, includeContent = false) {
if (includeContent) { if (includeContent) {
session.content = getSessionContent(sessionPath); session.content = getSessionContent(sessionPath);
session.metadata = parseSessionMetadata(session.content); session.metadata = parseSessionMetadata(session.content);
session.stats = getSessionStats(sessionPath); // Pass pre-read content to avoid a redundant disk read
session.stats = getSessionStats(session.content || '');
} }
return session; return session;

View File

@@ -57,10 +57,20 @@ function getTempDir() {
/** /**
* Ensure a directory exists (create if not) * Ensure a directory exists (create if not)
* @param {string} dirPath - Directory path to create
* @returns {string} The directory path
* @throws {Error} If directory cannot be created (e.g., permission denied)
*/ */
function ensureDir(dirPath) { function ensureDir(dirPath) {
if (!fs.existsSync(dirPath)) { try {
fs.mkdirSync(dirPath, { recursive: true }); if (!fs.existsSync(dirPath)) {
fs.mkdirSync(dirPath, { recursive: true });
}
} catch (err) {
// EEXIST is fine (race condition with another process creating it)
if (err.code !== 'EEXIST') {
throw err;
}
} }
return dirPath; return dirPath;
} }
@@ -187,10 +197,29 @@ function findFiles(dir, pattern, options = {}) {
/** /**
* Read JSON from stdin (for hook input) * Read JSON from stdin (for hook input)
* @param {object} options - Options
* @param {number} options.timeoutMs - Timeout in milliseconds (default: 5000).
* Prevents hooks from hanging indefinitely if stdin never closes.
* @returns {Promise<object>} Parsed JSON object, or empty object if stdin is empty
*/ */
async function readStdinJson() { async function readStdinJson(options = {}) {
const { timeoutMs = 5000 } = options;
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
let data = ''; let data = '';
let settled = false;
const timer = setTimeout(() => {
if (!settled) {
settled = true;
// Resolve with whatever we have so far rather than hanging
try {
resolve(data.trim() ? JSON.parse(data) : {});
} catch {
resolve({});
}
}
}, timeoutMs);
process.stdin.setEncoding('utf8'); process.stdin.setEncoding('utf8');
process.stdin.on('data', chunk => { process.stdin.on('data', chunk => {
@@ -198,18 +227,22 @@ async function readStdinJson() {
}); });
process.stdin.on('end', () => { process.stdin.on('end', () => {
if (settled) return;
settled = true;
clearTimeout(timer);
try { try {
if (data.trim()) { resolve(data.trim() ? JSON.parse(data) : {});
resolve(JSON.parse(data));
} else {
resolve({});
}
} catch (err) { } catch (err) {
reject(err); reject(err);
} }
}); });
process.stdin.on('error', reject); process.stdin.on('error', err => {
if (settled) return;
settled = true;
clearTimeout(timer);
reject(err);
});
}); });
} }
@@ -313,7 +346,10 @@ function isGitRepo() {
} }
/** /**
* Get git modified files * Get git modified files, optionally filtered by regex patterns
* @param {string[]} patterns - Array of regex pattern strings to filter files.
* Invalid patterns are silently skipped.
* @returns {string[]} Array of modified file paths
*/ */
function getGitModifiedFiles(patterns = []) { function getGitModifiedFiles(patterns = []) {
if (!isGitRepo()) return []; if (!isGitRepo()) return [];
@@ -324,12 +360,18 @@ function getGitModifiedFiles(patterns = []) {
let files = result.output.split('\n').filter(Boolean); let files = result.output.split('\n').filter(Boolean);
if (patterns.length > 0) { if (patterns.length > 0) {
files = files.filter(file => { // Pre-compile patterns, skipping invalid ones
return patterns.some(pattern => { const compiled = [];
const regex = new RegExp(pattern); for (const pattern of patterns) {
return regex.test(file); try {
}); compiled.push(new RegExp(pattern));
}); } catch {
// Skip invalid regex patterns
}
}
if (compiled.length > 0) {
files = files.filter(file => compiled.some(regex => regex.test(file)));
}
} }
return files; return files;
@@ -349,12 +391,23 @@ function replaceInFile(filePath, search, replace) {
/** /**
* Count occurrences of a pattern in a file * Count occurrences of a pattern in a file
* @param {string} filePath - Path to the file
* @param {string|RegExp} pattern - Pattern to count. Strings are treated as
* global regex patterns. RegExp instances are used as-is but the global
* flag is enforced to ensure correct counting.
* @returns {number} Number of matches found
*/ */
function countInFile(filePath, pattern) { function countInFile(filePath, pattern) {
const content = readFile(filePath); const content = readFile(filePath);
if (content === null) return 0; if (content === null) return 0;
const regex = pattern instanceof RegExp ? pattern : new RegExp(pattern, 'g'); let regex;
if (pattern instanceof RegExp) {
// Ensure global flag is set for correct counting
regex = pattern.global ? pattern : new RegExp(pattern.source, pattern.flags + 'g');
} else {
regex = new RegExp(pattern, 'g');
}
const matches = content.match(regex); const matches = content.match(regex);
return matches ? matches.length : 0; return matches ? matches.length : 0;
} }