Fix media attachment actions

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

@@ -0,0 +1,100 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { applyImetaUpdate, compactImetaSlots } from "./imetaSlots.ts";
+
+// An immediate upload reserves a `null` placeholder and later fills it *by
+// index*. Updates written against the compacted list must therefore be mapped
+// back onto the slot layout: replacing the array renumbers it under an
+// in-flight upload, whose fillSlot would then overwrite an unrelated
+// attachment.
+
+const SNAPSHOT = { url: "snapshot.png", sha256: "5555" };
+const append = (descriptor) => (current) => [...current, descriptor];
+
+test("compaction hides in-flight placeholders", () => {
+  const only = { url: "only.png", sha256: "1111" };
+  assert.deepEqual(compactImetaSlots([null, only, null]), [only]);
+  assert.deepEqual(compactImetaSlots([]), []);
+});
+
+test("a snapshot paste during an in-flight upload does not take its slot", () => {
+  // Repro: attach a photo (slot 0 reserved, still uploading), then paste an
+  // agent snapshot. The snapshot must land after the placeholder so the
+  // photo's fillSlot(0, ...) cannot overwrite it.
+  const slots = applyImetaUpdate([null], append(SNAPSHOT));
+  assert.deepEqual(slots, [null, SNAPSHOT]);
+
+  // The upload completes and fills its own reserved index.
+  const photo = { url: "photo.png", sha256: "aaaa" };
+  const filled = [...slots];
+  filled[0] = photo;
+  assert.deepEqual(filled, [photo, SNAPSHOT]);
+});
+
+test("an append keeps already-filled attachments at their own indexes", () => {
+  const first = { url: "first.png", sha256: "1111" };
+  assert.deepEqual(applyImetaUpdate([first, null], append(SNAPSHOT)), [
+    first,
+    null,
+    SNAPSHOT,
+  ]);
+});
+
+test("an updater returning its input leaves the slots untouched", () => {
+  // handleSnapshotPaste returns `current` unchanged when the snapshot is
+  // already attached; that must not disturb a reserved placeholder.
+  const existing = [SNAPSHOT, null];
+  const slots = applyImetaUpdate(existing, (current) => current);
+  assert.equal(slots, existing, "same array identity, no re-render churn");
+});
+
+test("a removal nulls its slot instead of renumbering", () => {
+  // Removing an attachment must not shift the index a pending upload holds.
+  const keep = { url: "keep.png", sha256: "1111" };
+  const drop = { url: "drop.png", sha256: "2222" };
+  const slots = applyImetaUpdate([keep, drop, null], (current) =>
+    current.filter((d) => d.url !== "drop.png"),
+  );
+  assert.deepEqual(slots, [keep, null, null]);
+});
+
+test("clearing every attachment keeps the reserved placeholders", () => {
+  const one = { url: "one.png", sha256: "1111" };
+  assert.deepEqual(
+    applyImetaUpdate([one, null], () => []),
+    [null, null],
+  );
+});
+
+test("the updater only ever sees real attachments", () => {
+  const only = { url: "only.png", sha256: "1111" };
+  let seen = null;
+  applyImetaUpdate([null, only, null], (current) => {
+    seen = current;
+    return current;
+  });
+  assert.deepEqual(seen, [only]);
+});
+
+test("descriptors are matched on url and digest together", () => {
+  // Same url, different bytes: the new descriptor is an append, not a survivor.
+  const original = { url: "same.png", sha256: "1111" };
+  const reuploaded = { url: "same.png", sha256: "2222" };
+  assert.deepEqual(applyImetaUpdate([original, null], append(reuploaded)), [
+    original,
+    null,
+    reuploaded,
+  ]);
+});
+
+test("a reorder does not move descriptors out of their slots", () => {
+  // Reordering cannot be honored while an upload holds an index; keeping the
+  // existing positions is what protects the pending fillSlot.
+  const a = { url: "a.png", sha256: "1111" };
+  const b = { url: "b.png", sha256: "2222" };
+  const slots = applyImetaUpdate([a, b, null], (current) =>
+    [...current].reverse(),
+  );
+  assert.deepEqual(slots, [a, b, null]);
+});

desktop/src/features/messages/lib/imetaSlots.ts

@@ -0,0 +1,65 @@
+import type { BlobDescriptor } from "@/shared/api/tauri";
+
+/**
+ * Slot bookkeeping for composer attachments.
+ *
+ * Attachments live in a sparse array: an immediate upload calls `reserveSlots`
+ * to claim an index up front and fills it by that index when it completes, so
+ * concurrent uploads publish in the order they were attached. A `null` is a
+ * placeholder for an upload still in flight.
+ *
+ * Consumers of the composer only ever see the compacted list of real
+ * attachments, so any update expressed against that view has to be mapped back
+ * onto the slot layout — never applied to it directly.
+ */
+
+/**
+ * Identity of a descriptor. `url` alone can repeat across re-uploads of
+ * identical bytes, so pair it with the digest.
+ */
+function descriptorKey(descriptor: BlobDescriptor): string {
+  return `${descriptor.url}\u0000${descriptor.sha256 ?? ""}`;
+}
+
+/** The real attachments, in order, with in-flight placeholders dropped. */
+export function compactImetaSlots(
+  slots: (BlobDescriptor | null)[],
+): BlobDescriptor[] {
+  return slots.filter((d): d is BlobDescriptor => d !== null);
+}
+
+/**
+ * Apply an updater written against the compacted list back onto `slots`.
+ *
+ * Replacing the array with the updater's result would renumber it while an
+ * in-flight upload still holds an index from `reserveSlots`, so that upload's
+ * `fillSlot` would overwrite an unrelated attachment. Instead:
+ *
+ * - survivors stay at the index they already occupy;
+ * - removals become `null` rather than shifting their neighbours;
+ * - genuinely new descriptors append after the reserved tail, where no pending
+ *   `fillSlot` can reach them.
+ *
+ * An updater that returns its input unchanged (e.g. the snapshot-paste dedupe)
+ * leaves `slots` exactly as it was, identity included.
+ */
+export function applyImetaUpdate(
+  slots: (BlobDescriptor | null)[],
+  update: (current: BlobDescriptor[]) => BlobDescriptor[],
+): (BlobDescriptor | null)[] {
+  const current = compactImetaSlots(slots);
+  const next = update(current);
+  if (next === current) return slots;
+
+  const survivingKeys = new Set(next.map(descriptorKey));
+  const preserved = slots.map((descriptor) =>
+    descriptor === null || survivingKeys.has(descriptorKey(descriptor))
+      ? descriptor
+      : null,
+  );
+  const presentKeys = new Set(current.map(descriptorKey));
+  const appended = next.filter(
+    (descriptor) => !presentKeys.has(descriptorKey(descriptor)),
+  );
+  return appended.length > 0 ? [...preserved, ...appended] : preserved;
+}