SMS workflow reminder retry count tracking

[akshayutture-augment]

Test 9

---

Replicated from ai-code-review-evaluation/cal.com-greptile#9

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[akshayutture-augment]

@greptileai

[greptile-apps]

Greptile Overview

Confidence Score: 1/5

• Critical bug will cause data loss across workflow reminder methods
• The deletion logic in scheduleSMSReminders.ts:38-42 uses an OR condition that deletes reminders with retryCount > 1 without filtering by method, affecting EMAIL and WHATSAPP reminders system-wide
• Pay critical attention to packages/features/ee/workflows/api/scheduleSMSReminders.ts - the deletion logic must be fixed before merge

Important Files Changed

File Analysis

Filename	Score	Overview
packages/prisma/migrations/20240508134359_add_retry_count_to_workflow_reminder/migration.sql	5/5	adds retryCount column with default 0, migration is clean and correct
packages/prisma/schema.prisma	5/5	adds retryCount Int @default(0) to WorkflowReminder model, schema change is correct
packages/features/ee/workflows/api/scheduleSMSReminders.ts	1/5	implements retry tracking with increment logic, but deletion logic has critical bug that affects non-SMS reminders

Sequence Diagram

sequenceDiagram
    participant Cron as Cron Job
    participant API as scheduleSMSReminders
    participant DB as Database
    participant Twilio as Twilio API

    Cron->>API: POST /scheduleSMSReminders
    
    Note over API,DB: Cleanup Phase
    API->>DB: deleteMany (scheduledDate <= now OR retryCount > 1)
    Note right of DB: BUG: Deletes ALL reminders<br/>with retryCount > 1<br/>(not just SMS!)
    
    Note over API,DB: Fetch Phase
    API->>DB: findMany (SMS, unscheduled, scheduledDate <= 7 days)
    DB-->>API: reminders with retryCount
    
    loop For each reminder
        Note over API: Build message & variables
        
        API->>Twilio: scheduleSMS(phone, message, date)
        
        alt Twilio Success
            Twilio-->>API: response with SID
            API->>DB: update(scheduled=true, referenceId=sid)
        else Twilio Returns null/falsy
            Note right of API: No error thrown
            API->>DB: update(retryCount = retryCount + 1)
        end
        
        alt Exception Thrown
            Note right of API: catch block
            API->>DB: update(retryCount = retryCount + 1)
        end
    end
    
    API-->>Cron: 200 OK

Loading

[greptile-apps]

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: deletion logic will remove ALL workflow reminders across different methods

the OR condition deletes reminders where retryCount > 1 WITHOUT checking method: WorkflowMethods.SMS, affecting EMAIL and WHATSAPP reminders too

Suggested change

	{
	retryCount: {
	gt: 1,
	},
	},
	{
	method: WorkflowMethods.SMS,
	retryCount: {
	gt: 1,
	},
	},

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/features/ee/workflows/api/scheduleSMSReminders.ts
Line: 38:42

Comment:
**logic:** deletion logic will remove ALL workflow reminders across different methods

the OR condition deletes reminders where `retryCount > 1` WITHOUT checking `method: WorkflowMethods.SMS`, affecting EMAIL and WHATSAPP reminders too

```suggestion
        {
          method: WorkflowMethods.SMS,
          retryCount: {
            gt: 1,
          },
        },
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

style: hardcoded threshold of 1 retry - no constant or config

the gt: 1 means reminders are deleted after 2 failures, but there's no named constant or explanation for why this specific value

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/features/ee/workflows/api/scheduleSMSReminders.ts
Line: 39:40

Comment:
**style:** hardcoded threshold of 1 retry - no constant or config

the `gt: 1` means reminders are deleted after 2 failures, but there's no named constant or explanation for why this specific value

How can I resolve this? If you propose a fix, please make it concise.