fix(money): manual payment path flags suspected dupes (409) w/o losing legit ones (Track A)

The deliberate manual POST /payments had no dedupe guard (double-submit
made a second payment), but silently deduping it like the auto paths
would lose a legitimately-identical same-day payment — a money bug. So
the server returns 409 DUPLICATE_SUSPECTED (existing payment attached),
and a shared client helper (client/lib/paymentActions.ts, used by the
tracker inline cell, partial-payment dialog, and bill modal) turns it
into an 'add anyway?' toast that resends with allow_duplicate. Automated
one-click paths keep deduping silently (a repeat there is a retry).

Dropped a now-unused api import in PaymentLedgerDialog (Track E sweep).
Server suite 188 green; typecheck/lint/build clean; probe 17/17.

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

HISTORY.md

@@ -3,7 +3,8 @@
 
 ### 💰 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.)
+- **[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.
+- **[Money integrity] Manual `POST /payments` now flags a suspected duplicate instead of blindly creating one — without silently dropping a legitimate one.** The deliberate manual-add path had *no* dedupe guard, so a double-submit created a second payment. But unlike the automated one-click paths (`/quick`, `/bulk`, autopay-confirm — which dedupe *silently* because a repeat there is a retry), the manual path may legitimately record two identical payments on the same day, so silently deduping would *lose a real payment* — itself a money bug. Resolution: the server returns **`409 DUPLICATE_SUSPECTED`** (with the existing payment attached) on a same `bill+date+amount`; a shared client helper (`client/lib/paymentActions.ts`, used by the tracker inline cell, the partial-payment ledger dialog, and the bill modal) turns that into an **"add anyway?"** toast that resends with `allow_duplicate`. Best of both: retry-safety on the auto paths, no lost-write on the deliberate one. (A DB `UNIQUE` index was rejected — it would forbid legitimately-identical payments; an idempotency-key header is the documented upgrade if this ever goes multi-process.) Full server suite 188 green; e2e probe 17/17. (Track A)
 
 ### 🔷 TypeScript foundation + branded money types
 

client/components/BillModal.tsx

@@ -16,6 +16,7 @@ import {
   Select, SelectTrigger, SelectValue, SelectContent, SelectItem,
 } from '@/components/ui/select';
 import { api } from '@/api';
+import { createPaymentOrConfirm } from '@/lib/paymentActions';
 import { cn, todayStr, errMessage } from '@/lib/utils';
 import DebtDetailsSection from '@/components/bill-modal/DebtDetailsSection';
 import AutopayTrustIndicator from '@/components/bill-modal/AutopayTrustIndicator';
@@ -368,17 +369,22 @@ export default function BillModal({ bill, initialBill, categories, onClose, onSa
         notes: paymentNotes || null,
         payment_source: 'manual',
       };
+      const finishPaymentSave = async (successMessage: string) => {
+        toast.success(successMessage);
+        resetPaymentForm();
+        setPaymentFormOpen(false);
+        await loadPayments();
+        onSave?.();
+      };
       if (editingPayment) {
         await api.updatePayment(editingPayment.id, payload);
-        toast.success('Payment updated');
+        await finishPaymentSave('Payment updated');
       } else {
-        await api.createPayment({ ...payload, bill_id: bill?.id });
+        await createPaymentOrConfirm(
-        toast.success('Payment added');
+          { ...payload, bill_id: bill?.id },
+          () => finishPaymentSave('Payment added'),
+        );
       }
-      resetPaymentForm();
-      setPaymentFormOpen(false);
-      await loadPayments();
-      onSave?.();
     } catch (err) {
       toast.error(errMessage(err, 'Payment could not be saved.'));
     } finally {

client/components/tracker/EditableCell.tsx

@@ -1,6 +1,7 @@
 import { useState, useRef } from 'react';
 import { toast } from 'sonner';
 import { api } from '@/api';
+import { createPaymentOrConfirm } from '@/lib/paymentActions';
 import { cn, fmt, fmtDate, errMessage } from '@/lib/utils';
 import { Input } from '@/components/ui/input';
 import type { Dollars } from '@/lib/money';
@@ -47,15 +48,18 @@ export function EditableCell({ row, field, threshold, defaultPaymentDate, refres
         if (field === 'amount') update.amount    = parseFloat(val);
         if (field === 'date')   update.paid_date = val;
         await api.updatePayment(row.payments[0]!.id, update);
+        toast.success('Saved');
+        refresh();
       } else {
-        await api.createPayment({
+        await createPaymentOrConfirm(
-          bill_id:   row.id,
+          {
-          amount:    field === 'amount' ? parseFloat(val) : threshold,
+            bill_id:   row.id,
-          paid_date: field === 'date'   ? val             : defaultPaymentDate,
+            amount:    field === 'amount' ? parseFloat(val) : threshold,
-        });
+            paid_date: field === 'date'   ? val             : defaultPaymentDate,
+          },
+          () => { toast.success('Saved'); refresh(); },
+        );
       }
-      toast.success('Saved');
-      refresh();
     } catch (err) {
       toast.error(errMessage(err));
     }

client/components/tracker/PaymentLedgerDialog.tsx

@@ -1,7 +1,7 @@
 import { useState } from 'react';
 import { Plus } from 'lucide-react';
 import { toast } from 'sonner';
-import { api } from '@/api';
+import { createPaymentOrConfirm } from '@/lib/paymentActions';
 import { fmt, fmtDate, errMessage } from '@/lib/utils';
 import { METHOD_NONE, paymentSummary } from '@/lib/trackerUtils';
 import { Button } from '@/components/ui/button';
@@ -53,16 +53,16 @@ export function PaymentLedgerDialog({ row, year, month, threshold, defaultPaymen
 
     setBusy(true);
     try {
-      await api.createPayment({
+      await createPaymentOrConfirm(
-        bill_id: row.id,
+        {
-        amount: parsedAmount,
+          bill_id: row.id,
-        paid_date: date,
+          amount: parsedAmount,
-        method: method === METHOD_NONE ? null : method,
+          paid_date: date,
-        notes: notes || null,
+          method: method === METHOD_NONE ? null : method,
-      });
+          notes: notes || null,
-      toast.success('Partial payment added');
+        },
-      onSaved?.();
+        () => { toast.success('Partial payment added'); onSaved?.(); onClose?.(); },
-      onClose?.();
+      );
     } catch (err) {
       toast.error(errMessage(err, 'Failed to add payment'));
     } finally {

client/lib/paymentActions.ts

@@ -0,0 +1,43 @@
+import { api, type ApiError } from '@/api';
+import { toast } from 'sonner';
+import { errMessage } from '@/lib/utils';
+
+/**
+ * Create a manual payment, treating the server's "duplicate suspected" (409) as a
+ * question rather than an error. The deliberate manual-add path must never
+ * *silently* drop a legitimately-identical second payment (same bill, date,
+ * amount) — losing a real payment is itself a money-integrity bug. So the server
+ * returns `409 DUPLICATE_SUSPECTED` instead of silently deduping, and here we
+ * surface an "add anyway?" toast; confirming resends with `allow_duplicate`.
+ *
+ * `onSuccess` runs after any *real* creation — immediate OR confirmed-retry — so
+ * callers put their toast/refresh/close there. Any non-duplicate error re-throws
+ * for the caller's existing `catch`.
+ */
+export async function createPaymentOrConfirm(
+  payload: Record<string, unknown>,
+  onSuccess: () => void | Promise<void>,
+): Promise<void> {
+  const create = async (allowDuplicate: boolean) => {
+    await api.createPayment(allowDuplicate ? { ...payload, allow_duplicate: true } : payload);
+    await onSuccess();
+  };
+  try {
+    await create(false);
+  } catch (err) {
+    const e = err as ApiError;
+    if (e?.status === 409 && e?.code === 'DUPLICATE_SUSPECTED') {
+      toast.warning('A matching payment already exists for this bill, date, and amount.', {
+        description: 'Add it anyway?',
+        action: {
+          label: 'Add anyway',
+          onClick: () => {
+            create(true).catch(retryErr => toast.error(errMessage(retryErr, 'Failed to add payment')));
+          },
+        },
+      });
+      return; // handled — not an error the caller should re-toast
+    }
+    throw err;
+  }
+}

routes/payments.js

@@ -190,7 +190,7 @@ router.post('/:id/undo-auto', (req, res) => {
 // POST /api/payments — create single payment
 router.post('/', (req, res) => {
   const db = getDb();
-  const { bill_id, amount, paid_date, method, notes, payment_source, autopay_failure } = req.body;
+  const { bill_id, amount, paid_date, method, notes, payment_source, autopay_failure, allow_duplicate } = req.body;
 
   const validation = validatePaymentInput({ bill_id, amount, paid_date, payment_source: payment_source ?? 'manual' });
   if (validation.error) {
@@ -202,6 +202,26 @@ router.post('/', (req, res) => {
   if (!bill)
     return res.status(404).json(standardizeError('Bill not found', 'NOT_FOUND', 'bill_id'));
 
+  // The deliberate manual add must not *silently* drop a same-key payment — a user
+  // may legitimately record two identical payments on the same day, and dropping
+  // one is itself a money-integrity bug. Surface a 409 the client turns into an
+  // "add anyway?" confirm; a resend with allow_duplicate bypasses this. (The
+  // automated one-click paths — /quick, /bulk, autopay-confirm — dedupe silently
+  // instead, because a repeat there is a retry, not a second intent.)
+  if (!allow_duplicate) {
+    const existingDuplicate = db.prepare(
+      `SELECT p.* FROM payments p
+       WHERE p.bill_id = ? AND p.paid_date = ? AND p.amount = ? AND p.${SQL_NOT_DELETED}
+       ORDER BY p.id DESC LIMIT 1`
+    ).get(bill.id, payment.paid_date, payment.amount);
+    if (existingDuplicate) {
+      return res.status(409).json({
+        ...standardizeError('A matching payment already exists for this bill, date, and amount', 'DUPLICATE_SUSPECTED', 'amount'),
+        existing: serializePayment(existingDuplicate),
+      });
+    }
+  }
+
   const balCalc = computeBalanceDelta(bill, payment.amount);
   const failureFlag = autopay_failure ? 1 : 0;
 

tests/paymentsRoute.test.js

@@ -98,3 +98,26 @@ test('PUT edit reverses the old amount and applies the new one atomically', asyn
   // reverse $100 (→ 1000.00) then apply $50 (→ 950.00)
   assert.equal(balanceOf(billId), 95000, '1000.00 − 50.00 = 950.00 (cents)');
 });
+
+test('manual POST of a same bill+date+amount is a suspected duplicate (409, no new row, balance unchanged)', async () => {
+  const before = balanceOf(billId);
+  const { status, data } = await call('post', '/', {
+    userId, body: { bill_id: billId, amount: 50, paid_date: '2026-07-01' },
+  });
+  assert.equal(status, 409);
+  assert.equal(data.code, 'DUPLICATE_SUSPECTED');
+  assert.ok(data.existing?.id, 'returns the existing payment so the client can show it');
+  assert.equal(balanceOf(billId), before, 'balance not touched by a rejected duplicate');
+  const active = getDb().prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c;
+  assert.equal(active, 1, 'still one payment');
+});
+
+test('manual POST with allow_duplicate bypasses the guard and records the second payment', async () => {
+  const { status } = await call('post', '/', {
+    userId, body: { bill_id: billId, amount: 50, paid_date: '2026-07-01', allow_duplicate: true },
+  });
+  assert.equal(status, 201);
+  assert.equal(balanceOf(billId), 90000, '950.00 − 50.00 = 900.00 (cents)');
+  const active = getDb().prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c;
+  assert.equal(active, 2, 'a legitimately-identical second payment is recorded');
+});