fix(meshcore): full official-app DM retry cadence + single-bubble fix (#3977)

Extends the DM ack-timeout retry from a minimal one-shot flood to the full
official MeshCore app cadence (MeshCore FAQ §5.3), and fixes the duplicate-
message bug in the previous phase.

Cadence (named constants):
- DM_SAME_PATH_RETRIES = 2: on ack timeout, resend on the CURRENT cached path.
- DM_FLOOD_RETRIES = 1: after same-path retries exhaust, reset the path and
  resend via flood with the default scope; the ACK teaches the new route,
  which the firmware auto-persists via the existing contact_path_updated push.
- After all retries exhaust, mark the message failed.
These match the documented firmware/app default ("fail after 3 retries, reset
path and flood on the last retry"): 2 same-path + 1 flood = 3 retries, ~4
transmissions incl. the initial send. Each attempt waits the firmware's own
estTimeout (+20% margin).

Single-bubble fix (previous phase created a duplicate):
- An auto-retry resend no longer creates a new message row or re-emits a
  `message` event (that produced a second bubble with contradictory
  failed/delivered status and a duplicate DB row). The retry reuses the
  ORIGINAL message; performScopedSend skips addMessage/emit when isAutoRetry.
- handleDmAckTimeout re-points the message's tracked expectedAckCrc/estTimeout
  to each new attempt via a new `meshcore:message:updated` event, so the UI
  keeps ONE bubble that ends `delivered` on any ACK or `failed` only after all
  attempts are exhausted.
- Frontend (useMeshCore) handles the event: cancels the prior attempt's fail
  timer, updates the message in place, and re-arms against the new CRC.

Concurrency: the reset+resend runs inside runSerialized (per-source lock, no
re-entrancy — the timer fires outside the lock). Timers are keyed by the
current attempt's CRC, cleared on ack and in bulk on disconnect; a torn-down
manager cannot fire a stray retry mid-sequence.

Tests: meshcoreManager.dmRetry.test.ts rewritten (10 cases) — same-path retry
sequence, flood-after-same-path-exhausted, single-bubble/no-duplicate-emit,
delivered-cancels-sequence, exhaustion→failed, reset_path-failure, path
persistence on flood ack, channel-send excluded, disconnect cleanup mid-seq.
Full suite green (8068 passed, 0 failed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub

Author: Yeraze

src/components/MeshCore/hooks/useMeshCore.ts

@@ -590,6 +590,50 @@ export function useMeshCore(options: UseMeshCoreOptions): UseMeshCoreState {
       ));
     };
 
+    // DM ack-timeout retry update (#3977): the server re-sent a DM on a new
+    // path (same-path resend, or flood after reset) and re-points this message
+    // to the new attempt's ack CRC, or marks it failed once all retries are
+    // exhausted. Keep a SINGLE bubble — update the existing message in place
+    // rather than appending a new one, and follow the fail timer to the new CRC.
+    const onMessageUpdated = (evt: {
+      sourceId: string;
+      id: string;
+      previousAckCrc?: number;
+      expectedAckCrc?: number;
+      estTimeout?: number;
+      deliveryStatus?: MessageDeliveryStatus;
+    }) => {
+      if (evt.sourceId !== sourceId) return;
+      // Cancel the fail timer armed for the prior attempt's CRC — the new
+      // attempt (or terminal status) supersedes it.
+      if (evt.previousAckCrc != null) {
+        const prevTimer = ackTimers.get(evt.previousAckCrc);
+        if (prevTimer) {
+          clearTimeout(prevTimer);
+          ackTimers.delete(evt.previousAckCrc);
+        }
+      }
+      setMessages(prev => prev.map(m => {
+        if (m.id !== evt.id) return m;
+        return {
+          ...m,
+          ...(evt.expectedAckCrc != null ? { expectedAckCrc: evt.expectedAckCrc } : {}),
+          ...(evt.estTimeout != null ? { estTimeout: evt.estTimeout } : {}),
+          ...(evt.deliveryStatus ? { deliveryStatus: evt.deliveryStatus } : {}),
+        };
+      }));
+      // Arm a fresh fail timer for the new attempt unless this was a terminal
+      // status (delivered/failed).
+      if (
+        evt.expectedAckCrc != null &&
+        evt.estTimeout != null &&
+        evt.deliveryStatus !== 'failed' &&
+        evt.deliveryStatus !== 'delivered'
+      ) {
+        startAckTimeout(evt.expectedAckCrc, evt.estTimeout);
+      }
+    };
+
     // Channel "heard repeaters" (#3700): a repeater re-flooded one of our
     // outgoing channel messages and the server correlated the self-echo. The
     // event carries the current full heard-by set, so we replace state by id.
@@ -609,6 +653,7 @@ export function useMeshCore(options: UseMeshCoreOptions): UseMeshCoreState {
     socket.on('meshcore:status:updated', onStatusUpdated);
     socket.on('meshcore:local-node:updated', onLocalNodeUpdated);
     socket.on('meshcore:send-confirmed', onSendConfirmed);
+    socket.on('meshcore:message:updated', onMessageUpdated);
     socket.on('meshcore:channel-heard', onChannelHeard);
     socket.io.on('reconnect', onReconnect);
 
@@ -621,6 +666,7 @@ export function useMeshCore(options: UseMeshCoreOptions): UseMeshCoreState {
       socket.off('meshcore:status:updated', onStatusUpdated);
       socket.off('meshcore:local-node:updated', onLocalNodeUpdated);
       socket.off('meshcore:send-confirmed', onSendConfirmed);
+      socket.off('meshcore:message:updated', onMessageUpdated);
       socket.off('meshcore:channel-heard', onChannelHeard);
       socket.io.off('reconnect', onReconnect);
     };

src/server/meshcoreManager.dmRetry.test.ts

@@ -1,17 +1,20 @@
 /**
- * Tests for MeshCore DM auto-retry-via-flood on ack timeout (#3977).
+ * Tests for MeshCore DM auto-retry on ack timeout (#3977).
  *
- * A DM sent on a stale cached path just goes out once today and is never
- * retried, so it silently never arrives. This automates exactly what the
- * existing "Reset Path" button already does by hand: if no `SendConfirmed`
- * ack arrives within the firmware's own `estTimeout` (+margin), reset the
- * contact's cached path and resend once via flood. The retry's own send is
- * not itself tracked — a second miss is left to the frontend's existing
- * per-message ack timer, exactly as any other unacked send today.
+ * A DM sent on a stale cached path used to go out once and never retry, so it
+ * silently never arrived. This replicates the official MeshCore app cadence
+ * (MeshCore FAQ §5.3): on each ack timeout, resend on the CURRENT cached path
+ * `DM_SAME_PATH_RETRIES` (2) times, then reset the path and resend via flood
+ * `DM_FLOOD_RETRIES` (1) time, then give up and mark the message failed. The
+ * whole sequence reuses the ORIGINAL message — it never creates a second bubble
+ * / DB row; instead it re-points the message's tracked ack CRC via
+ * `emitMeshCoreMessageUpdated` and settles on `delivered` (any ACK) or `failed`
+ * (all retries exhausted).
  */
 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { MeshCoreManager, MeshCoreDeviceType } from './meshcoreManager.js';
 import databaseService from '../services/database.js';
+import { dataEventEmitter } from './services/dataEventEmitter.js';
 
 interface BridgeCall { cmd: string; params: Record<string, unknown>; }
 
@@ -30,7 +33,13 @@ function makeManager(opts: {
 
   const bridgeCalls: BridgeCall[] = [];
   let sendIdx = 0;
-  const sends = opts.sends ?? [{ ackCrc: 111, estTimeout: 8000 }, { ackCrc: 222, estTimeout: 8000 }];
+  // Distinct CRC per attempt so the retry sequence re-points cleanly.
+  const sends = opts.sends ?? [
+    { ackCrc: 111, estTimeout: 8000 },
+    { ackCrc: 222, estTimeout: 8000 },
+    { ackCrc: 333, estTimeout: 8000 },
+    { ackCrc: 444, estTimeout: 8000 },
+  ];
 
   (m as any).sendBridgeCommand = async (cmd: string, params: Record<string, unknown>) => {
     bridgeCalls.push({ cmd, params });
@@ -69,6 +78,8 @@ function sendConfirmed(m: MeshCoreManager, ackCode: number): void {
 }
 
 const PUBKEY = 'bob'.padEnd(64, '0');
+const sends = (calls: BridgeCall[]) => calls.filter(c => c.cmd === 'send_message');
+const resets = (calls: BridgeCall[]) => calls.filter(c => c.cmd === 'reset_path');
 
 describe('MeshCoreManager — DM ack-timeout auto-retry (#3977)', () => {
   beforeEach(() => {
@@ -88,83 +99,173 @@ describe('MeshCoreManager — DM ack-timeout auto-retry (#3977)', () => {
 
     await vi.advanceTimersByTimeAsync(20_000);
 
-    const sendCalls = bridgeCalls.filter(c => c.cmd === 'send_message');
-    expect(sendCalls).toHaveLength(1);
-    expect(bridgeCalls.some(c => c.cmd === 'reset_path')).toBe(false);
+    expect(sends(bridgeCalls)).toHaveLength(1);
+    expect(resets(bridgeCalls)).toHaveLength(0);
+    expect((manager as any).pendingDmRetries.size).toBe(0);
   });
 
-  it('resets the path and resends once via flood when no ack arrives in time', async () => {
+  it('resends on the CURRENT path (no reset) for the first same-path retries', async () => {
     const { manager, bridgeCalls } = makeManager();
 
     await manager.sendMessageWithResult('hello', PUBKEY);
-    // No send_confirmed dispatched — let the ack timeout (estTimeout=8000, +20% margin) fire.
+
+    // First timeout -> same-path retry #1 (no reset).
+    await vi.advanceTimersByTimeAsync(10_000);
+    expect(sends(bridgeCalls)).toHaveLength(2);
+    expect(resets(bridgeCalls)).toHaveLength(0);
+    expect(sends(bridgeCalls)[1].params.to).toBe(PUBKEY);
+
+    // Second timeout -> same-path retry #2 (still no reset).
     await vi.advanceTimersByTimeAsync(10_000);
+    expect(sends(bridgeCalls)).toHaveLength(3);
+    expect(resets(bridgeCalls)).toHaveLength(0);
+  });
+
+  it('falls back to flood (reset path) only after same-path retries are exhausted', async () => {
+    const { manager, bridgeCalls } = makeManager();
 
-    const resetCalls = bridgeCalls.filter(c => c.cmd === 'reset_path');
-    expect(resetCalls).toHaveLength(1);
-    expect(resetCalls[0].params.public_key).toBe(PUBKEY);
+    await manager.sendMessageWithResult('hello', PUBKEY);
+    await vi.advanceTimersByTimeAsync(10_000); // same-path retry #1
+    await vi.advanceTimersByTimeAsync(10_000); // same-path retry #2
+    expect(resets(bridgeCalls)).toHaveLength(0);
 
-    const sendCalls = bridgeCalls.filter(c => c.cmd === 'send_message');
-    expect(sendCalls).toHaveLength(2);
-    expect(sendCalls[1].params.text).toBe('hello');
-    expect(sendCalls[1].params.to).toBe(PUBKEY);
+    // Third timeout -> path is reset and the message floods.
+    await vi.advanceTimersByTimeAsync(10_000);
+    expect(resets(bridgeCalls)).toHaveLength(1);
+    expect(resets(bridgeCalls)[0].params.public_key).toBe(PUBKEY);
+    expect(sends(bridgeCalls)).toHaveLength(4);
+    expect(sends(bridgeCalls)[3].params.to).toBe(PUBKEY);
   });
 
-  it('gives up silently if the flood retry also goes unacked (no second retry)', async () => {
+  it('marks the message failed after all same-path + flood retries are exhausted', async () => {
     const { manager, bridgeCalls } = makeManager();
+    const updated = vi.spyOn(dataEventEmitter, 'emitMeshCoreMessageUpdated');
 
     await manager.sendMessageWithResult('hello', PUBKEY);
-    await vi.advanceTimersByTimeAsync(10_000); // first timeout -> retry fires
-    await vi.advanceTimersByTimeAsync(10_000); // retry's own timeout -> should NOT retry again
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #1
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #2
+    await vi.advanceTimersByTimeAsync(10_000); // flood
+    await vi.advanceTimersByTimeAsync(10_000); // exhausted -> failed
+
+    // 1 initial + 2 same-path + 1 flood = 4 total, exactly one reset.
+    expect(sends(bridgeCalls)).toHaveLength(4);
+    expect(resets(bridgeCalls)).toHaveLength(1);
+
+    // A terminal 'failed' status was emitted for the single message.
+    const failedCalls = updated.mock.calls.filter(c => (c[0] as any).deliveryStatus === 'failed');
+    expect(failedCalls).toHaveLength(1);
+    expect((manager as any).pendingDmRetries.size).toBe(0);
 
-    expect(bridgeCalls.filter(c => c.cmd === 'reset_path')).toHaveLength(1);
-    expect(bridgeCalls.filter(c => c.cmd === 'send_message')).toHaveLength(2);
+    // No further sends after exhaustion.
+    await vi.advanceTimersByTimeAsync(30_000);
+    expect(sends(bridgeCalls)).toHaveLength(4);
   });
 
-  it('does not resend when reset_path itself fails', async () => {
-    const { manager, bridgeCalls } = makeManager({ resetPathOk: false });
+  it('keeps a SINGLE bubble: retries update the original message, never emit a new one', async () => {
+    const { manager } = makeManager();
+    const newMessage = vi.spyOn(dataEventEmitter, 'emitMeshCoreMessage');
+    const updated = vi.spyOn(dataEventEmitter, 'emitMeshCoreMessageUpdated');
 
     await manager.sendMessageWithResult('hello', PUBKEY);
-    await vi.advanceTimersByTimeAsync(10_000);
+    const originalId = (manager as any).messages[0].id;
+
+    // Exactly one new-message emit — the original send.
+    expect(newMessage).toHaveBeenCalledTimes(1);
+
+    await vi.advanceTimersByTimeAsync(10_000); // same-path retry #1
+
+    // Still only one new-message emit; the retry updated the original instead.
+    expect(newMessage).toHaveBeenCalledTimes(1);
+    // No second message row was persisted.
+    expect((manager as any).messages).toHaveLength(1);
 
-    expect(bridgeCalls.filter(c => c.cmd === 'reset_path')).toHaveLength(1);
-    // Only the original send — the resend never fires since reset_path failed.
-    expect(bridgeCalls.filter(c => c.cmd === 'send_message')).toHaveLength(1);
+    // The update re-points the SAME message id from CRC 111 -> 222.
+    const call = updated.mock.calls.at(-1)![0] as any;
+    expect(call.id).toBe(originalId);
+    expect(call.previousAckCrc).toBe(111);
+    expect(call.expectedAckCrc).toBe(222);
+    expect(call.deliveryStatus).toBe('sent');
+
+    // The in-memory message tracks the latest attempt's CRC.
+    expect((manager as any).messages[0].expectedAckCrc).toBe(222);
   });
 
-  it('cancels the retry if the ack for the flood resend arrives', async () => {
+  it('settles delivered (cancels the sequence) when a later attempt is acked', async () => {
     const { manager, bridgeCalls } = makeManager();
 
     await manager.sendMessageWithResult('hello', PUBKEY);
-    await vi.advanceTimersByTimeAsync(10_000); // first timeout -> retry fires with ackCrc=222
-    sendConfirmed(manager, 222);
-    await vi.advanceTimersByTimeAsync(10_000);
+    await vi.advanceTimersByTimeAsync(10_000); // same-path retry #1 -> CRC 222
+    sendConfirmed(manager, 222);               // ack for the retry
+    await vi.advanceTimersByTimeAsync(30_000); // no further retries should fire
+
+    expect(sends(bridgeCalls)).toHaveLength(2);
+    expect((manager as any).pendingDmRetries.size).toBe(0);
+  });
+
+  it('does not resend when reset_path itself fails (flood impossible)', async () => {
+    const { manager, bridgeCalls } = makeManager({ resetPathOk: false });
+    const updated = vi.spyOn(dataEventEmitter, 'emitMeshCoreMessageUpdated');
 
-    expect(bridgeCalls.filter(c => c.cmd === 'send_message')).toHaveLength(2);
+    await manager.sendMessageWithResult('hello', PUBKEY);
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #1
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #2
+    await vi.advanceTimersByTimeAsync(10_000); // flood attempt -> reset_path fails
+
+    expect(resets(bridgeCalls)).toHaveLength(1);
+    // 1 initial + 2 same-path only; the flood resend never fires.
+    expect(sends(bridgeCalls)).toHaveLength(3);
+    // The failed flood marks the message failed.
+    expect(updated.mock.calls.some(c => (c[0] as any).deliveryStatus === 'failed')).toBe(true);
   });
 
   it('does not schedule a retry for channel/broadcast sends', async () => {
     const { manager, bridgeCalls } = makeManager();
 
     await manager.sendMessageWithResult('hello', undefined, 0);
-    await vi.advanceTimersByTimeAsync(30_000);
+    await vi.advanceTimersByTimeAsync(40_000);
+
+    expect(sends(bridgeCalls)).toHaveLength(1);
+    expect(resets(bridgeCalls)).toHaveLength(0);
+    expect((manager as any).pendingDmRetries.size).toBe(0);
+  });
 
-    expect(bridgeCalls.filter(c => c.cmd === 'send_message')).toHaveLength(1);
-    expect(bridgeCalls.some(c => c.cmd === 'reset_path')).toBe(false);
+  it('learns/persists the new path when a flood ack arrives (contact_path_updated)', async () => {
+    const { manager, bridgeCalls } = makeManager();
+    // schedulePathRefresh is the existing entry point that re-reads and
+    // persists the firmware-learned route (debounced); assert the flood-ack
+    // push still drives it (path persistence "exactly like PathUpdated").
+    const refresh = vi.spyOn(manager as any, 'schedulePathRefresh').mockImplementation(() => {});
+
+    await manager.sendMessageWithResult('hello', PUBKEY);
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #1
+    await vi.advanceTimersByTimeAsync(10_000); // same-path #2
+    await vi.advanceTimersByTimeAsync(10_000); // flood (reset + resend)
+    expect(resets(bridgeCalls)).toHaveLength(1);
+
+    // Firmware auto-emits contact_path_updated after the flood ACK teaches the
+    // route; the existing handler persists it onto the contact.
+    // @ts-expect-error - exercising private handler
+    manager.handleBridgeEvent({
+      event_type: 'contact_path_updated',
+      data: { public_key: PUBKEY },
+    });
+
+    expect(refresh).toHaveBeenCalledWith(PUBKEY);
   });
 
-  it('clears pending retry timers on disconnect so a torn-down manager cannot retry', async () => {
+  it('clears pending retry timers on disconnect mid-sequence so a torn-down manager cannot retry', async () => {
     const { manager, bridgeCalls } = makeManager();
     vi.spyOn(manager as any, 'stopVirtualNodeServer').mockResolvedValue(undefined);
 
     await manager.sendMessageWithResult('hello', PUBKEY);
+    await vi.advanceTimersByTimeAsync(10_000); // one same-path retry, mid-sequence
     expect((manager as any).pendingDmRetries.size).toBe(1);
 
     await manager.disconnect();
     expect((manager as any).pendingDmRetries.size).toBe(0);
 
     const callsBefore = bridgeCalls.length;
-    await vi.advanceTimersByTimeAsync(30_000);
+    await vi.advanceTimersByTimeAsync(40_000);
     expect(bridgeCalls.length).toBe(callsBefore);
   });
 });