Skip to content

refactor: schedule SMS reminders via tasker for BEFORE_EVENT/AFTER_EVENT#27942

Open
anikdhabal wants to merge 3 commits intomainfrom
sms-reminder
Open

refactor: schedule SMS reminders via tasker for BEFORE_EVENT/AFTER_EVENT#27942
anikdhabal wants to merge 3 commits intomainfrom
sms-reminder

Conversation

@anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented Feb 13, 2026

Summary by cubic

Schedules BEFORE_EVENT/AFTER_EVENT SMS reminders via Tasker for reliable queuing and cancellation. Adds lazy SMS scheduling and a Tasker task to send SMS with templates and fallback email while keeping the email path unchanged.

  • Refactors
    • Route SMS actions in reminderScheduler to Tasker; emails keep existing flow.
    • Add WorkflowService.scheduleLazySMSWorkflow to compute schedule, create WorkflowReminder, and enqueue a Tasker job with referenceUid (skips past dates for BEFORE_EVENT).
    • Introduce sendWorkflowSMS task: fetches reminder data, verifies step, resolves recipient (incl. seats), builds message (custom/template), shortens links, appends opt‑out, checks credits, and sends SMS or fallback email; skips if opted out or unverified.
    • Support cancellation of scheduled SMS by cancelling Tasker jobs via workflowReminder.uuid when no provider reference is present.
    • Register task and payload schema in Tasker; add repository method to load fields required by the SMS task.

Written for commit c2d5fd5. Summary will update on new commits.

@graphite-app graphite-app bot added the core area: core, team members only label Feb 13, 2026
@devin-ai-integration
Copy link
Contributor

I reviewed this PR thoroughly. The overall approach is solid — mirroring the lazy email scheduling pattern for SMS via Tasker is the right fix. Here's my review:

Summary

The PR correctly identifies and fixes the root cause: shouldUseTwilio rejects SMS reminders scheduled >2 hours out, and the CRON job only processes within a 2-hour window, so BEFORE_EVENT/AFTER_EVENT SMS reminders for distant events are never sent.

The fix routes these reminders through the Tasker instead, which handles scheduling at any future date.

Core changes look good

  • scheduleLazySMSWorkflow in WorkflowService mirrors the email version correctly
  • reminderScheduler.ts routing is clean
  • deleteScheduledSMSReminder properly cancels Tasker jobs when no Twilio referenceId exists
  • Task + type registration in tasker is correct

Issues found in sendWorkflowSMS.ts

I've left inline comments on the specific lines, but here's the summary:

  1. Missing verifiedAt check — The email task handler (EmailWorkflowService.handleSendEmailWorkflowTask) checks workflowStep.verifiedAt before sending. The SMS task skips this, meaning unverified workflow steps could send SMS. The shared select object doesn't include verifiedAt, so it needs to be fetched separately or the select needs extending.

  2. Missing opt-out checkscheduleSMSReminder checks WorkflowOptOutContactRepository.isOptedOut(reminderPhone) before sending. The task doesn't, so opted-out numbers could receive SMS through the lazy path.

  3. Missing number verification for SMS_NUMBERscheduleSMSReminder verifies the phone number exists in verifiedNumber table when action is SMS_NUMBER. The task skips this.

  4. user.id not in select → broken profile query (line 91) — The shared select object doesn't select user.id, but reminder.booking.user?.id is used in the profile query. Since user type is Partial<User>, id will be undefined at runtime, causing prisma.profile.findFirst({ where: { userId: undefined } }) to match arbitrarily or return nothing — leading to an incorrect bookerUrl.

  5. Unused importWorkflowMethods is imported but never used.

I'd recommend looking at how EmailWorkflowService structures its task handler as a reference — it uses a dedicated repository method (findByIdIncludeStepAndWorkflow) that selects exactly the fields it needs including verifiedAt, rather than reusing the shared CRON select.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 6 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/features/tasker/tasks/sendWorkflowSMS.ts">

<violation number="1" location="packages/features/tasker/tasks/sendWorkflowSMS.ts:89">
P2: Limit the profile query to only the needed fields (organizationId) instead of fetching the full row.</violation>

<violation number="2" location="packages/features/tasker/tasks/sendWorkflowSMS.ts:169">
P2: The opt-out message is being added to `bodyWithoutOptOut` while the SMS body uses the unmodified message. This causes SMS to miss the opt-out text when enabled and emails to include it. Swap the usage so SMS gets the opt-out version and email uses the plain message.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@github-actions
Copy link
Contributor

Devin AI is addressing Cubic AI's review feedback

A Devin session has been created to address the issues identified by Cubic AI.

View Devin Session

@devin-ai-integration
Copy link
Contributor

I reviewed both Cubic AI violations and checked their confidence scores:

  1. Violation 1 (line 89 — limit profile query fields): Confidence 8/10 — skipping per policy (threshold is 9/10).
  2. Violation 2 (line 169 — opt-out message swap): Confidence 7/10 — skipping per policy (threshold is 9/10).

Neither issue meets the 9/10 confidence threshold required for automated fixes. These should be reviewed manually by a human reviewer if needed.

@paragon-review
Copy link

Paragon: tests updated

4 updated tests generated for this PR.

Updated Tests

  • scheduleLazySMSWorkflow unit tests — Tests WorkflowService.scheduleLazySMSWorkflow: BEFORE_EVENT/AFTER_EVENT scheduling, missing bookingUid, null time/timeUnit, past scheduled dates, AFTER_EVENT not skipped, seat references, null id/uuid from repository, repository errors, correct SMS method, correct tasker task type, and MINUTE/DAY timeUnit calculations.
  • reminderScheduler SMS lazy routing tests — Tests _scheduleWorkflowReminders routing: BEFORE_EVENT+SMS_ATTENDEE routes through scheduleLazySMSWorkflow, AFTER_EVENT+SMS_NUMBER routes through lazy path, email steps route through scheduleLazyEmailWorkflow (not SMS), mixed steps both route correctly, NEW_EVENT+SMS does NOT use lazy path, empty workflows skipped, seatReferenceId passed, isDryRun prevents scheduling, empty workflows array, multiple workflows with different triggers.
  • sendWorkflowSMS tasker task tests — Tests sendWorkflowSMS task handler: ZSendWorkflowSMSSchema validation (valid/invalid payloads), early returns for missing reminder/workflowStep/booking, SMS_ATTENDEE with custom template, SMS_NUMBER to specific number, REMINDER template fallback, empty message handling, missing phone number, seat reference attendee lookup, seat reference fallback, invalid JSON/schema payloads, opt-out message inclusion, team workflow with teamId, timezone selection based on action.
  • smsReminderManager unit tests — Tests deleteScheduledSMSReminder: cancel via Twilio with referenceId, cancel via tasker.cancelWithReference when referenceId is null (using sendWorkflowSMS task type), no tasker cancel when uuid missing, no cancel when reminder not found, error handling for both paths. Also tests scheduleSMSReminder guard clauses: null verifiedAt, null reminderPhone, opted-out phone, unverified SMS_NUMBER, SMS_ATTENDEE bypassing verification.

Accept Changes Open in Paragon

Details

Updated Tests

  • scheduleLazySMSWorkflow unit tests (unit)
  • reminderScheduler SMS lazy routing tests (unit)
  • sendWorkflowSMS tasker task tests (unit)
  • smsReminderManager unit tests (unit)

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/features/ee/workflows/repositories/WorkflowReminderRepository.ts">

<violation number="1" location="packages/features/ee/workflows/repositories/WorkflowReminderRepository.ts:188">
P2: `attendees: true` pulls all attendee columns. The SMS task only uses a handful of attendee fields (email/name/phone/timezone/locale), so this over-fetches data and increases exposure. Select only the fields actually used.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@github-actions
Copy link
Contributor

Devin AI is addressing Cubic AI's review feedback

A Devin session has been created to address the issues identified by Cubic AI.

View Devin Session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant