Skip to content

fix(credits): hold approval credits before delivering, not after - #59

Merged
ralyodio merged 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/approval-credit-hold-race
Jul 28, 2026
Merged

fix(credits): hold approval credits before delivering, not after#59
ralyodio merged 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/approval-credit-hold-race

Conversation

@clawedassistant26

Copy link
Copy Markdown
Contributor

Approval ingest gates paid channels on the balance, but the gate leaks under concurrency: it reads the balance, delivers, and only then inserts the charge.

Repro (run against unmodified main, real router + real libsql)

A user with 12 credits and one enabled sms channel (12 credits per delivery). Two POST /api/approvals in flight at once:

balance before: 12
A: 201 {"delivered":["sms"],"charged":12}
B: 201 {"delivered":["sms"],"charged":12}
balance after: -12
ledger: [ +12 seed, -12 approval.delivered, -12 approval.delivered ]

Both handlers ran bal >= fullCost against the same 12 credits before either charge landed, so both delivered and both charged. The same two requests sequentially behave correctly — B falls back to free channels and returns the insufficient credits warning — so this is purely the concurrent window.

This is reachable from normal use: a moshscript can fire ask() / notify() in parallel, and each one is an ingest. The cost is real money on the operator side, since sms / slack / telegram are outbound provider sends.

Fix

apps/pwa/src/lib/credits.mjs gains reserve() and settle():

  • reserve() writes the charge with INSERT ... SELECT ... WHERE (SELECT COALESCE(SUM(delta),0) ...) >= ?, so the balance check happens inside the insert. This is the same atomic-claim shape /cli/token, /cli/device/token and /webhooks/coinpay already use — only the first reservation a balance can cover is written.
  • settle() then updates that one row down to what fanOut actually accepted, so the ledger keeps its existing shape: one approval.delivered row per delivery, for exactly what went out. Settling to 0 releases the hold, matching the current behaviour of not writing a row when nothing was charged.

apps/pwa/src/routes/approvals.mjs reserves before fanOut instead of charging after it. The response contract is unchanged (delivered, charged, warning).

Tests

New apps/pwa/test/approvals-credits.test.mjs (5 tests), reusing the harness from test/credits-webhook.test.mjs — statements deferred to a macrotask so handlers genuinely interleave (the local libsql driver resolves in microtasks and hides every read-check-write race), and raw http.request({agent:false}) so keep-alive does not serialise the requests.

  • a paid delivery is charged exactly once
  • concurrent ingests cannot spend the same credits twice — fails on unpatched main (4 pass / 1 fail), passes with the fix
  • a second sequential ingest still falls back to free channels
  • a channel that fails to deliver is not charged for (slack with no target releases its 4 credits)
  • a free-only account is never charged and writes no zero-value ledger row

Full root suite: 192 tests / 192 pass / 0 fail (187 before these 5). Run npm install in apps/pwa or the PWA tests skip.

Approval ingest read the balance, delivered to every enabled channel, then
inserted the charge. Two ingests in flight at once — a moshscript firing
ask()/notify() in parallel — both read a balance that covers one paid
delivery, both deliver, and both charge, driving the balance negative and
sending paid notifications (sms, slack, telegram) nobody paid for.

Reserve the cost in a single INSERT ... SELECT ... WHERE guarded by the
user's summed balance, the same way /cli/token and /webhooks/coinpay claim
their rows, then settle the hold down to what actually went out. The ledger
still keeps one row per delivery for exactly what was delivered.
@ralyodio
ralyodio merged commit 413d611 into moshcoder:main Jul 28, 2026
3 checks passed
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.

2 participants