diff --git a/js/services/nip71MirrorFlags.js b/js/services/nip71MirrorFlags.js index 899e2c84..6331381f 100644 --- a/js/services/nip71MirrorFlags.js +++ b/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" }; diff --git a/tests/nip71-mirror-flags.test.mjs b/tests/nip71-mirror-flags.test.mjs index 62318e16..992fe006 100644 --- a/tests/nip71-mirror-flags.test.mjs +++ b/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", () => { diff --git a/tests/nip71-mirror-sync.test.mjs b/tests/nip71-mirror-sync.test.mjs index f76d210e..252a5191 100644 --- a/tests/nip71-mirror-sync.test.mjs +++ b/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", () => {