feat: Enhance automation workflows with new context providers and input handling - #8680
Conversation
…ut handling - Added `WorkflowEditScopeProvider` and `WorkflowNodeProvider` for better state management in workflow editing. - Introduced local state management for query parameters in `AutomationProvider` to support scoped providers. - Implemented new GraphQL mutations and queries for managing workflow templates. - Enhanced node event handling to support workflow-specific actions and prevent sidebar toggling. - Updated node generation logic to accommodate new workflow input nodes and maintain proper connections. - Added utility functions for rewriting action inputs and normalizing workflows. - Introduced context for handling automation variable insertion in placeholder inputs. - Improved test coverage for workflow input handling and action rewriting. - Fixed minor UI issues in trigger condition summaries.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Wlkr123, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR adds automation workflow templates, nested workflow execution, workflow input normalization, builder editing and template surfaces, AI agent tool calling, output-variable handling, payment URL utilities, and POS automation integrations. ChangesAutomation workflows and integrations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx (1)
66-102: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not interpolate template labels into
innerHTML.Template names can contain HTML, so dragging a maliciously named template can create executable markup. Build the ghost with DOM nodes and assign the label through
textContent; avoidinnerHTMLfor user-controlled values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx` around lines 66 - 102, Replace the innerHTML-based ghost construction in the NodeLibraryRow drag flow with DOM node creation, keeping the existing layout and styles while assigning the user-controlled label through textContent. Ensure nodeType and iconMarkup are not interpolated into an HTML string either; use safe DOM APIs for all dynamic values and append the resulting nodes to ghost.
🟠 Major comments (24)
frontend/core-ui/src/modules/automations/utils/workflowInputs.ts-143-149 (1)
143-149: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck every action reference in a binding.
String.matchinspects only the first placeholder. For{{ actions.valid.id }}-{{ actions.deleted.id }}, this returnsfalseand misses the dangling second reference. Iterate withmatchAlland return true if any referenced ID is absent.Proposed fix
export const isDanglingBinding = ( expression: string, existingActionIds: Set<string>, ) => { - const actionRef = expression.match(/\{\{\s*actions\.([^.\s}]+)/); - return !!actionRef && !existingActionIds.has(actionRef[1]); + const actionRefs = expression.matchAll(/\{\{\s*actions\.([^.\s}]+)/g); + return Array.from(actionRefs).some( + (actionRef) => !existingActionIds.has(actionRef[1]), + ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/utils/workflowInputs.ts` around lines 143 - 149, Update isDanglingBinding to inspect every actions reference in the expression rather than only the first match. Iterate over all matches using matchAll and return true when any referenced action ID is missing from existingActionIds; otherwise preserve the false result.frontend/core-ui/src/modules/automations/utils/workflowInputs.ts-221-245 (1)
221-245: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not finalize migration when legacy members are missing.
When any
memberActionIdsentry is absent fromremainingActions, this silently drops the legacy metadata and can setentryActionIdto an action absent from the snapshot. Validate that every ID resolved before removing members or preserve the legacy workflow and report the invalid data.Proposed guard
const members = remainingActions.filter((action) => memberSet.has(action.id), ); + + if (members.length !== memberSet.size) { + return workflow; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/utils/workflowInputs.ts` around lines 221 - 245, The workflow migration around memberSet and remapConnectionRefs must not finalize when any memberActionIds entry is missing from remainingActions. Validate that every requested member ID resolves before removing legacy metadata or changing entryActionId; if validation fails, preserve the legacy workflow and report the invalid data, otherwise retain the existing migration behavior.frontend/core-ui/src/modules/automations/utils/workflowInputs.ts-14-17 (1)
14-17: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftReplace the
anyworkflow shapes with explicit types.
mapConfigStrings,normalizeWorkflowInputs, andnormalizeAutomationWorkflowsall bypass TypeScript here, andworkflowInputs.spec.tsrelies onas anyto cover it up. Type the workflow/config snapshot shapes and use typed fixtures instead of casts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/utils/workflowInputs.ts` around lines 14 - 17, The workflow input utilities use untyped workflow/config shapes and the tests hide type errors with any casts. In frontend/core-ui/src/modules/automations/utils/workflowInputs.ts:14-17, 120-123, and 202-210, define explicit types for configuration strings, workflow inputs, and automation workflow snapshots, then apply them to mapConfigStrings, normalizeWorkflowInputs, and normalizeAutomationWorkflows without any. In frontend/core-ui/src/modules/automations/utils/workflowInputs.spec.ts:31-157, replace as any fixtures and assertions with values conforming to those shared types.Source: Coding guidelines
backend/core-api/src/modules/automations/graphql/resolvers/queries.ts-450-455 (1)
450-455: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape user input before constructing regular expressions.
The
searchValueparameter is injected directly into aRegExpconstructor without escaping. This can crash the query with a syntax error (e.g., if the user enters[) and opens the door to Regular Expression Denial of Service (ReDoS) attacks.🛡️ Proposed fix to escape regular expression characters
if (searchValue) { + const escapedSearchValue = searchValue.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&'); filter.$or = [ - { name: new RegExp(`.*${searchValue}.*`, 'i') }, - { description: new RegExp(`.*${searchValue}.*`, 'i') }, + { name: new RegExp(`.*${escapedSearchValue}.*`, 'i') }, + { description: new RegExp(`.*${escapedSearchValue}.*`, 'i') }, ]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/core-api/src/modules/automations/graphql/resolvers/queries.ts` around lines 450 - 455, Escape searchValue before interpolating it into the RegExp patterns in the query resolver, using the project’s existing regex-escaping utility if available. Apply the escaped value to both name and description filters while preserving case-insensitive substring matching.backend/core-api/src/modules/automations/db/definitions/automationWorkflowTemplate.ts-4-16 (1)
4-16: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove
schemaWrapperusage.As per coding guidelines for backend schemas, new schemas must be defined directly using
new Schema(...)with explicit fields rather than usingschemaWrapper. Please removeschemaWrapperand explicitly define any necessary fields (likeprocessId) that were previously injected by the wrapper.♻️ Proposed fix
-export const automationWorkflowTemplateSchema = schemaWrapper( - new Schema({ +export const automationWorkflowTemplateSchema = new Schema({ _id: mongooseStringRandomId, + processId: { type: String, optional: true }, name: { type: String, label: 'Template name', required: true }, description: { type: String, label: 'Template description' }, entryActionId: { type: String, label: 'Entry action ID' }, actions: { type: [Object], label: 'Member actions snapshot' }, inputs: { type: Object, label: 'Input default bindings' }, createdBy: { type: String, label: 'Created by user ID' }, createdAt: { type: Date, label: 'Created at', default: new Date() }, updatedAt: { type: Date, label: 'Updated at', default: new Date() }, - }), -); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/core-api/src/modules/automations/db/definitions/automationWorkflowTemplate.ts` around lines 4 - 16, Update automationWorkflowTemplateSchema to instantiate Schema directly instead of wrapping it with schemaWrapper. Remove the wrapper import or usage and explicitly add any fields previously injected by schemaWrapper, including processId if required by the schema conventions; preserve all existing template fields and defaults.Source: Coding guidelines
backend/erxes-api-shared/src/core-modules/automations/workflowValidation.ts-6-27 (1)
6-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSupport extracting multiple action references from a single expression.
Currently,
String(expression).match(ACTION_REF_REGEX)only finds the first action reference in the string. If an expression contains multiple bindings (e.g.,Hello {{ actions.123.value }} and {{ actions.456.value }}), subsequent action references will not be validated, allowing invalid references to slip through and cause runtime errors.Consider using the
/gflag withmatchAllto ensure all references in the expression are validated.🐛 Proposed fix
-const ACTION_REF_REGEX = /\{\{\s*actions\.([^.\s{}]+)/; +const ACTION_REF_REGEX = /\{\{\s*actions\.([^.\s{}]+)/g; // Problems that must block activating an automation: workflow input bindings // referencing actions that don't exist in the automation. export const validateWorkflowBindings = ( workflows: IAutomationWorkflow[] = [], actions: IAutomationAction[] = [], ) => { const actionIds = new Set(actions.map(({ id }) => id)); const errors: string[] = []; for (const workflow of workflows) { for (const [name, expression] of Object.entries( workflow.config?.inputs || {}, )) { - const actionRef = String(expression).match(ACTION_REF_REGEX); - - if (actionRef && !actionIds.has(actionRef[1])) { - errors.push( - `Workflow "${workflow.name}": input "${name}" references missing action "${actionRef[1]}"`, - ); - } + const matches = String(expression).matchAll(ACTION_REF_REGEX); + for (const match of matches) { + if (!actionIds.has(match[1])) { + errors.push( + `Workflow "${workflow.name}": input "${name}" references missing action "${match[1]}"`, + ); + } + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/erxes-api-shared/src/core-modules/automations/workflowValidation.ts` around lines 6 - 27, The validateWorkflowBindings function only validates the first action reference in each input expression. Update ACTION_REF_REGEX and the expression-processing logic to iterate over every match, such as via matchAll with a global pattern, and report each referenced action whose ID is absent from actionIds.backend/services/automations/src/executions/executeCoreActions.ts-44-52 (1)
44-52: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPersist the parent wait state before starting the child.
startWorkflowExecutionimmediately enqueues the child, while the parent workflow action is only saved aswaitingafter this function returns. A fast child can reachresumeParentExecutionWorkerbefore that waiting action exists, racing the later parent save.Move child job publication after
handleExecutionActionResponse(..., 'waiting')has persisted the parent state, or otherwise make both transitions atomic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/executions/executeCoreActions.ts` around lines 44 - 52, Update the WORKFLOW branch in executeCoreActions so the parent action’s waiting state is persisted via handleExecutionActionResponse before startWorkflowExecution publishes the child job. Preserve the existing childExecutionId assignment and shouldBreak response, ensuring child execution cannot resume the parent before the waiting state exists.backend/services/automations/src/bullmq/actionHandlerWorker/workflow.ts-101-120 (1)
101-120: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTreat a missing workflow as an error before resuming the parent.
When the automation exists but the workflow does not, Line 120 supplies
undefined;executeActionsthen marks the parent complete instead of reporting the broken workflow reference.- if (!automation) { + if (!automation || !workflow) { execution.status = AUTOMATION_EXECUTION_STATUS.MISSID; - execution.description = 'Not found automation of execution'; + execution.description = !automation + ? 'Not found automation of execution' + : `Not found workflow ${workflowId} of execution`; await execution.save(); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/bullmq/actionHandlerWorker/workflow.ts` around lines 101 - 120, Update the workflow handling in the worker before the ACTIVE status is saved or executeActions is called: when automation exists but the lookup in the workflow find expression returns no workflow, mark the execution as failed/error, persist it, and return without resuming the parent. Preserve the existing missing-automation path and valid workflow execution behavior.backend/services/automations/src/bullmq/actionHandlerWorker/playWait.ts-57-63 (1)
57-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAwait the resumed execution.
Without
await, the worker finishes early and the surroundingtry/catchcannot handle rejected execution promises.- executeActions( + await executeActions(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/bullmq/actionHandlerWorker/playWait.ts` around lines 57 - 63, Await the resumed execution returned by executeActions in the playWait flow so the worker remains active until completion and the surrounding try/catch handles rejected promises.backend/services/automations/src/bullmq/actionHandlerWorker/workflow.ts-32-36 (1)
32-36: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNotify the parent when workflow startup fails.
The child is marked
MISSID, but its parent remainsWAITINGforever because no error notification is sent.Proposed fix
if (!automation || !workflow) { execution.status = AUTOMATION_EXECUTION_STATUS.MISSID; execution.description = `Not found workflow ${execution.workflowId} of execution`; await execution.save(); + await notifyParentExecution( + subdomain, + execution, + 'error', + execution.description, + ); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/bullmq/actionHandlerWorker/workflow.ts` around lines 32 - 36, Update the missing automation/workflow branch in the workflow startup handler to notify the parent execution of the failure before returning. Preserve the child’s MISSID status, description, save operation, and early return, and reuse the existing parent-error notification mechanism used by this worker.backend/services/automations/src/executions/actions/webhook/incoming/incomingWebhookHandler.ts-188-188 (1)
188-188: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude
workflowsin the automation projection.
getExecutionActionsMapneedsautomation.workflows, but Line 40 selects only_id triggers actions status. Consequently, webhook-triggered root executions omit every workflow node from their action map.- .select('_id triggers actions status') + .select('_id triggers actions workflows status')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/executions/actions/webhook/incoming/incomingWebhookHandler.ts` at line 188, Update the automation projection near getExecutionActionsMap to include the workflows field alongside _id, triggers, actions, and status. Ensure the automation object passed into getExecutionActionsMap contains automation.workflows so webhook-triggered root executions include workflow nodes in their action map.backend/services/automations/src/bullmq/actionHandlerWorker/playWait.ts-54-62 (1)
54-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject a missing waiting action instead of completing the execution.
When
actionsMap[waitingActionId]is absent, Line 62 passesundefined;executeActionsinterprets that as successful completion and may resume the parent as successful. Mark the executionMISSIDor throw before continuing.Proposed fix
const action = actionsMap[waitingActionId]; +if (!action) { + throw new Error(`Waiting action not found: ${waitingActionId}`); +} + executeActions( subdomain, execution.triggerType, execution, actionsMap, - action?.nextActionId, + action.nextActionId, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/bullmq/actionHandlerWorker/playWait.ts` around lines 54 - 62, Validate the result of actionsMap[waitingActionId] in playWait before calling executeActions. If the waiting action is missing, reject the execution by marking it MISSID or throwing an error; only invoke executeActions with a defined action so the execution cannot be completed successfully.backend/services/automations/src/executions/startWorkflowExecution.ts-81-104 (1)
81-104: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAwait BullMQ job enqueue calls in the workflow path.
sendWorkerQueue(...).add(...)returns a promise here, so these fire-and-forget calls can drop enqueue failures and leave child executions or parent resumes stuck. MakestartWorkflowExecutionawait the child-start job and makenotifyParentExecutionasync soexecuteActionscan await it too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/executions/startWorkflowExecution.ts` around lines 81 - 104, The workflow execution path does not await BullMQ enqueue operations, allowing child starts and parent resumes to fail silently. In backend/services/automations/src/executions/startWorkflowExecution.ts:69-73, await the child-start job enqueue; in backend/services/automations/src/executions/startWorkflowExecution.ts:81-104, make notifyParentExecution async and await its resumeParentExecution enqueue; in backend/services/automations/src/executions/executeActions.ts:58-71 and :148, await the relevant notifyParentExecution calls so executeActions propagates enqueue failures.frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowNode.tsx-145-154 (1)
145-154: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly offer unconversion when member actions can actually be preserved.
The dialog exposes “Unconvert, keep actions” for external or empty workflows, although the dropdown correctly gates unconversion with
!data.automationId && hasMemberActions. Mirror that guard here to avoid presenting a destructive action as data-preserving.Proposed fix
- <AlertDialog.Action onClick={handleUnconvertAndRemove}> - Unconvert, keep actions - </AlertDialog.Action> + {!data.automationId && hasMemberActions && ( + <AlertDialog.Action onClick={handleUnconvertAndRemove}> + Unconvert, keep actions + </AlertDialog.Action> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowNode.tsx` around lines 145 - 154, Update the AlertDialog in WorkflowNode to render the “Unconvert, keep actions” action only when the workflow is not external and has member actions, matching the existing dropdown guard of !data.automationId && hasMemberActions. Keep the delete action available for all applicable workflows.frontend/core-ui/src/modules/automations/utils/automationBuilderUtils/generateNodes.ts-32-32 (1)
32-32: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the newly introduced
anyassertion.Use a type guard or add
iconto the workflow domain type instead.Proposed fix
- icon: (node as any).icon || 'IconArrowsSplit2', + icon: + 'icon' in node && typeof node.icon === 'string' + ? node.icon + : 'IconArrowsSplit2',As per coding guidelines, “
**/*.ts: Do not useanytype in TypeScript code; always provide explicit types.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/utils/automationBuilderUtils/generateNodes.ts` at line 32, Remove the any assertion from the icon assignment in generateNodes. Use an appropriate type guard or extend the workflow domain type to declare the icon property, while preserving the existing IconArrowsSplit2 fallback.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationNodeOutput.ts-28-35 (1)
28-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTreat defined static variables as authoritative, including an empty array.
Workflow-input pseudo sources still execute
AUTOMATION_NODE_OUTPUT, andstaticVariables: []falls back to fetched variables because the check uses.length. Skip the query wheneverstaticVariablesis defined and return that array directly.Proposed fix
+ const hasStaticVariables = activeSourceNode?.staticVariables !== undefined; + const { data, loading } = useQuery<TAutomationNodeOutputResponse>( AUTOMATION_NODE_OUTPUT, { - skip: !activeSourceNode?.type, + skip: !activeSourceNode?.type || hasStaticVariables, variables: { nodeType }, }, ); - const mergedVariables = activeSourceNode?.staticVariables?.length - ? activeSourceNode.staticVariables + const mergedVariables = hasStaticVariables + ? activeSourceNode?.staticVariables || [] : coreVariables.filter(Also applies to: 53-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationNodeOutput.ts` around lines 28 - 35, Update the useAutomationNodeOutput hook to treat staticVariables as authoritative whenever it is defined, including when it is an empty array. Skip AUTOMATION_NODE_OUTPUT in that case and return staticVariables directly; only execute and use fetched variables when staticVariables is undefined.frontend/core-ui/src/modules/automations/components/builder/hooks/useConvertSelectionToWorkflow.ts-90-137 (1)
90-137: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve or reject selection boundaries during conversion.
The conversion collapses every inbound edge to one
entryActionId, but does not extract outgoing edges intonewWorkflow.nextActionIdor remove external references from member actions. Multi-entry selections and selections ending at an unselected action therefore change execution semantics or leave child actions pointing outside their workflow.Validate that the selection has one supported entry/exit and remap both boundaries, or reject unsupported selections before mutating form state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useConvertSelectionToWorkflow.ts` around lines 90 - 137, Update the conversion flow around entryActionId, remapConnections, and newWorkflow to validate that the selected members have exactly one supported entry and exit boundary. Remap the selected entry and outgoing exit to the child workflow’s entryActionId and nextActionId, and remove or rewrite external references in member actions; reject unsupported multi-entry or multi-exit selections before any setAutomationBuilderFormValue calls.frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationCoreNodeOutput.ts-45-164 (1)
45-164: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftReplace the newly introduced
any-typed workflow boundaries with shared domain schemas.
frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationCoreNodeOutput.ts#L45-L164: define explicit source-config, AI field, find-object target, and transform-mapping types.frontend/core-ui/src/modules/automations/utils/automationFormDefinitions.ts#L80-L80: define the known workflow configuration fields in Zod.frontend/core-ui/src/modules/automations/components/builder/sidebar/states/automationNodeLibrary.tsx#L10-L11: validate dragged templates using the saved workflow-template schema.As per coding guidelines,
**/*.ts: “Do not useanytype in TypeScript code; always provide explicit types.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationCoreNodeOutput.ts` around lines 45 - 164, Replace the any-typed workflow boundaries with shared domain schemas: in useAutomationCoreNodeOutput, define explicit types for sourceNodeConfig, AI field definitions, find-object targets, and transform mappings; in frontend/core-ui/src/modules/automations/utils/automationFormDefinitions.ts:80, add the known workflow configuration fields to the Zod schema; and in frontend/core-ui/src/modules/automations/components/builder/sidebar/states/automationNodeLibrary.tsx:10-11, validate dragged templates with the saved workflow-template schema. Remove all any usage while preserving the existing output-variable behavior.Source: Coding guidelines
frontend/core-ui/src/modules/automations/utils/automationFormDefinitions.ts-82-82 (1)
82-82: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake
iconoptional and explicitly typed.The builder’s converter creates workflows without
icon, while this schema requires it. The Zod resolver will reject saving a converted workflow. Use the actual icon schema—likely an optional string—instead of requiredz.any().- icon: z.any(), + icon: z.string().optional(),As per coding guidelines,
**/*.ts: “Do not useanytype in TypeScript code; always provide explicit types.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/utils/automationFormDefinitions.ts` at line 82, Update the icon field in the automation form schema to use the actual icon type, likely an optional string, instead of required z.any(). Ensure workflows created by the builder converter can omit icon while preserving validation for provided values, and do not introduce any TypeScript any types.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowInputBindings.ts-39-47 (1)
39-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUpdate the input record rather than interpolating its key into a form path.
Names containing
.or brackets are interpreted as nested React Hook Form paths, and a missing workflow producesworkflows.-1.... Guard the index and replace the completeconfig.inputsrecord using[name]: expression.Proposed fix
const updateBinding = useCallback( (name: string, expression: string) => { + if (workflowIndex < 0) { + return; + } + setAutomationBuilderFormValue( - `workflows.${workflowIndex}.config.inputs.${name}` as Path<TAutomationBuilderForm>, - expression, + `workflows.${workflowIndex}.config.inputs` as Path<TAutomationBuilderForm>, + { + ...(workflows?.[workflowIndex]?.config?.inputs || {}), + [name]: expression, + }, ); }, - [workflowIndex, setAutomationBuilderFormValue], + [workflowIndex, workflows, setAutomationBuilderFormValue], );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowInputBindings.ts` around lines 39 - 47, Update updateBinding to guard against an invalid workflowIndex, including -1, before writing any form value. Replace the complete workflows.${workflowIndex}.config.inputs record using a computed [name]: expression entry rather than interpolating name into the form path, preserving existing input bindings while safely handling names containing dots or brackets.frontend/core-ui/src/modules/automations/components/builder/hooks/useInsertWorkflowTemplate.ts-42-46 (1)
42-46: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRemap IDs structurally instead of replacing arbitrary text.
split(action.id).join(newId)also rewrites IDs occurring as substrings in labels, configuration values, or other IDs. Overlapping IDs can additionally corrupt later mappings. Restrict replacement to connection fields and parsedactions.<id>placeholders.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useInsertWorkflowTemplate.ts` around lines 42 - 46, Update the ID-remapping loop in useInsertWorkflowTemplate to parse the workflow structure and replace only connection fields and parsed actions.<id> placeholders, rather than performing global text substitution. Ensure replacements match complete IDs so substrings and overlapping IDs in labels, configuration values, or other action IDs remain unchanged, while preserving the existing new-ID generation and usedIds tracking.frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx-9-25 (1)
9-25: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace the newly introduced
anytypes with explicit contracts.
frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx#L9-L25: explicitly model optional row metadata and type the drag callback with the row item plusnodeType.frontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useNodeMetaEdit.ts#L31-L31: derive the form entry type fromTAutomationBuilderFormrather than casting toRecord<string, any>.As per coding guidelines, “Do not use
anytype in TypeScript code; always provide explicit types.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx` around lines 9 - 25, Replace the any-based contracts in NodeLibraryRow.tsx: explicitly define optional row metadata on TNodeLibraryRowItem and type onDragStart with the row item data plus nodeType, preserving the existing callback behavior. In useNodeMetaEdit.ts at line 31, derive the form entry type from TAutomationBuilderForm instead of casting to Record<string, any>.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts-76-76 (1)
76-76: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace the new
anyerror types withunknown.Narrow the value before reading
message.Proposed fix
- } catch (error: any) { + } catch (error: unknown) { toast({ title: 'Failed to save template', - description: error.message, + description: + error instanceof Error ? error.message : 'An unknown error occurred',Apply the same change to the update-template handler.
As per coding guidelines, “Do not use
anytype in TypeScript code; always provide explicit types.”Also applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts` at line 76, Replace the any catch error types with unknown in both the create-template and update-template handlers, then narrow each error before accessing its message while preserving the existing error handling behavior.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowCanvasSheet.tsx-78-89 (1)
78-89: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMark workflow input changes as template edits.
updateTemplateFromWorkflowpersistsworkflow.config.inputs, but this successful input update never invokesonMembersChanged. Closing after an input-only edit therefore skips the template update prompt.Proposed fix
outerForm.setValue( `workflows.${workflowIndex}.config.inputs` as FieldPath<TAutomationBuilderForm>, { ...inputs, [trimmed]: '' }, { shouldDirty: true }, ); + onMembersChanged?.();Also include
onMembersChangedin the callback dependencies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowCanvasSheet.tsx` around lines 78 - 89, Update addInput in WorkflowCanvasSheet so that after successfully setting a new workflow input, it invokes onMembersChanged to mark the template as edited and trigger the update prompt. Add onMembersChanged to the useCallback dependency array while preserving the existing validation and setValue behavior.
🟡 Minor comments (8)
docs/decisions/automation-decisions.md-116-159 (1)
116-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign this decision with the implemented untyped input bindings.
The implementation now creates
input.*placeholders and stores binding maps. Keep typed schemas deferred, but document the untyped binding model instead of stating that mappings and workflow inputs are absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/decisions/automation-decisions.md` around lines 116 - 159, The decision text should reflect the implemented untyped input-binding model: retain the deferral of typed schemas, required-variable contracts, validation, and versioning, but document that workflows expose input.* placeholders and store binding maps that resolve them from the parent execution context. Update the “Do not add” list, examples, and “Initial Rule” section accordingly without implying workflow inputs or mappings are absent.backend/erxes-api-shared/src/core-modules/automations/definitions/automations.ts-42-70 (1)
42-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace
anytypes withunknownto satisfy strict typing guidelines.The newly added TypeScript definitions utilize the
anytype, which bypasses type checking and violates the repository's coding guidelines. As per coding guidelines, do not useanytype in TypeScript code; always provide explicit types.
backend/erxes-api-shared/src/core-modules/automations/definitions/automations.ts#L42-L70: Replace[key: string]: any;andposition?: any;withunknownor a specific object interface.backend/erxes-api-shared/src/core-modules/automations/definitions/executions.ts#L35-L42: Update theinputsdefinition fromRecord<string, any>toRecord<string, unknown>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/erxes-api-shared/src/core-modules/automations/definitions/automations.ts` around lines 42 - 70, Replace the any types in IAutomationWorkflow with unknown: use unknown for the config index signature and position. In backend/erxes-api-shared/src/core-modules/automations/definitions/executions.ts lines 35-42, update the inputs field from Record<string, any> to Record<string, unknown>; preserve the existing interfaces and field behavior.Source: Coding guidelines
frontend/core-ui/src/modules/automations/hooks/useReactFlowEditor.ts-81-88 (1)
81-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPosition the workflow input according to
flowDirection.Vertical workflows still place the input node to the left of the entry action, producing a sideways input edge instead of placing it above the entry.
Proposed fix
position: { - x: entryNode.position.x - 260, - y: entryNode.position.y, + x: + flowDirection === 'vertical' + ? entryNode.position.x + : entryNode.position.x - 260, + y: + flowDirection === 'vertical' + ? entryNode.position.y - 260 + : entryNode.position.y, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/hooks/useReactFlowEditor.ts` around lines 81 - 88, Update the workflow input node positioning in useReactFlowEditor so its coordinates depend on flowDirection: retain the left-of-entry placement for horizontal flows and place it above the entry action for vertical flows. Keep the existing offset magnitude and alignment consistent with the corresponding workflow direction.frontend/core-ui/src/modules/automations/components/builder/MarqueeSelectionPanel.tsx-64-79 (1)
64-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssociate visible labels with both workflow fields.
The inputs have no accessible names;
nameand placeholder attributes do not replace<label htmlFor>. Add labels and matching IDs for “Name” and “Description”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/MarqueeSelectionPanel.tsx` around lines 64 - 79, Add visible “Name” and “Description” labels in MarqueeSelectionPanel alongside the two Input components, and assign matching unique IDs to each input via htmlFor. Keep the existing values and onChange behavior unchanged.frontend/core-ui/src/modules/automations/components/builder/AutomationBuilderControls.tsx-268-274 (1)
268-274: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive the icon-only control an accessible name.
labelis only rendered in the tooltip. Forward it to the underlying button asaria-labelso assistive technology can identify the new control.Proposed fix
<Button type="button" + aria-label={label} variant="ghost"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/AutomationBuilderControls.tsx` around lines 268 - 274, Update the marquee ControlButton in AutomationBuilderControls to forward its existing label value as the underlying button’s aria-label, ensuring the icon-only control has an accessible name while preserving the tooltip and toggle behavior.frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowInputBindings.tsx-37-39 (1)
37-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive dangling-binding warnings an accessible description.
The warning icon and count do not explain what is wrong to screen-reader users—or to sighted users unfamiliar with the icon. Add an
aria-label, visually hidden text, or a tooltip such as “Dangling workflow input bindings.”Also applies to: 50-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowInputBindings.tsx` around lines 37 - 39, Update the dangling-binding indicator in WorkflowInputBindings to provide an accessible, user-facing description such as “Dangling workflow input bindings,” using an aria-label, visually hidden text, or tooltip; apply the same treatment to both warning-icon occurrences.frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/WorkflowNodeLibraryContent.tsx-127-135 (1)
127-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd an accessible name to the template delete button.
This icon-only button needs
aria-label={Delete ${template.name} template}or equivalent text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/WorkflowNodeLibraryContent.tsx` around lines 127 - 135, Add an accessible name to the icon-only delete Button in the template library component by setting its aria-label to identify the specific template, such as “Delete [template name] template.” Keep the existing handlers, styling, and IconTrash rendering unchanged.frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts-112-114 (1)
112-114: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle template deletion failures.
The dialog clears its removal target immediately, but this discarded mutation promise can reject without any feedback. Await it and show a destructive toast on failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts` around lines 112 - 114, Update removeTemplate to await the removeTemplateMutation promise before clearing the removal target, and catch failures to show a destructive toast with the error feedback. Preserve the existing mutation variables and ensure rejected deletions are surfaced instead of becoming unhandled promises.
🧹 Nitpick comments (5)
backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts (1)
341-343: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlways destructure
subdomainfrom context in backend resolvers.As per coding guidelines, backend resolvers and services must consistently extract the
subdomainproperty alongsidemodelsfrom the context object, even if it is not immediately used inside the method body.
backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts#L341-L343: Update{ user, models, checkPermission }to{ user, subdomain, models, checkPermission }inautomationWorkflowTemplatesAdd.backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts#L368-L370: Update context extraction to{ subdomain, models, checkPermission }inautomationWorkflowTemplatesEdit.backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts#L381-L383: Update context extraction to{ subdomain, models, checkPermission }inautomationWorkflowTemplatesRemove.backend/core-api/src/modules/automations/graphql/resolvers/queries.ts#L445-L447: Update context extraction to{ subdomain, models }inautomationWorkflowTemplates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts` around lines 341 - 343, Always destructure subdomain from the resolver context alongside the existing properties: update automationWorkflowTemplatesAdd, automationWorkflowTemplatesEdit, and automationWorkflowTemplatesRemove in backend/core-api/src/modules/automations/graphql/resolvers/mutations.ts at lines 341-343, 368-370, and 381-383, and automationWorkflowTemplates in backend/core-api/src/modules/automations/graphql/resolvers/queries.ts at lines 445-447; preserve all other resolver behavior.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/WorkflowsNodeLibrary.tsx (1)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombine transition classes for intended effect.
Applying both
transition-shadowandtransition-colorscan cause one to override the other depending on the stylesheet order. Usetransition-all(or the defaulttransitionutility) to ensure both shadow and colors animate.♻️ Proposed fix
- className="hover:shadow-md transition-shadow cursor-pointer border-accent cursor-grab hover:bg-accent transition-colors h-16 mb-2 w-[350px] sm:w-[500px] hover:border-info" + className="hover:shadow-md cursor-pointer border-accent cursor-grab hover:bg-accent transition-all h-16 mb-2 w-[350px] sm:w-[500px] hover:border-info"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/WorkflowsNodeLibrary.tsx` at line 33, Update the className on the workflow library item to replace the separate transition-shadow and transition-colors utilities with a single transition-all or transition utility, preserving the existing hover shadow and color behavior.frontend/core-ui/src/modules/automations/components/builder/nodes/actions/webhooks/components/OutgoingWebhookRequest.tsx (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract inline array and remove dead code.
The inline array for the
disabledprop is quite long, making the JSX harder to read. Additionally, the oldDISABLED_WEBHOOK_URL_SUGGESTIONSobject (lines 8-18) is now unused dead code. Consider extracting this array into a new constant and deleting the old unused object.♻️ Proposed fix
Replace the old
DISABLED_WEBHOOK_URL_SUGGESTIONSobject with this array:const DISABLED_WEBHOOK_URL_SUGGESTIONS = [ TPlaceholderInputSuggestion.Attribute, TPlaceholderInputSuggestion.Emoji, TPlaceholderInputSuggestion.Date, TPlaceholderInputSuggestion.Option, TPlaceholderInputSuggestion.CallUser, TPlaceholderInputSuggestion.CallTag, TPlaceholderInputSuggestion.CallProduct, TPlaceholderInputSuggestion.CallCompany, TPlaceholderInputSuggestion.CallCustomer, ];And then pass it to the component:
- disabled={[TPlaceholderInputSuggestion.Attribute, TPlaceholderInputSuggestion.Emoji, TPlaceholderInputSuggestion.Date, TPlaceholderInputSuggestion.Option, TPlaceholderInputSuggestion.CallUser, TPlaceholderInputSuggestion.CallTag, TPlaceholderInputSuggestion.CallProduct, TPlaceholderInputSuggestion.CallCompany, TPlaceholderInputSuggestion.CallCustomer]} + disabled={DISABLED_WEBHOOK_URL_SUGGESTIONS}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/nodes/actions/webhooks/components/OutgoingWebhookRequest.tsx` at line 58, Extract the inline disabled suggestion list from the disabled prop in OutgoingWebhookRequest into a DISABLED_WEBHOOK_URL_SUGGESTIONS constant, then pass that constant to the component. Remove the existing unused DISABLED_WEBHOOK_URL_SUGGESTIONS object and preserve all listed TPlaceholderInputSuggestion values.frontend/plugins/frontline_ui/src/widgets/automations/modules/facebook/components/trigger/components/summary/TriggerConditionSummary.tsx (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid using array indices as React keys if possible.
Using
indexas a key can cause rendering issues if theconditionSummarieslist is ever reordered, filtered, or modified dynamically. Ifconditionhas a unique identifier (like anidor a uniquely identifiable string), prefer using that instead. If this list is strictly static during the component's lifecycle,indexis acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/frontline_ui/src/widgets/automations/modules/facebook/components/trigger/components/summary/TriggerConditionSummary.tsx` at line 13, Update the list rendering in TriggerConditionSummary to use a stable unique key derived from each condition or its summary instead of the array index. Preserve the current rendering behavior and only retain index-based keys if no unique identifier exists and the list is guaranteed static.frontend/libs/ui-modules/src/modules/automations/components/placeholderInput/PlaceholderInput.tsx (1)
66-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrevent potential null-pointer errors if
onChangeis optional.If
onChangecan be undefined inPlaceholderInputProps, invokingonChangeRef.current(nextValue)directly will cause a crash. Adding optional chaining ensures safe execution.♻️ Proposed fix
- onChangeRef.current(nextValue); + onChangeRef.current?.(nextValue);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/libs/ui-modules/src/modules/automations/components/placeholderInput/PlaceholderInput.tsx` around lines 66 - 108, Update the onChange invocation inside insertVariable to safely handle an absent optional callback, using the existing onChangeRef symbol and preserving the current update and selection behavior.
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [innerActions, workflowIndex]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Remove the reference to the unavailable ESLint rule.
Static analysis reports this directive itself as an error because react-hooks/exhaustive-deps is not defined. Remove the directive and declare the effect dependencies explicitly.
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [innerActions, workflowIndex]);
+ }, [innerActions, onMembersChanged, outerForm, workflowIndex]);📝 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.
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [innerActions, workflowIndex]); | |
| }, [innerActions, onMembersChanged, outerForm, workflowIndex]); |
🧰 Tools
🪛 ESLint
[error] 73-73: Definition for rule 'react-hooks/exhaustive-deps' was not found.
(react-hooks/exhaustive-deps)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowCanvasSheet.tsx`
around lines 73 - 74, Remove the eslint-disable directive above the effect in
WorkflowCanvasSheet and update the effect dependency array to explicitly include
every value referenced by its callback, preserving the existing innerActions and
workflowIndex dependencies.
Source: Linters/SAST tools
…ow execution results feat: enhance AutomationHistoryDetailContext to manage child execution navigation refactor: update useAutomationBilderWorkSpace to utilize new automation context fix: log errors in useAutomationHeader for better debugging refactor: simplify useWorkflowTemplatePrompt by removing unnecessary state management feat: implement AiAgentToolBuilder for managing AI agent tools in configuration feat: extend AiAgentNodeContent to display tools associated with generateText goal type feat: add tools tab in AiAgentConfigForm for configuring AI agent tools refactor: improve WorkflowCanvasSheet to handle unsaved changes and template updates refactor: streamline sidebar hooks in automation builder for better state management fix: update useNodeEvents to handle node interactions more effectively
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import { | ||
| isCoreAutomationNodeType, | ||
| useAutomationCoreNodeOutput, | ||
| } from './useAutomationCoreNodeOutput'; |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
backend/plugins/sales_api/src/modules/pos/meta/automations/constants.ts (1)
124-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove debugging artifacts.
The
console.logstatements appear to be leftover debugging artifacts and should be removed to keep the logs clean.🧹 Proposed fix
- onlinePaymentUrl: async ({ subdomain, source }) => { - console.log({ source }); - const orderId = - typeof source.targetId === 'string' - ? source.targetId - : String(source._id || ''); - - const result = await resolvePosOrderPaymentUrl({ orderId, subdomain }); - console.log({ result }); - return result; - }, + onlinePaymentUrl: async ({ subdomain, source }) => { + const orderId = + typeof source.targetId === 'string' + ? source.targetId + : String(source._id || ''); + + return await resolvePosOrderPaymentUrl({ orderId, subdomain }); + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/plugins/sales_api/src/modules/pos/meta/automations/constants.ts` around lines 124 - 134, Remove both console.log calls from the onlinePaymentUrl handler while preserving the existing orderId resolution, resolvePosOrderPaymentUrl invocation, and returned result.frontend/core-ui/src/modules/automations/components/builder/hooks/useAutomationHeader.ts (1)
127-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove debug logging.
There is a
console.logleft in the production code. Consider removing it to keep the console clean and avoid inadvertently logging sensitive error details.♻️ Proposed fix
- console.log({ errors });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useAutomationHeader.ts` at line 127, Remove the debug console.log({ errors }) statement from the useAutomationHeader hook, leaving the surrounding error-handling behavior unchanged.backend/erxes-api-shared/src/core-modules/automations/outputResolvers.test.ts (1)
1-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for the targetId-preferred resolution path; consider adding cases for
input.placeholders and thetarget-fallback branch.The two existing tests correctly validate
resolveCurrentOutputSourcerefreshing fields viatargetIdand the passthrough behavior when notargetIdis present. The newinput.sourceType placeholder path (and thegetSourceReferenceTargetbranch that falls back to{ target: source }whentargetIdis absent) aren't covered here — both are newly introduced, non-trivial branches inoutputResolvers.tsworth locking in with tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/erxes-api-shared/src/core-modules/automations/outputResolvers.test.ts` around lines 1 - 97, Add tests for the input. sourceType placeholder handling and the getSourceReferenceTarget fallback that uses { target: source } when targetId is absent. Verify input-based paths resolve through the expected source type, and the fallback passes the source object as target while preserving the existing direct-root behavior.backend/erxes-api-shared/src/core-modules/automations/outputResolvers.ts (1)
151-184: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant reference-resolution calls for paths sharing the same top-level field.
resolveCurrentOutputSourceis invoked once per uniquepathinside the loop inresolveOutputPathsFromDefinition. When two paths share the sameheadsegment (e.g.assigneeId.displayNameandassigneeId.email), each iteration re-derives the samesourceField(assigneeId) and issues a separateresolveRecordReferenceValueround-trip for it, even though the fetched value would be identical.Since
resolveRecordReferenceValuedispatches throughsendCoreModuleProducer(an inter-service/plugin call), this is a real, avoidable I/O cost that scales with the number of unique top-level fields referenced in a template, not just once per distinct field.Consider caching the resolved value per
sourceField(or pertargetId+sourceField) across thefor (const path of ...)loop inresolveOutputPathsFromDefinitionso repeated top-level fields are only fetched once.♻️ Suggested caching approach
const result: Record<string, unknown> = {}; +const currentSourceCache = new Map<string, TAutomationOutputSource>(); for (const path of [...new Set(paths)]) { - const currentSource = await resolveCurrentOutputSource({ - definition, - path, - source, - subdomain, - }); + const [pathHead] = path.split('.'); + if (!currentSourceCache.has(pathHead)) { + currentSourceCache.set( + pathHead, + await resolveCurrentOutputSource({ definition, path, source, subdomain }), + ); + } + const currentSource = currentSourceCache.get(pathHead) as TAutomationOutputSource;Also applies to: 367-373
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/erxes-api-shared/src/core-modules/automations/outputResolvers.ts` around lines 151 - 184, Cache resolved reference values in resolveOutputPathsFromDefinition, keyed by the relevant targetId and sourceField, and reuse them when resolveCurrentOutputSource processes paths sharing the same top-level field. Ensure each unique field is fetched through resolveRecordReferenceValue only once while preserving the existing fallback when the resolved value is undefined.backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts (1)
13-25: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueExclude arrays when safely parsing JSON objects.
Because
typeof [] === 'object', this helper will return an array ifvalueis an array or if the parsed JSON resolves to an array (e.g.,"[]"). Explicitly checking!Array.isArray(...)ensures we strictly return an object record as intended, which avoids downstream issues if the LLM hallucinates an array for tool arguments.♻️ Proposed refactor
const safeParseJsonObject = (value: unknown): Record<string, unknown> => { - if (value && typeof value === 'object') { + if (value && typeof value === 'object' && !Array.isArray(value)) { return value as Record<string, unknown>; } if (typeof value !== 'string' || !value.trim()) { return {}; } try { const parsed = JSON.parse(value); - return parsed && typeof parsed === 'object' ? parsed : {}; + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : {}; } catch (_error) { return {}; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts` around lines 13 - 25, Update safeParseJsonObject to exclude arrays from both direct inputs and JSON.parse results by requiring !Array.isArray(value) and !Array.isArray(parsed) alongside the existing object checks. Return an empty record for arrays while preserving the current handling of valid object values and invalid inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/plugins/frontline_api/src/modules/integrations/facebook/meta/automation/messages/index.ts`:
- Around line 252-264: Remove the console.log statement that serializes
messages, resolvedConfig, and outputResolvedConfig near the executionId/actionId
context. Do not log these payloads; if debugging is required, route only
explicitly redacted fields through the existing structured logger behind an
appropriate debug guard.
In `@backend/services/automations/src/ai/bridge/types.ts`:
- Line 60: Replace all disallowed any usage with unknown or appropriately shaped
Record<string, unknown> types across
backend/services/automations/src/ai/bridge/types.ts:60,
backend/services/automations/src/executions/receiveTrigger.ts:33-35,
backend/services/automations/src/ai/bridge/anthropicMessages/invoke.ts:12-19,
backend/services/automations/src/ai/bridge/anthropicMessages/request.ts:155,
backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts:33-41, and
backend/services/automations/src/ai/bridge/openaiCompatible/request.ts:158.
Update the affected raw, targets, response, part, array, assertions, and record
types while preserving existing behavior and adding any necessary narrowing for
unknown values.
In `@backend/services/automations/src/executions/actions/aiAgentTools.ts`:
- Around line 58-66: Update the resolvedBindings accumulator in the surrounding
input-resolution flow to use a prototype-free object, such as one created with
Object.create(null), before the for loop assigns resolvedBindings[name].
Preserve the existing binding checks and modelParamNames behavior while
preventing special keys like __proto__ and constructor from affecting the object
prototype.
In `@backend/services/automations/src/executions/startWorkflowExecution.ts`:
- Around line 47-58: Harden the handoff merge in startWorkflowExecution by
excluding array handoffArgs and rejecting unsafe binding keys such as __proto__,
constructor, and prototype before computed assignments to inputs. Preserve the
existing binding and undefined-value precedence for valid keys, while ensuring
only own, safe properties are assigned.
In `@backend/services/automations/src/trpc/automations/trigger.ts`:
- Line 30: Remove the leftover debug console.log statements from trigger.ts
lines 30-30, handleTrigger.ts lines 54-54, and checkIsWaitingActionTarget.ts
lines 96-102, including all waiting-action, target, and input payload logging.
If diagnostics are still required in checkIsWaitingActionTarget, use the
existing debugError or debugInfo utilities instead.
---
Nitpick comments:
In
`@backend/erxes-api-shared/src/core-modules/automations/outputResolvers.test.ts`:
- Around line 1-97: Add tests for the input. sourceType placeholder handling and
the getSourceReferenceTarget fallback that uses { target: source } when targetId
is absent. Verify input-based paths resolve through the expected source type,
and the fallback passes the source object as target while preserving the
existing direct-root behavior.
In `@backend/erxes-api-shared/src/core-modules/automations/outputResolvers.ts`:
- Around line 151-184: Cache resolved reference values in
resolveOutputPathsFromDefinition, keyed by the relevant targetId and
sourceField, and reuse them when resolveCurrentOutputSource processes paths
sharing the same top-level field. Ensure each unique field is fetched through
resolveRecordReferenceValue only once while preserving the existing fallback
when the resolved value is undefined.
In `@backend/plugins/sales_api/src/modules/pos/meta/automations/constants.ts`:
- Around line 124-134: Remove both console.log calls from the onlinePaymentUrl
handler while preserving the existing orderId resolution,
resolvePosOrderPaymentUrl invocation, and returned result.
In `@backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts`:
- Around line 13-25: Update safeParseJsonObject to exclude arrays from both
direct inputs and JSON.parse results by requiring !Array.isArray(value) and
!Array.isArray(parsed) alongside the existing object checks. Return an empty
record for arrays while preserving the current handling of valid object values
and invalid inputs.
In
`@frontend/core-ui/src/modules/automations/components/builder/hooks/useAutomationHeader.ts`:
- Line 127: Remove the debug console.log({ errors }) statement from the
useAutomationHeader hook, leaving the surrounding error-handling behavior
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 049d1013-899a-4672-8b70-60c950938ab8
📒 Files selected for processing (53)
backend/erxes-api-shared/src/core-modules/automations/outputResolvers.test.tsbackend/erxes-api-shared/src/core-modules/automations/outputResolvers.tsbackend/erxes-api-shared/src/core-modules/automations/zodTypes.tsbackend/plugins/frontline_api/src/modules/integrations/facebook/meta/automation/messages/index.tsbackend/plugins/payment_api/src/modules/payment/graphql/resolvers/mutations/invoices.tsbackend/plugins/payment_api/src/modules/payment/services/invoiceUrl.spec.tsbackend/plugins/payment_api/src/modules/payment/services/invoiceUrl.tsbackend/plugins/payment_api/src/trpc/init-trpc.tsbackend/plugins/sales_api/src/modules/pos/fieldUtils.spec.tsbackend/plugins/sales_api/src/modules/pos/fieldUtils.tsbackend/plugins/sales_api/src/modules/pos/meta/automations/constants.tsbackend/plugins/sales_api/src/modules/pos/meta/automations/resolvers/resolvePosOrderPaymentUrl.spec.tsbackend/plugins/sales_api/src/modules/pos/meta/automations/resolvers/resolvePosOrderPaymentUrl.tsbackend/plugins/sales_api/src/trpc/init-trpc.tsbackend/services/automations/src/ai/aiAction/contract.tsbackend/services/automations/src/ai/aiAction/index.tsbackend/services/automations/src/ai/aiAction/run.tsbackend/services/automations/src/ai/aiAction/tools.tsbackend/services/automations/src/ai/bridge/anthropicMessages/invoke.tsbackend/services/automations/src/ai/bridge/anthropicMessages/request.tsbackend/services/automations/src/ai/bridge/index.tsbackend/services/automations/src/ai/bridge/openaiCompatible/invoke.tsbackend/services/automations/src/ai/bridge/openaiCompatible/request.tsbackend/services/automations/src/ai/bridge/types.tsbackend/services/automations/src/executions/actions/aiAgentTools.tsbackend/services/automations/src/executions/actions/executeAiAgentAction.tsbackend/services/automations/src/executions/checkIsWaitingActionTarget.tsbackend/services/automations/src/executions/handleTrigger.tsbackend/services/automations/src/executions/receiveTrigger.tsbackend/services/automations/src/executions/repeatActionExecution.tsbackend/services/automations/src/executions/startWorkflowExecution.tsbackend/services/automations/src/trpc/automations/trigger.tsfrontend/core-ui/src/modules/automations/components/builder/header/AutomationBuilderHeader.tsxfrontend/core-ui/src/modules/automations/components/builder/history/components/AutomationHistoryByTable.tsxfrontend/core-ui/src/modules/automations/components/builder/history/components/AutomationHistoryDetail.tsxfrontend/core-ui/src/modules/automations/components/builder/history/components/WorkflowExecutionResultCell.tsxfrontend/core-ui/src/modules/automations/components/builder/history/context/AutomationHistoryDetailContext.tsxfrontend/core-ui/src/modules/automations/components/builder/hooks/useAutomationBilderWorkSpace.tsfrontend/core-ui/src/modules/automations/components/builder/hooks/useAutomationHeader.tsfrontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplatePrompt.tsfrontend/core-ui/src/modules/automations/components/builder/nodes/actions/aiAgent/components/AiAgentConfigForm.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/actions/aiAgent/components/AiAgentNodeContent.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/actions/aiAgent/components/AiAgentToolBuilder.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/actions/aiAgent/states/aiAgentForm.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/components/PlaceHolderNode.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowCanvasSheet.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/hooks/useAutomationNodeOutput.tsfrontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useAutomationActionContentSidebar.tsfrontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useAutomationBuilderSidebarHooks.tsfrontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useCoreCustomTriggerContent.tsfrontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useCustomTriggerContent.tsfrontend/core-ui/src/modules/automations/context/AutomationProvider.tsxfrontend/core-ui/src/modules/automations/hooks/useNodeEvents.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/core-ui/src/modules/automations/components/builder/history/components/AutomationHistoryByTable.tsx
- frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowCanvasSheet.tsx
|
|
||
| export type TAiBridgeInvokeResult = { | ||
| text: string; | ||
| raw?: any; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not use the any type.
As per coding guidelines, TypeScript code must not use the any type and should provide explicit types or use unknown. Please replace the following occurrences to enforce strict typing across the AI bridge and execution parameters:
backend/services/automations/src/ai/bridge/types.ts#L60-L60: Replaceraw?: any;withraw?: unknown;.backend/services/automations/src/executions/receiveTrigger.ts#L33-L35: Replacetargets: any[];withtargets: unknown[];andRecord<string, any>withRecord<string, unknown>.backend/services/automations/src/ai/bridge/anthropicMessages/invoke.ts#L12-L19: Replace theanytypes in theresponseandpartparameters withunknownorRecord<string, unknown>.backend/services/automations/src/ai/bridge/anthropicMessages/request.ts#L155-L155: Replaceany[]withRecord<string, unknown>[]or omit the explicit type entirely if it can be inferred.backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts#L33-L41: Replace theanyassertions and parameter types withunknownorRecord<string, unknown>.backend/services/automations/src/ai/bridge/openaiCompatible/request.ts#L158-L158: ReplaceRecord<string, any>withRecord<string, unknown>.
📍 Affects 6 files
backend/services/automations/src/ai/bridge/types.ts#L60-L60(this comment)backend/services/automations/src/executions/receiveTrigger.ts#L33-L35backend/services/automations/src/ai/bridge/anthropicMessages/invoke.ts#L12-L19backend/services/automations/src/ai/bridge/anthropicMessages/request.ts#L155-L155backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts#L33-L41backend/services/automations/src/ai/bridge/openaiCompatible/request.ts#L158-L158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/automations/src/ai/bridge/types.ts` at line 60, Replace all
disallowed any usage with unknown or appropriately shaped Record<string,
unknown> types across backend/services/automations/src/ai/bridge/types.ts:60,
backend/services/automations/src/executions/receiveTrigger.ts:33-35,
backend/services/automations/src/ai/bridge/anthropicMessages/invoke.ts:12-19,
backend/services/automations/src/ai/bridge/anthropicMessages/request.ts:155,
backend/services/automations/src/ai/bridge/openaiCompatible/invoke.ts:33-41, and
backend/services/automations/src/ai/bridge/openaiCompatible/request.ts:158.
Update the affected raw, targets, response, part, array, assertions, and record
types while preserving existing behavior and adding any necessary narrowing for
unknown values.
Source: Coding guidelines
| for (const name of inputNames) { | ||
| const hasBinding = String(bindings[name] ?? '').trim().length > 0; | ||
|
|
||
| if (hasBinding && resolved[name] !== undefined) { | ||
| resolvedBindings[name] = resolved[name]; | ||
| } else { | ||
| modelParamNames.push(name); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Guard against __proto__/constructor keys when building resolvedBindings. resolvedBindings[name] = resolved[name] uses a computed assignment where name comes from Object.keys(bindings). These keys are builder-defined workflow input names (low risk today), but a __proto__ key here would pollute the object prototype. A Object.create(null) accumulator or a key skip-list closes this cheaply.
🛡️ Proposed guard
- const resolvedBindings: Record<string, unknown> = {};
+ const resolvedBindings: Record<string, unknown> = Object.create(null);📝 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.
| for (const name of inputNames) { | |
| const hasBinding = String(bindings[name] ?? '').trim().length > 0; | |
| if (hasBinding && resolved[name] !== undefined) { | |
| resolvedBindings[name] = resolved[name]; | |
| } else { | |
| modelParamNames.push(name); | |
| } | |
| } | |
| const resolvedBindings: Record<string, unknown> = Object.create(null); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/automations/src/executions/actions/aiAgentTools.ts` around
lines 58 - 66, Update the resolvedBindings accumulator in the surrounding
input-resolution flow to use a prototype-free object, such as one created with
Object.create(null), before the for loop assigns resolvedBindings[name].
Preserve the existing binding checks and modelParamNames behavior while
preventing special keys like __proto__ and constructor from affecting the object
prototype.
Source: Linters/SAST tools
| if (handoffArgs && typeof handoffArgs === 'object') { | ||
| for (const name of Object.keys(bindings)) { | ||
| const hasBinding = String(bindings[name] ?? '').trim().length > 0; | ||
|
|
||
| if ( | ||
| (!hasBinding || inputs[name] === undefined) && | ||
| handoffArgs[name] !== undefined | ||
| ) { | ||
| inputs[name] = handoffArgs[name]; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent prototype pollution when merging keys.
As flagged by static analysis, assigning values via computed property assignment without validating the keys allows for prototype pollution if a malicious or malformed key (like __proto__) is introduced through bindings.
Additionally, the typeof handoffArgs === 'object' check evaluates to true for arrays. It's safer to explicitly exclude arrays to prevent unintended assignment of array properties or methods (e.g., if a binding key is length or map).
🛡️ Proposed fix to sanitize property assignment
- if (handoffArgs && typeof handoffArgs === 'object') {
+ if (handoffArgs && typeof handoffArgs === 'object' && !Array.isArray(handoffArgs)) {
for (const name of Object.keys(bindings)) {
+ if (name === '__proto__' || name === 'constructor' || name === 'prototype') {
+ continue;
+ }
+
const hasBinding = String(bindings[name] ?? '').trim().length > 0;
if (📝 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.
| if (handoffArgs && typeof handoffArgs === 'object') { | |
| for (const name of Object.keys(bindings)) { | |
| const hasBinding = String(bindings[name] ?? '').trim().length > 0; | |
| if ( | |
| (!hasBinding || inputs[name] === undefined) && | |
| handoffArgs[name] !== undefined | |
| ) { | |
| inputs[name] = handoffArgs[name]; | |
| } | |
| } | |
| } | |
| if (handoffArgs && typeof handoffArgs === 'object' && !Array.isArray(handoffArgs)) { | |
| for (const name of Object.keys(bindings)) { | |
| if (name === '__proto__' || name === 'constructor' || name === 'prototype') { | |
| continue; | |
| } | |
| const hasBinding = String(bindings[name] ?? '').trim().length > 0; | |
| if ( | |
| (!hasBinding || inputs[name] === undefined) && | |
| handoffArgs[name] !== undefined | |
| ) { | |
| inputs[name] = handoffArgs[name]; | |
| } | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 47-56: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. if (key === "__proto__" || key === "constructor" || key === "prototype") continue;), use a null-prototype object (Object.create(null)), or use a safe merge utility instead.
Context: for (const name of Object.keys(bindings)) {
const hasBinding = String(bindings[name] ?? '').trim().length > 0;
if (
(!hasBinding || inputs[name] === undefined) &&
handoffArgs[name] !== undefined
) {
inputs[name] = handoffArgs[name];
}
}
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').
(prototype-pollution-recursive-merge-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/automations/src/executions/startWorkflowExecution.ts` around
lines 47 - 58, Harden the handoff merge in startWorkflowExecution by excluding
array handoffArgs and rejecting unsafe binding keys such as __proto__,
constructor, and prototype before computed assignments to inputs. Preserve the
existing binding and undefined-value precedence for valid keys, while ensuring
only own, safe properties are assigned.
Source: Linters/SAST tools
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| try { | ||
| console.log({ input }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Remove leftover debug console.log statements on the trigger path. These are development artifacts left across the nested-execution flow; they run on every trigger and dump waiting-action/target/input payloads (including potentially sensitive conversation data) to stdout.
backend/services/automations/src/trpc/automations/trigger.ts#L30-L30: removeconsole.log({ input })— logs the full trigger payload/targets (PII).backend/services/automations/src/executions/handleTrigger.ts#L54-L54: removeconsole.log({ waitingAction }).backend/services/automations/src/executions/checkIsWaitingActionTarget.ts#L96-L102: removeconsole.log({ waitingActions })andconsole.log({ waitingAction, target })(replace withdebugError/debugInfoif diagnostics are needed).
📍 Affects 3 files
backend/services/automations/src/trpc/automations/trigger.ts#L30-L30(this comment)backend/services/automations/src/executions/handleTrigger.ts#L54-L54backend/services/automations/src/executions/checkIsWaitingActionTarget.ts#L96-L102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/automations/src/trpc/automations/trigger.ts` at line 30,
Remove the leftover debug console.log statements from trigger.ts lines 30-30,
handleTrigger.ts lines 54-54, and checkIsWaitingActionTarget.ts lines 96-102,
including all waiting-action, target, and input payload logging. If diagnostics
are still required in checkIsWaitingActionTarget, use the existing debugError or
debugInfo utilities instead.
- Introduced WorkflowTemplateColumns for displaying templates in a table format. - Added WorkflowTemplateEditView for creating and editing workflow templates. - Implemented WorkflowTemplatesList to list all templates with options to create, edit, and delete. - Enhanced AutomationIndexPage to toggle between automations and templates views. - Created WorkflowTemplateDetailPage for detailed editing of a specific template. - Added utility function getNodeColor to standardize node color assignments based on type. - Updated AutomationProvider to manage editing state for workflows. - Introduced useWorkflowTemplateList hook for CRUD operations on templates. - Enhanced GraphQL mutations and queries to support template operations. - Added new routes for template creation and detail views.
| @@ -0,0 +1,49 @@ | |||
| import { AutomationNodeType } from '@/automations/types'; | |||
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts (1)
61-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid using
anyin TypeScript.As per coding guidelines, do not use the
anytype in TypeScript code; always provide explicit types. In catch blocks, exceptions default tounknown(assuming standard TSstrictdefaults), which provides better type safety and satisfies the linting requirement.
frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts#L61-L67: Remove: anyand safely stringify or access the error message (e.g.,error instanceof Error ? error.message : String(error)).frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts#L88-L94: Apply the same fix as above to remove the: anytyping.backend/plugins/payment_api/src/modules/payment/graphql/resolvers/mutations/payments.ts#L111-L113: Remove: anyfrom the catch parameter (i.e.,catch (e)); the downstreamextractErrorMessageutility is capable of handlingunknownor untyped parameters.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts` around lines 61 - 67, Remove explicit any catch-parameter annotations in both catch blocks of useWorkflowTemplates.ts at lines 61-67 and 88-94, and safely derive the toast error message from unknown errors using an Error check or String conversion. Also remove the any annotation from the catch parameter in payments.ts at lines 111-113, passing the resulting unknown value to extractErrorMessage unchanged.Source: Coding guidelines
🧹 Nitpick comments (4)
frontend/core-ui/src/modules/app/components/AutomationRoutes.tsx (1)
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove outdated routing comment.
React Router v6 and v7 use a deterministic route ranking algorithm, meaning static routes like
createwill automatically take precedence over dynamic segments like:id, regardless of the order they are defined in the file. The comment can be safely removed.♻️ Proposed refactor
- {/* Create must precede the :id route so it isn't captured as an id */}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/app/components/AutomationRoutes.tsx` at line 36, Remove the outdated JSX routing comment above the create and :id routes in AutomationRoutes, leaving the route definitions and behavior unchanged.frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplatesList.tsx (1)
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid using
anyfor error handling.As per coding guidelines, avoid introducing new
anytypes in TypeScript code. Useunknownfor the catch block error and safely extract the message.♻️ Proposed refactor
- } catch (error: any) { + } catch (error: unknown) { toast({ title: 'Failed to rename template', - description: error.message, + description: error instanceof Error ? error.message : String(error), variant: 'destructive', });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplatesList.tsx` around lines 27 - 32, Update the catch block in the template rename flow to use unknown instead of any for the error value, and safely derive a message before passing it to the toast description. Preserve the existing failure title and destructive variant.Source: Coding guidelines
frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx (2)
59-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAvoid
anyin the catch clause.
catch (error: any)bypasses type checking and letserror.messageresolve toundefinedif a non-Erroris thrown, producing an unhelpful toast description.As per coding guidelines, "Avoid introducing new
anytypes, but do not rewrite unrelated legacy types" for**/*.{ts,tsx}.♻️ Proposed fix
- } catch (error: any) { + } catch (error) { + const message = error instanceof Error ? error.message : undefined; toast({ title: templateId ? 'Failed to update template' : 'Failed to create template', - description: error.message, + description: message, variant: 'destructive', }); // Rethrow so the editor keeps the unsaved state for a retry throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx` around lines 59 - 69, Update the catch clause in the template save handler around the existing toast and rethrow logic to remove the explicit any type and safely derive the toast description from the caught value, handling non-Error throws with a meaningful fallback while preserving the existing error rethrow behavior.Source: Coding guidelines
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded template route strings duplicated across files. Both files re-derive the same
/automations/templates/:idand/automations?view=templatesliterals instead of using the existingAutomationsPathroute constants (AutomationsPath.TemplateDetail/TemplateCreate, used for routing inAutomationRoutes.tsx) or theviewparam contract fromAutomationsViewToggle.tsx. A future route change would silently break navigation in every duplicated spot.
frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx#L10-L10: deriveTEMPLATES_PATHfromAutomationsPath.Index+ theview=templatesparam instead of a raw literal.frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx#L57-L57: build the created-template URL fromAutomationsPath.TemplateDetail(e.g. viageneratePath) rather than a template literal.frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateColumns.tsx#L48-L53: use the sameAutomationsPath.TemplateDetailhelper for the Edit menu item's navigation.frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateColumns.tsx#L108-L110: use the same helper for the "name" cell's anchor navigation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx` at line 10, Replace duplicated automation template route literals with the existing AutomationsPath helpers. In frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx:10-10, derive TEMPLATES_PATH from AutomationsPath.Index and the established templates view parameter; at 57-57, generate the created-template URL from AutomationsPath.TemplateDetail. In frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateColumns.tsx:48-53 and 108-110, use the same AutomationsPath.TemplateDetail generation helper for Edit navigation and the name anchor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditor.tsx`:
- Around line 137-166: Guard handleSave against concurrent executions by
tracking an in-flight save and returning immediately when one is already active;
clear the guard after onSave resolves or rejects. Propagate the resulting
isSaving state through WorkflowEditorHeader and disable its Save button while
saving, including the WorkflowTemplateEditView integration so its mutation
cannot be submitted twice.
In
`@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditView.tsx`:
- Around line 42-56: Update the handleSave logic in WorkflowEditView to always
write the current inputs value into config, using an empty object when inputs is
absent or cleared, instead of conditionally spreading inputs. Preserve the
existing workflow config merge and ensure saving after removing all bindings
overwrites stale inputs.
---
Outside diff comments:
In
`@frontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.ts`:
- Around line 61-67: Remove explicit any catch-parameter annotations in both
catch blocks of useWorkflowTemplates.ts at lines 61-67 and 88-94, and safely
derive the toast error message from unknown errors using an Error check or
String conversion. Also remove the any annotation from the catch parameter in
payments.ts at lines 111-113, passing the resulting unknown value to
extractErrorMessage unchanged.
---
Nitpick comments:
In `@frontend/core-ui/src/modules/app/components/AutomationRoutes.tsx`:
- Line 36: Remove the outdated JSX routing comment above the create and :id
routes in AutomationRoutes, leaving the route definitions and behavior
unchanged.
In
`@frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx`:
- Around line 59-69: Update the catch clause in the template save handler around
the existing toast and rethrow logic to remove the explicit any type and safely
derive the toast description from the caught value, handling non-Error throws
with a meaningful fallback while preserving the existing error rethrow behavior.
- Line 10: Replace duplicated automation template route literals with the
existing AutomationsPath helpers. In
frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsx:10-10,
derive TEMPLATES_PATH from AutomationsPath.Index and the established templates
view parameter; at 57-57, generate the created-template URL from
AutomationsPath.TemplateDetail. In
frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateColumns.tsx:48-53
and 108-110, use the same AutomationsPath.TemplateDetail generation helper for
Edit navigation and the name anchor.
In
`@frontend/core-ui/src/modules/automations/components/templates/WorkflowTemplatesList.tsx`:
- Around line 27-32: Update the catch block in the template rename flow to use
unknown instead of any for the error value, and safely derive a message before
passing it to the toast description. Preserve the existing failure title and
destructive variant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a164e143-ad25-408a-8899-fbe68d46cedb
📒 Files selected for processing (41)
backend/core-api/src/modules/automations/graphql/resolvers/mutations.tsbackend/erxes-api-shared/src/core-modules/automations/constants.tsbackend/plugins/frontline_api/src/modules/integrations/facebook/meta/automation/messages/index.tsbackend/plugins/payment_api/src/modules/payment/graphql/resolvers/mutations/payments.tsfrontend/core-ui/src/modules/app/components/AutomationRoutes.tsxfrontend/core-ui/src/modules/automations/components/builder/AutomationBuilder.tsxfrontend/core-ui/src/modules/automations/components/builder/AutomationBuilderCanvas.tsxfrontend/core-ui/src/modules/automations/components/builder/AutomationBuilderControls.tsxfrontend/core-ui/src/modules/automations/components/builder/AutomationBuilderWorkspace.tsxfrontend/core-ui/src/modules/automations/components/builder/MarqueeSelectionPanel.tsxfrontend/core-ui/src/modules/automations/components/builder/header/AutomationBuilderHeaderActions.tsxfrontend/core-ui/src/modules/automations/components/builder/history/components/AutomationHistoryByFlow.tsxfrontend/core-ui/src/modules/automations/components/builder/hooks/useInsertWorkflowTemplate.tsfrontend/core-ui/src/modules/automations/components/builder/hooks/useRemoveSelectedNodes.tsfrontend/core-ui/src/modules/automations/components/builder/hooks/useWorkflowTemplates.tsfrontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowActionMapper.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditView.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditor.tsxfrontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowNode.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/components/AutomationBuilderPrimarySidebar.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/components/AutomationVariableBrowserHeader.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/components/AutomationVariableSourceNodeList.tsxfrontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useAutomationActionContentSidebar.tsfrontend/core-ui/src/modules/automations/components/list/AutomationsRecordTable.tsxfrontend/core-ui/src/modules/automations/components/list/AutomationsViewToggle.tsxfrontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateColumns.tsxfrontend/core-ui/src/modules/automations/components/templates/WorkflowTemplateEditView.tsxfrontend/core-ui/src/modules/automations/components/templates/WorkflowTemplatesList.tsxfrontend/core-ui/src/modules/automations/constants/index.tsfrontend/core-ui/src/modules/automations/context/AutomationProvider.tsxfrontend/core-ui/src/modules/automations/graphql/automationMutations.tsfrontend/core-ui/src/modules/automations/graphql/automationQueries.tsfrontend/core-ui/src/modules/automations/hooks/useReactFlowEditor.tsfrontend/core-ui/src/modules/automations/hooks/useWorkflowTemplateList.tsfrontend/core-ui/src/modules/automations/utils/automationBuilderUtils/connectionUtils.tsfrontend/core-ui/src/modules/automations/utils/automationBuilderUtils/getNodeColor.tsfrontend/core-ui/src/modules/automations/utils/automationConnectionUtils.tsfrontend/core-ui/src/modules/types/paths/AutomationPath.tsfrontend/core-ui/src/pages/automations/AutomationIndexPage.tsxfrontend/core-ui/src/pages/automations/WorkflowTemplateDetailPage.tsx
💤 Files with no reviewable changes (1)
- frontend/core-ui/src/modules/automations/components/builder/AutomationBuilder.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- backend/erxes-api-shared/src/core-modules/automations/constants.ts
- frontend/core-ui/src/modules/automations/graphql/automationMutations.ts
- backend/plugins/frontline_api/src/modules/integrations/facebook/meta/automation/messages/index.ts
- frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/components/AutomationVariableBrowserHeader.tsx
- frontend/core-ui/src/modules/automations/components/builder/sidebar/components/output-variables/components/AutomationVariableSourceNodeList.tsx
- frontend/core-ui/src/modules/automations/hooks/useReactFlowEditor.ts
- frontend/core-ui/src/modules/automations/components/builder/AutomationBuilderCanvas.tsx
- frontend/core-ui/src/modules/automations/utils/automationConnectionUtils.ts
- frontend/core-ui/src/modules/automations/components/builder/sidebar/components/AutomationBuilderPrimarySidebar.tsx
- frontend/core-ui/src/modules/automations/graphql/automationQueries.ts
- frontend/core-ui/src/modules/automations/components/builder/hooks/useInsertWorkflowTemplate.ts
- frontend/core-ui/src/modules/automations/context/AutomationProvider.tsx
- frontend/core-ui/src/modules/automations/components/builder/sidebar/components/library/NodeLibraryRow.tsx
- frontend/core-ui/src/modules/automations/components/builder/sidebar/hooks/useAutomationActionContentSidebar.ts
| const handleSave = useCallback(async () => { | ||
| const actions = innerForm.getValues('actions') || []; | ||
| // The stored entry may have been deleted while editing; fall back to the | ||
| // first member so the workflow always has an entry point. | ||
| const entryActionId = actions.some( | ||
| ({ id }) => id === initial.entryActionId, | ||
| ) | ||
| ? initial.entryActionId | ||
| : actions[0]?.id; | ||
|
|
||
| try { | ||
| await onSave({ | ||
| name: meta.name, | ||
| description: meta.description, | ||
| actions, | ||
| entryActionId, | ||
| inputs, | ||
| }); | ||
| setSavedSnapshot( | ||
| snapshotOf({ | ||
| name: meta.name, | ||
| description: meta.description, | ||
| actions, | ||
| inputs, | ||
| }), | ||
| ); | ||
| } catch { | ||
| // The caller surfaces the failure; stay dirty so the edits aren't lost | ||
| } | ||
| }, [innerForm, initial.entryActionId, inputs, meta, onSave]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Save button has no double-submit guard.
handleSave is async and wired directly to the header Button with no disabled state or re-entrancy guard. Rapid double-clicks (or a slow network) can fire onSave twice; for WorkflowTemplateEditView.tsx this can create two templates from a single "Save" intent since its saving/updatingTemplate mutation-loading flags are never consulted here.
🔒 Proposed fix: guard against concurrent saves
+ const [isSaving, setIsSaving] = useState(false);
+
const handleSave = useCallback(async () => {
+ if (isSaving) {
+ return;
+ }
+ setIsSaving(true);
const actions = innerForm.getValues('actions') || [];
const entryActionId = actions.some(
({ id }) => id === initial.entryActionId,
)
? initial.entryActionId
: actions[0]?.id;
try {
await onSave({
name: meta.name,
description: meta.description,
actions,
entryActionId,
inputs,
});
setSavedSnapshot(
snapshotOf({
name: meta.name,
description: meta.description,
actions,
inputs,
}),
);
} catch {
// The caller surfaces the failure; stay dirty so the edits aren't lost
+ } finally {
+ setIsSaving(false);
}
- }, [innerForm, initial.entryActionId, inputs, meta, onSave]);
+ }, [innerForm, initial.entryActionId, inputs, isSaving, meta, onSave]);Then pass isSaving to WorkflowEditorHeader and use it to disable the Save button:
- <Button onClick={onSave}>{saveLabel}</Button>
+ <Button onClick={onSave} disabled={isSaving}>{saveLabel}</Button>Also applies to: 294-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditor.tsx`
around lines 137 - 166, Guard handleSave against concurrent executions by
tracking an in-flight save and returning immediately when one is already active;
clear the guard after onSave resolves or rejects. Propagate the resulting
isSaving state through WorkflowEditorHeader and disable its Save button while
saving, including the WorkflowTemplateEditView integration so its mutation
cannot be submitted twice.
| setWorkflowValue('name', name); | ||
| setWorkflowValue('description', description ?? ''); | ||
| setWorkflowValue('actions', actions); | ||
| setWorkflowValue('config', { | ||
| ...(workflow?.config || {}), | ||
| entryActionId, | ||
| ...(inputs && Object.keys(inputs).length ? { inputs } : {}), | ||
| }); | ||
|
|
||
| if (hasTemplate) { | ||
| setPromptOpen(true); | ||
| } | ||
| }, | ||
| [workflowIndex, outerForm, workflow?.config, hasTemplate, setPromptOpen], | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clearing all input bindings doesn't persist — stale inputs survive the merge.
...(inputs && Object.keys(inputs).length ? { inputs } : {}) is skipped once inputs becomes {}, so the earlier spread of workflow?.config (which still has the old, non-empty inputs) wins. A user who removes all bindings and saves will find them still there. WorkflowTemplateEditView.tsx's handleSave sets inputs: inputs || {} unconditionally and doesn't have this problem.
🐛 Proposed fix
setWorkflowValue('config', {
...(workflow?.config || {}),
entryActionId,
- ...(inputs && Object.keys(inputs).length ? { inputs } : {}),
+ inputs: inputs || {},
});📝 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.
| setWorkflowValue('name', name); | |
| setWorkflowValue('description', description ?? ''); | |
| setWorkflowValue('actions', actions); | |
| setWorkflowValue('config', { | |
| ...(workflow?.config || {}), | |
| entryActionId, | |
| ...(inputs && Object.keys(inputs).length ? { inputs } : {}), | |
| }); | |
| if (hasTemplate) { | |
| setPromptOpen(true); | |
| } | |
| }, | |
| [workflowIndex, outerForm, workflow?.config, hasTemplate, setPromptOpen], | |
| ); | |
| setWorkflowValue('name', name); | |
| setWorkflowValue('description', description ?? ''); | |
| setWorkflowValue('actions', actions); | |
| setWorkflowValue('config', { | |
| ...(workflow?.config || {}), | |
| entryActionId, | |
| inputs: inputs || {}, | |
| }); | |
| if (hasTemplate) { | |
| setPromptOpen(true); | |
| } | |
| }, | |
| [workflowIndex, outerForm, workflow?.config, hasTemplate, setPromptOpen], | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/core-ui/src/modules/automations/components/builder/nodes/components/WorkflowEditView.tsx`
around lines 42 - 56, Update the handleSave logic in WorkflowEditView to always
write the current inputs value into config, using an empty object when inputs is
absent or cleared, instead of conditionally spreading inputs. Preserve the
existing workflow config merge and ensure saving after removing all bindings
overwrites stale inputs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/erxes-api-shared/src/core-modules/automations/zodTypes.ts`:
- Around line 36-38: Update the startWaitingDate field in
AutomationExecutionInput to preprocess or transform incoming ISO strings into
Date instances before z.date() validation, while continuing to accept in-process
Date values and remain optional. Ensure ReceiveActionsInput and
SetPropertiesInput can parse serialized automation execution payloads
successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91a4cfe9-34ac-44a5-b009-0b41ddbb29cf
📒 Files selected for processing (2)
backend/erxes-api-shared/src/core-modules/automations/zodTypes.tsfrontend/core-ui/src/modules/automations/components/list/AutomationsRecordTable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/core-ui/src/modules/automations/components/list/AutomationsRecordTable.tsx
| // Arrives as a Date in-process but as an ISO string after crossing the | ||
| // queue/producer JSON boundary (same reason createdAt is a string above) | ||
| startWaitingDate: z.date().optional(), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 3 'AutomationExecutionInput|startWaitingDate|setExecutionWaitAction' \
backend/erxes-api-shared backend/services/automationsRepository: erxes/erxes
Length of output: 15944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' backend/erxes-api-shared/src/core-modules/automations/zodTypes.ts
printf '\n----\n'
sed -n '1,140p' backend/services/automations/src/bullmq/actionHandlerWorker/setWait.ts
printf '\n----\n'
sed -n '1,120p' backend/services/automations/src/mongo/waitingActionsToExecute.ts
printf '\n----\n'
rg -n -C 3 'waitingActionsToExecute|AutomationExecutionInput|parse\(|safeParse\(|zodTypes' backend/services/automations backend/erxes-api-sharedRepository: erxes/erxes
Length of output: 39368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' backend/erxes-api-shared/src/core-modules/automations/setupAutomationProducers.ts
printf '\n----\n'
sed -n '1,220p' backend/erxes-api-shared/src/utils/trpc/sendCoreModuleProducer.ts
printf '\n----\n'
rg -n -C 3 'AutomationExecutionInput\.|ReceiveActionsInput|SetPropertiesInput|startWaitingDate|sendWorkerQueue\\(' backend/erxes-api-shared backend/services/automationsRepository: erxes/erxes
Length of output: 9838
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' backend/erxes-api-shared/src/core-modules/automations/types.ts
printf '\n----\n'
rg -n -C 4 'sendCoreModuleProducer\\(|ReceiveActionsInput|SetPropertiesInput|execution:' backend/services/automations backend/erxes-api-shared/src/core-modules/automationsRepository: erxes/erxes
Length of output: 5562
Normalize startWaitingDate before z.date().
AutomationExecutionInput crosses the tRPC/JSON boundary, so a Date becomes an ISO string before ReceiveActionsInput and SetPropertiesInput parse it. z.date() rejects that payload, which will break valid automation executions. Accept/transform the string here, or preprocess it to a Date.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/erxes-api-shared/src/core-modules/automations/zodTypes.ts` around
lines 36 - 38, Update the startWaitingDate field in AutomationExecutionInput to
preprocess or transform incoming ISO strings into Date instances before z.date()
validation, while continuing to accept in-process Date values and remain
optional. Ensure ReceiveActionsInput and SetPropertiesInput can parse serialized
automation execution payloads successfully.
|
1 participant
WorkflowEditScopeProviderandWorkflowNodeProviderfor better state management in workflow editing.AutomationProviderto support scoped providers.Summary by CodeRabbit