Code quality: comprehensive nonsense audit cleanup (20 issues) (#400)

* fix: prevent initialization promise from resolving on failure

Background initialization was resolving the promise even when it failed,
causing the readiness check to incorrectly indicate the worker was ready.
Now the promise stays pending on failure, ensuring /api/readiness
continues returning 503 until initialization succeeds.

Fixes critical issue #1 from nonsense audit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: improve error handling in context inject and settings update routes

* Enhance error handling for ChromaDB failures in SearchManager

- Introduced a flag to track ChromaDB failure states.
- Updated logging messages to provide clearer feedback on ChromaDB initialization and failure.
- Modified the response structure to inform users when semantic search is unavailable due to ChromaDB issues, including installation instructions for UVX/Python.

* refactor: remove deprecated silent-debug utility functions

* Enhance error handling and validation in hooks

- Added validation for required fields in `summary-hook.ts` and `save-hook.ts` to ensure necessary inputs are provided before processing.
- Improved error messages for missing `cwd` in `save-hook.ts` and `transcript_path` in `summary-hook.ts`.
- Cleaned up code by removing unnecessary error handling logic and directly throwing errors when required fields are missing.
- Updated binary file `mem-search.zip` to reflect changes in the plugin.

* fix: improve error handling in summary hook to ensure errors are not masked

* fix: add error handling for unknown message content format in transcript parser

* fix: log error when failing to notify worker of session end

* Refactor date formatting functions: move to shared module

- Removed redundant date formatting functions from SearchManager.ts.
- Consolidated date formatting logic into shared timeline-formatting.ts.
- Updated functions to accept both ISO date strings and epoch milliseconds.

* Refactor tag stripping functions to extract shared logic

- Introduced a new internal function `stripTagsInternal` to handle the common logic for stripping memory tags from both JSON and prompt content.
- Updated `stripMemoryTagsFromJson` to utilize the new internal function, simplifying its implementation.
- Modified `stripMemoryTagsFromPrompt` to also call `stripTagsInternal`, reducing code duplication and improving maintainability.
- Removed redundant type checks and logging from both functions, as they now rely on the internal function for processing.

* Refactor settings validation in SettingsRoutes

- Consolidated multiple individual setting validations into a single validateSettings method.
- Updated handleUpdateSettings to use the new validation method for improved maintainability.
- Each setting now has its validation logic encapsulated within validateSettings, ensuring a single source of truth for validation rules.

* fix: add error logging to ProcessManager.getPidInfo()

Previously getPidInfo() returned null silently for three cases:
1. File not found (expected - no action needed)
2. JSON parse error (corrupted file - now logs warning)
3. Type validation failure (malformed data - now logs warning)

This fix adds warning logs for cases 2 and 3 to provide visibility
into PID file corruption issues. Logs include context like parsed
data structure or error message with file path.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: remove overly defensive try-catch in SessionRoutes

Remove unnecessary try-catch block that was masking potential errors when
checking file paths for session-memory meta-observations. Property access
on parsed JSON objects never throws - existing truthiness checks already
safely handle undefined/null values.

Issue #12 from nonsense audit: SessionRoutes catch-all exception masking

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: remove redundant try-catch from getWorkerPort()

Simplified getWorkerPort() by removing unnecessary try-catch wrapper.
SettingsDefaultsManager.loadFromFile() already handles missing files
by returning defaults, and .get() never throws - making the catch
block completely redundant.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* refactor: eliminate ceremonial wrapper in hook-response.ts

Replace buildHookResponse() function with direct constant export.
Most hook responses were calling a function just to return the same
constant object. Only SessionStart with context needs special handling.

Changes:
- Export STANDARD_HOOK_RESPONSE constant directly
- Simplify createHookResponse() to only handle SessionStart special case
- Update all hooks to use STANDARD_HOOK_RESPONSE instead of function call
- Eliminate buildHookResponse() function with redundant branching

Files modified:
- src/hooks/hook-response.ts: Export constant, simplify function
- src/hooks/new-hook.ts: Use STANDARD_HOOK_RESPONSE
- src/hooks/save-hook.ts: Use STANDARD_HOOK_RESPONSE
- src/hooks/summary-hook.ts: Use STANDARD_HOOK_RESPONSE

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: make getWorkerHost() consistent with getWorkerPort()

- Use SettingsDefaultsManager.get('CLAUDE_MEM_DATA_DIR') for path resolution
  instead of hardcoded ~/.claude-mem (supports custom data directories)
- Add caching to getWorkerHost() (same pattern as getWorkerPort())
- Update clearPortCache() to also clear host cache
- Both functions now have identical patterns: caching, consistent path
  resolution, and same error handling via SettingsDefaultsManager

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* refactor: inline single-use timeout constants in ProcessManager

Remove 6 timeout constants used only once each, inlining their values
directly at the point of use. Following YAGNI principle - constants
should only exist when used multiple times.

Removed constants:
- PROCESS_STOP_TIMEOUT_MS (5000ms)
- HEALTH_CHECK_TIMEOUT_MS (10000ms)
- HEALTH_CHECK_INTERVAL_MS (200ms)
- HEALTH_CHECK_FETCH_TIMEOUT_MS (1000ms)
- PROCESS_EXIT_CHECK_INTERVAL_MS (100ms)
- HTTP_SHUTDOWN_TIMEOUT_MS (2000ms)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: replace overly broad path filter in HTTP logging middleware

Replace `req.path.includes('.')` with explicit static file extension
checking to prevent incorrectly skipping API endpoint logging.

- Add `staticExtensions` array with legitimate asset types
- Use `.endsWith()` matching instead of `.includes()`
- API endpoints containing periods (if any) now logged correctly
- Static assets (.js, .css, .svg, etc.) still skip logging as intended

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* refactor: expand logger.formatTool() to handle all tool types

Replace hard-coded tool formatting for 4 tools with comprehensive coverage:

File operations (Read, Edit, Write, NotebookEdit):
- Consolidated file_path handling for all file operations
- Added notebook_path support for NotebookEdit
- Shows filename only (not full path)

Search tools (Glob, Grep):
- Glob: shows pattern
- Grep: shows pattern (truncated if > 30 chars)

Network tools (WebFetch, WebSearch):
- Shows URL or query (truncated if > 40 chars)

Meta tools (Task, Skill, LSP):
- Task: shows subagent_type or description
- Skill: shows skill name
- LSP: shows operation type

This eliminates the "hard-coded 4 tools" limitation and provides
meaningful log output for all tool types.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: remove all truncation from logger.formatTool()

Truncation hides critical debugging information. Show everything:

- Bash: full command (was truncated at 50 chars)
- File operations: full path (was showing filename only)
- Grep: full pattern (was truncated at 30 chars)
- WebFetch/WebSearch: full URL/query (was truncated at 40 chars)
- Task: full description (was truncated at 30 chars)

Logs exist to provide complete information. Never hide details.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* refactor: replace array indexing with regex capture for drive letter

Use explicit regex capture group to extract Windows drive letter instead
of assuming cwd[0] is always the first character. Safer and more explicit.

- Changed cwd.match(/^[A-Z]:\\/i) to cwd.match(/^([A-Z]):\\/i)
- Extract drive letter from driveMatch[1] instead of cwd[0]
- Restructured control flow to avoid nested conditionals

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: return computed values from DataRoutes processing endpoint

The handleSetProcessing endpoint was computing queueDepth and
activeSessions but not including them in the response. This commit
includes all computed values in the API response.

- Return queueDepth and activeSessions in /api/processing response
- Eliminates dead code pattern where values are computed but unused
- API callers can now access these metrics

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: move error handling into SettingsDefaultsManager.loadFromFile()

Wrap the entire loadFromFile() method in try-catch so it handles ALL
error cases (missing file, corrupted JSON, permission errors, I/O failures)
instead of forcing every caller to add redundant try-catch blocks.

This follows DRY principle: one function owns error handling, all callers
stay simple and clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* Refactor hook response handling and optimize token estimation

- Removed the HookType and HookResponse types and the createHookResponse function from hook-response.ts to simplify the response handling for hooks.
- Introduced a standardized hook response for all hooks in hook-response.ts.
- Moved the estimateTokens function from SearchManager.ts to timeline-formatting.ts for better reusability and clarity.
- Cleaned up redundant estimateTokens function definitions in SearchManager.ts.

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2025-12-20 19:41:33 -05:00
committed by GitHub
parent a8ae879398
commit a537433eae
32 changed files with 461 additions and 893 deletions
+45 -14
View File
@@ -101,27 +101,58 @@ class Logger {
try {
const input = typeof toolInput === 'string' ? JSON.parse(toolInput) : toolInput;
// Special formatting for common tools
// Bash: show full command
if (toolName === 'Bash' && input.command) {
const cmd = input.command.length > 50
? input.command.substring(0, 50) + '...'
: input.command;
return `${toolName}(${cmd})`;
return `${toolName}(${input.command})`;
}
if (toolName === 'Read' && input.file_path) {
const path = input.file_path.split('/').pop() || input.file_path;
return `${toolName}(${path})`;
// File operations: show full path
if (input.file_path) {
return `${toolName}(${input.file_path})`;
}
if (toolName === 'Edit' && input.file_path) {
const path = input.file_path.split('/').pop() || input.file_path;
return `${toolName}(${path})`;
// NotebookEdit: show full notebook path
if (input.notebook_path) {
return `${toolName}(${input.notebook_path})`;
}
if (toolName === 'Write' && input.file_path) {
const path = input.file_path.split('/').pop() || input.file_path;
return `${toolName}(${path})`;
// Glob: show full pattern
if (toolName === 'Glob' && input.pattern) {
return `${toolName}(${input.pattern})`;
}
// Grep: show full pattern
if (toolName === 'Grep' && input.pattern) {
return `${toolName}(${input.pattern})`;
}
// WebFetch/WebSearch: show full URL or query
if (input.url) {
return `${toolName}(${input.url})`;
}
if (input.query) {
return `${toolName}(${input.query})`;
}
// Task: show subagent_type or full description
if (toolName === 'Task') {
if (input.subagent_type) {
return `${toolName}(${input.subagent_type})`;
}
if (input.description) {
return `${toolName}(${input.description})`;
}
}
// Skill: show skill name
if (toolName === 'Skill' && input.skill) {
return `${toolName}(${input.skill})`;
}
// LSP: show operation type
if (toolName === 'LSP' && input.operation) {
return `${toolName}(${input.operation})`;
}
// Default: just show tool name
+10 -8
View File
@@ -22,15 +22,17 @@ export function getProjectName(cwd: string | null | undefined): string {
if (basename === '') {
// Extract drive letter on Windows, or use 'root' on Unix
const isWindows = process.platform === 'win32';
if (isWindows && cwd.match(/^[A-Z]:\\/i)) {
const driveLetter = cwd[0].toUpperCase();
const projectName = `drive-${driveLetter}`;
logger.info('PROJECT_NAME', 'Drive root detected', { cwd, projectName });
return projectName;
} else {
logger.warn('PROJECT_NAME', 'Root directory detected, using fallback', { cwd });
return 'unknown-project';
if (isWindows) {
const driveMatch = cwd.match(/^([A-Z]):\\/i);
if (driveMatch) {
const driveLetter = driveMatch[1].toUpperCase();
const projectName = `drive-${driveLetter}`;
logger.info('PROJECT_NAME', 'Drive root detected', { cwd, projectName });
return projectName;
}
}
logger.warn('PROJECT_NAME', 'Root directory detected, using fallback', { cwd });
return 'unknown-project';
}
return basename;
-77
View File
@@ -1,77 +0,0 @@
/**
* Happy Path Error With Fallback
*
* @deprecated This function is deprecated. Use logger.happyPathError() instead.
* All usages have been migrated to the new logger system which consolidates logs
* into the regular worker logs instead of separate silent.log files.
*
* Migration example:
* OLD: happy_path_error__with_fallback('Missing value', { data }, 'default')
* NEW: logger.happyPathError('COMPONENT', 'Missing value', undefined, { data }, 'default')
*
* See: src/utils/logger.ts for the new happyPathError method
* Issue: #312 - Consolidate silent logs into regular worker logs
*/
import { appendFileSync } from 'fs';
import { homedir } from 'os';
import { join } from 'path';
const LOG_FILE = join(homedir(), '.claude-mem', 'silent.log');
/**
* Write an error message to silent.log and return fallback value
* @param message - Error message describing what went wrong
* @param data - Optional data to include (will be JSON stringified)
* @param fallback - Value to return (defaults to empty string)
* @returns The fallback value (for use in || fallbacks)
*/
export function happy_path_error__with_fallback(message: string, data?: any, fallback: string = ''): string {
const timestamp = new Date().toISOString();
// Capture stack trace to get caller location
const stack = new Error().stack || '';
const stackLines = stack.split('\n');
// Line 0: "Error"
// Line 1: "at silentDebug ..."
// Line 2: "at <CALLER> ..." <- We want this one
const callerLine = stackLines[2] || '';
const callerMatch = callerLine.match(/at\s+(?:.*\s+)?\(?([^:]+):(\d+):(\d+)\)?/);
const location = callerMatch
? `${callerMatch[1].split('/').pop()}:${callerMatch[2]}`
: 'unknown';
let logLine = `[${timestamp}] [HAPPY-PATH-ERROR] [${location}] ${message}`;
if (data !== undefined) {
try {
logLine += ` ${JSON.stringify(data)}`;
} catch (error) {
logLine += ` [stringify error: ${error}]`;
}
}
logLine += '\n';
try {
appendFileSync(LOG_FILE, logLine);
} catch (error) {
// If we can't write to the log file, fail silently (it's a debug utility after all)
// Only write to stderr as a last resort
console.error('[silent-debug] Failed to write to log:', error);
}
return fallback;
}
/**
* Clear the silent log file
*/
export function clearSilentLog(): void {
try {
appendFileSync(LOG_FILE, `\n${'='.repeat(80)}\n[${new Date().toISOString()}] Log cleared\n${'='.repeat(80)}\n\n`);
} catch (error) {
// Expected: Log file may not be writable
}
}
+15 -37
View File
@@ -31,20 +31,10 @@ function countTags(content: string): number {
}
/**
* Strip memory tags from JSON-serialized content (tool inputs/responses)
*
* @param content - Stringified JSON content from tool_input or tool_response
* @returns Cleaned content with tags removed, or '{}' if non-string/invalid
*
* Note: Returns '{}' for non-strings because this is used in JSON context
* where we need a valid JSON object if the input is invalid.
* Internal function to strip memory tags from content
* Shared logic extracted from both JSON and prompt stripping functions
*/
export function stripMemoryTagsFromJson(content: string): string {
if (typeof content !== 'string') {
logger.happyPathError('SYSTEM', 'received non-string for JSON context', undefined, { type: typeof content }, '{}');
return '{}'; // Safe default for JSON context
}
function stripTagsInternal(content: string): string {
// ReDoS protection: limit tag count before regex processing
const tagCount = countTags(content);
if (tagCount > MAX_TAG_COUNT) {
@@ -62,34 +52,22 @@ export function stripMemoryTagsFromJson(content: string): string {
.trim();
}
/**
* Strip memory tags from JSON-serialized content (tool inputs/responses)
*
* @param content - Stringified JSON content from tool_input or tool_response
* @returns Cleaned content with tags removed, or '{}' if invalid
*/
export function stripMemoryTagsFromJson(content: string): string {
return stripTagsInternal(content);
}
/**
* Strip memory tags from user prompt content
*
* @param content - Raw user prompt text
* @returns Cleaned content with tags removed, or '' if non-string/invalid
*
* Note: Returns '' (empty string) for non-strings because this is used in prompt context
* where an empty prompt indicates the user didn't provide any content.
* @returns Cleaned content with tags removed
*/
export function stripMemoryTagsFromPrompt(content: string): string {
if (typeof content !== 'string') {
logger.happyPathError('SYSTEM', 'received non-string for prompt context', undefined, { type: typeof content }, '');
return ''; // Safe default for prompt content
}
// ReDoS protection: limit tag count before regex processing
const tagCount = countTags(content);
if (tagCount > MAX_TAG_COUNT) {
logger.warn('SYSTEM', 'tag count exceeds limit', undefined, {
tagCount,
maxAllowed: MAX_TAG_COUNT,
contentLength: content.length
});
// Still process but log the anomaly
}
return content
.replace(/<claude-mem-context>[\s\S]*?<\/claude-mem-context>/g, '')
.replace(/<private>[\s\S]*?<\/private>/g, '')
.trim();
return stripTagsInternal(content);
}