nip71: tear down the addressable mirror on delete regardless of local flag

resolveDeleteSync gated mirror teardown on the per-video opt-in flag,
which lives only in this browser's localStorage. A video shared on one
device and deleted from another (or after a cache clear) skipped teardown,
orphaning the NIP-71 mirror on other apps. Delete now always attempts
teardown when the feature is on (remove() is idempotent). The flag still
gates the UI toggle and publishing.

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

js/services/nip71MirrorFlags.js

@@ -137,9 +137,18 @@ export function resolveEditSync({ featureOn, enabled, eligible } = {}) {
   return eligible === true ? { action: "publish" } : { action: "unshare" };
 }
 
-// On DELETE of a mirrored video: remove the mirror and clear the flag.
+// On DELETE of a video: always attempt to tear the mirror down whenever the
+// feature is on — do NOT gate on the local `enabled` flag. That flag lives only
+// in this browser's localStorage (see top of file), so trusting it here orphans
+// the NIP-71 mirror whenever the video was shared on one device/session and
+// deleted from another (or after the cache was cleared): the mirror keeps living
+// on other apps even though the bitvid video is gone. `remove()` is idempotent
+// and cheap (NIP-09 + empty tombstone are no-ops if no mirror exists), so an
+// unconditional teardown is safe. `enabled` is accepted for signature symmetry
+// with resolveEditSync but is intentionally ignored.
 export function resolveDeleteSync({ featureOn, enabled } = {}) {
-  if (featureOn !== true || enabled !== true) {
+  void enabled;
+  if (featureOn !== true) {
     return { action: "none" };
   }
   return { action: "unshare" };

tests/nip71-mirror-flags.test.mjs

@@ -70,10 +70,19 @@ test("resolveEditSync: keeps a mirrored video in lockstep, unshares when ineligi
   assert.deepEqual(resolveEditSync({ featureOn: true, enabled: true, eligible: false }), { action: "unshare" });
 });
 
-test("resolveDeleteSync: removes the mirror only when it was shared", () => {
+test("resolveDeleteSync: attempts teardown whenever the feature is on, regardless of the local flag", () => {
+  // Spec correction: the previous expectation gated teardown on the local
+  // `enabled` flag (enabled:false -> none). That flag is browser-local only, so
+  // a video shared on one device and deleted from another (or after a cache
+  // clear) would skip teardown and orphan the NIP-71 mirror on other apps. The
+  // teardown (NIP-09 + empty tombstone) is idempotent, so delete must attempt it
+  // whenever the feature is on. `enabled` is intentionally ignored for delete.
   assert.deepEqual(resolveDeleteSync({ featureOn: true, enabled: true }), { action: "unshare" });
-  assert.deepEqual(resolveDeleteSync({ featureOn: true, enabled: false }), { action: "none" });
+  assert.deepEqual(resolveDeleteSync({ featureOn: true, enabled: false }), { action: "unshare" });
+  assert.deepEqual(resolveDeleteSync({ featureOn: true, enabled: undefined }), { action: "unshare" });
+  // The feature flag still fully gates it: a build with the mirror off never publishes.
   assert.deepEqual(resolveDeleteSync({ featureOn: false, enabled: true }), { action: "none" });
+  assert.deepEqual(resolveDeleteSync({ featureOn: false, enabled: false }), { action: "none" });
 });
 
 test("auto-share preference persists per pubkey, off by default", () => {

tests/nip71-mirror-sync.test.mjs

@@ -4,26 +4,63 @@
 
 import "./test-helpers/setup-localstorage.mjs";
 import assert from "node:assert/strict";
-import test from "node:test";
+import test, { mock } from "node:test";
 import {
   syncNip71MirrorAfterDelete,
   initNip71MirrorSync,
 } from "../js/services/nip71MirrorSync.js";
 import { isMirrorEnabled, setMirrorEnabled } from "../js/services/nip71MirrorFlags.js";
+import { nip71MirrorService } from "../js/services/nip71MirrorService.js";
 
 const PK = "a".repeat(64);
 
-test("deleting a SHARED video clears its opt-in flag (mirror torn down)", async () => {
+test("deleting a SHARED video tears the mirror down and clears its opt-in flag", async () => {
   localStorage.clear();
   setMirrorEnabled(PK, "root-x", true);
-  await syncNip71MirrorAfterDelete({ videoRootId: "root-x", video: {}, pubkey: PK });
-  assert.equal(isMirrorEnabled(PK, "root-x"), false, "flag cleared after delete");
+  const removeSpy = mock.method(nip71MirrorService, "remove", async () => ({ ok: true }));
+  try {
+    await syncNip71MirrorAfterDelete({ videoRootId: "root-x", video: {}, pubkey: PK });
+    assert.equal(removeSpy.mock.callCount(), 1, "mirror teardown attempted");
+    assert.equal(isMirrorEnabled(PK, "root-x"), false, "flag cleared after delete");
+  } finally {
+    removeSpy.mock.restore();
+  }
 });
 
-test("deleting a NON-shared video is a no-op (and never throws)", async () => {
+test("deleting an UNFLAGGED video still attempts teardown (cross-device / cleared cache)", async () => {
+  // Regression: the per-video opt-in flag is browser-local, so a video shared on
+  // one device and deleted from another (or after a cache clear) has no flag set
+  // here. Teardown (NIP-09 + empty tombstone) is idempotent, so delete must still
+  // attempt it — otherwise the NIP-71 mirror is orphaned on other apps.
   localStorage.clear();
-  await syncNip71MirrorAfterDelete({ videoRootId: "root-y", video: {}, pubkey: PK });
-  assert.equal(isMirrorEnabled(PK, "root-y"), false);
+  assert.equal(isMirrorEnabled(PK, "root-y"), false, "precondition: no local flag");
+  const removeSpy = mock.method(nip71MirrorService, "remove", async () => ({ ok: true }));
+  try {
+    await syncNip71MirrorAfterDelete({ videoRootId: "root-y", video: {}, pubkey: PK });
+    assert.equal(
+      removeSpy.mock.callCount(),
+      1,
+      "teardown must be attempted even with no local opt-in flag",
+    );
+    const arg = removeSpy.mock.calls[0].arguments[0];
+    assert.equal(arg.videoRootId, "root-y", "teardown targets the deleted root");
+    assert.equal(arg.pubkey, PK, "teardown targets the author");
+  } finally {
+    removeSpy.mock.restore();
+  }
+});
+
+test("delete sync never throws when teardown fails (best-effort)", async () => {
+  localStorage.clear();
+  const removeSpy = mock.method(nip71MirrorService, "remove", async () => {
+    throw new Error("relay unreachable");
+  });
+  try {
+    await syncNip71MirrorAfterDelete({ videoRootId: "root-z", video: {}, pubkey: PK });
+    assert.equal(isMirrorEnabled(PK, "root-z"), false, "flag stays clear even on failure");
+  } finally {
+    removeSpy.mock.restore();
+  }
 });
 
 test("initNip71MirrorSync subscribes to edit+delete events, idempotently", () => {