fix(money): make every payment balance-mutation atomic (Track A)

Quick/bulk-pay were already atomic (X1); the audit found the row write +
applyBalanceDelta split across two un-transactional statements on manual
POST /payments, autopay-confirm, DELETE (unpay), restore, and PUT (edit,
where the bill-balance write and the payment UPDATE weren't atomic). A
failure between the two could leave a payment without its balance
adjustment (or a balance reversed without the row deleted). Each is now a
single db.transaction(). Behavior-preserving.

tests/paymentsRoute.test.js pins the create→delete→restore→edit balance
round-trip + double-restore no-op. Full suite 186 green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

HISTORY.md

@@ -1,6 +1,10 @@
 # Bill Tracker — Changelog
 ## v0.41.0
 
+### 💰 Payment atomicity audit (Track A) — every balance mutation is now transactional
+
+- **[Money integrity] Wrapped the four remaining non-atomic payment paths in `db.transaction()`.** Quick-pay and bulk-pay were already atomic (X1), but the audit found the INSERT/UPDATE + `applyBalanceDelta` pair split across two un-transactional statements on **manual `POST /payments`**, **autopay-suggestion confirm**, **`DELETE` (unpay)**, and **`POST /:id/restore`** — plus **`PUT` (edit)**, where the bill-balance write and the payment `UPDATE` weren't atomic. A crash/constraint failure between the two statements could leave a payment without its balance adjustment (or a balance reversed without the row deleted). Each is now a single transaction: the row write and the balance mutation land together or neither. Behavior-preserving — `tests/paymentsRoute.test.js` pins the create → delete → restore → edit balance round-trip and the double-restore no-op; full server suite green (186 tests). (Dedupe-guard work for the manual path follows as a separate change.)
+
 ### 🔷 TypeScript foundation + branded money types
 
 - **[Client] Adopted TypeScript incrementally, starting with the money boundary** — the move off plain JS. Upgraded `jsconfig` → `tsconfig` with full `strict` + `noUncheckedIndexedAccess`, but `allowJs` + `checkJs:false` so every existing `.js`/`.jsx` keeps resolving and running untouched — only `.ts`/`.tsx` are type-checked, so it's a file-by-file migration with nothing breaking in between. Added `npm run typecheck` (`tsc --noEmit`) and wired it into `npm run ci`. Installed TypeScript 6 + `@types/react`/`react-dom` 19.

routes/payments.js

@@ -204,13 +204,19 @@ router.post('/', (req, res) => {
 
   const balCalc = computeBalanceDelta(bill, payment.amount);
   const failureFlag = autopay_failure ? 1 : 0;
-  const result = db.prepare(
-    'INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source, autopay_failure) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)'
-  ).run(payment.bill_id, payment.amount, payment.paid_date, method || null, notes || null, balCalc?.balance_delta ?? null, balCalc?.interest_delta ?? null, payment.payment_source, failureFlag);
 
-  applyBalanceDelta(db, bill.id, balCalc);
+  // Atomic: the INSERT and the balance update land together or neither, so a
+  // mid-way failure never leaves a payment without its balance adjustment.
+  const insertManualPayment = db.transaction(() => {
+    const r = db.prepare(
+      'INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source, autopay_failure) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)'
+    ).run(payment.bill_id, payment.amount, payment.paid_date, method || null, notes || null, balCalc?.balance_delta ?? null, balCalc?.interest_delta ?? null, payment.payment_source, failureFlag);
+    applyBalanceDelta(db, bill.id, balCalc);
+    return r.lastInsertRowid;
+  });
+  const paymentId = insertManualPayment();
 
-  res.status(201).json(serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(result.lastInsertRowid)));
+  res.status(201).json(serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(paymentId)));
 });
 
 // POST /api/payments/quick — pay a bill (expected amount, today)
@@ -315,25 +321,31 @@ router.post('/autopay-suggestions/:billId/confirm', (req, res) => {
   }
 
   const balCalc = computeBalanceDelta(bill, suggestedPayment.amount);
-  const result = db.prepare(`
-    INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source)
-    VALUES (?, ?, ?, ?, ?, ?, ?, ?)
-  `).run(
-    bill.id,
-    suggestedPayment.amount,
-    suggestedPayment.paid_date,
-    'autopay',
-    'Confirmed autopay suggestion',
-    balCalc?.balance_delta ?? null,
-    balCalc?.interest_delta ?? null,
-    'manual',
-  );
 
-  applyBalanceDelta(db, bill.id, balCalc);
+  // Atomic: the payment INSERT, its balance update, and clearing the dismissal
-  db.prepare('DELETE FROM autopay_suggestion_dismissals WHERE user_id = ? AND bill_id = ? AND year = ? AND month = ?')
+  // must all land or none, so a mid-way failure can't leave a half-recorded pay.
-    .run(req.user.id, bill.id, ym.year, ym.month);
+  const confirmAutopay = db.transaction(() => {
+    const r = db.prepare(`
+      INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source)
+      VALUES (?, ?, ?, ?, ?, ?, ?, ?)
+    `).run(
+      bill.id,
+      suggestedPayment.amount,
+      suggestedPayment.paid_date,
+      'autopay',
+      'Confirmed autopay suggestion',
+      balCalc?.balance_delta ?? null,
+      balCalc?.interest_delta ?? null,
+      'manual',
+    );
+    applyBalanceDelta(db, bill.id, balCalc);
+    db.prepare('DELETE FROM autopay_suggestion_dismissals WHERE user_id = ? AND bill_id = ? AND year = ? AND month = ?')
+      .run(req.user.id, bill.id, ym.year, ym.month);
+    return r.lastInsertRowid;
+  });
+  const paymentId = confirmAutopay();
 
-  res.status(201).json({ created: true, payment: serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(result.lastInsertRowid)) });
+  res.status(201).json({ created: true, payment: serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(paymentId)) });
 });
 
 // POST /api/payments/autopay-suggestions/:billId/dismiss
@@ -467,13 +479,18 @@ router.put('/:id', (req, res) => {
 
   const bill = db.prepare('SELECT * FROM bills WHERE id = ? AND user_id = ? AND deleted_at IS NULL').get(existing.bill_id, req.user.id);
   let nextInterestDelta = existing.interest_delta ?? null;
+  // Compute the balance mutation without writing yet — the write happens inside
+  // the transaction below so it lands atomically with the payment UPDATE.
+  let balCalc = null;
+  let paymentPortion = null;
+  let restoredBalance = null;
   if (bill) {
     // Reverse only the *payment* portion of the stored delta (not the interest component)
     // so that interest already charged this month is not double-counted. For legacy rows
     // where interest_delta is NULL, fall back to reversing the full delta as before.
     const interestPortion = existing.interest_delta ?? 0;
-    const paymentPortion  = existing.balance_delta != null ? existing.balance_delta - interestPortion : null;
+    paymentPortion  = existing.balance_delta != null ? existing.balance_delta - interestPortion : null;
-    let restoredBalance = bill.current_balance;
+    restoredBalance = bill.current_balance;
     if (paymentPortion != null && bill.current_balance != null) {
       restoredBalance = Math.max(0, bill.current_balance - paymentPortion);
     }
@@ -481,38 +498,45 @@ router.put('/:id', (req, res) => {
     // interest_accrued_month is still set to this month (if interest was charged) so
     // computeBalanceDelta will skip interest when the payment is within the same month,
     // and charge a fresh month of interest if editing into a new calendar month.
-    const balCalc = computeBalanceDelta({ ...bill, current_balance: restoredBalance }, nextAmount);
+    balCalc = computeBalanceDelta({ ...bill, current_balance: restoredBalance }, nextAmount);
     nextBalanceDelta  = balCalc?.balance_delta ?? null;
     nextInterestDelta = balCalc?.interest_delta ?? null;
-    if (balCalc) {
-      applyBalanceDelta(db, existing.bill_id, balCalc);
-    } else if (paymentPortion != null && restoredBalance != null) {
-      db.prepare("UPDATE bills SET current_balance = ?, updated_at = datetime('now') WHERE id = ?")
-        .run(restoredBalance, existing.bill_id);
-    }
   }
 
   const { autopay_failure } = req.body;
   const nextAutopayFailure = autopay_failure !== undefined ? (autopay_failure ? 1 : 0) : existing.autopay_failure;
 
-  db.prepare(`
+  // Atomic: the bill-balance write and the payment UPDATE land together or neither,
-    UPDATE payments SET
+  // so an edit never leaves the balance reversed without the row updated (or vice versa).
-      amount = ?, paid_date = ?, method = ?, notes = ?, balance_delta = ?, interest_delta = ?,
+  const applyEdit = db.transaction(() => {
-      payment_source = ?, autopay_failure = ?, updated_at = datetime('now')
+    if (bill) {
-    WHERE id = ?
+      if (balCalc) {
-      AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)
+        applyBalanceDelta(db, existing.bill_id, balCalc);
-  `).run(
+      } else if (paymentPortion != null && restoredBalance != null) {
-    nextAmount,
+        db.prepare("UPDATE bills SET current_balance = ?, updated_at = datetime('now') WHERE id = ?")
-    nextPaidDate,
+          .run(restoredBalance, existing.bill_id);
-    method    !== undefined ? (method || null)  : existing.method,
+      }
-    notes     !== undefined ? (notes  || null)  : existing.notes,
+    }
-    nextBalanceDelta,
+    db.prepare(`
-    nextInterestDelta,
+      UPDATE payments SET
-    nextPaymentSource,
+        amount = ?, paid_date = ?, method = ?, notes = ?, balance_delta = ?, interest_delta = ?,
-    nextAutopayFailure,
+        payment_source = ?, autopay_failure = ?, updated_at = datetime('now')
-    req.params.id,
+      WHERE id = ?
-    req.user.id,
+        AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)
-  );
+    `).run(
+      nextAmount,
+      nextPaidDate,
+      method    !== undefined ? (method || null)  : existing.method,
+      notes     !== undefined ? (notes  || null)  : existing.notes,
+      nextBalanceDelta,
+      nextInterestDelta,
+      nextPaymentSource,
+      nextAutopayFailure,
+      req.params.id,
+      req.user.id,
+    );
+  });
+  applyEdit();
 
   res.json(serializePayment(db.prepare(`SELECT p.* FROM payments p JOIN bills b ON b.id = p.bill_id WHERE p.id = ? AND p.${SQL_NOT_DELETED} AND b.user_id = ? AND b.deleted_at IS NULL`).get(req.params.id, req.user.id)));
 });
@@ -524,24 +548,27 @@ router.delete('/:id', (req, res) => {
   if (!payment) return res.status(404).json(standardizeError('Payment not found', 'NOT_FOUND', 'id'));
   if (isTransactionLinkedPayment(payment)) return rejectTransactionLinkedPayment(res);
 
-  // Reverse any balance delta that was stored when this payment was created.
+  // Atomic: reverse the balance delta and soft-delete the payment together, so a
-  // If this payment was the one that charged interest this month, clear
+  // failure can't leave the balance restored while the payment is still active
+  // (or vice versa). If this payment charged interest this month, clear
   // interest_accrued_month so the next payment can re-accrue correctly.
-  if (!payment.accounting_excluded && payment.balance_delta != null) {
+  const softDeletePayment = db.transaction(() => {
-    const bill = db.prepare('SELECT current_balance FROM bills WHERE id = ? AND user_id = ? AND deleted_at IS NULL').get(payment.bill_id, req.user.id);
+    if (!payment.accounting_excluded && payment.balance_delta != null) {
-    if (bill?.current_balance != null) {
+      const bill = db.prepare('SELECT current_balance FROM bills WHERE id = ? AND user_id = ? AND deleted_at IS NULL').get(payment.bill_id, req.user.id);
-      const restored = Math.max(0, bill.current_balance - payment.balance_delta);
+      if (bill?.current_balance != null) {
-      db.prepare(`
+        const restored = Math.max(0, bill.current_balance - payment.balance_delta);
-        UPDATE bills
+        db.prepare(`
-        SET current_balance        = ?,
+          UPDATE bills
-            interest_accrued_month = CASE WHEN ? THEN NULL ELSE interest_accrued_month END,
+          SET current_balance        = ?,
-            updated_at             = datetime('now')
+              interest_accrued_month = CASE WHEN ? THEN NULL ELSE interest_accrued_month END,
-        WHERE id = ?
+              updated_at             = datetime('now')
-      `).run(restored, payment.interest_delta != null ? 1 : 0, payment.bill_id);
+          WHERE id = ?
+        `).run(restored, payment.interest_delta != null ? 1 : 0, payment.bill_id);
+      }
     }
-  }
+    db.prepare("UPDATE payments SET deleted_at = datetime('now') WHERE id = ? AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)").run(req.params.id, req.user.id);
-
+  });
-  db.prepare("UPDATE payments SET deleted_at = datetime('now') WHERE id = ? AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)").run(req.params.id, req.user.id);
+  softDeletePayment();
   res.json({ success: true });
 });
 
@@ -552,25 +579,29 @@ router.post('/:id/restore', (req, res) => {
   if (!payment) return res.status(404).json(standardizeError('Deleted payment not found', 'NOT_FOUND', 'id'));
   if (isTransactionLinkedPayment(payment)) return rejectTransactionLinkedPayment(res);
 
-  // Re-apply the balance delta (undo the reversal done on delete).
+  // Atomic: re-apply the balance delta and un-delete the payment together, so a
-  // If this payment originally charged interest, restore interest_accrued_month
+  // failure can't leave the balance re-applied while the payment stays deleted
-  // to the month of the payment so future same-month payments skip interest.
+  // (or vice versa). If this payment originally charged interest, restore
-  if (!payment.accounting_excluded && payment.balance_delta != null) {
+  // interest_accrued_month to the payment's month so future same-month payments
-    const bill = db.prepare('SELECT current_balance FROM bills WHERE id = ? AND user_id = ? AND deleted_at IS NULL').get(payment.bill_id, req.user.id);
+  // skip interest.
-    if (bill?.current_balance != null) {
+  const restorePayment = db.transaction(() => {
-      const reapplied    = Math.max(0, bill.current_balance + payment.balance_delta);
+    if (!payment.accounting_excluded && payment.balance_delta != null) {
-      const interestMonth = payment.interest_delta != null ? (payment.paid_date?.slice(0, 7) ?? null) : null;
+      const bill = db.prepare('SELECT current_balance FROM bills WHERE id = ? AND user_id = ? AND deleted_at IS NULL').get(payment.bill_id, req.user.id);
-      db.prepare(`
+      if (bill?.current_balance != null) {
-        UPDATE bills
+        const reapplied    = Math.max(0, bill.current_balance + payment.balance_delta);
-        SET current_balance        = ?,
+        const interestMonth = payment.interest_delta != null ? (payment.paid_date?.slice(0, 7) ?? null) : null;
-            interest_accrued_month = CASE WHEN ? IS NOT NULL THEN ? ELSE interest_accrued_month END,
+        db.prepare(`
-            updated_at             = datetime('now')
+          UPDATE bills
-        WHERE id = ?
+          SET current_balance        = ?,
-      `).run(reapplied, interestMonth, interestMonth, payment.bill_id);
+              interest_accrued_month = CASE WHEN ? IS NOT NULL THEN ? ELSE interest_accrued_month END,
+              updated_at             = datetime('now')
+          WHERE id = ?
+        `).run(reapplied, interestMonth, interestMonth, payment.bill_id);
+      }
     }
-  }
+    db.prepare('UPDATE payments SET deleted_at = NULL WHERE id = ? AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)').run(req.params.id, req.user.id);
-
+  });
-  db.prepare('UPDATE payments SET deleted_at = NULL WHERE id = ? AND bill_id IN (SELECT id FROM bills WHERE user_id = ? AND deleted_at IS NULL)').run(req.params.id, req.user.id);
+  restorePayment();
   res.json(serializePayment(db.prepare(`SELECT p.* FROM payments p JOIN bills b ON b.id = p.bill_id WHERE p.id = ? AND p.${SQL_NOT_DELETED} AND b.user_id = ? AND b.deleted_at IS NULL`).get(req.params.id, req.user.id)));
 });
 

tests/paymentsRoute.test.js

@@ -0,0 +1,100 @@
+'use strict';
+
+// Track A — the manual create / delete / restore / edit payment paths must apply
+// their balance mutation atomically with the row write (each is now wrapped in a
+// single db.transaction()). These tests pin the *observable* invariant the wrap
+// guarantees: the bill balance and the payment state stay consistent across a
+// create → delete → restore → edit round-trip, and a double-restore is a no-op.
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const os = require('node:os');
+const path = require('node:path');
+const fs = require('node:fs');
+
+const dbPath = path.join(os.tmpdir(), `bill-tracker-payments-${process.pid}.sqlite`);
+process.env.DB_PATH = dbPath;
+
+const { getDb, closeDb } = require('../db/database');
+const router = require('../routes/payments');
+
+function handler(method, routePath) {
+  const layer = router.stack.find(l => l.route?.path === routePath && l.route.methods[method]);
+  assert.ok(layer, `${method.toUpperCase()} ${routePath} exists`);
+  return layer.route.stack[layer.route.stack.length - 1].handle;
+}
+function call(method, routePath, { userId, params = {}, body = {} } = {}) {
+  const h = handler(method, routePath);
+  return new Promise((resolve) => {
+    const req = { params, body, query: {}, user: { id: userId, role: 'user' } };
+    const res = {
+      statusCode: 200,
+      status(c) { this.statusCode = c; return this; },
+      json(d) { resolve({ status: this.statusCode, data: d }); },
+    };
+    h(req, res);
+  });
+}
+function balanceOf(billId) {
+  return getDb().prepare('SELECT current_balance FROM bills WHERE id = ?').get(billId).current_balance;
+}
+
+let userId, billId;
+test.before(() => {
+  const db = getDb();
+  userId = db.prepare("INSERT INTO users (username, password_hash, role, active) VALUES ('pay-user','x','user',1)").run().lastInsertRowid;
+  // $100 expected, $1,000 balance (cents), 0% interest so deltas are pure principal.
+  billId = db.prepare(
+    "INSERT INTO bills (user_id, name, due_day, expected_amount, current_balance, minimum_payment, interest_rate, active) VALUES (?, 'Loan', 1, 10000, 100000, 5000, 0, 1)",
+  ).run(userId).lastInsertRowid;
+});
+test.after(() => {
+  closeDb();
+  for (const s of ['', '-wal', '-shm']) { try { fs.unlinkSync(dbPath + s); } catch {} }
+});
+
+let paymentId;
+test('manual POST /payments creates the payment (201) and drops the balance once', async () => {
+  const { status, data } = await call('post', '/', {
+    userId, body: { bill_id: billId, amount: 100, paid_date: '2026-07-01' },
+  });
+  assert.equal(status, 201);
+  assert.equal(data.amount, 100, 'serialized amount in dollars');
+  paymentId = data.id;
+  assert.equal(balanceOf(billId), 90000, '1000.00 − 100.00 = 900.00 (cents)');
+  const count = getDb().prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c;
+  assert.equal(count, 1);
+});
+
+test('DELETE reverses the balance and soft-deletes atomically', async () => {
+  const { status, data } = await call('delete', '/:id', { userId, params: { id: paymentId } });
+  assert.equal(status, 200);
+  assert.equal(data.success, true);
+  assert.equal(balanceOf(billId), 100000, 'balance restored to 1000.00');
+  const active = getDb().prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c;
+  assert.equal(active, 0, 'payment is soft-deleted');
+});
+
+test('restore re-applies the balance drop atomically', async () => {
+  const { status } = await call('post', '/:id/restore', { userId, params: { id: paymentId } });
+  assert.equal(status, 200);
+  assert.equal(balanceOf(billId), 90000, 'balance dropped to 900.00 again');
+  const active = getDb().prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c;
+  assert.equal(active, 1, 'payment is active again');
+});
+
+test('double-restore is a no-op (404, balance unchanged)', async () => {
+  const before = balanceOf(billId);
+  const { status } = await call('post', '/:id/restore', { userId, params: { id: paymentId } });
+  assert.equal(status, 404, 'already-active payment cannot be restored again');
+  assert.equal(balanceOf(billId), before, 'balance not double-applied');
+});
+
+test('PUT edit reverses the old amount and applies the new one atomically', async () => {
+  const { status, data } = await call('put', '/:id', {
+    userId, params: { id: paymentId }, body: { amount: 50 },
+  });
+  assert.equal(status, 200);
+  assert.equal(data.amount, 50, 'amount updated to $50');
+  // reverse $100 (→ 1000.00) then apply $50 (→ 950.00)
+  assert.equal(balanceOf(billId), 95000, '1000.00 − 50.00 = 950.00 (cents)');
+});