variable-panel done with looping gmail execution#69
Conversation
📝 WalkthroughWalkthroughIntroduces a comprehensive variable interpolation system enabling users to view available variables from previous workflow nodes, test individual nodes, and automatically resolve variable placeholders in config fields before execution. Adds node test output management, frontend variable display UI, and backend spreadsheet-driven loop capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Frontend)
participant VP as VariablePanel
participant Redux as Redux Store
participant ConfigModal as ConfigModal
participant API as Backend API
participant Executor as Executor Engine
User->>ConfigModal: Select node & field (activeField)
ConfigModal->>VP: Render with previousNodes & activeField
VP->>Redux: Read selectAllOutputs
Redux-->>VP: Return node test outputs
VP-->>User: Display available variables with pills
User->>VP: Click variable pill
VP->>ConfigModal: onInsert({{nodeName.variablePath}})
ConfigModal->>ConfigModal: Update field with variable
User->>ConfigModal: Click "Test Node"
ConfigModal->>ConfigModal: buildTestContext from previousNodes
ConfigModal->>ConfigModal: resolveConfigVariables
ConfigModal->>API: execute.node(nodeId, config)
API->>Executor: POST /execute/node
Executor->>Executor: buildInterpolationContext
Executor->>Executor: resolveConfigVariables
Executor->>Executor: Detect spreadsheet input
alt Spreadsheet Loop
Executor->>Executor: Per-row execution with retry
Executor->>Executor: Aggregate LoopExecutionResult
else Single Execution
Executor->>Executor: Execute node once
end
Executor-->>API: Return { success, output, error }
API-->>ConfigModal: Response with output
ConfigModal->>Redux: dispatch setNodeOutput(NodeTestOutput)
Redux-->>Redux: Store output + variables
ConfigModal->>User: Show result toast & variables
User->>VP: Variables now available for other fields
Estimated code review effort🎯 5 (Critical) | ⏱️ ~110 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/worker/package.json (1)
2-2:⚠️ Potential issue | 🟡 MinorTypo in package name:
@repo/workder→@repo/worker.This appears pre-existing, but it could cause issues with workspace resolution or package references.
Proposed fix
- "name": "@repo/workder", + "name": "@repo/worker",apps/worker/src/engine/executor.ts (1)
227-272:⚠️ Potential issue | 🟠 MajorPartial-failure semantics are contradictory — workflow aborts even when rows partially succeed.
Line 230 sets
success: !hasFailures, so any single row failure marks the node as failed. Lines 254–272 then mark the entire workflow as"Failed"andreturnearly, preventing subsequent nodes from running. TheisPartialFailurevariable (line 252) only controls whetheroutputDatais persisted—it doesn't change the abort behavior.If the intent is "loop completes even if some rows fail" (line 227 comment), the workflow should continue (with a degraded status or warning), or at minimum give the user a way to configure this tolerance. As-is, one transient row error after 3 retries kills the whole workflow.
apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
311-311:⚠️ Potential issue | 🟡 MinorDebug
console.logrendered as JSX expression.
{console.log(options)}inside JSX is a debug leftover. It evaluates toundefinedwhich React renders as nothing, but it fires on every render and pollutes the console.Proposed fix
- {console.log(options)} <option value="">Select {field.label.toLowerCase()}</option>
🤖 Fix all issues with AI agents
In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts`:
- Around line 36-40: The console.log that prints the full execution context
(including credentialId derived from nodeData.CredentialsID /
config?.credentialId) should not leak identifiers in production; either remove
the console.log in executionRoutes.ts or replace it with a guarded debug-level
logger (check an isDebug/isProd flag or logger.level) and ensure credentialId is
redacted before logging (e.g., omit or mask context.credentialId) so sensitive
identifiers are never emitted in non-debug environments.
In `@apps/web/app/components/ui/variable-panel.tsx`:
- Around line 17-44: Remove the verbose console.log calls in the VariablePanel
render/processing flow: delete the top-level logs of previousNodes and
testedOutputs and the per-node logs inside the enrichedNodes mapping (the logs
that print testedData details and full variable lists), or guard them behind a
debug flag (e.g., only log when process.env.NODE_ENV === 'development' or a
dedicated isDebug prop) so VariablePanel no longer emits sensitive/internal
structures; update the enrichedNodes mapping that uses previousNodes,
testedOutputs, PreviousNodeOutput, and the _tested marker accordingly to avoid
leaving stray debug statements.
- Around line 140-143: The click handler builds a child variable path by naively
concatenating variable.path and child.path (in the onClick that calls
handleClick), which produces broken references like "rows[0]email"; fix by
inserting a dot between parent and child when both are present and not already
separated — e.g. compute a combinedPath (for use in the handleClick call) that
uses child.path when variable.path is empty, otherwise uses
`${variable.path}.${child.path}` while avoiding duplicate dots if either side
already contains a trailing/leading dot; update the onClick invocation that
currently passes `{ ...child, path: `${variable.path}${child.path}` }` to use
this combinedPath.
In `@apps/web/app/lib/api.ts`:
- Around line 123-141: The axios.post call inside the node async function
currently throws on non-2xx responses so the subsequent res.data?.data?.success
check and custom error are never reached; wrap the axios call in a try/catch (or
use axios validateStatus to treat the backend's structured error responses as
resolved) and in the catch extract the backend payload (res?.response?.data or
the error response body) to build and throw the same custom Error used when
res.data indicates failure, ensuring you still log the response
(console.log('[api.execute.node] Response:', ...)) and return
res.data.data.output when success.
In `@apps/web/app/lib/nodeConfigs/gmail.action.ts`:
- Around line 51-55: The outputSchema in gmail.action.ts is using incorrect
Gmail API field names: update the outputSchema array (symbol: outputSchema) to
map the Gmail response fields returned by users.messages.send() by replacing the
"messageId" path with "id" and removing the non-existent "status" entry so that
only valid fields like "id" and "threadId" are returned (i.e., keep entries for
"Message ID" -> path "id" and "Thread ID" -> path "threadId").
In `@apps/web/app/lib/nodeConfigs/googleSheet.action.ts`:
- Around line 61-74: The outputSchema in googleSheet.action.ts lists a "Sheet
Name" path of "sheetName" but the executor (google-sheets.executor.ts) returns
sheetId (the spreadsheetId) instead; update the schema entry in outputSchema so
the path matches the executor output (e.g., change the path from "sheetName" to
"sheetId" or "spreadsheetId" and update the name/description accordingly) to
ensure variable interpolation resolves correctly—verify the exact key used in
google-sheets.executor.ts and make the schema match that key.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 88-162: The handleTestNode function sets loading true via
dispatch(setNodeLoading({ nodeId: selectedNode.id, loading: true })) but never
clears it; add a finally block (or ensure both try and catch paths) to
dispatch(setNodeLoading({ nodeId: selectedNode.id, loading: false })) so the
spinner/button state is reset after success or failure; update handleTestNode to
dispatch the loading:false in a finally clause (referencing handleTestNode,
dispatch, and setNodeLoading) and ensure no early returns skip that dispatch.
In `@apps/worker/src/engine/executor.ts`:
- Around line 96-106: The executor is printing full JSON of large structures
which will flood logs; replace or gate the console.log calls around
buildInterpolationContext and resolveConfigVariables (and the per-row logging
near where nodeConfig is used) by using the existing logger at debug level (or
wrap with a feature flag like isDebugEnabled) and avoid JSON.stringify of entire
objects—log concise summaries (e.g., keys, size, or truncated preview) instead;
specifically update the code paths that reference buildInterpolationContext,
resolveConfigVariables, executedNodeOutputs, and nodeConfig so they only emit
safe, bounded debug output or are skipped in production.
- Around line 164-170: The current mapping in loopOutputs uses a fragile
reference-equality check (o.outputData === currentInputData) which can fail if
outputs are cloned/serialized; instead identify the producer once and match by a
stable identifier: when you first find the executedNodeOutputs entry that
corresponds to the spreadsheet input (using the existing reference check or a
safer deep/id match), capture its unique property (e.g., nodeName, nodeId, or
producerName) and then rebuild loopOutputs by replacing entries where o.nodeName
=== capturedProducerName (or o.nodeId === capturedProducerId) with outputData:
rowContext; update references to executedNodeOutputs/currentInputData
accordingly in the mapping to use that stable identifier.
In `@packages/common/src/interpolation.ts`:
- Around line 154-161: In resolveConfigVariables, remove the accidental double
space in the function declaration (change "function resolveConfigVariables" to
"function resolveConfigVariables") and fix the console.log message typo from
"onject" to "object"; update the log string to a clear, correct message (e.g.,
"[---*---] config not an object type") so the user-facing text is accurate.
- Around line 120-121: The function interpolateString declares a string return
but currently returns template when it is falsy or non-string; change the
early-return behavior in interpolateString (and any related callers using
InterpolationContext) so it always returns a string—e.g., coerce
falsy/non-string template values to an empty string or String(template) before
returning and proceed with interpolation; update the early-return branch in
interpolateString to return that coerced string rather than template.
- Around line 120-144: The interpolateString function (and similarly
resolveConfigVariables) currently emits verbose console.log calls for every
template and placeholder which will flood logs; remove these per-placeholder
console.log statements or wrap them behind a configurable debug/log-level check
(e.g., check a boolean debug flag on the InterpolationContext or use a provided
logger with level checks) so normal runs produce no per-row debug output while
retaining the ability to enable detailed logging for troubleshooting; update
interpolateString to use the context’s debug flag or logger (and do the same in
resolveConfigVariables) instead of unconditional console.log calls.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts`:
- Around line 276-281: The output payload mistakenly uses the key sheetId and
assigns it spreadsheetId (variable spreadsheetId) which is misleading and
conflicts with the expected outputSchema (googleSheet.action.ts expects
sheetName); update the object returned from the executor (the object containing
rows, columns, dataStartIndex, rowCount, sheetId, hasHeaders) to use the correct
key name (sheetName) and populate it with the actual sheet/tab identifier or
name (use the existing variable that holds the sheet name if one exists,
otherwise derive it and replace spreadsheetId), and update any references that
consume sheetId to use sheetName so names align with outputSchema.
🧹 Nitpick comments (9)
apps/http-backend/src/routes/userRoutes/executionRoutes.ts (1)
37-37: Empty-string fallback forcredentialIdwill produce a confusing downstream error.If neither
nodeData.CredentialsIDnorconfig?.credentialIdis set, the executor receives"", which will likely fail deep inside the OAuth service with a misleading error. Consider returning a clear 400 response early instead.Proposed fix
- credentialId: nodeData.CredentialsID || config?.credentialId || "" + credentialId: nodeData.CredentialsID || config?.credentialId } + + if (!context.credentialId) { + return res.status(statusCodes.BAD_REQUEST).json({ + message: "No credential found for this node" + }); + }packages/common/package.json (1)
16-21: Export path"./zod"now also exposes interpolation utilities — consider adding a dedicated export.The
"./zod"subpath now re-exports interpolation utilities (viaindex.ts), making the import path misleading for consumers (e.g.,import { resolveVariable } from "@repo/common/zod"). Consider adding a separate"./interpolation"export or renaming to a more generic path like".".packages/nodes/src/google-sheets/google-sheets.executor.ts (2)
208-211:rangeStartsFromRow1doesn't handle column-only ranges like"A:Z".A range like
"A:Z"(no row number) returns all rows including headers, but the regex finds no digit, so the method returnsfalse. This triggers an unnecessary separate header fetch. Not a correctness bug (the output is still correct), but worth a note.
247-265: If the data-rows fetch fails after the header fetch succeeds, the error is caught by the outer catch — but the header fetch error is silently swallowed.The asymmetric error handling is intentional (headers are optional), but consider logging at
warnlevel rather than a plainconsole.logfor better observability.packages/common/src/interpolation.ts (1)
14-15: Module-level regex with/gflag is fragile.
VARIABLE_REGEXat line 15 is a global regex used inextractVariables. Because/gmakeslastIndexstateful, any forgotten reset (or future reuse) can cause intermittent misses. The functionextractVariablesdoes reset it, butinterpolateStringwisely creates a local instance. Consider makingVARIABLE_REGEXnon-global and only creating/ginstances locally where needed, or just inlining it.apps/worker/src/engine/executor.ts (1)
136-138:shouldLoopheuristic can false-positive on literal{{in config values.
JSON.stringify(node.config).includes('{{')matches any occurrence of{{in any config value, including user-supplied text that legitimately contains double-braces (e.g., Mustache/Handlebars templates in email bodies). Consider using the existingextractVariablesutility to check for actual{{...}}variable patterns.Proposed fix
+import { extractVariables } from "@repo/common/zod"; ... - const shouldLoop = isSpreadsheetInput(currentInputData) && - JSON.stringify(node.config).includes('{{'); + const shouldLoop = isSpreadsheetInput(currentInputData) && + extractVariables(node.config as Record<string, any>).length > 0;apps/web/app/workflows/[id]/page.tsx (1)
29-56:getPreviousNodesonly surfaces direct predecessors — worker resolves variables from all prior nodes.This function traverses one level of incoming edges, so for a chain
A → B → C, node C only sees variables from B. However, the backend executor (executor.tslines 100–101, 284–288) builds an interpolation context from every previously executed node, allowingCto reference{{a.field}}. Users who see only B's variables in the panel but type{{a.field}}manually will find it works at runtime, which is confusing.Consider recursively collecting all ancestor nodes, or document that manual variable paths for non-adjacent nodes are supported.
apps/web/app/components/ui/variable-panel.tsx (1)
105-105: Node name normalization duplicated across files.
.toLowerCase().replace(/\s+/g, '_')appears here (lines 105, 117, 141), inbuildInterpolationContext, inConfigModal.buildTestContext, etc. Extract a sharednormalizeNodeNameutility in the common package to keep this consistent and DRY.apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
63-85:buildTestContextduplicatesbuildInterpolationContextlogic from the common package.This function manually normalizes node names and builds a context object—exactly what
buildInterpolationContextfrom@repo/common/zod(already imported on line 5) does. Reuse it to stay DRY and consistent.Proposed refactor
const buildTestContext = (): InterpolationContext => { - const context: InterpolationContext = {}; - for (const [nodeId, testOutput] of Object.entries(allTestedOutputs)) { - if (testOutput.success && testOutput.data) { - const normalizedName = testOutput.nodeName.toLowerCase().replace(/\s+/g, '_'); - context[normalizedName] = testOutput.data; - } - } - return context; + const nodeOutputs = Object.values(allTestedOutputs) + .filter(t => t.success && t.data) + .map(t => ({ nodeName: t.nodeName, outputData: t.data })); + return buildInterpolationContext(nodeOutputs); };
| config: config, | ||
| credentialId: nodeData.CredentialsID || config?.credentialId || "" | ||
| } | ||
|
|
||
| console.log(`Execution context: ${JSON.stringify(context)}`) |
There was a problem hiding this comment.
Avoid logging credential identifiers in production.
Line 40 logs the full execution context including credentialId. This is fine for development but should be removed or guarded behind a debug flag before shipping to production to avoid leaking internal identifiers in logs.
🤖 Prompt for AI Agents
In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts` around lines 36 -
40, The console.log that prints the full execution context (including
credentialId derived from nodeData.CredentialsID / config?.credentialId) should
not leak identifiers in production; either remove the console.log in
executionRoutes.ts or replace it with a guarded debug-level logger (check an
isDebug/isProd flag or logger.level) and ensure credentialId is redacted before
logging (e.g., omit or mask context.credentialId) so sensitive identifiers are
never emitted in non-debug environments.
| // Debug logging | ||
| console.log('[VariablePanel] previousNodes:', previousNodes); | ||
| console.log('[VariablePanel] testedOutputs from Redux:', testedOutputs); | ||
|
|
||
| // Merge static outputSchema with dynamic tested data | ||
| const enrichedNodes: PreviousNodeOutput[] = previousNodes.map(node => { | ||
| const testedData = testedOutputs[node.nodeId]; | ||
|
|
||
| console.log(`[VariablePanel] Node ${node.nodeName} (${node.nodeId}):`, { | ||
| hasTestedData: !!testedData, | ||
| testedSuccess: testedData?.success, | ||
| testedVariablesCount: testedData?.variables?.length, | ||
| staticVariablesCount: node.variables?.length | ||
| }); | ||
|
|
||
| // If node was tested, use the dynamic variables from real output | ||
| if (testedData?.success && testedData.variables.length > 0) { | ||
| console.log(`[VariablePanel] Using DYNAMIC variables for ${node.nodeName}:`, testedData.variables); | ||
| return { | ||
| ...node, | ||
| variables: testedData.variables, | ||
| // Mark as dynamically discovered | ||
| _tested: true | ||
| } as PreviousNodeOutput & { _tested?: boolean }; | ||
| } | ||
|
|
||
| // Otherwise use the static outputSchema | ||
| console.log(`[VariablePanel] Using STATIC variables for ${node.nodeName}:`, node.variables); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove verbose debug logging.
Lines 18–19, 25–30, 34, and 44 log every render's previousNodes, per-node tested data details, and full variable lists to the console. This is noisy in development and leaks internal data structure details in production.
🤖 Prompt for AI Agents
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 17 - 44, Remove
the verbose console.log calls in the VariablePanel render/processing flow:
delete the top-level logs of previousNodes and testedOutputs and the per-node
logs inside the enrichedNodes mapping (the logs that print testedData details
and full variable lists), or guard them behind a debug flag (e.g., only log when
process.env.NODE_ENV === 'development' or a dedicated isDebug prop) so
VariablePanel no longer emits sensitive/internal structures; update the
enrichedNodes mapping that uses previousNodes, testedOutputs,
PreviousNodeOutput, and the _tested marker accordingly to avoid leaving stray
debug statements.
| onClick={() => handleClick( | ||
| node.nodeName.toLowerCase().replace(/\s+/g, '_'), | ||
| { ...child, path: `${variable.path}${child.path}` } | ||
| )} |
There was a problem hiding this comment.
Bug: missing dot separator in child variable path — produces broken references like rows[0]email.
Line 142 concatenates variable.path and child.path without a . between them. For a parent path "rows[0]" and child path "email", the resulting interpolation reference becomes {{node.rows[0]email}} instead of {{node.rows[0].email}}, which will fail to resolve at runtime.
Proposed fix
onClick={() => handleClick(
node.nodeName.toLowerCase().replace(/\s+/g, '_'),
- { ...child, path: `${variable.path}${child.path}` }
+ { ...child, path: `${variable.path}.${child.path}` }
)}📝 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.
| onClick={() => handleClick( | |
| node.nodeName.toLowerCase().replace(/\s+/g, '_'), | |
| { ...child, path: `${variable.path}${child.path}` } | |
| )} | |
| onClick={() => handleClick( | |
| node.nodeName.toLowerCase().replace(/\s+/g, '_'), | |
| { ...child, path: `${variable.path}.${child.path}` } | |
| )} |
🤖 Prompt for AI Agents
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 140 - 143, The
click handler builds a child variable path by naively concatenating
variable.path and child.path (in the onClick that calls handleClick), which
produces broken references like "rows[0]email"; fix by inserting a dot between
parent and child when both are present and not already separated — e.g. compute
a combinedPath (for use in the handleClick call) that uses child.path when
variable.path is empty, otherwise uses `${variable.path}.${child.path}` while
avoiding duplicate dots if either side already contains a trailing/leading dot;
update the onClick invocation that currently passes `{ ...child, path:
`${variable.path}${child.path}` }` to use this combinedPath.
| node: async (nodeId: string, config?: any) => { | ||
| const res = await axios.post(`${BACKEND_URL}/execute/node`, | ||
| { NodeId: nodeId, Config: config }, | ||
| { | ||
| withCredentials: true, | ||
| headers: { "Content-Type": "application/json" }, | ||
| } | ||
| ); | ||
| console.log('[api.execute.node] Response:', res); | ||
|
|
||
| // Check if execution was successful | ||
| if (res.data?.data?.success) { | ||
| return res.data.data.output; | ||
| } | ||
|
|
||
| // If not successful, throw error with message | ||
| const errorMessage = res.data?.data?.error || res.data?.message || "Execution failed"; | ||
| throw new Error(errorMessage); | ||
| } |
There was a problem hiding this comment.
Error handling for failed executions is unreachable.
When the backend returns a non-2xx status (e.g., 403 on execution failure at executionRoutes.ts line 51), axios throws before reaching line 134. The res.data?.data?.success check and the custom error at line 140 are effectively dead code for the failure path. You'd need a try/catch around the axios call (or use validateStatus) to handle the backend's structured error response.
Proposed fix
node: async (nodeId: string, config?: any) => {
- const res = await axios.post(`${BACKEND_URL}/execute/node`,
- { NodeId: nodeId, Config: config },
- {
- withCredentials: true,
- headers: { "Content-Type": "application/json" },
- }
- );
- console.log('[api.execute.node] Response:', res);
-
- // Check if execution was successful
- if (res.data?.data?.success) {
- return res.data.data.output;
+ try {
+ const res = await axios.post(`${BACKEND_URL}/execute/node`,
+ { NodeId: nodeId, Config: config },
+ {
+ withCredentials: true,
+ headers: { "Content-Type": "application/json" },
+ }
+ );
+
+ if (res.data?.data?.success) {
+ return res.data.data.output;
+ }
+
+ throw new Error(res.data?.data?.error || res.data?.message || "Execution failed");
+ } catch (error: any) {
+ const message = error.response?.data?.message
+ || error.response?.data?.data?.error
+ || error.message
+ || "Execution failed";
+ throw new Error(message);
}
-
- // If not successful, throw error with message
- const errorMessage = res.data?.data?.error || res.data?.message || "Execution failed";
- throw new Error(errorMessage);
}📝 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.
| node: async (nodeId: string, config?: any) => { | |
| const res = await axios.post(`${BACKEND_URL}/execute/node`, | |
| { NodeId: nodeId, Config: config }, | |
| { | |
| withCredentials: true, | |
| headers: { "Content-Type": "application/json" }, | |
| } | |
| ); | |
| console.log('[api.execute.node] Response:', res); | |
| // Check if execution was successful | |
| if (res.data?.data?.success) { | |
| return res.data.data.output; | |
| } | |
| // If not successful, throw error with message | |
| const errorMessage = res.data?.data?.error || res.data?.message || "Execution failed"; | |
| throw new Error(errorMessage); | |
| } | |
| node: async (nodeId: string, config?: any) => { | |
| try { | |
| const res = await axios.post(`${BACKEND_URL}/execute/node`, | |
| { NodeId: nodeId, Config: config }, | |
| { | |
| withCredentials: true, | |
| headers: { "Content-Type": "application/json" }, | |
| } | |
| ); | |
| if (res.data?.data?.success) { | |
| return res.data.data.output; | |
| } | |
| throw new Error(res.data?.data?.error || res.data?.message || "Execution failed"); | |
| } catch (error: any) { | |
| const message = error.response?.data?.message | |
| || error.response?.data?.data?.error | |
| || error.message | |
| || "Execution failed"; | |
| throw new Error(message); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/app/lib/api.ts` around lines 123 - 141, The axios.post call inside
the node async function currently throws on non-2xx responses so the subsequent
res.data?.data?.success check and custom error are never reached; wrap the axios
call in a try/catch (or use axios validateStatus to treat the backend's
structured error responses as resolved) and in the catch extract the backend
payload (res?.response?.data or the error response body) to build and throw the
same custom Error used when res.data indicates failure, ensuring you still log
the response (console.log('[api.execute.node] Response:', ...)) and return
res.data.data.output when success.
| outputSchema: [ | ||
| { name: "Message ID", path: "messageId", type: "string" }, | ||
| { name: "Status", path: "status", type: "string" }, | ||
| { name: "Thread ID", path: "threadId", type: "string" }, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the gmail.action.ts file
find . -name "gmail.action.ts" -type fRepository: Dev-Pross/BuildFlow
Length of output: 109
🏁 Script executed:
# Search for gmail-related files
fd -e ts "gmail" apps/web/app/lib/Repository: Dev-Pross/BuildFlow
Length of output: 107
🏁 Script executed:
# Also check the executor to understand what output is being returned
rg "sendEmail" -A 10 -B 2 apps/web/app/lib/Repository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
cat -n ./apps/web/app/lib/nodeConfigs/gmail.action.tsRepository: Dev-Pross/BuildFlow
Length of output: 2102
🏁 Script executed:
# Search for the executor or service that processes Gmail
fd -e ts "gmail" apps/web/app/lib/ --exec cat -n {}Repository: Dev-Pross/BuildFlow
Length of output: 2102
🏁 Script executed:
# Search for executor files related to gmail
fd -e ts "executor" apps/web/app/lib/ | grep -i gmailRepository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Search more broadly for where this action is executed
rg "gmailActionConfig" -A 5 -B 5 apps/web/app/lib/Repository: Dev-Pross/BuildFlow
Length of output: 2415
🏁 Script executed:
# Look for sendEmail implementation
rg "sendEmail|gmailService" -A 10 apps/web/app/lib/Repository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Find files that might handle action execution
fd -e ts "action|executor" apps/web/app/lib/ --type fRepository: Dev-Pross/BuildFlow
Length of output: 158
🏁 Script executed:
# Look for Google API or Gmail service integration
fd -e ts "" apps/web/app/lib/ | head -30Repository: Dev-Pross/BuildFlow
Length of output: 305
🏁 Script executed:
# Search for how output is generated for actions
rg "outputSchema" -B 5 -A 5 apps/web/app/Repository: Dev-Pross/BuildFlow
Length of output: 5069
🏁 Script executed:
cat -n apps/web/app/lib/api.ts | head -100Repository: Dev-Pross/BuildFlow
Length of output: 4142
🏁 Script executed:
# Search for where Gmail service or Google API is used
rg "google|gmail|Gmail" -i apps/web/app/ --type ts --type tsx | head -40Repository: Dev-Pross/BuildFlow
Length of output: 90
🏁 Script executed:
# Look for any API handlers or routes that might execute actions
fd -e ts "route|handler|api" apps/web/app/ --type fRepository: Dev-Pross/BuildFlow
Length of output: 162
🏁 Script executed:
# Look for backend or server code
fd -e ts "server|backend|executor" . --type f | head -20Repository: Dev-Pross/BuildFlow
Length of output: 205
🏁 Script executed:
# Check if there's a packages or server directory
ls -la . | head -20Repository: Dev-Pross/BuildFlow
Length of output: 1241
🏁 Script executed:
# Search the entire repo for Gmail API calls or sendEmail
rg "sendEmail|users\.messages\.send|gmail" -i --type ts | head -30Repository: Dev-Pross/BuildFlow
Length of output: 2751
🏁 Script executed:
cat -n ./packages/nodes/src/gmail/gmail.executor.tsRepository: Dev-Pross/BuildFlow
Length of output: 2567
🏁 Script executed:
# Also check the gmail service to understand the API call
cat -n ./packages/nodes/src/gmail/gmail.service.tsRepository: Dev-Pross/BuildFlow
Length of output: 3754
Fix outputSchema paths to match Gmail API response fields.
The Gmail users.messages.send() API returns { id, threadId, labelIds }. The outputSchema paths are incorrect: messageId should be id, and the status field does not exist in the Gmail API response. Users referencing these variables will encounter undefined values.
Correct mapping:
outputSchema: [
{ name: "Message ID", path: "id", type: "string" },
{ name: "Thread ID", path: "threadId", type: "string" },
],
🤖 Prompt for AI Agents
In `@apps/web/app/lib/nodeConfigs/gmail.action.ts` around lines 51 - 55, The
outputSchema in gmail.action.ts is using incorrect Gmail API field names: update
the outputSchema array (symbol: outputSchema) to map the Gmail response fields
returned by users.messages.send() by replacing the "messageId" path with "id"
and removing the non-existent "status" entry so that only valid fields like "id"
and "threadId" are returned (i.e., keep entries for "Message ID" -> path "id"
and "Thread ID" -> path "threadId").
| // Rebuild interpolation context with current row index | ||
| const loopOutputs = executedNodeOutputs.map(o => { | ||
| if (o.outputData === currentInputData) { | ||
| return { ...o, outputData: rowContext }; | ||
| } | ||
| return o; | ||
| }); |
There was a problem hiding this comment.
Fragile reference-equality check to identify the spreadsheet node's output.
Line 166 uses o.outputData === currentInputData (strict reference equality) to decide which previous-node output to patch with the per-row context. If the output object is ever cloned, serialized/deserialized, or mutated between nodes, this silently falls through and the row-specific context is never injected—causing every row to resolve the same (likely first-row) data.
Match by node name or a stable identifier instead.
Proposed fix — match by node name
You already know which node produced the spreadsheet data (the one whose output matches currentInputData). Track its name/index explicitly:
+ // Identify the source node that produced the spreadsheet data
+ const spreadsheetSourceNode = executedNodeOutputs.find(
+ o => o.outputData?.rows && o.outputData?.columns && o.outputData?.dataStartIndex !== undefined
+ );
+
for (let rowIdx = startIdx; rowIdx < spreadsheet.rows.length; rowIdx++) {
loopResult.totalProcessed++;
const rowContext = { ...spreadsheet, _currentRowIndex: rowIdx };
const loopOutputs = executedNodeOutputs.map(o => {
- if (o.outputData === currentInputData) {
+ if (spreadsheetSourceNode && o.nodeName === spreadsheetSourceNode.nodeName) {
return { ...o, outputData: rowContext };
}
return o;
});🤖 Prompt for AI Agents
In `@apps/worker/src/engine/executor.ts` around lines 164 - 170, The current
mapping in loopOutputs uses a fragile reference-equality check (o.outputData ===
currentInputData) which can fail if outputs are cloned/serialized; instead
identify the producer once and match by a stable identifier: when you first find
the executedNodeOutputs entry that corresponds to the spreadsheet input (using
the existing reference check or a safer deep/id match), capture its unique
property (e.g., nodeName, nodeId, or producerName) and then rebuild loopOutputs
by replacing entries where o.nodeName === capturedProducerName (or o.nodeId ===
capturedProducerId) with outputData: rowContext; update references to
executedNodeOutputs/currentInputData accordingly in the mapping to use that
stable identifier.
| export function interpolateString(template: string, context: InterpolationContext): string { | ||
| if (!template || typeof template !== 'string') return template; |
There was a problem hiding this comment.
Return type mismatch when template is falsy.
interpolateString declares a string return type, but line 121 returns template as-is when it's null, undefined, or non-string. This can cause downstream surprises (e.g., null propagating where a string is expected).
Proposed fix
export function interpolateString(template: string, context: InterpolationContext): string {
- if (!template || typeof template !== 'string') return template;
+ if (!template || typeof template !== 'string') return template ?? '';🤖 Prompt for AI Agents
In `@packages/common/src/interpolation.ts` around lines 120 - 121, The function
interpolateString declares a string return but currently returns template when
it is falsy or non-string; change the early-return behavior in interpolateString
(and any related callers using InterpolationContext) so it always returns a
string—e.g., coerce falsy/non-string template values to an empty string or
String(template) before returning and proceed with interpolation; update the
early-return branch in interpolateString to return that coerced string rather
than template.
| export function interpolateString(template: string, context: InterpolationContext): string { | ||
| if (!template || typeof template !== 'string') return template; | ||
|
|
||
| // Create a new regex instance to avoid global flag issues | ||
| const regex = /\{\{([^}]+)\}\}/g; | ||
|
|
||
| console.log(`[interpolateString] Input: "${template}"`); | ||
| console.log(`[interpolateString] Context keys: ${Object.keys(context).join(', ')}`); | ||
|
|
||
| const result = template.replace(regex, (match, variable) => { | ||
| console.log(`[interpolateString] Found variable: "${variable}"`); | ||
| console.log(`[interpolateString] MATCH variable: "${match}"`); | ||
| const resolved = resolveVariable(variable, context); | ||
| console.log(`[interpolateString] Resolved to: ${JSON.stringify(resolved)}`); | ||
|
|
||
| // Convert non-string values to string for template replacement | ||
| if (typeof resolved === 'object') { | ||
| return JSON.stringify(resolved); | ||
| } | ||
| return String(resolved ?? match); | ||
| }); | ||
|
|
||
| console.log(`[interpolateString] Output: "${result}"`); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Remove verbose debug logging before merging.
Lines 126–133 and 142 emit detailed console.log on every single placeholder match in every interpolated string. In the worker loop path, this will flood logs with per-row, per-field output (potentially thousands of lines for a large spreadsheet). The same applies to resolveConfigVariables at line 159.
Strip or gate these behind a debug flag / log level.
♻️ Proposed fix
export function interpolateString(template: string, context: InterpolationContext): string {
if (!template || typeof template !== 'string') return template;
- // Create a new regex instance to avoid global flag issues
const regex = /\{\{([^}]+)\}\}/g;
- console.log(`[interpolateString] Input: "${template}"`);
- console.log(`[interpolateString] Context keys: ${Object.keys(context).join(', ')}`);
-
const result = template.replace(regex, (match, variable) => {
- console.log(`[interpolateString] Found variable: "${variable}"`);
- console.log(`[interpolateString] MATCH variable: "${match}"`);
const resolved = resolveVariable(variable, context);
- console.log(`[interpolateString] Resolved to: ${JSON.stringify(resolved)}`);
- // Convert non-string values to string for template replacement
if (typeof resolved === 'object') {
return JSON.stringify(resolved);
}
return String(resolved ?? match);
});
- console.log(`[interpolateString] Output: "${result}"`);
return result;
}📝 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.
| export function interpolateString(template: string, context: InterpolationContext): string { | |
| if (!template || typeof template !== 'string') return template; | |
| // Create a new regex instance to avoid global flag issues | |
| const regex = /\{\{([^}]+)\}\}/g; | |
| console.log(`[interpolateString] Input: "${template}"`); | |
| console.log(`[interpolateString] Context keys: ${Object.keys(context).join(', ')}`); | |
| const result = template.replace(regex, (match, variable) => { | |
| console.log(`[interpolateString] Found variable: "${variable}"`); | |
| console.log(`[interpolateString] MATCH variable: "${match}"`); | |
| const resolved = resolveVariable(variable, context); | |
| console.log(`[interpolateString] Resolved to: ${JSON.stringify(resolved)}`); | |
| // Convert non-string values to string for template replacement | |
| if (typeof resolved === 'object') { | |
| return JSON.stringify(resolved); | |
| } | |
| return String(resolved ?? match); | |
| }); | |
| console.log(`[interpolateString] Output: "${result}"`); | |
| return result; | |
| } | |
| export function interpolateString(template: string, context: InterpolationContext): string { | |
| if (!template || typeof template !== 'string') return template; | |
| const regex = /\{\{([^}]+)\}\}/g; | |
| const result = template.replace(regex, (match, variable) => { | |
| const resolved = resolveVariable(variable, context); | |
| if (typeof resolved === 'object') { | |
| return JSON.stringify(resolved); | |
| } | |
| return String(resolved ?? match); | |
| }); | |
| return result; | |
| } |
🤖 Prompt for AI Agents
In `@packages/common/src/interpolation.ts` around lines 120 - 144, The
interpolateString function (and similarly resolveConfigVariables) currently
emits verbose console.log calls for every template and placeholder which will
flood logs; remove these per-placeholder console.log statements or wrap them
behind a configurable debug/log-level check (e.g., check a boolean debug flag on
the InterpolationContext or use a provided logger with level checks) so normal
runs produce no per-row debug output while retaining the ability to enable
detailed logging for troubleshooting; update interpolateString to use the
context’s debug flag or logger (and do the same in resolveConfigVariables)
instead of unconditional console.log calls.
| export function resolveConfigVariables<T extends Record<string, any>>( | ||
| config: T, | ||
| context: InterpolationContext | ||
| ): T { | ||
| if (!config || typeof config !== 'object') { | ||
| console.log("[---*---] config not an onject type") | ||
| return config; | ||
| } |
There was a problem hiding this comment.
Fix typo and extra whitespace.
Line 154 has a double space (function resolveConfigVariables), and Line 159 has a typo: "onject" → "object". These are user-facing log messages (even if they should be removed eventually).
Proposed fix
-export function resolveConfigVariables<T extends Record<string, any>>(
+export function resolveConfigVariables<T extends Record<string, any>>(
config: T,
context: InterpolationContext
): T {
if (!config || typeof config !== 'object') {
- console.log("[---*---] config not an onject type")
+ console.log("[---*---] config not an object type")
return config;
}📝 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.
| export function resolveConfigVariables<T extends Record<string, any>>( | |
| config: T, | |
| context: InterpolationContext | |
| ): T { | |
| if (!config || typeof config !== 'object') { | |
| console.log("[---*---] config not an onject type") | |
| return config; | |
| } | |
| export function resolveConfigVariables<T extends Record<string, any>>( | |
| config: T, | |
| context: InterpolationContext | |
| ): T { | |
| if (!config || typeof config !== 'object') { | |
| console.log("[---*---] config not an object type") | |
| return config; | |
| } |
🤖 Prompt for AI Agents
In `@packages/common/src/interpolation.ts` around lines 154 - 161, In
resolveConfigVariables, remove the accidental double space in the function
declaration (change "function resolveConfigVariables" to "function
resolveConfigVariables") and fix the console.log message typo from "onject" to
"object"; update the log string to a clear, correct message (e.g., "[---*---]
config not an object type") so the user-facing text is accurate.
| rows: combinedRows, | ||
| columns: columns, | ||
| dataStartIndex: 1, | ||
| rowCount: dataRowCount, | ||
| sheetId: spreadsheetId, | ||
| hasHeaders: Object.keys(columns).length > 0 |
There was a problem hiding this comment.
Output field sheetId is actually spreadsheetId — misleading naming.
sheetId (line 280) is set to spreadsheetId, which in Google Sheets API terminology refers to the entire spreadsheet, not an individual sheet/tab. This will confuse consumers and conflicts with the outputSchema in googleSheet.action.ts which expects sheetName.
Proposed fix
rows: combinedRows,
columns: columns,
dataStartIndex: 1,
rowCount: dataRowCount,
- sheetId: spreadsheetId,
+ spreadsheetId: spreadsheetId,
+ sheetName: sheetName,
hasHeaders: Object.keys(columns).length > 0📝 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.
| rows: combinedRows, | |
| columns: columns, | |
| dataStartIndex: 1, | |
| rowCount: dataRowCount, | |
| sheetId: spreadsheetId, | |
| hasHeaders: Object.keys(columns).length > 0 | |
| rows: combinedRows, | |
| columns: columns, | |
| dataStartIndex: 1, | |
| rowCount: dataRowCount, | |
| spreadsheetId: spreadsheetId, | |
| sheetName: sheetName, | |
| hasHeaders: Object.keys(columns).length > 0 |
🤖 Prompt for AI Agents
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 276
- 281, The output payload mistakenly uses the key sheetId and assigns it
spreadsheetId (variable spreadsheetId) which is misleading and conflicts with
the expected outputSchema (googleSheet.action.ts expects sheetName); update the
object returned from the executor (the object containing rows, columns,
dataStartIndex, rowCount, sheetId, hasHeaders) to use the correct key name
(sheetName) and populate it with the actual sheet/tab identifier or name (use
the existing variable that holds the sheet name if one exists, otherwise derive
it and replace spreadsheetId), and update any references that consume sheetId to
use sheetName so names align with outputSchema.
3 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes