fix(watch-history): removal must replace the list, not merge it back

Root cause of "watch history delete does not work at all": the removal paths
call watchHistoryService.snapshot(remaining, …), but snapshot published via
updateWatchHistoryList WITHOUT replace:true. The default merges incoming items
with the existing cached list, so the just-removed item was merged straight back
and the deletion silently had no effect. Removing the LAST item was worse: an
empty list fell through to republishing the pending queue instead of clearing.

- snapshot now honors options.replace: forwards it to updateWatchHistoryList,
  treats an explicit empty replace as a clear (publishes empty instead of
  short-circuiting or falling back to the queue), and skips the cached-items
  merge when replacing. Adds (the default) keep merge semantics.
- Both removal call sites (watchHistoryController.handleWatchHistoryRemoval and
  historyView.defaultRemoveHandler) now pass replace:true.

Tests (snapshot at the updateWatchHistoryList boundary): removing an item
publishes the remaining list with replace:true; removing the last item publishes
an empty replace (not a queue republish / no-op); a normal watch-tracking
snapshot still merges (replace not forced). Mutation-verified — dropping the
replace forwarding fails 2 cases and reverting the empty-clear handling fails the
clear case.

Build + full lint green.

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

js/historyView.js

@@ -327,7 +327,9 @@ async function defaultRemoveHandler({
         .filter(Boolean)
     : [];
   if (typeof snapshot === "function") {
-    await snapshot(sanitized, { actor, reason });
+    // Removal replaces the list — without this the snapshot would merge the
+    // removed item back from cache and the deletion would have no effect.
+    await snapshot(sanitized, { actor, reason, replace: true });
   }
   try {
     if (typeof updateWatchHistoryList === "function") {

js/ui/watchHistoryController.js

@@ -313,6 +313,8 @@ export default class WatchHistoryController {
       const snapshotResult = await this.watchHistoryService.snapshot(normalizedItems, {
         actor: actorCandidate || undefined,
         reason,
+        // Removal must replace the list, not merge the removed item back in.
+        replace: true,
       });
 
       try {

js/watchHistoryService.js

@@ -784,6 +784,10 @@ async function publishView(pointerInput, createdAt) {
 
 async function snapshot(items, options = {}) {
   const reason = typeof options.reason === "string" ? options.reason : "manual";
+  // Removal/clear paths must REPLACE the published list — otherwise the items the
+  // user just removed get merged back from the existing cache and the deletion
+  // silently has no effect. Adds (the default) keep merge semantics.
+  const replace = options.replace === true;
   const actorKey = resolveActorKey(options.actor);
   if (!actorKey) {
     return { ok: false, error: "missing-actor" };
@@ -810,11 +814,16 @@ async function snapshot(items, options = {}) {
     payloadItems = items
       .map((item) => normalizePointerInput(item))
       .filter(Boolean);
+  } else if (replace && Array.isArray(items)) {
+    // Explicit replace with an empty list = clear the history. Must NOT fall
+    // back to the pending queue (that would resurrect items just removed) and
+    // must still publish so relays drop the old list.
+    payloadItems = [];
   } else {
     payloadItems = collectQueueItems(actorKey);
   }
 
-  if (!payloadItems.length) {
+  if (!payloadItems.length && !replace) {
     const result = { ok: true, empty: true, actor: actorKey };
     emit("snapshot-empty", result);
     return result;
@@ -830,7 +839,7 @@ async function snapshot(items, options = {}) {
         )
       : new Set();
 
-  if (!items) {
+  if (!items && !replace) {
     const cachedItems = getCachedSnapshotItems(actorKey);
     if (cachedItems.length) {
       payloadItems = [...payloadItems, ...cachedItems];
@@ -854,6 +863,7 @@ async function snapshot(items, options = {}) {
       payloadItems,
       {
         actorPubkey: actorKey,
+        replace,
       }
     );
 

tests/watch-history-remove-snapshot.test.mjs

@@ -0,0 +1,98 @@
+// Watch-history removal must publish the reduced list with REPLACE semantics.
+//
+// Bug (SCN-watch-history-remove-replaces): the removal path calls
+// watchHistoryService.snapshot(remaining, …) but snapshot published via
+// updateWatchHistoryList WITHOUT replace:true — the default merges incoming
+// items with the existing cached list, so the just-removed item was merged
+// straight back and the deletion had no effect ("watch history delete does not
+// work at all"). Removing the LAST item was even worse: an empty list fell
+// through to republishing the pending queue instead of clearing.
+//
+// Observable outcome asserted at the boundary: the options passed to
+// nostrClient.updateWatchHistoryList (the relay-publish entry point).
+
+import "./test-helpers/setup-localstorage.mjs";
+import assert from "node:assert/strict";
+import test from "node:test";
+import { watchHistoryService } from "../js/watchHistoryService.js";
+import { nostrClient } from "../js/nostrClientFacade.js";
+
+// snapshot calls these as property lookups on the shared singleton, so stubbing
+// the properties is sufficient to virtualize the relay boundary.
+function installStub() {
+  const calls = [];
+  nostrClient.updateWatchHistoryList = async (items, opts) => {
+    calls.push({ items: items.slice(), opts });
+    return { ok: true, items };
+  };
+  nostrClient.getWatchHistoryFingerprint = async () => `fp-${calls.length}`;
+  return calls;
+}
+
+test("removing an item snapshots the remaining list with replace:true", async () => {
+  const calls = installStub();
+  const actor = "1".repeat(64);
+  const remaining = [
+    { type: "e", value: "keep-1" },
+    { type: "e", value: "keep-2" },
+  ];
+
+  await watchHistoryService.snapshot(remaining, {
+    actor,
+    reason: "remove-item",
+    replace: true,
+  });
+
+  assert.equal(calls.length, 1, "must publish exactly once");
+  assert.equal(
+    calls[0].opts.replace,
+    true,
+    "snapshot must forward replace:true so the removed item is not merged back",
+  );
+  assert.deepEqual(
+    calls[0].items.map((p) => p.value).sort(),
+    ["keep-1", "keep-2"],
+    "must publish the remaining items",
+  );
+});
+
+test("removing the LAST item publishes an empty replace (clears, not queue republish)", async () => {
+  const calls = installStub();
+  const actor = "2".repeat(64);
+
+  const result = await watchHistoryService.snapshot([], {
+    actor,
+    reason: "remove-item",
+    replace: true,
+  });
+
+  assert.notEqual(
+    result?.empty,
+    true,
+    "an empty replace must NOT short-circuit as a no-op",
+  );
+  assert.equal(calls.length, 1, "empty replace must still publish to clear relays");
+  assert.equal(calls[0].opts.replace, true);
+  assert.equal(
+    calls[0].items.length,
+    0,
+    "must publish an empty list, never fall back to the pending queue",
+  );
+});
+
+test("adds (no replace) keep merge semantics — replace is not forced", async () => {
+  const calls = installStub();
+  const actor = "3".repeat(64);
+
+  await watchHistoryService.snapshot([{ type: "e", value: "new-watch" }], {
+    actor,
+    reason: "watch",
+  });
+
+  assert.equal(calls.length, 1);
+  assert.notEqual(
+    calls[0].opts.replace,
+    true,
+    "a normal watch-tracking snapshot must NOT replace (it appends)",
+  );
+});