fix(readState): move lastPublishedContexts reset inside split→single guard

The reset was placed outside the extraSlotIds.length > 0 guard, causing
it to fire on every debounce cycle in steady-state single-slot mode. This
cleared lastPublishedContexts before isIdenticalToLastPublished, making
it always return false and reintroducing the original retry storm.

Move the reset inside the guard so it only fires on the split→single
transition. Also adds publishSplitSlots_noopSuppression_skipsWhenUnchanged
— a ReadStateManager integration test that stubs publishOneSlot to avoid
tauri and verifies the no-op suppression path in publishSplitSlots.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

Author: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7

desktop/src/features/channels/readState/readStateManager.test.mjs

@@ -2,13 +2,52 @@ import assert from "node:assert/strict";
 import test from "node:test";
 
 import {
+  ReadStateManager,
   applyRemoteContextTimestamp,
   evictDominatedEntries,
   resolveEffectiveTimestamp,
   splitContextsIntoBudgetedSlots,
   trimContextsToBudget,
 } from "./readStateManager.ts";
 
+// ── ReadStateManager integration helpers ─────────────────────────────────────
+// Provide browser globals required by ReadStateManager (localStorage,
+// window.setTimeout/clearTimeout). Each test that uses ReadStateManager
+// constructs a fresh in-memory store so tests are isolated.
+
+function makeLocalStorage() {
+  const store = new Map();
+  return {
+    getItem: (key) => store.get(key) ?? null,
+    setItem: (key, value) => store.set(key, value),
+    removeItem: (key) => store.delete(key),
+  };
+}
+
+// Install browser globals required by ReadStateManager. window.localStorage is
+// replaced per-test for isolation; the bare `localStorage` global proxies to it.
+{
+  const ls = makeLocalStorage();
+  if (typeof globalThis.window === "undefined") {
+    globalThis.window = {
+      localStorage: ls,
+      clearTimeout: (id) => clearTimeout(id),
+      setTimeout: (fn, ms) => setTimeout(fn, ms),
+    };
+  } else {
+    globalThis.window.localStorage = ls;
+    if (!globalThis.window.clearTimeout) {
+      globalThis.window.clearTimeout = (id) => clearTimeout(id);
+      globalThis.window.setTimeout = (fn, ms) => setTimeout(fn, ms);
+    }
+  }
+  // Ensure bare `localStorage` always proxies to window.localStorage.
+  Object.defineProperty(globalThis, "localStorage", {
+    get: () => globalThis.window.localStorage,
+    configurable: true,
+  });
+}
+
 const threadKey = `thread:${"a".repeat(64)}`;
 const channelKey = "channel-1";
 const channelResolver = (ctx) =>
@@ -572,3 +611,70 @@ test("splitContextsIntoBudgetedSlots_threadMsgTrimmedWhenPrimarySlotOverBudget",
   const size = blobSize(CLIENT_ID, result.slots[0]);
   assert.ok(size <= budget, `slot 0 size ${size} exceeds budget ${budget}`);
 });
+
+// ── ReadStateManager.publish — no-op suppression in split mode ────────────────
+
+// Verify that publishSplitSlots returns early (no relay writes) when the
+// union of all slot contexts is identical to lastPublishedContexts.
+//
+// Strategy: construct a ReadStateManager with enough channel keys to force
+// split mode, then mock publishOneSlot (private, accessed via bracket notation)
+// to avoid tauri calls while still simulating its effect on lastPublishedContexts.
+// Call publish() twice with the same effectiveState and assert that
+// publishOneSlot is called only on the first publish (no-op on the second).
+test("publishSplitSlots_noopSuppression_skipsWhenUnchanged", async () => {
+  // Isolate localStorage so slot IDs don't leak between tests.
+  globalThis.window.localStorage = makeLocalStorage();
+
+  const fakeRelay = {
+    fetchEvents: async () => [],
+    publishEvent: async () => {},
+    subscribeLive: () => () => {},
+  };
+
+  const pubkey = "b".repeat(64);
+  const mgr = new ReadStateManager(pubkey, fakeRelay);
+
+  // Add enough channel keys to exceed the 32KB single-slot budget.
+  // Each key is ~70 bytes in the blob; 700 keys ≈ 49KB > 32KB.
+  const ts = 1_000_000;
+  for (let i = 0; i < 700; i++) {
+    const channelId = `channel-${i.toString().padStart(64, "0")}`;
+    mgr.markContextRead(channelId, ts);
+  }
+
+  // Confirm split mode: currentContexts() must return null.
+  assert.equal(
+    mgr.currentContexts(),
+    null,
+    "precondition: 700 channel keys must exceed single-slot budget",
+  );
+
+  // Replace publishOneSlot with a stub that records calls and simulates the
+  // lastPublishedContexts merge (the only side-effect the no-op check depends
+  // on). This avoids tauri (nip44EncryptToSelf / signRelayEvent) while keeping
+  // the suppression logic under test.
+  let publishOneSlotCallCount = 0;
+  mgr.publishOneSlot = async (_slotId, contexts) => {
+    publishOneSlotCallCount++;
+    for (const [key, tsVal] of Object.entries(contexts)) {
+      mgr.lastPublishedContexts[key] = tsVal;
+    }
+  };
+
+  // First publish: contexts differ from lastPublishedContexts ({}) → must publish.
+  await mgr.publish();
+  const callsAfterFirst = publishOneSlotCallCount;
+  assert.ok(callsAfterFirst > 0, "first publish must call publishOneSlot");
+
+  // Second publish with identical effectiveState: union equals lastPublishedContexts
+  // → no-op suppression must fire → publishOneSlot must NOT be called again.
+  await mgr.publish();
+  assert.equal(
+    publishOneSlotCallCount,
+    callsAfterFirst,
+    "second publish with unchanged state must not call publishOneSlot (no-op suppression)",
+  );
+
+  mgr.destroy();
+});

desktop/src/features/channels/readState/readStateManager.ts

@@ -741,15 +741,16 @@ export class ReadStateManager {
 
     // Transitioning from split to single mode: delete stale extra-slot blobs
     // from the relay so fetchOwnBlobBeforePublish stops re-inflating
-    // lastPublishedContexts from them.
+    // lastPublishedContexts from them. Reset lastPublishedContexts here (inside
+    // the guard) so stale keys from the previous split don't cause
+    // isIdenticalToLastPublished to return false forever. The reset must stay
+    // inside the guard — resetting unconditionally would clear the relay-fetched
+    // state on every debounce cycle and reintroduce the retry storm.
     if (this.extraSlotIds.length > 0) {
       await this.deleteExtraSlots();
+      this.lastPublishedContexts = {};
     }
 
-    // Suppress no-op publishes. Reset lastPublishedContexts first so stale
-    // keys from a previous split don't cause isIdenticalToLastPublished to
-    // return false forever.
-    this.lastPublishedContexts = {};
     if (this.isIdenticalToLastPublished(contexts)) return;
 
     await this.publishOneSlot(this.slotId, contexts);