OUT-3896: make payment.succeeded expense sync idempotent - #63
Conversation
Repeated payment.succeeded webhooks no longer create duplicate Xero bank transactions or synced_payments rows. - Add partial unique index on (portal_id, tenant_id, copilot_payment_id) WHERE copilot_payment_id IS NOT NULL - Thread an idempotency key into createBankTransaction - Guard createPlatformExpensePayment with a select on copilot_payment_id and an onConflictDoNothing insert - Reconcile by reference: look up an existing AUTHORISED expense in Xero before creating, so retries outside Xero's idempotency window reuse it - Move the invoice id from reference into the line-item description Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Greptile SummaryCommit
Confidence Score: 5/5Safe to merge — the refactoring is straightforward and the behavioral change in findLegacyExpenseByInvoice moves from a path that created more duplicates to one that reuses existing transactions. Both changes are tightly scoped. The schema barrel registration is a required companion to the findFirst migration and is correctly paired in the same commit. The multi-match fallback in findLegacyExpenseByInvoice now adopts the first candidate instead of returning undefined; the old path was actually more dangerous because it caused createBankTransaction to run and produce another Xero duplicate — adopting any one of the same-invoice, same-amount candidates is safe since they represent the same underlying expense, and the partial unique index on copilot_payment_id prevents any incorrect DB row from being written regardless of which candidate is chosen. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([payment.succeeded webhook]) --> B{DB: expense exists\nfor copilot_payment_id?}
B -- Yes early return --> Z([return undefined])
B -- No --> C[Fetch invoice + accounts]
C --> D{Xero: SPEND tx\nwith Reference==payment_id?}
D -- Found AUTHORISED --> G
D -- Not found --> E{Xero: legacy SPEND tx\nwith Reference==xero_invoice_id\nand matching amount?}
E -- Found AUTHORISED\none or more candidates\nwarn if more than one --> G[Reuse existing transaction]
E -- Not found --> F[createBankTransaction\nwith idempotency key]
F --> G
G --> H[Insert synced_payments row\nonConflictDoNothing]
H -- inserted.length == 0\nconcurrent race won --> I([return transaction\nskip sync log])
H -- inserted.length > 0 --> J[Write SUCCESS sync_log]
J --> K([return transaction])
style B fill:#f0f4ff
style D fill:#f0f4ff
style E fill:#f0f4ff
style H fill:#f0f4ff
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A([payment.succeeded webhook]) --> B{DB: expense exists\nfor copilot_payment_id?}
B -- Yes early return --> Z([return undefined])
B -- No --> C[Fetch invoice + accounts]
C --> D{Xero: SPEND tx\nwith Reference==payment_id?}
D -- Found AUTHORISED --> G
D -- Not found --> E{Xero: legacy SPEND tx\nwith Reference==xero_invoice_id\nand matching amount?}
E -- Found AUTHORISED\none or more candidates\nwarn if more than one --> G[Reuse existing transaction]
E -- Not found --> F[createBankTransaction\nwith idempotency key]
F --> G
G --> H[Insert synced_payments row\nonConflictDoNothing]
H -- inserted.length == 0\nconcurrent race won --> I([return transaction\nskip sync log])
H -- inserted.length > 0 --> J[Write SUCCESS sync_log]
J --> K([return transaction])
style B fill:#f0f4ff
style D fill:#f0f4ff
style E fill:#f0f4ff
style H fill:#f0f4ff
Reviews (4): Last reviewed commit: "docs(OUT-3896): log xeroPaymentId after ..." | Re-trigger Greptile |
Expenses created before the reference change carry the invoice id as their Xero reference, not the payment id, so the payment-id reconcile lookup misses them. A retry after deploy would create a duplicate. - Add findLegacyExpenseByInvoice: search by the old invoice-id reference, match on exact integer cents, and adopt only a unique result (warn and skip when ambiguous) so we never pick the wrong payment's expense - Fall back to it in createPlatformExpensePayment before creating - Drop the unverified quote-escape on the reference filter; references are platform-generated ids/GUIDs with no quotes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai re-review |
The existing-expense select guard ran outside the try, so a DB error there propagated undecorated and skipped the sync_logs FAILED row that every other path produces. Move the guard inside the try so its failures are wrapped with failedSyncLogPayload like the rest of the flow. The region precondition check stays outside the try by design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… match - getExpenseByCopilotPaymentId now uses the relational query API (db.query.syncedPayments.findFirst) instead of a manual select + slice - Register syncedPayments in the schema barrel so db.query knows it - findLegacyExpenseByInvoice adopts the first match (only one fee exists per invoice, so multiple matches are duplicates of the same expense); warn when duplicates are present so they can be cleaned up Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai review commit 1b8e706 |
Summary
Repeated
payment.succeededwebhooks were creating duplicate Xero bank transactions and duplicatesynced_paymentsrows. This makes the expense sync idempotent across the failure modes we hit in prod.What changed
synced_payments (portal_id, tenant_id, copilot_payment_id) WHERE copilot_payment_id IS NOT NULL— durable DB guarantee of one expense per payment. Partial so the NULL-keyed PAYMENT rows are unaffected.createBankTransaction(copilotPaymentId) — handles the seconds-apart concurrent race within Xero's idempotency window.createPlatformExpensePayment: selects oncopilot_payment_idand returns early on a replay; the insert usesonConflictDoNothing()as the race backstop.referenceand reuse it. This covers the 2-hourly retry cron, which runs long after Xero's idempotency window. The invoice id moved fromreferenceinto the line-item description.Layers of defense
onConflictDoNothing→ durable backstopDeployment note (order matters)
The one-time dedupe of existing duplicate
synced_paymentsrows must run before this migration is applied, or the unique index creation will fail on existing duplicates. (Dedupe SQL and the ops scripts are intentionally not part of this PR.)Out of scope / follow-ups
BankTransactions already in Xero (manual accounting reconciliation).🤖 Generated with Claude Code