feat(r2): clean up the superseded object when a video's URL is replaced on edit

Editing a video with a newly-uploaded file produced a new content-addressed
object and orphaned the old one. editModalController.handleSubmit now removes the
old object (best-effort) when the hosted URL was genuinely replaced — guarded so
it only fires when both old and new URLs are present and different (so clearing
the URL, which may still be referenced by a magnet web-seed, never deletes it),
and it deliberately leaves thumbnails alone (the new note may still point at
them). Reuses R2Service.deleteVideoStorage (owned-objects-only, never throws).

Documented the cleanup contract in AGENTS.md §4a.

Test: tests/ui/edit-modal-storage-cleanup.test.mjs (3). Full suite green.

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

AGENTS.md

@@ -70,6 +70,7 @@ For any nostr related work, please review the nip documentation located in /docs
 * **Whole-file hash before upload.** When the modal doesn't pre-seed an info-hash, `resolveUploadIdentifier` runs `calculateTorrentInfoHash(file)`, which reads the *entire* file before the upload begins and before any progress is emitted — on multi-GB files this looks like a hang. Emit a "preparing/hashing" status or parallelize.
 * **Publish can report success unconfirmed.** `publishEventToRelay` (`js/nostrPublish.js`) has an optimistic-success fallthrough when the relay handle exposes no `on()`/`then()` — a note may show "Published" with no relay ACK. Don't tighten blindly (legacy `seen`-only relays rely on it), but log it.
 * **Silent partial failures.** Thumbnail and `.torrent` upload failures are caught and `warn`-only; the video note still publishes with no user signal. Empty (0-byte) files hit `CompleteMultipartUpload` with `Parts:[]` and surface a cryptic error — guard `file.size === 0`.
+* **Object cleanup (deletes/edits).** `R2Service.deleteVideoStorage()` removes the backing objects so "deleted" videos aren't left publicly downloadable. Contract to preserve: it's **best-effort and never throws** (must not block note deletion), it derives keys via `collectVideoStorageKeys()` and **only ever touches URLs under the owner's bucket base** (never external/3rd-party URLs), and it skips cleanly when storage is locked/credential-less. Wired into delete (`app.handleDeleteModalConfirm`) and edit-with-replaced-URL (`editModalController.handleSubmit`). Edit cleanup intentionally does **not** delete thumbnails (the new note may still reference them) and only fires when both old and new URLs are present and different.
 
 ---
 

js/ui/editModalController.js

@@ -1,4 +1,5 @@
 import { devLogger } from "../utils/logger.js";
+import r2Service from "../services/r2Service.js";
 
 export default class EditModalController {
   constructor({ services, state, ui, callbacks, helpers }) {
@@ -90,6 +91,34 @@ export default class EditModalController {
         updatedData,
         pubkey,
       });
+
+      // Best-effort: if the hosted video URL was genuinely replaced, the old
+      // R2/S3 object (and its .torrent) is now orphaned — remove it. We require
+      // both URLs to be non-empty and different so we never delete an object
+      // that's still referenced (e.g. via a magnet web-seed when the URL was
+      // merely cleared). Thumbnails are intentionally left alone here to avoid
+      // deleting one the new note still points at. Never blocks the edit.
+      try {
+        const oldUrl = detail.video?.url || "";
+        const newUrl = updatedData?.url || "";
+        if (updatedData?.urlEdited && oldUrl && newUrl && oldUrl !== newUrl) {
+          const cleanup = await r2Service.deleteVideoStorage({
+            videos: [{ url: oldUrl }],
+            pubkey,
+          });
+          if (cleanup?.deleted?.length) {
+            devLogger.log(
+              `[edit] Removed ${cleanup.deleted.length} superseded storage object(s).`
+            );
+          }
+        }
+      } catch (cleanupErr) {
+        devLogger.warn(
+          "[edit] Storage cleanup failed (edit still succeeded):",
+          cleanupErr
+        );
+      }
+
       await this.callbacks.loadVideos();
 
       const videosMap = this.state.getVideosMap();

tests/ui/edit-modal-storage-cleanup.test.mjs

@@ -0,0 +1,78 @@
+// Scenario (SCN-edit-storage-cleanup):
+//   Editing a video with a genuinely replaced hosted URL should remove the OLD
+//   R2/S3 object (now orphaned), but only when both old and new URLs are present
+//   and different — never when the URL was merely cleared or only metadata
+//   changed (so we don't delete an object the new note still references).
+
+import "../test-helpers/setup-localstorage.mjs";
+import test from "node:test";
+import assert from "node:assert/strict";
+
+globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = true;
+const { default: EditModalController } = await import(
+  "../../js/ui/editModalController.js"
+);
+const r2Service = (await import("../../js/services/r2Service.js")).default;
+
+const BASE = "https://pub.bitvid.network";
+
+function makeController() {
+  const calls = [];
+  r2Service.deleteVideoStorage = async (args) => {
+    calls.push(args);
+    return { deleted: ["x"], failed: [], skipped: false, reason: "" };
+  };
+  const controller = new EditModalController({
+    services: { nostrService: { handleEditVideoSubmit: async () => {} } },
+    state: { getPubkey: () => "ownerhex", getVideosMap: () => null },
+    ui: {
+      getEditModal: () => ({ setSubmitState() {}, close() {} }),
+      showError() {},
+      showSuccess() {},
+    },
+    callbacks: { loadVideos: async () => {}, forceRefreshAllProfiles: () => {} },
+    helpers: {},
+  });
+  return { controller, calls };
+}
+
+const submit = (controller, { oldUrl, newUrl, urlEdited }) =>
+  controller.handleSubmit({
+    detail: {
+      originalEvent: { id: "1", pubkey: "ownerhex", videoRootId: "root" },
+      updatedData: { url: newUrl, urlEdited },
+      video: { url: oldUrl },
+    },
+  });
+
+test("cleans the old object when the URL is genuinely replaced", async () => {
+  const { controller, calls } = makeController();
+  await submit(controller, {
+    oldUrl: `${BASE}/u/np/OLD/clip.mp4`,
+    newUrl: `${BASE}/u/np/NEW/clip.mp4`,
+    urlEdited: true,
+  });
+  assert.equal(calls.length, 1, "should request cleanup once");
+  assert.equal(calls[0].videos[0].url, `${BASE}/u/np/OLD/clip.mp4`);
+  assert.equal(calls[0].pubkey, "ownerhex");
+});
+
+test("does NOT clean when only metadata changed (urlEdited false)", async () => {
+  const { controller, calls } = makeController();
+  await submit(controller, {
+    oldUrl: `${BASE}/u/np/OLD/clip.mp4`,
+    newUrl: `${BASE}/u/np/OLD/clip.mp4`,
+    urlEdited: false,
+  });
+  assert.equal(calls.length, 0);
+});
+
+test("does NOT clean when the URL was cleared (new URL empty)", async () => {
+  const { controller, calls } = makeController();
+  await submit(controller, {
+    oldUrl: `${BASE}/u/np/OLD/clip.mp4`,
+    newUrl: "",
+    urlEdited: true,
+  });
+  assert.equal(calls.length, 0, "clearing the URL must not delete the object (magnet web-seed may still use it)");
+});