Skip to content

fix: PR A — concurrency + money bugs (round 4, bugs 1-4)#78

Merged
telivity-otaip merged 1 commit into
mainfrom
fix/pr-a-concurrency-money
Apr 18, 2026
Merged

fix: PR A — concurrency + money bugs (round 4, bugs 1-4)#78
telivity-otaip merged 1 commit into
mainfrom
fix/pr-a-concurrency-money

Conversation

@telivity-otaip
Copy link
Copy Markdown
Collaborator

Summary

First of 4 PRs from round-4 codex review. Covers the concurrency + money class of bugs.

# Severity Bug Fix
1 CRITICAL Payment capture/refund double-execution race Conditional UPDATE-by-status + Stripe idempotency keys
2 HIGH `folio.transferCharge` non-atomic db.transaction + FOR UPDATE on both folios in id-order
3 HIGH `transferToCityLedger` non-atomic All 4 steps in db.transaction
4 HIGH Night audit concurrent execution for same date Unique index + onConflictDoNothing + running-audit return

Monetary math policy

All financial arithmetic now uses `decimal.js` on Drizzle's string numeric representations. No `parseFloat` on money values. `recalculateBalance` is the first method migrated — the rest follow in PR D.

Test plan

  • `pnpm build` green
  • `pnpm typecheck` green
  • `pnpm test` — 552/552 (added 1 night-audit race test, replaced 1 stale test)

Still pending from round 4

  • PR B: trust boundaries — WebSocket auth, guest PII scoping, Stripe raw-body, JWT aud
  • PR C: operational — webhook delivery, schema completeness, audit logs
  • PR D: decimal.js migration across tax engine

🤖 Generated with Claude Code

Bug 1 (CRITICAL): payment capture/refund/void race
- Conditional UPDATE-by-status claims the row atomically
- Stripe idempotency keys (cap_/ref_/void_) prevent gateway double-charge
- Two-phase pattern: short tx claims state, network call outside, tx finalizes

Bug 2 (HIGH): folio.transferCharge non-atomic
- Wrapped in db.transaction, both folios locked FOR UPDATE in id-order
- Helpers accept optional tx; recalculateBalance uses decimal.js (no parseFloat)

Bug 3 (HIGH): transferToCityLedger non-atomic
- All 4 steps wrapped in db.transaction
- Monetary comparisons use decimal.js on string representations

Bug 4 (HIGH): night audit concurrent execution
- Unique index on (property_id, business_date)
- onConflictDoNothing + re-select: completed → ConflictException,
  running → return as active audit

All financial math now uses decimal.js on Drizzle's string numerics.
No parseFloat on monetary values.

552/552 tests passing, build + typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@telivity-otaip telivity-otaip merged commit b3e146f into main Apr 18, 2026
0 of 3 checks passed
@telivity-otaip telivity-otaip deleted the fix/pr-a-concurrency-money branch April 18, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant