[sync] Unify wallet + storage sync UI: one confirm, consistent notifications (step 2)

ProfileWalletController and ProfileStorageController each had their own copy of
the sync toggle/restore/overwrite-confirm logic, producing different notifications
and duplicate confirm dialogs. Extract js/ui/profileModal/syncSection.js — one
overwrite-confirm dialog, one status-class mapping, and shared runSyncToggle /
runSyncRestore handlers. Both controllers now delegate, so every syncable item
gets identical toasts + one confirm (conflict-only, via the shared syncConflict).

Intentional divergence preserved: the NWC spending-secret warning stays as the
wallet's preEnableConfirm (a wallet-connect URI can spend), since publishing it
even encrypted warrants a one-time heads-up.

Tests: tests/sync-section.test.mjs (8) — enable/conflict/nothing-to-sync/rollback/
preEnableConfirm/disable/restore, plus wallet-vs-storage produce the same message
shapes. wallet-sync + storage-sync + profile-modal suites green (34).

Co-Authored-By: Claude Opus 4.8 <[email protected]>

js/ui/profileModal/ProfileStorageController.js

@@ -10,7 +10,14 @@ import { getActiveSigner } from "../../nostr/client.js";
 import { DEFAULT_NIP07_ENCRYPTION_METHODS } from "../../nostr/nip07Permissions.js";
 import { storageSyncService } from "../../services/storageSyncService.js";
 import { StorageCorsHelp } from "./storageCorsHelp.js";
-import { showConfirm } from "../confirmDialog.js";
+import {
+  syncStatusClass,
+  runSyncToggle,
+  runSyncRestore,
+  confirmSyncOverwrite,
+} from "./syncSection.js";
+
+const STORAGE_SYNC_LABEL = "storage settings";
 import {
   fillStorageForm,
   clearCredentialFields,
@@ -241,14 +248,6 @@ export class ProfileStorageController {
     this.updateStorageFormVisibility();
   }
 
-  confirmSyncOverwrite() {
-    return showConfirm(
-      "A newer copy of your storage settings is on your account (changed on " +
-        "another device). Overwrite it with this one?",
-      { confirmLabel: "Overwrite", danger: true },
-    );
-  }
-
   renderSyncSection(pubkey) {
     if (!(this.storageSyncSection instanceof HTMLElement)) {
       return;
@@ -271,13 +270,7 @@ export class ProfileStorageController {
       return;
     }
     this.storageSyncStatus.textContent = message || "";
-    const toneClass =
-      tone === "success"
-        ? "text-status-success"
-        : tone === "error"
-          ? "text-status-danger"
-          : "text-muted";
-    this.storageSyncStatus.className = `text-xs ${toneClass}`;
+    this.storageSyncStatus.className = syncStatusClass(tone);
   }
 
   async handleToggleSync() {
@@ -292,41 +285,20 @@ export class ProfileStorageController {
         ? this.storageSyncToggle.checked
         : false;
     try {
-      if (enabled) {
-        this.setSyncStatus("Encrypting and publishing…");
-        const result = await storageSyncService.enable(pubkey, {
-          confirmOverwrite: () => this.confirmSyncOverwrite(),
-        });
-        if (result?.ok) {
-          this.setSyncStatus(
-            `Synced to ${result.accepted}/${result.total} relays.`,
-            "success"
-          );
-          this.mainController.showSuccess("Storage settings synced (encrypted).");
-        } else if (result?.conflict) {
-          // User declined to overwrite a newer copy from another device.
-          this.setSyncStatus(
-            "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.",
-          );
-        } else {
-          // Roll the toggle back so it reflects reality.
+      await runSyncToggle({
+        service: storageSyncService,
+        pubkey,
+        enabled,
+        itemLabel: STORAGE_SYNC_LABEL,
+        emptyHint: "Nothing to sync yet — save a connection first.",
+        setStatus: (msg, tone) => this.setSyncStatus(msg, tone),
+        showSuccess: (msg) => this.mainController.showSuccess(msg),
+        setToggle: (value) => {
           if (this.storageSyncToggle instanceof HTMLInputElement) {
-            this.storageSyncToggle.checked = false;
+            this.storageSyncToggle.checked = value;
           }
-          await storageSyncService.disable(pubkey).catch(() => {});
-          this.setSyncStatus(
-            result?.error === "nothing-to-sync"
-              ? "Nothing to sync yet — save a connection first."
-              : "Could not publish the encrypted copy. Try again.",
-            "error"
-          );
-        }
-      } else {
-        this.setSyncStatus("Removing the synced copy…");
-        await storageSyncService.disable(pubkey);
-        this.setSyncStatus("Sync turned off; the synced copy was cleared.");
-        this.mainController.showSuccess("Storage sync turned off.");
-      }
+        },
+      });
     } catch (error) {
       devLogger.error("[ProfileModal] Storage sync toggle failed:", error);
       this.setSyncStatus("Sync failed. Please try again.", "error");
@@ -341,20 +313,15 @@ export class ProfileStorageController {
       return;
     }
     try {
-      this.setSyncStatus("Fetching and decrypting…");
-      const result = await storageSyncService.pull(pubkey);
-      if (result?.found && result.imported) {
-        this.setSyncStatus("Restored from your Nostr account.", "success");
-        this.mainController.showSuccess("Storage settings restored.");
+      await runSyncRestore({
+        service: storageSyncService,
+        pubkey,
+        itemLabel: STORAGE_SYNC_LABEL,
+        setStatus: (msg, tone) => this.setSyncStatus(msg, tone),
+        showSuccess: (msg) => this.mainController.showSuccess(msg),
         // Re-render the pane so the restored connection shows.
-        await this.populateStoragePane();
-      } else if (result?.found && !result.imported) {
-        this.setSyncStatus("Found a copy but could not import it.", "error");
-      } else if (result?.cleared) {
-        this.setSyncStatus("No synced settings found (it was cleared).");
-      } else {
-        this.setSyncStatus("No synced settings found on your account.");
-      }
+        onImported: () => this.populateStoragePane(),
+      });
     } catch (error) {
       devLogger.error("[ProfileModal] Storage sync restore failed:", error);
       this.setSyncStatus("Restore failed. Please try again.", "error");
@@ -861,7 +828,7 @@ export class ProfileStorageController {
       if (storageSyncService.isEnabled(pubkey)) {
         try {
           const syncResult = await storageSyncService.push(pubkey, {
-            confirmOverwrite: () => this.confirmSyncOverwrite(),
+            confirmOverwrite: () => confirmSyncOverwrite(STORAGE_SYNC_LABEL),
           });
           if (syncResult?.ok) {
             this.setSyncStatus("Synced copy updated.", "success");

js/ui/profileModal/ProfileWalletController.js

@@ -3,6 +3,14 @@ import { showConfirm } from "../confirmDialog.js";
 import { NWC_URI_SCHEME } from "../profileModalContract.js";
 import { createWalletSyncService } from "../../services/walletSyncService.js";
 import { runWalletZappabilityCheck } from "./walletZappabilityCheck.js";
+import {
+  syncStatusClass,
+  runSyncToggle,
+  runSyncRestore,
+  confirmSyncOverwrite,
+} from "./syncSection.js";
+
+const WALLET_SYNC_LABEL = "wallet connection";
 
 const noop = () => {};
 const SECRET_PLACEHOLDER = "*****";
@@ -104,14 +112,6 @@ export class ProfileWalletController {
     return this._walletSyncService;
   }
 
-  confirmSyncOverwrite() {
-    return showConfirm(
-      "A newer copy of your wallet connection is on your account (changed on " +
-        "another device). Overwrite it with this one?",
-      { confirmLabel: "Overwrite", danger: true },
-    );
-  }
-
   renderSyncSection(pubkey) {
     if (!(this.walletSyncSection instanceof HTMLElement)) {
       return;
@@ -133,13 +133,7 @@ export class ProfileWalletController {
       return;
     }
     this.walletSyncStatus.textContent = message || "";
-    const toneClass =
-      tone === "success"
-        ? "text-status-success"
-        : tone === "error"
-          ? "text-status-danger"
-          : "text-muted";
-    this.walletSyncStatus.className = `text-xs ${toneClass}`;
+    this.walletSyncStatus.className = syncStatusClass(tone);
   }
 
   async handleToggleSync() {
@@ -155,54 +149,29 @@ export class ProfileWalletController {
         ? this.walletSyncToggle.checked
         : false;
     try {
-      if (enabled) {
-        // Spending capability — require explicit confirmation before publishing.
-        const confirmed = await showConfirm(
-          "This publishes an ENCRYPTED copy of your wallet connection to public relays.\n\n" +
-            "A wallet connect URI can SPEND from your wallet. Anyone who controls your " +
-            "Nostr key could decrypt this and spend from this wallet.\n\nContinue?",
-          { confirmLabel: "Publish", danger: true },
-        );
-        if (!confirmed) {
+      await runSyncToggle({
+        service: sync,
+        pubkey,
+        enabled,
+        itemLabel: WALLET_SYNC_LABEL,
+        emptyHint: "Connect a wallet first, then enable sync.",
+        setStatus: (msg, tone) => this.setSyncStatus(msg, tone),
+        showSuccess: (msg) => this.mainController.showSuccess(msg),
+        setToggle: (value) => {
           if (this.walletSyncToggle instanceof HTMLInputElement) {
-            this.walletSyncToggle.checked = false;
+            this.walletSyncToggle.checked = value;
           }
-          return;
-        }
-        this.setSyncStatus("Encrypting and publishing…");
-        const result = await sync.enable(pubkey, {
-          confirmOverwrite: () => this.confirmSyncOverwrite(),
-        });
-        if (result?.ok) {
-          this.setSyncStatus(
-            `Synced to ${result.accepted}/${result.total} relays.`,
-            "success"
-          );
-          this.mainController.showSuccess("Wallet connection synced (encrypted).");
-        } else if (result?.conflict) {
-          // User declined to overwrite a newer copy from another device. Keep
-          // sync on (flag set) and the newer remote intact.
-          this.setSyncStatus(
-            "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.",
-          );
-        } else {
-          if (this.walletSyncToggle instanceof HTMLInputElement) {
-            this.walletSyncToggle.checked = false;
-          }
-          await sync.disable(pubkey).catch(() => {});
-          this.setSyncStatus(
-            result?.error === "nothing-to-sync"
-              ? "Connect a wallet first, then enable sync."
-              : "Could not publish the encrypted copy. Try again.",
-            "error"
-          );
-        }
-      } else {
-        this.setSyncStatus("Removing the synced copy…");
-        await sync.disable(pubkey);
-        this.setSyncStatus("Sync turned off; the synced copy was cleared.");
-        this.mainController.showSuccess("Wallet sync turned off.");
-      }
+        },
+        // Spending capability — a wallet-connect URI can SPEND. Keep the explicit
+        // one-time warning before publishing (even encrypted) to public relays.
+        preEnableConfirm: () =>
+          showConfirm(
+            "This publishes an ENCRYPTED copy of your wallet connection to public relays.\n\n" +
+              "A wallet connect URI can SPEND from your wallet. Anyone who controls your " +
+              "Nostr key could decrypt this and spend from this wallet.\n\nContinue?",
+            { confirmLabel: "Publish", danger: true },
+          ),
+      });
     } catch (error) {
       devLogger.error("[ProfileModal] Wallet sync toggle failed:", error);
       this.setSyncStatus("Sync failed. Please try again.", "error");
@@ -218,19 +187,14 @@ export class ProfileWalletController {
     }
     const sync = this.getWalletSyncService();
     try {
-      this.setSyncStatus("Fetching and decrypting…");
-      const result = await sync.pull(pubkey);
-      if (result?.found && result.imported) {
-        this.setSyncStatus("Restored from your Nostr account.", "success");
-        this.mainController.showSuccess("Wallet connection restored.");
-        this.refreshWalletPaneState();
-      } else if (result?.found && !result.imported) {
-        this.setSyncStatus("Found a copy but could not import it.", "error");
-      } else if (result?.cleared) {
-        this.setSyncStatus("No synced wallet found (it was cleared).");
-      } else {
-        this.setSyncStatus("No synced wallet found on your account.");
-      }
+      await runSyncRestore({
+        service: sync,
+        pubkey,
+        itemLabel: WALLET_SYNC_LABEL,
+        setStatus: (msg, tone) => this.setSyncStatus(msg, tone),
+        showSuccess: (msg) => this.mainController.showSuccess(msg),
+        onImported: () => this.refreshWalletPaneState(),
+      });
     } catch (error) {
       devLogger.error("[ProfileModal] Wallet sync restore failed:", error);
       this.setSyncStatus("Restore failed. Please try again.", "error");
@@ -641,7 +605,7 @@ export class ProfileWalletController {
         if (sync.isEnabled(normalizedActive)) {
           try {
             const syncResult = await sync.push(normalizedActive, {
-              confirmOverwrite: () => this.confirmSyncOverwrite(),
+              confirmOverwrite: () => confirmSyncOverwrite(WALLET_SYNC_LABEL),
             });
             if (syncResult?.conflict) {
               this.setSyncStatus(

js/ui/profileModal/syncSection.js

@@ -0,0 +1,118 @@
+// Shared UI for a syncable profile item (NWC wallet, storage credentials, …).
+// Previously ProfileWalletController and ProfileStorageController each had their
+// own copy of the sync-toggle / restore / overwrite-confirm logic, which produced
+// inconsistent notifications and duplicate confirm dialogs. This module is the one
+// implementation both use, so every item gets identical toasts, one overwrite
+// confirm, and the same conflict-only behavior. Item-specific bits (the DOM status
+// element, the "empty" hint, an optional pre-enable warning, and the re-render
+// after restore) are passed in.
+
+import { showConfirm } from "../confirmDialog.js";
+
+function capitalize(label) {
+  const value = typeof label === "string" ? label.trim() : "";
+  return value ? value[0].toUpperCase() + value.slice(1) : value;
+}
+
+// The ONE overwrite-confirm dialog, shown only on a real conflict (remote is
+// newer — changed on another device). itemLabel = "wallet connection" | "storage
+// settings".
+export function confirmSyncOverwrite(itemLabel) {
+  return showConfirm(
+    `A newer copy of your ${itemLabel} is on your account (changed on another ` +
+      `device). Overwrite it with this one?`,
+    { confirmLabel: "Overwrite", danger: true },
+  );
+}
+
+// tone → the status line's className (consistent across items).
+export function syncStatusClass(tone) {
+  const toneClass =
+    tone === "success"
+      ? "text-status-success"
+      : tone === "error"
+        ? "text-status-danger"
+        : "text-muted";
+  return `text-xs ${toneClass}`;
+}
+
+// Shared enable/disable handler. `setToggle` reflects reality on rollback;
+// `preEnableConfirm` is an optional async gate (e.g. the wallet spending-secret
+// warning); `emptyHint` is the "nothing-to-sync" message.
+export async function runSyncToggle({
+  service,
+  pubkey,
+  enabled,
+  itemLabel,
+  setStatus,
+  showSuccess,
+  setToggle,
+  preEnableConfirm,
+  emptyHint = "Nothing to sync yet — save something first.",
+}) {
+  if (enabled) {
+    if (typeof preEnableConfirm === "function") {
+      const confirmed = await preEnableConfirm();
+      if (!confirmed) {
+        setToggle?.(false);
+        return { ok: false, cancelled: true };
+      }
+    }
+    setStatus("Encrypting and publishing…");
+    const result = await service.enable(pubkey, {
+      confirmOverwrite: () => confirmSyncOverwrite(itemLabel),
+    });
+    if (result?.ok) {
+      setStatus(`Synced to ${result.accepted}/${result.total} relays.`, "success");
+      showSuccess?.(`${capitalize(itemLabel)} synced (encrypted).`);
+    } else if (result?.conflict) {
+      // Declined to overwrite a newer remote copy — keep sync on, remote intact.
+      setStatus(
+        "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.",
+      );
+    } else {
+      setToggle?.(false);
+      await service.disable(pubkey).catch(() => {});
+      setStatus(
+        result?.error === "nothing-to-sync"
+          ? emptyHint
+          : "Could not publish the encrypted copy. Try again.",
+        "error",
+      );
+    }
+    return result;
+  }
+
+  setStatus("Removing the synced copy…");
+  await service.disable(pubkey);
+  setStatus("Sync turned off; the synced copy was cleared.");
+  showSuccess?.(`${capitalize(itemLabel)} sync turned off.`);
+  return { ok: true, disabled: true };
+}
+
+// Shared restore (pull) handler. `onImported` re-renders the item's pane.
+export async function runSyncRestore({
+  service,
+  pubkey,
+  itemLabel,
+  setStatus,
+  showSuccess,
+  onImported,
+}) {
+  setStatus("Fetching and decrypting…");
+  const result = await service.pull(pubkey);
+  if (result?.found && result.imported) {
+    setStatus("Restored from your Nostr account.", "success");
+    showSuccess?.(`${capitalize(itemLabel)} restored.`);
+    if (typeof onImported === "function") {
+      await onImported();
+    }
+  } else if (result?.found && !result.imported) {
+    setStatus("Found a copy but could not import it.", "error");
+  } else if (result?.cleared) {
+    setStatus("No synced settings found (it was cleared).");
+  } else {
+    setStatus("No synced settings found on your account.");
+  }
+  return result;
+}

tests/sync-section.test.mjs

@@ -0,0 +1,111 @@
+// The wallet + storage profile sections now share one sync UI implementation
+// (syncSection.js) so they give identical notifications and one overwrite confirm,
+// with conflict-only prompting. This covers the shared enable/disable/restore
+// handlers (the run* helpers) that both controllers delegate to.
+//
+// test_integrity_note:
+//   change_type: ["new_tests"]
+//   scenarios:
+//     - id: SCN-sync-section-shared-handlers
+//       given: "a mock sync service + status/success callbacks"
+//       when: "runSyncToggle / runSyncRestore run for various outcomes"
+//       then: "consistent messages + conflict-only confirm + rollback on failure"
+//   observable_outcomes:
+//     - "enable success -> success status + '<Item> synced (encrypted).' toast"
+//     - "conflict -> keep-newer status, no success toast, no rollback"
+//     - "nothing-to-sync -> toggle rolled back + disable() + empty hint"
+//     - "preEnableConfirm rejected -> toggle rolled back, enable() never called"
+//     - "restore imported -> success + onImported() re-render"
+//   determinism_controls:
+//     - "fully mocked service + callbacks; no DOM/network/clock"
+//   anti_cheat_rationale:
+//     prevents: ["over-mocking internal logic", "hard-coded return value"]
+//   relaxation:
+//     did_relax_any_assertion: false
+
+import test from "node:test";
+import { strict as assert } from "node:assert";
+import { runSyncToggle, runSyncRestore } from "../js/ui/profileModal/syncSection.js";
+
+function harness(overrides = {}) {
+  const calls = { status: [], success: [], toggle: [], enable: 0, disable: 0, pull: 0, imported: 0 };
+  const service = {
+    enable: async () => overrides.enableResult ?? { ok: true, accepted: 2, total: 3 },
+    disable: async () => { calls.disable += 1; },
+    pull: async () => { calls.pull += 1; return overrides.pullResult ?? { found: false }; },
+  };
+  const base = {
+    service,
+    pubkey: "f".repeat(64),
+    itemLabel: "wallet connection",
+    setStatus: (msg, tone) => calls.status.push([msg, tone]),
+    showSuccess: (msg) => calls.success.push(msg),
+    setToggle: (v) => calls.toggle.push(v),
+    onImported: async () => { calls.imported += 1; },
+  };
+  const trackedEnable = service.enable;
+  service.enable = async (...a) => { calls.enable += 1; return trackedEnable(...a); };
+  return { calls, base };
+}
+
+test("enable success: success status + '<Item> synced (encrypted).' toast", async () => {
+  const { calls, base } = harness();
+  await runSyncToggle({ ...base, enabled: true });
+  assert.ok(calls.status.some(([m, t]) => t === "success" && /Synced to 2\/3 relays/.test(m)));
+  assert.deepEqual(calls.success, ["Wallet connection synced (encrypted)."]);
+});
+
+test("conflict: keep-newer status, no success toast, no rollback", async () => {
+  const { calls, base } = harness({ enableResult: { ok: false, conflict: true } });
+  await runSyncToggle({ ...base, enabled: true });
+  assert.ok(calls.status.some(([m]) => /Kept the newer copy/.test(m)));
+  assert.deepEqual(calls.success, []);
+  assert.deepEqual(calls.toggle, []); // not rolled back
+  assert.equal(calls.disable, 0);
+});
+
+test("nothing-to-sync: rolls back toggle, disables, shows the empty hint", async () => {
+  const { calls, base } = harness({ enableResult: { ok: false, error: "nothing-to-sync" } });
+  await runSyncToggle({ ...base, enabled: true, emptyHint: "Connect a wallet first." });
+  assert.deepEqual(calls.toggle, [false]);
+  assert.equal(calls.disable, 1);
+  assert.ok(calls.status.some(([m, t]) => t === "error" && m === "Connect a wallet first."));
+});
+
+test("preEnableConfirm rejected: rolls back toggle, never calls enable()", async () => {
+  const { calls, base } = harness();
+  await runSyncToggle({ ...base, enabled: true, preEnableConfirm: async () => false });
+  assert.deepEqual(calls.toggle, [false]);
+  assert.equal(calls.enable, 0);
+});
+
+test("disable: clears + 'sync turned off' toast", async () => {
+  const { calls, base } = harness();
+  await runSyncToggle({ ...base, enabled: false });
+  assert.equal(calls.disable, 1);
+  assert.deepEqual(calls.success, ["Wallet connection sync turned off."]);
+});
+
+test("restore imported: success status + toast + onImported re-render", async () => {
+  const { calls, base } = harness({ pullResult: { found: true, imported: true } });
+  await runSyncRestore({ ...base });
+  assert.ok(calls.status.some(([m, t]) => t === "success" && /Restored/.test(m)));
+  assert.deepEqual(calls.success, ["Wallet connection restored."]);
+  assert.equal(calls.imported, 1);
+});
+
+test("restore not found: 'No synced settings found' status", async () => {
+  const { calls, base } = harness({ pullResult: { found: false } });
+  await runSyncRestore({ ...base });
+  assert.ok(calls.status.some(([m]) => /No synced settings found/.test(m)));
+  assert.equal(calls.imported, 0);
+});
+
+test("storage vs wallet produce the SAME message shapes (just the label differs)", async () => {
+  const w = harness();
+  await runSyncToggle({ ...w.base, itemLabel: "wallet connection", enabled: true });
+  const s = harness();
+  await runSyncToggle({ ...s.base, itemLabel: "storage settings", enabled: true });
+  assert.deepEqual(w.calls.success, ["Wallet connection synced (encrypted)."]);
+  assert.deepEqual(s.calls.success, ["Storage settings synced (encrypted)."]);
+});