fix: address medium severity issues from code review

- Fix inconsistent null handling in state-store (use ?? instead of ||)
- Add JSON.stringify error handling with specific messages
- Add error handling for SSE init event in app.js
- Fix spawn orchestrator silent failures (add logging)
- Fix statSync race condition in subagent-watcher
- Fix unbounded _taskNumberToContent Map in ralph-tracker
- Fix YAML pattern case sensitivity in ralph-config

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
arkon
2026-01-26 13:58:34 +01:00
co-authored by Claude Opus 4.5
parent e81dc4665a
commit 3c2e89f427
9 changed files with 106 additions and 34 deletions
+1 -1
View File
@@ -21,7 +21,7 @@ When user says "COM": 1) Increment version in BOTH `package.json` AND `CLAUDE.md
Claudeman is a Claude Code session manager with a web interface and autonomous Ralph Loop. It spawns Claude CLI processes via PTY, streams output in real-time via SSE, and supports scheduled/timed runs. Claudeman is a Claude Code session manager with a web interface and autonomous Ralph Loop. It spawns Claude CLI processes via PTY, streams output in real-time via SSE, and supports scheduled/timed runs.
**Version**: 0.1382 (must match `package.json`) **Version**: 0.1383 (must match `package.json`)
**Tech Stack**: TypeScript (ES2022/NodeNext, strict mode), Node.js, Fastify, Server-Sent Events, node-pty **Tech Stack**: TypeScript (ES2022/NodeNext, strict mode), Node.js, Fastify, Server-Sent Events, node-pty
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "claudeman", "name": "claudeman",
"version": "0.1382", "version": "0.1383",
"description": "The missing control plane for Claude Code - run 20 autonomous agents with real-time monitoring and session persistence", "description": "The missing control plane for Claude Code - run 20 autonomous agents with real-time monitoring and session persistence",
"type": "module", "type": "module",
"main": "dist/index.js", "main": "dist/index.js",
+2 -2
View File
@@ -18,7 +18,7 @@ const CLAUDE_MD_PROMISE_PATTERN = /<promise>\s*([A-Z0-9_-]+)\s*<\/promise>/gi;
// Pattern to parse YAML frontmatter from ralph-loop.local.md // Pattern to parse YAML frontmatter from ralph-loop.local.md
// Extracts key: value pairs from between --- markers // Extracts key: value pairs from between --- markers
const YAML_FRONTMATTER_PATTERN = /^---\s*\n([\s\S]*?)\n---/; const YAML_FRONTMATTER_PATTERN = /^---\s*\n([\s\S]*?)\n---/;
const YAML_LINE_PATTERN = /^([a-z-]+):\s*"?([^"\n]+)"?\s*$/gm; const YAML_LINE_PATTERN = /^([a-zA-Z_-]+):\s*"?([^"\n]+)"?\s*$/gm;
/** /**
* Ralph Loop configuration from .claude/ralph-loop.local.md * Ralph Loop configuration from .claude/ralph-loop.local.md
@@ -91,7 +91,7 @@ export function parseRalphLoopConfigFromContent(content: string): RalphLoopConfi
switch (key) { switch (key) {
case 'enabled': case 'enabled':
config.enabled = value === 'true'; config.enabled = value.toLowerCase() === 'true' || value.toLowerCase() === 'yes';
break; break;
case 'iteration': case 'iteration':
config.iteration = parseInt(value, 10) || 0; config.iteration = parseInt(value, 10) || 0;
+20
View File
@@ -217,6 +217,9 @@ const TASK_DONE_PATTERN = /(?:task|item|todo)\s*(?:#?\d+|"\s*[^"]+\s*")?\s*(?:is
*/ */
const ANSI_ESCAPE_PATTERN = /\x1b\[[0-9;]*[A-Za-z]/g; const ANSI_ESCAPE_PATTERN = /\x1b\[[0-9;]*[A-Za-z]/g;
/** Maximum number of task number to content mappings to track */
const MAX_TASK_MAPPINGS = 100;
// ========== Event Types ========== // ========== Event Types ==========
/** /**
@@ -1159,6 +1162,7 @@ export class RalphTracker extends EventEmitter {
const content = match[2].trim(); const content = match[2].trim();
if (content.length >= 5) { if (content.length >= 5) {
this._taskNumberToContent.set(taskNum, content); this._taskNumberToContent.set(taskNum, content);
this.enforceTaskMappingLimit();
this.upsertTodo(content, 'pending'); this.upsertTodo(content, 'pending');
updated = true; updated = true;
} }
@@ -1173,6 +1177,7 @@ export class RalphTracker extends EventEmitter {
// Only register if not already known from a "created" line // Only register if not already known from a "created" line
if (!this._taskNumberToContent.has(taskNum)) { if (!this._taskNumberToContent.has(taskNum)) {
this._taskNumberToContent.set(taskNum, content); this._taskNumberToContent.set(taskNum, content);
this.enforceTaskMappingLimit();
} }
this.upsertTodo(this._taskNumberToContent.get(taskNum) || content, 'pending'); this.upsertTodo(this._taskNumberToContent.get(taskNum) || content, 'pending');
updated = true; updated = true;
@@ -1456,6 +1461,21 @@ export class RalphTracker extends EventEmitter {
this.emit('loopUpdate', this.loopState); this.emit('loopUpdate', this.loopState);
} }
/**
* Enforce size limit on _taskNumberToContent map.
* Removes lowest task numbers (oldest tasks) when limit exceeded.
*/
private enforceTaskMappingLimit(): void {
if (this._taskNumberToContent.size <= MAX_TASK_MAPPINGS) return;
// Sort keys and remove lowest (oldest) task numbers
const sortedKeys = Array.from(this._taskNumberToContent.keys()).sort((a, b) => a - b);
const keysToRemove = sortedKeys.slice(0, this._taskNumberToContent.size - MAX_TASK_MAPPINGS);
for (const key of keysToRemove) {
this._taskNumberToContent.delete(key);
}
}
/** /**
* Clear all state and disable the tracker. * Clear all state and disable the tracker.
* *
+16
View File
@@ -1250,6 +1250,9 @@ export class Session extends EventEmitter {
} }
private processOutput(data: string): void { private processOutput(data: string): void {
// Early return if session is stopped to prevent any processing or timer creation
if (this._isStopped) return;
// Try to extract JSON from output (Claude may output JSON in stream mode) // Try to extract JSON from output (Claude may output JSON in stream mode)
this._lineBuffer += data; this._lineBuffer += data;
@@ -1391,8 +1394,12 @@ export class Session extends EventEmitter {
} }
} }
/** Maximum number of task descriptions to keep */
private static readonly MAX_TASK_DESCRIPTIONS = 100;
/** /**
* Remove task descriptions older than TASK_DESCRIPTION_MAX_AGE_MS. * Remove task descriptions older than TASK_DESCRIPTION_MAX_AGE_MS.
* Also enforces MAX_TASK_DESCRIPTIONS size limit.
*/ */
private cleanupOldTaskDescriptions(): void { private cleanupOldTaskDescriptions(): void {
const cutoff = Date.now() - Session.TASK_DESCRIPTION_MAX_AGE_MS; const cutoff = Date.now() - Session.TASK_DESCRIPTION_MAX_AGE_MS;
@@ -1401,6 +1408,15 @@ export class Session extends EventEmitter {
this._recentTaskDescriptions.delete(timestamp); this._recentTaskDescriptions.delete(timestamp);
} }
} }
// Enforce size limit by removing oldest entries
if (this._recentTaskDescriptions.size > Session.MAX_TASK_DESCRIPTIONS) {
const sortedKeys = Array.from(this._recentTaskDescriptions.keys()).sort((a, b) => a - b);
const keysToRemove = sortedKeys.slice(0, this._recentTaskDescriptions.size - Session.MAX_TASK_DESCRIPTIONS);
for (const key of keysToRemove) {
this._recentTaskDescriptions.delete(key);
}
}
} }
/** /**
+32 -20
View File
@@ -408,7 +408,8 @@ export class SpawnOrchestrator extends EventEmitter {
try { try {
const content = readFileSync(progressPath, 'utf-8'); const content = readFileSync(progressPath, 'utf-8');
return JSON.parse(content) as AgentProgress; return JSON.parse(content) as AgentProgress;
} catch { } catch (err) {
console.warn(`[spawn-orchestrator] Failed to read progress for agent ${agentId}: ${getErrorMessage(err)}`);
return null; return null;
} }
} }
@@ -423,26 +424,37 @@ export class SpawnOrchestrator extends EventEmitter {
const messagesDir = join(agent.commsDir, 'messages'); const messagesDir = join(agent.commsDir, 'messages');
if (!existsSync(messagesDir)) return []; if (!existsSync(messagesDir)) return [];
const files = readdirSync(messagesDir) try {
.filter(f => f.endsWith('.md')) const files = readdirSync(messagesDir)
.sort(); .filter(f => f.endsWith('.md'))
.sort();
const messages: SpawnMessage[] = []; const messages: SpawnMessage[] = [];
for (const file of files) { for (const file of files) {
const match = file.match(/^(\d+)-(parent|agent)\.md$/); const match = file.match(/^(\d+)-(parent|agent)\.md$/);
if (!match) continue; if (!match) continue;
const content = readFileSync(join(messagesDir, file), 'utf-8'); try {
messages.push({ const filePath = join(messagesDir, file);
sequence: parseInt(match[1]), const content = readFileSync(filePath, 'utf-8');
sender: match[2] as 'parent' | 'agent', messages.push({
content, sequence: parseInt(match[1]),
sentAt: statSync(join(messagesDir, file)).mtimeMs, sender: match[2] as 'parent' | 'agent',
read: true, content,
}); sentAt: statSync(filePath).mtimeMs,
read: true,
});
} catch (err) {
console.warn(`[spawn-orchestrator] Failed to read message file ${file}: ${getErrorMessage(err)}`);
// Continue processing other messages
}
}
return messages;
} catch (err) {
console.warn(`[spawn-orchestrator] Failed to read messages for agent ${agentId}: ${getErrorMessage(err)}`);
return [];
} }
return messages;
} }
/** /**
@@ -839,8 +851,8 @@ export class SpawnOrchestrator extends EventEmitter {
if (agent.sessionId && this._sessionCreator) { if (agent.sessionId && this._sessionCreator) {
try { try {
await this._sessionCreator.stopSession(agent.sessionId); await this._sessionCreator.stopSession(agent.sessionId);
} catch { } catch (err) {
// Ignore cleanup errors console.warn(`[spawn-orchestrator] Failed to stop session for agent ${agentId}: ${getErrorMessage(err)}`);
} }
} }
+21 -7
View File
@@ -123,11 +123,18 @@ export class StateStore {
this.ensureDir(); this.ensureDir();
// Atomic write: write to temp file, then rename (atomic on POSIX) // Atomic write: write to temp file, then rename (atomic on POSIX)
const tempPath = this.filePath + '.tmp'; const tempPath = this.filePath + '.tmp';
let json: string;
try { try {
writeFileSync(tempPath, JSON.stringify(this.state, null, 2), 'utf-8'); json = JSON.stringify(this.state, null, 2);
} catch (err) {
console.error('[StateStore] Failed to serialize state (circular reference or invalid data):', err);
throw err;
}
try {
writeFileSync(tempPath, json, 'utf-8');
renameSync(tempPath, this.filePath); renameSync(tempPath, this.filePath);
} catch (err) { } catch (err) {
console.error('[StateStore] Failed to save state:', err); console.error('[StateStore] Failed to write state file:', err);
// Try to clean up temp file on error // Try to clean up temp file on error
try { try {
if (existsSync(tempPath)) { if (existsSync(tempPath)) {
@@ -156,7 +163,7 @@ export class StateStore {
/** Returns a session state by ID, or null if not found. */ /** Returns a session state by ID, or null if not found. */
getSession(id: string) { getSession(id: string) {
return this.state.sessions[id] || null; return this.state.sessions[id] ?? null;
} }
/** Sets a session state and triggers a debounced save. */ /** Sets a session state and triggers a debounced save. */
@@ -178,7 +185,7 @@ export class StateStore {
/** Returns a task state by ID, or null if not found. */ /** Returns a task state by ID, or null if not found. */
getTask(id: string) { getTask(id: string) {
return this.state.tasks[id] || null; return this.state.tasks[id] ?? null;
} }
/** Sets a task state and triggers a debounced save. */ /** Sets a task state and triggers a debounced save. */
@@ -457,11 +464,18 @@ export class StateStore {
const data = Object.fromEntries(this.ralphStates); const data = Object.fromEntries(this.ralphStates);
// Atomic write: write to temp file, then rename (atomic on POSIX) // Atomic write: write to temp file, then rename (atomic on POSIX)
const tempPath = this.ralphStatePath + '.tmp'; const tempPath = this.ralphStatePath + '.tmp';
let json: string;
try { try {
writeFileSync(tempPath, JSON.stringify(data, null, 2), 'utf-8'); json = JSON.stringify(data, null, 2);
} catch (err) {
console.error('[StateStore] Failed to serialize Ralph state (circular reference or invalid data):', err);
throw err;
}
try {
writeFileSync(tempPath, json, 'utf-8');
renameSync(tempPath, this.ralphStatePath); renameSync(tempPath, this.ralphStatePath);
} catch (err) { } catch (err) {
console.error('[StateStore] Failed to save Ralph state:', err); console.error('[StateStore] Failed to write Ralph state file:', err);
// Try to clean up temp file on error // Try to clean up temp file on error
try { try {
if (existsSync(tempPath)) { if (existsSync(tempPath)) {
@@ -475,7 +489,7 @@ export class StateStore {
/** Returns inner state for a session, or null if not found. */ /** Returns inner state for a session, or null if not found. */
getRalphState(sessionId: string): RalphSessionState | null { getRalphState(sessionId: string): RalphSessionState | null {
return this.ralphStates.get(sessionId) || null; return this.ralphStates.get(sessionId) ?? null;
} }
/** Sets inner state for a session and triggers a debounced save. */ /** Sets inner state for a session and triggers a debounced save. */
+8 -2
View File
@@ -798,8 +798,14 @@ export class SubagentWatcher extends EventEmitter {
const agentId = basename(filePath).replace('agent-', '').replace('.jsonl', ''); const agentId = basename(filePath).replace('agent-', '').replace('.jsonl', '');
// Initial info // Initial info - handle race condition where file may be deleted between discovery and stat
const stat = statSync(filePath); let stat;
try {
stat = statSync(filePath);
} catch {
// File was deleted between discovery and stat - skip this agent
return;
}
// Extract description - prefer reading from parent transcript (most reliable) // Extract description - prefer reading from parent transcript (most reliable)
// The parent transcript has the exact Task tool call with description parameter // The parent transcript has the exact Task tool call with description parameter
+5 -1
View File
@@ -985,7 +985,11 @@ class ClaudemanApp {
}; };
this.eventSource.addEventListener('init', (e) => { this.eventSource.addEventListener('init', (e) => {
this.handleInit(JSON.parse(e.data)); try {
this.handleInit(JSON.parse(e.data));
} catch (err) {
console.error('[SSE] Failed to parse init event:', err);
}
}); });
this.eventSource.addEventListener('session:created', (e) => { this.eventSource.addEventListener('session:created', (e) => {