-
Notifications
You must be signed in to change notification settings - Fork 11.6k
feat: workflows v3 UI #22772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: workflows v3 UI #22772
Conversation
…ng, might have broken
WalkthroughThis PR refactors the workflows UI and related components: WorkflowDetailsPage now requires server-sourced WorkflowPermissions and removes internal router/delete-dialog logic; WorkflowStepContainer expands its props (external agentData, delete-dialog control, selection controls, onSaveWorkflow) and removes its internal agent query; the top-level workflow page replaces the Shell layout with a header-driven layout, adds inline-editable workflow name state/handlers, and centralizes permission computation; FormCard.label now accepts ReactNode and a new FormCardBody export was added; PhoneInput gains web styling props; AddVariablesDropdown receives responsive/layout changes; SkeletonLoaderEdit simplified; locale keys and small CSS/test mocks/ESLint directives were added or adjusted. Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Graphite Automations"Add platform team as reviewer" took an action on this PR • (07/28/25)1 reviewer was added to this PR based on Keith Williams's automation. |
|
This PR is being marked as stale due to inactivity. |
|
Closing since stale. We can reopen when work continues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/web/public/static/locales/en/common.json (1)
849-850: Propagate new key to all locales (dup of earlier note).
hi_how_are_you_doingis added here; ensure it exists across other locale files to prevent English fallback.Run to see where the key is missing and where it’s used:
#!/bin/bash # Where the key is referenced rg -n --no-ignore -S "\bhi_how_are_you_doing\b" . # Which locale files define it rg -n --no-ignore -S '"hi_how_are_you_doing"' apps/web/public/static/locales | awk -F/ '{print $(NF-1)}' | sort | uniq -cpackages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
378-381: Verification status effects can go stale when lists change without length changesThe current dependency on array lengths can miss updates when array contents change without length changes (e.g., replacing one phone number with another).
-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
579-579: Checkbox onChange uses wrong propertyThe code uses
e.target.valueinstead ofe.target.checked, which will always be truthy ("on"), causing the checkbox to behave as always checked.- if (e.target.value) { + if (e.target.checked) {
701-704: Use empty strings instead of null for controlled text inputsSetting form values to
nullcan cause controlled inputs to flip to uncontrolled, leading to validation issues and inconsistent behavior.- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, "");- form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");Also applies to: 712-715
🧹 Nitpick comments (1)
apps/web/public/static/locales/en/common.json (1)
3577-3577: Sentence casing for UI consistency.Value reads “Set up Agent”. Elsewhere strings use sentence case (“Set up later”, “Set up your organization”). Recommend “Set up agent”.
Apply:
- "set_up_agent": "Set up Agent", + "set_up_agent": "Set up agent",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/public/static/locales/en/common.json(3 hunks)packages/features/ee/workflows/components/WorkflowStepContainer.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (7)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)shouldScheduleEmailReminder(17-19)packages/ui/components/badge/index.ts (2)
InfoBadge(5-5)Badge(1-1)packages/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(3-6)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(55-78)packages/ui/components/editor/Editor.tsx (1)
Editor(59-119)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (11)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (11)
83-93: Good expansion of the props interfaceThe new props provide clear external control over the component's state, enabling better integration with parent workflows components. The prop names are descriptive and the types are appropriate.
436-440: LGTM! Clean helper functionThe
hasCalAIActionfunction provides a clear way to check if any workflow steps contain Cal.AI actions, which is used appropriately for conditional rendering.
494-536: Well-structured trigger selection UIThe restructured trigger section with proper labeling, dynamic time defaults based on trigger type, and controlled form updates is clean and user-friendly. The conditional rendering logic appropriately handles different trigger scenarios.
538-591: Comprehensive event/team selection implementationThe multi-select checkbox integration with proper validation, conditional labeling for organizations vs. event types, and the "apply to all" functionality is well implemented. The restriction logic for Cal.AI actions is also correctly applied.
739-786: Excellent Cal.AI agent setup flowThe conditional rendering for agent setup with proper state management, error handling, and user-friendly messaging provides a smooth user experience. The automatic agent creation flow with workflow saving is well orchestrated.
788-857: Well-designed agent status displayThe agent data section clearly shows phone number status, provides appropriate actions (test/connect), and uses proper loading states. The conditional rendering based on phone number availability is logical.
[approve_comment_end]
903-998: Comprehensive phone verification flowThe phone number input with verification code workflow is well-implemented with proper validation, error handling, and user feedback. The UI appropriately handles different states (verified, unverified, disabled).
1019-1113: Solid email verification implementationThe email verification flow mirrors the phone verification pattern with appropriate adaptations. The validation and state management are consistent with the phone number implementation.
1114-1188: Template selection with dynamic updatesThe template selection logic properly updates both reminder body and email subject based on template type and action. The conditional rendering for Cal.AI actions is correctly implemented.
1189-1293: Rich text editing with proper validationThe email subject and body editing sections with the AddVariablesDropdown integration and Editor component provide a comprehensive authoring experience. The conditional rendering and validation error display are well handled.
1541-1618: Thorough delete confirmation dialogThe delete step dialog properly handles the complexity of Cal.AI agents with associated phone numbers, providing clear warnings about subscription cancellations and phone number deletions. The optional chaining for
setIsDeleteStepDialogOpenis appropriate for external control.
| "cancel_booking_acknowledge_no_show_fee": "I acknowledge that by cancelling the booking within {{timeValue}} {{timeUnit}} of the start time I will be charged the no show fee of {{amount, currency}}", | ||
| "contact_organizer": "If you have any questions, please contact the organizer.", | ||
| "testing_sms_workflow_info_message": "When testing this workflow, be aware that SMS need to be scheduled at least 15 minutes in advance", | ||
| "ADD_NEW_STRINGS_ABOVE_THIS_LINE_TO_PREVENT_MERGE_CONFLICTS": "↑↑↑↑↑↑↑↑↑↑↑↑↑ Add your new strings above here ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix grammar in user-facing copy; consider parametrizing minutes.
“SMS need to be scheduled” → “SMS messages must be scheduled”. Immediate fix:
- "testing_sms_workflow_info_message": "When testing this workflow, be aware that SMS need to be scheduled at least 15 minutes in advance"
+ "testing_sms_workflow_info_message": "When testing this workflow, SMS messages must be scheduled at least 15 minutes in advance"Optional: make minutes configurable/localizable, e.g.
"testing_sms_workflow_info_message": "When testing this workflow, SMS messages must be scheduled at least {{minutes}} $t(minute_timeUnit) in advance" and pass minutes: 15.
Confirm old key was removed everywhere and new key is referenced:
🏁 Script executed:
#!/bin/bash
# Old key should have no references
rg -n --no-ignore -S "\btesting_workflow_info_message\b" .
# New key should be referenced in code
rg -n --no-ignore -S "\btesting_sms_workflow_info_message\b" packages/ apps/Length of output: 7890
Fix grammar, sync i18n keys across locales, and parametrize minutes
Search shows the old key testing_workflow_info_message still exists in i18n.lock and many non‑en locale files while the new key testing_sms_workflow_info_message is present only in en and referenced by the code — reconcile these now.
- Immediate (English) fix — apply diff:
- "testing_sms_workflow_info_message": "When testing this workflow, be aware that SMS need to be scheduled at least 15 minutes in advance"
+ "testing_sms_workflow_info_message": "When testing this workflow, SMS messages must be scheduled at least 15 minutes in advance"- Mandatory: remove or replace testing_workflow_info_message across translations and update i18n.lock so all locales use the same key (or keep the old key and update the English string consistently).
- Locations found: i18n.lock:2321 and apps/web/public/static/locales/*/common.json (many languages, ~line 2324).
- Confirmed good: apps/web/public/static/locales/en/common.json:3631 contains the new string and packages/features/ee/workflows/components/WorkflowStepContainer.tsx:600 references t("testing_sms_workflow_info_message").
- Verify after fixes:
- rg -n --no-ignore -S "\btesting_workflow_info_message\b" i18n.lock apps/web/public/static/locales
- rg -n --no-ignore -S "\btesting_sms_workflow_info_message\b" packages/ apps/web/public/static/locales
- Optional (recommended): parametrize minutes for localization, e.g.
"testing_sms_workflow_info_message": "When testing this workflow, SMS messages must be scheduled at least {{minutes}} $t(minute_timeUnit) in advance" with minutes: 15.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "ADD_NEW_STRINGS_ABOVE_THIS_LINE_TO_PREVENT_MERGE_CONFLICTS": "↑↑↑↑↑↑↑↑↑↑↑↑↑ Add your new strings above here ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑" | |
| "testing_sms_workflow_info_message": "When testing this workflow, SMS messages must be scheduled at least 15 minutes in advance", | |
| "ADD_NEW_STRINGS_ABOVE_THIS_LINE_TO_PREVENT_MERGE_CONFLICTS": "↑↑↑↑↑↑↑↑↑↑↑↑↑ Add your new strings above here ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑" |
🤖 Prompt for AI Agents
In apps/web/public/static/locales/en/common.json around line 3632, replace the
English-only ad hoc key with the normalized key
testing_sms_workflow_info_message and parametrize the minutes (e.g. "When
testing this workflow, SMS messages must be scheduled at least {{minutes}}
$t(minute_timeUnit) in advance") plus add a minutes default (15) or ensure
callers pass it; then in i18n.lock and every
apps/web/public/static/locales/*/common.json remove the old
testing_workflow_info_message key and add or rename the
testing_sms_workflow_info_message key so all locales and the lock file use the
same key, update corresponding translated strings (or mark them for
translators), and run the provided ripgrep checks (rg -n --no-ignore -S
"\btesting_workflow_info_message\b" i18n.lock apps/web/public/static/locales and
rg -n --no-ignore -S "\btesting_sms_workflow_info_message\b" packages/
apps/web/public/static/locales) to verify no stale references remain.
| <div> | ||
| {!!timeSectionText && ( | ||
| <div className="mb-3 mt-3"> | ||
| <Label className="text-default mb-0">{timeSectionText}</Label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
404-407: Verification toast severity is always “success” even for wrong codesUse error severity when the code is invalid.
- showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error");
442-445: Email verify sets verified=true unconditionally and uses success severity for failuresRespect
isVerifiedand set toast severity accordingly.- showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); - setEmailVerified(true); + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); + setEmailVerified(isVerified);
♻️ Duplicate comments (5)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (5)
378-381: Effects can go stale — depend on arrays, not just their lengthLength-only deps miss content changes with equal length.
-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
49-51: Broken Option type import — import from MultiSelectCheckboxesThe
MultiSelectCheckboxesOptionTypealias does not exist on the form barrel; this will fail type-checking. Import the actualOptiontype from the checkbox module.-import { MultiSelectCheckbox } from "@calcom/ui/components/form"; -import type { MultiSelectCheckboxesOptionType as Option } from "@calcom/ui/components/form"; +import { MultiSelectCheckbox } from "@calcom/ui/components/form"; +import type { Option } from "@calcom/ui/components/form/checkbox/MultiSelectCheckboxes";
583-591: Checkbox onChange must use e.target.checked (not value) and propagate booleanUsing
e.target.valuealways yields "on", so Select All stays checked. Also passcheckedto RHF.- onChange={(e) => { - onChange(e); - if (e.target.value) { - setSelectedOptions(allOptions); - form.setValue("activeOn", allOptions); - } - }} + onChange={(e) => { + const checked = e.target.checked; + onChange(checked); + if (checked) { + setSelectedOptions(allOptions); + form.setValue("activeOn", allOptions); + } else { + const prev = selectedOptions ?? []; + setSelectedOptions(prev); + form.setValue("activeOn", prev); + } + }}
700-703: Avoid flipping controlled inputs to uncontrolled — use empty strings instead of nullSetting text fields to null can cause controlled/uncontrolled warnings and validation quirks.
- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, "");
711-714: Same: keep sendTo controlledUse empty string for
sendTo.- form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");
🧹 Nitpick comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
1169-1171: Don’t mix controlled and uncontrolled props on SelectUse either value or defaultValue, not both. Keep it controlled.
- defaultValue={selectedTemplate} - value={selectedTemplate} + value={selectedTemplate}
103-105: Remove non-null assertion + eslint-disable by narrowing typeTiny clean-up; keeps types happy without disabling lint.
- // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return t(triggerMap[trigger]!); + return t(triggerMap[trigger] as string);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (9)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/ee/workflows/lib/actionHelperFunctions.ts (4)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)packages/ui/components/badge/index.ts (2)
InfoBadge(5-5)Badge(1-1)packages/ui/components/form/index.ts (1)
MultiSelectCheckbox(1-1)packages/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(3-6)packages/ui/components/dropdown/Dropdown.tsx (5)
Dropdown(12-12)DropdownMenuTrigger(15-26)DropdownMenuContent(34-51)DropdownMenuItem(63-71)DropdownItem(161-181)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(55-78)packages/ui/components/editor/Editor.tsx (1)
Editor(59-119)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
404-406: Verification toasts and state updates: treat wrong code as error; don’t mark verified on failure.Currently both branches show “success”, and email path sets
emailVerifiedto true even when invalid.// Phone verification - showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); setNumberVerified(isVerified); // Email verification - showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); - setEmailVerified(true); + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); + setEmailVerified(isVerified);Also applies to: 441-444
♻️ Duplicate comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
702-705: Avoid flipping controlled inputs to uncontrolled: use empty strings instead of null.Setting text fields to
nullcan cause React controlled/uncontrolled warnings.- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, ""); - form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");Also applies to: 713-715
378-381: Effects can go stale by depending on “.length”. Depend on arrays themselves.Length-only deps miss content changes with same length and require eslint disables.
-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
49-51: Fix incorrect Option type import (breaks type-checking).
MultiSelectCheckboxesOptionTypeisn’t exported from the form barrel. The correct type isOptionfrom the checkbox module.-import { MultiSelectCheckbox } from "@calcom/ui/components/form"; -import type { MultiSelectCheckboxesOptionType as Option } from "@calcom/ui/components/form"; +import { MultiSelectCheckbox } from "@calcom/ui/components/form"; +import type { Option } from "@calcom/ui/components/form/checkbox/MultiSelectCheckboxes";
585-593: Checkbox onChange must use e.target.checked (current logic always truthy).Using
e.target.valueyields"on", so “Select all” stays checked and overwrites selections.- onChange={(e) => { - onChange(e); - if (e.target.value) { - setSelectedOptions(allOptions); - form.setValue("activeOn", allOptions); - } - }} + onChange={(e) => { + const checked = e.target.checked; + onChange(checked); + if (checked) { + setSelectedOptions(allOptions); + form.setValue("activeOn", allOptions); + } else { + // optional: restore previous selection + form.setValue("activeOn", selectedOptions ?? []); + setSelectedOptions(selectedOptions ?? []); + } + }}
🧹 Nitpick comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
634-641: Name and memoize active phone numbers.
arePhoneNumbersActiveis an array (misleading name) andgetActivePhoneNumbers(...)is repeated. Rename and memoize.- const arePhoneNumbersActive = getActivePhoneNumbers( + const activePhoneNumbers = getActivePhoneNumbers( agentData?.outboundPhoneNumbers?.map((phone) => ({ ...phone, subscriptionStatus: phone.subscriptionStatus ?? undefined, })) );Also replace downstream usages (
arePhoneNumbersActive) withactivePhoneNumbers. Optionally:const activePhoneNumbers = useMemo( () => getActivePhoneNumbers( agentData?.outboundPhoneNumbers?.map((p) => ({ ...p, subscriptionStatus: p.subscriptionStatus ?? undefined })) ), [agentData?.outboundPhoneNumbers] );
1456-1463: Don’t rely on non-null assertion for agent ID in update.Guard instead of
!to avoid runtime surprises.- onUpdate={(data) => { - updateAgentMutation.mutate({ - //eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: stepAgentId!, + onUpdate={(data) => { + if (!stepAgentId) return; + updateAgentMutation.mutate({ + id: stepAgentId, teamId: teamId, generalPrompt: data.generalPrompt, beginMessage: data.beginMessage, generalTools: data.generalTools, }); }}
1141-1169: Avoid double-updating form state in template Select.You call both
field.onChange(val.value)andform.setValue(...)for the same field.- field.onChange(val.value); - form.setValue(`steps.${step.stepNumber - 1}.template`, val.value); + field.onChange(val.value);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (7)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)shouldScheduleEmailReminder(17-19)packages/ui/components/badge/index.ts (2)
InfoBadge(5-5)Badge(1-1)packages/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(3-6)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(55-78)packages/ui/components/editor/Editor.tsx (1)
Editor(59-119)
🔇 Additional comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
571-573: Confirm whether MultiSelectCheckbox.localizes countText internally.If it doesn’t translate keys itself, wrap with
t().If it needs text, use:
- countText={isOrganization ? "count_team" : "nr_event_type"} + countText={isOrganization ? t("count_team") : t("nr_event_type")}
721-734: Cal.AI action filtering respects org/select-all constraints.Hiding Cal.AI when
selectAllis enabled or in org mode matches the established product rule.
383-392: Variable insertion pattern matches codebase convention.Uppercasing and replacing spaces with underscores is consistent with workflows/agent components.
1542-1543: Ensure delete-step Dialog controlled props are always provided.
open={isDeleteStepDialogOpen}andonOpenChange={setIsDeleteStepDialogOpen}are optional in props. If absent, Dialog can flip controlled/uncontrolled.If props may be undefined, coerce and guard:
- <Dialog open={isDeleteStepDialogOpen} onOpenChange={setIsDeleteStepDialogOpen}> + <Dialog open={!!isDeleteStepDialogOpen} onOpenChange={setIsDeleteStepDialogOpen}>Also applies to: 1590-1614
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
404-407: Wrong toast severity on phone verification failure.Failure still shows “success”. Use error variant when not verified.
- showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error");
442-445: Email verification: always sets verified=true and success toast on failure.Respect the result and use error variant when code is wrong.
- onSuccess: (isVerified) => { - showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); - setEmailVerified(true); + onSuccess: (isVerified) => { + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); + setEmailVerified(isVerified);Also applies to: 456-459
♻️ Duplicate comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
378-381: Effects depend on array lengths; can go stale.Depend on the arrays, not just
.length, and drop the eslint disables.-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
702-705: Avoid flipping controlled inputs to uncontrolled (use "" instead of null).Using
nullfor text fields can trigger React controlled/uncontrolled warnings and validation oddities.- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, "");- form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");Also applies to: 713-715
50-50: Fix incorrect Option type import (breaks build).
MultiSelectCheckboxesOptionTypeisn’t exported from the barrel. Import the actualOptiontype from MultiSelectCheckboxes.-import type { MultiSelectCheckboxesOptionType as Option } from "@calcom/ui/components/form"; +import type { Option } from "@calcom/ui/components/form/checkbox/MultiSelectCheckboxes";
581-593: Checkbox onChange uses e.target.value instead of checked (logic bug).This sets “select all” even when unchecking, and also passes the event object to RHF instead of a boolean.
- onChange={(e) => { - onChange(e); - if (e.target.value) { - setSelectedOptions(allOptions); - form.setValue("activeOn", allOptions); - } - }} + onChange={(e) => { + const checked = e.target.checked; + onChange(checked); + if (checked) { + setSelectedOptions(allOptions); + form.setValue("activeOn", allOptions); + } else { + const prev = selectedOptions ?? []; + setSelectedOptions(prev); + form.setValue("activeOn", prev); + } + }}
🧹 Nitpick comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
524-527: Clear errors when unregistering time fields.Prevents lingering validation errors after trigger changes.
setTimeSectionText(null); form.unregister("time"); form.unregister("timeUnit"); + form.clearErrors(["time", "timeUnit"]);
1078-1086: Optional: separate verification codes for phone/email.Sharing one
verificationCodestate between the two flows can bleed input if both UIs are visible when toggling actions.
- Introduce
phoneVerificationCodeandemailVerificationCodelocal states and bind them in their respective sections.Also applies to: 963-971
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (6)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)shouldScheduleEmailReminder(17-19)packages/ui/components/badge/index.ts (2)
InfoBadge(5-5)Badge(1-1)packages/ui/components/form/index.ts (3)
MultiSelectCheckbox(1-1)EmailField(5-5)TextArea(11-11)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(55-78)
🔇 Additional comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
721-729: Good: CAL.AI action hidden for org workflows and when select-all is on.Matches the product decision captured in prior learnings.
383-392: Good: variable insertion pattern is consistent with existing components.Uses
{${variable.toUpperCase().replace(/ /g, "_")}}as established.
…mprove-workflows-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
444-447: Email verification state set to true even on failure; also wrong toast variant for failure.
- Set emailVerified to the actual result.
- Use “error” toast when code is wrong (applies to phone and email).
// Phone - onSuccess: async (isVerified) => { - showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + onSuccess: async (isVerified) => { + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); setNumberVerified(isVerified);// Email - onSuccess: (isVerified) => { - showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); - setEmailVerified(true); + onSuccess: (isVerified) => { + showToast(isVerified ? t("verified_successfully") : t("wrong_code"), isVerified ? "success" : "error"); + setEmailVerified(isVerified);Also applies to: 406-408
♻️ Duplicate comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
380-383: Effects can go stale; depend on arrays, remove lint disables.Length-only deps miss content changes. Depend on the arrays themselves.
-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
49-51: Fix incorrect Option type import (breaks type-checking).
Optionisn’t exported from the form barrel andMultiSelectCheckboxesOptionTypedoesn’t exist. Import the real type from MultiSelectCheckboxes.-import { MultiSelectCheckbox } from "@calcom/ui/components/form"; -import type { MultiSelectCheckboxesOptionType as Option } from "@calcom/ui/components/form"; +import { MultiSelectCheckbox } from "@calcom/ui/components/form"; +import type { Option } from "@calcom/ui/components/form/checkbox/MultiSelectCheckboxes";
581-596: Checkbox uses e.target.value; should use e.target.checked (functional bug).This keeps “Select all” stuck on. Also pass boolean to Controller’s onChange and restore selections when unchecked.
- render={({ field: { value, onChange } }) => ( + render={({ field: { value, onChange } }) => ( <CheckboxField description={isOrganization ? t("apply_to_all_teams") : t("apply_to_all_event_types")} disabled={props.readOnly} descriptionClassName="ml-0" - onChange={(e) => { - onChange(e); - if (e.target.value) { - setSelectedOptions(allOptions); - form.setValue("activeOn", allOptions); - } - }} + onChange={(e) => { + const checked = e.target.checked; + onChange(checked); + if (checked) { + setSelectedOptions(allOptions); + form.setValue("activeOn", allOptions); + } else { + const prev = selectedOptions ?? []; + setSelectedOptions(prev); + form.setValue("activeOn", prev); + } + }} checked={value} /> )}
704-707: Avoid null for controlled inputs (prevents uncontrolled flip).Use empty strings for text fields; keep non-text IDs as null if needed.
- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, "");- form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");Also applies to: 715-717
🧹 Nitpick comments (6)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (6)
603-606: Use design tokens instead of hardcoded gray.Replace
text-gray-500with semantic token used elsewhere.- {!!timeSectionText && ( - <div className="mt-1 flex text-gray-500"> + {!!timeSectionText && ( + <div className="mt-1 flex text-subtle">
104-106: Remove non-null assertion and lint disable by narrowing.
triggerMap[trigger]is already checked. Cast to string and drop the disable.- // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return t(triggerMap[trigger]!); + return t(triggerMap[trigger] as string);
917-931: Guard “Send code” button when phone is empty.Prevents accidental requests with empty numbers.
- <Button + <Button color="secondary" - disabled={numberVerified || props.readOnly || false} + disabled={ + numberVerified || + props.readOnly || + !form.getValues(`steps.${step.stepNumber - 1}.sendTo`) + }
1442-1449: Drop non-null assertion on stepAgentId in update handler.Avoid
!by guarding inside the callback.- onUpdate={(data) => { - updateAgentMutation.mutate({ - //eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: stepAgentId!, - teamId: teamId, - generalPrompt: data.generalPrompt, - beginMessage: data.beginMessage, - generalTools: data.generalTools, - }); - }} + onUpdate={(data) => { + if (!stepAgentId) return; + updateAgentMutation.mutate({ + id: stepAgentId, + teamId, + generalPrompt: data.generalPrompt, + beginMessage: data.beginMessage, + generalTools: data.generalTools, + }); + }}
908-913: Normalize phone before verification check (optional).Whitespace-only compare can misclassify; consider E.164 normalization before comparing.
- const isAlreadyVerified = !!verifiedNumbers - ?.concat([]) - .find((number) => number.replace(/\s/g, "") === val?.replace(/\s/g, "")); + const normalize = (n?: string) => (n ?? "").replace(/\D/g, ""); + const isAlreadyVerified = verifiedNumbers + ?.some((n) => normalize(n) === normalize(val));
140-140: Prefer named export (optional).Consider exporting
WorkflowStepContaineras a named export for consistency and tree-shaking. Exemption is reasonable if this is the module’s primary component.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
apps/web/public/static/locales/en/common.json(3 hunks)packages/features/ee/workflows/components/WorkflowStepContainer.tsx(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:07:30.058Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/workflows/update.handler.ts:738-763
Timestamp: 2025-08-15T00:07:30.058Z
Learning: In calcom/cal.com workflows, Cal AI phone call actions (CAL_AI_PHONE_CALL) intentionally always require the phone number field when the action is present, unlike SMS/WhatsApp actions which respect the step.numberRequired flag. This is the intended behavior per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (10)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/calAIPhone/providers/retellAI/types.ts (1)
RetellAgentWithDetails(104-132)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)shouldScheduleEmailReminder(17-19)packages/ui/components/badge/index.ts (2)
InfoBadge(5-5)Badge(1-1)packages/ui/components/form/index.ts (3)
MultiSelectCheckbox(1-1)EmailField(5-5)TextArea(11-11)packages/lib/constants.ts (1)
SENDER_ID(30-30)packages/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(3-6)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(55-78)packages/ui/components/editor/Editor.tsx (1)
Editor(59-119)
🔇 Additional comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
722-737: Intentionally hiding Cal.AI in org/select-all contexts — LGTM.Filtering out Cal.AI actions when
selectAllorisOrganizationis true matches the established behavior (per learnings).
385-394: Variable insertion pattern is consistent — LGTM.Uppercasing and space-to-underscore tokenization matches the established approach across the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
409-411: Wrong toast variant when verification fails“Wrong code” shows as success. Use error for negative outcome.
Apply:
- showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + showToast( + isVerified ? t("verified_successfully") : t("wrong_code"), + isVerified ? "success" : "error" + );
447-449: Same toast variant bug for email verificationMirror the fix for email flow.
Apply:
- showToast(isVerified ? t("verified_successfully") : t("wrong_code"), "success"); + showToast( + isVerified ? t("verified_successfully") : t("wrong_code"), + isVerified ? "success" : "error" + );
♻️ Duplicate comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
51-51: Broken type import: Option is not exported from the form barrelImport the correct type from MultiSelectCheckboxes; current symbol doesn’t exist and will break type-checking.
Apply:
-import type { MultiSelectCheckboxesOptionType as Option } from "@calcom/ui/components/form"; +import type { Option } from "@calcom/ui/components/form/checkbox/MultiSelectCheckboxes";
583-600: Fix checkbox handler: use e.target.checked (not value) and restore selections on uncheckUsing value yields "on" and always truthy; also pass the boolean to RHF.
Apply:
- <Controller + <Controller name="selectAll" - render={({ field: { value, onChange } }) => ( + control={form.control} + render={({ field: { value, onChange } }) => ( <CheckboxField description={isOrganization ? t("apply_to_all_teams") : t("apply_to_all_event_types")} disabled={props.readOnly} descriptionClassName="ml-0" - onChange={(e) => { - onChange(e); - if (e.target.value) { + onChange={(e) => { + const checked = e.target.checked; + onChange(checked); + if (checked) { setSelectedOptions(allOptions); form.setValue("activeOn", allOptions); + } else { + form.setValue("activeOn", selectedOptions ?? []); } }} checked={value} /> )} />
383-386: Effects can go stale by depending on .length onlyDepend on the arrays so changes without length deltas update verification state; remove eslint disables.
Apply:
-// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setNumberVerified(getNumberVerificationStatus()), [verifiedNumbers.length]); -// eslint-disable-next-line react-hooks/exhaustive-deps -useEffect(() => setEmailVerified(getEmailVerificationStatus()), [verifiedEmails.length]); +useEffect(() => { + setNumberVerified(getNumberVerificationStatus()); +}, [verifiedNumbers]); +useEffect(() => { + setEmailVerified(getEmailVerificationStatus()); +}, [verifiedEmails]);
707-710: Avoid flipping controlled inputs to uncontrolled: don’t set string fields to nullUse empty strings for emailSubject, reminderBody, sender, and sendTo.
Apply:
- form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, null); - form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, null); - form.setValue(`steps.${step.stepNumber - 1}.sender`, null); + form.setValue(`steps.${step.stepNumber - 1}.emailSubject`, ""); + form.setValue(`steps.${step.stepNumber - 1}.reminderBody`, ""); + form.setValue(`steps.${step.stepNumber - 1}.sender`, "");- form.setValue(`steps.${step.stepNumber - 1}.sendTo`, null); + form.setValue(`steps.${step.stepNumber - 1}.sendTo`, "");Also applies to: 718-721
🧹 Nitpick comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
1189-1190: Redundant controlled props: don’t pass both value and defaultValueSelect is controlled via value; drop defaultValue to avoid ambiguity.
Apply:
- defaultValue={selectedTemplate} - value={selectedTemplate} + value={selectedTemplate}
949-968: Minor: remove noop|| falsein disabled expressions
|| falsedoesn’t change logic.Apply:
- disabled={numberVerified || props.readOnly || false} + disabled={numberVerified || props.readOnly}- disabled={emailVerified || props.readOnly || false} + disabled={emailVerified || props.readOnly}Also applies to: 1062-1077
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, variable insertion in both WorkflowStepContainer.tsx (addVariableEmailSubject) and AgentConfigurationSheet.tsx (addVariableToGeneralPrompt) follows the identical pattern: `{${variable.toUpperCase().replace(/ /g, "_")}}`. This is the established, consistent approach across the codebase for transforming variable labels into tokens, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
packages/ui/components/form/checkbox/MultiSelectCheckboxes.tsx (1)
Option(18-21)packages/features/calAIPhone/providers/retellAI/types.ts (1)
RetellAgentWithDetails(104-132)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
isCalAIAction(37-39)getTemplateBodyForAction(118-143)isSMSAction(25-27)isWhatsappAction(29-31)shouldScheduleEmailReminder(17-19)packages/ui/components/editor/plugins/AddVariablesDropdown.tsx (1)
AddVariablesDropdown(19-115)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
726-735: Cal.AI action filtering for organizations/select-all is correctThe filter aligns with the established design to hide Cal.AI for org workflows and when selectAll is active. Good catch.


before

after
Screen.Recording.2025-09-05.at.7.02.46.AM.mov