From b6dea4432cd62039f10269d873689c34fadd06e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Feb 2026 03:30:47 +0000 Subject: [PATCH] fix: address Copilot code review findings (PR #137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 3 critical issues identified in Copilot/Codex review: ## 1. Fixed Partial Config Override Bug (P1 - CRITICAL) **File:** src/Socket/socket.ts **Issue:** Using `||` operator caused partial configs to bypass defaults, resulting in undefined fields (cleanupHour, intervalMs, etc.) **Example bug:** ```typescript // ❌ BEFORE - Broken! sessionCleanupConfig: { autoCleanCorrupted: false } // Results in: { autoCleanCorrupted: false } ← missing all other fields! ``` **Fix:** Proper object spread merge ```typescript // ✅ AFTER - Correct! const sessionCleanupConfig = { ...DEFAULT_SESSION_CLEANUP_CONFIG, ...(config.sessionCleanupConfig || {}) } // Results in: { enabled: true, intervalMs: 86400000, ..., autoCleanCorrupted: false } ``` **Impact:** Prevents hot cleanup loops and undefined behavior --- ## 2. Fixed Nullish Coalescing Bug **File:** src/Socket/messages-recv.ts **Issue:** Using `||` instead of `??` caused undefined when config exists but lacks specific field **Fix:** ```typescript // ❌ BEFORE const autoCleanCorrupted = (sessionCleanupConfig || DEFAULT).autoCleanCorrupted // If sessionCleanupConfig = {}, this returns undefined! // ✅ AFTER const autoCleanCorrupted = sessionCleanupConfig?.autoCleanCorrupted ?? DEFAULT.autoCleanCorrupted ``` **Impact:** Always gets correct default value --- ## 3. Improved API Usability **File:** src/Types/Socket.ts **Change:** `SessionCleanupConfig` → `Partial` **Benefit:** Users can now override single fields without TypeScript errors ```typescript // Now valid: makeWASocket({ sessionCleanupConfig: { autoCleanCorrupted: false } }) ``` ## Validation - ✅ TypeScript compilation: 0 errors - ✅ Proper config merging tested - ✅ Backward compatible ## Related - PR #137 (Copilot review findings) - Original fixes: PR #136 https://claude.ai/code/session_01SoNUGBEWbJwWWws3F2fuzh --- src/Socket/messages-recv.ts | 3 ++- src/Socket/socket.ts | 6 +++++- src/Types/Socket.ts | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Socket/messages-recv.ts b/src/Socket/messages-recv.ts index 2afb87ab..de15ad01 100644 --- a/src/Socket/messages-recv.ts +++ b/src/Socket/messages-recv.ts @@ -79,7 +79,8 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => { enableCTWARecovery, sessionCleanupConfig } = config - const autoCleanCorrupted = (sessionCleanupConfig || DEFAULT_SESSION_CLEANUP_CONFIG).autoCleanCorrupted + // Use nullish coalescing to handle partial config properly + const autoCleanCorrupted = sessionCleanupConfig?.autoCleanCorrupted ?? DEFAULT_SESSION_CLEANUP_CONFIG.autoCleanCorrupted const sock = makeMessagesSocket(config) const { ev, diff --git a/src/Socket/socket.ts b/src/Socket/socket.ts index 5d76e62a..705e2dec 100644 --- a/src/Socket/socket.ts +++ b/src/Socket/socket.ts @@ -506,7 +506,11 @@ export const makeSocket = (config: SocketConfig) => { const sessionActivityTracker = makeSessionActivityTracker(keys, logger) // Session cleanup manager - removes inactive/orphaned sessions - const sessionCleanupConfig = config.sessionCleanupConfig || DEFAULT_SESSION_CLEANUP_CONFIG + // Merge user config with defaults to prevent partial overrides from breaking cleanup + const sessionCleanupConfig = { + ...DEFAULT_SESSION_CLEANUP_CONFIG, + ...(config.sessionCleanupConfig || {}) + } const sessionCleanup = makeSessionCleanup( keys, signalRepository.lidMapping, diff --git a/src/Types/Socket.ts b/src/Types/Socket.ts index 425c20d1..9df4734d 100644 --- a/src/Types/Socket.ts +++ b/src/Types/Socket.ts @@ -254,6 +254,6 @@ export type SocketConfig = { */ enableUnifiedSession?: boolean - /** Session cleanup configuration (optional, uses DEFAULT_SESSION_CLEANUP_CONFIG if not provided) */ - sessionCleanupConfig?: SessionCleanupConfig + /** Session cleanup configuration (optional, partial overrides merged with defaults) */ + sessionCleanupConfig?: Partial }