From 8805ebbab49ba10c4e830e05e96da717a95aaa6c Mon Sep 17 00:00:00 2001
From: WebPerson <jonlwheat2@gmail.com>
Date: Sun, 13 Sep 2026 11:22:37 -0500
Subject: [PATCH] fix(db): forward-progress guard in both
 deleteCompletedBatches chunk loops (#13377)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Issue #13377 asks for a forward-progress guard in BOTH `for (;;)` loops of
`deleteCompletedBatches` — the key-scoped loop and the instance-wide loop.

Both loops walk completed batches in chunks of `INSTANCE_SWEEP_CHUNK`:

    SELECT id FROM batches WHERE status = 'completed' [AND api_key_id = ?]
    ORDER BY rowid LIMIT ?

Each iteration sweeps its chunk (file soft-deletes + checkpoint DELETE + batch
DELETE) and then re-selects the next chunk. If a DELETE ever fails to make
progress — a concurrent deleter holding the same rows, or a swallowed no-op
delete — the same chunk is returned forever and the request thread spins.

The guard remembers the first id of the previous chunk and throws when the next
chunk starts with the same id:

    deleteCompletedBatches: chunk sweep made no forward progress (first id repeated)

Failure semantics match each mode:
- Key mode wraps the whole chunk loop in ONE outer transaction, so throwing rolls
  the entire key sweep back (all-or-nothing, unchanged).
- Instance mode commits per chunk (SEC-D), so throwing leaves earlier chunks
  committed, rolls the current chunk back, and rethrows.

Stacked on #13374 (`fix/batches-sweep-file-ownership-chunks`), whose two chunked
loops this guard protects.
---
 src/lib/db/batches.ts | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/lib/db/batches.ts b/src/lib/db/batches.ts
index 0f03cb47cea..5a72cae3188 100644
--- a/src/lib/db/batches.ts
+++ b/src/lib/db/batches.ts
@@ -540,11 +540,24 @@ export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): {
     );
     const sweepKey = db.transaction(() => {
       const totals = { deletedBatches: 0, deletedFiles: 0 };
+      let previousFirstId: string | null = null;
       for (;;) {
         const ids = (keyChunk.all(apiKeyId, INSTANCE_SWEEP_CHUNK) as Array<{ id: string }>).map(
           (r) => r.id
         );
         if (ids.length === 0) break;
+        // Forward-progress guard. The chunk SELECT is `ORDER BY rowid`, so a
+        // chunk that still STARTS with the id we just swept means the DELETE
+        // removed nothing and this loop would spin forever. Throw instead of
+        // hanging (a concurrent deleter, or a swallowed DELETE, must not wedge
+        // the request thread). Key mode is one outer transaction, so throwing
+        // rolls the whole sweep back.
+        if (ids[0] === previousFirstId) {
+          throw new Error(
+            "deleteCompletedBatches: chunk sweep made no forward progress (first id repeated)"
+          );
+        }
+        previousFirstId = ids[0];
         const part = sweepIds(ids);
         totals.deletedBatches += part.deletedBatches;
         totals.deletedFiles += part.deletedFiles;
@@ -558,9 +571,19 @@ export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): {
   const nextChunk = db.prepare(
     "SELECT id FROM batches WHERE status = 'completed' ORDER BY rowid LIMIT ?"
   );
+  let previousFirstId: string | null = null;
   for (;;) {
     const ids = (nextChunk.all(INSTANCE_SWEEP_CHUNK) as Array<{ id: string }>).map((r) => r.id);
     if (ids.length === 0) break;
+    // Forward-progress guard, same reasoning as key mode above. Instance mode
+    // commits per chunk, so throwing here leaves the already-committed chunks
+    // in place and rethrows (SEC-D) rather than looping forever.
+    if (ids[0] === previousFirstId) {
+      throw new Error(
+        "deleteCompletedBatches: chunk sweep made no forward progress (first id repeated)"
+      );
+    }
+    previousFirstId = ids[0];
     const part = sweepIds(ids);
     totals.deletedBatches += part.deletedBatches;
     totals.deletedFiles += part.deletedFiles;