diff --git a/js/services/nip71MirrorService.js b/js/services/nip71MirrorService.js index 25bbec5c..a707b427 100644 --- a/js/services/nip71MirrorService.js +++ b/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(); diff --git a/tests/nip71-mirror-service.test.mjs b/tests/nip71-mirror-service.test.mjs index ab9718cd..921aaf28 100644 --- a/tests/nip71-mirror-service.test.mjs +++ b/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, + }); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 84f8445a..c2bd45ad 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/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