fix(review): PKI key refresh, MQTT stale-client guard, log threshold

src/main/meshcore-mqtt-adapter.test.ts

@@ -141,6 +141,77 @@ describe('MeshcoreMqttAdapter — PING logging', () => {
   });
 });
 
+describe('MeshcoreMqttAdapter — stale client isolation', () => {
+  let adapter: MeshcoreMqttAdapter;
+
+  interface AdapterPacketPrivate {
+    lastPacketReceivedAt: number;
+  }
+
+  const makeClient = () => ({
+    on: vi.fn(),
+    end: vi.fn(),
+    removeAllListeners: vi.fn(),
+    connected: false,
+    publish: vi.fn(),
+    subscribe: vi.fn(),
+    reschedulePing: vi.fn(),
+    options: {},
+    stream: {},
+  });
+
+  const lastHandler = (
+    client: { on: ReturnType<typeof vi.fn> },
+    name: string,
+  ): ((packet: { cmd: string }) => void) => {
+    const hits = client.on.mock.calls.filter((c: unknown[]) => c[0] === name);
+    return hits[hits.length - 1]?.[1] as (packet: { cmd: string }) => void;
+  };
+
+  beforeEach(async () => {
+    const mqtt = await import('mqtt');
+    vi.mocked(mqtt.connect).mockClear();
+    adapter = new MeshcoreMqttAdapter();
+    adapter.on('error', () => {});
+  });
+
+  afterEach(() => {
+    adapter.disconnect();
+    vi.restoreAllMocks();
+  });
+
+  it('ignores packet events from a client that is no longer this.client', async () => {
+    const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
+    const mqttMod = await import('mqtt');
+    const first = makeClient();
+    const second = makeClient();
+    vi.mocked(mqttMod.connect)
+      .mockImplementationOnce(() => first as never)
+      .mockImplementationOnce(() => second as never);
+
+    adapter.connect({ ...BASE_SETTINGS });
+    // Reconnect: connect() force-ends `first` and installs `second` as this.client.
+    adapter.connect({ ...BASE_SETTINGS });
+
+    const priv = adapter as unknown as AdapterPacketPrivate;
+    priv.lastPacketReceivedAt = 0;
+
+    // Stale `first` client emits after it was replaced — must be ignored entirely.
+    lastHandler(first, 'packetreceive')({ cmd: 'pingresp' });
+    lastHandler(first, 'packetsend')({ cmd: 'pingreq' });
+    expect(priv.lastPacketReceivedAt).toBe(0);
+    expect(debugSpy.mock.calls.filter((c) => String(c[0]).includes('PINGRESP'))).toHaveLength(0);
+    expect(debugSpy.mock.calls.filter((c) => String(c[0]).includes('PINGREQ'))).toHaveLength(0);
+
+    // Live `second` client still updates state and consumes the first-ping logs.
+    lastHandler(second, 'packetreceive')({ cmd: 'pingresp' });
+    lastHandler(second, 'packetsend')({ cmd: 'pingreq' });
+    expect(priv.lastPacketReceivedAt).toBeGreaterThan(0);
+    expect(debugSpy.mock.calls.filter((c) => String(c[0]).includes('PINGRESP'))).toHaveLength(1);
+    expect(debugSpy.mock.calls.filter((c) => String(c[0]).includes('PINGREQ'))).toHaveLength(1);
+  });
+});
+
 describe('MeshcoreMqttAdapter — clientId', () => {
   let adapter: MeshcoreMqttAdapter;
 

src/main/meshcore-mqtt-adapter.ts

@@ -316,6 +316,10 @@ export class MeshcoreMqttAdapter extends EventEmitter {
     this.setStatus('connecting');
     this.connectAbortByWatchdog = false;
     this.client = mqtt.connect(connectOpts);
+    // Capture this session's client so listeners from an already-replaced (stale) client
+    // — mqtt.js can still emit after end() — cannot touch lastPacketReceivedAt or consume
+    // the new session's first-ping logs.
+    const sessionClient = this.client;
     this.client.on('error', (err) => {
       this.clearConnectTimers();
       console.error(
@@ -406,7 +410,8 @@ export class MeshcoreMqttAdapter extends EventEmitter {
       // Schedule proactive token refresh
       this.scheduleTokenRefresh();
     });
-    this.client.on('packetsend', (packet) => {
+    sessionClient.on('packetsend', (packet) => {
+      if (this.client !== sessionClient) return;
       if (packet.cmd === 'pingreq' && !this.pingReqLogged) {
         this.pingReqLogged = true;
         console.debug(
@@ -415,7 +420,8 @@ export class MeshcoreMqttAdapter extends EventEmitter {
         );
       }
     });
-    this.client.on('packetreceive', (packet) => {
+    sessionClient.on('packetreceive', (packet) => {
+      if (this.client !== sessionClient) return;
       this.lastPacketReceivedAt = Date.now();
       if (packet.cmd === 'pingresp' && !this.pingRespLogged) {
         this.pingRespLogged = true;

src/renderer/lib/meshtastic/meshtasticRuntimeWireEffects.ts

@@ -622,11 +622,19 @@ export function attachMeshtasticRuntimeWireEffects(
   });
   unsubscribesRef.current.push(unsub2);
 
-  const maybeRequestNodeInfoForNode = (from: number): void => {
+  const maybeRequestNodeInfoForNode = (
+    from: number,
+    opts?: { ignoreDisplayIdentity?: boolean },
+  ): void => {
     if (from === 0 || from === myNodeNumRef.current) return;
     if (isConfiguringRef.current) return;
-    const existing = getIdentityNode(meshtasticIdentityIdRef.current, from);
-    if (existing && !meshtasticNodeLacksDisplayIdentity(existing, from)) return;
+    // Missing-recipient-key recovery must refresh even nodes that already have a
+    // display name (we know who they are, we just lack a usable public key), so it
+    // opts out of the display-identity short-circuit while keeping the rate limit.
+    if (!opts?.ignoreDisplayIdentity) {
+      const existing = getIdentityNode(meshtasticIdentityIdRef.current, from);
+      if (existing && !meshtasticNodeLacksDisplayIdentity(existing, from)) return;
+    }
     const now = Date.now();
     const last = lastNodeInfoRequestAtRef.current.get(from) ?? 0;
     if (now - last < REQUEST_NODEINFO_MIN_INTERVAL_MS) return;
@@ -834,7 +842,9 @@ export function attachMeshtasticRuntimeWireEffects(
       myNodeNum: myNodeNumRef.current,
       identityId: meshtasticIdentityIdRef.current,
       tempIdToWirePacketId: ackMeshPacketIdByTempIdRef.current,
-      onMissingRecipientKey: maybeRequestNodeInfoForNode,
+      onMissingRecipientKey: (recipientNodeNum) => {
+        maybeRequestNodeInfoForNode(recipientNodeNum, { ignoreDisplayIdentity: true });
+      },
     });
   };
 
@@ -843,7 +853,9 @@ export function attachMeshtasticRuntimeWireEffects(
       myNodeNum: myNodeNumRef.current,
       identityId: meshtasticIdentityIdRef.current,
       tempIdToWirePacketId: ackMeshPacketIdByTempIdRef.current,
-      onMissingRecipientKey: maybeRequestNodeInfoForNode,
+      onMissingRecipientKey: (recipientNodeNum) => {
+        maybeRequestNodeInfoForNode(recipientNodeNum, { ignoreDisplayIdentity: true });
+      },
     });
     if (!uiApplied) {
       const parsed = reason as { id?: number; packetId?: number; error?: number };

src/renderer/lib/meshtastic/meshtasticSdkRoutingErrorLog.ts

@@ -4,6 +4,7 @@ import { resolveMeshtasticOutboundStoreKey } from '@/renderer/lib/sessions/mesht
 import { messageRecordsToChatMessages } from '@/renderer/lib/storeRecordAdapters';
 import type { ChatMessage } from '@/renderer/lib/types';
 import { updateMessageStatus, useMessageStore } from '@/renderer/stores/messageStore';
+import { isMeshtasticBroadcastNodeNum } from '@/shared/nodeNameUtils';
 
 import { meshtasticRoutingErrorName } from './meshtasticApplyErrorMessage';
 import {
@@ -202,7 +203,13 @@ export function applyMeshtasticOutboundRoutingError(
   const storeMessageId = resolveStoreMessageId(target, parsed.packetId);
   updateMessageStatus(identityId, storeMessageId, 'failed', errorText);
   // Missing recipient public key: fetch the recipient's NODEINFO so a retry can succeed.
-  if (isMeshtasticMissingRecipientKeyError(parsed.errorName) && target.to != null) {
+  // Only for real DM recipients — never the broadcast address (a PKI NAK there is not
+  // a per-node key gap, and 0xffffffff has no NODEINFO to fetch).
+  if (
+    isMeshtasticMissingRecipientKeyError(parsed.errorName) &&
+    target.to != null &&
+    !isMeshtasticBroadcastNodeNum(target.to)
+  ) {
     ctx.onMissingRecipientKey?.(target.to);
   }
   // The DB row may still hold the optimistic temp packet id (device never acked,

src/renderer/stores/reticulumPeerStore.test.ts

@@ -1101,6 +1101,56 @@ describe('reticulumPeerStore', () => {
     });
   });
 
+  const stubRefreshWindow = (): void => {
+    vi.stubGlobal('window', {
+      electronAPI: {
+        reticulum: {
+          proxyGet: vi.fn((path: string) => {
+            if (path === '/api/v1/contacts') return Promise.resolve({ contacts: [] });
+            if (path === '/api/v1/peers' || path.startsWith('/api/v1/peers?')) {
+              return Promise.resolve({ peers: [{ destination_hash: 'bb', hops: 3 }] });
+            }
+            if (path === '/api/v1/nomadnetwork/nodes') return Promise.resolve({ nodes: [] });
+            return Promise.resolve({});
+          }),
+        },
+        db: { getReticulumDestinations: vi.fn().mockResolvedValue([]) },
+      },
+    });
+  };
+
+  it('does not log a full-refresh debug line when refresh completes under 2s', async () => {
+    stubRefreshWindow();
+    // started and elapsed both read the same clock value → elapsed 0ms.
+    vi.spyOn(performance, 'now').mockReturnValue(1000);
+    const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
+
+    await refreshReticulumPeersFromSidecar({ forceRefresh: true });
+
+    expect(useReticulumPeerStore.getState().peers.get('bb')?.hops).toBe(3);
+    expect(debugSpy.mock.calls.filter((c) => String(c[0]).includes('full refresh'))).toHaveLength(
+      0,
+    );
+    debugSpy.mockRestore();
+  });
+
+  it('logs a full-refresh debug line when refresh exceeds the 2s threshold', async () => {
+    stubRefreshWindow();
+    // First now() call seeds `started` at 0; all later calls (incl. elapsed calc) → 5000ms.
+    vi.spyOn(performance, 'now').mockReturnValueOnce(0).mockReturnValue(5000);
+    const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
+
+    await refreshReticulumPeersFromSidecar({ forceRefresh: true });
+
+    expect(useReticulumPeerStore.getState().peers.get('bb')?.hops).toBe(3);
+    const fullRefreshLogs = debugSpy.mock.calls.filter((c) =>
+      String(c[0]).includes('full refresh'),
+    );
+    expect(fullRefreshLogs).toHaveLength(1);
+    expect(String(fullRefreshLogs[0][0])).toContain('5000ms');
+    debugSpy.mockRestore();
+  });
+
   it('applyReticulumAnnounceReceivedOptimistic inserts a peer before path-table refresh', () => {
     applyReticulumAnnounceReceivedOptimistic({
       destination_hash: 'AaBbCcDdEeFf00112233445566778899',