Add flow to run output #10220
Add flow to run output #10220
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR enhances workflow run history by storing workflow version triggers and steps directly in the run output, ensuring runs remain accurate even if draft versions are modified later.
- Added
flowfield inWorkflowRunOutputto store version trigger and steps in/packages/twenty-server/src/modules/workflow/common/standard-objects/workflow-run.workspace-entity.ts - Renamed
stepstostepsOutputin output structures for clarity and separation of concerns - Added
WORKFLOW_RUN_INVALIDexception code in/packages/twenty-server/src/modules/workflow/workflow-runner/exceptions/workflow-run.exception.ts - Modified
startWorkflowRunto include initial flow data in/packages/twenty-server/src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service.ts - Removed Logger dependency from workflow execution services for simplified error handling
5 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
.../twenty-server/src/modules/workflow/common/standard-objects/workflow-run.workspace-entity.ts
Show resolved
Hide resolved
...y-server/src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service.ts
Show resolved
Hide resolved
| export type WorkflowExecutorState = { | ||
| flow: WorkflowRunOutput['flow']; | ||
| stepsOutput: WorkflowRunOutput['stepsOutput']; | ||
| status: WorkflowRunStatus; | ||
| }; |
There was a problem hiding this comment.
logic: flow field is defined but never populated in the executor state
| await this.workflowRunWorkspaceService.saveWorkflowRunState({ | ||
| workflowRunId, | ||
| output: { | ||
| steps: updatedOutputSteps, | ||
| stepsOutput: updatedStepsOutput, | ||
| }, | ||
| context: updatedContext, | ||
| }); |
There was a problem hiding this comment.
logic: flow field is missing in saveWorkflowRunState calls, which could lead to incomplete state persistence
| trigger: WorkflowTrigger; | ||
| steps: WorkflowAction[]; | ||
| }; | ||
| stepsOutput: Record<string, StepRunOutput>; |
| flow: { | ||
| trigger: workflowVersion.trigger, | ||
| steps: workflowVersion.steps, | ||
| }, | ||
| stepsOutput: {}, |
| output: { | ||
| flow: workflowRunToUpdate.output?.flow ?? { | ||
| trigger: undefined, | ||
| steps: [], | ||
| }, |
There was a problem hiding this comment.
I feel like the workflowRunToUpdate.output?.flow should never be undefined. What do you think?
There was a problem hiding this comment.
it can actually. When the run is not started yet. We should only store the flow once started because this is the version we'll execute
We need the version trigger and steps to be stored in the output. We should not rely on the version itself because some run are made on draft versions. Which means versions could be edited afterwards.