fix(webhook): E08+E09 — upsert idempotente em handleIncoming e handleOutgoing - #119
Closed
adm01-debug wants to merge 1 commit into
Closed
adm01-debug wants to merge 1 commit into
adm01-debug wants to merge 1 commit into
Conversation
handleOutgoingWhatsAppMessage e handleIncomingMessage: - INSERT final → upsert com ignoreDuplicates:true (ON CONFLICT DO NOTHING) - msgError.code === '23505' tratado como sucesso silencioso Com ux_messages_dedup (E07), race conditions geram 23505 desnecessariamente. Agora o segundo webhook retorna sem ruído. Lógica de preservação de status/content (existingMessage path) permanece intacta.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
adm01-debug has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 105 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
🟡 Changes recommended
The diff introduces a TypeScript syntax error and uses an onConflict target that likely won’t match the repo’s partial unique index, risking runtime failures instead of idempotent behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the main Evolution GO webhook message handlers to make message persistence idempotent under concurrent deliveries by switching from insert().select().single() to upsert({ ignoreDuplicates: true }), avoiding duplicate-key failures and “0 rows” .single() errors.
Changes:
- Convert outgoing and incoming message inserts to idempotent
upsert(..., { ignoreDuplicates: true })to tolerate concurrent webhook POSTs. - Adjust incoming handler to use
.maybeSingle()when selecting the inserted id (to allow “no row” when the insert is ignored).
File summaries
| File | Description |
|---|---|
| supabase/functions/_shared/evolution-webhook-messages.ts | Moves incoming/outgoing message persistence to idempotent upserts and tweaks follow-up selection/error handling. |
Review details
Suppressed comments (1)
supabase/functions/_shared/evolution-webhook-messages.ts:228
- Same as outgoing handler: specifying
onConflict: 'whatsapp_connection_id,external_id,sender'can fail if the only matching unique index is partial (ux_messages_dedup ... WHERE external_id IS NOT NULL). Dropping the explicit conflict target keeps the upsert idempotent (ON CONFLICT DO NOTHING) without relying on index inference.
}, { onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true })
.select('id').maybeSingle();
// E09: 23505 = conflito único (invocação concorrente do webhook) → mensagem já inserida → ok
if (msgError && msgError.code !== '23505') {
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ? new Date((data.messageTimestamp as number) * 1000).toISOString() : new Date().toISOString(); | ||
|
|
||
| const recentCutoff = new Date(Date.now() - 60_000).toISOString(); | ||
| const recentCutoff = new Date(Date.now.) - 60_000).toISOString(); |
Comment on lines
+117
to
+124
| const { error: msgError } = await supabase.from('messages').upsert({ | ||
| contact_id: contact.id, whatsapp_connection_id: connection.id, content: parsed.content, | ||
| message_type: parsed.messageType, media_url: mediaUrl, sender: 'agent', external_id: externalId, | ||
| status: 'sent', created_at: messageCreatedAt, agent_id: contact.assigned_to || null, | ||
| }).select('id').single(); | ||
| }, { onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true }); | ||
|
|
||
| if (msgError) { console.error('[FROM_ME] Error inserting outgoing message:', msgError); return; } | ||
| // E09: 23505 = conflito único (invocação concorrente do webhook) → mensagem já inserida → ok | ||
| if (msgError && msgError.code !== '23505') { console.error('[FROM_ME] Error inserting outgoing message:', msgError); return; } |
| if (bytes.length > 100) { | ||
| const fileName = `sticker_${Date.now()}_${key.id.replace(/[^a-zA-Z0-9]/g, '')}.webp`; | ||
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '31536000' }); | ||
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '315360000' }); |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/functions/_shared/evolution-webhook-messages.ts">
<violation number="1" location="supabase/functions/_shared/evolution-webhook-messages.ts:121">
P0: When the checked-in E07 migration supplies `ux_messages_dedup`, these upserts fail because `onConflict` cannot infer its partial index without a predicate. Both handlers then log the error and drop the message; make the index non-partial or avoid `onConflict` and handle `23505` from `insert`.</violation>
<violation number="2" location="supabase/functions/_shared/evolution-webhook-messages.ts:276">
P3: When a sticker is fetched through `directMediaUrl`, this sets a ten-year cache TTL, unlike the one-year base64 path. Keep both paths at `31536000` unless ten-year retention is intentional.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| message_type: parsed.messageType, media_url: mediaUrl, sender: 'agent', external_id: externalId, | ||
| status: 'sent', created_at: messageCreatedAt, agent_id: contact.assigned_to || null, | ||
| }).select('id').single(); | ||
| }, { onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true }); |
There was a problem hiding this comment.
P0: When the checked-in E07 migration supplies ux_messages_dedup, these upserts fail because onConflict cannot infer its partial index without a predicate. Both handlers then log the error and drop the message; make the index non-partial or avoid onConflict and handle 23505 from insert.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_shared/evolution-webhook-messages.ts, line 121:
<comment>When the checked-in E07 migration supplies `ux_messages_dedup`, these upserts fail because `onConflict` cannot infer its partial index without a predicate. Both handlers then log the error and drop the message; make the index non-partial or avoid `onConflict` and handle `23505` from `insert`.</comment>
<file context>
@@ -113,13 +113,15 @@ export async function handleOutgoingWhatsAppMessage(
message_type: parsed.messageType, media_url: mediaUrl, sender: 'agent', external_id: externalId,
status: 'sent', created_at: messageCreatedAt, agent_id: contact.assigned_to || null,
- }).select('id').single();
+ }, { onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true });
- if (msgError) { console.error('[FROM_ME] Error inserting outgoing message:', msgError); return; }
</file context>
| if (bytes.length > 100) { | ||
| const fileName = `sticker_${Date.now()}_${key.id.replace(/[^a-zA-Z0-9]/g, '')}.webp`; | ||
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '31536000' }); | ||
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '315360000' }); |
There was a problem hiding this comment.
P3: When a sticker is fetched through directMediaUrl, this sets a ten-year cache TTL, unlike the one-year base64 path. Keep both paths at 31536000 unless ten-year retention is intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_shared/evolution-webhook-messages.ts, line 276:
<comment>When a sticker is fetched through `directMediaUrl`, this sets a ten-year cache TTL, unlike the one-year base64 path. Keep both paths at `31536000` unless ten-year retention is intentional.</comment>
<file context>
@@ -268,7 +273,7 @@ export async function handleStickerMedia(
if (bytes.length > 100) {
const fileName = `sticker_${Date.now()}_${key.id.replace(/[^a-zA-Z0-9]/g, '')}.webp`;
- const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '31536000' });
+ const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '315360000' });
if (!uploadErr) { const { data: urlData } = supabase.storage.from('whatsapp-media').getPublicUrl(`stickers/${fileName}`); mediaUrl = urlData.publicUrl; }
}
</file context>
Suggested change
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '315360000' }); | |
| const { error: uploadErr } = await supabase.storage.from('whatsapp-media').upload(`stickers/${fileName}`, bytes, { contentType: 'image/webp', cacheControl: '31536000' }); |
Owner
Author
|
Fechando: este PR tem um bug de sintaxe introduzido no diff ( O PR #120 cobre o mesmo escopo (E08 completo nos dois handlers principais) sem esse erro, mais o guard E35 de LID. Seguindo por lá. Generated by Claude Code |
adm01-debug
pushed a commit
that referenced
this pull request
Sep 1, 2026
…a PK) Copilot, coderabbitai e cubic apontaram independentemente o mesmo bug real na revisão: sem onConflict, o PostgREST usa a PK da tabela (messages.id, UUID gerado novo a cada insert) como alvo do conflito. Como id nunca colide, o DO NOTHING do ignoreDuplicates nunca disparava contra o índice ux_messages_dedup — o "fix" de idempotência do E08 não protegia nada, e dois webhooks concorrentes para o mesmo external_id continuariam criando duas linhas. Minha réplica no PR (baseada no review do #119, que criticava onConflict porque o índice era parcial na época) estava desatualizada: a migration 20260901100002 já recriou ux_messages_dedup como índice único NÃO-parcial em (whatsapp_connection_id, external_id, sender), então onConflict aponta para um índice inferível e volta a funcionar como pretendido. Também corrige o texto do warning de normalizePhone ("sem DDI válido" → linguagem que não afirma uma validação E.164 real que o código não faz — a checagem é só tamanho + prefixo de grupo). Validado: lint-ratchet (0 novas), typecheck, manifest regenerado, 167/167 testes de contrato. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018N9zUcTpab3dWsuR3ZSSBj
adm01-debug
added a commit
that referenced
this pull request
Sep 1, 2026
) * fix(webhook): E35 normalizePhone rejeita LIDs de 14+ digitos - Detecta sufixo @lid → return null (protecao primaria) - Fallback length >= 14 sem DDI → return null (LIDs sem sufixo) - Excecoes: grupos WA 120363/120392/120415/120496 passam - 410 contatos-LID existentes NAO sao afetados (dados historicos) - Novos LIDs parao de criar contatos fragmentados imediatamente" * fix(webhook): E08 completo — handleIncomingMessage + handleOutgoingWhatsAppMessage - INSERT -> upsert com { ignoreDuplicates: true } nos dois handlers principais - handleIncomingMessage: .single() -> .maybeSingle() + guard if (!insertedMessage) return silencia race condition sem perder mensagens legitimas - handleOutgoingWhatsAppMessage: remove .select().single() desnecessario (ID nao e usado apos insert neste caminho)" * fix(webhook): remove churn não relacionado ao E08/E35, corrige lint O diff original desta branch misturava o fix real (E08 upsert idempotente + E35 guard de LID em normalizePhone) com reformatação e remoção de comentários explicativos em ~15 pontos não relacionados (resolveEventJid, connection cache, getContactByPhone, generatePhoneVariants, handleStickerMedia, handleAudioTranscription). Reverte esse churn mantendo só a mudança semântica de cada fix, sobre a base atual de origin/main. Troca o console.log novo (guard de duplicata silenciosa em handleIncomingMessage) por console.warn — único nível permitido pela regra no-console deste projeto; o lint-ratchet acusava 1 nova ocorrência de dívida. Validado nesta branch: - lint-ratchet: 0 novas ocorrências (scripts/ci/lint-ratchet.mjs) - typecheck: OK (tsc --noEmit, escopo src/) - edge deployment manifest: regenerado (supabase/deployment-manifest.json) - testes de contrato (vitest.contracts.config.ts): 167/167 passam - guards CI (scripts/ci/*.unit.mjs, scripts/edge-deploy/*.unit.mjs): 30/30 passam - check-workflow-pins: OK - supabase-usage-guard: 0 violações novas Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018N9zUcTpab3dWsuR3ZSSBj * fix(webhook): onConflict explícito no upsert de E08 (PostgREST usava a PK) Copilot, coderabbitai e cubic apontaram independentemente o mesmo bug real na revisão: sem onConflict, o PostgREST usa a PK da tabela (messages.id, UUID gerado novo a cada insert) como alvo do conflito. Como id nunca colide, o DO NOTHING do ignoreDuplicates nunca disparava contra o índice ux_messages_dedup — o "fix" de idempotência do E08 não protegia nada, e dois webhooks concorrentes para o mesmo external_id continuariam criando duas linhas. Minha réplica no PR (baseada no review do #119, que criticava onConflict porque o índice era parcial na época) estava desatualizada: a migration 20260901100002 já recriou ux_messages_dedup como índice único NÃO-parcial em (whatsapp_connection_id, external_id, sender), então onConflict aponta para um índice inferível e volta a funcionar como pretendido. Também corrige o texto do warning de normalizePhone ("sem DDI válido" → linguagem que não afirma uma validação E.164 real que o código não faz — a checagem é só tamanho + prefixo de grupo). Validado: lint-ratchet (0 novas), typecheck, manifest regenerado, 167/167 testes de contrato. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018N9zUcTpab3dWsuR3ZSSBj --------- Co-authored-by: Claude <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexto
PR #112 cobriu
evolution-webhook-msg-handlers.ts(handleMessagesUpdate/Delete/Set).Este PR cobre o arquivo complementar
evolution-webhook-messages.tscom os handlers principais de tráfego real.Problema
Com
ux_messages_dedupativo, invocações concorrentes do webhook (dois POSTs simultâneos da Evolution GO para o mesmoexternal_id) causam:handleIncomingMessage:INSERTfalha silenciosamente comPGRST116(0 rows do.single()) ou expõe23505como errohandleOutgoingWhatsAppMessage: idem —.single()falha quando conflito ignoradoSolução
Substituição de
INSERT + .select().single()por:upsert({ onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true })msgErrorverificado apenas paracode !== '23505'(E09 — 23505 = duplicata ignorada pelo índice = ok)Por que
onConflictexplícito funciona aquiO
ux_messages_dedupno banco é não parcial (semWHERE):O
ON CONFLICT (col1, col2, col3)é inferível. NULLs emexternal_idnão conflitam (comportamento padrão do UNIQUE).Simulação de race condition
ignoreDuplicates→ silêncio23505retornado)Ambos retornam 200. Banco fica com 1 linha. ✅
Handlers alterados
handleOutgoingWhatsAppMessageevolution-webhook-messages.tsinsert().select('id').single()upsert(ignoreDuplicates)handleIncomingMessageevolution-webhook-messages.tsinsert().select('id').single()upsert(ignoreDuplicates).select('id').maybeSingle()Summary by cubic
Fixes concurrent webhook handling by switching to idempotent upserts in both main message handlers, so duplicate POSTs from Evolution GO no longer cause PGRST116 or 23505 errors. The second webhook now returns silently while the first insert is preserved.
upsert({ onConflict: 'whatsapp_connection_id,external_id,sender', ignoreDuplicates: true })forhandleOutgoingWhatsAppMessageandhandleIncomingMessage.23505as success because it indicates a conflicting concurrent insert.cacheControlfrom31536000to315360000.recentCutoff(new Date(Date.now.)) that should be fixed.Written for commit af643f6. Summary will update on new commits.