reconcilePayment Never Checks Amount, Marks Contracts Paid

Conclusion

A payment reconciliation / transaction-linking process marked a contract “paid” as long as the linking operation itself completed without error, never comparing the paid amount against the contract’s invoice total. Partial payments, overpayments, and linking a transaction to the wrong contract all completed normally, so there was no way to notice any of them. Following a code review finding (H-1), the discrepancy is now detected by a pure function, paymentDiscrepancy, and reported via a Slack alert and audit log — without blocking the reconciliation itself.

Symptom

Both reconcilePayment (reconciliation) and linkTransactionToContract (transaction linking) were implemented like this. Each cared only about successfully linking a transaction to a contract, and never read the amount at all.

// actions.ts (equivalent of reconcilePayment before the fix)
// Fetch the payment status before the change, as a double-send guard
// (so an already-paid contract doesn't get its receipt re-sent on re-reconciliation).
const { data: before } = await supabase
  .from("contracts")
  .select("payment_status")
  .eq("id", contractId)
  .maybeSingle();
// … (omitted: determining the payment date, etc.) …
// From here, payment_status is updated to "paid" without ever
// referencing the paid amount.

When staff picked a transaction from the payments list and clicked “reconcile,” the contract became “paid” as long as the link itself succeeded — no matter how far the chosen transaction’s amount was from the contract’s invoice total. Paying only ¥30,000 against a ¥100,000 invoice, or linking a completely different customer’s payment by mistake, produced the identical “paid” label on screen, with no error and no warning.

Cause

This code only looked at whether the operation of “linking a transaction to a contract” succeeded. It never performed the separate check of “does the linked transaction’s amount actually match that contract’s invoice total.” Whether the operation succeeds (the database update goes through without error) and whether it’s correct from a business standpoint (the paid amount matches the invoice total) are two different conditions, but the code treated the first as proof of the second. In billing logic, partial payments, overpayments, and mismatched transactions are all operations that “succeed” in the technical sense — without a separate check along the axis of amount, they structurally pass through silently.

The fix

A pure function, paymentDiscrepancy, was added to compute the difference between the tax-included invoice total and the paid amount. The contract’s amount column excludes tax, but what a customer actually pays is the invoice’s tax-included total, so comparing tax-included (gross) totals avoids false positives from tax-rate differences.

// billing.ts
export const PAYMENT_MISMATCH_TOLERANCE_YEN = 1000;
export function paymentDiscrepancy(
  contractAmount: number | null | undefined,
  taxIncluded: boolean,
  paidAmount: number | null | undefined,
  toleranceYen = PAYMENT_MISMATCH_TOLERANCE_YEN
): { gross: number; paid: number; diff: number; mismatched: boolean } | null {
  if (contractAmount == null || paidAmount == null) return null;
  const gross = calcTotalsFor(Number(contractAmount), taxIncluded).gross;
  const paid = Math.round(Number(paidAmount));
  const diff = paid - gross;
  return { gross, paid, diff, mismatched: Math.abs(diff) > toleranceYen };
}

The default tolerance (¥1,000) is set around the scale of a typical bank transfer fee. Warning on every small shortfall caused by a deducted transfer fee would desensitize staff to warnings and make them more likely to miss a discrepancy that actually matters.

This check was wired into both reconcilePayment and linkTransactionToContract, but the design deliberately does not block reconciliation itself.

// actions.ts (after the fix)
const disc = paymentDiscrepancy(
  before?.amount as number | null,
  (before?.tax_included as boolean | undefined) ?? false,
  txnAmount
);
if (disc?.mismatched) {
  const sign = disc.diff > 0 ? "overpaid" : "short";
  await notifySlack(
    `⚠️ Payment amount mismatch: invoice (tax incl.) ${formatYen(disc.gross)} / paid ${formatYen(
      disc.paid
    )} (${sign} by ${formatYen(Math.abs(disc.diff))}) ${slackLink("/admin/finance", "Review reconciliation")}`
  );
}

Making reconciliation fail-closed on a mismatch would also block legitimate partial payments, such as installment plans, breaking existing operations. So a detected discrepancy only triggers a Slack notification and an audit-log entry, while the operation itself still goes through. The screen (finance/page.tsx) was also updated to show the discrepancy as an amber warning, both on auto-suggested matches before reconciliation and on already-reconciled line items.

// page.tsx
{sugDisc?.mismatched && (
  <div className="mt-1 rounded bg-amber-50 px-2 py-1 text-[11px] text-amber-700">
    ⚠ Suggested payment amount {formatYen(sugDisc.paid)} differs from invoice
    (tax incl.) {formatYen(sugDisc.gross)} (
    {sugDisc.diff > 0 ? "overpaid" : "short"} by{" "}
    {formatYen(Math.abs(sugDisc.diff))}) — please verify before reconciling
  </div>
)}

Preventing a repeat

paymentDiscrepancy is factored out as a pure function with clear inputs and outputs, and 8 tests were added covering boundary values, tax handling, and tolerance to numbers passed as strings (part of the 551 tests that run continuously in CI). Because reconcilePayment, linkTransactionToContract, and the on-screen display all call the same function for the amount check, it is harder for the three call sites to drift apart if only one of them gets changed later.