fix(mirror): idempotent NIP-71 re-mirror + self-heal cross-kind duplicates (#34)

The mirror kind (34235 normal / 34236 short) is inferred from the video's
dimensions, but (kind,pubkey,d) is the addressable identity — so re-mirroring with
different dims published the OTHER kind and showed the video TWICE in NIP-71
clients.

nip71MirrorService.publish() now looks up the author's existing mirror (both kinds,
via the SubscriptionManager chokepoint) and REUSES the existing kind so a re-publish
replaces the same coordinate. If both kinds already exist (the bug's aftermath), it
self-heals by NIP-09-deleting the stale-kind coordinate. Explicit options.short still
wins. Added findMirror(video) for relay-truth detection ({mirrored,kinds,duplicate}).

Dependency-injected; 4 new scenario tests (12/12 green); lint clean. Follow-up (b):
wire findMirror into MyVideosController so the UI toggle reflects relay truth instead
of the device-local flag.

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

js/services/nip71MirrorService.js

@@ -10,7 +10,11 @@
 //
 // Dependency-injected so it unit-tests without a live signer/relay pool.
 
-import { buildNip71MirrorEvent } from "../nostr/nip71Mirror.js";
+import {
+  buildNip71MirrorEvent,
+  NIP71_NORMAL_VIDEO_KIND,
+  NIP71_SHORT_VIDEO_KIND,
+} from "../nostr/nip71Mirror.js";
 import { nostrClient } from "../nostrClientFacade.js";
 import { getActiveSigner } from "../nostr/index.js";
 import {
@@ -34,6 +38,40 @@ export function createNip71MirrorService({
       ? nostrClient.getDeletePublishRelays()
       : [],
   getPool = () => nostrClient?.pool || null,
+  // L1 chokepoint (no direct pool.list — see scripts/check-direct-pool-access.mjs).
+  getSubscriptionManager = () =>
+    typeof nostrClient?.getSubscriptionManager === "function"
+      ? nostrClient.getSubscriptionManager()
+      : null,
+  // Look up the author's existing mirror events (both kinds) for a video root, on
+  // the write relays where mirrors are published. Returns [{ kind, created_at }].
+  // Injected so the idempotency logic unit-tests without a live relay manager.
+  fetchExistingMirrors = async ({ pubkey, root }) => {
+    const sm = getSubscriptionManager();
+    const relays = getWriteRelays() || [];
+    if (!sm || typeof sm.list !== "function" || !relays.length) {
+      return [];
+    }
+    try {
+      const events = await sm.list({
+        filters: [
+          {
+            kinds: [NIP71_NORMAL_VIDEO_KIND, NIP71_SHORT_VIDEO_KIND],
+            authors: [pubkey],
+            "#d": [root],
+          },
+        ],
+        relays,
+      });
+      return (Array.isArray(events) ? events : []).map((e) => ({
+        kind: e?.kind,
+        created_at: Number(e?.created_at) || 0,
+      }));
+    } catch (error) {
+      userLogger.warn("[nip71Mirror] existing-mirror lookup failed:", error);
+      return [];
+    }
+  },
   publishEventToRelays = defaultPublishEventToRelays,
   summarizePublishResults = defaultSummarize,
   signEvent = async (template) => {
@@ -82,9 +120,37 @@ export function createNip71MirrorService({
     }
 
     const pubkey = str(video?.pubkey) || getActivePubkey();
+    const root = str(video?.videoRootId);
+
+    // Idempotency (#34): the mirror kind (34235 normal / 34236 short) is inferred
+    // from the video's dimensions, which aren't always present — so re-mirroring the
+    // same video could publish the OTHER kind, and since (kind,pubkey,d) is the
+    // addressable identity that produced a DUPLICATE in NIP-71 clients. Reuse the kind
+    // of any existing mirror so a re-publish REPLACES the same coordinate, and remember
+    // a stray of the other kind to clean up (self-heals duplicates created earlier).
+    let forcedShort = options.short; // explicit caller override always wins
+    let staleKind = null;
+    let reusedExistingKind = false;
+    if (forcedShort === undefined && pubkey && root) {
+      const existing = await fetchExistingMirrors({ pubkey, root });
+      if (Array.isArray(existing) && existing.length) {
+        const newest = existing.reduce((a, b) =>
+          (b.created_at || 0) > (a.created_at || 0) ? b : a,
+        );
+        forcedShort = newest.kind === NIP71_SHORT_VIDEO_KIND;
+        reusedExistingKind = true;
+        const otherKind = forcedShort
+          ? NIP71_NORMAL_VIDEO_KIND
+          : NIP71_SHORT_VIDEO_KIND;
+        if (existing.some((e) => e.kind === otherKind)) {
+          staleKind = otherKind;
+        }
+      }
+    }
+
     const built = buildMirrorEvent(
       { ...video, pubkey },
-      { ...options, createdAt: Math.floor(now() / 1000) },
+      { ...options, short: forcedShort, createdAt: Math.floor(now() / 1000) },
     );
     if (!built.ok) {
       return { ok: false, reason: built.reason };
@@ -106,11 +172,37 @@ export function createNip71MirrorService({
 
     const results = await publishEventToRelays(pool, relays, signed);
     const { accepted, failed } = summarizePublishResults(results);
+
+    // Self-heal: if a duplicate of the OTHER kind already existed, tear it down via a
+    // NIP-09 delete so the video stops showing twice in NIP-71 clients.
+    let healedStaleKind = false;
+    if (staleKind != null && accepted.length > 0) {
+      try {
+        const del = await signEvent({
+          kind: 5,
+          pubkey,
+          created_at: Math.floor(now() / 1000),
+          content: "Removed duplicate NIP-71 mirror",
+          tags: [
+            ["a", `${staleKind}:${pubkey}:${root}`],
+            ["k", String(staleKind)],
+          ],
+        });
+        const delResults = await publishEventToRelays(pool, relays, del);
+        healedStaleKind = summarizePublishResults(delResults).accepted.length > 0;
+      } catch (error) {
+        userLogger.warn("[nip71Mirror] Failed to clean up duplicate mirror:", error);
+      }
+    }
+
     return {
       ok: accepted.length > 0,
       accepted: accepted.length,
       total: accepted.length + failed.length,
       event: signed,
+      kind: built.event.kind,
+      reusedExistingKind,
+      healedStaleKind,
     };
   }
 
@@ -186,7 +278,23 @@ export function createNip71MirrorService({
     return { ok: publishedOk > 0, published: publishedOk, total };
   }
 
-  return { isAvailable, canMirror, publish, remove };
+  // Relay-truth detection (#34): derive "is this mirrored?" from the actual
+  // published events instead of the device-local flag. `kinds.length > 1` flags a
+  // pre-existing cross-kind duplicate. (UI wiring is a follow-up — batch these.)
+  async function findMirror(video) {
+    const pubkey = str(video?.pubkey) || getActivePubkey();
+    const root = str(video?.videoRootId);
+    if (!pubkey || !root) {
+      return { mirrored: false, kinds: [], duplicate: false };
+    }
+    const existing = await fetchExistingMirrors({ pubkey, root });
+    const kinds = [
+      ...new Set((Array.isArray(existing) ? existing : []).map((e) => e.kind).filter(Boolean)),
+    ];
+    return { mirrored: kinds.length > 0, kinds, duplicate: kinds.length > 1 };
+  }
+
+  return { isAvailable, canMirror, publish, remove, findMirror };
 }
 
 export const nip71MirrorService = createNip71MirrorService();

tests/nip71-mirror-service.test.mjs

@@ -145,3 +145,80 @@ test("remove() is unavailable without a signer", async () => {
   assert.equal(result.ok, false);
   assert.equal(result.error, "unavailable");
 });
+
+// #34: idempotent re-mirror. The mirror KIND is inferred from dimensions
+// (34236 short for portrait, else 34235), but (kind,pubkey,d) is the addressable
+// identity — so a kind flip between mirror attempts produced a DUPLICATE in NIP-71
+// clients. publish() must reuse the kind of any existing mirror instead.
+function makeServiceWithExisting(existing) {
+  const relay = makeRelay();
+  const signer = { signEvent: async (tpl) => ({ ...tpl, id: "id", sig: "sig" }) };
+  const service = createNip71MirrorService({
+    getActivePubkey: () => PUBKEY,
+    getSigner: () => signer,
+    getWriteRelays: () => ["wss://write.relay"],
+    getPool: () => relay,
+    publishEventToRelays,
+    summarizePublishResults,
+    signEvent: signer.signEvent,
+    allowNsfw: () => false,
+    fetchExistingMirrors: async () => existing,
+  });
+  return { service, relay };
+}
+
+test("re-mirror reuses the existing mirror's kind (idempotent — no cross-kind duplicate)", async () => {
+  // An existing SHORT (34236) mirror exists; the video object has no dims, so the
+  // inference would otherwise pick 34235 (normal) → a duplicate. Must reuse 34236.
+  const { service, relay } = makeServiceWithExisting([
+    { kind: 34236, created_at: 100 },
+  ]);
+  const result = await service.publish(baseVideo());
+  assert.equal(result.ok, true);
+  assert.equal(result.reusedExistingKind, true);
+  assert.equal(relay.events.length, 1, "replaces the same coordinate, no second event");
+  assert.equal(relay.events[0].kind, 34236, "reused the existing kind, not the inferred 34235");
+});
+
+test("self-heals a pre-existing cross-kind duplicate by NIP-09-deleting the stale kind", async () => {
+  // Both kinds already exist (the bug's aftermath). Reuse the newest (34236) and
+  // tear down the stale 34235 so the video stops showing twice.
+  const { service, relay } = makeServiceWithExisting([
+    { kind: 34236, created_at: 200 },
+    { kind: 34235, created_at: 100 },
+  ]);
+  const result = await service.publish(baseVideo());
+  assert.equal(result.ok, true);
+  assert.equal(result.healedStaleKind, true);
+  assert.equal(relay.events.length, 2, "the mirror + a delete for the stale kind");
+  assert.equal(relay.events[0].kind, 34236);
+  const del = relay.events[1];
+  assert.equal(del.kind, 5, "NIP-09 delete");
+  assert.ok(
+    del.tags.some((t) => t[0] === "a" && t[1] === `34235:${PUBKEY}:root-1`),
+    "delete targets the stale 34235 coordinate",
+  );
+});
+
+test("an explicit options.short override is respected over any existing mirror", async () => {
+  const { service, relay } = makeServiceWithExisting([{ kind: 34236, created_at: 100 }]);
+  const result = await service.publish(baseVideo(), { short: false });
+  assert.equal(result.ok, true);
+  assert.equal(result.reusedExistingKind, false);
+  assert.equal(relay.events[0].kind, 34235, "explicit override wins");
+});
+
+test("findMirror derives mirror state from relays (truth), flagging duplicates", async () => {
+  const dup = makeServiceWithExisting([{ kind: 34235 }, { kind: 34236 }]).service;
+  assert.deepEqual(await dup.findMirror(baseVideo()), {
+    mirrored: true,
+    kinds: [34235, 34236],
+    duplicate: true,
+  });
+  const none = makeServiceWithExisting([]).service;
+  assert.deepEqual(await none.findMirror(baseVideo()), {
+    mirrored: false,
+    kinds: [],
+    duplicate: false,
+  });
+});

todo/TODO_2026-06-20_pre-launch.md

@@ -762,15 +762,23 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro
          a video has dimensions on one mirror attempt but not another, the kind flips, so
          re-mirroring publishes the OTHER kind → two copies in NIP-71 clients. "Delete
          removes both" confirms the shared d-tag / two-kind duplicate.
-- [ ] **FIX PLAN:**
-      (a) **Idempotent publish:** before mirroring, query relays for an existing mirror
-          by author + `#d=videoRootId` across BOTH kinds; if one exists, REUSE its kind
-          (replace it) and delete any stray of the other kind. Going forward also make
-          the kind determination stable (don't flip to 34235 just because dims are absent
-          — fall back to the existing/previous kind).
-      (b) **Relay-truth detection:** derive "is mirrored" from the published events (the
-          query above), not just the local flag; keep the flag as a cache/hint only.
-      Reproduction-dependent (needs NIP-71 relay state) — verify on unstable after.
+- [x] **FIX (a) Idempotent publish + self-heal — DONE 2026-06-25.**
+      `nip71MirrorService.publish()` now looks up the author's existing mirror (both
+      kinds) via the SubscriptionManager chokepoint and **reuses the existing kind** so
+      a re-publish REPLACES the same addressable coordinate instead of creating the
+      cross-kind duplicate. If both kinds already exist (the bug's aftermath), it
+      **self-heals** by NIP-09-deleting the stale-kind coordinate. An explicit
+      `options.short` override still wins. Also added `findMirror(video)` (relay-truth
+      `{mirrored, kinds, duplicate}`). Dependency-injected; 4 new scenario tests in
+      `tests/nip71-mirror-service.test.mjs` (12/12 green); lint clean (no direct
+      pool.list — uses `getSubscriptionManager().list`).
+- [ ] **FIX (b) Relay-truth detection in the UI (follow-up).** Wire
+      `nip71MirrorService.findMirror` into `MyVideosController` (currently uses the
+      device-local `isMirrorEnabled` flag) so the toggle reflects the published events,
+      not just this device's flag — keep the flag as a cache/hint. Batch the lookups
+      across the My Videos list to avoid N relay queries.
+- [ ] **VERIFY on unstable with Amber:** re-mirror an already-mirrored video → it should
+      replace (one copy), and a previously-duplicated video should collapse to one.
 
 ### 35. Admin whitelisting tool is slow & cumbersome — improve with application forms
 - [ ] The current admin whitelist tool is slow and clunky to use. Improve the UX when