← back to Costa Rica
costa-rica: webhook idempotency-marker release on failure + poll rescue for succeeded-but-pending bookings (Cody gate, cycle 12) — TK-10346
ce0a6078b84664768eb6a894010fa1496fa5f185 · 2026-09-23 21:40:49 -0700 · Steve
Audit of routes/webhooks.js (payment webhook money path).
BUG (fixed): the idempotency marker was claimed BEFORE processing. firstTime()
INSERTs webhook_events (source, external_id) then returns first-seen; if the
subsequent processing (UPDATE payments / confirmBooking / refund) then throws, the
outer catch returned 500 — but the marker was already committed, so the provider's
RETRY (on our 500) hit the dedupe gate, got 'dup' 200, and the confirmation was
lost forever (payment succeeded, booking never confirmed). Fix: wrap the processing
in try/catch; on failure RELEASE the marker (DELETE webhook_events) so the retry
re-processes, then surface the 500. Dedupe still blocks true duplicates (they
succeed and keep the marker) and concurrent double-delivery (ON CONFLICT DO NOTHING).
Cody gate — found a defense-in-depth regression the marker fix INTRODUCED, fixed
in the same commit:
- The release creates a NEW valid state: payments.status='succeeded' while its
booking is still 'pending' (webhook marked the payment, confirmBooking then
failed + released the marker + 500'd for a retry). GET /payments/:id only
reconciled when status==='processing', so a client poll landing before the
provider retry would return 'succeeded' while the booking sat pending, invisible
to the only in-repo rescue path (there is no cron). Fix: GET /payments/:id now
calls confirmBooking(booking_id) whenever the payment is 'succeeded' (idempotent),
not just inside the 'processing' branch — so the poll rescues that state from any
interleaving.
- The release DELETE's `.catch(()=>{})` swallowed a failed release silently
(regressing to the original bug with no trace). Now logs distinctly which event
stayed broken.
Cody cleared (verified, no defect): double-processing on retry (confirmBooking is
guarded by status='pending' -> no 2nd WhatsApp; refund UPDATE is a flat idempotent
SET); evId/ref key drift (release deletes by the same evId firstTime inserted); and
the 0-rows-matched case (correctly NOT released — payments.status stays 'processing'
so the existing poll covers it; releasing would only hammer retry budgets on
genuinely-unmatchable test/ping events).
Tests (+5, suite 170 -> 175):
- webhooks-route.test.js (+2): a processing failure -> 500 AND a DELETE
webhook_events (marker released); a successful delivery -> NO release.
- payment-poll-rescue.test.js (NEW, 3): a succeeded+pending booking is rescued by
the poll's confirmBooking; an already-confirmed booking is a no-op (no
double-notify); a still-processing payment polls as before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
Files touched
M routes/app.jsM routes/webhooks.jsA test/payment-poll-rescue.test.jsM test/webhooks-route.test.js
Diff
commit ce0a6078b84664768eb6a894010fa1496fa5f185
Author: Steve <steve@designerwallcoverings.com>
Date: Wed Sep 23 21:40:49 2026 -0700
costa-rica: webhook idempotency-marker release on failure + poll rescue for succeeded-but-pending bookings (Cody gate, cycle 12) — TK-10346
Audit of routes/webhooks.js (payment webhook money path).
BUG (fixed): the idempotency marker was claimed BEFORE processing. firstTime()
INSERTs webhook_events (source, external_id) then returns first-seen; if the
subsequent processing (UPDATE payments / confirmBooking / refund) then throws, the
outer catch returned 500 — but the marker was already committed, so the provider's
RETRY (on our 500) hit the dedupe gate, got 'dup' 200, and the confirmation was
lost forever (payment succeeded, booking never confirmed). Fix: wrap the processing
in try/catch; on failure RELEASE the marker (DELETE webhook_events) so the retry
re-processes, then surface the 500. Dedupe still blocks true duplicates (they
succeed and keep the marker) and concurrent double-delivery (ON CONFLICT DO NOTHING).
Cody gate — found a defense-in-depth regression the marker fix INTRODUCED, fixed
in the same commit:
- The release creates a NEW valid state: payments.status='succeeded' while its
booking is still 'pending' (webhook marked the payment, confirmBooking then
failed + released the marker + 500'd for a retry). GET /payments/:id only
reconciled when status==='processing', so a client poll landing before the
provider retry would return 'succeeded' while the booking sat pending, invisible
to the only in-repo rescue path (there is no cron). Fix: GET /payments/:id now
calls confirmBooking(booking_id) whenever the payment is 'succeeded' (idempotent),
not just inside the 'processing' branch — so the poll rescues that state from any
interleaving.
- The release DELETE's `.catch(()=>{})` swallowed a failed release silently
(regressing to the original bug with no trace). Now logs distinctly which event
stayed broken.
Cody cleared (verified, no defect): double-processing on retry (confirmBooking is
guarded by status='pending' -> no 2nd WhatsApp; refund UPDATE is a flat idempotent
SET); evId/ref key drift (release deletes by the same evId firstTime inserted); and
the 0-rows-matched case (correctly NOT released — payments.status stays 'processing'
so the existing poll covers it; releasing would only hammer retry budgets on
genuinely-unmatchable test/ping events).
Tests (+5, suite 170 -> 175):
- webhooks-route.test.js (+2): a processing failure -> 500 AND a DELETE
webhook_events (marker released); a successful delivery -> NO release.
- payment-poll-rescue.test.js (NEW, 3): a succeeded+pending booking is rescued by
the poll's confirmBooking; an already-confirmed booking is a no-op (no
double-notify); a still-processing payment polls as before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
routes/app.js | 12 +++++-
routes/webhooks.js | 39 +++++++++++++-----
test/payment-poll-rescue.test.js | 88 ++++++++++++++++++++++++++++++++++++++++
test/webhooks-route.test.js | 26 ++++++++++++
4 files changed, 153 insertions(+), 12 deletions(-)
diff --git a/routes/app.js b/routes/app.js
index 6df152f..44832ef 100644
--- a/routes/app.js
+++ b/routes/app.js
@@ -316,11 +316,21 @@ router.get('/payments/:id', authRequired, async (req, res) => {
const latest = await provider.getCharge(pay.provider_ref);
if (latest.status !== 'processing') {
await pool.query(`UPDATE payments SET status=$1, updated_at=NOW() WHERE id=$2`, [latest.status, pay.id]);
- if (latest.status === 'succeeded') await confirmBooking(pay.booking_id);
pay.status = latest.status;
}
} catch { /* leave processing */ }
}
+ // Rescue any 'succeeded' payment whose booking is still pending — covers a
+ // just-polled success AND a payment a webhook already marked 'succeeded' but
+ // whose confirmBooking then failed (the webhook releases its idempotency marker
+ // and 500s for a provider retry, but until that retry lands the booking sits
+ // pending while payments.status is already succeeded — a state this poll used to
+ // skip because it only acted on 'processing'). confirmBooking is idempotent.
+ // (Cody gate, cycle 12, TK-10346.)
+ if (pay.status === 'succeeded') {
+ try { await confirmBooking(pay.booking_id); }
+ catch (e) { console.error('[poll] confirmBooking', pay.booking_id, e.message); }
+ }
ok(res, { payment: { id: pay.id, status: pay.status, amount: pay.amount, currency: pay.currency } });
});
diff --git a/routes/webhooks.js b/routes/webhooks.js
index ba6cff3..841a74d 100644
--- a/routes/webhooks.js
+++ b/routes/webhooks.js
@@ -38,17 +38,34 @@ async function paymentWebhook(providerName, req, res) {
if (!evId) return res.status(400).send('missing event id');
if (!(await firstTime(providerName, evId, evType, event))) return res.status(200).send('dup');
- // Resolve the charge id the adapter reported to us.
- const ref = event?.paymentId || event?.id || event?.data?.id;
- if (ref) {
- const latest = await provider.getCharge(ref).catch(() => null);
- const status = latest?.status || (/(succe|approved|paid)/i.test(String(evType)) ? 'succeeded' : 'processing');
- const { rows } = await pool.query(
- `UPDATE payments SET status=$1, raw=$2, updated_at=NOW()
- 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]);
+ // Once we've CLAIMED the event (marked it seen above), a failure in the
+ // processing below must NOT leave it marked — otherwise the provider's retry
+ // (triggered by our 500) hits the idempotency gate, returns 'dup', and the
+ // confirmation is lost forever (payment succeeded, booking never confirmed). On
+ // any processing error, RELEASE the marker so the retry re-processes, then
+ // surface the 500. The dedupe still blocks true duplicates (which succeed and
+ // 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;
+ if (ref) {
+ const latest = await provider.getCharge(ref).catch(() => null);
+ const status = latest?.status || (/(succe|approved|paid)/i.test(String(evType)) ? 'succeeded' : 'processing');
+ const { rows } = await pool.query(
+ `UPDATE payments SET status=$1, raw=$2, updated_at=NOW()
+ 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]);
+ }
+ } catch (e) {
+ // Best-effort release. If it ALSO fails, log distinctly — we've silently
+ // regressed to the original bug (this event's retry will be deduped as 'dup'
+ // forever), and this line is the only trace of which event stayed broken.
+ await pool.query(`DELETE FROM webhook_events WHERE source=$1 AND external_id=$2`, [providerName, String(evId)])
+ .catch((de) => console.error('[payment webhook] marker release FAILED', providerName, evId, de && de.message));
+ throw e; // -> whFail -> 500 (retryable); the released marker lets the retry re-process
}
res.status(200).send('ok');
}
diff --git a/test/payment-poll-rescue.test.js b/test/payment-poll-rescue.test.js
new file mode 100644
index 0000000..70b7c4e
--- /dev/null
+++ b/test/payment-poll-rescue.test.js
@@ -0,0 +1,88 @@
+'use strict';
+// Cody gate, cycle 12 (TK-10346): the webhook idempotency-release fix introduced a
+// new valid state — payments.status='succeeded' while its booking is still 'pending'
+// (a webhook marked the payment succeeded, then confirmBooking failed, released its
+// marker, and 500'd for a provider retry). GET /payments/:id used to only reconcile
+// when status==='processing', so it would return 'succeeded' to the client while the
+// booking sat pending, invisible to the poll. The fix calls confirmBooking whenever
+// the payment is 'succeeded' (idempotent). These tests prove the poll now rescues
+// that state, and does nothing harmful when the booking is already confirmed.
+// No real DB: pool.query is a queue mock.
+
+const { test, before, after } = require('node:test');
+const assert = require('node:assert');
+const http = require('node:http');
+const express = require('express');
+
+const { signToken } = require('../lib/auth');
+const db = require('../lib/db');
+const { router } = require('../routes/app');
+
+let responses = [];
+let calls = [];
+const origQuery = db.pool.query;
+let server, base;
+
+before(async () => {
+ db.pool.query = async (sql, args) => { calls.push({ sql, args }); return responses.length ? responses.shift() : { rows: [], rowCount: 0 }; };
+ const app = express();
+ app.use(express.json());
+ app.use('/api/app', router);
+ await new Promise(r => { server = app.listen(0, r); });
+ base = `http://127.0.0.1:${server.address().port}`;
+});
+after(() => { db.pool.query = origQuery; server && server.close(); });
+
+const hasSql = (re) => calls.some(c => re.test(c.sql));
+function reset(resp) { responses = resp.slice(); calls = []; }
+
+function get(path, token) {
+ return new Promise((resolve, reject) => {
+ http.get(base + path, { headers: token ? { authorization: 'Bearer ' + token } : {} },
+ res => { let b = ''; res.on('data', c => b += c); res.on('end', () => resolve({ status: res.statusCode, json: JSON.parse(b || '{}') })); }).on('error', reject);
+ });
+}
+
+test('GET /payments/:id: a SUCCEEDED payment with a still-PENDING booking is rescued — confirmBooking fires from the poll', async () => {
+ const token = signToken({ sub: 3, role: 'guest' });
+ reset([
+ // SELECT the payment (already succeeded — e.g. a webhook set it, then confirmBooking failed)
+ { rows: [{ id: 55, status: 'succeeded', booking_id: 7, provider: 'tilopay', provider_ref: 'ref-1', amount: 12000, currency: 'USD' }] },
+ // confirmBooking: UPDATE bookings ... WHERE status='pending' RETURNING * -> a pending row is found + confirmed
+ { rows: [{ id: 7, code: 'CR-P', currency: 'USD', total: 12000, traveler_id: 3, place_id: 5 }] },
+ { rows: [{ phone_e164: '+50600000000', wa_opt_in: false }] }, // confirmBooking loads user (opted out -> no WA)
+ { rows: [{ name: 'Casa' }] }, // confirmBooking loads place
+ ]);
+ const r = await get('/api/app/payments/55', token);
+ assert.equal(r.status, 200);
+ assert.equal(r.json.payment.status, 'succeeded');
+ assert.ok(hasSql(/UPDATE bookings SET status='confirmed'/), 'the poll rescues a succeeded-but-pending booking via confirmBooking');
+});
+
+test('GET /payments/:id: a succeeded payment whose booking is ALREADY confirmed does nothing harmful (idempotent)', async () => {
+ const token = signToken({ sub: 3, role: 'guest' });
+ reset([
+ { rows: [{ id: 56, status: 'succeeded', booking_id: 8, provider: 'tilopay', provider_ref: 'ref-2', amount: 12000, currency: 'USD' }] },
+ { rows: [] }, // confirmBooking UPDATE matched no pending row (already confirmed) -> early return, no WA
+ ]);
+ const r = await get('/api/app/payments/56', token);
+ assert.equal(r.status, 200);
+ assert.equal(r.json.payment.status, 'succeeded');
+ assert.ok(hasSql(/UPDATE bookings SET status='confirmed'/), 'confirmBooking is attempted (idempotent guard makes it a no-op)');
+ assert.equal(hasSql(/SELECT phone_e164/), false, 'no WhatsApp lookup when the guarded UPDATE matched nothing (no double-notify)');
+});
+
+test('GET /payments/:id: a still-PROCESSING payment polls the provider (unchanged happy path)', async () => {
+ const token = signToken({ sub: 3, role: 'guest' });
+ reset([
+ { rows: [{ id: 57, status: 'processing', booking_id: 9, provider: 'tilopay', provider_ref: 'ref_sbx_x', amount: 12000, currency: 'USD' }] },
+ // getCharge is the sandbox provider (real), returns succeeded for an _sbx_ ref -> UPDATE payments
+ { rowCount: 1 }, // UPDATE payments SET status
+ { rows: [{ id: 9, code: 'CR-Q', currency: 'USD', total: 12000, traveler_id: 3, place_id: 5 }] }, // confirmBooking UPDATE
+ { rows: [{ phone_e164: '+50600000000', wa_opt_in: false }] },
+ { rows: [{ name: 'Casa' }] },
+ ]);
+ const r = await get('/api/app/payments/57', token);
+ assert.equal(r.status, 200);
+ assert.equal(r.json.payment.status, 'succeeded', 'the poll moved processing -> succeeded');
+});
diff --git a/test/webhooks-route.test.js b/test/webhooks-route.test.js
index 8c70faa..d0c2a7e 100644
--- a/test/webhooks-route.test.js
+++ b/test/webhooks-route.test.js
@@ -29,12 +29,14 @@ let sqls = [];
// (first-seen) or 0 (duplicate). Default 1; a test can flip the NEXT webhook_events
// insert to 0 to simulate a replay. Everything else returns rowCount:1 as before.
let nextWebhookInsertRowCount = null;
+let throwOnSql = null; // a test can set a regex to make matching queries throw (simulate a DB error mid-processing)
const origQuery = db.pool.query;
const origHandle = wa.handleInbound;
let server, base;
before(async () => {
db.pool.query = async (sql) => {
sqls.push(sql);
+ if (throwOnSql && throwOnSql.test(sql)) throw new Error('simulated DB error');
if (/INSERT INTO webhook_events/.test(sql) && nextWebhookInsertRowCount !== null) {
const rc = nextWebhookInsertRowCount; nextWebhookInsertRowCount = null;
return { rows: [], rowCount: rc };
@@ -136,6 +138,30 @@ test('P3: a replayed Tilopay webhook (same event id) → "dup" on the 2nd, NO 2n
assert.equal(sqls.filter(s => /UPDATE payments/.test(s)).length, 0, 'a replay must NOT re-run the payments UPDATE / confirmBooking');
});
+// RELIABILITY (cycle 12): the idempotency marker is claimed BEFORE processing.
+// If processing then fails (a transient DB error), the old code left the marker in
+// place -> the provider's retry (on our 500) hit the dedupe gate, got 'dup', and
+// the confirmation was lost forever. The fix RELEASES the marker on failure so the
+// retry re-processes. Prove: a processing failure -> 500 AND a DELETE webhook_events.
+test('RELIABILITY: a processing failure after the dedupe insert releases the marker (retry not swallowed as dup)', async () => {
+ sqls = [];
+ throwOnSql = /UPDATE payments/; // DB blows up mid-processing, after the event was claimed
+ const body = JSON.stringify({ paymentId: 'fail-ref-1', status: 'succeeded' });
+ const r = await post('/webhooks/tilopay', body, { 'x-tilopay-signature': tiloSign(body) });
+ throwOnSql = null;
+ assert.equal(r.status, 500, 'a processing failure returns 500 (retryable), not a false 200');
+ assert.ok(sqls.some(s => /INSERT INTO webhook_events/.test(s)), 'the event was claimed (dedupe insert ran)');
+ assert.ok(sqls.some(s => /DELETE FROM webhook_events/.test(s)), 'the marker is RELEASED on failure so the provider retry re-processes (not deduped away)');
+});
+
+test('RELIABILITY: a SUCCESSFUL delivery keeps the marker (no spurious release; true replays still dedupe)', async () => {
+ sqls = [];
+ const body = JSON.stringify({ paymentId: 'ok-ref-1', status: 'processing' });
+ const r = await post('/webhooks/tilopay', body, { 'x-tilopay-signature': tiloSign(body) });
+ assert.equal(r.status, 200);
+ assert.equal(sqls.some(s => /DELETE FROM webhook_events/.test(s)), false, 'a successful delivery must NOT release the marker');
+});
+
// P6 — R3: a signed-but-malformed (unparseable JSON) payment body. verifyWebhook
// returns { ok:true, event:null }; the route must 400 'bad body' and do NO DB write
// (no phantom firstTime insert, no payments UPDATE, no confirmBooking).
← 47088ae cycle 11 docs: YOLO_NOTES ledger — systemic async-error hard
·
back to Costa Rica
·
cycle 12 docs: YOLO_NOTES ledger — webhook idempotency-relea 11478ab →