fix(chat): keep Meshtastic channel selection across reconnect

src/renderer/components/ChatPanel.tsx

@@ -87,13 +87,15 @@ import { playMessageNotification } from '../lib/chatNotifications';
 import {
   dismissedDmTabsStorageKey,
   lastReadStorageKey,
+  loadActiveChannelInitial,
   loadActiveDmInitial,
   loadMutedViews,
   loadOpenDmTabsInitial,
   loadPersistedLastReadInitial,
   loadStarred,
   notifyPersistedLastReadChanged,
   openDmTabsStorageKey,
+  saveActiveChannel,
   saveActiveDm,
   saveMutedViews,
   saveStarred,
@@ -655,12 +657,215 @@ function ChatPanel({
   }, []);
 
   useImperativeHandle(scrollToTopRef, () => scrollToTop, [scrollToTop]);
-  const [channel, setChannel] = useState(() => (channels.length > 0 ? channels[0].index : 0));
+  const defaultChannelIndex = channels.length > 0 ? channels[0].index : 0;
+  const [channel, setChannel] = useState(() => {
+    const persisted = myNodeNum > 0 ? loadActiveChannelInitial(protocol, myNodeNum) : null;
+    if (persisted != null && channels.some((c) => c.index === persisted)) return persisted;
+    return defaultChannelIndex;
+  });
   useEffect(() => {
     if (channels.length > 0 && !channels.some((c) => c.index === channel)) {
-      setChannel(channels[0].index);
+      setChannel(defaultChannelIndex);
     }
-  }, [channels, channel]);
+  }, [channels, channel, defaultChannelIndex]);
+  /**
+   * ChatPanel mounts once per protocol tab and often before the radio finishes
+   * connecting, so `myNodeNum` can still be 0 (no restore attempted) at the lazy
+   * initializer above. Re-attempt the restore once a real node number is known —
+   * covers both "connected after mount" and "switched to a different node while
+   * this panel stayed mounted". Scoped by protocol + node (not node alone): a
+   * protocol switch remounts ChatPanel (App.tsx keys it on protocol) so this is
+   * belt-and-suspenders, but costs nothing.
+   */
+  const channelRestoreScopeKey = myNodeNum > 0 ? `${protocol}:${myNodeNum}` : null;
+  interface ChannelRestoreState {
+    scope: string | null;
+    resolved: boolean;
+    indexSignature: string | null;
+    /**
+     * Whether this scope was ever entered from a genuinely *different* prior
+     * scope — captured once when a scope is first seen and carried forward
+     * unchanged through every later re-check of that same scope (recomputing
+     * "did the scope just change" fresh on each pending retry reads false
+     * once we've already been pending on it for a tick, which is exactly
+     * when the leak-prevention reset below is needed most: the moment
+     * restoration gives up on a stale value, `channel` can still be the
+     * *previous* scope's leftover selection).
+     */
+    arrivedFromDifferentScope: boolean;
+  }
+  /**
+   * `resolved: false` means restoration for `scope` hasn't been settled yet —
+   * either a saved value exists for it but the current `channels` list hasn't
+   * confirmed it (e.g. still showing a *previous* node's stale, carried-forward
+   * list — see useMeshtasticRuntime's lastKnownChannelsRef), or `myNodeNum` is
+   * still 0. Saving is suppressed the whole time a scope is unresolved: a saved
+   * value not yet found in a stale list is NOT the same as "no saved value" —
+   * treating them the same let a real save get clobbered by the default the
+   * moment a different node's carried-forward list didn't happen to contain it.
+   * `indexSignature` is the set of indices `channels` had on the last pending
+   * check for this scope, used only to detect the list has *stopped* changing
+   * (see below) — never to decide whether the saved value matches.
+   *
+   * The initial value mirrors the `channel` lazy initializer above rather than
+   * assuming "resolved" outright: if `myNodeNum` is already known at mount but
+   * `channels` hasn't arrived yet (a normal race — they come from separate
+   * packets), a saved value can't be found on that first render either, and
+   * marking it resolved unconditionally would permanently skip ever retrying
+   * once the real list arrives with the match.
+   *
+   * Computed via an "initialized" guard rather than directly as `useRef`'s
+   * argument — that argument is evaluated on every render even though only
+   * the first one is ever used, and ChatPanel re-renders often (every
+   * message, every scroll update), which would repeat the localStorage read
+   * below on every one of those renders for no reason.
+   */
+  const channelRestoreInitializedRef = useRef(false);
+  const channelRestoreRef = useRef<ChannelRestoreState>({
+    scope: null,
+    resolved: true,
+    indexSignature: null,
+    arrivedFromDifferentScope: false,
+  });
+  if (!channelRestoreInitializedRef.current) {
+    channelRestoreInitializedRef.current = true;
+    if (channelRestoreScopeKey != null) {
+      const persisted = loadActiveChannelInitial(protocol, myNodeNum);
+      channelRestoreRef.current =
+        persisted == null || channels.some((c) => c.index === persisted)
+          ? {
+              scope: channelRestoreScopeKey,
+              resolved: true,
+              indexSignature: null,
+              arrivedFromDifferentScope: false,
+            }
+          : // `indexSignature: null` (not the real, computed signature) —
+            // otherwise the very first restore-effect run right after mount
+            // would compare against this same unchanged snapshot, see a
+            // "match" on indexSignature alone, and give up immediately
+            // before `channels` ever gets a chance to actually update.
+            // Stability can only be concluded by comparing two *effect*
+            // observations, never the initializer's own snapshot against
+            // itself.
+            {
+              scope: channelRestoreScopeKey,
+              resolved: false,
+              indexSignature: null,
+              arrivedFromDifferentScope: false,
+            };
+    }
+  }
+  /** True for the one save-effect run right after a restore/reset-triggered setChannel,
+   * so that run doesn't persist the pre-transition value it hasn't caught up to yet. */
+  const skipNextChannelSaveRef = useRef(false);
+  useEffect(() => {
+    if (channelRestoreScopeKey == null) return;
+    const prior = channelRestoreRef.current;
+    if (prior.scope === channelRestoreScopeKey && prior.resolved) return; // already settled
+    const arrivedFromDifferentScope =
+      prior.scope === channelRestoreScopeKey
+        ? prior.arrivedFromDifferentScope
+        : prior.scope !== null;
+
+    const persisted = loadActiveChannelInitial(protocol, myNodeNum);
+    if (persisted != null && channels.some((c) => c.index === persisted)) {
+      channelRestoreRef.current = {
+        scope: channelRestoreScopeKey,
+        resolved: true,
+        indexSignature: null,
+        arrivedFromDifferentScope,
+      };
+      if (persisted !== channel) {
+        skipNextChannelSaveRef.current = true;
+        setChannel(persisted);
+      }
+      return;
+    }
+    if (persisted != null) {
+      // A value IS saved for this scope, but `channels` doesn't contain it.
+      // Could mean the list hasn't finished arriving yet (stay pending, keep
+      // saving suppressed, re-check next `channels` change) — OR the channel
+      // was genuinely removed from this node's config since it was saved, in
+      // which case the list will stop changing and we must NOT wait forever:
+      // that would silently disable saving *any* future selection for this
+      // node for the rest of the session. Give up once the set of available
+      // indices is identical to the last pending check (content-stable, not
+      // just a new array reference).
+      const indexSignature = channels.map((c) => c.index).join(',');
+      if (prior.scope === channelRestoreScopeKey && prior.indexSignature === indexSignature) {
+        channelRestoreRef.current = {
+          scope: channelRestoreScopeKey,
+          resolved: true,
+          indexSignature: null,
+          arrivedFromDifferentScope,
+        };
+        // Fall through to the "nothing to restore" handling below — same
+        // outcome as if nothing had ever been saved for this scope.
+      } else {
+        channelRestoreRef.current = {
+          scope: channelRestoreScopeKey,
+          resolved: false,
+          indexSignature,
+          arrivedFromDifferentScope,
+        };
+        return;
+      }
+    } else {
+      channelRestoreRef.current = {
+        scope: channelRestoreScopeKey,
+        resolved: true,
+        indexSignature: null,
+        arrivedFromDifferentScope,
+      };
+    }
+    // Nothing to restore (never saved, or saved but genuinely gone). If this
+    // scope was ever arrived at from a *different* scope, `channel` may still
+    // hold that other scope's index and `channels` may still be showing its
+    // stale, carried-forward list — don't let that leak into this scope's
+    // saved preference. Force back to the default explicitly; the
+    // pre-existing clamp effect above can't catch this because the stale
+    // index is still "valid" against the stale list.
+    if (arrivedFromDifferentScope && defaultChannelIndex !== channel) {
+      skipNextChannelSaveRef.current = true;
+      setChannel(defaultChannelIndex);
+    }
+  }, [protocol, myNodeNum, channelRestoreScopeKey, channels, channel, defaultChannelIndex]);
+  useEffect(() => {
+    if (channelRestoreScopeKey == null) return;
+    const status = channelRestoreRef.current;
+    if (status.scope !== channelRestoreScopeKey || !status.resolved) return; // still pending
+    if (skipNextChannelSaveRef.current) {
+      skipNextChannelSaveRef.current = false;
+      return;
+    }
+    // Only persist a selection the current channel list actually has — never a
+    // momentarily-invalid index from a channel list that just shrank (the clamp
+    // effect above will correct `channel` next render; this run simply skips).
+    if (!channels.some((c) => c.index === channel)) return;
+    saveActiveChannel(protocol, myNodeNum, channel);
+  }, [protocol, myNodeNum, channelRestoreScopeKey, channel, channels]);
+  /**
+   * Deliberate, user-initiated channel selection. Immediately marks this
+   * scope's restore as resolved — a race otherwise exists where a restore is
+   * still pending (waiting for a saved value to show up in `channels`) when
+   * the user manually picks a *different* channel; if the saved value later
+   * matches, the pending restore would fire and silently overwrite the user's
+   * manual pick. A deliberate selection always wins and unblocks saving.
+   */
+  const selectChannel = useCallback(
+    (index: number) => {
+      if (channelRestoreScopeKey != null) {
+        channelRestoreRef.current = {
+          scope: channelRestoreScopeKey,
+          resolved: true,
+          indexSignature: null,
+          arrivedFromDifferentScope: false,
+        };
+      }
+      setChannel(index);
+    },
+    [channelRestoreScopeKey],
+  );
   const [chatActionError, setChatActionError] = useState<{
     message: string;
     viewKey: string;
@@ -2015,7 +2220,7 @@ function ChatPanel({
                     key={`ch-${ch.index}-${chIdx}-${ch.name}`}
                     aria-label={`${ch.name}${channelUnreadSuffix}`}
                     onClick={() => {
-                      setChannel(ch.index);
+                      selectChannel(ch.index);
                       setViewMode('channels');
                     }}
                     className={`relative shrink-0 rounded-full px-3 py-1 text-xs font-medium transition-colors ${
@@ -2491,7 +2696,7 @@ function ChatPanel({
                             if (type === 'dm' && raw) {
                               openDmTo(Number(raw));
                             } else {
-                              if (raw !== undefined) setChannel(Number(raw));
+                              if (raw !== undefined) selectChannel(Number(raw));
                               setViewMode('channels');
                             }
                           }}

src/renderer/lib/chatPanelProtocolStorage.test.ts

@@ -1,6 +1,7 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import {
+  activeChannelStorageKey,
   activeDmStorageKey,
   clearDraft,
   clearFloodScopeOverride,
@@ -11,6 +12,7 @@ import {
   floodScopeOverridesStorageKey,
   getSanitizedMeshtasticChatLastRead,
   lastReadStorageKey,
+  loadActiveChannelInitial,
   loadActiveDmInitial,
   loadDraftsInitial,
   loadFloodScopeOverridesInitial,
@@ -26,6 +28,7 @@ import {
   sanitizeMeshcoreRoomsLastRead,
   sanitizeMeshtasticChatLastRead,
   sanitizeReticulumChatLastRead,
+  saveActiveChannel,
   saveActiveDm,
   saveDraft,
   saveFloodScopeOverride,
@@ -70,6 +73,54 @@ describe('chatPanelProtocolStorage', () => {
     expect(loadActiveDmInitial('reticulum')).toBeNull();
   });
 
+  it('persists and loads last-selected channel per protocol + node number', () => {
+    expect(loadActiveChannelInitial('meshtastic', 0x12345678)).toBeNull();
+    saveActiveChannel('meshtastic', 0x12345678, 3);
+    expect(localStorage.getItem(activeChannelStorageKey('meshtastic', 0x12345678))).toBe('3');
+    expect(loadActiveChannelInitial('meshtastic', 0x12345678)).toBe(3);
+
+    // Different node number (e.g. connected to a different physical device that
+    // happens to reuse the same internal identity slot) must not see it.
+    expect(loadActiveChannelInitial('meshtastic', 0x87654321)).toBeNull();
+    // Different protocol must not see it either.
+    expect(loadActiveChannelInitial('meshcore', 0x12345678)).toBeNull();
+  });
+
+  it('ignores invalid inputs for active-channel persistence', () => {
+    saveActiveChannel('meshtastic', 0, 3); // no node number yet — no-op
+    expect(loadActiveChannelInitial('meshtastic', 0)).toBeNull();
+
+    localStorage.setItem(activeChannelStorageKey('meshtastic', 5), 'not-a-number');
+    expect(loadActiveChannelInitial('meshtastic', 5)).toBeNull();
+
+    saveActiveChannel('meshtastic', 5, NaN);
+    expect(loadActiveChannelInitial('meshtastic', 5)).toBeNull();
+    saveActiveChannel('meshtastic', 5, 1.5);
+    expect(loadActiveChannelInitial('meshtastic', 5)).toBeNull();
+    saveActiveChannel('meshtastic', 5, -2); // below the -1 sentinel floor
+    expect(loadActiveChannelInitial('meshtastic', 5)).toBeNull();
+  });
+
+  it('rejects malformed node numbers and whitespace-only stored values', () => {
+    // Fractional/infinite node numbers must not read or write a key at all.
+    saveActiveChannel('meshtastic', 1.5, 3);
+    expect(loadActiveChannelInitial('meshtastic', 1.5)).toBeNull();
+    saveActiveChannel('meshtastic', Infinity, 3);
+    expect(loadActiveChannelInitial('meshtastic', Infinity)).toBeNull();
+    expect(loadActiveChannelInitial('meshtastic', NaN)).toBeNull();
+
+    // A whitespace-only stored value must not silently parse as channel 0
+    // (Number(' ') === 0 in JS).
+    localStorage.setItem(activeChannelStorageKey('meshtastic', 9), '   ');
+    expect(loadActiveChannelInitial('meshtastic', 9)).toBeNull();
+  });
+
+  it('persists and loads the MeshCore primary-channel sentinel (-1)', () => {
+    saveActiveChannel('meshcore', 7, -1);
+    expect(localStorage.getItem(activeChannelStorageKey('meshcore', 7))).toBe('-1');
+    expect(loadActiveChannelInitial('meshcore', 7)).toBe(-1);
+  });
+
   it('migrates legacy lastRead only into meshtastic key', () => {
     localStorage.setItem('mesh-client:lastRead', JSON.stringify({ 'ch:0': 1 }));
     const mt = loadPersistedLastReadInitial('meshtastic');

src/renderer/lib/chatPanelProtocolStorage.ts

@@ -121,6 +121,64 @@ export function saveActiveDm(protocol: MeshProtocol, nodeId: number | null): voi
   }
 }
 
+/**
+ * Scoped by the connected device's own node number (not just protocol) so that
+ * switching to a genuinely different physical node — which can reuse the same
+ * internal identity slot as the node just disconnected from — doesn't restore a
+ * channel selection that belonged to the previous device. A same-node
+ * disconnect/reconnect keeps the same node number, so the prior selection is
+ * still found and restored; a different node simply misses and falls back to
+ * the default channel.
+ */
+export function activeChannelStorageKey(protocol: MeshProtocol, nodeNum: number): string {
+  return `mesh-client:activeChannel:${protocol}:${nodeNum}`;
+}
+
+/**
+ * ChatPanel uses channel index -1 as the "primary" sentinel (MeshCore); every
+ * other valid index is >= 0. Reject anything else, including other negatives.
+ */
+function isValidChannelIndex(value: number): boolean {
+  return Number.isFinite(value) && Number.isInteger(value) && value >= -1;
+}
+
+/** Node numbers are always positive integers; reject fractional/infinite input. */
+function isValidNodeNum(value: number): boolean {
+  return Number.isSafeInteger(value) && value > 0;
+}
+
+/** Last-selected channel index for this protocol + node (null if missing/invalid). */
+export function loadActiveChannelInitial(protocol: MeshProtocol, nodeNum: number): number | null {
+  if (!isValidNodeNum(nodeNum)) return null;
+  try {
+    const raw = localStorage.getItem(activeChannelStorageKey(protocol, nodeNum));
+    if (raw == null || raw.trim() === '') return null;
+    const parsed = Number(raw);
+    if (!isValidChannelIndex(parsed)) return null;
+    return parsed;
+  } catch (e) {
+    console.debug(
+      '[chatPanelProtocolStorage] loadActiveChannelInitial failed ' + errLikeToLogString(e),
+    );
+    return null;
+  }
+}
+
+/** Persist last-selected channel index for this protocol + node. No-op without a node number. */
+export function saveActiveChannel(
+  protocol: MeshProtocol,
+  nodeNum: number,
+  channelIndex: number,
+): void {
+  if (!isValidNodeNum(nodeNum)) return;
+  if (!isValidChannelIndex(channelIndex)) return;
+  try {
+    localStorage.setItem(activeChannelStorageKey(protocol, nodeNum), String(channelIndex));
+  } catch (e) {
+    console.debug('[chatPanelProtocolStorage] saveActiveChannel failed ' + errLikeToLogString(e));
+  }
+}
+
 export function draftsStorageKey(protocol: MeshProtocol): string {
   return `mesh-client:drafts:${protocol}`;
 }