fix: break circular dependency by passing creditCheckFn in messageDispatcher #25343

hbjORbj · 2025-11-22T05:55:14Z

What does this PR do?

This PR completes the circular dependency fix between the workflows and billing packages by making creditCheckFn a required parameter throughout the workflow reminder system, eliminating the dynamic import fallback entirely.

Context: This builds on PR #25312 which fixed the reminderScheduler circular dependency. This PR addresses the remaining circular dependency in messageDispatcher and related functions.

The circular dependency:

credit-service → reminderScheduler → smsReminderManager → messageDispatcher → credit-service (dynamic import)

The solution: Use dependency injection by making creditCheckFn required and wiring it from all callers (which live outside the workflows package and can safely import CreditService).

Mandatory Tasks

I have self-reviewed the code
I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - internal refactoring only
I confirm automated tests are in place that prove my fix is effective or that my feature works.

…jection Break the 4-file circular dependency chain: credit-service → reminderScheduler → smsReminderManager → messageDispatcher → credit-service Solution: - Add optional creditCheckFn parameter to messageDispatcher functions - Thread creditCheckFn through the call chain: scheduleWorkflowReminders → scheduleSMSReminder/scheduleWhatsappReminder → messageDispatcher - When creditCheckFn is provided, use it; otherwise fall back to dynamic CreditService import for backward compatibility - This breaks the workflows → billing import while preserving immediate fallback behavior Changes: - messageDispatcher: Accept optional creditCheckFn parameter, use it if provided - smsReminderManager: Thread creditCheckFn through scheduleSMSReminder - whatsappReminderManager: Thread creditCheckFn through scheduleWhatsappReminder - reminderScheduler: Add creditCheckFn to ScheduleWorkflowRemindersArgs and pass through processWorkflowStep All type checks, lint checks, and unit tests pass. Co-Authored-By: [email protected] <[email protected]>

devin-ai-integration · 2025-11-22T05:55:17Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR that start with 'DevinAI' or '@devin'.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment and CI monitoring

…ency fix - Add creditCheckFn parameter to WorkflowService.scheduleFormWorkflows - Wire creditCheckFn from all 10 entry points that call workflow scheduling: * formSubmissionUtils.ts (form submissions) * roundRobinManualReassignment.ts (round-robin reassignment) * triggerFormSubmittedNoEventWorkflow.ts (form workflow trigger) * handleBookingRequested.ts (booking requests) * RegularBookingService.ts (2 calls - payment initiated & new bookings) * handleSeats.ts (seated bookings) * handleConfirmation.ts (2 calls - confirmation & payment) * handleMarkNoShow.ts (no-show updates) * confirm.handler.ts (booking rejection) - Update test expectations to use expect.objectContaining() - Fix pre-existing lint warning in handleMarkNoShow.ts (any type) - This completes the messageDispatcher circular dependency fix by ensuring creditCheckFn is actually passed through the call chain, breaking the 4-file circular dependency at runtime Co-Authored-By: [email protected] <[email protected]>

vercel · 2025-11-24T07:58:52Z

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

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
cal	Ignored			Nov 26, 2025 0:34am
cal-eu	Ignored			Nov 26, 2025 0:34am

… check error - Replace constrained type with generic type parameter - Add proper type guard for rejected promises - Fixes CI type check failure in handleMarkNoShow.ts:385 - Avoids 'any' type while accepting any fulfilled value shape Co-Authored-By: [email protected] <[email protected]>

github-actions · 2025-11-24T09:42:08Z

E2E results are ready!

Workflow #90176.2 latest results

…backs - Wire creditCheckFn in packages/sms/sms-manager.ts (can safely import CreditService) - Create makeHandler factory pattern for CRON endpoints (scheduleSMSReminders.ts, scheduleWhatsappReminders.ts) - Wire creditCheckFn from apps/web CRON routes to factories - Add warning log in messageDispatcher when fallback is used - Complete creditCheckFn wiring from all direct callers (activateEventType.handler.ts, util.ts) This eliminates all fallbacks to dynamic import except as a safety net for unforeseen call sites. The circular dependency (workflows ↔ billing) remains acceptable as discussed with user (Option C). Co-Authored-By: [email protected] <[email protected]>

The scheduleFormWorkflows function now receives creditCheckFn as a parameter. Updated test assertions to use expect.objectContaining() with creditCheckFn: expect.any(Function) to account for the new dependency injection parameter. Co-Authored-By: [email protected] <[email protected]>

The sendSmsOrFallbackEmail function now receives creditCheckFn as a parameter. Updated test assertion to use expect.objectContaining() with creditCheckFn: expect.any(Function) to account for the new dependency injection parameter. Also removed teamId: undefined assertion as the key may be omitted entirely from the actual call. Co-Authored-By: [email protected] <[email protected]>

This commit completes the circular dependency fix by making creditCheckFn required throughout the call chain, eliminating the dynamic import fallback entirely. Changes: - Make creditCheckFn required in messageDispatcher functions (sendSmsOrFallbackEmail, scheduleSmsOrFallbackEmail) - Remove dynamic import fallback and warning log from messageDispatcher - Make creditCheckFn required in ScheduleTextReminderArgs (smsReminderManager) - Make creditCheckFn required in processWorkflowStep and ScheduleWorkflowRemindersArgs (reminderScheduler) - Add creditCheckFn to SendCancelledRemindersArgs and wire from handleCancelBooking The circular dependency is now fully broken - no more dynamic imports of CreditService from within the workflows package. All callers must explicitly provide creditCheckFn via dependency injection. Co-Authored-By: [email protected] <[email protected]>

…lows This commit fixes the CI type check error by making creditCheckFn required in WorkflowService.scheduleFormWorkflows. Previously, creditCheckFn was optional in scheduleFormWorkflows but required in scheduleWorkflowReminders, causing a type mismatch. Changes: - Make creditCheckFn required in scheduleFormWorkflows signature - Update WorkflowService.test.ts to pass mock creditCheckFn in all test cases - Add responseId and routedEventTypeId to test calls for completeness All callers of scheduleFormWorkflows already pass creditCheckFn, so this change is safe and completes the circular dependency fix. Co-Authored-By: [email protected] <[email protected]>

cubic-dev-ai

13 issues found across 24 files

Prompt for AI agents (all 13 issues)


Understand the root cause of the following 13 issues and fix them.


<file name="packages/features/handleMarkNoShow.ts">

<violation number="1" location="packages/features/handleMarkNoShow.ts:307">
Bind CreditService.hasAvailableCredits before passing it as creditCheckFn; the current unbound method loses its this context and will throw at runtime, preventing reminders from being sent.</violation>
</file>

<file name="packages/features/ee/workflows/api/scheduleWhatsappReminders.ts">

<violation number="1" location="packages/features/ee/workflows/api/scheduleWhatsappReminders.ts:130">
Bind CreditService.hasAvailableCredits before passing it to scheduleSmsOrFallbackEmail; otherwise the callback runs with the wrong `this` and crashes when it calls private helpers.</violation>
</file>

<file name="packages/sms/sms-manager.ts">

<violation number="1" location="packages/sms/sms-manager.ts:51">
Passing `creditService.hasAvailableCredits` unbound drops its `this` context, so the credit check throws at runtime. Bind or wrap the method before injecting it.</violation>
</file>

<file name="packages/features/bookings/lib/handleSeats/handleSeats.ts">

<violation number="1" location="packages/features/bookings/lib/handleSeats/handleSeats.ts:135">
Bind the CreditService instance when wiring creditCheckFn; passing the method unbound causes runtime failures when messageDispatcher calls it without a `this` context.</violation>
</file>

<file name="packages/app-store/routing-forms/lib/formSubmissionUtils.ts">

<violation number="1" location="packages/app-store/routing-forms/lib/formSubmissionUtils.ts:236">
Passing the instance method without binding drops its `this` context, so any workflow reminder credit check will throw. Bind the method before passing it.</violation>
</file>

<file name="packages/features/bookings/lib/handleBookingRequested.ts">

<violation number="1" location="packages/features/bookings/lib/handleBookingRequested.ts:123">
Bind the CreditService instance when passing `creditCheckFn`; otherwise `this` becomes undefined and credit checks throw once the limit-reached path executes.</violation>
</file>

<file name="packages/features/ee/workflows/api/scheduleSMSReminders.ts">

<violation number="1" location="packages/features/ee/workflows/api/scheduleSMSReminders.ts:199">
creditCheckFn is passed an unbound instance method, so inside messageDispatcher `this` becomes undefined and credit checks throw before reminders can be scheduled. Bind the method to the CreditService instance.</violation>
</file>

<file name="packages/trpc/server/routers/viewer/workflows/activateEventType.handler.ts">

<violation number="1" location="packages/trpc/server/routers/viewer/workflows/activateEventType.handler.ts:425">
`creditService.hasAvailableCredits` is passed unbound, so `this` is undefined when the reminder invokes `creditCheckFn`, crashing credit checks.</violation>
</file>

<file name="packages/features/ee/round-robin/roundRobinManualReassignment.ts">

<violation number="1" location="packages/features/ee/round-robin/roundRobinManualReassignment.ts:618">
`creditCheckFn` is passed an unbound instance method, so `hasAvailableCredits` will throw when accessed without its `this` context.</violation>
</file>

<file name="packages/features/bookings/lib/handleCancelBooking.ts">

<violation number="1" location="packages/features/bookings/lib/handleCancelBooking.ts:377">
Bind CreditService.hasAvailableCredits before passing it as creditCheckFn; otherwise the callback throws because it relies on this.</violation>
</file>

<file name="packages/features/bookings/lib/handleConfirmation.ts">

<violation number="1" location="packages/features/bookings/lib/handleConfirmation.ts:374">
Passing `creditService.hasAvailableCredits` directly loses the `this` binding, so workflow reminders will crash when `creditCheckFn` runs. Bind the method (e.g., `.bind(creditService)` or wrap in a lambda) before passing it down.</violation>
</file>

<file name="packages/features/bookings/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:2391">
Bind the hasAvailableCredits method before passing it into scheduleWorkflowsForNewBooking so the callback keeps the CreditService instance context.</violation>
</file>

<file name="packages/trpc/server/routers/viewer/workflows/util.ts">

<violation number="1" location="packages/trpc/server/routers/viewer/workflows/util.ts:847">
`creditService.hasAvailableCredits` is passed without binding, so the credit check throws when messageDispatcher calls it. Bind the method to the instance before injecting.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

packages/features/handleMarkNoShow.ts

packages/features/ee/workflows/api/scheduleWhatsappReminders.ts

packages/sms/sms-manager.ts

packages/features/bookings/lib/handleSeats/handleSeats.ts

packages/app-store/routing-forms/lib/formSubmissionUtils.ts

packages/features/ee/round-robin/roundRobinManualReassignment.ts

packages/features/bookings/lib/handleCancelBooking.ts

packages/features/bookings/lib/handleConfirmation.ts

packages/features/bookings/lib/service/RegularBookingService.ts

packages/trpc/server/routers/viewer/workflows/util.ts

hbjORbj · 2025-11-26T02:46:07Z

packages/features/ee/workflows/lib/reminders/messageDispatcher.ts

    t: TFunction;
    replyTo: string;
  };
+  creditCheckFn: CreditCheckFn;


hbjORbj · 2025-11-26T02:46:13Z

packages/features/ee/workflows/lib/reminders/messageDispatcher.ts

    replyTo: string;
    workflowStepId?: number;
  };
+  creditCheckFn: CreditCheckFn;


cubic-dev-ai

No issues found across 24 files

ThyMinimalDev · 2025-11-26T08:20:16Z

packages/features/handleMarkNoShow.ts

+const isRejected = <T>(x: PromiseSettledResult<T>): x is PromiseRejectedResult => x.status === "rejected";
+


is this used somewhere ?

ah no, will revert now

cubic-dev-ai

No issues found across 24 files

pull-request-size bot added the size/M label Nov 22, 2025

keithwillcode added core area: core, team members only foundation labels Nov 22, 2025

devin-ai-integration bot mentioned this pull request Nov 22, 2025

fix: break circular dependency in messageDispatcher via dependency injection #25342

Closed

10 tasks

vercel bot deployed to Preview – dev November 22, 2025 06:10 View deployment

pull-request-size bot removed the size/M label Nov 24, 2025

pull-request-size bot added the size/L label Nov 24, 2025

vercel bot deployed to Preview – dev November 24, 2025 08:26 View deployment

hbjORbj added 3 commits November 24, 2025 17:42

wip

ba7ae4c

wip

3db6d92

wip

7c96fb8

hbjORbj added the ready-for-e2e label Nov 24, 2025

vercel bot deployed to Preview – dev November 24, 2025 09:08 View deployment

hbjORbj added 2 commits November 25, 2025 12:07

revert

4f4cf37

revert

2821381

hbjORbj force-pushed the devin/messagedispatcher-circular-1763790475 branch from 2821381 to 4f4cf37 Compare November 25, 2025 03:14

vercel bot deployed to Preview – dev November 25, 2025 03:27 View deployment

devin-ai-integration bot and others added 2 commits November 25, 2025 04:05

vercel bot deployed to Preview – dev November 25, 2025 04:36 View deployment

vercel bot had a problem deploying to Preview – dev November 25, 2025 05:24 Failure

devin-ai-integration bot and others added 2 commits November 25, 2025 06:10

hbjORbj added 2 commits November 25, 2025 17:04

refactor

ef5ee6b

wip

6c0fc96

pull-request-size bot added size/L and removed size/XXL labels Nov 25, 2025

hbjORbj changed the title ~~fix: break circular dependency by making creditCheckFn required~~ fix: break circular dependency by passing creditCheckFn Nov 25, 2025

hbjORbj changed the title ~~fix: break circular dependency by passing creditCheckFn~~ fix: break circular dependency by passing creditCheckFn in messageDispatcher Nov 25, 2025

hbjORbj added ready-for-e2e and removed ready-for-e2e labels Nov 25, 2025

hbjORbj marked this pull request as ready for review November 25, 2025 08:44

hbjORbj requested a review from a team as a code owner November 25, 2025 08:44

cubic-dev-ai bot reviewed Nov 25, 2025

View reviewed changes

vercel bot had a problem deploying to Preview – dev November 25, 2025 09:04 Failure

hbjORbj marked this pull request as draft November 25, 2025 12:08

hbjORbj added 2 commits November 25, 2025 21:12

fix

d1e45cb

fix

e9227a5

vercel bot deployed to Preview – dev November 25, 2025 12:28 View deployment

Merge branch 'main' into devin/messagedispatcher-circular-1763790475

e45282c

vercel bot deployed to Preview – dev November 26, 2025 00:17 View deployment

hbjORbj commented Nov 26, 2025

View reviewed changes

hbjORbj marked this pull request as ready for review November 26, 2025 02:47

cubic-dev-ai bot reviewed Nov 26, 2025

View reviewed changes

ThyMinimalDev reviewed Nov 26, 2025

View reviewed changes

rm

e379c69

cubic-dev-ai bot reviewed Nov 26, 2025

View reviewed changes

vercel bot deployed to Preview – dev November 26, 2025 13:16 View deployment

hbjORbj requested a review from ThyMinimalDev November 27, 2025 07:09

ThyMinimalDev approved these changes Nov 27, 2025

View reviewed changes

hbjORbj merged commit d9cddd8 into main Nov 27, 2025
87 of 92 checks passed

hbjORbj deleted the devin/messagedispatcher-circular-1763790475 branch November 27, 2025 07:31

		const isRejected = <T>(x: PromiseSettledResult<T>): x is PromiseRejectedResult => x.status === "rejected";

fix: break circular dependency by passing creditCheckFn in messageDispatcher #25343

fix: break circular dependency by passing creditCheckFn in messageDispatcher #25343

Uh oh!

Conversation

hbjORbj commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Mandatory Tasks

Uh oh!

devin-ai-integration bot commented Nov 22, 2025

🤖 Devin AI Engineer

Uh oh!

vercel bot commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E results are ready!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hbjORbj Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

hbjORbj Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

ThyMinimalDev Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

hbjORbj Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

hbjORbj Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hbjORbj commented Nov 22, 2025 •

edited

Loading

vercel bot commented Nov 24, 2025 •

edited

Loading

github-actions bot commented Nov 24, 2025 •

edited

Loading