Flag over-billed bills on PO return + standing COGS on cancel (audit-C4, audit-H8)

[OneTwo3D]

Wave 1, item 4 of the workflow-audit remediation (epic onetwo3d-ims-r3xh). Phase-1 alert-only coverage for two purchasing-reversal gaps. Neither posts an accounting correction yet — phase 2 (credit memo / COGS journal) is tracked separately.

audit-C4 — PO returns don't flag the over-billed bill

returnPurchaseOrder reverses stock + FIFO cost layers but never touches supplier invoices recorded against the PO. Returning billed goods left the AP liability at the full amount silently.

• New pure helper computePurchaseOrderOverBilling() sums billed qty/value per PO line (across all bills) and compares against the quantity now kept (received − returned).
• On return, when a billed line is over-billed, emits a return_overbilled_bill WARNING activity log with machine-readable metadata (per-line over-billed qty/value + bill references).
• PO detail page shows an amber alert naming the bills and the over-billed amount, telling the operator to raise a supplier credit.

audit-H8 — cancellation leaves consumed-unit COGS standing

reversePurchaseOrderCostLayersForCancellation only reverses remainingQty; units already sold/used keep COGS sourced from the cancelled receipt — P&L silently carries it.

• New summarizeConsumedCostLayers() (pure) + readPurchaseOrderConsumedCostForCancellation() compute consumed = receivedQty − remainingQty and its value.
• The cancellation service reads this before the reversal runs, so the about-to-be-zeroed remaining qty isn't miscounted as consumed (covered by a test that asserts read-before-reversal ordering).
• When consumed qty is non-zero: emits a cancelled_consumed_cogs_standing WARNING, returns consumedCost on the result, and the client shows an amber post-cancel alert. Cancellation stays fully blocked once an invoice exists (unchanged).

Tests

• purchasing-reversal-alerts.test.ts — 6 cases (over-billing across multiple bills, no-over-billing, unbilled PO, freight lines with no poLineId).
• po-cancellation.test.ts — summarizeConsumedCostLayers totals + the service WARNING/consumedCost/read-ordering path; existing idempotency test updated for the new dep + result shape.
• 86/86 purchasing suite, type-check + lint clean.

Closes onetwo3d-ims-pbu4, onetwo3d-ims-j5ic.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[OneTwo3D]

Codex adversarial review — findings + fixes (commit 5f4e3b7)

Codex was at its usage cap; the adversarial pass ran by hand with equal rigour. It found two HIGH issues plus several MEDIUM/LOW — all addressed.

HIGH — C4 over-billing read after commit (race). The over-billing figure was re-read from the DB after the return transaction closed, so a concurrent second return could increment qtyReturned again and inflate the logged figure. Fixed: the read now happens inside the transaction (so qtyReturned reflects exactly this return), and the summary is returned from db.$transaction; the WARNING is logged outside, isolated in a try/catch.

HIGH — consumed-COGS WARNING could fail a committed cancellation. The new cancelled_consumed_cogs_standing logActivity sat in the outer try with no local guard — a log failure after the PO was already cancelled/committed would propagate to the outer catch and return { success: false }, desyncing UI from DB. Fixed: wrapped in its own try/catch (matching the existing enqueueStockSync pattern).

MEDIUM — over-billing named every bill on the PO. bills and the WARNING's "across N bill(s)" counted all invoices, not just those that billed an over-billed line — sending finance to investigate correctly-billed invoices. Fixed: tracks contributing invoiceIds and names only those. New test names only the bills that billed an over-billed line.

MEDIUM — alert heading was backwards. "Returned goods exceed what was billed back" described the opposite scenario. Fixed to "Supplier bill exceeds goods kept after return."

MEDIUM — negative netReceived inflated over-billing. A corrupt qtyReturned > qtyReceived made netReceived negative, so billed − netReceived over-stated the figure. Fixed: netReceived clamps to >= 0. New test clamps a corrupt qtyReturned > qtyReceived.

MEDIUM — hardcoded £. Both WARNING descriptions hardcoded the GBP symbol though the values are base-currency. Fixed: dropped the symbol, labelled "(base currency)". (UI alerts already format with the real base-currency symbol via formatMoney.)

LOW — clarity/consistency. UI bills line now states the totals are gross (incl. tax); added a comment on why the consumed-cost reader keeps ORDER BY and omits FOR UPDATE (consumed portion is immutable, same tx, read before the reversal locks remaining layers); switched the over-billing/consumed thresholds to lte(0) for Decimal-exact comparison.

Verified-clean angles (no action needed)

• Decimal serialization across RSC→client: both summaries are fully string-typed; no Decimal crosses the boundary.
• Read-before-reversal (H8): consumed read and reversal are in the same tx; the read runs first and sees pre-reversal remainingQty. Test asserts the ordering.
• Opening-stock / production layers: excluded via poLineId = ANY(...) — correct, they aren't PO-originated.
• SQL column casing + ANY($)::text[]: match schema; same pattern as the existing reversal query.
• Cancellation still fully blocked once an invoice exists (unchanged).

Validation

• purchasing-reversal-alerts.test.ts 8 + po-cancellation.test.ts consumed-cost/WARNING/ordering paths; 88/88 purchasing suite.
• type-check + lint clean.

Deferred to phase 2 (alert-only scope, accepted)

• No credit-memo / COGS-correction journal is posted yet (tracked separately).
• Over-billing WARNING re-fires on each partial return (no dedup); the alert persists on the page until the PO state changes, since the supplier credit is external to IMS. Documented in docs/workflows.md.