feat: enhance workflow execution logging, add Gmail node, and update … #51
Conversation
…Google Sheets service
📝 WalkthroughWalkthroughThis diff introduces Gmail node integration with executor and service classes, enhances Google Sheets token management with expiry checking and refresh capabilities, adds worker initialization with Google Sheets executor registration, updates Next.js configuration for Prisma, makes ExecutionContext's nodeId optional, and reorganizes test file references in build metadata. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/nodes/src/gmail/gmail.executor.ts (1)
46-54: Token refresh failure is not handled.If
refreshAccessToken()fails (i.e.,refreshResult.successis false), the code continues to execute the email send operation with potentially invalid credentials. This could result in unclear auth errors downstream.🔎 Proposed fix to handle refresh failure
// Check token expiry and refresh if needed if (this.gmailService.isTokenExpired()) { const refreshResult: any = await this.gmailService.refreshAccessToken(); if (refreshResult.success) { await this.oauthService.updateCredentials( credentials.id, refreshResult.data ); + } else { + return { + success: false, + error: refreshResult.error || "Failed to refresh access token", + }; } }packages/nodes/src/common/google-oauth-service.ts (1)
139-144: Filteringexpiry_datewith truthiness may incorrectly exclude valid values.The check
if (tokens.expiry_date)will filter outexpiry_date: 0, which is a valid value indicating an expired token. Similarly, empty string checks are fine for string fields, but numeric fields need explicitundefinedornullchecks.🔎 Proposed fix
- if (tokens.access_token) filteredTokens.access_token = tokens.access_token; - if (tokens.refresh_token) filteredTokens.refresh_token = tokens.refresh_token; - if (tokens.token_type) filteredTokens.token_type = tokens.token_type; - if (tokens.expiry_date) filteredTokens.expiry_date = tokens.expiry_date; - if (tokens.scope) filteredTokens.scope = tokens.scope; + if (tokens.access_token !== undefined && tokens.access_token !== '') filteredTokens.access_token = tokens.access_token; + if (tokens.refresh_token !== undefined && tokens.refresh_token !== '') filteredTokens.refresh_token = tokens.refresh_token; + if (tokens.token_type !== undefined && tokens.token_type !== '') filteredTokens.token_type = tokens.token_type; + if (tokens.expiry_date !== undefined) filteredTokens.expiry_date = tokens.expiry_date; + if (tokens.scope !== undefined && tokens.scope !== '') filteredTokens.scope = tokens.scope;packages/nodes/src/index.ts (1)
15-15: Remove debug console.log statement.This "Hello World" log will execute on every module import, cluttering production logs unnecessarily.
🔎 Proposed fix
-console.log("Hello World From node / index.ts");apps/worker/src/engine/registory.ts (1)
35-42: Critical: Type safety bypassed with unsafe casts.Both
GmailExecutorandGoogleSheetsNodeExecutorare being force-cast toNodeExecutorusingas unknown as NodeExecutor. This completely bypasses TypeScript's type checking and can lead to runtime errors if these executors don't actually implement theNodeExecutorinterface.The TODO comments indicate this is known to be problematic. This should be addressed before merging.
🔎 Recommended fix
The executors should properly implement the
NodeExecutorinterface. Based on the interface definition:export interface NodeExecutor { execute(context: ExecutionContext): Promise<ExecutionResult>; }Update
GmailExecutorandGoogleSheetsNodeExecutorto explicitly implement this interface in their class definitions:// In the Gmail and Google Sheets executor files class GmailExecutor implements NodeExecutor { async execute(context: ExecutionContext): Promise<ExecutionResult> { // implementation } } class GoogleSheetsNodeExecutor implements NodeExecutor { async execute(context: ExecutionContext): Promise<ExecutionResult> { // implementation } }Then remove the unsafe casts:
- this.register("gmail", new GmailExecutor() as unknown as NodeExecutor); - this.register("google_sheet", new GoogleSheetsNodeExecutor() as unknown as NodeExecutor) + this.register("gmail", new GmailExecutor()); + this.register("google_sheet", new GoogleSheetsNodeExecutor());
🤖 Fix all issues with AI Agents
In @apps/worker/src/engine/executor.ts:
- Line 52: The console.log in executor.ts is serializing the context with a
template literal which yields “[object Object]”; update the log in the function
where `console.log(\`Executing with context: ${context}\`)` appears to serialize
the object using JSON.stringify (match the existing approach used on line 66) —
e.g., replace the template interpolation with JSON.stringify(context, null, 2)
so the context prints as readable JSON while preserving the surrounding message.
- Line 37: The condition `if(!update.error)` is wrong because Prisma's `update`
throws on failure rather than returning an `error` field; locate the Prisma call
that assigns `update` (e.g., `prisma.workflowExecution.update` in executor.ts),
remove the `if(!update.error)` check, and wrap the update call in a try-catch:
on success log the updated message (e.g., "updated the workflow execution"), and
in the catch log the error and either rethrow or handle it according to the
surrounding error-handling policy.
- Around line 54-64: Restore the omitted error-handling branch so failed node
executions stop the workflow: in the executor where the execute result is
checked (use execute.success and execute.error), re-enable the
prismaClient.workflowExecution.update call to set status to "Failed", set error
to execute.error and completedAt to new Date(), then return early (use
workflowExecutionId to identify the record) so subsequent nodes are not
processed and the workflow is not marked completed.
- Around line 46-48: The ExecutionContext type in apps/worker/src/types.ts is
mismatched with the object passed to executors: callers use credId but the type
currently declares credentialsId. Update the ExecutionContext interface by
replacing or renaming the property credentialsId?: string to credId?: string
(optional) so it matches the context passed from executor code (e.g., where
credId is read in executor.ts); leave nodeId optional as-is.
In @apps/worker/src/engine/registory.ts:
- Line 40: Fix the typo in the comment that reads "wehen visits this next time
make sure chang gmail executor implements NodeExecutor" by replacing "wehen"
with "when" (so it reads "when visits this next time..."); update the same
comment text in registory.ts where that comment appears to correct the spelling.
In @apps/worker/src/index.ts:
- Around line 11-12: register.initialize() is called with no error handling;
wrap the initialization call (register.initialize()) in a try/catch (and await
it if it returns a Promise) so any thrown/rejected error is caught, log the
error with a clear message using the existing logger (e.g., processLogger.error
or console.error) including the error details, and exit with a non‑zero status
(process.exit(1)) to avoid an uninformative crash and make failures visible.
In @packages/nodes/src/gmail/gmail.node.ts:
- Line 35: Remove the commented-out debug console.log statements left in the
Gmail node (e.g., the lines containing "// console.log(`✅ Registered node:
${this.definition.name}`);" and any other commented "console.log" instances
around the registration logic); simply delete these commented lines from the
gmail.node.ts so only active code remains and no commented debug artifacts are
left.
- Line 14: There are three typos using the property name "require" in the Gmail
node parameter definitions; replace each "require" key with the correct
validator key "required" (i.e., update every parameter object that currently has
"require: true" to "required: true") so the fields are properly validated;
locate the occurrences by searching for the string "require: true" in the
gmail.node.ts parameter definitions and change them to "required: true".
- Around line 12-15: Remove the unused "From" config field from the Gmail node
definition (the field with name "From" and type "email") so users are no longer
required to supply it; update the node's config/schema in gmail.node.ts to
delete that field, remove any validation or required-flag references to "From",
and ensure gmail.executor.ts continues to extract only to/subject/body (no
change needed there) and that no code attempts to read a "From" value; also
remove or update any tests, docs, or UI references that mention the "From" field
to avoid confusion.
- Around line 10-26: The fields array in the Gmail node definition is missing a
required "Subject" entry; update the fields array (alongside the existing
"From", "To", and "Body" entries in gmail.node.ts) to include a "Subject" field
with an appropriate type (e.g., "string" or "text") and require: true so subject
lines are captured for composed emails; ensure the new field follows the same
object shape as the other entries and is inserted where form order makes sense.
In @packages/nodes/src/google-sheets/google-sheets.service.ts:
- Around line 55-64: The current truthy check uses the full response object
returned by drive.files.list() (variable files); change the condition to verify
the actual items array (files.data.files) and whether it has length, and return
that array as data; specifically update the branch in the function using
drive.files.list() to check "files?.data?.files && files.data.files.length" and
on success return success: true with data: files.data.files, otherwise return
success: false and data: null.
- Around line 105-110: The isTokenExpired() method currently returns false when
this.auth.credentials.expiry_date is missing, but it should behave like
GmailService.isTokenExpired() and treat missing/invalid expiry info as expired
to force refresh; update isTokenExpired() to return true when
credentials.expiry_date is undefined, null, or 0, and otherwise compute
Date.now() >= expiry_date - (5*60*1000), referencing the isTokenExpired() method
and this.auth.credentials.expiry_date to locate the change.
🧹 Nitpick comments (9)
packages/nodes/src/google-sheets/google-sheets.executor.ts (1)
142-142: Consider removing or replacing debug console.log.This debug statement logs operation data to the console. In production, this can clutter logs and potentially expose sensitive configuration details. Consider using a structured logging framework with appropriate log levels, or remove this debug statement before merging.
packages/nodes/src/common/google-oauth-service.ts (1)
160-160: Consider removing or adjusting logging level for credential updates.Logging credential IDs in production could be a security concern. Consider using a proper logging framework with appropriate levels, or remove this debug statement.
packages/nodes/src/google-sheets/google-sheets.service.ts (1)
118-128: Emptyrefresh_tokenmay overwrite valid tokens during credential update.Initializing
refresh_tokento an empty string and only setting it if returned by Google meansupdateCredentialscould receive an empty string and potentially overwrite a valid stored refresh token. The filtering inupdateCredentialshandles empty strings, but it's cleaner to omit the field entirely.🔎 Proposed fix to conditionally include refresh_token
const result: GoogleSheetsCredentials = { access_token: credentials.access_token || '', - refresh_token: '', // Will be set below if present + refresh_token: credentials.refresh_token || '', // Keep existing if not returned token_type: credentials.token_type || '', expiry_date: credentials.expiry_date || 0 }; - - // Only include refresh_token if Google actually returned one - if (credentials.refresh_token) { - result.refresh_token = credentials.refresh_token; - }packages/nodes/src/registry/node-registry.ts (1)
3-4: Unused import:GmailService.
GmailServiceis imported but never used in this file. OnlyGmailNodeis needed for the registration flow.🔎 Proposed fix
-import { GmailService } from "../gmail/gmail.service.js"; import { GmailNode } from "../gmail/gmail.node.js";packages/nodes/src/gmail/gmail.node.ts (1)
8-8: Consider a more specific description.The description 'Communicate with Gmail' is somewhat vague. Consider making it more specific about the node's primary function, such as "Send emails via Gmail" or "Send email messages using Gmail API".
apps/web/next.config.mjs (1)
3-4: Consider adding bundlePagesRouterDependencies and Prisma webpack plugin for complete Next.js 15 setup.The
serverExternalPackagesconfiguration for Prisma is correct and matches the official Next.js 15 recommendations. However, the setup is incomplete:
- Add
bundlePagesRouterDependencies: trueto unify bundling behavior (recommended by Next.js 15)- For this monorepo setup, consider adding the
PrismaPluginvia webpack to avoid potential bundling/runtime issues with Prisma in monoreposimport { PrismaPlugin } from "@prisma/nextjs-monorepo-workaround-plugin"; const nextConfig = { bundlePagesRouterDependencies: true, transpilePackages: ["@workspace/ui", "@repo/db"], serverExternalPackages: ["@prisma/client", "prisma"], webpack: (config, { isServer }) => { if (isServer) config.plugins?.push(new PrismaPlugin()); return config; }, };packages/db/prisma/schema.prisma (1)
3-3: Remove commented-out code.The commented-out output path adds no value and can cause confusion. If this configuration is not needed, remove it entirely rather than keeping it commented out.
🔎 Proposed fix
generator client { provider = "prisma-client-js" - // output = "../node_modules/.prisma/client" }apps/worker/src/engine/registory.ts (1)
23-23: Inconsistent semicolon usage.The semicolon was removed from this line, but other statements in the codebase still use semicolons. Consider being consistent with your project's style guide.
apps/worker/src/engine/executor.ts (1)
37-75: Consider using a structured logging framework.Multiple
console.logstatements have been added for execution tracking. While this works for development, consider adopting a structured logging framework (such aswinstonorpino) for production use. This would provide:
- Log levels (debug, info, warn, error) for filtering
- Consistent formatting and structured output
- Better integration with log aggregation tools
- Performance optimizations for high-volume logging
Example with a hypothetical logger:
import { logger } from '../utils/logger'; // Instead of: console.log('updated the workflow execution') // Use: logger.info('Workflow execution updated', { workflowExecutionId, status: 'InProgress' });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/http-backend/tsconfig.tsbuildinfoapps/web/next.config.mjsapps/worker/src/engine/executor.tsapps/worker/src/engine/registory.tsapps/worker/src/index.tsapps/worker/src/types.tsapps/worker/tsconfig.tsbuildinfopackage.jsonpackages/db/package.jsonpackages/db/prisma/schema.prismapackages/db/tsconfig.tsbuildinfopackages/nodes/src/common/google-oauth-service.tspackages/nodes/src/gmail/gmail.executor.tspackages/nodes/src/gmail/gmail.node.tspackages/nodes/src/google-sheets/google-sheets.executor.tspackages/nodes/src/google-sheets/google-sheets.service.tspackages/nodes/src/index.tspackages/nodes/src/registry/node-registry.tspackages/nodes/tsconfig.tsbuildinfo
🧰 Additional context used
🧬 Code graph analysis (6)
packages/nodes/src/registry/node-registry.ts (2)
packages/nodes/src/gmail/gmail.node.ts (1)
GmailNode(4-43)packages/nodes/src/gmail/gmail.service.ts (2)
GmailService(11-111)constructor(14-31)
packages/nodes/src/index.ts (1)
packages/nodes/src/gmail/gmail.service.ts (1)
GmailService(11-111)
packages/nodes/src/gmail/gmail.executor.ts (2)
packages/nodes/src/gmail/gmail.service.ts (1)
GmailService(11-111)packages/nodes/src/google-sheets/google-sheets.node.ts (1)
getExecutor(41-43)
apps/worker/src/index.ts (3)
apps/worker/src/engine/registory.ts (2)
register(7-9)register(47-47)packages/nodes/src/registry/node-registry.ts (1)
register(19-44)apps/worker/src/tests/test.ts (1)
testDirect(4-14)
apps/worker/src/engine/registory.ts (2)
apps/worker/src/types.ts (1)
NodeExecutor(14-16)packages/nodes/src/google-sheets/google-sheets.node.ts (2)
getExecutor(41-43)GoogleSheetNode(4-44)
packages/nodes/src/google-sheets/google-sheets.service.ts (1)
packages/nodes/src/index.ts (1)
GoogleSheetsService(13-13)
🔇 Additional comments (12)
packages/nodes/tsconfig.tsbuildinfo (1)
1-1: LGTM!Build metadata correctly updated to include the new Gmail node source file.
packages/nodes/src/gmail/gmail.executor.ts (1)
74-76: LGTM!Named export aligns with the import in
packages/nodes/src/index.ts. The commented-out export alternatives can be removed for cleaner code.packages/nodes/src/index.ts (1)
4-5: LGTM!Named export for
GmailExecutorcorrectly matches the export style ingmail.executor.ts.packages/nodes/src/registry/node-registry.ts (1)
71-74: LGTM!Gmail node registration follows the established pattern used by GoogleSheetNode.
packages/nodes/src/gmail/gmail.node.ts (1)
40-42: LGTM!The
getExecutor()method correctly instantiates and returns a newGmailExecutorinstance.apps/worker/src/types.ts (1)
2-2: No breaking change risk for optional nodeId.Making
nodeIdoptional inExecutionContextdoes not introduce a breaking change. The executor implementations (GoogleSheetsNodeExecutor, GmailExecutor) define and use their ownNodeExecutionContextinterface that does not includenodeId. Additionally,nodeIdis commented out inapps/worker/src/engine/executor.tsand is never accessed by any executor implementation, so the optional status has no runtime impact on existing code.package.json (1)
18-18: pnpm@10.27.0 is available and stable. No high-severity vulnerabilities or widely reported blockers exist. Review the release notes for the documented semi-breaking change related to store and registry behavior before upgrading.packages/db/prisma/schema.prisma (1)
119-123: LGTM - Field reordering for better organization.Moving the
sentfield next toworkflowExecutionIdimproves readability by grouping related fields together. This change has no functional impact.apps/worker/tsconfig.tsbuildinfo (1)
1-1: Build metadata reflects test file restructuring.This build info update correctly reflects the reorganization of test files from a single
test.tsto multiple test files under thetests/directory. No issues detected.apps/http-backend/tsconfig.tsbuildinfo (1)
1-1: Build metadata reflects backend restructuring.This update correctly reflects the addition of token scheduler and token refresh services, along with the reorganization of user routes. No issues detected.
apps/worker/src/engine/registory.ts (1)
3-3: LGTM - Import added for Google Sheets executor.The import statement is correct and follows the same pattern as the Gmail executor import.
apps/worker/src/index.ts (1)
3-3: LGTM - Registry import added.The import is correctly added to support executor initialization before starting the worker.
| status: "InProgress", | ||
| }, | ||
| }); | ||
| if(!update.error) console.log('updated the workflow execution') |
There was a problem hiding this comment.
Suspicious error check on Prisma update result.
The condition if(!update.error) will always be true because Prisma's update operation throws exceptions on errors rather than returning an object with an error field. If error checking is intended here, wrap the update in a try-catch block instead.
🔎 Proposed fix if error handling is needed
- const update = await prismaClient.workflowExecution.update({
- where: {
- id: workflowExecutionId,
- },
- data: {
- status: "InProgress",
- },
- });
- if(!update.error) console.log('updated the workflow execution')
+ try {
+ await prismaClient.workflowExecution.update({
+ where: {
+ id: workflowExecutionId,
+ },
+ data: {
+ status: "InProgress",
+ },
+ });
+ console.log('updated the workflow execution')
+ } catch (error) {
+ console.error('Failed to update workflow execution:', error);
+ throw error;
+ }🤖 Prompt for AI Agents
In @apps/worker/src/engine/executor.ts at line 37, The condition
`if(!update.error)` is wrong because Prisma's `update` throws on failure rather
than returning an `error` field; locate the Prisma call that assigns `update`
(e.g., `prisma.workflowExecution.update` in executor.ts), remove the
`if(!update.error)` check, and wrap the update call in a try-catch: on success
log the updated message (e.g., "updated the workflow execution"), and in the
catch log the error and either rethrow or handle it according to the surrounding
error-handling policy.
| // nodeId: node.id, | ||
| userId: data.workflow.userId, | ||
| credentialId: node.credentials[0]?.id, | ||
| credId: node.credentials[0]?.id, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and view the executor.ts file
find . -name "executor.ts" -type f | grep -v node_modules | head -20Repository: Dev-Pross/BuildFlow
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Let's look at the structure around lines 46-48 in the executor file
if [ -f "apps/worker/src/engine/executor.ts" ]; then
echo "=== File content around lines 40-55 ==="
cat -n "apps/worker/src/engine/executor.ts" | sed -n '40,55p'
fiRepository: Dev-Pross/BuildFlow
Length of output: 828
🏁 Script executed:
#!/bin/bash
# Search for all references to credentialId in TypeScript files
echo "=== Searching for 'credentialId' usage ==="
rg -n 'credentialId' --type ts -C 2 | head -100Repository: Dev-Pross/BuildFlow
Length of output: 8361
🏁 Script executed:
#!/bin/bash
# Search for references to nodeId in executor-related files
echo "=== Searching for 'nodeId' in executor files ==="
rg -n 'nodeId' --type ts -g '*executor*' -C 2 | head -100Repository: Dev-Pross/BuildFlow
Length of output: 470
🏁 Script executed:
#!/bin/bash
# Find all executor class implementations
echo "=== Finding executor implementations ==="
fd 'executor' --type f -e ts | grep -v node_modules | grep -v '.next'Repository: Dev-Pross/BuildFlow
Length of output: 240
🏁 Script executed:
#!/bin/bash
# Find the NodeExecutionContext type definition
echo "=== Finding NodeExecutionContext definition ==="
rg -n "NodeExecutionContext" --type ts | head -20Repository: Dev-Pross/BuildFlow
Length of output: 1426
🏁 Script executed:
#!/bin/bash
# Look at the types.ts or interface files
find . -name "types.ts" -o -name "*types.ts" | grep -v node_modules | head -10Repository: Dev-Pross/BuildFlow
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Check gmail executor implementation
if [ -f "packages/nodes/src/gmail/gmail.executor.ts" ]; then
echo "=== Gmail executor context usage ==="
cat -n "packages/nodes/src/gmail/gmail.executor.ts" | grep -A 5 -B 5 "context.cred"
fiRepository: Dev-Pross/BuildFlow
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Search for any usage of context.credentialId (the old field name)
echo "=== Searching for 'context.credentialId' usage ==="
rg -n 'context\.credentialId' --type tsRepository: Dev-Pross/BuildFlow
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Check if NodeExecutionContext is defined with credId or credentialId
echo "=== Searching for NodeExecutionContext definition ==="
rg -n "interface NodeExecutionContext|type NodeExecutionContext" --type ts -A 10Repository: Dev-Pross/BuildFlow
Length of output: 1740
🏁 Script executed:
#!/bin/bash
# Check worker types.ts for NodeExecutionContext
if [ -f "apps/worker/src/types.ts" ]; then
echo "=== Worker types.ts content ==="
cat -n "apps/worker/src/types.ts"
fiRepository: Dev-Pross/BuildFlow
Length of output: 576
🏁 Script executed:
#!/bin/bash
# Check if nodeId is used anywhere in the codebase
echo "=== Searching for references to node.id or nodeId usage ==="
rg -n "node\.id" --type ts apps/worker/src/engine/ -A 2 -B 2Repository: Dev-Pross/BuildFlow
Length of output: 481
🏁 Script executed:
#!/bin/bash
# Search for any executor that might expect nodeId
echo "=== Checking if any code expects nodeId in context ==="
rg -n "context\.nodeId|nodeId" --type ts packages/nodes/ | head -20Repository: Dev-Pross/BuildFlow
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Check if there's a type mismatch - does credId match credentialsId?
echo "=== Checking type definition mismatch ==="
echo "Type definition in types.ts line 4:"
sed -n '4p' apps/worker/src/types.ts
echo ""
echo "Actual context object line 48 in executor.ts:"
sed -n '48p' apps/worker/src/engine/executor.ts
echo ""
echo "Executor interface definition in google-sheets:"
grep -A 5 "interface NodeExecutionContext" packages/nodes/src/google-sheets/google-sheets.executor.ts | head -6Repository: Dev-Pross/BuildFlow
Length of output: 460
🏁 Script executed:
#!/bin/bash
# Search for any usage of context.credentialsId (the type definition name)
echo "=== Checking if credentialsId is used anywhere ==="
rg -n 'context\.credentialsId|credentialsId' --type ts | head -20Repository: Dev-Pross/BuildFlow
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check if there are TypeScript compilation errors or type incompatibilities
echo "=== Looking for tsconfig and checking overall type structure ==="
find . -name "tsconfig.json" | head -5Repository: Dev-Pross/BuildFlow
Length of output: 272
Fix type definition mismatch in ExecutionContext interface.
The context object passed to executors uses credId (line 48), but the ExecutionContext type definition in apps/worker/src/types.ts still declares it as credentialsId (line 4). Update the type definition to match:
credId?: string; // instead of credentialsId?: string;The nodeId being commented out is appropriate since it's optional in the type definition and no executor implementations use it.
🤖 Prompt for AI Agents
In @apps/worker/src/engine/executor.ts around lines 46 - 48, The
ExecutionContext type in apps/worker/src/types.ts is mismatched with the object
passed to executors: callers use credId but the type currently declares
credentialsId. Update the ExecutionContext interface by replacing or renaming
the property credentialsId?: string to credId?: string (optional) so it matches
the context passed from executor code (e.g., where credId is read in
executor.ts); leave nodeId optional as-is.
| inputData: currentInputData, | ||
| }; | ||
|
|
||
| console.log(`Executing with context: ${context}`) |
There was a problem hiding this comment.
Fix object logging to output readable JSON.
The template literal ${context} will output [object Object] instead of the actual context data, making this log useless for debugging. Use JSON.stringify() to properly serialize the object, as done correctly on line 66.
🔎 Proposed fix
- console.log(`Executing with context: ${context}`)
+ console.log(`Executing with context: ${JSON.stringify(context, null, 2)}`)📝 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.
| console.log(`Executing with context: ${context}`) | |
| console.log(`Executing with context: ${JSON.stringify(context, null, 2)}`) |
🤖 Prompt for AI Agents
In @apps/worker/src/engine/executor.ts at line 52, The console.log in
executor.ts is serializing the context with a template literal which yields
“[object Object]”; update the log in the function where `console.log(\`Executing
with context: ${context}\`)` appears to serialize the object using
JSON.stringify (match the existing approach used on line 66) — e.g., replace the
template interpolation with JSON.stringify(context, null, 2) so the context
prints as readable JSON while preserving the surrounding message.
| // if (!execute.success) { | ||
| // await prismaClient.workflowExecution.update({ | ||
| // where: { id: workflowExecutionId }, | ||
| // data: { | ||
| // status: "Failed", | ||
| // error: execute.error, | ||
| // completedAt: new Date(), | ||
| // }, | ||
| // }); | ||
| // return; | ||
| // } |
There was a problem hiding this comment.
Critical: Restore error handling for failed node executions.
The error handling block that marks workflows as "Failed" has been completely commented out. This means:
- If a node execution fails, the error is silently ignored
- The workflow continues processing subsequent nodes
- The workflow is marked as "Completed" even though it failed
- No error information is recorded in the database
This breaks the core execution contract and will mislead users about workflow success/failure status.
🔎 Proposed fix to restore error handling
console.log(`Executing with context: ${context}`)
const execute = await register.execute(nodeType, context);
- // if (!execute.success) {
- // await prismaClient.workflowExecution.update({
- // where: { id: workflowExecutionId },
- // data: {
- // status: "Failed",
- // error: execute.error,
- // completedAt: new Date(),
- // },
- // });
- // return;
- // }
+ if (!execute.success) {
+ await prismaClient.workflowExecution.update({
+ where: { id: workflowExecutionId },
+ data: {
+ status: "Failed",
+ error: execute.error,
+ completedAt: new Date(),
+ },
+ });
+ console.log(`Workflow failed at node ${node.name}: ${execute.error}`);
+ return;
+ }
currentInputData = execute.output;
console.log("output: ", JSON.stringify(execute))📝 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 (!execute.success) { | |
| // await prismaClient.workflowExecution.update({ | |
| // where: { id: workflowExecutionId }, | |
| // data: { | |
| // status: "Failed", | |
| // error: execute.error, | |
| // completedAt: new Date(), | |
| // }, | |
| // }); | |
| // return; | |
| // } | |
| console.log(`Executing with context: ${context}`) | |
| const execute = await register.execute(nodeType, context); | |
| if (!execute.success) { | |
| await prismaClient.workflowExecution.update({ | |
| where: { id: workflowExecutionId }, | |
| data: { | |
| status: "Failed", | |
| error: execute.error, | |
| completedAt: new Date(), | |
| }, | |
| }); | |
| console.log(`Workflow failed at node ${node.name}: ${execute.error}`); | |
| return; | |
| } | |
| currentInputData = execute.output; | |
| console.log("output: ", JSON.stringify(execute)) |
🤖 Prompt for AI Agents
In @apps/worker/src/engine/executor.ts around lines 54 - 64, Restore the omitted
error-handling branch so failed node executions stop the workflow: in the
executor where the execute result is checked (use execute.success and
execute.error), re-enable the prismaClient.workflowExecution.update call to set
status to "Failed", set error to execute.error and completedAt to new Date(),
then return early (use workflowExecutionId to identify the record) so subsequent
nodes are not processed and the workflow is not marked completed.
| // For now, this cast suppresses the type error. Refactor as soon as possible! | ||
|
|
||
|
|
||
| //wehen visits this next time make sure chang gmail executor implements NodeExecutor |
There was a problem hiding this comment.
Fix typo in comment.
"wehen" should be "when".
🔎 Proposed fix
- //wehen visits this next time make sure chang gmail executor implements NodeExecutor
+ //when someone visits this next time make sure to change gmail executor to implement NodeExecutor📝 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.
| //wehen visits this next time make sure chang gmail executor implements NodeExecutor | |
| //when someone visits this next time make sure to change gmail executor to implement NodeExecutor |
🤖 Prompt for AI Agents
In @apps/worker/src/engine/registory.ts at line 40, Fix the typo in the comment
that reads "wehen visits this next time make sure chang gmail executor
implements NodeExecutor" by replacing "wehen" with "when" (so it reads "when
visits this next time..."); update the same comment text in registory.ts where
that comment appears to correct the spelling.
| name: "From", | ||
| type: "email", | ||
| require: true | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for gmail-related service and executor implementations
find packages/nodes/src/gmail -type f -name "*.ts" | head -20Repository: Dev-Pross/BuildFlow
Length of output: 186
🏁 Script executed:
# Look at the gmail.node.ts file structure to understand the From field context
wc -l packages/nodes/src/gmail/gmail.node.tsRepository: Dev-Pross/BuildFlow
Length of output: 104
🏁 Script executed:
# Search for GmailExecutor and GmailService implementations
rg -l "GmailExecutor|GmailService" packages/nodes/src/gmail/ --type=tsRepository: Dev-Pross/BuildFlow
Length of output: 186
🏁 Script executed:
# Read the entire gmail.node.ts file to understand context
cat -n packages/nodes/src/gmail/gmail.node.tsRepository: Dev-Pross/BuildFlow
Length of output: 1532
🏁 Script executed:
# Read gmail.executor.ts to see how From field is used
cat -n packages/nodes/src/gmail/gmail.executor.tsRepository: Dev-Pross/BuildFlow
Length of output: 2555
🏁 Script executed:
# Read gmail.service.ts to see OAuth and sender logic
cat -n packages/nodes/src/gmail/gmail.service.tsRepository: Dev-Pross/BuildFlow
Length of output: 3754
Remove the "From" field entirely—it is never used and users are forced to enter a required value that is ignored.
The code extracts only to, subject, and body from config (line 57 in gmail.executor.ts), and the sendEmail method never sets a From header. Gmail API automatically uses the authenticated user's email when no From header is provided, making this field not just redundant but completely unused. Keeping it as required is confusing and misleading.
🤖 Prompt for AI Agents
In @packages/nodes/src/gmail/gmail.node.ts around lines 12 - 15, Remove the
unused "From" config field from the Gmail node definition (the field with name
"From" and type "email") so users are no longer required to supply it; update
the node's config/schema in gmail.node.ts to delete that field, remove any
validation or required-flag references to "From", and ensure gmail.executor.ts
continues to extract only to/subject/body (no change needed there) and that no
code attempts to read a "From" value; also remove or update any tests, docs, or
UI references that mention the "From" field to avoid confusion.
| { | ||
| name: "From", | ||
| type: "email", | ||
| require: true |
There was a problem hiding this comment.
Fix typo: require should be required.
The property name require is incorrect and should be required to properly validate that these fields are mandatory. This typo appears on lines 14, 19, and 24.
🔎 Proposed fix
{
name: "From",
type: "email",
- require: true
+ required: true
},
{
name: "To",
type: "email",
- require: true
+ required: true
},
{
name:"Body",
type: "textArea",
- require: true
+ required: true
}Also applies to: 19-19, 24-24
🤖 Prompt for AI Agents
In @packages/nodes/src/gmail/gmail.node.ts at line 14, There are three typos
using the property name "require" in the Gmail node parameter definitions;
replace each "require" key with the correct validator key "required" (i.e.,
update every parameter object that currently has "require: true" to "required:
true") so the fields are properly validated; locate the occurrences by searching
for the string "require: true" in the gmail.node.ts parameter definitions and
change them to "required: true".
|
|
||
| static async register(){ | ||
| await NodeRegistry.register(this.definition) | ||
| // console.log(`✅ Registered node: ${this.definition.name}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out debug logs.
These commented console.log statements are debug artifacts that should be removed to keep the codebase clean.
🔎 Proposed cleanup
static async register(){
await NodeRegistry.register(this.definition)
- // console.log(`✅ Registered node: ${this.definition.name}`);
await NodeRegistry.registerTrigger(this.definition)
- // console.log(`✅ Registered Trigger: ${this.definition.name}`);
}Also applies to: 37-37
🤖 Prompt for AI Agents
In @packages/nodes/src/gmail/gmail.node.ts at line 35, Remove the commented-out
debug console.log statements left in the Gmail node (e.g., the lines containing
"// console.log(`✅ Registered node: ${this.definition.name}`);" and any other
commented "console.log" instances around the registration logic); simply delete
these commented lines from the gmail.node.ts so only active code remains and no
commented debug artifacts are left.
| if(files){ | ||
| return { | ||
| success: true, | ||
| data: files | ||
| } | ||
| } | ||
| return { | ||
| success: false, | ||
| data: null | ||
| } |
There was a problem hiding this comment.
files object is always truthy; check files.data.files instead.
The drive.files.list() returns a response object, which is always truthy. The check should verify if the actual files array exists and has entries.
🔎 Proposed fix
- if(files){
+ if(files.data.files && files.data.files.length > 0){
return {
success: true,
- data: files
+ data: files.data.files
}
}🤖 Prompt for AI Agents
In @packages/nodes/src/google-sheets/google-sheets.service.ts around lines 55 -
64, The current truthy check uses the full response object returned by
drive.files.list() (variable files); change the condition to verify the actual
items array (files.data.files) and whether it has length, and return that array
as data; specifically update the branch in the function using drive.files.list()
to check "files?.data?.files && files.data.files.length" and on success return
success: true with data: files.data.files, otherwise return success: false and
data: null.
| isTokenExpired():boolean { | ||
| const credentials = this.auth.credentials; | ||
| if( !credentials.expiry_date) return false; | ||
|
|
||
| return Date.now() >= credentials.expiry_date - (5 *60 * 1000); | ||
| } |
There was a problem hiding this comment.
Token expiry check returns false when expiry_date is missing.
If credentials.expiry_date is undefined or 0, this returns false (not expired). This is inconsistent with GmailService.isTokenExpired() which returns true when expiry info is missing. Returning true (expired) is safer as it triggers a refresh.
🔎 Proposed fix to align with GmailService behavior
isTokenExpired():boolean {
const credentials = this.auth.credentials;
- if( !credentials.expiry_date) return false;
+ if( !credentials || !credentials.expiry_date) return true;
return Date.now() >= credentials.expiry_date - (5 *60 * 1000);
}📝 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.
| isTokenExpired():boolean { | |
| const credentials = this.auth.credentials; | |
| if( !credentials.expiry_date) return false; | |
| return Date.now() >= credentials.expiry_date - (5 *60 * 1000); | |
| } | |
| isTokenExpired():boolean { | |
| const credentials = this.auth.credentials; | |
| if( !credentials || !credentials.expiry_date) return true; | |
| return Date.now() >= credentials.expiry_date - (5 *60 * 1000); | |
| } |
🤖 Prompt for AI Agents
In @packages/nodes/src/google-sheets/google-sheets.service.ts around lines 105 -
110, The isTokenExpired() method currently returns false when
this.auth.credentials.expiry_date is missing, but it should behave like
GmailService.isTokenExpired() and treat missing/invalid expiry info as expired
to force refresh; update isTokenExpired() to return true when
credentials.expiry_date is undefined, null, or 0, and otherwise compute
Date.now() >= expiry_date - (5*60*1000), referencing the isTokenExpired() method
and this.auth.credentials.expiry_date to locate the change.
Vamsi-o
left a comment
Vamsi-o
left a comment
There was a problem hiding this comment.
Make sure you add all the necessary changes requested
…Google Sheets service
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.