mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 08:59:40 +02:00
fix: medium severity audit fixes — races, leaks, perf, warnings
- Clear teams/teamTasks maps in handleInit() on SSE reconnect - Fix saveNowAsync() race condition with in-flight promise guard - Add setMaxListeners() on Session, WebServer, SubagentWatcher, ImageWatcher, TmuxManager to prevent MaxListenersExceeded warnings - Debounce persistSessionState() per-session (100ms) to reduce redundant toState() serialization across 25+ call sites - Cache getLightSessionsState() with 1s TTL to avoid re-serializing all sessions on every SSE connect and /api/sessions request Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -87,6 +87,7 @@ export class ImageWatcher extends EventEmitter {
|
|||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
super();
|
super();
|
||||||
|
this.setMaxListeners(50);
|
||||||
}
|
}
|
||||||
|
|
||||||
// ========== Public API ==========
|
// ========== Public API ==========
|
||||||
|
|||||||
@@ -369,6 +369,7 @@ export class Session extends EventEmitter {
|
|||||||
niceConfig?: NiceConfig; // Nice prioritying configuration
|
niceConfig?: NiceConfig; // Nice prioritying configuration
|
||||||
}) {
|
}) {
|
||||||
super();
|
super();
|
||||||
|
this.setMaxListeners(25);
|
||||||
|
|
||||||
// Default error handler prevents unhandled 'error' events from crashing the process.
|
// Default error handler prevents unhandled 'error' events from crashing the process.
|
||||||
// Server attaches its own handler after construction — this is a safety net for the gap.
|
// Server attaches its own handler after construction — this is a safety net for the gap.
|
||||||
|
|||||||
@@ -64,6 +64,9 @@ export class StateStore {
|
|||||||
private consecutiveSaveFailures: number = 0;
|
private consecutiveSaveFailures: number = 0;
|
||||||
private circuitBreakerOpen: boolean = false;
|
private circuitBreakerOpen: boolean = false;
|
||||||
|
|
||||||
|
// Guard against concurrent saveNowAsync() calls (debounce can race with in-flight write)
|
||||||
|
private _saveInFlight: Promise<void> | null = null;
|
||||||
|
|
||||||
constructor(filePath?: string) {
|
constructor(filePath?: string) {
|
||||||
this.filePath = filePath || join(homedir(), '.claudeman', 'state.json');
|
this.filePath = filePath || join(homedir(), '.claudeman', 'state.json');
|
||||||
this.ralphStatePath = this.filePath.replace('.json', '-inner.json');
|
this.ralphStatePath = this.filePath.replace('.json', '-inner.json');
|
||||||
@@ -130,8 +133,25 @@ export class StateStore {
|
|||||||
* Async version of saveNow — used by the debounced save() path.
|
* Async version of saveNow — used by the debounced save() path.
|
||||||
* Uses non-blocking fs.promises to avoid blocking the event loop during
|
* Uses non-blocking fs.promises to avoid blocking the event loop during
|
||||||
* the debounced write cycle. For synchronous shutdown flush, use saveNow().
|
* the debounced write cycle. For synchronous shutdown flush, use saveNow().
|
||||||
|
*
|
||||||
|
* Guards against concurrent execution: if a save is already in flight,
|
||||||
|
* waits for it to complete then re-checks dirty flag before starting another.
|
||||||
*/
|
*/
|
||||||
async saveNowAsync(): Promise<void> {
|
async saveNowAsync(): Promise<void> {
|
||||||
|
if (this._saveInFlight) {
|
||||||
|
await this._saveInFlight;
|
||||||
|
// After waiting, re-check if still dirty (the previous save may have handled it)
|
||||||
|
if (!this.dirty) return;
|
||||||
|
}
|
||||||
|
this._saveInFlight = this._doSaveAsync();
|
||||||
|
try {
|
||||||
|
await this._saveInFlight;
|
||||||
|
} finally {
|
||||||
|
this._saveInFlight = null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private async _doSaveAsync(): Promise<void> {
|
||||||
if (this.saveTimeout) {
|
if (this.saveTimeout) {
|
||||||
clearTimeout(this.saveTimeout);
|
clearTimeout(this.saveTimeout);
|
||||||
this.saveTimeout = null;
|
this.saveTimeout = null;
|
||||||
|
|||||||
@@ -177,6 +177,7 @@ export class SubagentWatcher extends EventEmitter {
|
|||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
super();
|
super();
|
||||||
|
this.setMaxListeners(50);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -141,6 +141,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
|||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
super();
|
super();
|
||||||
|
this.setMaxListeners(50);
|
||||||
if (!IS_TEST_MODE) {
|
if (!IS_TEST_MODE) {
|
||||||
this.loadSessions();
|
this.loadSessions();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3097,6 +3097,8 @@ class ClaudemanApp {
|
|||||||
this.terminalBuffers.clear();
|
this.terminalBuffers.clear();
|
||||||
this.terminalBufferCache.clear();
|
this.terminalBufferCache.clear();
|
||||||
this.projectInsights.clear();
|
this.projectInsights.clear();
|
||||||
|
this.teams.clear();
|
||||||
|
this.teamTasks.clear();
|
||||||
// Clear all idle timers to prevent stale timers from firing
|
// Clear all idle timers to prevent stale timers from firing
|
||||||
for (const timer of this.idleTimers.values()) {
|
for (const timer of this.idleTimers.values()) {
|
||||||
clearTimeout(timer);
|
clearTimeout(timer);
|
||||||
|
|||||||
+43
-3
@@ -115,6 +115,8 @@ const DEC_SYNC_START = '\x1b[?2026h'; // Begin synchronized update
|
|||||||
const DEC_SYNC_END = '\x1b[?2026l'; // End synchronized update (flush to screen)
|
const DEC_SYNC_END = '\x1b[?2026l'; // End synchronized update (flush to screen)
|
||||||
// State update debounce interval (batch expensive toDetailedState() calls)
|
// State update debounce interval (batch expensive toDetailedState() calls)
|
||||||
const STATE_UPDATE_DEBOUNCE_INTERVAL = 500;
|
const STATE_UPDATE_DEBOUNCE_INTERVAL = 500;
|
||||||
|
// Cache TTL for getLightSessionsState() — avoids re-serializing all sessions on every SSE init / /api/sessions call
|
||||||
|
const SESSIONS_LIST_CACHE_TTL = 1000;
|
||||||
// Scheduled runs cleanup interval (check every 5 minutes)
|
// Scheduled runs cleanup interval (check every 5 minutes)
|
||||||
const SCHEDULED_CLEANUP_INTERVAL = 5 * 60 * 1000;
|
const SCHEDULED_CLEANUP_INTERVAL = 5 * 60 * 1000;
|
||||||
// Completed scheduled runs max age (1 hour)
|
// Completed scheduled runs max age (1 hour)
|
||||||
@@ -373,6 +375,8 @@ export class WebServer extends EventEmitter {
|
|||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
private cachedLightState: { data: Record<string, unknown>; timestamp: number } | null = null;
|
private cachedLightState: { data: Record<string, unknown>; timestamp: number } | null = null;
|
||||||
private static readonly LIGHT_STATE_CACHE_TTL_MS = 1000;
|
private static readonly LIGHT_STATE_CACHE_TTL_MS = 1000;
|
||||||
|
// Cached sessions list for getLightSessionsState() (avoids re-serializing all sessions on every call)
|
||||||
|
private cachedSessionsList: { data: unknown[]; timestamp: number } | null = null;
|
||||||
// Token recording for daily stats (track what's been recorded to avoid double-counting)
|
// Token recording for daily stats (track what's been recorded to avoid double-counting)
|
||||||
private lastRecordedTokens: Map<string, { input: number; output: number }> = new Map();
|
private lastRecordedTokens: Map<string, { input: number; output: number }> = new Map();
|
||||||
private tokenRecordingTimer: NodeJS.Timeout | null = null;
|
private tokenRecordingTimer: NodeJS.Timeout | null = null;
|
||||||
@@ -382,6 +386,7 @@ export class WebServer extends EventEmitter {
|
|||||||
private pendingRespawnStarts: Map<string, NodeJS.Timeout> = new Map();
|
private pendingRespawnStarts: Map<string, NodeJS.Timeout> = new Map();
|
||||||
// Active plan orchestrators (for cancellation via API)
|
// Active plan orchestrators (for cancellation via API)
|
||||||
private activePlanOrchestrators: Map<string, PlanOrchestrator> = new Map();
|
private activePlanOrchestrators: Map<string, PlanOrchestrator> = new Map();
|
||||||
|
private persistDebounceTimers: Map<string, ReturnType<typeof setTimeout>> = new Map();
|
||||||
// Grace period before starting restored respawn controllers (2 minutes)
|
// Grace period before starting restored respawn controllers (2 minutes)
|
||||||
private static readonly RESPAWN_RESTORE_GRACE_PERIOD_MS = 2 * 60 * 1000;
|
private static readonly RESPAWN_RESTORE_GRACE_PERIOD_MS = 2 * 60 * 1000;
|
||||||
// Stored listener handlers for cleanup
|
// Stored listener handlers for cleanup
|
||||||
@@ -401,6 +406,7 @@ export class WebServer extends EventEmitter {
|
|||||||
} | null = null;
|
} | null = null;
|
||||||
constructor(port: number = 3000, https: boolean = false, testMode: boolean = false) {
|
constructor(port: number = 3000, https: boolean = false, testMode: boolean = false) {
|
||||||
super();
|
super();
|
||||||
|
this.setMaxListeners(0);
|
||||||
this.port = port;
|
this.port = port;
|
||||||
this.https = https;
|
this.https = https;
|
||||||
this.testMode = testMode;
|
this.testMode = testMode;
|
||||||
@@ -3438,8 +3444,21 @@ NOW: Generate the implementation plan for the task above. Think step by step.`;
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Persists full session state including respawn config to state.json */
|
/** Debounced wrapper — coalesces rapid persistSessionState calls per session */
|
||||||
private persistSessionState(session: Session): void {
|
private persistSessionState(session: Session): void {
|
||||||
|
const existing = this.persistDebounceTimers.get(session.id);
|
||||||
|
if (existing) clearTimeout(existing);
|
||||||
|
this.persistDebounceTimers.set(session.id, setTimeout(() => {
|
||||||
|
this.persistDebounceTimers.delete(session.id);
|
||||||
|
// Session may have been removed during debounce
|
||||||
|
if (this.sessions.has(session.id)) {
|
||||||
|
this._persistSessionStateNow(session);
|
||||||
|
}
|
||||||
|
}, 100));
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Persists full session state including respawn config to state.json */
|
||||||
|
private _persistSessionStateNow(session: Session): void {
|
||||||
const state = session.toState();
|
const state = session.toState();
|
||||||
const controller = this.respawnControllers.get(session.id);
|
const controller = this.respawnControllers.get(session.id);
|
||||||
if (controller) {
|
if (controller) {
|
||||||
@@ -4341,9 +4360,15 @@ NOW: Generate the implementation plan for the task above. Think step by step.`;
|
|||||||
* on-demand when switching tabs via /api/sessions/:id/buffer
|
* on-demand when switching tabs via /api/sessions/:id/buffer
|
||||||
*/
|
*/
|
||||||
private getLightSessionsState() {
|
private getLightSessionsState() {
|
||||||
|
const now = Date.now();
|
||||||
|
if (this.cachedSessionsList && (now - this.cachedSessionsList.timestamp) < SESSIONS_LIST_CACHE_TTL) {
|
||||||
|
return this.cachedSessionsList.data;
|
||||||
|
}
|
||||||
// getSessionStateWithRespawn already uses toLightDetailedState() which
|
// getSessionStateWithRespawn already uses toLightDetailedState() which
|
||||||
// excludes terminalBuffer and textOutput — no extra stripping needed
|
// excludes terminalBuffer and textOutput — no extra stripping needed
|
||||||
return Array.from(this.sessions.values()).map(s => this.getSessionStateWithRespawn(s));
|
const data = Array.from(this.sessions.values()).map(s => this.getSessionStateWithRespawn(s));
|
||||||
|
this.cachedSessionsList = { data, timestamp: now };
|
||||||
|
return data;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Clean up old completed scheduled runs
|
// Clean up old completed scheduled runs
|
||||||
@@ -4449,9 +4474,10 @@ NOW: Generate the implementation plan for the task above. Think step by step.`;
|
|||||||
}
|
}
|
||||||
|
|
||||||
private broadcast(event: string, data: unknown): void {
|
private broadcast(event: string, data: unknown): void {
|
||||||
// Invalidate light state cache on any state-changing broadcast
|
// Invalidate caches on any state-changing broadcast
|
||||||
if (event.startsWith('session:') || event === 'respawn:') {
|
if (event.startsWith('session:') || event === 'respawn:') {
|
||||||
this.cachedLightState = null;
|
this.cachedLightState = null;
|
||||||
|
this.cachedSessionsList = null;
|
||||||
}
|
}
|
||||||
// Performance optimization: serialize JSON once for all clients
|
// Performance optimization: serialize JSON once for all clients
|
||||||
let message: string;
|
let message: string;
|
||||||
@@ -5000,6 +5026,20 @@ NOW: Generate the implementation plan for the task above. Think step by step.`;
|
|||||||
// Stop multiplexer and flush pending saves
|
// Stop multiplexer and flush pending saves
|
||||||
this.mux.destroy();
|
this.mux.destroy();
|
||||||
|
|
||||||
|
// Flush any pending persist-debounce timers and persist dirty sessions
|
||||||
|
for (const [sessionId, timer] of this.persistDebounceTimers) {
|
||||||
|
clearTimeout(timer);
|
||||||
|
const session = this.sessions.get(sessionId);
|
||||||
|
if (session) {
|
||||||
|
this._persistSessionStateNow(session);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
this.persistDebounceTimers.clear();
|
||||||
|
|
||||||
|
// Clear cached state
|
||||||
|
this.cachedLightState = null;
|
||||||
|
this.cachedSessionsList = null;
|
||||||
|
|
||||||
// Clear all pending respawn start timers (from restoration grace period)
|
// Clear all pending respawn start timers (from restoration grace period)
|
||||||
for (const timer of this.pendingRespawnStarts.values()) {
|
for (const timer of this.pendingRespawnStarts.values()) {
|
||||||
clearTimeout(timer);
|
clearTimeout(timer);
|
||||||
|
|||||||
Reference in New Issue
Block a user