← back to Costa Rica
costa-rica: fix payment-webhook refund path — ONVO status map, per-event idempotency key, guarded refund UPDATE (cycle 29) — TK-10346
762091f47907f99cb59d0590354bb2ac13387127 · 2026-09-24 07:07:50 -0700 · Steve
Cold Cody audit of the webhook state machine: the confirm path is idempotent even
under concurrency (confirmBooking WHERE status='pending'), but the REFUND path had
a live asymmetric gap. Three fixes:
- ONVO refunds were silently swallowed: lib/payments/onvo.js STATUS_MAP had no
refund key, so getCharge() could never return 'refunded' -> a refund mapped to
'processing' (the || default), REGRESSING payments.status and NEVER flipping the
booking to refunded. Added refunded/reversed -> 'refunded' (tilopay already did
this) + a documented GO-LIVE gate: ONVO's real post-refund vocabulary is
UNVERIFIED (may be Stripe-like where the PI stays 'succeeded').
- Idempotency-key collision: evId could equal the charge id (Tilopay's shape is
{paymentId, status} with no distinct event id), so a 'succeeded' then a genuinely
distinct 'refunded' event for the SAME charge both keyed firstTime() on the same
id -> the 2nd hit ON CONFLICT DO NOTHING, was treated as 'dup', and the refund was
NEVER processed (booking confirmed forever, silently). evId now prefers a distinct
event id, else a paymentId:type composite so lifecycle events get distinct keys
while true replays still dedupe. (Reorder also fixes event_id being shadowed by
paymentId.) ref reuses the same derived chargeId, byte-identical.
- The refunded booking UPDATE now sets updated_at=NOW() (every other status mutation
does; a reconcile keyed off it would miss refunds) and guards status<>'refunded'
(idempotent replay). Booking status is a single 'refunded' enum, so the guard only
no-ops a redundant event, never blocks a needed transition.
test/webhooks-refund-idempotency.test.js: distinct keys for succeeded-vs-refunded,
replay still dedupes to one payments UPDATE, a refunded getCharge reaches the guarded
UPDATE, ONVO mapStatus. Suite 230 -> 234, serial green.
Cody-gated (money path). The gate confirmed the fix + caught a doc contradiction:
GO-LIVE had called Tilopay's reversed path "verified" while item 3 says neither map
is live-verified; folded Tilopay into the same unverified + void-vs-refund caveat
(reversed can be an auth VOID, not a refund) — whichever provider goes live first
must pass the $1 refund test.
Latent (noted, not fixed): the refund UPDATE has no clawback link to the payouts row
— moot while the completion/payout pipeline is unwired; tracked with refund-after-payout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
Files touched
M docs/GO-LIVE.mdM lib/payments/onvo.jsM routes/webhooks.jsA test/webhooks-refund-idempotency.test.js
Diff
commit 762091f47907f99cb59d0590354bb2ac13387127
Author: Steve <steve@designerwallcoverings.com>
Date: Thu Sep 24 07:07:50 2026 -0700
costa-rica: fix payment-webhook refund path — ONVO status map, per-event idempotency key, guarded refund UPDATE (cycle 29) — TK-10346
Cold Cody audit of the webhook state machine: the confirm path is idempotent even
under concurrency (confirmBooking WHERE status='pending'), but the REFUND path had
a live asymmetric gap. Three fixes:
- ONVO refunds were silently swallowed: lib/payments/onvo.js STATUS_MAP had no
refund key, so getCharge() could never return 'refunded' -> a refund mapped to
'processing' (the || default), REGRESSING payments.status and NEVER flipping the
booking to refunded. Added refunded/reversed -> 'refunded' (tilopay already did
this) + a documented GO-LIVE gate: ONVO's real post-refund vocabulary is
UNVERIFIED (may be Stripe-like where the PI stays 'succeeded').
- Idempotency-key collision: evId could equal the charge id (Tilopay's shape is
{paymentId, status} with no distinct event id), so a 'succeeded' then a genuinely
distinct 'refunded' event for the SAME charge both keyed firstTime() on the same
id -> the 2nd hit ON CONFLICT DO NOTHING, was treated as 'dup', and the refund was
NEVER processed (booking confirmed forever, silently). evId now prefers a distinct
event id, else a paymentId:type composite so lifecycle events get distinct keys
while true replays still dedupe. (Reorder also fixes event_id being shadowed by
paymentId.) ref reuses the same derived chargeId, byte-identical.
- The refunded booking UPDATE now sets updated_at=NOW() (every other status mutation
does; a reconcile keyed off it would miss refunds) and guards status<>'refunded'
(idempotent replay). Booking status is a single 'refunded' enum, so the guard only
no-ops a redundant event, never blocks a needed transition.
test/webhooks-refund-idempotency.test.js: distinct keys for succeeded-vs-refunded,
replay still dedupes to one payments UPDATE, a refunded getCharge reaches the guarded
UPDATE, ONVO mapStatus. Suite 230 -> 234, serial green.
Cody-gated (money path). The gate confirmed the fix + caught a doc contradiction:
GO-LIVE had called Tilopay's reversed path "verified" while item 3 says neither map
is live-verified; folded Tilopay into the same unverified + void-vs-refund caveat
(reversed can be an auth VOID, not a refund) — whichever provider goes live first
must pass the $1 refund test.
Latent (noted, not fixed): the refund UPDATE has no clawback link to the payouts row
— moot while the completion/payout pipeline is unwired; tracked with refund-after-payout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
docs/GO-LIVE.md | 4 +-
lib/payments/onvo.js | 14 ++++-
routes/webhooks.js | 24 +++++++--
test/webhooks-refund-idempotency.test.js | 90 ++++++++++++++++++++++++++++++++
4 files changed, 125 insertions(+), 7 deletions(-)
diff --git a/docs/GO-LIVE.md b/docs/GO-LIVE.md
index de09722..8913135 100644
--- a/docs/GO-LIVE.md
+++ b/docs/GO-LIVE.md
@@ -67,8 +67,8 @@ These closed in the /yoloforever cycles; they need no creds and are already live
1. **Webhook signature encoding** — adapters compare hex; if the provider signs base64, set `Buffer.from(sig,'base64')` explicitly once the real delivery is observable.
2. **Provider-honored idempotency key (#6 live half)** — pass the provider's real idempotency header/field (ONVO `Idempotency-Key`-style; Tilopay per its live account) on `createCharge` **and** on the SINPE `payout` transfer, so a retry after a timeout can't double-charge a traveler or double-pay a host. The local pre-charge-row half is already in; this is the provider-side guarantee.
3. **Confirm each provider's REAL decline status strings** against `mapStatus()` in `lib/payments/tilopay.js`/`onvo.js` — the maps are built from documented vocabularies (Stripe-like for ONVO), not the live account; a status string the map doesn't recognize safely defaults to `'processing'` (not `'failed'`), so an unmapped real-world decline string would silently NOT fail closed. Watch the `raw` field on any `processing` row that never resolves.
-4. **⚠ ONVO refund detection (BLOCKER before `PAYMENT_PROVIDER=onvo`)** — the webhook decides "refunded" from `getCharge().status`. `onvo.js` `STATUS_MAP` now maps `refunded`/`reversed`→`'refunded'` (cycle-29 fix; Tilopay already mapped `reversed`), BUT ONVO's EXACT post-refund vocabulary is UNVERIFIED. If ONVO is Stripe-like, the payment-intent status STAYS `'succeeded'` after a refund (refund is a separate object / a `charge.refunded` flag), in which case the status-map key never fires and `getCharge` must instead inspect the refund object. Verify against ONVO's real API + the $1 refund test below and adjust `getCharge`/`STATUS_MAP` accordingly — do NOT flip ONVO live until a real refund is observed flipping the booking to `refunded`. (Tilopay's `reversed` path is the verified reference.)
-5. **Verify with a $1 real charge + refund** before opening bookings — confirm the refund flips both `payments.status` and `bookings.status` to `refunded` for the ACTIVE provider.
+4. **⚠ ONVO refund detection (BLOCKER before `PAYMENT_PROVIDER=onvo`)** — the webhook decides "refunded" from `getCharge().status`. `onvo.js` `STATUS_MAP` now maps `refunded`/`reversed`→`'refunded'` (cycle-29 fix; Tilopay already mapped `reversed`), BUT ONVO's EXACT post-refund vocabulary is UNVERIFIED. If ONVO is Stripe-like, the payment-intent status STAYS `'succeeded'` after a refund (refund is a separate object / a `charge.refunded` flag), in which case the status-map key never fires and `getCharge` must instead inspect the refund object. Verify against ONVO's real API + the $1 refund test below and adjust `getCharge`/`STATUS_MAP` accordingly — do NOT flip ONVO live until a real refund is observed flipping the booking to `refunded`. **The same caveat applies to Tilopay** (`reversed`→`refunded`, tilopay.js:94): neither map is verified against a live account (see item 3), and `reversed` in a real processor can mean an auth VOID of a never-captured charge (money never moved) as easily as a post-capture REFUND (money returned) — both currently collapse to `refunded`. Whichever provider goes live FIRST must pass item 5 before taking real money; treat neither map as pre-verified.
+5. **Verify with a $1 real charge + refund** before opening bookings — confirm the refund flips both `payments.status` and `bookings.status` to `refunded` for the ACTIVE provider (and that an auth void, if the flow allows one, is NOT mis-recorded as a refund).
## 6. Verify money math
`total = platform_fee + host_payout` (10% platform fee; `fees` is display-only). Enforced by DB CHECK `bookings_total_reconciles` + `test/money.test.js`. Booking amounts are capped: stays > `MAX_BOOKING_NIGHTS` (default 365) and any amount that would overflow the INTEGER (int4) money columns are rejected with a 400 at `POST /bookings` (cycle-14 guards).
diff --git a/lib/payments/onvo.js b/lib/payments/onvo.js
index 0738969..134df89 100644
--- a/lib/payments/onvo.js
+++ b/lib/payments/onvo.js
@@ -48,9 +48,21 @@ async function createCharge({ amount, currency, method = 'card', booking, return
// Shared by createCharge (a synchronous decline) AND getCharge (polling) so a
// decline is recognized identically regardless of which call surfaces it.
// Exported (pure, no I/O) so it is unit-testable without live creds.
+// NB: 'refunded'/'reversed' map to 'refunded' so a refunded-charge status from
+// getCharge reaches the webhook's refund branch — tilopay.js already does this
+// ('reversed'->'refunded') and WITHOUT it an ONVO refund silently mapped to
+// 'processing' (the || default), regressing payments.status + never flipping the
+// booking to refunded. (Cody webhook-state-machine audit, cycle 29.)
+// ⚠ GO-LIVE GATE: ONVO's EXACT post-refund vocabulary is UNVERIFIED here. If ONVO
+// is Stripe-like, the payment-intent status may STAY 'succeeded' after a refund
+// (refund tracked as a separate object / a charge.refunded flag), in which case a
+// status-map key is insufficient and getCharge must inspect the refund object.
+// Verify against ONVO's real API BEFORE flipping PAYMENT_PROVIDER=onvo live — see
+// docs/GO-LIVE.md.
const STATUS_MAP = { succeeded: 'succeeded', processing: 'processing',
requires_action: 'processing', requires_payment_method: 'failed',
- canceled: 'failed', declined: 'failed', failed: 'failed' };
+ canceled: 'failed', declined: 'failed', failed: 'failed',
+ refunded: 'refunded', reversed: 'refunded' };
function mapStatus(s) { return STATUS_MAP[s] || 'processing'; }
async function getCharge(ref) {
diff --git a/routes/webhooks.js b/routes/webhooks.js
index 7b0ce64..7538771 100644
--- a/routes/webhooks.js
+++ b/routes/webhooks.js
@@ -31,8 +31,18 @@ async function paymentWebhook(providerName, req, res) {
// WhatsApp malformed-body 400 and STOP: never fall through to firstTime() with
// a null id (which would `return true` and let a phantom event "pass" processing).
if (!event) return res.status(400).send('bad body');
- const evId = event?.id || event?.paymentId || event?.event_id;
const evType = event?.type || event?.status;
+ // Idempotency key MUST be per-EVENT, not per-CHARGE. A distinct notification id
+ // is preferred, but some provider shapes carry only `paymentId` (the charge id) —
+ // e.g. Tilopay's sandbox payloads are `{paymentId, status}` with no event id. If
+ // we keyed on the charge id, a 'succeeded' event and a LATER, genuinely distinct
+ // 'refunded' event for the SAME charge would collide: the second INSERT hits ON
+ // CONFLICT DO NOTHING, is treated as a 'dup', and the refund is NEVER processed
+ // (booking stays confirmed forever, silently). Fall back to a `paymentId:type`
+ // composite so lifecycle events for one charge get DISTINCT keys while true
+ // replays (same charge+type) still dedupe. (Cody webhook audit, cycle 29.)
+ const chargeId = event?.paymentId || event?.id || event?.data?.id;
+ const evId = event?.id || event?.event_id || (chargeId ? `${chargeId}:${evType}` : null);
// A signed event with no resolvable id is suspicious — do NOT silently pass the
// idempotency gate (which best-efforts to firstTime===true on a missing id).
if (!evId) return res.status(400).send('missing event id');
@@ -47,8 +57,8 @@ async function paymentWebhook(providerName, req, res) {
// keep the marker) and concurrent double-delivery (ON CONFLICT DO NOTHING lets
// only one insert win). (Cody gate, cycle 12, TK-10346.)
try {
- // Resolve the charge id the adapter reported to us.
- const ref = event?.paymentId || event?.id || event?.data?.id;
+ // The charge id the adapter reported (same value derived above for the key).
+ const ref = chargeId;
if (ref) {
const latest = await provider.getCharge(ref).catch(() => null);
const status = latest?.status || (/(succe|approved|paid)/i.test(String(evType)) ? 'succeeded' : 'processing');
@@ -57,7 +67,13 @@ async function paymentWebhook(providerName, req, res) {
WHERE provider=$3 AND provider_ref=$4 RETURNING id, booking_id`,
[status, JSON.stringify(event || {}), providerName, ref]);
if (rows[0] && status === 'succeeded') await confirmBooking(rows[0].booking_id);
- if (rows[0] && status === 'refunded') await pool.query(`UPDATE bookings SET status='refunded' WHERE id=$1`, [rows[0].booking_id]);
+ // Set updated_at (every other status mutation does — a reconcile keyed off it
+ // would otherwise miss refunds) and guard status<>'refunded' so a replayed
+ // refund event is idempotent. NOTE (latent, Cody cycle-29): this has no
+ // clawback link to the payouts row — moot while the completion/payout pipeline
+ // is unwired, but a refund on an already-paid-out completed booking would need
+ // one. Tracked with the refund-after-payout deferred item.
+ if (rows[0] && status === 'refunded') await pool.query(`UPDATE bookings SET status='refunded', updated_at=NOW() WHERE id=$1 AND status<>'refunded'`, [rows[0].booking_id]);
}
} catch (e) {
// Best-effort release. If it ALSO fails, log distinctly — we've silently
diff --git a/test/webhooks-refund-idempotency.test.js b/test/webhooks-refund-idempotency.test.js
new file mode 100644
index 0000000..37b63b1
--- /dev/null
+++ b/test/webhooks-refund-idempotency.test.js
@@ -0,0 +1,90 @@
+'use strict';
+// Payment-webhook refund + idempotency-key fixes (Cody state-machine audit, cycle 29):
+// #1 ONVO STATUS_MAP maps refunded/reversed -> 'refunded' (was silently 'processing',
+// so an ONVO refund never flipped the booking).
+// #2 the webhook idempotency key is per-EVENT: a `paymentId:type` composite when the
+// payload carries no distinct event id, so a 'succeeded' then a genuinely distinct
+// 'refunded' for the SAME charge no longer collide on the key + drop the refund.
+// #3 the refunded booking UPDATE sets updated_at and is idempotent (status<>'refunded').
+//
+// Secrets set before require (captured at module load). pool.query is a recording
+// mock (captures args); tilopay.getCharge is stubbed per-test.
+process.env.TILOPAY_WEBHOOK_SECRET = 'itest-tilo-secret';
+
+const { test, before, after } = require('node:test');
+const assert = require('node:assert');
+const http = require('node:http');
+const crypto = require('crypto');
+const express = require('express');
+
+const db = require('../lib/db');
+const tilopay = require('../lib/payments/tilopay');
+const onvo = require('../lib/payments/onvo');
+const webhooks = require('../routes/webhooks');
+
+let calls = [];
+let webhookInsertRowCounts = []; // queue for successive webhook_events inserts (default 1 = first-seen)
+let getChargeStub = null;
+const origQuery = db.pool.query;
+const origGetCharge = tilopay.getCharge;
+let server, base;
+before(async () => {
+ db.pool.query = async (sql, args) => {
+ calls.push({ sql, args });
+ if (/INSERT INTO webhook_events/.test(sql)) return { rows: [], rowCount: webhookInsertRowCounts.length ? webhookInsertRowCounts.shift() : 1 };
+ if (/UPDATE payments/.test(sql)) return { rows: [{ id: 1, booking_id: 42 }], rowCount: 1 }; // resolve a booking so the branch fires
+ return { rows: [], rowCount: 1 };
+ };
+ tilopay.getCharge = async (ref) => (getChargeStub ? getChargeStub(ref) : { status: 'processing', raw: {} });
+ const app = express();
+ app.use('/webhooks', webhooks);
+ await new Promise(r => { server = app.listen(0, r); });
+ base = `http://127.0.0.1:${server.address().port}`;
+});
+after(() => { db.pool.query = origQuery; tilopay.getCharge = origGetCharge; server && server.close(); });
+
+const tiloSign = (raw) => crypto.createHmac('sha256', 'itest-tilo-secret').update(raw).digest('hex');
+function postTilo(obj) {
+ const body = JSON.stringify(obj);
+ const opts = { method: 'POST', headers: { 'content-type': 'application/json', 'content-length': Buffer.byteLength(body), 'x-tilopay-signature': tiloSign(body) } };
+ return new Promise((resolve, reject) => {
+ const r = http.request(base + '/webhooks/tilopay', opts, res => { let b = ''; res.on('data', c => b += c); res.on('end', () => resolve({ status: res.statusCode, body: b })); });
+ r.on('error', reject); r.end(body);
+ });
+}
+const externalIdsInserted = () => calls.filter(c => /INSERT INTO webhook_events/.test(c.sql)).map(c => c.args && c.args[1]);
+
+test('#1 ONVO mapStatus maps a refund/reversal to refunded (was silently processing)', () => {
+ assert.equal(onvo.mapStatus('refunded'), 'refunded');
+ assert.equal(onvo.mapStatus('reversed'), 'refunded');
+ assert.equal(onvo.mapStatus('succeeded'), 'succeeded');
+ assert.equal(onvo.mapStatus('gibberish'), 'processing'); // default unchanged
+});
+
+test('#2 succeeded then refunded for the SAME charge get DISTINCT idempotency keys (refund not dropped)', async () => {
+ calls = []; webhookInsertRowCounts = [];
+ await postTilo({ paymentId: 'chg-1', status: 'succeeded' });
+ await postTilo({ paymentId: 'chg-1', status: 'refunded' });
+ assert.deepEqual(externalIdsInserted(), ['chg-1:succeeded', 'chg-1:refunded'],
+ 'the two lifecycle events must key on distinct external_ids so the refund is not deduped away');
+});
+
+test('#2b a true replay (same charge + same status) still dedupes to ONE processing', async () => {
+ calls = []; webhookInsertRowCounts = [1, 0]; // 1st insert wins; 2nd hits ON CONFLICT -> dup
+ const a = await postTilo({ paymentId: 'chg-2', status: 'succeeded' });
+ const b = await postTilo({ paymentId: 'chg-2', status: 'succeeded' });
+ assert.equal(a.status, 200); assert.equal(b.status, 200);
+ assert.equal(calls.filter(c => /UPDATE payments/.test(c.sql)).length, 1, 'a replayed identical event must be deduped (one payments UPDATE only)');
+});
+
+test('#3 a refunded charge flips the booking with updated_at + an idempotent status guard', async () => {
+ calls = []; webhookInsertRowCounts = [];
+ getChargeStub = () => ({ status: 'refunded', raw: {} });
+ const r = await postTilo({ paymentId: 'chg-3', status: 'refunded' });
+ getChargeStub = null;
+ assert.equal(r.status, 200);
+ const refundUpd = calls.find(c => /UPDATE bookings SET status='refunded'/.test(c.sql));
+ assert.ok(refundUpd, 'a refunded charge must reach the booking refund UPDATE');
+ assert.match(refundUpd.sql, /updated_at=NOW\(\)/, 'refund UPDATE must set updated_at (reconcile keys off it)');
+ assert.match(refundUpd.sql, /status<>'refunded'/, 'refund UPDATE must be idempotent on replay');
+});
← fc341d1 auto-data-snapshot: 2026-09-24T07:06:26 (1 data files) — doc
·
back to Costa Rica
·
cycle 29 docs: YOLO_NOTES ledger — payment-webhook refund-pa d94c613 →