fix(sections-sync): guard storage handler against peer cache writes while pending, require version in sections parser

P1.1: Both whole-blob storage event handlers (sections + sort) replaced React
state unconditionally from the shared scoped cache, bypassing the
hasPendingEdit() guard that applyRemote uses for relay arrivals. A peer window
writing B1 while A has a pending edit (A1) caused the updater for A2 to derive
from B1, stranding A1's outbox record (Carl r8).

Fix: check hasPendingEdit() in both storage handlers; defer the peer cache
write until the pending edit's debounced publish-or-adopt cycle completes.
When nothing is pending the peer write applies immediately, preserving
cross-window state mirroring for the idle case.

P1.2: parseChannelSectionPayload did not check version; relay decryptAndParse
called it bare, so a future {version: 2} relay head parsed as v1 state instead
of triggering the retain/retry path. Sort, stars, and mutes all reject non-v1
in their parsers; sections was the odd one out.

Fix: add version !== 1 guard at the top of parseChannelSectionPayload. The
local-storage parse path (parseRaw) already checked version before calling the
parser, so no local-storage read is affected.

Tests: two new hook regression tests (sections + sort) for P1.1 using
StorageEvent dispatch; one new manager regression test and one new storage unit
test for P1.2; existing parser tests updated to include version: 1 in
previously version-less test fixtures.

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

desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs

@@ -62,13 +62,34 @@ test("parseChannelSectionPayload: non-object input returns null", () => {
   assert.equal(parseChannelSectionPayload(true), null);
 });
 
+test("parseChannelSectionPayload: unsupported version returns null (Carl P1.2)", () => {
+  // A future schema version must not be accepted as v1 state — must return null
+  // so the relay path triggers retain/retry rather than publishing over it.
+  assert.equal(
+    parseChannelSectionPayload({ version: 2, sections: [], assignments: {} }),
+    null,
+    "version 2 must be rejected",
+  );
+  assert.equal(
+    parseChannelSectionPayload({ sections: [], assignments: {} }),
+    null,
+    "missing version must be rejected",
+  );
+  assert.equal(
+    parseChannelSectionPayload({ version: 0, sections: [], assignments: {} }),
+    null,
+    "version 0 must be rejected",
+  );
+});
+
 test("parseChannelSectionPayload: missing sections returns empty sections array", () => {
-  const result = parseChannelSectionPayload({ assignments: {} });
+  const result = parseChannelSectionPayload({ version: 1, assignments: {} });
   assert.deepEqual(result?.sections, []);
 });
 
 test("parseChannelSectionPayload: malformed section entries are filtered out", () => {
   const payload = {
+    version: 1,
     sections: [
       { id: 123, name: "Bad ID", order: 0 },
       { id: "s1", name: 456, order: 0 },
@@ -84,6 +105,7 @@ test("parseChannelSectionPayload: malformed section entries are filtered out", (
 
 test("parseChannelSectionPayload: valid sections with some invalid ones filters correctly", () => {
   const payload = {
+    version: 1,
     sections: [
       { id: "s1", name: "Valid", order: 0 },
       { id: 99, name: "Bad ID", order: 1 },
@@ -99,12 +121,13 @@ test("parseChannelSectionPayload: valid sections with some invalid ones filters
 });
 
 test("parseChannelSectionPayload: missing assignments returns empty assignments object", () => {
-  const result = parseChannelSectionPayload({ sections: [] });
+  const result = parseChannelSectionPayload({ version: 1, sections: [] });
   assert.deepEqual(result?.assignments, {});
 });
 
 test("parseChannelSectionPayload: assignments with non-string values are filtered out", () => {
   const payload = {
+    version: 1,
     sections: [{ id: "s1", name: "Test", order: 0 }],
     assignments: { chan1: "s1", chan2: 42, chan3: null, chan4: true },
   };
@@ -114,6 +137,7 @@ test("parseChannelSectionPayload: assignments with non-string values are filtere
 
 test("parseChannelSectionPayload: orphaned assignments are stripped", () => {
   const payload = {
+    version: 1,
     sections: [{ id: "s1", name: "Exists", order: 0 }],
     assignments: { chan1: "s1", chan2: "missing-section" },
   };

desktop/src/features/sidebar/lib/channelSectionsStorage.ts

@@ -87,6 +87,13 @@ export function parseChannelSectionPayload(
 ): ChannelSectionStore | null {
   if (typeof json !== "object" || json === null) return null;
   const obj = json as Record<string, unknown>;
+  // Reject payloads from a future schema version we cannot safely interpret:
+  // a v2+ blob on the relay must trigger the retain/retry path so a later local
+  // edit never publishes over authoritative state this client does not understand
+  // (matches sort/stars/mutes parsers and Carl P1). Local-storage reads always
+  // go through `parseRaw`, which checks `version !== 1` before calling here, so
+  // this guard does not change the local-storage parse contract.
+  if (obj.version !== 1) return null;
   const sections: ChannelSection[] = Array.isArray(obj.sections)
     ? obj.sections.flatMap((entry: unknown): ChannelSection[] => {
         if (typeof entry !== "object" || entry === null) return [];

desktop/src/features/sidebar/lib/channelSectionsSync.test.mjs

@@ -441,6 +441,63 @@ test("malformed (non-JSON) head payload retains the pending edit, never publishi
   }
 });
 
+// Carl P1.2 regression (sections): an unsupported/future payload version
+// arriving from the relay must trigger the retain/retry path, never publish
+// over it. Sort, stars, and mutes all check `version !== 1` in their parsers;
+// sections was the odd one out. `parseChannelSectionPayload` now rejects
+// non-v1 payloads so `decryptAndParse` returns null and the manager retains.
+//
+// Mutation: removing `if (obj.version !== 1) return null` from
+// `parseChannelSectionPayload` causes the parser to accept the v2 blob as v1
+// state, `decryptAndParse` returns a non-null result, and the manager falls
+// through to a publish that overwrites authoritative state this client does not
+// understand.
+test("unsupported head payload version (sections) retains the pending edit, never publishing", async () => {
+  mock.method(relayClient, "fetchEvents", () =>
+    Promise.resolve([
+      {
+        pubkey: "pk-secver",
+        content: "good-cipher",
+        created_at: 500,
+        id: "evt-secver",
+      },
+    ]),
+  );
+  const publishCalls = [];
+  mock.method(relayClient, "publishEvent", (...args) => {
+    publishCalls.push(args);
+    return Promise.resolve();
+  });
+  const fw = makeFakeWindow();
+  const restore = installFakeWindow(fw);
+  // Decrypts cleanly but carries a future schema version the parser now rejects.
+  const tauri = installTauriMock(
+    JSON.stringify({ version: 2, sections: [], assignments: {} }),
+  );
+  try {
+    const manager = new ChannelSectionSyncManager("pk-secver", RELAY);
+    manager.publishSections(
+      makeSectionsStore([{ id: "s1", name: "Work", order: 0 }]),
+    );
+    fw._fireTimer();
+    await new Promise((r) => setTimeout(r, 20));
+    assert.equal(
+      publishCalls.length,
+      0,
+      "must not publish over a head whose payload version we do not support",
+    );
+    assert.ok(
+      manager.getPendingStore() !== null,
+      "unsupported head must retain the pending edit",
+    );
+    assert.ok(fw._hasTimer(), "a retry must be scheduled");
+  } finally {
+    tauri.restore();
+    restore();
+    mock.reset();
+  }
+});
+
 // ─── Failed pre-publish fetch: retain, never publish (Carl P1) ────────────────
 
 // The pre-publish fetch THROWS (timeout / auth / socket) — this is NOT proof

desktop/src/features/sidebar/lib/useChannelSections.test.mjs

@@ -77,6 +77,136 @@ test("assignChannel refreshes an existing assignment before the next eviction",
   }
 });
 
+// Carl P1.1 regression (sections): a peer-cache storage event arriving while a
+// local whole-blob edit is pending must NOT clobber the optimistic edit or
+// strand its outbox. The storage handler must defer to the pending edit just as
+// `applyRemote` does for relay arrivals. The pending edit's debounced publish
+// owns convergence and will publish-or-adopt against the relay.
+//
+// Scenario: window A creates section A1 (pending in outbox). Window B writes
+// B1 (a different store) to the shared cache. A's storage handler fires. Before
+// A's debounce fires, the user makes a second edit A2 inside the same debounce
+// window. The resulting store must be derived from A1's state (A2 added on top
+// of A1), and the outbox must still hold A's intent — not B1's shape.
+//
+// Mutation: removing the `hasPendingEdit()` guard in the storage handler causes
+// setStore to apply B1, so A2 is derived from B1 instead. The outbox record
+// still holds A's first edit shape, not the B1-derived A2 — an inconsistency
+// the manager cannot resolve. The test detects the replacement: if B1 were
+// applied, the final sections array would include B1's section id.
+test("storage event while a local edit is pending is deferred, not applied", async () => {
+  const { act, cleanup, renderHook } = await import("@testing-library/react");
+  const { relayClient } = await import("@/shared/api/relayClient");
+  const { useChannelSections } = await import("./useChannelSections.ts");
+  const { storageKey } = await import("./channelSectionsStorage.ts");
+
+  const origFetch = relayClient.fetchEvents;
+  const origLive = relayClient.subscribeLive;
+  const origReconnect = relayClient.subscribeToReconnects;
+  const origPublish = relayClient.publishEvent;
+  const origTauri = window.__TAURI_INTERNALS__;
+
+  // Block all publishes so the debounce never fires during the test.
+  relayClient.fetchEvents = async () => [];
+  relayClient.subscribeLive = async () => async () => {};
+  relayClient.subscribeToReconnects = () => () => {};
+  relayClient.publishEvent = async () => {};
+  window.__TAURI_INTERNALS__ = {
+    invoke: (cmd) => {
+      if (cmd === "nip44_encrypt_to_self") return Promise.resolve("ct");
+      if (cmd === "sign_event")
+        return Promise.resolve(
+          JSON.stringify({
+            id: "signed",
+            pubkey: "pk-storage-guard",
+            content: "ct",
+            created_at: 0,
+            kind: 30078,
+            tags: [],
+            sig: "s",
+          }),
+        );
+      return Promise.reject(new Error(`unmocked ${cmd}`));
+    },
+  };
+
+  const pubkey = "pk-storage-guard";
+  const relayUrl = "wss://r.storage-guard";
+  let hook = null;
+  try {
+    await act(async () => {
+      hook = renderHook(() => useChannelSections(pubkey, relayUrl));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // A1: window A creates a section — this becomes the pending edit.
+    await act(async () => {
+      hook.result.current.createSection("A1-Section");
+    });
+    const storeAfterA1 = hook.result.current.sections.map((s) => s.id);
+    assert.ok(
+      storeAfterA1.some((id) => id.length > 0),
+      "A1 section created",
+    );
+
+    // Window B writes B1 to the shared scoped cache key (simulates a peer
+    // window's edit landing in localStorage and firing the storage event).
+    const b1Store = JSON.stringify({
+      version: 1,
+      sections: [{ id: "b1-section", name: "B1", order: 0 }],
+      assignments: {},
+    });
+    await act(async () => {
+      window.localStorage.setItem(storageKey(pubkey, relayUrl), b1Store);
+      window.dispatchEvent(
+        new window.StorageEvent("storage", {
+          key: storageKey(pubkey, relayUrl),
+          newValue: b1Store,
+          storageArea: window.localStorage,
+        }),
+      );
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // B1 must NOT have replaced A1's optimistic state.
+    const storeAfterStorageEvent = hook.result.current.sections.map(
+      (s) => s.id,
+    );
+    assert.ok(
+      !storeAfterStorageEvent.includes("b1-section"),
+      "storage event while pending must not apply B1 to A's optimistic state",
+    );
+    assert.deepEqual(
+      storeAfterStorageEvent,
+      storeAfterA1,
+      "optimistic state must be unchanged after the deferred storage event",
+    );
+
+    // A2: user makes a second edit inside the debounce window. It is derived
+    // from A1's state (since B1 was deferred), so the result contains A1's
+    // section plus A2's.
+    await act(async () => {
+      hook.result.current.createSection("A2-Section");
+    });
+    const finalIds = hook.result.current.sections.map((s) => s.id);
+    assert.ok(
+      !finalIds.includes("b1-section"),
+      "A2 must be derived from A1 state, not B1 — b1-section must not appear",
+    );
+
+    hook.unmount();
+  } finally {
+    cleanup();
+    relayClient.fetchEvents = origFetch;
+    relayClient.subscribeLive = origLive;
+    relayClient.subscribeToReconnects = origReconnect;
+    relayClient.publishEvent = origPublish;
+    window.__TAURI_INTERNALS__ = origTauri;
+  }
+});
+
 // Fix 2 regression: a live remote arriving while a local edit is pending must
 // NOT overwrite the optimistic edit or strand its durable outbox. The pending
 // edit's own debounced publish owns convergence (publish-or-adopt). Reverting

desktop/src/features/sidebar/lib/useChannelSections.ts

@@ -82,6 +82,15 @@ export function useChannelSections(
       if (e.key !== key) {
         return;
       }
+      // A peer window wrote to the shared cache. Honour the same pending-aware
+      // guard that relay arrivals use in `applyRemote`: if a local whole-blob
+      // edit is in flight the manager owns convergence (its debounced
+      // publish-or-adopt is the single arbitration point). Applying a peer cache
+      // write unconditionally would clobber the optimistic edit and, on the next
+      // keystroke, strand the outbox record for the old intent (Carl P1).
+      // When nothing is pending the peer write applies immediately, preserving
+      // cross-window state mirroring for the idle case.
+      if (managerRef.current?.hasPendingEdit()) return;
       setStore(readChannelSectionsStore(pubkey, relayUrl));
     };
     window.addEventListener("storage", handler);

desktop/src/features/sidebar/lib/useChannelSortPreference.test.mjs

@@ -18,6 +18,124 @@ before(() => {
 
 after(() => dom.window.close());
 
+// Carl P1.1 regression (sort): a peer-cache storage event arriving while a
+// local whole-blob sort edit is pending must NOT clobber the optimistic edit.
+// The storage handler must defer to the pending edit just as `applyRemote` does
+// for relay arrivals. Peer writes arrive immediately when nothing is pending.
+//
+// Scenario: window A sets sort mode for "channels" (pending). Window B writes
+// B1 (sort mode "recent" for "channels") to the shared cache. A's storage event
+// fires. A2 is then set inside the debounce window. The resulting sort mode for
+// A must reflect A2, not B1's value — B1's write was ignored while A1 was live.
+//
+// Mutation: removing the `hasPendingEdit()` guard in the storage handler lets
+// B1 apply, so A2's `setStore` updater derives from B1, and the visible mode
+// reverts to B1's value.
+test("storage event while a local sort edit is pending is deferred, not applied", async () => {
+  const { act, cleanup, renderHook } = await import("@testing-library/react");
+  const { relayClient } = await import("@/shared/api/relayClient");
+  const { useChannelSortPreference } = await import(
+    "./useChannelSortPreference.ts"
+  );
+  const { storageKey } = await import("./channelSortPreference.ts");
+
+  const origFetch = relayClient.fetchEvents;
+  const origLive = relayClient.subscribeLive;
+  const origReconnect = relayClient.subscribeToReconnects;
+  const origPublish = relayClient.publishEvent;
+  const origTauri = window.__TAURI_INTERNALS__;
+
+  relayClient.fetchEvents = async () => [];
+  relayClient.subscribeLive = async () => async () => {};
+  relayClient.subscribeToReconnects = () => () => {};
+  relayClient.publishEvent = async () => {};
+  window.__TAURI_INTERNALS__ = {
+    invoke: (cmd) => {
+      if (cmd === "nip44_encrypt_to_self") return Promise.resolve("ct");
+      if (cmd === "sign_event")
+        return Promise.resolve(
+          JSON.stringify({
+            id: "signed",
+            pubkey: "pk-sort-sg",
+            content: "ct",
+            created_at: 0,
+            kind: 30078,
+            tags: [],
+            sig: "s",
+          }),
+        );
+      return Promise.reject(new Error(`unmocked ${cmd}`));
+    },
+  };
+
+  const pubkey = "pk-sort-sg";
+  const relayUrl = "wss://r.sort-sg";
+  let hook = null;
+  try {
+    await act(async () => {
+      hook = renderHook(() => useChannelSortPreference(pubkey, relayUrl));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // A1: set "channels" to "alpha" — becomes the pending edit.
+    await act(async () => {
+      hook.result.current.setSortModeFor("channels", "alpha");
+    });
+    assert.equal(
+      hook.result.current.sortModeFor("channels"),
+      "alpha",
+      "A1: channels sort set to alpha",
+    );
+
+    // Window B writes "recent" for "channels" to the shared cache and fires a
+    // storage event.
+    const b1Store = JSON.stringify({
+      version: 1,
+      groups: { channels: "recent" },
+    });
+    await act(async () => {
+      window.localStorage.setItem(storageKey(pubkey, relayUrl), b1Store);
+      window.dispatchEvent(
+        new window.StorageEvent("storage", {
+          key: storageKey(pubkey, relayUrl),
+          newValue: b1Store,
+          storageArea: window.localStorage,
+        }),
+      );
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // B1's "recent" must NOT have replaced A1's "alpha".
+    assert.equal(
+      hook.result.current.sortModeFor("channels"),
+      "alpha",
+      "storage event while pending must not apply B1 (recent) over A1 (alpha)",
+    );
+
+    // A2: set "starred" to "recent" — derived from A1 state (channels stays alpha).
+    await act(async () => {
+      hook.result.current.setSortModeFor("starred", "recent");
+    });
+    // channels must still reflect A1 (alpha), not B1 (recent).
+    assert.equal(
+      hook.result.current.sortModeFor("channels"),
+      "alpha",
+      "A2 must be derived from A1 state — channels must remain alpha, not revert to B1 recent",
+    );
+
+    hook.unmount();
+  } finally {
+    cleanup();
+    relayClient.fetchEvents = origFetch;
+    relayClient.subscribeLive = origLive;
+    relayClient.subscribeToReconnects = origReconnect;
+    relayClient.publishEvent = origPublish;
+    window.__TAURI_INTERNALS__ = origTauri;
+  }
+});
+
 // Carl P1-sort regression: a live remote arriving while a local sort edit is
 // still pending must NOT overwrite the optimistic edit or strand its durable
 // outbox. Reverting applyRemote's `hasPendingEdit()` guard (or restoring the

desktop/src/features/sidebar/lib/useChannelSortPreference.ts

@@ -82,6 +82,15 @@ export function useChannelSortPreference(
     const key = storageKey(pubkey, relayUrl);
     const handler = (e: StorageEvent) => {
       if (e.key !== key) return;
+      // A peer window wrote to the shared cache. Honour the same pending-aware
+      // guard that relay arrivals use in `applyRemote`: if a local whole-blob
+      // edit is in flight the manager owns convergence (its debounced
+      // publish-or-adopt is the single arbitration point). Applying a peer cache
+      // write unconditionally would clobber the optimistic edit and, on the next
+      // keystroke, strand the outbox record for the old intent (Carl P1).
+      // When nothing is pending the peer write applies immediately, preserving
+      // cross-window state mirroring for the idle case.
+      if (managerRef.current?.hasPendingEdit()) return;
       setStore(readChannelSortStore(pubkey, relayUrl));
     };
     window.addEventListener("storage", handler);