fix(sales): route TrackShip delivery poll through status-transition path (closes audit-C2)

[OneTwo3D]

Summary

Closes onetwo3d-ims-erl0 (audit finding C2, Wave 1).

The gap: lib/trackship.ts:checkDeliveryStatus marked orders DELIVERED with a raw db.salesOrder.update — no transition guard, no side effects. A cron-detected delivery skipped everything the manual DELIVERED path runs (the status_changed activity log, the WooCommerce status push, cache revalidation), and could silently overwrite an order that had moved to CANCELLED/REFUNDED between the SHIPPED query and the write. Verified directly during the audit.

Fix

Delivery marking now routes through applySalesOrderStatusTransition(id, 'DELIVERED', undefined, { internalBypassToken }):

• The cron has no session, so it passes the internal bypass token to skip the sales.process permission check — but the transition guard and all side effects still run.
• The sales-order state machine only allows DELIVERED from SHIPPED/COMPLETED; an order that became CANCELLED/REFUNDED in the meantime is rejected, and the cron logs delivery_status_skipped (WARNING) instead of forcing the write.

Extracted the marking into markOrderDelivered(target, deps) with injected transition + log so the routing and skip-on-reject behaviour are unit-testable without the DB or external API.

Validation

• npx tsx --test tests/trackship-mark-delivered.test.ts — 2/2 pass (routes through transition with the bypass token; skips + warns on rejection)
• npm run type-check — clean
• npm run lint — clean

Notes

• docs/workflows.md now documents that cron-driven DELIVERED runs the same guard + side effects as a manual delivery.
• The delivered count returned by checkDeliveryStatus now reflects only orders that actually transitioned (rejected ones are not counted).

🤖 Generated with Claude Code

[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 39d9c9b)

Codex was at its usage cap, so the adversarial pass ran by hand with equal rigour. It found that the original commit did not actually deliver the protection it claimed — a HIGH that I'd otherwise have shipped.

HIGH — the guard was being bypassed, not enforced. The internal bypass token makes validateManualSalesOrderStatusTransition short-circuit { success: true }, skipping both the permission check and the state-machine guard (action-guards.ts:74). updateSalesOrderStatusUnderLock uses the same bypassed guard. So a SHIPPED → CANCELLED race between the poll's query and the write would still overwrite CANCELLED with DELIVERED — and my docs/PR claim that "the guard rejects the change" was false.

Fix: added a narrower skipPermissionCheck option to applySalesOrderStatusTransition that skips only the permission check; the state-machine guard still runs (pre-lock and under-lock). The WooCommerce status-sync path keeps the bypass token (skip-both) unchanged — it legitimately forces mapped statuses and already handles rejection, so no WC blast radius. TrackShip now uses skipPermissionCheck, so a since-cancelled/refunded order is genuinely rejected under the lock and logged delivery_status_skipped. The protection is now real.

MEDIUM — as never in the test masked a type mismatch (fixed). The deps transition type now matches the real signature (extra/options optional), the mock is structurally compatible, and the as never cast is gone — a future signature drift will now fail the test.

LOW — dynamic import inside the loop (fixed). Hoisted await import('@/app/actions/sales') above the per-order loop.

LOW — one order's failure aborted the batch (fixed). Wrapped the per-order mark in try/catch so a transient failure (e.g. a log write) no longer drops the remaining orders in the run.

Verified-clean angles (no action needed)

• No WC double-push: DELIVERED isn't in IMS_TO_WC, so pushImsStatusToWc('DELIVERED') is a no-op — confirmed.
• No WC double-write: the connector delivery path (getOrderDeliveryStatus) is read-only; only the cron decides to mark delivered.
• No caller depends on the old delivered counter semantics — the cron route just returns the JSON.

Validation after hardening

• tests/trackship-mark-delivered.test.ts — 2/2
• tests/domain/sales/*.test.ts — 93/93 (option addition didn't disturb existing callers)
• type-check + lint clean