fix(automations): stop inbound runs being dropped and mislogged - #444
Merged
Conversation
Two defects that together produce the reported symptom — automation logs that read `status: 'success'` with `steps_executed: []`. 1. The dispatch loop in the inbound webhook was fire-and-forget inside the route's `after()` block. `after()` only keeps the function alive for promises it can see, so the run could be frozen after the log row was inserted but before any step executed. This is the same failure mode as issue #301, recurring one level down — the comments on the AI auto-reply and `message.received` dispatches immediately below already spell out the invariant this violated. Now awaited; the per-iteration `.catch` is kept so one trigger type can't skip the rest of the loop. 2. `executeAutomation` seeded the log row with `status: 'success'` before running anything. A run that died mid-flight therefore left a permanent success with no steps, indistinguishable from an automation that genuinely had nothing to do. Seeded as `'failed'` instead, so the status only becomes success if execution actually reaches a terminal path. No migration needed — 'failed' is already in the column's CHECK constraint, and every terminal path overwrites it. Two more points from the issue are deliberately not addressed here, as both change existing behaviour and want a maintainer decision: switching `contains` keyword matching to word boundaries (would alter which automations fire for current users) and re-opening a closed conversation on an inbound reply. Refs #409. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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.
Summary
Fixes the two defects behind #409's main symptom — automation logs reading
status: 'success'withsteps_executed: []("No steps recorded"). Both are confirmed againstmain.What changed
1.
src/app/api/whatsapp/webhook/route.ts— await the dispatch loop.runAutomationsForTriggerwas called fire-and-forget inside the route'safter()block.after()only keeps the function alive for promises it can see, so the run could be frozen after the log row was inserted but before any step ran. This is issue #301's failure mode recurring one level down — and the file already documents the invariant: the AI auto-reply dispatch 10 lines below is awaited "(same reason as the webhook dispatch below)", anddispatchWebhookEventbelow that carries "Awaited — not fire-and-forget — because we're inside the route'safter()block". The automations loop was the one outlier.The per-iteration
.catchis kept, so one trigger type failing can't skip the rest of the loop.2.
src/lib/automations/engine.ts— seed the log status pessimistically.executeAutomationinserted the log row withstatus: 'success'before executing anything. A run that died mid-flight therefore left a permanentsuccesswith an empty step list — indistinguishable from an automation that genuinely had nothing to do, which is exactly what the issue describes. Seeded as'failed'instead: the status only becomes success if execution reaches a terminal path.No migration needed —
'failed'is already in the column'sCHECK (status IN ('success','partial','failed')), and every terminal path (appendResultsat the outermost scope,finalizeLog) overwrites the seed. The wait-step park path still writes'partial'as before.Deliberately not in this PR
Two further points from the issue change existing behaviour and want your call rather than mine:
triggerMatchesdoes use rawhaystack.includes(k)forcontains(engine.ts:659), so short keywords do match inside unrelated words. But switching to\balters which automations fire for anyone already relying on substring matches — a behaviour change, not a bug fix.status. Whether a customer reply should reopen a thread an agent deliberately closed is a product decision. Note the issue also claims this blocks automations — it doesn't; dispatch is keyed on the contact, not the conversation status. The real effect is inbox visibility.Test plan
npm run typecheckclean.npm run lint— 39 warnings, identical tomain.npm test— 647 tests pass (2 new).npm run buildsucceeds.engine.test.ts: the log row is inserted asfailedwith no steps, and a completed run is still promoted tosuccess. Mutation-checked — restoring the'success'seed fails the first and only the first.The
after()change isn't unit-tested; there's no route-level harness onmainand the behaviour is a runtime lifetime property rather than something the module boundary exposes.Related
Refs #409 (partial — see "deliberately not in this PR").