feat: Implement Gmail node executor and introduce workflow testing an…#75
Merged
Conversation
…d configuration UI components.
📝 WalkthroughWalkthroughA testing feature is introduced that allows users to run and display test results for workflow nodes. A new TestPanel component displays formatted test outputs, VariablePanel gains testing capabilities, ConfigModal integrates the testing workflow with result storage, and the Gmail executor's success output is restructured to include metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigModal
participant VariablePanel
participant Workflow Engine
participant Redux Store
participant TestPanel
User->>ConfigModal: Selects node to test
User->>VariablePanel: Clicks "Test Node" button
VariablePanel->>ConfigModal: onTestNode(nodeId)
ConfigModal->>Workflow Engine: handleTestPreviousNode:<br/>resolve config + execute node
Workflow Engine-->>ConfigModal: testResult + metadata
ConfigModal->>Redux Store: Store testResult<br/>(nodeName, nodeIcon, output)
ConfigModal->>TestPanel: Render with testResult
TestPanel->>TestPanel: Route rendering logic:<br/>strings → preformatted<br/>arrays → tables<br/>objects → formatted display
TestPanel-->>User: Display formatted test output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/components/ui/TestPanel.tsx`:
- Around line 51-80: renderSpreadsheetTable currently maps every returned row
which can freeze the UI for large Sheets results; limit the preview by slicing
rows before rendering (e.g., keep headers plus at most N data rows) or implement
virtualization. Specifically, in renderSpreadsheetTable, compute a boundedRows
or previewRows from rows (use the first row as headers and slice dataRows to a
MAX_PREVIEW_ROWS constant consistent with VariablePanel /
renderArrayOfObjectsTable), then use those bounded values in the table rendering
and optionally show a count/truncation indicator.
In `@apps/web/app/components/ui/variable-panel.tsx`:
- Around line 80-98: The interactive mapping headers/cells (the div using
activeField, isTested, formattedNodeName, strictConcatPath and invoking
handleInsert) are currently mouse-only; replace the non-focusable divs/ths/tds
with semantic <button> elements where possible or, if structural constraints
prevent that, add tabIndex=0, role="button", aria-disabled when !isTested or
!activeField, and an onKeyDown handler that invokes the same insertion/expansion
logic when Enter or Space is pressed (call the same handleInsert logic and
stopPropagation). Ensure the title text and visual disabled styling remain, and
apply the same changes to the other similar blocks referenced (around the usages
at lines you noted) so keyboard users can focus and activate these controls.
- Around line 80-97: The UI currently blocks insertion whenever isTested is
false, which prevents schema-backed variables from being inserted; change the
click/hover/title checks to allow insertion when the node has a schema fallback
(e.g., variables or outputSchema present) even if not isTested. Concretely,
compute a boolean like hasSchemaFallback = Boolean(node.variables?.length ||
node.outputSchema) and replace occurrences of checks that require isTested (used
with activeField, handleInsert, and the title logic around
formattedNodeName/strictConcatPath) with (isTested || hasSchemaFallback); apply
the same change to the other similar conditional at the referenced block (lines
~403-405) so schema-backed rows remain insertable for untested nodes.
- Around line 249-279: The drag-and-drop and click handlers build dot-access
expressions like `{{${formattedNodeName}.map(item => item.${header})}}` which
break for keys that are not valid identifiers; update the construction in the
onDragStart handler and the onClick/handleInsert call to use bracket notation
when needed (e.g., use item[headerString] or `${formattedNodeName}.map(item =>
item[${quotedHeader}])`) by testing header against a JS identifier regex and
quoting/escaping it when using bracket form; adjust the title string generation
similarly so it shows a valid expression; touch the same logic around the other
similar block (the block referenced at lines ~298-309) so both drag click and
title use bracket notation for non-identifier keys.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 42-47: The useEffect that currently only clears testResult when
selectedNode?.id changes should also reset other node-scoped modal state (clear
activeField and dynamicOptions) so no stale field or options from the previous
node remain; locate the state variables testResult, activeField, dynamicOptions
and update the effect that depends on selectedNode?.id to set them all to their
initial values, and change TestPanel usage to derive its displayed status from
nodeTestOutput (from Redux) instead of local testResult so the panel reflects
Redux updates correctly.
- Around line 433-435: The option value expression in ConfigModal's render
(inside options.map) is using incorrect precedence so items with both value and
id pick id; change the expression used for the <option value=...> to use a
nullish/coalescing chain (e.g., prefer opt.value, then opt.id, then the raw opt)
so the saved config matches the selected option; update the value expression in
the options.map callback in ConfigModal.tsx accordingly.
In `@apps/web/app/workflows/`[id]/page.tsx:
- Line 617: The test callbacks currently pass stale identifier properties (e.g.,
Trigger.TriggerId / node.NodeId) which breaks after rehydration; update the
callbacks that call testNodeFromCanvas to derive a single canonical testId
(e.g., const testId = actionId ?? action.id ?? trigger.id ?? node.id) and pass
that testId into testNodeFromCanvas (instead of Trigger.TriggerId/node.NodeId).
Locate all occurrences where testNodeFromCanvas is invoked (including the
existing onTest: ()=> testNodeFromCanvas(actionId, action.name, "action") usage
and the other callbacks that reference Trigger.TriggerId or node.NodeId) and
replace them to use the canonical testId so reopened workflows use the correct
id path.
In `@packages/nodes/src/gmail/gmail.executor.ts`:
- Around line 60-71: The current return always sets output.status to "sent" even
when result.success is false; update the return so output is only populated on
success (using result.success) and on failure either omit output or set a
failure-aware object (e.g., status: "failed" plus error details). Locate the
return block that references result.success, output, messageId, threadId, to,
and subject in gmail.executor.ts and wrap the output population behind a
conditional: when result.success === true include the sent metadata (status:
"sent", messageId, threadId, to, subject, summary), otherwise set output to null
or a small failure object containing status: "failed" and result.error to avoid
misleading downstream nodes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e916fe66-4469-4578-a003-461976dbe061
📒 Files selected for processing (7)
.gitignoreapps/web/app/components/ui/TestPanel.tsxapps/web/app/components/ui/variable-panel.tsxapps/web/app/workflows/[id]/components/ConfigModal.tsxapps/web/app/workflows/[id]/page.tsxpackages/nodes/src/gmail/gmail.executor.tspackages/nodes/tsconfig.tsbuildinfo
💤 Files with no reviewable changes (1)
- packages/nodes/tsconfig.tsbuildinfo
Comment on lines
+51
to
+80
| const renderSpreadsheetTable = (rows: any[][]) => { | ||
| if (rows.length === 0) return <EmptyState message="No rows in response" />; | ||
| const headers = rows[0] as string[]; | ||
| const dataRows = rows.slice(1); | ||
|
|
||
| return ( | ||
| <div className="w-full border border-[#2a2f3e] rounded-lg overflow-hidden overflow-x-auto"> | ||
| <table className="w-full text-left text-xs text-gray-300 min-w-max border-collapse"> | ||
| <thead className="bg-[#161b22] sticky top-0"> | ||
| <tr> | ||
| <th className="px-3 py-2.5 w-10 text-center border-r border-b border-[#2a2f3e] text-gray-500 font-normal text-[10px]">#</th> | ||
| {headers.map((h, i) => ( | ||
| <th key={i} className="px-4 py-2.5 border-r border-b border-[#2a2f3e] font-medium text-blue-300 truncate max-w-[160px]"> | ||
| {h || `Column ${i + 1}`} | ||
| </th> | ||
| ))} | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {dataRows.map((row, ri) => ( | ||
| <tr key={ri} className="border-b border-[#1a1f2e]/50 hover:bg-[#1f2536] transition-colors"> | ||
| <td className="px-3 py-2 text-center border-r border-[#1a1f2e]/50 text-gray-600 bg-[#161b26] text-[10px]">{ri + 1}</td> | ||
| {headers.map((_, ci) => ( | ||
| <td key={ci} className="px-4 py-2 border-r border-[#1a1f2e]/50 truncate max-w-[200px] text-gray-300"> | ||
| {String(row[ci] ?? '')} | ||
| </td> | ||
| ))} | ||
| </tr> | ||
| ))} | ||
| </tbody> |
There was a problem hiding this comment.
Limit spreadsheet previews before rendering.
renderSpreadsheetTable maps every returned row, unlike renderArrayOfObjectsTable and the preview limits in VariablePanel. A large Sheets test result can lock the modal as soon as it opens; slice or virtualize this branch before rendering.
♻️ Suggested fix
- const dataRows = rows.slice(1);
+ const PREVIEW_ROW_LIMIT = 100;
+ const dataRows = rows.slice(1, PREVIEW_ROW_LIMIT + 1);📝 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.
Suggested change
| const renderSpreadsheetTable = (rows: any[][]) => { | |
| if (rows.length === 0) return <EmptyState message="No rows in response" />; | |
| const headers = rows[0] as string[]; | |
| const dataRows = rows.slice(1); | |
| return ( | |
| <div className="w-full border border-[#2a2f3e] rounded-lg overflow-hidden overflow-x-auto"> | |
| <table className="w-full text-left text-xs text-gray-300 min-w-max border-collapse"> | |
| <thead className="bg-[#161b22] sticky top-0"> | |
| <tr> | |
| <th className="px-3 py-2.5 w-10 text-center border-r border-b border-[#2a2f3e] text-gray-500 font-normal text-[10px]">#</th> | |
| {headers.map((h, i) => ( | |
| <th key={i} className="px-4 py-2.5 border-r border-b border-[#2a2f3e] font-medium text-blue-300 truncate max-w-[160px]"> | |
| {h || `Column ${i + 1}`} | |
| </th> | |
| ))} | |
| </tr> | |
| </thead> | |
| <tbody> | |
| {dataRows.map((row, ri) => ( | |
| <tr key={ri} className="border-b border-[#1a1f2e]/50 hover:bg-[#1f2536] transition-colors"> | |
| <td className="px-3 py-2 text-center border-r border-[#1a1f2e]/50 text-gray-600 bg-[#161b26] text-[10px]">{ri + 1}</td> | |
| {headers.map((_, ci) => ( | |
| <td key={ci} className="px-4 py-2 border-r border-[#1a1f2e]/50 truncate max-w-[200px] text-gray-300"> | |
| {String(row[ci] ?? '')} | |
| </td> | |
| ))} | |
| </tr> | |
| ))} | |
| </tbody> | |
| const renderSpreadsheetTable = (rows: any[][]) => { | |
| if (rows.length === 0) return <EmptyState message="No rows in response" />; | |
| const headers = rows[0] as string[]; | |
| const PREVIEW_ROW_LIMIT = 100; | |
| const dataRows = rows.slice(1, PREVIEW_ROW_LIMIT + 1); | |
| return ( | |
| <div className="w-full border border-[`#2a2f3e`] rounded-lg overflow-hidden overflow-x-auto"> | |
| <table className="w-full text-left text-xs text-gray-300 min-w-max border-collapse"> | |
| <thead className="bg-[`#161b22`] sticky top-0"> | |
| <tr> | |
| <th className="px-3 py-2.5 w-10 text-center border-r border-b border-[`#2a2f3e`] text-gray-500 font-normal text-[10px]">#</th> | |
| {headers.map((h, i) => ( | |
| <th key={i} className="px-4 py-2.5 border-r border-b border-[`#2a2f3e`] font-medium text-blue-300 truncate max-w-[160px]"> | |
| {h || `Column ${i + 1}`} | |
| </th> | |
| ))} | |
| </tr> | |
| </thead> | |
| <tbody> | |
| {dataRows.map((row, ri) => ( | |
| <tr key={ri} className="border-b border-[`#1a1f2e`]/50 hover:bg-[`#1f2536`] transition-colors"> | |
| <td className="px-3 py-2 text-center border-r border-[`#1a1f2e`]/50 text-gray-600 bg-[`#161b26`] text-[10px]">{ri + 1}</td> | |
| {headers.map((_, ci) => ( | |
| <td key={ci} className="px-4 py-2 border-r border-[`#1a1f2e`]/50 truncate max-w-[200px] text-gray-300"> | |
| {String(row[ci] ?? '')} | |
| </td> | |
| ))} | |
| </tr> | |
| ))} | |
| </tbody> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/components/ui/TestPanel.tsx` around lines 51 - 80,
renderSpreadsheetTable currently maps every returned row which can freeze the UI
for large Sheets results; limit the preview by slicing rows before rendering
(e.g., keep headers plus at most N data rows) or implement virtualization.
Specifically, in renderSpreadsheetTable, compute a boundedRows or previewRows
from rows (use the first row as headers and slice dataRows to a MAX_PREVIEW_ROWS
constant consistent with VariablePanel / renderArrayOfObjectsTable), then use
those bounded values in the table rendering and optionally show a
count/truncation indicator.
Comment on lines
+80
to
+98
| <div | ||
| className={` | ||
| flex items-center group transition-colors border-b border-[#1a1f2e]/50 | ||
| ${activeField && isTested ? "hover:bg-[#202737] cursor-pointer" : "cursor-not-allowed opacity-50"} | ||
| `} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| if (activeField && isTested) { | ||
| handleInsert(`{{${formattedNodeName}.${strictConcatPath}}}`); | ||
| } | ||
| }} | ||
| title={ | ||
| !isTested | ||
| ? "Test node first to map data" | ||
| : activeField | ||
| ? `Insert {{${formattedNodeName}.${strictConcatPath}}}` | ||
| : "Select a field first" | ||
| } | ||
| > |
There was a problem hiding this comment.
The new mapping interactions are mouse-only.
These div/th/td click targets are not focusable and have no keyboard equivalent, so keyboard users cannot expand nodes or insert variables. Please use real buttons where possible, or add tabIndex, role, and onKeyDown handling to the interactive headers and cells.
Also applies to: 165-170, 183-189, 249-279, 298-309, 380-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 80 - 98, The
interactive mapping headers/cells (the div using activeField, isTested,
formattedNodeName, strictConcatPath and invoking handleInsert) are currently
mouse-only; replace the non-focusable divs/ths/tds with semantic <button>
elements where possible or, if structural constraints prevent that, add
tabIndex=0, role="button", aria-disabled when !isTested or !activeField, and an
onKeyDown handler that invokes the same insertion/expansion logic when Enter or
Space is pressed (call the same handleInsert logic and stopPropagation). Ensure
the title text and visual disabled styling remain, and apply the same changes to
the other similar blocks referenced (around the usages at lines you noted) so
keyboard users can focus and activate these controls.
Comment on lines
+80
to
+97
| <div | ||
| className={` | ||
| flex items-center group transition-colors border-b border-[#1a1f2e]/50 | ||
| ${activeField && isTested ? "hover:bg-[#202737] cursor-pointer" : "cursor-not-allowed opacity-50"} | ||
| `} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| if (activeField && isTested) { | ||
| handleInsert(`{{${formattedNodeName}.${strictConcatPath}}}`); | ||
| } | ||
| }} | ||
| title={ | ||
| !isTested | ||
| ? "Test node first to map data" | ||
| : activeField | ||
| ? `Insert {{${formattedNodeName}.${strictConcatPath}}}` | ||
| : "Select a field first" | ||
| } |
There was a problem hiding this comment.
Keep schema-backed variables insertable.
previousNodes still arrives with variables populated from each node's static outputSchema, and this card now says "Using sample schema", but the new isTested checks make every row read-only until the node has been executed once. That turns the schema fallback into dead UI for untested nodes. Please only require testing when a node has no schema to insert from.
Also applies to: 403-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 80 - 97, The UI
currently blocks insertion whenever isTested is false, which prevents
schema-backed variables from being inserted; change the click/hover/title checks
to allow insertion when the node has a schema fallback (e.g., variables or
outputSchema present) even if not isTested. Concretely, compute a boolean like
hasSchemaFallback = Boolean(node.variables?.length || node.outputSchema) and
replace occurrences of checks that require isTested (used with activeField,
handleInsert, and the title logic around formattedNodeName/strictConcatPath)
with (isTested || hasSchemaFallback); apply the same change to the other similar
conditional at the referenced block (lines ~403-405) so schema-backed rows
remain insertable for untested nodes.
Comment on lines
+249
to
+279
| draggable={true} | ||
| onDragStart={(e) => { | ||
| const variableText = `{{${formattedNodeName}.map(item => item.${header})}}`; | ||
|
|
||
| e.dataTransfer.setData('text/plain', variableText); | ||
|
|
||
| e.dataTransfer.setData('application/buildflow-variable', JSON.stringify({ | ||
| nodeName: formattedNodeName, | ||
| path: header, | ||
| display: `${formattedNodeName}.${header}` | ||
| })) | ||
|
|
||
| e.dataTransfer.effectAllowed = 'copy'; | ||
|
|
||
| }} | ||
| key={header} | ||
| className={`px-3 py-2 border-r border-b border-[#2a2f3e] font-medium text-gray-400 truncate max-w-[150px] transition-colors | ||
| ${activeField && isTested ? "hover:bg-blue-500/20 hover:text-blue-300 cursor-grab active:cursor-grabbing" : "cursor-not-allowed opacity-50"} | ||
| `} | ||
| onClick={() => { | ||
| if (activeField && isTested) { | ||
| handleInsert(`{{${formattedNodeName}.map(item => item.${header})}}`); | ||
| } | ||
| }} | ||
| title={ | ||
| !isTested | ||
| ? "Test node first to map data" | ||
| : activeField | ||
| ? `Insert entire column as array: {{${formattedNodeName}.map(item => item.${header})}}` | ||
| : "Select a field first" | ||
| } |
There was a problem hiding this comment.
Use bracket notation for non-identifier keys.
These templates assume every object key is dot-safe. Keys like message-id, from name, or 1stValue will generate invalid expressions such as item.message-id or {{node[0].from name}}, so users end up inserting broken variables. Build the accessor with bracket notation when the key is not a valid identifier.
♻️ Suggested fix
onDragStart={(e) => {
- const variableText = `{{${formattedNodeName}.map(item => item.${header})}}`;
+ const accessor = /^[A-Za-z_$][\w$]*$/.test(header)
+ ? `.${header}`
+ : `[${JSON.stringify(header)}]`;
+ const variableText = `{{${formattedNodeName}.map(item => item${accessor})}}`;
@@
onClick={() => {
if (activeField && isTested) {
- handleInsert(`{{${formattedNodeName}[${rowIndex}].${header}}}`);
+ const accessor = /^[A-Za-z_$][\w$]*$/.test(header)
+ ? `.${header}`
+ : `[${JSON.stringify(header)}]`;
+ handleInsert(`{{${formattedNodeName}[${rowIndex}]${accessor}}}`);
}
}}Also applies to: 298-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 249 - 279, The
drag-and-drop and click handlers build dot-access expressions like
`{{${formattedNodeName}.map(item => item.${header})}}` which break for keys that
are not valid identifiers; update the construction in the onDragStart handler
and the onClick/handleInsert call to use bracket notation when needed (e.g., use
item[headerString] or `${formattedNodeName}.map(item => item[${quotedHeader}])`)
by testing header against a JS identifier regex and quoting/escaping it when
using bracket form; adjust the title string generation similarly so it shows a
valid expression; touch the same logic around the other similar block (the block
referenced at lines ~298-309) so both drag click and title use bracket notation
for non-identifier keys.
Comment on lines
42
to
+47
| const [testResult, setTestResult] = useState<any>(null); | ||
|
|
||
|
|
||
| // Reset test result when switching to a different node | ||
| useEffect(() => { | ||
| setTestResult(null); | ||
| }, [selectedNode?.id]); |
There was a problem hiding this comment.
Reset all node-scoped modal state together.
Only testResult is cleared when selectedNode changes. activeField and dynamicOptions survive the switch, so variable insertion can still target a field from the previous node, while TestPanel ignores the nodeTestOutput already in Redux and falls back to "Not tested yet". Reopening a tested node—or failing a retest that only updates Redux—can therefore show the wrong state. I'd derive the panel from nodeTestOutput and reset the other node-specific state here as well.
Also applies to: 56-62, 685-685
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 42 - 47,
The useEffect that currently only clears testResult when selectedNode?.id
changes should also reset other node-scoped modal state (clear activeField and
dynamicOptions) so no stale field or options from the previous node remain;
locate the state variables testResult, activeField, dynamicOptions and update
the effect that depends on selectedNode?.id to set them all to their initial
values, and change TestPanel usage to derive its displayed status from
nodeTestOutput (from Redux) instead of local testResult so the panel reflects
Redux updates correctly.
Comment on lines
433
to
435
| {options.map((opt: any) => ( | ||
| <option key={opt.value || opt.id || opt} value={opt.value || opt.id !== undefined ? opt.id : opt }> | ||
| <option key={opt.value || opt.id || opt} value={opt.value || opt.id !== undefined ? opt.id : opt}> | ||
| {opt.label || opt.name || opt} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ConfigModal.tsx" | head -5Repository: Dev-Pross/BuildFlow
Length of output: 119
🏁 Script executed:
if [ -f "apps/web/app/workflows/[id]/components/ConfigModal.tsx" ]; then
wc -l "apps/web/app/workflows/[id]/components/ConfigModal.tsx"
else
fd ConfigModal.tsx
fiRepository: Dev-Pross/BuildFlow
Length of output: 121
🏁 Script executed:
# Check the exact lines and surrounding context
sed -n '430,440p' apps/web/app/workflows/[id]/components/ConfigModal.tsxRepository: Dev-Pross/BuildFlow
Length of output: 467
🏁 Script executed:
node <<'EOF'
// Test the actual parsing of: opt.value || opt.id !== undefined ? opt.id : opt
// Case 1: opt.value is defined, opt.id is also defined
let opt = { value: "A", id: "B" };
let result1 = opt.value || opt.id !== undefined ? opt.id : opt;
console.log("Case 1 - both defined:");
console.log(" opt.value:", opt.value);
console.log(" opt.id:", opt.id);
console.log(" Result:", result1);
console.log(" Expected (value):", opt.value);
console.log(" Match:", result1 === opt.value ? "✓" : "✗ BUG");
// Case 2: opt.value is undefined, opt.id is defined
opt = { value: undefined, id: "B" };
result1 = opt.value || opt.id !== undefined ? opt.id : opt;
console.log("\nCase 2 - only id defined:");
console.log(" opt.value:", opt.value);
console.log(" opt.id:", opt.id);
console.log(" Result:", result1);
console.log(" Expected (id):", opt.id);
console.log(" Match:", result1 === opt.id ? "✓" : "✗");
// Now test the suggested fix
console.log("\n--- Testing suggested fix: opt?.value ?? opt?.id ?? opt ---");
opt = { value: "A", id: "B" };
let result2 = opt?.value ?? opt?.id ?? opt;
console.log("Case 1 - both defined:");
console.log(" Result:", result2);
console.log(" Expected (value):", opt.value);
console.log(" Match:", result2 === opt.value ? "✓" : "✗");
opt = { value: undefined, id: "B" };
result2 = opt?.value ?? opt?.id ?? opt;
console.log("\nCase 2 - only id defined:");
console.log(" Result:", result2);
console.log(" Expected (id):", opt.id);
console.log(" Match:", result2 === opt.id ? "✓" : "✗");
EOFRepository: Dev-Pross/BuildFlow
Length of output: 463
Fix the dropdown option value expression.
The expression opt.value || opt.id !== undefined ? opt.id : opt is parsed as (opt.value || opt.id !== undefined) ? opt.id : opt due to operator precedence. This causes an option with both value and id to submit opt.id instead of opt.value. Use a nullish coalescing chain to ensure the saved config matches the selected option:
{options.map((opt: any) => {
const optionValue = opt?.value ?? opt?.id ?? opt;
return (
<option key={String(optionValue)} value={optionValue}>
{opt.label || opt.name || opt}
</option>
);
})}📝 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.
Suggested change
| {options.map((opt: any) => ( | |
| <option key={opt.value || opt.id || opt} value={opt.value || opt.id !== undefined ? opt.id : opt }> | |
| <option key={opt.value || opt.id || opt} value={opt.value || opt.id !== undefined ? opt.id : opt}> | |
| {opt.label || opt.name || opt} | |
| {options.map((opt: any) => { | |
| const optionValue = opt?.value ?? opt?.id ?? opt; | |
| return ( | |
| <option key={String(optionValue)} value={optionValue}> | |
| {opt.label || opt.name || opt} | |
| </option> | |
| ); | |
| })} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 433 -
435, The option value expression in ConfigModal's render (inside options.map) is
using incorrect precedence so items with both value and id pick id; change the
expression used for the <option value=...> to use a nullish/coalescing chain
(e.g., prefer opt.value, then opt.id, then the raw opt) so the saved config
matches the selected option; update the value expression in the options.map
callback in ConfigModal.tsx accordingly.
| icon: action.icon | ||
| }), | ||
| onTest: ()=> testNodeFromCanvas(action.NodeId, action.name, "action") | ||
| onTest: ()=> testNodeFromCanvas(actionId, action.name, "action") |
There was a problem hiding this comment.
Carry the corrected test id through the rehydration paths too.
This callback is correct for freshly created actions, but the callbacks around Line 391, Line 417, and Line 709 still call testNodeFromCanvas with Trigger.TriggerId / node.NodeId on objects that are otherwise read via id. Fresh nodes will test fine; reopened workflows can still hit api.execute.node(undefined, ...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/workflows/`[id]/page.tsx at line 617, The test callbacks
currently pass stale identifier properties (e.g., Trigger.TriggerId /
node.NodeId) which breaks after rehydration; update the callbacks that call
testNodeFromCanvas to derive a single canonical testId (e.g., const testId =
actionId ?? action.id ?? trigger.id ?? node.id) and pass that testId into
testNodeFromCanvas (instead of Trigger.TriggerId/node.NodeId). Locate all
occurrences where testNodeFromCanvas is invoked (including the existing onTest:
()=> testNodeFromCanvas(actionId, action.name, "action") usage and the other
callbacks that reference Trigger.TriggerId or node.NodeId) and replace them to
use the canonical testId so reopened workflows use the correct id path.
Comment on lines
60
to
71
| return { | ||
| success: result.success, | ||
| output: result.data, | ||
| output: { | ||
| status: "sent", | ||
| messageId: result.data?.id, | ||
| threadId: result.data?.threadId, | ||
| to: to, | ||
| subject: subject, | ||
| summary: `Email sent to ${to} with subject "${subject}"`, | ||
| }, | ||
| error: result.error, | ||
| }; |
There was a problem hiding this comment.
status: "sent" is returned even when the email fails to send.
When result.success is false, the output still contains status: "sent", which is misleading for downstream nodes and logging. The structured output should either be omitted on failure or reflect the actual status.
🐛 Proposed fix to conditionally set output on success
return {
success: result.success,
- output: {
- status: "sent",
- messageId: result.data?.id,
- threadId: result.data?.threadId,
- to: to,
- subject: subject,
- summary: `Email sent to ${to} with subject "${subject}"`,
- },
+ output: result.success
+ ? {
+ status: "sent",
+ messageId: result.data?.id,
+ threadId: result.data?.threadId,
+ to: to,
+ subject: subject,
+ summary: `Email sent to ${to} with subject "${subject}"`,
+ }
+ : undefined,
error: result.error,
};📝 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.
Suggested change
| return { | |
| success: result.success, | |
| output: result.data, | |
| output: { | |
| status: "sent", | |
| messageId: result.data?.id, | |
| threadId: result.data?.threadId, | |
| to: to, | |
| subject: subject, | |
| summary: `Email sent to ${to} with subject "${subject}"`, | |
| }, | |
| error: result.error, | |
| }; | |
| return { | |
| success: result.success, | |
| output: result.success | |
| ? { | |
| status: "sent", | |
| messageId: result.data?.id, | |
| threadId: result.data?.threadId, | |
| to: to, | |
| subject: subject, | |
| summary: `Email sent to ${to} with subject "${subject}"`, | |
| } | |
| : undefined, | |
| error: result.error, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nodes/src/gmail/gmail.executor.ts` around lines 60 - 71, The current
return always sets output.status to "sent" even when result.success is false;
update the return so output is only populated on success (using result.success)
and on failure either omit output or set a failure-aware object (e.g., status:
"failed" plus error details). Locate the return block that references
result.success, output, messageId, threadId, to, and subject in
gmail.executor.ts and wrap the output population behind a conditional: when
result.success === true include the sent metadata (status: "sent", messageId,
threadId, to, subject, summary), otherwise set output to null or a small failure
object containing status: "failed" and result.error to avoid misleading
downstream nodes.
Vamsi-o
approved these changes
Mar 17, 2026
Vamsi-o
left a comment
Vamsi-o
left a comment
Contributor
There was a problem hiding this comment.
Potential issue | 🟠 Major
Limit spreadsheet previews before rendering.
renderSpreadsheetTable maps every returned row, unlike renderArrayOfObjectsTable and the preview limits in VariablePanel. A large Sheets test result can lock the modal as soon as it opens; slice or virtualize this branch before rendering
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…d configuration UI components.
Summary by CodeRabbit
Release Notes
New Features
Improvements