Skip to content

fix: honour SMS opt-outs in the live sender and close prospect races - #43

Open
Mcgyver-ai wants to merge 5 commits into
mainfrom
fix/audit-followups
Open

Mcgyver-ai wants to merge 5 commits into
mainfrom
fix/audit-followups

Conversation

@Mcgyver-ai

Copy link
Copy Markdown
Owner

Summary

Follow-ups from a full-repo audit of main (e493421), covering findings 1–4 and 6. Branched from main, so it's independent of #39–#42.

# Fix Test
1 The live SMS sender now honours STOP. The Firebase sendReminders function has been the only live reminder sender since July, and it never read smsSuppressed, so leads who replied STOP kept getting texts. #41 fixes only the disabled Vercel cron route. The function now normalizes the number to E.164, marks non-E.164 numbers failed: invalid number, and marks opted-out leads skipped (functions/smsGuard.js, CommonJS, since functions/ can't import lib/). lib/__tests__/smsGuard.test.js: suppressed lead stopped, local-format number failed without a lookup, valid number allowed.
2 A double "booked" no longer double-fires. updateProspect reads, decides and writes in one runTransaction, so the second of two concurrent booked updates sees the first one's write and fires no webhooks, reminders or GHL appointment. prospectService.test.js: a transaction reading an already-booked prospect fires nothing; normal transition still fires; missing prospect → 404.
3 No duplicate prospects from back-to-back replies. ingestInboundReply matches-or-creates inside a transaction (tx.get(query) + tx.set). Firestore server transactions are serializable, so concurrent Meta webhooks for one sender can't both create a prospect. prospectService.test.js: creates inside the transaction when there's no match, reuses the match otherwise.
4 Outreach to must be exactly one address. nodemailer sends to every address in a comma list, and the unsubscribe check hashed the whole string, so "unsub@x.com, new@y.com" reached an unsubscribed address. New outreachRecipientSchema (single trimmed email); anything else → 400. schemas.test.js: comma, semicolon and display-name forms rejected.
6 Long threads can be deleted. deleteProspect used one batch, which caps at 500 writes. It now uses db.recursiveDelete, the same as accountService. prospectService.test.js updated.

Manual step after merge: the Cloud Function is deployed separately from Vercel. Run firebase deploy --only functions:sendReminders, otherwise fix 1 isn't live.

Not in this PR (logged in TASKS.md):

Test plan

  • yarn test: Test Files 21 passed (21) / Tests 150 passed (150)
  • yarn build passes (CI placeholder env): ✓ Compiled successfully / ✓ Generating static pages using 7 workers (75/75)
  • node --check functions/index.js functions/smsGuard.js
  • Manually verified locally / in preview. Not done: the Cloud Function isn't redeployed, and Vercel previews have no Firebase creds.

The Firebase function is the only live reminder sender and never read smsSuppressed, so leads who replied STOP kept getting texts.
…ransactional

A double-clicked booked update fired reminders, webhooks and GHL appointments twice; concurrent inbound webhooks created duplicate prospects. deleteProspect uses recursiveDelete instead of a 500-write batch.
A comma-separated to slipped unsubscribed addresses past the suppression check, which hashes the whole string.
Copilot AI balanced review requested due to automatic review settings October 17, 2026 13:36
@vercel

vercel Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
dm-forge Ready Ready Preview Oct 17, 2026 1:36pm UTC

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch was successfully deployed

1 active deployment
Preview — 091af450 Deployed Oct 17, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants