From 96692163f19ba313617645837d2bac861f148573 Mon Sep 17 00:00:00 2001
From: marcuspaico <marcus@thepai.co>
Date: Mon, 17 Aug 2026 15:52:59 -0700
Subject: [PATCH] =?UTF-8?q?fix(labs):=20synchronous=20confirm=20transactio?=
 =?UTF-8?q?n=20=E2=80=94=20async=20callback=20broke=20atomicity?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

bun-sqlite's Database.transaction is synchronous, but async callbacks return
a pending Promise at the first await, causing immediate COMMIT before the
entire callback completes. This allowed partial inserts with no rollback.

Fixed by:
- Remove async from transaction callback
- Add .run() to each insert/update to execute synchronously
- Add regression test proving atomicity: transaction that throws mid-loop
  rolls back all changes (both tables empty after failure)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 server/src/routes/labs.ts        | 12 ++++++------
 server/test/labs-confirm.test.ts | 21 ++++++++++++++++++++-
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/server/src/routes/labs.ts b/server/src/routes/labs.ts
index 193dbfe..bfffc62 100644
--- a/server/src/routes/labs.ts
+++ b/server/src/routes/labs.ts
@@ -87,19 +87,19 @@ export function labsRoutes(deps: LabsDeps) {
       panel: m.panel, name: m.name, value: m.value, unit: m.unit,
       referenceRange: m.referenceRange, flagged: m.flagged,
     }));
-    await deps.db.transaction(async (tx) => {
-      await tx.insert(labDraws).values({
+    deps.db.transaction((tx) => {
+      tx.insert(labDraws).values({
         id: drawId, collectedAt: body.data.collectedDate, labName: body.data.labName,
         draftId: row.id, createdAt: Date.now(),
-      });
+      }).run();
       for (const n of norm) {
-        await tx.insert(biomarkers).values({
+        tx.insert(biomarkers).values({
           drawId, panel: n.panel, name: n.name, marker: n.marker, analyteKey: n.analyteKey,
           value: n.value, valueNum: n.valueNum, unit: n.unit, referenceRange: n.referenceRange,
           flagged: n.flagged ? 1 : 0, valueCanonical: n.valueCanonical, canonicalUnit: n.canonicalUnit,
-        });
+        }).run();
       }
-      await tx.update(labDrafts).set({ status: "confirmed" }).where(eq(labDrafts.id, row.id));
+      tx.update(labDrafts).set({ status: "confirmed" }).where(eq(labDrafts.id, row.id)).run();
     });
     return c.json({ drawId }, 201);
   });
diff --git a/server/test/labs-confirm.test.ts b/server/test/labs-confirm.test.ts
index 4640f6c..d19846e 100644
--- a/server/test/labs-confirm.test.ts
+++ b/server/test/labs-confirm.test.ts
@@ -5,7 +5,7 @@ import { join } from "node:path";
 import { eq } from "drizzle-orm";
 import { createApp } from "../src/app";
 import { openDb } from "../src/db";
-import { biomarkers, labDrafts } from "../src/db/schema";
+import { biomarkers, labDrafts, labDraws } from "../src/db/schema";
 import { loadOrCreateKey } from "../src/lib/crypto";
 
 async function setup() {
@@ -83,4 +83,23 @@ describe("draft review", () => {
     });
     expect(bad.status).toBe(400);
   });
+
+  test("sync transaction rolls back on mid-loop failure", () => {
+    const dir = mkdtempSync(join(tmpdir(), "helios-"));
+    const db = openDb(dir);
+
+    expect(() =>
+      db.transaction((tx) => {
+        tx.insert(labDraws).values({ id: "dX", collectedAt: "2026-01-01", labName: null, draftId: null, createdAt: 1 }).run();
+        tx.insert(biomarkers).values({ drawId: "dX", panel: "p", name: "n", marker: "m", analyteKey: null, value: "1", valueNum: 1, unit: null, referenceRange: null, flagged: 0, valueCanonical: null, canonicalUnit: null }).run();
+        throw new Error("boom");
+      }),
+    ).toThrow("boom");
+
+    // Verify both tables are empty after rollback
+    const drawRows = db.select().from(labDraws).all();
+    const bioRows = db.select().from(biomarkers).all();
+    expect(drawRows).toHaveLength(0);
+    expect(bioRows).toHaveLength(0);
+  });
 });