Fix media attachment actions

desktop/src/features/messages/lib/useMediaUpload.test.mjs

@@ -142,3 +142,221 @@ test("reserveSlots pads if slots array is shorter than expected start index", ()
   assert.equal(next[3], null); // reserved
   assert.equal(next[4], null); // reserved
 });
+
+// ── Draft-boundary epoch guard (pure logic) ───────────────────────────
+// Photos/files upload immediately, so an upload can still be in flight when
+// the composer swaps drafts (channel switch, post-send clear, edit restore).
+// Every wholesale `setPendingImeta` replacement bumps an epoch; uploads pin
+// the epoch at start and discard their descriptor if it no longer matches, so
+// one draft's attachment can never land in — or overwrite a slot reserved by —
+// another draft. Mirrors `isUploadStale` + `fillSlot`/`onUploaded`.
+
+function fillSlotIfCurrent(slots, index, descriptor, epoch, currentEpoch) {
+  if (epoch !== currentEpoch) return slots;
+  const next = [...slots];
+  next[index] = descriptor;
+  return next;
+}
+
+test("upload completing in the same draft fills its slot", () => {
+  const a = { url: "a.png", sha256: "aaaa" };
+  const next = fillSlotIfCurrent([null], 0, a, 0, 0);
+  assert.deepEqual(next, [a]);
+});
+
+test("upload completing after a draft switch is discarded", () => {
+  // Draft A reserves slot 0 at epoch 0, user switches channels (epoch → 1),
+  // then the upload resolves. It must not write into draft B's slots.
+  const a = { url: "a.png", sha256: "aaaa" };
+  const draftBSlots = [null];
+  const next = fillSlotIfCurrent(draftBSlots, 0, a, 0, 1);
+  assert.deepEqual(next, [null]);
+  assert.equal(next, draftBSlots);
+});
+
+test("stale upload cannot overwrite a slot the new draft already filled", () => {
+  // Draft B has its own attachment in slot 0; draft A's late upload targets
+  // the same index and must leave B's descriptor intact.
+  const stale = { url: "stale.png", sha256: "aaaa" };
+  const current = { url: "current.png", sha256: "bbbb" };
+  const next = fillSlotIfCurrent([current], 0, stale, 0, 2);
+  assert.deepEqual(next, [current]);
+});
+
+test("appending to the current draft does not bump the epoch", () => {
+  // Only wholesale replacement (`setPendingImeta(array)`) is a draft boundary.
+  // The updater form appends within the current draft, so in-flight uploads
+  // for that same draft must still be considered current.
+  let epoch = 0;
+  const bumpIfReplacement = (action) => {
+    if (typeof action !== "function") epoch += 1;
+  };
+  bumpIfReplacement((current) => [...current, { url: "pasted.png" }]);
+  assert.equal(epoch, 0);
+  bumpIfReplacement([]);
+  assert.equal(epoch, 1);
+});
+
+// ── Cancel guard for stale previews (pure logic) ───────────────────────
+// The epoch bump makes completions discard their descriptors, but the old
+// preview row (and its cancel button) can still be on screen. Cancelling it
+// must not null a slot in the draft now on screen, because the preview carries
+// the *previous* draft's slotIndex. Mirrors `cancelUpload`'s `isStalePreview`.
+
+function cancelSlotIndex(preview, currentEpoch) {
+  if (preview?.slotIndex === undefined) return undefined;
+  const isStale =
+    preview.uploadEpoch !== undefined && preview.uploadEpoch !== currentEpoch;
+  return isStale ? undefined : preview.slotIndex;
+}
+
+test("cancelling a preview from the current draft nulls its slot", () => {
+  assert.equal(cancelSlotIndex({ slotIndex: 1, uploadEpoch: 3 }, 3), 1);
+});
+
+test("cancelling a stale preview does not null the new draft's slot", () => {
+  // Draft A reserved slot 0 at epoch 0; draft B now owns slot 0. Cancelling
+  // A's leftover preview must leave B's attachment intact.
+  assert.equal(cancelSlotIndex({ slotIndex: 0, uploadEpoch: 0 }, 1), undefined);
+});
+
+test("cancelling a preview with no slot is a no-op for slots", () => {
+  // `handlePaperclip`'s native-picker preview has no reserved slot.
+  assert.equal(cancelSlotIndex({ uploadEpoch: 0 }, 0), undefined);
+});
+
+// ── Retiring in-flight uploads at a draft boundary (pure logic) ────────
+// Bumping the epoch alone discards descriptors but leaves the previous draft's
+// preview rows on screen and its uploads counted, which keeps `isUploading`
+// true and holds the *new* draft's send gate closed. A wholesale replacement
+// must therefore retire those uploads outright. Mirrors `beginNewDraftEpoch`.
+
+function beginNewDraftEpoch(state) {
+  const next = {
+    epoch: state.epoch + 1,
+    active: new Set(state.active),
+    canceled: new Set(state.canceled),
+    previews: state.previews,
+    uploadingCount: state.uploadingCount,
+  };
+  if (next.active.size === 0) return next;
+  // Mirrors the real callback: snapshot, clear the live set, then schedule the
+  // updaters. `applyUpdates` below runs them afterwards, the way React does.
+  const retiredIds = new Set(next.active);
+  const retiredCount = retiredIds.size;
+  next.active.clear();
+  for (const id of retiredIds) next.canceled.add(id);
+  next.pendingUpdates = [
+    (s) => {
+      s.previews = s.previews.filter((preview) => !retiredIds.has(preview.id));
+    },
+    (s) => {
+      s.uploadingCount = Math.max(0, s.uploadingCount - retiredCount);
+    },
+  ];
+  return next;
+}
+
+/** Run the scheduled state updaters, as React does after the event handler. */
+function applyUpdates(state) {
+  for (const update of state.pendingUpdates ?? []) update(state);
+  state.pendingUpdates = [];
+  return state;
+}
+
+test("a draft boundary retires in-flight uploads so the new draft can send", () => {
+  // Draft A has one upload in flight; switching to draft B must leave B with
+  // no previews and nothing counted as uploading.
+  const after = applyUpdates(
+    beginNewDraftEpoch({
+      epoch: 0,
+      active: new Set([1]),
+      canceled: new Set(),
+      previews: [{ id: 1, slotIndex: 0, uploadEpoch: 0 }],
+      uploadingCount: 1,
+    }),
+  );
+  assert.equal(after.epoch, 1);
+  assert.deepEqual(after.previews, []);
+  assert.equal(after.uploadingCount, 0);
+  assert.equal(after.active.size, 0);
+  // Canceled so the late completion/error paths stay silent in the new draft.
+  assert.ok(after.canceled.has(1));
+});
+
+test("retiring several concurrent uploads clears the count exactly once each", () => {
+  const after = applyUpdates(
+    beginNewDraftEpoch({
+      epoch: 4,
+      active: new Set([7, 8, 9]),
+      canceled: new Set(),
+      previews: [{ id: 7 }, { id: 8 }, { id: 9 }],
+      uploadingCount: 3,
+    }),
+  );
+  assert.equal(after.uploadingCount, 0);
+  assert.deepEqual(after.previews, []);
+});
+
+test("a draft boundary with no uploads in flight still advances the epoch", () => {
+  const after = applyUpdates(
+    beginNewDraftEpoch({
+      epoch: 2,
+      active: new Set(),
+      canceled: new Set(),
+      previews: [],
+      uploadingCount: 0,
+    }),
+  );
+  assert.equal(after.epoch, 3);
+  assert.equal(after.uploadingCount, 0);
+});
+
+test("the retired count never drives uploadingCount negative", () => {
+  // Defensive: a preview already settled by finishUpload must not be
+  // double-decremented into a negative count that would wedge the gate.
+  const after = applyUpdates(
+    beginNewDraftEpoch({
+      epoch: 0,
+      active: new Set([1, 2]),
+      canceled: new Set(),
+      previews: [{ id: 1 }, { id: 2 }],
+      uploadingCount: 1,
+    }),
+  );
+  assert.equal(after.uploadingCount, 0);
+});
+
+test("retirement holds even though the live active set is cleared first", () => {
+  // Regression: the updaters must not read the live `active` set, which is
+  // emptied before React runs them. Closing over it filtered against an empty
+  // set and subtracted 0, leaving the stale preview and a stuck send gate.
+  const state = beginNewDraftEpoch({
+    epoch: 0,
+    active: new Set([1]),
+    canceled: new Set(),
+    previews: [{ id: 1 }],
+    uploadingCount: 1,
+  });
+  assert.equal(state.active.size, 0, "live set is cleared before updates run");
+  // Updates land only now — after the clear — exactly as React schedules them.
+  applyUpdates(state);
+  assert.deepEqual(state.previews, []);
+  assert.equal(state.uploadingCount, 0);
+});
+
+test("replayed updaters stay idempotent", () => {
+  // React may invoke an updater more than once (StrictMode double-render).
+  const state = beginNewDraftEpoch({
+    epoch: 0,
+    active: new Set([1]),
+    canceled: new Set(),
+    previews: [{ id: 1 }],
+    uploadingCount: 1,
+  });
+  const updates = state.pendingUpdates;
+  for (const update of updates) update(state);
+  for (const update of updates) update(state);
+  assert.deepEqual(state.previews, []);
+  assert.equal(state.uploadingCount, 0);
+});