From c33d6e0987a96ec0aec0bd273281bbed3229b103 Mon Sep 17 00:00:00 2001 From: arkon Date: Wed, 21 Jan 2026 14:01:47 +0100 Subject: [PATCH] refactor: improve code quality with stricter TypeScript and memory leak prevention - Add stricter TypeScript compiler flags (noUnusedLocals, noUnusedParameters, noImplicitReturns, noImplicitOverride, noFallthroughCasesInSwitch, allowUnreachableCode, allowUnusedLabels) - Remove unused variables caught by stricter flags: - Remove unused `renameSession` destructuring in App.tsx - Remove unused `BG_GRAY` constant in DirectAttach.ts - Remove unused `INPUT_BATCH_INTERVAL` constant in useSessionManager.ts - Add proper EventEmitter cleanup to RalphLoop: - Store bound event handlers for cleanup - Add cleanupEventHandlers() method - Add destroy() method for complete cleanup - Add destroyRalphLoop() singleton cleanup function - Update ralph-loop tests to use destroy() instead of stop() to prevent MaxListenersExceededWarning Co-Authored-By: Claude Opus 4.5 --- src/ralph-loop.ts | 59 +++++++++++++++++++++++++----- src/tui/App.tsx | 1 - src/tui/DirectAttach.ts | 1 - src/tui/hooks/useSessionManager.ts | 1 - test/ralph-loop.test.ts | 3 +- tsconfig.json | 9 ++++- 6 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/ralph-loop.ts b/src/ralph-loop.ts index 2144283e..695b483a 100644 --- a/src/ralph-loop.ts +++ b/src/ralph-loop.ts @@ -72,6 +72,13 @@ export class RalphLoop extends EventEmitter { private tasksCompleted: number = 0; private tasksGenerated: number = 0; + /** Bound event handlers for cleanup (prevents memory leaks) */ + private sessionEventHandlers: { + completion: (sessionId: string, phrase: string) => void; + error: (sessionId: string, error: string) => void; + stopped: (sessionId: string) => void; + } | null = null; + constructor(options: RalphLoopOptions = {}) { super(); this.sessionManager = getSessionManager(); @@ -95,17 +102,32 @@ export class RalphLoop extends EventEmitter { } private setupEventHandlers(): void { - this.sessionManager.on('sessionCompletion', (sessionId: string, phrase: string) => { - this.handleSessionCompletion(sessionId, phrase); - }); + // Store bound handlers for later cleanup + this.sessionEventHandlers = { + completion: (sessionId: string, phrase: string) => { + this.handleSessionCompletion(sessionId, phrase); + }, + error: (sessionId: string, error: string) => { + this.handleSessionError(sessionId, error); + }, + stopped: (sessionId: string) => { + this.handleSessionStopped(sessionId); + }, + }; - this.sessionManager.on('sessionError', (sessionId: string, error: string) => { - this.handleSessionError(sessionId, error); - }); + this.sessionManager.on('sessionCompletion', this.sessionEventHandlers.completion); + this.sessionManager.on('sessionError', this.sessionEventHandlers.error); + this.sessionManager.on('sessionStopped', this.sessionEventHandlers.stopped); + } - this.sessionManager.on('sessionStopped', (sessionId: string) => { - this.handleSessionStopped(sessionId); - }); + /** Remove event listeners to prevent memory leaks */ + private cleanupEventHandlers(): void { + if (this.sessionEventHandlers) { + this.sessionManager.off('sessionCompletion', this.sessionEventHandlers.completion); + this.sessionManager.off('sessionError', this.sessionEventHandlers.error); + this.sessionManager.off('sessionStopped', this.sessionEventHandlers.stopped); + this.sessionEventHandlers = null; + } } get status(): RalphLoopStatus { @@ -427,6 +449,17 @@ export class RalphLoop extends EventEmitter { this.minDurationMs = hours * 60 * 60 * 1000; this.store.setRalphLoopState({ minDurationMs: this.minDurationMs }); } + + /** + * Destroys the loop and cleans up all resources. + * Use this for complete cleanup (e.g., in tests or before creating a new instance). + * After calling destroy(), the instance should not be reused. + */ + destroy(): void { + this.stop(); + this.cleanupEventHandlers(); + this.removeAllListeners(); + } } // Singleton instance @@ -439,3 +472,11 @@ export function getRalphLoop(options?: RalphLoopOptions): RalphLoop { } return loopInstance; } + +/** Destroys the singleton instance. Use in tests or for cleanup. */ +export function destroyRalphLoop(): void { + if (loopInstance) { + loopInstance.destroy(); + loopInstance = null; + } +} diff --git a/src/tui/App.tsx b/src/tui/App.tsx index 7cc30409..bd181e68 100644 --- a/src/tui/App.tsx +++ b/src/tui/App.tsx @@ -122,7 +122,6 @@ export function App(): React.ReactElement { cases, lastUsedCase, toggleRespawn, - renameSession, } = useSessionManager(); // Calculate terminal height based on stdout dimensions diff --git a/src/tui/DirectAttach.ts b/src/tui/DirectAttach.ts index f1f6b70c..401daa88 100644 --- a/src/tui/DirectAttach.ts +++ b/src/tui/DirectAttach.ts @@ -17,7 +17,6 @@ const RESET = `${CSI}0m`; const BOLD = `${CSI}1m`; const DIM = `${CSI}2m`; const BG_BLUE = `${CSI}44m`; -const BG_GRAY = `${CSI}48;5;238m`; const FG_WHITE = `${CSI}37m`; const FG_CYAN = `${CSI}36m`; const FG_YELLOW = `${CSI}33m`; diff --git a/src/tui/hooks/useSessionManager.ts b/src/tui/hooks/useSessionManager.ts index facdcf54..1965d36d 100644 --- a/src/tui/hooks/useSessionManager.ts +++ b/src/tui/hooks/useSessionManager.ts @@ -37,7 +37,6 @@ const SCREENS_FILE = join(homedir(), '.claudeman', 'screens.json'); const INNER_STATE_FILE = join(homedir(), '.claudeman', 'state-inner.json'); const SETTINGS_FILE = join(homedir(), '.claudeman', 'settings.json'); const OUTPUT_POLL_INTERVAL = 300; // Poll terminal output every 300ms (faster refresh) -const INPUT_BATCH_INTERVAL = 16; // Batch input every 16ms (60fps) /** * Emoji to ASCII replacement map for screen hardcopy output. diff --git a/test/ralph-loop.test.ts b/test/ralph-loop.test.ts index 43d32281..32beac6a 100644 --- a/test/ralph-loop.test.ts +++ b/test/ralph-loop.test.ts @@ -124,7 +124,8 @@ describe('RalphLoop', () => { }); afterEach(() => { - loop.stop(); + // Use destroy() instead of stop() to clean up event listeners + loop.destroy(); vi.useRealTimers(); }); diff --git a/tsconfig.json b/tsconfig.json index dd444455..3209be66 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -15,7 +15,14 @@ "declarationMap": true, "sourceMap": true, "jsx": "react-jsx", - "jsxImportSource": "react" + "jsxImportSource": "react", + "noUnusedLocals": true, + "noUnusedParameters": true, + "noImplicitReturns": true, + "noImplicitOverride": true, + "noFallthroughCasesInSwitch": true, + "allowUnreachableCode": false, + "allowUnusedLabels": false }, "include": ["src/**/*"], "exclude": ["node_modules", "dist"]