fix(whatsapp): preserve inbound action recovery - #667
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughWhatsApp action handlers now propagate retryable failures, restore consumed Redis state, and cache committed replies. ChangesWhatsApp action reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WhatsAppService
participant Redis
participant WhatsAppActionService
participant InteractiveService
WhatsAppService->>Redis: Check cached action outcome
Redis-->>WhatsAppService: Cached reply or no outcome
WhatsAppService->>WhatsAppActionService: Handle confirmation or cancellation
WhatsAppActionService-->>WhatsAppService: Commit reply outcome
WhatsAppService->>Redis: Store outcome with TTL
WhatsAppService->>InteractiveService: Send committed reply
WhatsAppService->>InteractiveService: Replay cached reply on duplicate input
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.ts (1)
170-185: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNarrow delivery-confirmation retryable failures to relevant replies.
handleScopeReplyis called for every inbound message, and its stack-level catch treats everyWhatsAppRetryableActionErroras retryable. A Redis read failure here causes the same retry path for unrelated messages and users without delivery state as post-purchase/shopper-action handlers treat only confirm/cancel replies like that. Return{ handled: false }unless the reply looks relevant for delivery confirmation, then throw for those flows only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.ts` around lines 170 - 185, Update handleScopedReply so it identifies whether messageText is a delivery-confirmation reply before reading Redis; return { handled: false } immediately for unrelated messages or users without relevant state, and only propagate Redis read failures as WhatsAppRetryableActionError for confirm/cancel flows. Preserve the existing state handling for relevant replies.
🧹 Nitpick comments (6)
apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.spec.ts (1)
163-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for the dispute-open restore path; the read/cancel/consume throw sites remain untested here.
See consolidated comment for the cross-file test-coverage gap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.spec.ts` around lines 163 - 186, Extend the WhatsApp post-purchase service tests around handlePendingReply to cover failures from the pending-dispute read, cancellation, and consumption operations. Mock each relevant Redis call to reject and assert the expected error propagation or handling, while preserving the existing restore-path test for transient dispute-opening failures.apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.ts (1)
226-255: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPending dispute state isn't cleared when the shopper sends an unrelated reply.
When
!confirms && !cancels, this returns{ handled: false }and leaves the pending dispute key untouched.whatsapp-shopper-action.service.ts's equivalent branch deletes the stale pending action in this case. Without clearing here, a later unrelated "yes"/"confirm" sent within the TTL window (5 minutes) could be interpreted as confirming a stale, no-longer-intended dispute.♻️ Suggested fix to match shopper-action's stale-state handling
if (!confirms && !cancels) { + try { + await this.redisService.del(key); + } catch (error) { + this.logFailure("stale pending dispute clear", phone, error); + } return { handled: false }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.ts` around lines 226 - 255, Update the unrelated-reply branch in the pending dispute handling flow to delete the Redis key before returning `{ handled: false }`, matching the stale-state behavior in the equivalent shopper action service. Reuse the existing dispute-clear error handling used by `this.redisService.del(key)`, including logging via `logFailure` and throwing `WhatsAppRetryableActionError` if cleanup fails.apps/backend/src/channels/whatsapp/whatsapp-shopper-action.service.spec.ts (1)
163-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid coverage for read-failure and add_to_cart restore paths; cancel/consume throws and the
select_delivery_addresswrite-failure branch remain untested.See consolidated comment for the cross-file test-coverage gap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-shopper-action.service.spec.ts` around lines 163 - 193, Extend the WhatsApp shopper action tests around handlePendingActionReply to cover cancellation/consumption failures and the select_delivery_address write-failure path. Assert that consume-related errors propagate with the expected behavior and that a failed address-selection mutation restores the pending action for retry, matching the existing add_to_cart restoration assertions.apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.spec.ts (1)
233-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for the confirmDelivery restore path; other new throw sites in this file remain untested.
This test correctly verifies the restore-and-retry behavior. However,
handleScopedReply's initial state-read failure,selectOrder's attempt-count-read failure, andhandleConfirmationError's attempt-increment failure are new throw sites without dedicated tests here (see consolidated comment).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.spec.ts` around lines 233 - 256, The test suite covers only the confirmDelivery failure path; add dedicated tests for the new failure branches in handleScopedReply, selectOrder, and handleConfirmationError. Mock the initial state read, attempt-count read, and attempt increment operations to reject, then assert each method propagates the expected error behavior without changing the existing restore-and-retry test.apps/backend/src/channels/whatsapp/whatsapp.service.ts (1)
293-308: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated commit-and-persist pattern across three branches.
Delivery confirmation, dispute, and shopper-action handled-paths all repeat "build
InboundActionOutcome→ persist best-effort → send". Extracting a small helper (e.g.,commitInboundActionOutcome(phone, messageId, outcome, send)) would reduce the risk of a future fourth call site forgetting to persist the outcome.♻️ Sketch
+private async commitInboundActionOutcome( + phone: string, + messageId: string | undefined, + outcome: InboundActionOutcome, + send: (message: string) => Promise<void>, +): Promise<InboundActionOutcome> { + await this.persistInboundActionOutcomeBestEffort(phone, messageId, outcome); + await send(outcome.message); + return outcome; +}Also applies to: 324-338, 355-365
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp.service.ts` around lines 293 - 308, Extract the repeated “build outcome, persist best-effort, then send” sequence from the delivery-confirmation, dispute, and shopper-action branches into a shared helper near the existing inbound outcome methods, such as commitInboundActionOutcome. Update all three branches to call it with phone, messageId, the constructed InboundActionOutcome, and the appropriate send operation, preserving their existing messages and behavior.apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts (1)
1868-1909: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid replay test for the delivery-confirmation path; consider mirroring for dispute and shopper-action outcomes.
The test correctly exercises: commit → send failure → retryable rethrow → cache hit on retry → handler not re-invoked. The same commit/replay wiring exists for
pendingDisputeReplyandpendingActionReply(whatsapp.service.ts lines 324-338, 355-365) but isn't covered here, only the delivery-confirmation branch is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts` around lines 1868 - 1909, Extend the replay coverage in the WhatsApp service tests to mirror the existing “replays a committed action outcome without executing the action twice” case for both pendingDisputeReply and pendingActionReply. For each branch, cover commit, send failure with retryable rethrow, cached outcome replay, and verify the corresponding handler is invoked only once; reuse the existing cache key and outcome conventions from the delivery-confirmation test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.ts`:
- Around line 388-395: Prevent the transient failure path around confirmDelivery
from restoring enter_code state after the server-side confirmation may already
have succeeded. Update the handler containing logFailure, storeState, and
WhatsAppRetryableActionError so retries cannot resubmit the same OTP or repeat
delivery side effects; apply the same guard to equivalent retryable action
handlers in the post-purchase and shopper-action services.
In `@apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.ts`:
- Around line 295-309: Update the failure branch after safeOpenError in the
post-purchase dispute flow: do not call restorePendingDispute or throw
WhatsAppRetryableActionError for non-idempotent openBuyerDispute failures. Only
classify errors explicitly guaranteed to mean the dispute was not created as
retryable and restore their consumed state; handle all other failures without
reissuing the action.
In `@apps/backend/src/channels/whatsapp/whatsapp.service.ts`:
- Around line 293-308: Ensure duplicate-prevention outcomes are rethrow-safe
only when persistence succeeds: update persistInboundActionOutcomeBestEffort and
the send-failure handling around committedActionOutcome so Redis write failures
are tracked and prevent retrying an already-consumed action without a replayable
cache entry. Apply the same behavior to delivery confirmation, dispute, and
shopper-action outcome paths, while preserving retries when the outcome was
successfully cached.
---
Outside diff comments:
In
`@apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.ts`:
- Around line 170-185: Update handleScopedReply so it identifies whether
messageText is a delivery-confirmation reply before reading Redis; return {
handled: false } immediately for unrelated messages or users without relevant
state, and only propagate Redis read failures as WhatsAppRetryableActionError
for confirm/cancel flows. Preserve the existing state handling for relevant
replies.
---
Nitpick comments:
In
`@apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.spec.ts`:
- Around line 233-256: The test suite covers only the confirmDelivery failure
path; add dedicated tests for the new failure branches in handleScopedReply,
selectOrder, and handleConfirmationError. Mock the initial state read,
attempt-count read, and attempt increment operations to reject, then assert each
method propagates the expected error behavior without changing the existing
restore-and-retry test.
In `@apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.spec.ts`:
- Around line 163-186: Extend the WhatsApp post-purchase service tests around
handlePendingReply to cover failures from the pending-dispute read,
cancellation, and consumption operations. Mock each relevant Redis call to
reject and assert the expected error propagation or handling, while preserving
the existing restore-path test for transient dispute-opening failures.
In `@apps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.ts`:
- Around line 226-255: Update the unrelated-reply branch in the pending dispute
handling flow to delete the Redis key before returning `{ handled: false }`,
matching the stale-state behavior in the equivalent shopper action service.
Reuse the existing dispute-clear error handling used by
`this.redisService.del(key)`, including logging via `logFailure` and throwing
`WhatsAppRetryableActionError` if cleanup fails.
In `@apps/backend/src/channels/whatsapp/whatsapp-shopper-action.service.spec.ts`:
- Around line 163-193: Extend the WhatsApp shopper action tests around
handlePendingActionReply to cover cancellation/consumption failures and the
select_delivery_address write-failure path. Assert that consume-related errors
propagate with the expected behavior and that a failed address-selection
mutation restores the pending action for retry, matching the existing
add_to_cart restoration assertions.
In `@apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts`:
- Around line 1868-1909: Extend the replay coverage in the WhatsApp service
tests to mirror the existing “replays a committed action outcome without
executing the action twice” case for both pendingDisputeReply and
pendingActionReply. For each branch, cover commit, send failure with retryable
rethrow, cached outcome replay, and verify the corresponding handler is invoked
only once; reuse the existing cache key and outcome conventions from the
delivery-confirmation test.
In `@apps/backend/src/channels/whatsapp/whatsapp.service.ts`:
- Around line 293-308: Extract the repeated “build outcome, persist best-effort,
then send” sequence from the delivery-confirmation, dispute, and shopper-action
branches into a shared helper near the existing inbound outcome methods, such as
commitInboundActionOutcome. Update all three branches to call it with phone,
messageId, the constructed InboundActionOutcome, and the appropriate send
operation, preserving their existing messages and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cbb1eb13-08c9-4cb9-b62e-9bec31b714b0
📒 Files selected for processing (10)
apps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-delivery-confirmation.service.tsapps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-post-purchase.service.tsapps/backend/src/channels/whatsapp/whatsapp-retryable-action.error.tsapps/backend/src/channels/whatsapp/whatsapp-shopper-action.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-shopper-action.service.tsapps/backend/src/channels/whatsapp/whatsapp.constants.tsapps/backend/src/channels/whatsapp/whatsapp.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp.service.ts
1 participant
What does this PR do?
Preserves BullMQ retry semantics for confirmed WhatsApp shopper actions and prevents committed cart, address, dispute, and delivery-confirmation outcomes from being executed again when only the outbound Meta response fails. Transient Redis/domain failures now propagate as retryable processing errors, consumed confirmation state is restored when the underlying action has not completed, and safe committed outcomes are cached by normalized phone plus inbound Meta message ID for replay on retry.
Type of change
Area affected
How to test this
cd apps/backend && pnpm exec jest --runInBand src/channels/whatsapp/whatsapp-shopper-action.service.spec.ts src/channels/whatsapp/whatsapp-post-purchase.service.spec.ts src/channels/whatsapp/whatsapp-delivery-confirmation.service.spec.ts src/channels/whatsapp/whatsapp.service.spec.ts.cd apps/backend && pnpm typecheckandpnpm build.Expected result: all 4 focused suites (131 tests) pass; transient confirmed-action failures reject for BullMQ retry, consumed pending state is restored, deterministic shopper errors remain final safe replies, and a committed delivery confirmation is not executed twice when the first Meta send fails.
Pre-commit checklist
console.logleft in production code.envfiles committedanytypes addeddb push(no database changes)Screenshots
N/A. Backend WhatsApp reliability change only; no UI changed.
Notes for reviewer
WhatsAppRetryableActionErrorso the channel boundary distinguishes retryable confirmed-action failures from ordinary conversational/Gemini failures that still use the existing safe fallback.git diff --checkandgit diff --cached --checkpassed; staged secret/private-prompt scans returned no matches..env.local, secrets, or private prompt values were changed.Summary by CodeRabbit
New Features
Bug Fixes