[subscriptions] Stop spurious "Failed to update subscriptions" on unconfirmed publish

Subscribe/unsubscribe intermittently showed an error but "worked after refresh":
the kind-30000 list event WAS sent and persisted, but when no relay ACKed it
within the 10s window, assertAnyRelayAccepted threw -> toggleChannel threw -> the
button handler reverted the optimistic UI and showed "Failed to update your
subscriptions." The event was on the relays the whole time (hence the refresh).

Fix: in publishSubscriptionList, distinguish an all-timeout (unconfirmed) result
from an explicit rejection. The list is an idempotent, replaceable event that was
already sent + locally cached, so an all-timeout is treated as a SOFT SUCCESS
(record the event id, no throw, reconciles on next load) instead of a hard error.
Only an explicit relay rejection (or a pre-publish signer/encryption error, thrown
earlier) still fails loudly. New exported classifier
subscriptionPublishFailuresAreAllTimeouts().

Tests: tests/subscription-publish-timeout.test.mjs (5 — all-timeout=soft,
any-rejection=hard, empty=hard, case-insensitive).

Note: assertAnyRelayAccepted is used the same way by userBlocks/app list publishes
— they likely have the same false-negative and could get the same treatment later.

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

js/subscriptions.js

@@ -175,6 +175,25 @@ const subscribeVideoViewEventsApi = (pointer, options) =>
  * Also handles fetching and rendering subscribed channels' videos
  * in the same card style as your home page.
  */
+// A RelayPublishError whose failures are ALL "publish timeout" (no relay ACKed in
+// the window) means the event was sent but not confirmed — not that it was
+// rejected. For an idempotent, replaceable list this almost always persisted (the
+// user sees it after a refresh), so we treat it as a soft success rather than a
+// hard error that would revert the optimistic UI. An explicit relay rejection has
+// a different reason and is NOT a timeout, so it still fails loudly.
+export function subscriptionPublishFailuresAreAllTimeouts(publishError) {
+  const failures = Array.isArray(publishError?.relayFailures)
+    ? publishError.relayFailures
+    : [];
+  if (!failures.length) {
+    return false;
+  }
+  return failures.every((failure) => {
+    const reason = typeof failure?.reason === "string" ? failure.reason : "";
+    return /timeout/i.test(reason);
+  });
+}
+
 class SubscriptionsManager {
   constructor() {
     this.subscribedPubkeys = new Set();
@@ -1452,7 +1471,8 @@ class SubscriptionsManager {
       signedEvent
     );
 
-    let publishSummary;
+    let publishSummary = null;
+    let softSuccess = false;
     try {
       publishSummary = assertAnyRelayAccepted(publishResults, {
         context: "subscription list"
@@ -1461,17 +1481,28 @@ class SubscriptionsManager {
       if (publishError?.relayFailures?.length) {
         publishError.relayFailures.forEach(
           ({ url, error: relayError, reason }) => {
-            userLogger.error(
-              `[SubscriptionsManager] Subscription list rejected by ${url}: ${reason}`,
+            userLogger.warn(
+              `[SubscriptionsManager] Subscription list not acked by ${url}: ${reason}`,
               relayError || reason
             );
           }
         );
       }
-      throw publishError;
+      // Only an explicit relay rejection (or a pre-publish signer/encryption
+      // error, thrown earlier) is a real failure. A result where every relay
+      // merely timed out means the event was sent to an idempotent, replaceable
+      // list and almost certainly persisted — surface it as a soft success so the
+      // optimistic UI is NOT reverted and the user doesn't see a spurious error.
+      if (!subscriptionPublishFailuresAreAllTimeouts(publishError)) {
+        throw publishError;
+      }
+      userLogger.warn(
+        "[SubscriptionsManager] Subscription list not acknowledged by any relay within the timeout; treating as optimistic success (reconciles on next load).",
+      );
+      softSuccess = true;
     }
 
-    if (publishSummary.failed.length) {
+    if (publishSummary?.failed?.length) {
       publishSummary.failed.forEach(({ url, error: relayError }) => {
         const reason =
           relayError instanceof Error
@@ -1486,12 +1517,18 @@ class SubscriptionsManager {
       });
     }
 
+    // Record the published event id for both a confirmed and an
+    // optimistic/unconfirmed publish — the event carries this id regardless.
     this.subsEventId = signedEvent.id;
     this.subsEventCreatedAt = signedEvent.created_at;
     this.saveToCache(userPubkey);
-    const acceptedUrls = publishSummary.accepted.map(({ url }) => url);
+    const acceptedUrls = publishSummary
+      ? publishSummary.accepted.map(({ url }) => url)
+      : [];
     devLogger.log(
-      "Subscription list published, event id:",
+      softSuccess
+        ? "Subscription list published (optimistic/unconfirmed), event id:"
+        : "Subscription list published, event id:",
       signedEvent.id,
       "accepted relays:",
       acceptedUrls

tests/subscription-publish-timeout.test.mjs

@@ -0,0 +1,66 @@
+// Subscribe/unsubscribe intermittently showed "Failed to update your
+// subscriptions" but worked after a refresh — because the kind-30000 list event
+// was SENT and persisted, but no relay ACKed it within the 10s window, so
+// assertAnyRelayAccepted threw and the optimistic UI was reverted with a scary
+// error. Fix: an all-timeout (unconfirmed) publish of the idempotent replaceable
+// list is treated as a SOFT SUCCESS (reconciles on next load); only an explicit
+// relay rejection (or a pre-publish signer/encryption error) fails loudly. This
+// covers the classifier that makes that call.
+//
+// test_integrity_note:
+//   change_type: ["new_tests"]
+//   scenarios:
+//     - id: SCN-subscription-publish-timeout
+//       given: "a RelayPublishError from publishing the subscription list"
+//       when: "its per-relay failures are classified"
+//       then: "all-timeout => soft success; any explicit rejection => hard failure"
+//   observable_outcomes:
+//     - "every-relay-timeout classifies as all-timeouts (soft success)"
+//     - "an explicit rejection (e.g. 'blocked'/'auth-required') does NOT"
+//     - "no failures / empty does NOT (nothing to soft-succeed)"
+//   determinism_controls:
+//     - "constructs real RelayPublishError objects; no network/clock"
+//   anti_cheat_rationale:
+//     prevents: ["hard-coded return value", "over-mocking internal logic"]
+//   relaxation:
+//     did_relax_any_assertion: false
+
+import test from "node:test";
+import { strict as assert } from "node:assert";
+import { RelayPublishError } from "../js/nostrPublish.js";
+import { subscriptionPublishFailuresAreAllTimeouts } from "../js/subscriptions.js";
+
+// Build a RelayPublishError the same way assertAnyRelayAccepted does — from the
+// per-relay result objects (success:false + an Error whose message is the reason).
+const errFrom = (results) =>
+  new RelayPublishError("Failed to publish subscription list to any relay.", results, {
+    context: "subscription list",
+  });
+const timeout = (url) => ({ url, success: false, error: new Error("publish timeout") });
+const rejected = (url, reason) => ({ url, success: false, error: new Error(reason) });
+
+test("every relay timing out is classified as all-timeouts (soft success)", () => {
+  const e = errFrom([timeout("wss://a"), timeout("wss://b"), timeout("wss://c")]);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), true);
+});
+
+test("an explicit relay rejection is NOT all-timeouts (hard failure)", () => {
+  const e = errFrom([timeout("wss://a"), rejected("wss://b", "blocked: spam")]);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), false);
+});
+
+test("only-rejections are NOT all-timeouts", () => {
+  const e = errFrom([rejected("wss://a", "auth-required"), rejected("wss://b", "restricted")]);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), false);
+});
+
+test("no failures / empty does not soft-succeed", () => {
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(errFrom([])), false);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(undefined), false);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts({}), false);
+});
+
+test("case-insensitive: 'Publish Timeout' still counts as a timeout", () => {
+  const e = errFrom([rejected("wss://a", "Publish Timeout")]);
+  assert.equal(subscriptionPublishFailuresAreAllTimeouts(e), true);
+});