fix: resolve CODE step result variables in workflow templates#19064
fix: resolve CODE step result variables in workflow templates#19064jnMetaCode wants to merge 1 commit intotwentyhq:mainfrom
Conversation
…tion
When a CODE step produces output, the workflow run context now exposes
the result both at the top level (e.g. {{step.field}}) and under a
result key (e.g. {{step.result.field}}). This ensures that template
variables referencing CODE step outputs via the result path are
correctly substituted instead of rendering as undefined.
Fixes twentyhq#17109
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@jnMetaCode Please stop opening AI generated PR on this repository. You are not helping us and harming the community. This will be the only warning before ban: one more and that's it |
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
| if (typeof result === 'object' && result !== null) { | ||
| return [key, { ...result, result }]; | ||
| } |
There was a problem hiding this comment.
Bug: The spread syntax { ...result, result } in getWorkflowRunContext overwrites the result property of a step's output object, causing data loss if that key already exists.
Severity: HIGH
Suggested Fix
To preserve the original result property if it exists, reverse the order of the properties in the object literal to { result, ...result }. This ensures that the spread properties from the result object will overwrite the initial result key, effectively preserving the original key-value pair from the step's output.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/twenty-shared/src/workflow/utils/getWorkflowRunContext.ts#L13-L15
Potential issue: In the `getWorkflowRunContext` function, when a step's output
(`result`) is an object, the code uses `{ ...result, result }` to construct the new
context. If the original `result` object already contains a property named `result`,
this syntax overwrites that property's value with the entire `result` object itself. For
example, an output of `{ "result": "success", "userId": 123 }` is incorrectly
transformed into `{ "userId": 123, "result": { "result": "success", "userId": 123 } }`.
This leads to silent data corruption, as the original value of the `result` property is
lost, which can break subsequent workflow steps that depend on it.
Did we get this right? 👍 / 👎 to inform future reviews.
Summary
{{step.result.field}}pathgetWorkflowRunContextfunction now exposes step results both at the top level ({{step.field}}) and under aresultkey ({{step.result.field}}), so both access patterns resolve correctlygetWorkflowRunContextandvariable-resolverProblem
When a CODE step in a workflow produces output (e.g.,
{ memberName: "Mike Chen", eventName: "Microsoft Gala" }), subsequent steps like SEND_EMAIL that reference these values using{{transform-step-001.result.memberName}}getundefinedinstead of the actual value. This is becausegetWorkflowRunContextstrips theresultwrapper from step outputs, so only{{step.field}}works but{{step.result.field}}does not.Fix
Modified
getWorkflowRunContextinpackages/twenty-shared/src/workflow/utils/getWorkflowRunContext.tsto spread the result object at the top level AND include it under aresultkey. This preserves backward compatibility with existing UI-generated variables ({{step.field}}) while also supporting the{{step.result.field}}path that users naturally write based on the workflow run state structure.Test plan
{{step.field}}still resolves correctly (backward compatibility){{step.result.field}}now resolves correctly{{step.result.a}} and {{step.result.b}}workgetWorkflowRunContextandvariable-resolverFixes #17109
🤖 Generated with Claude Code