From 369b3d0fef99a9d3cae3ccdbc2577a9b8d8ce04a Mon Sep 17 00:00:00 2001
From: OneTwo3D IMS <dev@onetwo3d.com>
Date: Sat, 13 Jun 2026 17:00:57 +0000
Subject: [PATCH 1/2] fix(sales): order-to-cash medium gaps (closes
 audit-M-o2c, audit-M-sales)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cluster of order-to-cash medium findings (47fa + chc2, duplicate clusters):

1. Refund tolerance: the cumulative-refunded check used a 0.1% RELATIVE slack
   (so a large order had pounds of headroom, and partial refunds could creep
   over). New pure refundWouldExceedOrderTotal() applies a fixed rounding epsilon
   (0.011) to the cumulative total instead.
2. Deleting the last payment cleared paidAt but never reverted status. deletePayment
   now detects when the order became not-fully-paid while in an advanced status
   (SHIPPED/COMPLETED/DELIVERED/PARTIALLY_REFUNDED) and emits a payment_status_mismatch
   WARNING; the SO detail shows an amber '<status> but not paid' chip.
3. applySalesOrderStatusTransition now rejects status edits on ARCHIVED orders
   (manual + sessionless-cron callers); the WooCommerce force-sync bypass token
   keeps its escape-hatch semantics.
4. queueSalesInvoiceForOrder idempotency key — VERIFIED already payload-based
   (accountingPayloadKey(..., payload)); no change needed.
5. confirmSalesOrderShipments double-shipment — VERIFIED already guarded (it nets
   committed non-PENDING shipment lines out of the allocation before recreating);
   no change needed.
6. Documented COMPLETED vs DELIVERED semantics + the archived/payment-mismatch
   behaviour in docs/workflows.md.

Shared pure o2c-guards.ts (refundWouldExceedOrderTotal, isPaymentStatusMismatch,
PAID_EXPECTED_SALES_STATUSES) reused by both call sites.

Tests: 5 guard cases (refund within/at/over total incl. the relative-vs-fixed
contrast; mismatch true/false); 105/105 sales suite; type-check + lint clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../sales/[id]/so-detail-client.tsx           |  7 +++
 app/actions/sales.ts                          | 30 ++++++++++++-
 docs/workflows.md                             | 14 ++++++
 lib/domain/sales/o2c-guards.ts                | 43 +++++++++++++++++++
 lib/domain/sales/refund-service.ts            |  6 ++-
 tests/domain/sales/o2c-guards.test.ts         | 38 ++++++++++++++++
 6 files changed, 135 insertions(+), 3 deletions(-)
 create mode 100644 lib/domain/sales/o2c-guards.ts
 create mode 100644 tests/domain/sales/o2c-guards.test.ts

diff --git a/app/(dashboard)/sales/[id]/so-detail-client.tsx b/app/(dashboard)/sales/[id]/so-detail-client.tsx
index 232ba6fa..9acf8fa9 100644
--- a/app/(dashboard)/sales/[id]/so-detail-client.tsx
+++ b/app/(dashboard)/sales/[id]/so-detail-client.tsx
@@ -1007,6 +1007,13 @@ export function SoDetailClient({ order: so, warehouses, currencies, externalOrde
               </button>
             </span>
           )}
+          {/* audit-M-o2c: an order advanced past payment but no longer fully paid
+              (e.g. its last payment was deleted) — a status/payment mismatch. */}
+          {!so.paidAt && ['SHIPPED', 'COMPLETED', 'DELIVERED', 'PARTIALLY_REFUNDED'].includes(so.status) && (
+            <span className="inline-flex items-center gap-1 rounded-full border border-amber-300 bg-amber-50 px-2 py-0.5 text-xs font-medium text-amber-900 dark:border-amber-700 dark:bg-amber-950 dark:text-amber-200">
+              <AlertTriangle className="h-3 w-3" /> {STATUS_LABELS[so.status]} but not paid
+            </span>
+          )}
 
           {canRefund && (
             <Button type="button" variant="outline" size="sm" onClick={() => setShowRefund(true)} disabled={isPending}>
diff --git a/app/actions/sales.ts b/app/actions/sales.ts
index b8f5c895..c8617067 100644
--- a/app/actions/sales.ts
+++ b/app/actions/sales.ts
@@ -47,6 +47,7 @@ import {
 } from '@/lib/domain/sales/sales-order-tax-validation'
 import { isExternalRefundIdUniqueConflict } from '@/lib/domain/sales/refund-idempotency'
 import { shouldWarnPaidWithoutInvoice } from '@/lib/domain/sales/paid-without-invoice'
+import { isPaymentStatusMismatch } from '@/lib/domain/sales/o2c-guards'
 import {
   cancelSalesOrderFulfillmentState,
   updateSalesOrderStatusUnderLock,
@@ -1245,12 +1246,19 @@ export async function applySalesOrderStatusTransition(
         orderNumber: true,
         externalOrderNumber: true,
         status: true,
+        archived: true,
         shipFromWarehouseId: true,
         shoppingLinks: { where: { connector: 'woocommerce' }, select: { id: true }, take: 1 },
         lines: { select: { id: true, productId: true, sku: true, qty: true } },
       },
     })
     if (!so) return { success: false, error: 'Order not found' }
+    // audit-M-o2c: an archived order is filed away — block status edits. The
+    // internalBypassToken (WooCommerce force-sync) keeps its escape-hatch
+    // semantics; manual and sessionless-cron callers are rejected.
+    if (so.archived && !bypassPermission) {
+      return { success: false, error: 'This order is archived; unarchive it before changing its status.' }
+    }
 
     const transition = validateManualSalesOrderStatusTransition(so.status, targetStatus, {
       bypass: bypassPermission,
@@ -2390,6 +2398,7 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
           externalOrderNumber: true,
           currency: true,
           totalForeign: true,
+          status: true,
         },
       })
       if (!so) return { error: 'Order not found' }
@@ -2401,6 +2410,7 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
         return { error: 'Payment not found for this order' }
       }
       await tx.payment.delete({ where: { id: paymentId } })
+      let becameUnpaid = false
       if (!payment.refundId) {
         const remainingPayments = await tx.payment.findMany({
           where: { orderId, refundId: null },
@@ -2410,12 +2420,14 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
           if (p.currency !== so.currency) return sum
           return sum + Number(p.amount)
         }, 0)
+        const stillFullyPaid = totalPaid >= Number(so.totalForeign) - 0.0001
+        becameUnpaid = !stillFullyPaid
         await tx.salesOrder.update({
           where: { id: orderId },
-          data: { paidAt: totalPaid >= Number(so.totalForeign) - 0.0001 ? undefined : null },
+          data: { paidAt: stillFullyPaid ? undefined : null },
         })
       }
-      return { so, payment: { refundId: payment.refundId, amount: Number(payment.amount), currency: payment.currency } }
+      return { so, becameUnpaid, payment: { refundId: payment.refundId, amount: Number(payment.amount), currency: payment.currency } }
     }, STOCK_TX_OPTIONS)
     if ('error' in txResult) return { success: false, error: txResult.error }
     if (!txResult.payment.refundId) {
@@ -2461,6 +2473,20 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
       description: `Deleted payment from order ${getSalesOrderReference(txResult.so)}`,
       metadata: { orderNumber: getSalesOrderReference(txResult.so), paymentId },
     })
+    // audit-M-o2c: deleting the last payment clears paidAt but does not revert
+    // status — flag the mismatch when the order has already advanced past
+    // payment (shipped/completed) so it doesn't sit silently unpaid-but-shipped.
+    if (isPaymentStatusMismatch(txResult.so.status, txResult.becameUnpaid)) {
+      await logActivity({
+        entityType: 'SALES_ORDER',
+        entityId: orderId,
+        action: 'payment_status_mismatch',
+        tag: 'sales',
+        level: 'WARNING',
+        description: `Order ${getSalesOrderReference(txResult.so)} is ${txResult.so.status} but is no longer fully paid after deleting a payment. Review whether the status should be reverted.`,
+        metadata: { orderNumber: getSalesOrderReference(txResult.so), status: txResult.so.status, paymentId },
+      })
+    }
     return { success: true }
   } catch (e) {
     await logActivity({
diff --git a/docs/workflows.md b/docs/workflows.md
index 19e254ce..86996091 100644
--- a/docs/workflows.md
+++ b/docs/workflows.md
@@ -41,6 +41,20 @@ state (e.g. cancelled or refunded after dispatch) between the poll's SHIPPED
 query and the under-lock write, the guard rejects the change and the cron logs a
 `delivery_status_skipped` warning instead of forcing it.
 
+**`COMPLETED` vs `DELIVERED`.** Both are manual, operator-set terminal-ish
+statuses reached from `SHIPPED` (and `COMPLETED → DELIVERED`). `DELIVERED` means
+the carrier confirmed delivery and can be set automatically by the delivery-status
+cron; `COMPLETED` is a manual "this order is done from our side" marker for
+businesses that don't track delivery (no automatic trigger sets it). Neither is
+forced by any sync; both still allow `PARTIALLY_REFUNDED`/`REFUNDED` afterwards.
+Use `DELIVERED` when delivery tracking is in play, `COMPLETED` otherwise.
+
+Status edits are rejected on **archived** orders (unarchive first); deleting the
+last payment on an order that has advanced past payment (`SHIPPED`/`COMPLETED`/
+`DELIVERED`/`PARTIALLY_REFUNDED`) does not auto-revert the status but raises a
+`payment_status_mismatch` warning and an amber chip on the order so the operator
+can decide.
+
 ### Shipments
 
 | Status | Allowed next statuses | Notes |
diff --git a/lib/domain/sales/o2c-guards.ts b/lib/domain/sales/o2c-guards.ts
new file mode 100644
index 00000000..c056e572
--- /dev/null
+++ b/lib/domain/sales/o2c-guards.ts
@@ -0,0 +1,43 @@
+// ---------------------------------------------------------------------------
+// Order-to-cash guard predicates (audit-M-o2c)
+//
+// Small pure helpers for the order-to-cash medium-gap fixes, extracted so the
+// numeric/threshold logic is unit-testable without a database.
+// ---------------------------------------------------------------------------
+
+export type SalesOrderStatusName = string
+
+/** Fixed rounding slack for the cumulative-refund-vs-total check (a hair over a penny). */
+export const REFUND_TOTAL_EPSILON = 0.011
+
+/**
+ * Statuses that have advanced past payment. An order in one of these that is no
+ * longer fully paid (e.g. after deleting its last payment) is a status/payment
+ * mismatch worth surfacing.
+ */
+export const PAID_EXPECTED_SALES_STATUSES: ReadonlySet<string> = new Set([
+  'SHIPPED',
+  'COMPLETED',
+  'DELIVERED',
+  'PARTIALLY_REFUNDED',
+])
+
+/**
+ * Whether recording `thisRefund` on top of `previouslyRefunded` would push the
+ * cumulative refunded amount past the order total (within a fixed rounding
+ * epsilon). Cumulative — not per-refund — so N partial refunds can't each creep
+ * over a relative slack.
+ */
+export function refundWouldExceedOrderTotal(
+  thisRefund: number,
+  previouslyRefunded: number,
+  orderTotal: number,
+  epsilon: number = REFUND_TOTAL_EPSILON,
+): boolean {
+  return thisRefund + previouslyRefunded > orderTotal + epsilon
+}
+
+/** Whether an order that just became not-fully-paid is in a status that expected payment. */
+export function isPaymentStatusMismatch(status: string, becameUnpaid: boolean): boolean {
+  return becameUnpaid && PAID_EXPECTED_SALES_STATUSES.has(status)
+}
diff --git a/lib/domain/sales/refund-service.ts b/lib/domain/sales/refund-service.ts
index 9440848c..74bc4c16 100644
--- a/lib/domain/sales/refund-service.ts
+++ b/lib/domain/sales/refund-service.ts
@@ -14,6 +14,7 @@ import { roundQuantity, toDecimal, type DecimalInput } from '@/lib/domain/math/d
 import { getSalesOrderReference } from '@/lib/sales-order-display'
 import { validateRefundSalesOrderStatusUpdate } from '@/lib/domain/workflows/action-guards'
 import { isFullRefundAmount } from '@/lib/domain/sales/refund-thresholds'
+import { refundWouldExceedOrderTotal } from '@/lib/domain/sales/o2c-guards'
 import {
   isStockMovementIdempotencyConflict,
   refundInboundMovementKey,
@@ -1342,7 +1343,10 @@ export async function createSalesOrderRefund(
       select: { totalBase: true },
     })
     const previouslyRefunded = existingRefunds.reduce((sum, refund) => sum + refundBoundaryNumber(refund.totalBase), 0)
-    if (totalBase + previouslyRefunded > refundBoundaryNumber(so.totalBase) * 1.001) {
+    // audit-M-o2c: cumulative refunded must not exceed the order total, with a
+    // fixed rounding epsilon (not a 0.1% relative slack, which on a large order
+    // is pounds of headroom) so N partial refunds can't creep over.
+    if (refundWouldExceedOrderTotal(totalBase, previouslyRefunded, refundBoundaryNumber(so.totalBase))) {
       return { error: 'Refund total would exceed order total' } as const
     }
 
diff --git a/tests/domain/sales/o2c-guards.test.ts b/tests/domain/sales/o2c-guards.test.ts
new file mode 100644
index 00000000..7e69cd68
--- /dev/null
+++ b/tests/domain/sales/o2c-guards.test.ts
@@ -0,0 +1,38 @@
+import assert from 'node:assert/strict'
+import test from 'node:test'
+
+import {
+  refundWouldExceedOrderTotal,
+  isPaymentStatusMismatch,
+  REFUND_TOTAL_EPSILON,
+} from '@/lib/domain/sales/o2c-guards'
+
+test('refund within the order total is allowed', () => {
+  assert.equal(refundWouldExceedOrderTotal(40, 50, 100), false)
+})
+
+test('refund exactly at the order total is allowed (epsilon slack)', () => {
+  assert.equal(refundWouldExceedOrderTotal(50, 50, 100), false)
+})
+
+test('cumulative partial refunds cannot creep over the total (fixed epsilon, not relative)', () => {
+  // Old 0.1% relative slack on a £10,000 order allowed ~£10 over; the fixed
+  // epsilon rejects anything beyond a rounding penny.
+  assert.equal(refundWouldExceedOrderTotal(1, 10000, 10000), true)
+  // A sub-penny rounding remainder is still tolerated.
+  assert.equal(refundWouldExceedOrderTotal(0.01, 9999.99, 10000), false)
+  assert.ok(REFUND_TOTAL_EPSILON < 0.02)
+})
+
+test('isPaymentStatusMismatch: advanced status + became unpaid → mismatch', () => {
+  assert.equal(isPaymentStatusMismatch('SHIPPED', true), true)
+  assert.equal(isPaymentStatusMismatch('COMPLETED', true), true)
+  assert.equal(isPaymentStatusMismatch('DELIVERED', true), true)
+  assert.equal(isPaymentStatusMismatch('PARTIALLY_REFUNDED', true), true)
+})
+
+test('isPaymentStatusMismatch: pre-payment status or still-paid → no mismatch', () => {
+  assert.equal(isPaymentStatusMismatch('PROCESSING', true), false)
+  assert.equal(isPaymentStatusMismatch('ALLOCATED', true), false)
+  assert.equal(isPaymentStatusMismatch('SHIPPED', false), false)
+})

From c0c051fa43448489be9e5751a233b0f93538e232 Mon Sep 17 00:00:00 2001
From: OneTwo3D IMS <dev@onetwo3d.com>
Date: Sat, 13 Jun 2026 17:10:31 +0000
Subject: [PATCH 2/2] =?UTF-8?q?fix(sales):=20harden=20o2c=20guards=20?=
 =?UTF-8?q?=E2=80=94=20cron-on-archived,=20false=20mismatch,=20chip=20(adv?=
 =?UTF-8?q?ersarial=20review)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Codex adversarial pass on PR #204:

HIGH — the archived guard blocked the sessionless delivery cron, so an
archived-but-SHIPPED order could never auto-reach DELIVERED. The guard now also
lets skipPermissionCheck (the cron) through alongside the WC force-sync token;
only genuinely-manual edits are rejected.

HIGH — payment_status_mismatch fired even when the order was NEVER fully paid
(e.g. shipped on credit, a held partial payment deleted) because becameUnpaid
only checked 'not fully paid now'. Now requires a genuine paid→unpaid transition
(so.paidAt was set before the delete).

MEDIUM — the read-time amber chip keyed on !so.paidAt false-positived on
credit-term and WC-imported-partial-refund orders and contradicted the existing
Part. Paid indicator. Dropped it: a read-time chip can't distinguish 'never paid'
from 'was paid then unpaid' without a persisted marker, so the accurate signal is
the (now transition-gated) WARNING activity log plus the existing Paid/Part. Paid
chips. Docs updated to match.

LOW — added a single-shot full-refund test.

Verified sound: refund epsilon currency basis (all base); WC bypass intact;
refund-payment deletions don't flag; invoice idempotency key + confirmShipments
netting confirmed.

7/7 guard tests; type-check + lint clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 app/(dashboard)/sales/[id]/so-detail-client.tsx | 13 ++++++-------
 app/actions/sales.ts                            | 16 +++++++++++-----
 docs/workflows.md                               | 12 +++++++-----
 tests/domain/sales/o2c-guards.test.ts           |  5 +++++
 4 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/app/(dashboard)/sales/[id]/so-detail-client.tsx b/app/(dashboard)/sales/[id]/so-detail-client.tsx
index 9acf8fa9..bc63c5e0 100644
--- a/app/(dashboard)/sales/[id]/so-detail-client.tsx
+++ b/app/(dashboard)/sales/[id]/so-detail-client.tsx
@@ -1007,13 +1007,12 @@ export function SoDetailClient({ order: so, warehouses, currencies, externalOrde
               </button>
             </span>
           )}
-          {/* audit-M-o2c: an order advanced past payment but no longer fully paid
-              (e.g. its last payment was deleted) — a status/payment mismatch. */}
-          {!so.paidAt && ['SHIPPED', 'COMPLETED', 'DELIVERED', 'PARTIALLY_REFUNDED'].includes(so.status) && (
-            <span className="inline-flex items-center gap-1 rounded-full border border-amber-300 bg-amber-50 px-2 py-0.5 text-xs font-medium text-amber-900 dark:border-amber-700 dark:bg-amber-950 dark:text-amber-200">
-              <AlertTriangle className="h-3 w-3" /> {STATUS_LABELS[so.status]} but not paid
-            </span>
-          )}
+          {/* audit-M-o2c: the paid→unpaid mismatch from deleting a payment on an
+              advanced-status order is recorded as a payment_status_mismatch WARNING
+              activity log (the durable, accurate signal). A read-time chip on
+              `!paidAt` alone can't tell "shipped on credit, never paid" from
+              "was paid then unpaid", so it isn't shown here; the existing
+              Paid / Part. Paid indicators cover the payment state. */}
 
           {canRefund && (
             <Button type="button" variant="outline" size="sm" onClick={() => setShowRefund(true)} disabled={isPending}>
diff --git a/app/actions/sales.ts b/app/actions/sales.ts
index c8617067..2e516f5d 100644
--- a/app/actions/sales.ts
+++ b/app/actions/sales.ts
@@ -1253,10 +1253,12 @@ export async function applySalesOrderStatusTransition(
       },
     })
     if (!so) return { success: false, error: 'Order not found' }
-    // audit-M-o2c: an archived order is filed away — block status edits. The
-    // internalBypassToken (WooCommerce force-sync) keeps its escape-hatch
-    // semantics; manual and sessionless-cron callers are rejected.
-    if (so.archived && !bypassPermission) {
+    // audit-M-o2c: an archived order is filed away — block MANUAL status edits.
+    // Automated data-pushes still apply: the WooCommerce force-sync
+    // (internalBypassToken) and the sessionless delivery cron (skipPermissionCheck)
+    // are carrier/source-of-truth signals, so they bypass this guard — otherwise an
+    // archived-but-shipped order could never auto-reach DELIVERED.
+    if (so.archived && !bypassPermission && !options?.skipPermissionCheck) {
       return { success: false, error: 'This order is archived; unarchive it before changing its status.' }
     }
 
@@ -2399,6 +2401,7 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
           currency: true,
           totalForeign: true,
           status: true,
+          paidAt: true,
         },
       })
       if (!so) return { error: 'Order not found' }
@@ -2421,7 +2424,10 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
           return sum + Number(p.amount)
         }, 0)
         const stillFullyPaid = totalPaid >= Number(so.totalForeign) - 0.0001
-        becameUnpaid = !stillFullyPaid
+        // Only a genuine paid → not-paid transition is a mismatch. An order that
+        // was never fully paid (e.g. shipped on credit terms) isn't flagged just
+        // because a partial payment was removed.
+        becameUnpaid = so.paidAt !== null && !stillFullyPaid
         await tx.salesOrder.update({
           where: { id: orderId },
           data: { paidAt: stillFullyPaid ? undefined : null },
diff --git a/docs/workflows.md b/docs/workflows.md
index 86996091..e24d2df6 100644
--- a/docs/workflows.md
+++ b/docs/workflows.md
@@ -49,11 +49,13 @@ businesses that don't track delivery (no automatic trigger sets it). Neither is
 forced by any sync; both still allow `PARTIALLY_REFUNDED`/`REFUNDED` afterwards.
 Use `DELIVERED` when delivery tracking is in play, `COMPLETED` otherwise.
 
-Status edits are rejected on **archived** orders (unarchive first); deleting the
-last payment on an order that has advanced past payment (`SHIPPED`/`COMPLETED`/
-`DELIVERED`/`PARTIALLY_REFUNDED`) does not auto-revert the status but raises a
-`payment_status_mismatch` warning and an amber chip on the order so the operator
-can decide.
+Manual status edits are rejected on **archived** orders (unarchive first);
+automated pushes (WooCommerce force-sync, the delivery-status cron) still apply.
+Deleting a payment that takes an already-paid order in an advanced status
+(`SHIPPED`/`COMPLETED`/`DELIVERED`/`PARTIALLY_REFUNDED`) back below fully-paid
+does not auto-revert the status but raises a `payment_status_mismatch` warning
+activity log so the operator can decide (it fires only on a genuine paid→unpaid
+transition, not for orders that were never fully paid, e.g. credit terms).
 
 ### Shipments
 
diff --git a/tests/domain/sales/o2c-guards.test.ts b/tests/domain/sales/o2c-guards.test.ts
index 7e69cd68..54bdb2a3 100644
--- a/tests/domain/sales/o2c-guards.test.ts
+++ b/tests/domain/sales/o2c-guards.test.ts
@@ -15,6 +15,11 @@ test('refund exactly at the order total is allowed (epsilon slack)', () => {
   assert.equal(refundWouldExceedOrderTotal(50, 50, 100), false)
 })
 
+test('single-shot full refund (no prior refunds) is always allowed', () => {
+  assert.equal(refundWouldExceedOrderTotal(100, 0, 100), false)
+  assert.equal(refundWouldExceedOrderTotal(9999.99, 0, 9999.99), false)
+})
+
 test('cumulative partial refunds cannot creep over the total (fixed epsilon, not relative)', () => {
   // Old 0.1% relative slack on a £10,000 order allowed ~£10 over; the fixed
   // epsilon rejects anything beyond a rounding penny.