From 3dcdc7be6901ea38831c84ade376c387030933be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Jan 2026 12:32:56 +0000 Subject: [PATCH] fix(signal): critical fixes for identity key detection - batch 1 High Priority Fixes based on Copilot/Codex code review: 1. Propagate errorCode to shouldRecreateSession (CRITICAL) - Extract error attribute from retry node - Pass errorCode to shouldRecreateSession in both sendRetryRequest and sendMessagesAgain - Without this fix, MAC error detection was NOT working - Now immediate session recreation works for MAC errors 2. Use .unref() on metrics interval - Prevents blocking process exit in short-lived scripts/tests - Interval no longer keeps event loop alive unnecessarily 3. Reset circuit breaker regardless of state - Previously only reset when isOpen() - Now resets in any state (open, half-open, or closed with accumulated failures) - Ensures clean slate after identity change detection https://claude.ai/code/session_01SWAcNuGZQmEKyBPYkBhVHg --- src/Signal/libsignal.ts | 9 +++++---- src/Socket/messages-recv.ts | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Signal/libsignal.ts b/src/Signal/libsignal.ts index 2755086b..0aef6d22 100644 --- a/src/Signal/libsignal.ts +++ b/src/Signal/libsignal.ts @@ -255,9 +255,9 @@ export function makeLibSignalRepository( metrics.signalIdentityKeyCacheSize?.set(identityKeyCache.size) }, 60000) // Every minute - // Cleanup interval on process exit - if (typeof process !== 'undefined') { - process.on('beforeExit', () => clearInterval(cacheMetricsInterval)) + // Allow process to exit even with interval active (prevents blocking in short-lived scripts/tests) + if (typeof cacheMetricsInterval.unref === 'function') { + cacheMetricsInterval.unref() } const storage = signalStorage(auth, lidMapping, identityKeyCache, ev, preKeyCircuitBreaker, logger) @@ -334,7 +334,8 @@ export function makeLibSignalRepository( ) // Reset prekey circuit breaker since we identified the cause - if (preKeyCircuitBreaker?.isOpen()) { + // Reset regardless of state (could be open, half-open, or closed with accumulated failures) + if (preKeyCircuitBreaker) { preKeyCircuitBreaker.reset() logger.debug({ jid }, 'Reset prekey circuit breaker after identity key change detection') } diff --git a/src/Socket/messages-recv.ts b/src/Socket/messages-recv.ts index c0b30181..ccb66a53 100644 --- a/src/Socket/messages-recv.ts +++ b/src/Socket/messages-recv.ts @@ -452,12 +452,18 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => { // Check if we have a session with this JID const sessionId = signalRepository.jidToSignalProtocolAddress(fromJid) const hasSession = await signalRepository.validateSession(fromJid) - const result = messageRetryManager.shouldRecreateSession(fromJid, hasSession.exists) + + // Extract error code from retry node if present (for MAC error detection) + const retryNode = getBinaryNodeChild(node, 'retry') + const errorAttr = retryNode?.attrs?.error as string | undefined + const errorCode = messageRetryManager.parseRetryErrorCode(errorAttr) + + const result = messageRetryManager.shouldRecreateSession(fromJid, hasSession.exists, errorCode) shouldRecreateSession = result.recreate recreateReason = result.reason if (shouldRecreateSession) { - logger.debug({ fromJid, retryCount, reason: recreateReason }, 'recreating session for retry') + logger.debug({ fromJid, retryCount, reason: recreateReason, errorCode }, 'recreating session for retry') // Delete existing session to force recreation await authState.keys.set({ session: { [sessionId]: null } }) forceIncludeKeys = true @@ -1006,12 +1012,17 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => { const sessionId = signalRepository.jidToSignalProtocolAddress(participant) const hasSession = await signalRepository.validateSession(participant) - const result = messageRetryManager.shouldRecreateSession(participant, hasSession.exists) + + // Extract error code from retry node if present (for MAC error detection) + const errorAttr = retryNode?.attrs?.error as string | undefined + const errorCode = messageRetryManager.parseRetryErrorCode(errorAttr) + + const result = messageRetryManager.shouldRecreateSession(participant, hasSession.exists, errorCode) shouldRecreateSession = result.recreate recreateReason = result.reason if (shouldRecreateSession) { - logger.debug({ participant, retryCount, reason: recreateReason }, 'recreating session for outgoing retry') + logger.debug({ participant, retryCount, reason: recreateReason, errorCode }, 'recreating session for outgoing retry') await authState.keys.set({ session: { [sessionId]: null } }) } } catch (error) {