fix(collab): report the bytes sweepBlobs actually reclaimed

.changeset/blob-gc-overstated-reclaim.md

@@ -0,0 +1,7 @@
+---
+"@ifc-lite/collab": patch
+---
+
+Fix `sweepBlobs` reporting a blob as reclaimed even when the underlying `store.delete()` call failed.
+
+`sweepBlobs` computed how many deletes actually succeeded but then discarded that count and returned `decision.reclaimBytes` unconditionally — the full byte total `planBlobSweep` had planned to free, regardless of whether any individual `delete()` call reported failure (a remote backend 404, a race with another sweep, a transient error). A caller using the return value for storage-capacity accounting would believe more space was freed than actually was, while the undeleted blob kept consuming storage. `planBlobSweep` now records each dropped hash's byte length on the `SweepDecision`, and `sweepBlobs` sums only the bytes for hashes whose `delete()` actually returned `true`.

packages/collab/src/geometry/gc.ts

@@ -114,6 +114,12 @@ export interface SweepDecision {
   drop: BlobHash[];
   /** Bytes that will be reclaimed once `drop` is processed. May be undefined when the store doesn't expose sizes. */
   reclaimBytes: number;
+  /**
+   * Per-hash byte size for every entry in `drop`, so `sweepBlobs` can
+   * report the bytes actually reclaimed rather than the bytes merely
+   * planned — a `store.delete()` failure must not be counted.
+   */
+  dropByteLengths: Record<BlobHash, number>;
 }
 
 /**
@@ -134,6 +140,7 @@ export async function planBlobSweep(
   const now = options.now ? options.now() : Date.now();
   const all = await store.list();
   const drop: BlobHash[] = [];
+  const dropByteLengths: Record<BlobHash, number> = {};
   let reclaim = 0;
   for (const hash of all) {
     if (referenced.has(hash)) continue;
@@ -147,16 +154,18 @@ export async function planBlobSweep(
       // so the grace window keeps protecting in-flight uploads.
       if (!options.sweepUnknownAge) continue;
       drop.push(hash);
+      dropByteLengths[hash] = meta.byteLength;
       reclaim += meta.byteLength;
       continue;
     }
     const ageMs = Math.max(0, now - new Date(meta.uploadedAt).getTime());
     if (ageMs >= epochMs) {
       drop.push(hash);
+      dropByteLengths[hash] = meta.byteLength;
       reclaim += meta.byteLength;
     }
   }
-  return { drop, reclaimBytes: reclaim };
+  return { drop, reclaimBytes: reclaim, dropByteLengths };
 }
 
 async function metaFromGet(store: BlobStore, hash: BlobHash): Promise<BlobMeta | null> {
@@ -177,13 +186,20 @@ async function metaFromGet(store: BlobStore, hash: BlobHash): Promise<BlobMeta |
   };
 }
 
-/** Apply a sweep decision: delete the candidates from `store`. */
+/**
+ * Apply a sweep decision: delete the candidates from `store`.
+ *
+ * Returns the bytes actually reclaimed — i.e. only for hashes whose
+ * `store.delete()` reported success. A failed delete (backend race,
+ * 404, transient error) must not be counted as freed: callers use this
+ * return value for capacity accounting, and overstating it hides a
+ * blob that is still consuming storage.
+ */
 export async function sweepBlobs(store: BlobStore, decision: SweepDecision): Promise<number> {
   let freed = 0;
   for (const hash of decision.drop) {
     const ok = await store.delete(hash);
-    if (ok) freed += 1;
+    if (ok) freed += decision.dropByteLengths[hash] ?? 0;
   }
-  void freed;
-  return decision.reclaimBytes;
+  return freed;
 }

packages/collab/test/blob-gc.test.ts

@@ -91,4 +91,34 @@ describe('blob GC', () => {
     expect(decision.drop).toEqual([orphan.hash]);
     expect(decision.reclaimBytes).toBe(3);
   });
+
+  it('does not report bytes as reclaimed when the underlying delete() fails', async () => {
+    const inner = new MemoryBlobStore();
+    const a = await inner.put(new Uint8Array([1, 1, 1]));
+    const b = await inner.put(new Uint8Array([2, 2, 2, 2]));
+
+    // A store whose delete() reports failure for one of the two blobs —
+    // e.g. a remote backend that 404s or races with another sweep.
+    const flaky = {
+      put: inner.put.bind(inner),
+      get: inner.get.bind(inner),
+      has: inner.has.bind(inner),
+      list: inner.list.bind(inner),
+      stat: inner.stat.bind(inner),
+      delete: async (hash: string) => (hash === a.hash ? false : inner.delete(hash)),
+    };
+
+    const referenced = new Set<string>();
+    const decision = await planBlobSweep(flaky, referenced, { epochMs: 0 });
+    expect(decision.drop.sort()).toEqual([a.hash, b.hash].sort());
+    expect(decision.reclaimBytes).toBe(7);
+
+    const freed = await sweepBlobs(flaky, decision);
+
+    // Only `b` (4 bytes) was actually deleted; `a` (3 bytes) failed to
+    // delete and must still be present in the store.
+    expect(await inner.has(a.hash)).toBe(true);
+    expect(await inner.has(b.hash)).toBe(false);
+    expect(freed).toBe(4);
+  });
 });