Added Email Integration and complete worker #50
Conversation
📝 WalkthroughWalkthroughThis PR implements a workflow execution engine with OAuth credential management. It adds node executors registered to process workflow steps, integrates Gmail for email sending, implements OAuth endpoints for credential exchange and storage, and introduces a worker service that executes workflow nodes sequentially through the registry. Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant Backend as HTTP Backend
participant Kafka as Message Queue
participant Worker as Worker Service
participant Registry as Node Registry
participant Executor as Gmail Executor
participant GoogleAPI as Google APIs
participant Database as Database
User->>Backend: GET /auth/google/initiate
Backend->>GoogleAPI: Generate Auth URL (with userId state)
GoogleAPI-->>Backend: Authorization URL
Backend-->>User: Redirect to Google OAuth
User->>GoogleAPI: Grant permissions
GoogleAPI->>Backend: Redirect to /callback with code
Backend->>GoogleAPI: Exchange code for tokens
GoogleAPI-->>Backend: Access & Refresh tokens
Backend->>Database: Save credentials (type: google_oauth)
Database-->>Backend: Credential saved
Backend-->>User: Redirect to workflow page
User->>Backend: Trigger workflow
Backend->>Kafka: Publish workflowExecutionId
Kafka-->>Worker: Consume message
Worker->>Database: Load workflow & nodes by executionId
Database-->>Worker: Workflow with nodes
Worker->>Database: Update execution status to InProgress
loop For each node
Worker->>Registry: execute(nodeType, context)
Registry->>Executor: GmailExecutor.execute(context)
Executor->>Database: Get credentials by userId & credentialId
Database-->>Executor: OAuthTokens
Executor->>GoogleAPI: Check if token expired
alt Token expired
Executor->>GoogleAPI: Refresh tokens
GoogleAPI-->>Executor: New tokens
Executor->>Database: Update credentials
end
Executor->>GoogleAPI: Send email via Gmail API
GoogleAPI-->>Executor: Success
Executor-->>Registry: ExecutionResult
Registry-->>Worker: Result
end
Worker->>Database: Update execution status to Completed
Database-->>Worker: Status updated
Worker->>Kafka: Commit offset
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Fix all issues with AI Agents 🤖
In @apps/http-backend/src/index.ts:
- Line 22: Remove the temporary inline change marker on the route registration
line; update the app.use('/auth/google', googleAuth) statement to remove the
trailing comment "← CHANGED THIS LINE!" (and search for any similar inline
review markers in this file to delete) so the route declaration contains only
the code without review annotations.
In @apps/http-backend/src/routes/google_callback.ts:
- Line 13: The code uses const userId = req.user?.id and then embeds it into the
OAuth state without checking for undefined; update the Google callback handler
to guard against a missing req.user by validating userId before building the
state (e.g., if (!req.user || !userId) return res.status(400).send(...) or
redirect to login), and only stringify/include userId in the state when it is
defined; apply the same check where userId is used again (the later usage on the
state at the other occurrence) to avoid "undefined" being sent.
- Line 65: Replace hardcoded "http://localhost:3000/..." redirects in
google_callback.ts with an environment-driven base URL: read a FRONTEND_BASE_URL
(or similarly named) env var and use it to build the redirect target for all
res.redirect calls in the Google callback handler (the redirects at the
res.redirect(...) lines and the other occurrences at lines 68-70). Validate the
env var has a fallback or throw a clear error if missing, and update the
redirect expressions to concatenate the path (e.g.,
`${frontendBaseUrl}/workflow`) so production/staging use the correct host.
In @apps/http-backend/src/routes/userRoutes/userRoutes.ts:
- Around line 154-201: Remove the large commented-out GET credentials handler
block (the commented router.get('/getCredentials/:type' async handler) in
userRoutes.ts) — delete the entire commented region to eliminate noise, ensuring
you don't remove the active implementation below; verify no dependent commented
imports or symbols remain (e.g., GoogleSheetsNodeExecutor references) and run
tests/lint to confirm nothing else depended on the commented block.
- Around line 209-217: Remove the debug console logging that prints sensitive
user info (the console.log calls referencing req.user and userId) inside the
user credential route handler; either delete those console.log lines or replace
them with a structured logger at an appropriate debug level (e.g., logger.debug)
that avoids logging PII, and ensure you still reference req.user.sub (userId)
and req.params.type for logic but do not output their values to stdout.
In @apps/worker/src/engine/executor.ts:
- Around line 42-48: The context object in executor.ts uses the wrong field name
`credentialId` (singular) which doesn't match the `ExecutionContext` interface's
`credentialsId` (plural); update the context literal in the executor (the
`context` variable) to use `credentialsId` and populate it with the same value
currently assigned to `credentialId` (e.g., node.credentials[0]?.id) so the
shape matches the `ExecutionContext` defined in apps/worker/src/types.ts and
executors can access credentials correctly.
In @apps/worker/src/engine/registory.ts:
- Around line 33-42: GmailExecutor is being force-cast to NodeExecutor in
registory.initialize; fix this by updating the GmailExecutor class
(packages/nodes/src/gmail/gmail.executor.ts) to explicitly implement the
NodeExecutor interface, replacing usages of NodeExecutionContext with the worker
app's ExecutionContext and ensuring its execute/handle methods return the app's
ExecutionResult type; align field names (e.g., change credId → credentialsId)
and any payload/return shapes to match NodeExecutor signatures, then remove the
"as unknown as NodeExecutor" cast from registory.initialize and register new
GmailExecutor() directly.
- Around line 26-31: The catch block currently returns the raw error object into
the ExecutionResult.error field which is typed as string | undefined; update the
catch to convert the error to a string (e.g. use error?.message ?? String(error)
or String(error)) before assigning to the error property so
ExecutionResult.error receives a string and the types align (modify the catch in
registory.ts where the catch returns { success: false, error: error } to return
{ success: false, error: error?.message ?? String(error) }).
In @apps/worker/src/index.ts:
- Around line 4-5: Remove the leftover commented assignment for clientId (the
line containing // clientId: "Processing App") so only the active clientId =
"BuildFlow-Worker" remains; locate the clientId assignment in the top of the
file (the commented and uncommented lines shown together) and delete the
commented line, then run formatting/linting to ensure no trailing whitespace or
comment artifacts remain.
- Line 40: Remove the leftover commented test code "// await new Promise((r) =>
setTimeout(r, 5000));" from apps/worker/src/index.ts to keep the codebase clean;
delete that single-line comment, ensure no other similar debugging comments
remain in the module (e.g., near the module initialization/startup code), and
run your linter/formatter to confirm no stylistic issues remain.
- Around line 32-39: Remove the temporary debug console.log statements around
message processing (the logs printing "Recieved workflowExecutionId", "The value
we get from processor", and the toString version) and replace the final
misspelled log with a concise, production-safe log or structured logger call
that correctly spells "executing" (e.g., log the outcome of
executeWorkflow(workflowExecutionId) as "result of executing" or use the
application's logger). Ensure any remaining logs use the project logger (not
console) and avoid printing potentially sensitive message.value directly.
In @apps/worker/src/tests/setup-test.ts:
- Around line 54-58: The test file contains a hardcoded real email in the config
object; replace the value of config.to in apps/worker/src/tests/setup-test.ts
with a non-PII test placeholder (e.g., test@example.com or no-reply@example.com)
so tests don’t include real email addresses, and ensure any related test
assertions still pass after the change.
In @apps/worker/src/tests/test.ts:
- Around line 22-42: The debug function with hardcoded executionId and immediate
invocation (function debug(), executionId value, and call debug()) is leftover
debugging code; either remove the entire debug function and its invocation, or
refactor it into a reusable exported helper (e.g., export async function
debugExecution(executionId: string)) that calls
prismaClient.workflowExecution.findUnique and prismaClient.workflow.findUnique
(including nodes) and remove the hardcoded executionId and top-level debug()
call so it’s only used from tests or utilities.
In @apps/worker/tsconfig.tsbuildinfo:
- Line 1: The build is picking up a nonexistent ./src/test.ts and a
development-only ./src/tests/test.ts; update tsconfig.json to exclude the tests
directory (e.g., add "exclude": ["src/tests", "src/tests/**"] or adjust
"files"/"include" to omit tests) so test-only modules aren’t compiled, then
remove or regenerate the stale apps/worker/tsconfig.tsbuildinfo entry that
references ./src/test.ts (delete the tsbuildinfo or run a clean build) to clear
the invalid reference; target symbols: tsconfig.json,
apps/worker/tsconfig.tsbuildinfo, ./src/test.ts, ./src/tests/test.ts.
- Line 1: The source filename contains a typo: rename the module referenced as
"./src/engine/registory.ts" to "./src/engine/registry.ts" (rename the actual
file from registory.ts → registry.ts) and update every import/export that
references "registory" to "registry" (search for imports like from
'./engine/registory' and update them); after renaming, run a TypeScript build to
regenerate tsconfig.tsbuildinfo and fix any broken imports or exported symbols
(e.g., the execution registry class or functions) that now point to the
corrected module name.
In @packages/db/src/seed.ts:
- Around line 62-66: Replace the hard-coded personal address set on the config
object's to field (the line setting to: "iamvamsi0@gmail.com") with a
configurable value—read from an environment variable (e.g.,
process.env.SEED_EMAIL) and/or a non-personal placeholder (e.g.,
"example@example.com") as a fallback; update the seed/config initialization that
constructs this config object (the same scope where to is defined) to use the
env var and add a note to .env.example or docs to define SEED_EMAIL so no
personal emails are committed.
- Around line 74-86: The WorkflowExecution creation is not idempotent because it
uses prismaClient.workflowExecution.create with a hardcoded id; replace that
create call with an upsert (using prismaClient.workflowExecution.upsert) keyed
on id ("test_exec_001") so repeated seed runs update or return the existing
record instead of erroring, ensuring you supply the same data shape for create
and update and keep the console.log("✅ Execution created:", execution.id)
behavior.
- Around line 88-89: The console output prints a hardcoded recipient
("your-email@gmail.com") that may not match the configured recipient; update the
two console.log calls in packages/db/src/seed.ts (the lines that print the "Test
workflow ready!" and "Email will be sent to: ..." messages) to use the actual
recipient variable instead of the literal string—e.g., reference the configured
recipient variable (recipientEmail, email, config.recipient, or similar in this
file) or interpolate the real value returned/used when seeding so the logged
email always reflects the actual recipient.
In @packages/nodes/package.json:
- Line 22: Remove the unnecessary dependency "@types/dotenv" from package.json
and re-add "@types/node" under devDependencies (restore its previous semver) so
TypeScript has Node core types; ensure only dotenv remains in dependencies
(dotenv v17.2.3 provides its own types) and update package.json devDependencies
to include "@types/node" again.
In @packages/nodes/src/common/google-oauth-service.ts:
- Around line 83-110: The getCredentials method currently ignores the userId and
queries prisma.credential.findFirst without enforcing ownership, allowing any
credId to be retrieved; update the where clause in getCredentials to include
userId (e.g., where: { id: credId, userId, type: "google_oauth" }) so the DB
enforces credential ownership, remove or replace verbose console.log statements
with proper logging if needed, and ensure the method still returns null or
throws appropriately when no matching credential exists (preserving the existing
error handling behavior).
- Line 54: Fix the typo in the error message string inside
google-oauth-service.ts by changing "Unknow error" to "Unknown error" where the
template literal is built (the code that throws or logs `Failed to exchange code
for token: ${error instanceof Error ? error.message : "Unknow error"}`); update
the fallback text to "Unknown error" so the message reads `Failed to exchange
code for token: ${error instanceof Error ? error.message : "Unknown error"}`.
In @packages/nodes/src/gmail/gmail.executor.ts:
- Around line 4-15: The local interfaces NodeExecutionContext and
NodeExecutionResult duplicate and conflict with the canonical ExecutionContext
and ExecutionResult used by the worker, causing the unsafe cast in registory.ts;
remove the local interfaces and replace their usage with imports of
ExecutionContext and ExecutionResult from the worker/types module (or a shared
types re-export) so the executor uses the same credential field names (e.g.,
credentialsId) and shapes across the codebase; update any references in
gmail.executor.ts to use the imported types and run type-checking to confirm the
unsafe cast at registory.ts line 40 is no longer required.
- Around line 46-54: The code checks this.gmailService.isTokenExpired() and
calls refreshAccessToken() but doesn’t handle a failed refresh; update the block
around refreshAccessToken() to check refreshResult.success and if false, return
early (or throw a clear error) to avoid proceeding with an expired
token—specifically modify the refresh logic that calls
this.gmailService.refreshAccessToken() and the subsequent
this.oauthService.updateCredentials(...) so that on refreshResult.success ===
false you stop execution and surface a clear failure instead of continuing to
send the email.
- Around line 41-43: The constructor in gmail.executor.ts unsafely casts
credentials.tokens to GmailCredentials before validating their shape; update the
code to validate required fields (access_token, refresh_token, token_type,
expiry_date) on the object returned by GoogleOAuthService.getCredentials (or
strengthen GoogleOAuthService.getCredentials to return a validated OAuthTokens
type) before passing into new GmailService(GmailCredentials). Specifically, add
a guard or validation function that checks credentials.config /
credentials.tokens (the OAuthTokens returned by google-oauth-service.ts) for the
required properties and throw or handle a clear error if missing, or adjust the
google-oauth-service.ts contract so getCredentials returns a properly
typed/validated OAuthTokens rather than relying on as casting.
In @packages/nodes/src/google-sheets/google-sheets.service.ts:
- Around line 121-123: The error message in the catch block currently reads
"Falied to refresh token" which contains a typo; update the thrown error string
in the catch that uses `throw new Error(\`Falied to refresh token: ${error}\`);`
to read `Failed to refresh token` so it becomes `throw new Error(\`Failed to
refresh token: ${error}\`);`.
In @packages/nodes/src/index.ts:
- Line 10: Remove the debug console.log call currently printing "server running
perfectly from node" from packages/nodes/src/index.ts; locate the standalone
console.log in the module initialization (the top-level script in index.ts) and
delete it so no debug output remains in production.
- Line 3: Remove the stray trailing backslash from the malformed comment on line
3 so the comment becomes a valid single-line comment (e.g., "// Fixed lint
error: Use correct export name 'GoogleSheetNode'"); ensure no other stray
backslashes remain in that comment or adjacent comments and preserve the
intended text referencing GoogleSheetNode.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/nodes/src/google-sheets/test.ts (1)
13-18: Critical: Remove hardcoded OAuth tokens from source control.Real OAuth credentials (access token and refresh token) are hardcoded in this test file. Even if expired, these should never be committed to source control as they may:
- Still be valid and grant unauthorized access
- Be used to analyze token patterns
- Violate security best practices
🔎 Recommended fix
const testCredentials: GoogleSheetsCredentials = { - access_token: "ya29.a0ATi6K2tMD5v8dipZChm71fsgKuzkAsPrTuQ96iz6FSJaVVJNDE-qDDTrMTImcJiPm1E9Ev2A7LGhIrkRPX-sGSTmIkimr8T28r6V45GJK0RfpUkUFMRc8Aju4PGfUzbS3XGdwwZvjV2MiCyVYsyMawnOaBMYITAlViuLR8cpaTaQVITQQiHEYT6Z5lG9gHwvdZlvjpsaCgYKAeESARYSFQHGX2Mi4sCzLqC2VjxJ5OZIyxiSTg0206", - refresh_token: "1//0g--4dA-1bwg1CgYIARAAGBASNwF-L9IrLJEWrYso9Qdozd9MNc6p4ViZ2RwnWWhqRyZgzVHeny2qoeiIirKAA3R07_ZTBHrn88Y", + access_token: process.env.GOOGLE_TEST_ACCESS_TOKEN || "", + refresh_token: process.env.GOOGLE_TEST_REFRESH_TOKEN || "", expiry_date: 1234567890000, token_type: "Bearer" };Action items:
- Remove these tokens from the codebase immediately
- Revoke these tokens in Google Cloud Console
- Update the code to load tokens from environment variables
- Add
.envto.gitignoreif not already present- Consider using a
.env.examplefile with placeholder valuesapps/worker/package.json (1)
2-2: Fix typo in package name.The package name is misspelled as
@repo/workderinstead of@repo/worker. This could cause issues with workspace references and tooling.🔎 Proposed fix
- "name": "@repo/workder", + "name": "@repo/worker",apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
257-281: Missing response path and inconsistent error handling.
- The
if(creds)check is unnecessary sincefindManyalways returns an array, but if removed, the return is unconditional which is cleaner.- The error message on line 279 is missing "error" (says "Internal server from" instead of "Internal server error from").
- Typo on line 276: "Unkown" should be "Unknown".
🔎 Proposed fix
router.get('/getAllCreds', userMiddleware, async(req: AuthRequest, res:Response) =>{ try{ if(!req.user){ return res.status(statusCodes.BAD_REQUEST).json({ message: "User is not Loggedin" }) } const userId = req.user.sub; const creds = await prismaClient.credential.findMany({ where:{ userId: userId} }) - if(creds){ - return res.status(statusCodes.OK).json({ - message: "Fetched all credentials of the User!", - data: creds - }) - } + return res.status(statusCodes.OK).json({ + message: "Fetched all credentials of the User!", + data: creds + }) } catch(e){ - console.log("Error Fetching the credentials ", e instanceof Error ? e.message : "Unkown reason"); + console.log("Error Fetching the credentials ", e instanceof Error ? e.message : "Unknown reason"); return res .status(statusCodes.INTERNAL_SERVER_ERROR) - .json({ message: "Internal server from fetching the credentials" }); + .json({ message: "Internal server error from fetching the credentials" }); } })
🟠 Major comments (17)
apps/http-backend/src/routes/userRoutes/userRoutes.ts-154-201 (1)
154-201: Remove commented-out code.This large block of commented code (~50 lines) should be removed. It adds noise and is now superseded by the active implementation below. If needed for reference, Git history preserves it.
packages/db/src/seed.ts-62-66 (1)
62-66: Remove hard-coded personal email address.The hard-coded email
iamvamsi0@gmail.comappears to be a personal email and should not be committed to source control. Use an environment variable or placeholder instead.🔎 Proposed fix
config: { - to: "iamvamsi0@gmail.com", // ← CHANGE THIS! + to: process.env.TEST_EMAIL_RECIPIENT || "test@example.com", subject: "BuildFlow Test Email", body: "This is a test email from BuildFlow worker!", },apps/worker/src/tests/update-node.ts-1-17 (1)
1-17: Remove hard-coded personal email and improve test robustness.
- The email
iamvamsi0@gmail.comappears to be a personal email and should not be committed.- The hard-coded UUID makes this test non-portable.
- No error handling - if the update fails, it will throw an unhandled promise rejection.
🔎 Proposed fix
import { prismaClient } from "@repo/db/client"; +const TEST_EMAIL = process.env.TEST_EMAIL_RECIPIENT || "test@example.com"; +const TEST_NODE_ID = process.env.TEST_NODE_ID || "test_node_001"; + async function updateNode() { - await prismaClient.node.update({ - where: { id: 'f058ac5a-f329-4d6f-986a-e9a1f65d0b14' }, - data: { - config: { - to: 'iamvamsi0@gmail.com', - subject: 'Test from BuildFlow - Self Send', - body: 'This email is sent from and to the same account: iamvamsi0@gmail.com' + try { + await prismaClient.node.update({ + where: { id: TEST_NODE_ID }, + data: { + config: { + to: TEST_EMAIL, + subject: 'Test from BuildFlow - Self Send', + body: `This is a test email from BuildFlow worker.` + } } - } - }); - console.log('✅ Updated node config'); + }); + console.log('✅ Updated node config'); + } catch (error) { + console.error('❌ Failed to update node:', error); + process.exit(1); + } } updateNode();apps/http-backend/src/routes/userRoutes/userRoutes.ts-209-217 (1)
209-217: Remove debug logging of user data.These console.log statements expose user information (
req.user,userId) which could be a privacy/compliance concern in production logs. Consider removing them or using a structured logger with appropriate log levels.🔎 Proposed fix
async (req: AuthRequest, res) => { try { - console.log("user from getcredentials: ", req.user) if (!req.user) { return res.status(statusCodes.BAD_REQUEST).json({ message: "User is not Loggedin" }) } const userId = req.user.sub; const type = req.params.type - console.log(userId, " -userid")apps/http-backend/src/index.ts-22-22 (1)
22-22: Remove inline change marker comment.The comment "← CHANGED THIS LINE!" on line 22 appears to be a temporary marker for review and should be removed before merging.
🔎 Proposed fix
-app.use('/auth/google', googleAuth) // ← CHANGED THIS LINE! +app.use('/auth/google', googleAuth)apps/worker/src/index.ts-4-5 (1)
4-5: Remove commented code.The old
clientIdon line 4 should be removed as it's replaced by the new value on line 5.🔎 Proposed fix
- // clientId: "Processing App", clientId: "BuildFlow-Worker",apps/worker/src/index.ts-40-40 (1)
40-40: Remove commented code.The commented
setTimeouton line 40 should be removed if it's no longer needed for testing.🔎 Proposed fix
- // await new Promise((r) => setTimeout(r, 5000));apps/worker/src/index.ts-32-39 (1)
32-39: Remove debug logs and fix typo.Multiple console.log statements appear to be debug artifacts (lines 32-37). Line 39 contains a typo: "execuitng" should be "executing". Consider removing verbose debug logs before merging to production.
🔎 Proposed cleanup
- console.log(`Recieved workflowExecutionId ${workflowExecutionId}`); - console.log("The value we get from processor", message.value); - console.log( - "The value we get from processor to make it string", - message.value?.toString() - ); - const result = await executeWorkflow(workflowExecutionId) - console.log("This is the result fo the execuitng" , result) + const result = await executeWorkflow(workflowExecutionId); + console.log("Workflow execution result:", result);packages/nodes/src/index.ts-10-10 (1)
10-10: Remove debug console.log statement.The console.log on line 10 appears to be a debug artifact and should be removed from production code.
🔎 Proposed fix
export { default as GmailExecutor } from '../src/gmail/gmail.executor.js'; -console.log("server running perfectly from node ")apps/worker/src/tests/test.ts-22-42 (1)
22-42: Remove debug function or integrate it into the test suite.The
debugfunction (lines 22-40) with its hardcoded execution ID and immediate invocation on line 42 appears to be leftover debugging code. Consider removing it or refactoring it into a proper test utility.🔎 Option 1: Remove if no longer needed
- - -import { prismaClient } from "@repo/db/client"; - -async function debug() { - const executionId = "ca4210e3-c830-408e-a86d-961508e9b325"; - - // Step 1: Get execution only - const execution = await prismaClient.workflowExecution.findUnique({ - where: { id: executionId } - }); - console.log("Execution:", execution); - - if (execution?.workflowId) { - // Step 2: Get workflow - const workflow = await prismaClient.workflow.findUnique({ - where: { id: execution.workflowId }, - include: { nodes: true } - }); - console.log("Workflow:", workflow); - console.log("Number of nodes:", workflow?.nodes?.length); - } -} - -debug();Option 2: Refactor into a utility (if still needed)
export async function debugExecution(executionId: string) { const execution = await prismaClient.workflowExecution.findUnique({ where: { id: executionId } }); console.log("Execution:", execution); if (execution?.workflowId) { const workflow = await prismaClient.workflow.findUnique({ where: { id: execution.workflowId }, include: { nodes: true } }); console.log("Workflow:", workflow); console.log("Number of nodes:", workflow?.nodes?.length); } }Committable suggestion skipped: line range outside the PR's diff.
apps/worker/src/tests/setup-test.ts-54-58 (1)
54-58: Replace real email address with a test placeholder.The hardcoded email
"iamvamsi0@gmail.com"appears to be a real email address. Test files should avoid committing real PII to the repository.🔎 Proposed fix
config: { - to: "iamvamsi0@gmail.com", + to: "test@example.com", subject: "Test from BuildFlow Worker", body: "This is a test email sent from the worker!" }apps/worker/src/engine/registory.ts-33-42 (1)
33-42: Essential: Eliminate type cast by making GmailExecutor implement NodeExecutor.The type cast
as unknown as NodeExecutoron line 40 bypasses type safety and exists becauseGmailExecutoruses incompatible interfaces (NodeExecutionContextinstead ofExecutionContext, with field name mismatches likecredIdvscredentialsId). This creates a critical type safety gap in the execution pipeline.Required changes:
- Update
GmailExecutorinpackages/nodes/src/gmail/gmail.executor.tsto explicitly implementNodeExecutorinterface- Use
ExecutionContextandExecutionResultfromapps/worker/src/types.ts(see my comment on lines 4-15 of gmail.executor.ts)- Remove the type cast once interface compatibility is achieved
🔎 Example of proper registration after fixing GmailExecutor
initialize() { - // TODO: Ensure GmailExecutor implements NodeExecutor and is compatible with ExecutionContext - // If needed, adapt/extract a compatible Executor for registration. - // 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 - this.register("gmail", new GmailExecutor() as unknown as NodeExecutor); + this.register("gmail", new GmailExecutor()); console.log(`The current Executors are ${this.executors.size}`); }Committable suggestion skipped: line range outside the PR's diff.
packages/nodes/src/gmail/gmail.executor.ts-46-54 (1)
46-54: Handle token refresh failure explicitly.If the token is expired and refresh fails (
refreshResult.success === false), the code continues to attempt sending email with an expired token, which will fail at the Gmail API level with a potentially confusing error message.🔎 Proposed fix: Return early on 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: "Failed to refresh access token. Please re-authorize Gmail.", + }; } }packages/nodes/src/gmail/gmail.executor.ts-41-43 (1)
41-43: Add validation before type casting or strengthen the type contract.The cast
credentials.tokens as GmailCredentialsdepends onGoogleOAuthService.getCredentials()reliably returning tokens with all required fields (access_token,refresh_token,token_type,expiry_date). However, at line 103 ofgoogle-oauth-service.ts, the database value is cast ascredentials.config as unknown as OAuthTokenswithout validation, creating a chain of unsafe assumptions. Either validate the token structure before use or ensure the OAuth service contract guarantees the returned tokens match the expected shape.apps/http-backend/src/routes/google_callback.ts-13-13 (1)
13-13: Missing null check:userIdcould be undefined when passed tostate.If
userMiddlewarefails to setreq.user,userIdwill beundefined, which gets stringified as"undefined"in the OAuth state parameter.🔎 Proposed fix
const userId = req.user?.id; + if (!userId) { + return res.status(401).json({ error: "User not authenticated" }); + }Also applies to: 30-30
apps/http-backend/src/routes/google_callback.ts-65-65 (1)
65-65: Hardcoded localhost URLs will break in production.Use environment variables for redirect URLs to support different environments.
🔎 Proposed fix
+const FRONTEND_URL = process.env.FRONTEND_URL || "http://localhost:3000"; + // In the try block: - return res.redirect("http://localhost:3000/workflow"); + return res.redirect(`${FRONTEND_URL}/workflow`); } catch (err: any) { console.error("Google token exchange error:", err); - return res.redirect( - `http://localhost:3000/workflow?google=error&msg=${encodeURIComponent(err?.message ?? "Token exchange failed")}` - ); + return res.redirect( + `${FRONTEND_URL}/workflow?google=error&msg=${encodeURIComponent(err?.message ?? "Token exchange failed")}` + );Also applies to: 68-70
packages/nodes/src/common/google-oauth-service.ts-83-110 (1)
83-110: Security concern: userId not validated in getCredentials query.The
userIdparameter is accepted but commented out in the query (line 92). This allows any user to fetch credentials bycredIdregardless of ownership, which is an authorization bypass.🔎 Proposed fix
try { - console.log("user id: ", userId, " & ", credId, " from oauth service"); const credentials = await this.prisma.credential.findFirst({ where: { id: credId, - // userId: userId, + userId: userId, type: "google_oauth", }, - orderBy: { - id: "desc", - }, }); - console.log("credentails from oauth service: ", credentials); if (!credentials) return null;
🟡 Minor comments (7)
apps/worker/tsconfig.tsbuildinfo-1-1 (1)
1-1: Fix the typo in the filename: "registory.ts" → "registry.ts".The filename
./src/engine/registory.tscontains a typo and should be./src/engine/registry.ts. This build info file reflects the actual source file name, so you'll need to rename the source file itself. The AI summary confirms the intent is "execution registry."🔎 Steps to fix
Rename the source file:
git mv apps/worker/src/engine/registory.ts apps/worker/src/engine/registry.tsUpdate all imports referencing the old filename
The tsconfig.tsbuildinfo will automatically regenerate with the correct filename on next build
packages/nodes/src/google-sheets/google-sheets.service.ts-121-123 (1)
121-123: Fix typo in error message."Falied" should be "Failed".
🔎 Proposed fix
} catch (error) { - throw new Error(`Falied to refresh token: ${error}`); + throw new Error(`Failed to refresh token: ${error}`); }packages/db/src/seed.ts-74-86 (1)
74-86: WorkflowExecution creation is not idempotent.Using
createwith a hardcoded id will fail on subsequent runs with a duplicate key error. Consider usingupsertfor consistency with the other entities.🔎 Proposed fix
- const execution = await prismaClient.workflowExecution.create({ - data: { + const execution = await prismaClient.workflowExecution.upsert({ + where: { id: "test_exec_001" }, + update: {}, + create: { id: "test_exec_001", workflowId: workflow.id, status: "Pending", metadata: { trigger: "manual", timestamp: new Date().toISOString(), }, }, });packages/db/src/seed.ts-88-89 (1)
88-89: Misleading console output.Line 89 says the email will be sent to
your-email@gmail.combut the actual configured email is different. Update this to reflect the actual recipient or use a variable.🔎 Proposed fix
console.log("\n🎉 Test workflow ready!"); - console.log("📧 Email will be sent to: your-email@gmail.com"); + console.log(`📧 Email will be sent to: ${process.env.TEST_EMAIL_RECIPIENT || "test@example.com"}`);packages/nodes/src/index.ts-3-3 (1)
3-3: Fix malformed comment.The comment on line 3 has a trailing backslash that makes it syntactically incorrect. Remove the backslash at the end.
🔎 Proposed fix
-// Fixed lint error: Use correct export name 'GoogleSheetNode'\ +// Fixed lint error: Use correct export name 'GoogleSheetNode'apps/worker/src/engine/registory.ts-26-31 (1)
26-31: Convert error to string before assigning to error field.Line 29 assigns the error object directly, but
ExecutionResult.erroris typed asstring | undefined. This type mismatch could cause issues downstream.🔎 Proposed fix
} catch (error: any) { return { success: false, - error: error, + error: error instanceof Error ? error.message : String(error), }; }packages/nodes/src/common/google-oauth-service.ts-54-54 (1)
54-54: Typo: "Unknow" should be "Unknown".🔎 Proposed fix
- `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknow error"}` + `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknown error"}`
🧹 Nitpick comments (16)
README.md (1)
491-495: Documentation updated with license and authors.Good to see the placeholder values replaced with actual information. Consider the following minor improvements:
- Remove the extra space before the comma in the authors list
- Consider linking to a LICENSE file or providing the full license text
🔎 Suggested improvements
## 📄 License -[MIT license] +This project is licensed under the MIT License - see the [LICENSE](LICENSE) file for details. ## 👥 Authors -- [Vamsi , Teja] +- [Vamsi, Teja]packages/db/tsconfig.tsbuildinfo (1)
1-1: Consider adding tsbuildinfo to .gitignore.
tsconfig.tsbuildinfois an auto-generated build cache file and typically should not be committed to version control. It can cause merge conflicts and adds noise to the repository.packages/nodes/src/google-sheets/google-sheets.service.ts (1)
46-64: Improve type safety and fix unreliable condition.
- Return type
Promise<any>should be properly typed.- The
if (files)check on line 54 is always truthy sincedrive.files.listreturns an object. Consider checkingfiles.data.filesinstead.🔎 Proposed fix
- async getSheets(): Promise<any> { + async getSheets(): Promise<{ success: boolean; data: drive_v3.Schema$FileList | null }> { const files = await this.drive.files.list({ q: "mimeType='application/vnd.google-apps.spreadsheet'", spaces: "drive", pageSize: 10, fields: "files(id, name, createdTime)", }); - if (files) { + if (files.data.files && files.data.files.length > 0) { return { success: true, - data: files, + data: files.data, }; } return { success: false, data: null, }; }packages/nodes/tsconfig.tsbuildinfo (1)
1-1: Consider adding tsbuildinfo to .gitignore.Same as the
packages/dbtsbuildinfo - this auto-generated build cache file typically should not be committed to version control.apps/worker/src/tests/create-execution.ts (1)
3-17: Export the function instead of auto-invoking at module load.The immediate invocation on line 17 runs whenever this module is imported, which is unexpected behavior for a utility. Consider exporting the function so callers can invoke it explicitly when needed.
🔎 Proposed refactor
-async function createExecution() { +export async function createExecution() { const execution = await prismaClient.workflowExecution.create({ data: { workflowId: 'f5d1ae7c-104c-44ab-ab49-917f4716e2f3', status: 'Pending', metadata: { trigger: 'manual', test: 'gmail-api-enabled' } } }); console.log('✅ New execution created!'); console.log('Execution ID:', execution.id); console.log('\n📝 Update test.ts with:'); console.log(`await executeWorkflow("${execution.id}");`); } - -createExecution();Then invoke it explicitly from a test runner or setup script:
import { createExecution } from './create-execution.js'; await createExecution();apps/worker/src/tests/test.ts (1)
4-16: Make test execution IDs configurable and export the test function.The hardcoded execution ID on line 11 and immediate invocation on line 16 make this test inflexible. Consider making the execution ID a parameter and exporting the function for explicit invocation.
🔎 Proposed refactor
-async function testDirect() { +export async function testDirect(executionId: string = "83b1e9ef-c8f2-45a6-9f91-ef48d29fbb4c") { console.log("🧪 Testing Gmail integration directly...\n"); // Initialize registry register.initialize(); // Use the NEW execution ID from setup - await executeWorkflow("83b1e9ef-c8f2-45a6-9f91-ef48d29fbb4c"); + await executeWorkflow(executionId); console.log("\n✅ Test complete! Check your email inbox."); } - -testDirect().catch(console.error);Then invoke from a test runner:
import { testDirect } from './test.js'; await testDirect();packages/nodes/src/gmail/gmail.executor.ts (1)
75-76: Remove commented-out code.🔎 Proposed cleanup
export default GmailExecutor; -// export { default as GmailExecutor } from "./gmail.executor.js"; -// export { GmailService } from "./gmail.service.js";packages/nodes/src/common/google-oauth-service.ts (4)
65-73: Remove commented-out code and unnecessary JSON round-trip.The commented lines are debug artifacts. Also,
JSON.parse(JSON.stringify(tokens))is redundant sincetokensis already a plain object. Prisma handles JSON serialization automatically for JSON fields.🔎 Proposed fix
async saveCredentials( userId: string, tokens: OAuthTokens, nodeId?: string ): Promise<void> { try { - // const credentialId = `cred_google_${userId}_${Date.now()}` - await this.prisma.credential.create({ data: { - // id:credentialId, userId: userId, type: "google_oauth", - config: JSON.parse(JSON.stringify(tokens)), + config: tokens, nodeId: nodeId || null, }, });
88-99: Remove debug console.log statements before production.Debug logging with credential details can leak sensitive information to logs.
🔎 Proposed fix
- console.log("user id: ", userId, " & ", credId, " from oauth service"); const credentials = await this.prisma.credential.findFirst({ ... }); - console.log("credentails from oauth service: ", credentials);- console.log("logs from service - ", credentials); return credentials;Also applies to: 120-120
112-127: Consider using a stricter return type instead ofArray<any>.Using
anyloses type safety. Consider defining a proper return type or using the Prisma-generated type.🔎 Proposed fix
- async getAllCredentials(userId: string, type: string): Promise<Array<any>> { + async getAllCredentials(userId: string, type: string): Promise<Array<{ id: string; config: unknown }>> {
142-152: Type assertionas anybypasses type safety.Consider using a typed JSON field approach or a proper type assertion for the config update.
🔎 Proposed fix
const updatedConfig = { ...(existing.config as object), ...tokens, }; await this.prisma.credential.update({ where: { id: credentialId, }, data: { - config: updatedConfig as any, + config: updatedConfig as Record<string, unknown>, }, });apps/http-backend/src/routes/google_callback.ts (2)
8-36: Duplication: OAuth2 client and scopes are defined here and in GoogleOAuthService.The
GoogleOAuthService.getAuthUrl()method already handles OAuth URL generation with the same scopes. Consider reusing it to avoid divergence.🔎 Proposed fix
googleAuth.get( "/initiate", userMiddleware, async (req: AuthRequest, res: Response) => { const userId = req.user?.id; - - const oauth2 = new google.auth.OAuth2( - process.env.GOOGLE_CLIENT_ID, - process.env.GOOGLE_CLIENT_SECRET, - process.env.GOOGLE_REDIRECT_URI || - "http://localhost:3002/auth/google/callback" - ); - - const authUrl = oauth2.generateAuthUrl({ - access_type: "offline", - scope: [ - "https://www.googleapis.com/auth/spreadsheets", - "https://www.googleapis.com/auth/drive.readonly", - "https://www.googleapis.com/auth/gmail.send", - "https://www.googleapis.com/auth/gmail.readonly", - ], - state: userId, - prompt: "consent", - }); + if (!userId) { + return res.status(401).json({ error: "User not authenticated" }); + } + const oauthService = new GoogleOAuthService(); + const authUrl = oauthService.getAuthUrl(userId); return res.redirect(authUrl); } );
44-61: Duplication: Token exchange logic duplicates GoogleOAuthService.getTokens().You instantiate
GoogleOAuthServicebut then use a separate OAuth2 client to exchange tokens. Consider usingOauth.getTokens(code)instead.🔎 Proposed fix
const Oauth = new GoogleOAuthService(); if (!code || typeof code !== "string") { return res.json({ error: "Missing or invalid authorization code" }); } - const oauth2 = new google.auth.OAuth2( - process.env.GOOGLE_CLIENT_ID, - process.env.GOOGLE_CLIENT_SECRET, - process.env.GOOGLE_REDIRECT_URI || - "http://localhost:3002/auth/google/callback" - ); - try { - const { tokens } = await oauth2.getToken(code); + const tokens = await Oauth.getTokens(code); // Save tokens to database if userId (state) is provided if (state && typeof state === "string") { - await Oauth.saveCredentials(state, tokens as OAuthTokens); + await Oauth.saveCredentials(state, tokens); }packages/nodes/src/gmail/gmail.service.ts (3)
4-9: Consider reusingOAuthTokenstype from google-oauth-service.ts.
GmailCredentialsduplicatesOAuthTokensbut omits thescopefield. Reusing the existing type ensures consistency and reduces maintenance burden.🔎 Proposed fix
-export interface GmailCredentials { - access_token: string; - refresh_token: string; - token_type: string; - expiry_date: number; -} +import type { OAuthTokens } from "../common/google-oauth-service"; +export type GmailCredentials = OAuthTokens;
33-43: Missing "From" header in email construction.RFC 5322 requires a "From" header. While Gmail may auto-populate it, explicitly including it prevents potential issues with some configurations.
🔎 Proposed fix
- async sendEmail(to: string, subject: string, body: string) { + async sendEmail(to: string, subject: string, body: string, from?: string) { try { // Build the raw email string const emailLines = [ + from ? `From: ${from}` : "", `To: ${to}`, `Subject: ${subject}`, "Content-Type: text/html; charset=utf-8", "", body, - ]; + ].filter(Boolean);
52-55: Dead code:this.gmailis always initialized in constructor.The
gmailclient is initialized at line 30 in the constructor, so this null check will never be true.🔎 Proposed fix
- // Initialize gmail if needed - if (!this.gmail) { - this.gmail = google.gmail({ version: "v1", auth: this.auth }); - } - // Send email const result = await this.gmail.users.messages.send({
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
README.mdapps/http-backend/src/index.tsapps/http-backend/src/routes/google_callback.tsapps/http-backend/src/routes/userRoutes/userRoutes.tsapps/worker/package.jsonapps/worker/src/engine/executor.tsapps/worker/src/engine/registory.tsapps/worker/src/index.tsapps/worker/src/tests/create-execution.tsapps/worker/src/tests/setup-test.tsapps/worker/src/tests/test.tsapps/worker/src/tests/update-node.tsapps/worker/src/types.tsapps/worker/tsconfig.jsonapps/worker/tsconfig.tsbuildinfopackages/db/src/seed.tspackages/db/tsconfig.tsbuildinfopackages/nodes/.gitignorepackages/nodes/package.jsonpackages/nodes/src/common/google-oauth-service.tspackages/nodes/src/gmail/gmail.executor.tspackages/nodes/src/gmail/gmail.service.tspackages/nodes/src/google-sheets/google-sheets.service.tspackages/nodes/src/google-sheets/test.tspackages/nodes/src/index.tspackages/nodes/tsconfig.jsonpackages/nodes/tsconfig.tsbuildinfo
🧰 Additional context used
🧬 Code graph analysis (10)
apps/worker/src/tests/update-node.ts (1)
packages/db/src/index.ts (1)
prismaClient(17-18)
apps/worker/src/tests/create-execution.ts (1)
packages/db/src/index.ts (1)
prismaClient(17-18)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (2)
apps/http-backend/src/routes/userRoutes/userMiddleware.ts (2)
userMiddleware(18-42)AuthRequest(8-10)packages/db/src/index.ts (1)
prismaClient(17-18)
apps/worker/src/tests/setup-test.ts (1)
packages/db/src/index.ts (1)
prismaClient(17-18)
apps/worker/src/engine/registory.ts (1)
apps/worker/src/types.ts (3)
NodeExecutor(14-16)ExecutionContext(1-7)ExecutionResult(8-13)
packages/nodes/src/common/google-oauth-service.ts (2)
packages/nodes/src/index.ts (2)
OAuthTokens(5-5)GoogleOAuthService(7-7)packages/db/src/index.ts (1)
prismaClient(17-18)
apps/worker/src/tests/test.ts (3)
apps/worker/src/engine/registory.ts (2)
register(6-8)register(45-45)apps/worker/src/engine/executor.ts (1)
executeWorkflow(3-72)packages/db/src/index.ts (1)
prismaClient(17-18)
packages/db/src/seed.ts (1)
packages/db/src/index.ts (1)
prismaClient(17-18)
apps/worker/src/index.ts (1)
apps/worker/src/engine/executor.ts (1)
executeWorkflow(3-72)
apps/http-backend/src/index.ts (1)
apps/http-backend/src/routes/google_callback.ts (1)
googleAuth(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (15)
packages/nodes/src/google-sheets/test.ts (1)
1-1: LGTM! The.jsextension aligns with ES module resolution.The import path change to include the
.jsextension is correct for TypeScript'snodenextmodule resolution strategy used throughout the project.apps/worker/package.json (1)
8-8: LGTM! Build script and dependency additions are appropriate.The build script enables TypeScript compilation, and the
@repo/nodesdevDependency provides access to node executors needed for the worker's functionality.Also applies to: 15-15
apps/worker/tsconfig.json (1)
3-13: LGTM! TypeScript configuration properly updated for ES2020 and ES modules.The compiler options are well-configured:
- ES2020 target aligns with Node.js >=20 requirement
nodenextmodule resolution supports the.jsextension imports used throughout the codebase- Including
srcdirectory captures the new worker engine modulespackages/nodes/tsconfig.json (1)
3-7: No action required. The base TypeScript configuration atpackages/typescript-config/base.jsonalready provides appropriate module settings (module: "NodeNext",moduleResolution: "NodeNext"), confirming that relying on the base config after removing these options frompackages/nodes/tsconfig.jsonis safe and correct.packages/nodes/.gitignore (1)
1-31: LGTM!Standard .gitignore patterns covering build artifacts, dependencies, logs, IDE files, environment files, and OS-specific files. Appropriate for a TypeScript/Node.js package.
packages/db/src/seed.ts (1)
26-34: Credential lookup is not user-scoped.Using
findFirstwithout a userId filter could return credentials belonging to any user in multi-tenant scenarios. Consider making this more explicit or adding a user scope.packages/nodes/src/google-sheets/google-sheets.service.ts (1)
104-109: Reconsider behavior when expiry_date is missing.Returning
false(token not expired) whenexpiry_dateis undefined could be problematic. If expiry metadata is missing, it might be safer to treat the token as expired to force a refresh.apps/worker/src/index.ts (2)
44-55: Excellent error handling for offset commits.The try-catch block properly handles offset commit failures and logs the specific workflowExecutionId for debugging. This prevents the consumer from crashing when Kafka coordination fails.
61-63: Good addition of top-level error handler.The catch block on the main() invocation ensures unhandled promise rejections are logged rather than silently failing, improving observability.
apps/worker/src/types.ts (1)
1-16: LGTM - Clean type definitions for the execution system.These interfaces establish a clear contract for node executors. Ensure all executor implementations (especially
GmailExecutor) conform to these types.packages/nodes/src/common/google-oauth-service.ts (1)
4-10: LGTM!The
OAuthTokensinterface is well-defined with all necessary fields for Google OAuth token management.apps/http-backend/src/routes/google_callback.ts (1)
38-41: TheuserMiddlewareon /callback may conflict with state-based userId.The callback receives
state(userId) from Google's redirect. IfuserMiddlewarerequires authentication via session/token, the user may not be logged in during the OAuth flow. Additionally, line 41 usesRequestinstead ofAuthRequest, which is inconsistent with the middleware typing.Please verify:
- Whether the user session is maintained during the OAuth redirect flow
- If
userMiddlewareis necessary here since userId comes from statepackages/nodes/src/gmail/gmail.service.ts (3)
11-31: LGTM!The constructor properly initializes the OAuth2 client and sets credentials before creating the Gmail API client.
77-90: LGTM!Token expiry check with 1-minute buffer is a good practice to prevent edge-case failures during API calls.
92-110: Tokens are already being persisted inGmailExecutor.The
GmailExecutorcorrectly callsthis.oauthService.updateCredentials()immediately afterrefreshAccessToken()returns (gmail.executor.ts, lines 49-52), persisting the refreshed tokens to the database. No action needed.Likely an incorrect or invalid review comment.
| const context = { | ||
| nodeId: node.id, | ||
| userId: data.workflow.userId, | ||
| credentialId: node.credentials[0]?.id, | ||
| config: node.config as Record<string, any>, | ||
| inputData: currentInputData, | ||
| }; |
There was a problem hiding this comment.
Critical: Field name mismatch - credentialId should be credentialsId.
Line 45 uses credentialId (singular), but the ExecutionContext interface in apps/worker/src/types.ts line 4 defines the field as credentialsId (plural). This breaks the type contract and causes runtime issues when executors try to access the credentials.
🔎 Proposed fix
const context = {
nodeId: node.id,
userId: data.workflow.userId,
- credentialId: node.credentials[0]?.id,
+ credentialsId: node.credentials[0]?.id,
config: node.config as Record<string, any>,
inputData: currentInputData,
};📝 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.
| const context = { | |
| nodeId: node.id, | |
| userId: data.workflow.userId, | |
| credentialId: node.credentials[0]?.id, | |
| config: node.config as Record<string, any>, | |
| inputData: currentInputData, | |
| }; | |
| const context = { | |
| nodeId: node.id, | |
| userId: data.workflow.userId, | |
| credentialsId: node.credentials[0]?.id, | |
| config: node.config as Record<string, any>, | |
| inputData: currentInputData, | |
| }; |
🤖 Prompt for AI Agents
In @apps/worker/src/engine/executor.ts around lines 42-48, The context object in
executor.ts uses the wrong field name `credentialId` (singular) which doesn't
match the `ExecutionContext` interface's `credentialsId` (plural); update the
context literal in the executor (the `context` variable) to use `credentialsId`
and populate it with the same value currently assigned to `credentialId` (e.g.,
node.credentials[0]?.id) so the shape matches the `ExecutionContext` defined in
apps/worker/src/types.ts and executors can access credentials correctly.
| @@ -1 +1 @@ | |||
| {"root":["./src/index.ts"],"version":"5.7.3"} No newline at end of file | |||
| {"root":["./src/index.ts","./src/test.ts","./src/types.ts","./src/engine/executor.ts","./src/engine/registory.ts"],"version":"5.7.3"} No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Examine test.ts to verify if it should be in production build roots
# Check if test.ts exists and display its content
if [ -f "apps/worker/src/test.ts" ]; then
echo "=== Content of test.ts ==="
cat apps/worker/src/test.ts
echo -e "\n=== Checking if test.ts is imported by non-test files ==="
rg --type ts -g '!**/*.test.ts' -g '!**/*.spec.ts' "from ['\"].*test['\"]" apps/worker/src/
else
echo "test.ts not found"
fiRepository: Dev-Pross/BuildFlow
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Search for test.ts anywhere in the repository
echo "=== Searching for test.ts in the repository ==="
fd test.ts
echo -e "\n=== Listing files in apps/worker/src ==="
ls -la apps/worker/src/ 2>/dev/null || echo "Directory not found"
echo -e "\n=== Content of apps/worker/tsconfig.tsbuildinfo ==="
cat apps/worker/tsconfig.tsbuildinfo 2>/dev/null || echo "File not found"Repository: Dev-Pross/BuildFlow
Length of output: 784
🏁 Script executed:
#!/bin/bash
# Examine the actual test.ts file that exists
echo "=== Content of apps/worker/src/tests/test.ts ==="
cat apps/worker/src/tests/test.ts
echo -e "\n=== Content of apps/worker/tsconfig.json (if exists) ==="
cat apps/worker/tsconfig.json 2>/dev/null || echo "File not found"
echo -e "\n=== Checking if test.ts is imported anywhere ==="
rg "from.*test" apps/worker/src/ --type tsRepository: Dev-Pross/BuildFlow
Length of output: 1631
Remove test files from the TypeScript compilation or exclude them in tsconfig.json.
The referenced ./src/test.ts doesn't exist. However, ./src/tests/test.ts contains development-only code with direct function calls and database debug logic (hardcoded execution IDs, debug queries) that will execute at module load time. This must be excluded from production builds. Update tsconfig.json to exclude the tests directory or remove the stale tsbuildinfo entry.
🤖 Prompt for AI Agents
In @apps/worker/tsconfig.tsbuildinfo around line 1, The build is picking up a
nonexistent ./src/test.ts and a development-only ./src/tests/test.ts; update
tsconfig.json to exclude the tests directory (e.g., add "exclude": ["src/tests",
"src/tests/**"] or adjust "files"/"include" to omit tests) so test-only modules
aren’t compiled, then remove or regenerate the stale
apps/worker/tsconfig.tsbuildinfo entry that references ./src/test.ts (delete the
tsbuildinfo or run a clean build) to clear the invalid reference; target
symbols: tsconfig.json, apps/worker/tsconfig.tsbuildinfo, ./src/test.ts,
./src/tests/test.ts.
| "license": "ISC", | ||
| "packageManager": "pnpm@10.4.1", | ||
| "dependencies": { | ||
| "@types/dotenv": "^8.2.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the package.json file to see the current dependencies
cat packages/nodes/package.jsonRepository: Dev-Pross/BuildFlow
Length of output: 766
🏁 Script executed:
# Check if dotenv has built-in TypeScript types
cd packages/nodes && cat node_modules/dotenv/package.json 2>/dev/null | jq '.types, .typings' || echo "Cannot access node_modules; checking package info via npm"Repository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Check dotenv version and type definitions through npm registry
npm info dotenv@17.2.3 2>/dev/null | grep -E "(types|typings|exports)" || echo "Checking dotenv type definitions..."Repository: Dev-Pross/BuildFlow
Length of output: 98
🏁 Script executed:
# Check TypeScript compilation to see if there are type errors
cd packages/nodes && npm run build 2>&1Repository: Dev-Pross/BuildFlow
Length of output: 123
🌐 Web query:
dotenv 17.2.3 TypeScript types built-in support
💡 Result:
Yes — dotenv v17.2.3 ships bundled TypeScript declarations (lib/main.d.ts) and the package.json points to them, so you do not need @types/dotenv. [1][2]
Sources:
- dotenv package.json (shows "types": "lib/main.d.ts" / exports). [UNPKG package.json for dotenv 17.2.2 — same pattern used in 17.x series]. [1]
- Package listing showing lib/main.d.ts present in v17.2.3. [2]
Remove @types/dotenv entirely and restore @types/node to devDependencies.
@types/dotenv is unnecessary—dotenv v17.2.3 includes built-in TypeScript type definitions (types: "lib/main.d.ts"), so this package should be removed from dependencies rather than moved.
@types/node must be restored to devDependencies. This package is required for Node.js core module types and its removal will cause TypeScript compilation to fail.
Proposed fix
"dependencies": {
- "@types/dotenv": "^8.2.3",
"dotenv": "^17.2.3",
"google-auth-library": "^10.5.0",
"googleapis": "^166.0.0"
},
"devDependencies": {
+ "@types/node": "^20.19.9",
"@repo/db": "workspace:*",
"@workspace/typescript-config": "workspace:*"
}📝 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.
| "@types/dotenv": "^8.2.3", | |
| "dependencies": { | |
| "dotenv": "^17.2.3", | |
| "google-auth-library": "^10.5.0", | |
| "googleapis": "^166.0.0" | |
| }, | |
| "devDependencies": { | |
| "@types/node": "^20.19.9", | |
| "@repo/db": "workspace:*", | |
| "@workspace/typescript-config": "workspace:*" | |
| } |
🤖 Prompt for AI Agents
In @packages/nodes/package.json around line 22, Remove the unnecessary
dependency "@types/dotenv" from package.json and re-add "@types/node" under
devDependencies (restore its previous semver) so TypeScript has Node core types;
ensure only dotenv remains in dependencies (dotenv v17.2.3 provides its own
types) and update package.json devDependencies to include "@types/node" again.
| interface NodeExecutionContext { | ||
| credId: string; | ||
| userId: string; | ||
| config?: any; | ||
| inputData?: any; | ||
| } | ||
|
|
||
| interface NodeExecutionResult { | ||
| success: boolean; | ||
| output?: any; | ||
| error?: string; | ||
| } |
There was a problem hiding this comment.
Critical: Interface duplication and field name mismatch breaks type safety.
These local interfaces duplicate ExecutionContext and ExecutionResult from apps/worker/src/types.ts, but with incompatible field names (credId vs credentialsId). This mismatch forces the unsafe type cast in apps/worker/src/engine/registory.ts line 40 and undermines type safety across the execution flow.
🔎 Proposed fix: Remove local interfaces and use worker types
-import { GoogleOAuthService } from "../common/google-oauth-service.js";
+import { GoogleOAuthService } from "../common/google-oauth-service.js";
+import type { ExecutionContext, ExecutionResult } from "../../worker/src/types.js";
import { GmailService, GmailCredentials } from "./gmail.service.js";
-interface NodeExecutionContext {
- credId: string;
- userId: string;
- config?: any;
- inputData?: any;
-}
-
-interface NodeExecutionResult {
- success: boolean;
- output?: any;
- error?: string;
-}
-
class GmailExecutor {
private oauthService: GoogleOAuthService;
private gmailService: GmailService | null = null;
constructor() {
this.oauthService = new GoogleOAuthService();
}
- async execute(context: NodeExecutionContext): Promise<NodeExecutionResult> {
+ async execute(context: ExecutionContext): Promise<ExecutionResult> {
try {
// Get credentials
const credentials = await this.oauthService.getCredentials(
context.userId,
- context.credId
+ context.credentialsId
);Note: Adjust the import path based on your monorepo structure. Consider re-exporting these types from a shared location if direct imports create circular dependencies.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/nodes/src/gmail/gmail.executor.ts around lines 4-15, The local
interfaces NodeExecutionContext and NodeExecutionResult duplicate and conflict
with the canonical ExecutionContext and ExecutionResult used by the worker,
causing the unsafe cast in registory.ts; remove the local interfaces and replace
their usage with imports of ExecutionContext and ExecutionResult from the
worker/types module (or a shared types re-export) so the executor uses the same
credential field names (e.g., credentialsId) and shapes across the codebase;
update any references in gmail.executor.ts to use the imported types and run
type-checking to confirm the unsafe cast at registory.ts line 40 is no longer
required.
There was a problem hiding this comment.
Pull request overview
This PR adds Gmail integration for sending emails and completes the worker implementation to execute workflows. The changes enable the system to send emails via Gmail API and process workflow executions from Kafka messages.
Key Changes
- Added Gmail service and executor for sending emails via Gmail API
- Implemented workflow execution engine in the worker with registry pattern for node executors
- Updated OAuth service to include Gmail scopes (send and readonly)
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 30 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nodes/src/gmail/gmail.service.ts | New Gmail service for sending emails via Google APIs |
| packages/nodes/src/gmail/gmail.executor.ts | New executor implementation for Gmail node execution |
| packages/nodes/src/common/google-oauth-service.ts | Added Gmail scopes to OAuth authentication flow |
| apps/worker/src/engine/executor.ts | New workflow execution engine that processes nodes sequentially |
| apps/worker/src/engine/registory.ts | Registry for mapping node types to their executors |
| apps/worker/src/index.ts | Updated Kafka consumer to execute workflows on message receipt |
| apps/http-backend/src/routes/google_callback.ts | Added OAuth initiation endpoint and updated callback handler |
| apps/http-backend/src/routes/userRoutes/userRoutes.ts | Refactored credentials endpoint to check database directly |
| packages/nodes/package.json | Added @types/dotenv dependency, removed @types/node |
| packages/nodes/tsconfig.json | Removed explicit module configuration options |
| apps/worker/tsconfig.json | Added ES2020 lib and nodenext module resolution |
| packages/db/src/seed.ts | New seed script for creating test Gmail workflow |
| apps/worker/src/tests/* | Test utilities for workflow setup and execution |
| packages/nodes/.gitignore | Added comprehensive gitignore patterns |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| const tokens = await this.auth.refreshAccessToken(); | ||
| this.auth.setCredentials(tokens.credentials); | ||
| return { | ||
| success: true, | ||
| data: tokens.credentials, | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }; | ||
| } |
There was a problem hiding this comment.
The error handling here differs from GoogleSheetsService. This method returns a success/error object, while GoogleSheetsService.refreshAccessToken() returns the credentials object directly and throws on error. Consider standardizing the return pattern across services for consistency.
| try { | |
| const tokens = await this.auth.refreshAccessToken(); | |
| this.auth.setCredentials(tokens.credentials); | |
| return { | |
| success: true, | |
| data: tokens.credentials, | |
| }; | |
| } catch (error) { | |
| return { | |
| success: false, | |
| error: error instanceof Error ? error.message : String(error), | |
| }; | |
| } | |
| const tokens = await this.auth.refreshAccessToken(); | |
| this.auth.setCredentials(tokens.credentials); | |
| return tokens.credentials; |
| // TODO: Ensure GmailExecutor implements NodeExecutor and is compatible with ExecutionContext | ||
| // If needed, adapt/extract a compatible Executor for registration. | ||
| // 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.
Spelling/grammar error: "wehen visits" should be "when visiting". Also, this TODO comment is incomplete and should provide more specific guidance.
| // TODO: Ensure GmailExecutor implements NodeExecutor and is compatible with ExecutionContext | |
| // If needed, adapt/extract a compatible Executor for registration. | |
| // 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 | |
| // TODO: Before registering GmailExecutor here, ensure it correctly implements NodeExecutor | |
| // and is compatible with ExecutionContext. If it is not, adapt or wrap it in a compatible | |
| // Executor implementation and remove the `as unknown as NodeExecutor` cast. |
| console.log("✅ Execution created:", execution.id); | ||
|
|
||
| console.log("\n🎉 Test workflow ready!"); | ||
| console.log("📧 Email will be sent to: your-email@gmail.com"); |
There was a problem hiding this comment.
The message at line 89 contradicts the actual email address used in line 63. The comment says "your-email@gmail.com" but the code uses "iamvamsi0@gmail.com". These should match for clarity.
| console.log("📧 Email will be sent to: your-email@gmail.com"); | |
| console.log("📧 Email will be sent to: iamvamsi0@gmail.com"); |
| AvailableNodeID: availableNode.id, | ||
| position: 0, | ||
| config: { | ||
| to: "iamvamsi0@gmail.com", |
There was a problem hiding this comment.
Hard-coded email address in test data. This personal email address should be replaced with a placeholder or configuration variable to prevent accidental sends when other developers run these tests.
| to: "iamvamsi0@gmail.com", | |
| to: "test@example.com", |
| import { prismaClient } from "@repo/db/client"; | ||
|
|
||
| async function debug() { | ||
| const executionId = "ca4210e3-c830-408e-a86d-961508e9b325"; |
There was a problem hiding this comment.
Hard-coded execution ID that likely doesn't exist in other environments. This makes the debug function unusable without modification.
| // Initialize gmail if needed | ||
| if (!this.gmail) { | ||
| this.gmail = google.gmail({ version: "v1", auth: this.auth }); | ||
| } | ||
|
|
There was a problem hiding this comment.
This redundant check can be removed. The gmail instance is already initialized in the constructor at line 30, so this condition will never be true.
| // Initialize gmail if needed | |
| if (!this.gmail) { | |
| this.gmail = google.gmail({ version: "v1", auth: this.auth }); | |
| } |
| console.warn("message not found skipping"); | ||
| return ; | ||
| } | ||
| console.log(`Recieved workflowExecutionId ${workflowExecutionId}`); |
There was a problem hiding this comment.
Spelling error: "Recieved" should be "Received".
| console.log(`Recieved workflowExecutionId ${workflowExecutionId}`); | |
| console.log(`Received workflowExecutionId ${workflowExecutionId}`); |
| workflowId: workflow.id, | ||
| AvailableNodeID: availableNode.id, | ||
| config: { | ||
| to: "iamvamsi0@gmail.com", // ← CHANGE THIS! |
There was a problem hiding this comment.
Hard-coded email address in test/seed data. This should be replaced with a placeholder or environment variable to avoid accidentally sending test emails to a specific address when other developers run this seed script.
| register.initialize(); | ||
|
|
||
| // Use the NEW execution ID from setup | ||
| await executeWorkflow("83b1e9ef-c8f2-45a6-9f91-ef48d29fbb4c"); |
There was a problem hiding this comment.
Hard-coded workflow execution ID. This test file contains a specific execution ID that likely won't exist in other environments, making the test fail when run elsewhere. Consider generating or looking up a valid execution ID dynamically.
| return; | ||
| } | ||
|
|
||
| const update = await prismaClient.workflowExecution.update({ |
There was a problem hiding this comment.
Unused variable update.
| const update = await prismaClient.workflowExecution.update({ | |
| await prismaClient.workflowExecution.update({ |
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @apps/http-backend/src/routes/google_callback.ts:
- Around line 8-36: The route handler uses req.user?.id as state without
verifying authentication; add an explicit check after userMiddleware to ensure
req.user exists (e.g., validate in the googleAuth.get handler) and return an
appropriate 401/403 response if not authenticated so state is never undefined.
Locate the googleAuth.get handler, the userMiddleware usage, and the userId
variable assignment in this file and guard the call to oauth2.generateAuthUrl
(and subsequent res.redirect) with that check.
In @apps/http-backend/src/routes/userRoutes/userRoutes.ts:
- Line 217: Replace the inconsistent extraction of the authenticated user's id
by changing the assignment that reads const userId = req.user.sub to use
req.user.id instead so it matches the other handlers in this module (keep the
const userId variable name and use req.user.id consistently across this file).
In @packages/nodes/src/common/google-oauth-service.ts:
- Line 54: The error message string in the token exchange catch uses a
misspelled fallback "Unknow error"; update the template literal used in the
Failed to exchange code for token logging (the string `Failed to exchange code
for token: ${error instanceof Error ? error.message : "Unknow error"}` in
google-oauth-service.ts) to use the correct fallback "Unknown error".
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (3)
266-266: Inconsistent userId extraction.Same issue as in the
/getCredentials/:typeroute above: this usesreq.user.subwhile other routes usereq.user.id.🔎 Proposed fix
- const userId = req.user.sub; + const userId = req.user.id;
267-275: Missing response when credentials array is empty.If
credsis an empty array (which is a valid state), no response is sent to the client, potentially causing the request to hang. An empty credentials array should still return a success response.🔎 Proposed fix to always send a response
const creds = await prismaClient.credential.findMany({ where:{ userId: userId} }) - if(creds){ - return res.status(statusCodes.OK).json({ - message: "Fetched all credentials of the User!", - data: creds - }) - } + return res.status(statusCodes.OK).json({ + message: "Fetched all credentials of the User!", + data: creds + })
278-278: Typo in error message."Unkown" should be "Unknown".
🔎 Proposed fix
- console.log("Error Fetching the credentials ", e instanceof Error ? e.message : "Unkown reason"); + console.log("Error Fetching the credentials ", e instanceof Error ? e.message : "Unknown reason");apps/http-backend/src/routes/google_callback.ts (1)
90-126: Protect or remove debug endpoint in production.The
/debug/credentialsendpoint exposes credential metadata (user IDs, token presence, expiry) without any authentication. While it doesn't expose actual tokens, this information could be valuable to attackers.Consider one of these approaches:
- Remove this endpoint before deploying to production
- Protect it with authentication middleware
- Gate it behind an environment check (e.g., only available in development)
🔎 Example: Gate behind environment check
// Debug endpoint to check stored credentials -googleAuth.get('/debug/credentials', async(req: Request, res: Response)=>{ +googleAuth.get('/debug/credentials', async(req: Request, res: Response)=>{ + // Only allow in development + if (process.env.NODE_ENV === 'production') { + return res.status(404).json({ error: 'Not found' }); + } + try {packages/nodes/src/google-sheets/google-sheets.service.ts (1)
92-127: Critical: Method nesting error -refreshAccessTokenappears to be insidereadRows.The
refreshAccessTokenmethod (lines 103-127) appears to be incorrectly nested inside thereadRowsmethod (lines 92-102). This is a syntax error that would prevent the code from compiling.🔎 Proposed fix to correct method structure
async readRows(params: ReadRowsParams): Promise<any[][]> { try { const response = await this.sheets.spreadsheets.values.get({ spreadsheetId: params.spreadsheetId, range: params.range, }); return response.data.values || []; } catch (error) { throw new Error(`Failed to fetch the rows: ${error}`); } + } async refreshAccessToken(): Promise <GoogleSheetsCredentials>{Note: The closing brace for
readRowsis missing beforerefreshAccessTokenbegins.packages/nodes/src/common/google-oauth-service.ts (1)
112-166: Critical: Method nesting error -updateCredentialsappears to be insidegetAllCredentials.The
updateCredentialsmethod (lines 128-166) appears to be incorrectly nested inside thegetAllCredentialsmethod (lines 112-127). This is a syntax error that would prevent the code from compiling. Looking at the indentation and brace structure,getAllCredentialsseems to be missing its closing brace beforeupdateCredentialsbegins.🔎 Proposed fix to correct method structure
async getAllCredentials(userId: string, type: string): Promise<Array<any>> { try { const credentials = await this.prisma.credential.findMany({ where: { userId: userId, type: type, }, }); console.log("logs from service - ", credentials); return credentials; } catch (err) { throw new Error( `Failed to get all credentials: ${err instanceof Error ? err.message : "unknown error"}` ); + } + } async updateCredentials(credentialId: string, tokens: Partial<OAuthTokens>): Promise<void> {Note: The closing braces for the catch block and the method are missing before
updateCredentialsbegins.
🧹 Nitpick comments (2)
apps/http-backend/src/routes/google_callback.ts (2)
41-41: Use AuthRequest type for consistency with userMiddleware.The route uses
userMiddlewarebut declares the request asRequestinstead ofAuthRequest. While this doesn't cause runtime issues, it's inconsistent with the/initiateroute (line 11) and doesn't provide proper typing forreq.user.🔎 Proposed fix for type consistency
"/callback", userMiddleware, - async (req: Request, res: Response) => { + async (req: AuthRequest, res: Response) => {
80-85: Consider using environment variables for redirect URLs.The success and error redirect URLs are hard-coded to
http://localhost:3000/workflow. Consider using an environment variable (e.g.,FRONTEND_URL) to support different environments (development, staging, production).🔎 Proposed refactor to use environment variables
// Redirect to success page - return res.redirect("http://localhost:3000/workflow"); + return res.redirect(`${process.env.FRONTEND_URL || 'http://localhost:3000'}/workflow`); } catch (err: any) { console.error("Google token exchange error:", err); return res.redirect( - `http://localhost:3000/workflow?google=error&msg=${encodeURIComponent(err?.message ?? "Token exchange failed")}` + `${process.env.FRONTEND_URL || 'http://localhost:3000'}/workflow?google=error&msg=${encodeURIComponent(err?.message ?? "Token exchange failed")}` );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/http-backend/src/index.tsapps/http-backend/src/routes/google_callback.tsapps/http-backend/src/routes/userRoutes/userRoutes.tspackages/nodes/src/common/google-oauth-service.tspackages/nodes/src/google-sheets/google-sheets.service.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/nodes/src/common/google-oauth-service.ts (2)
packages/nodes/src/index.ts (2)
OAuthTokens(7-7)GoogleOAuthService(9-9)packages/db/src/index.ts (1)
prismaClient(17-18)
apps/http-backend/src/index.ts (2)
apps/http-backend/src/routes/google_callback.ts (1)
googleAuth(7-7)apps/http-backend/src/scheduler/token-scheduler.ts (1)
tokenScheduler(65-65)
🔇 Additional comments (1)
apps/http-backend/src/index.ts (1)
23-32: LGTM! Clean refactoring of route mounting and server startup.The route path change to
/auth/googlefollows REST conventions by grouping authentication-related endpoints. Moving the console log into the listen callback ensures accurate logging after the server starts.
| googleAuth.get( | ||
| "/initiate", | ||
| userMiddleware, | ||
| async (req: AuthRequest, res: Response) => { | ||
| // const userId = req.user?.id || "test_user"; // Get from auth middleware | ||
| const userId = req.user?.id; | ||
|
|
||
| googleAuth.get('/callback', async(req: Request, res: Response)=>{ | ||
| const oauth2 = new google.auth.OAuth2( | ||
| process.env.GOOGLE_CLIENT_ID, | ||
| process.env.GOOGLE_CLIENT_SECRET, | ||
| process.env.GOOGLE_REDIRECT_URI || | ||
| "http://localhost:3002/auth/google/callback" | ||
| ); | ||
|
|
||
| const authUrl = oauth2.generateAuthUrl({ | ||
| access_type: "offline", | ||
| scope: [ | ||
| "https://www.googleapis.com/auth/spreadsheets", | ||
| "https://www.googleapis.com/auth/drive.readonly", | ||
| "https://www.googleapis.com/auth/gmail.send", | ||
| "https://www.googleapis.com/auth/gmail.readonly", | ||
| ], | ||
| state: userId, | ||
| prompt: "consent", | ||
| }); | ||
|
|
||
| return res.redirect(authUrl); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Add explicit check for authenticated user.
While userMiddleware is applied, there's no explicit check if req.user exists before using req.user?.id as the OAuth state parameter (line 30). If req.user is undefined, the state will be undefined, which could cause the callback flow to fail when associating tokens with the user.
🔎 Proposed fix to add explicit authentication check
"/initiate",
userMiddleware,
async (req: AuthRequest, res: Response) => {
- // const userId = req.user?.id || "test_user"; // Get from auth middleware
- const userId = req.user?.id;
+ if (!req.user?.id) {
+ return res.status(401).json({
+ message: "User authentication required"
+ });
+ }
+ const userId = req.user.id;
const oauth2 = new google.auth.OAuth2(📝 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.
| googleAuth.get( | |
| "/initiate", | |
| userMiddleware, | |
| async (req: AuthRequest, res: Response) => { | |
| // const userId = req.user?.id || "test_user"; // Get from auth middleware | |
| const userId = req.user?.id; | |
| googleAuth.get('/callback', async(req: Request, res: Response)=>{ | |
| const oauth2 = new google.auth.OAuth2( | |
| process.env.GOOGLE_CLIENT_ID, | |
| process.env.GOOGLE_CLIENT_SECRET, | |
| process.env.GOOGLE_REDIRECT_URI || | |
| "http://localhost:3002/auth/google/callback" | |
| ); | |
| const authUrl = oauth2.generateAuthUrl({ | |
| access_type: "offline", | |
| scope: [ | |
| "https://www.googleapis.com/auth/spreadsheets", | |
| "https://www.googleapis.com/auth/drive.readonly", | |
| "https://www.googleapis.com/auth/gmail.send", | |
| "https://www.googleapis.com/auth/gmail.readonly", | |
| ], | |
| state: userId, | |
| prompt: "consent", | |
| }); | |
| return res.redirect(authUrl); | |
| } | |
| ); | |
| googleAuth.get( | |
| "/initiate", | |
| userMiddleware, | |
| async (req: AuthRequest, res: Response) => { | |
| if (!req.user?.id) { | |
| return res.status(401).json({ | |
| message: "User authentication required" | |
| }); | |
| } | |
| const userId = req.user.id; | |
| const oauth2 = new google.auth.OAuth2( | |
| process.env.GOOGLE_CLIENT_ID, | |
| process.env.GOOGLE_CLIENT_SECRET, | |
| process.env.GOOGLE_REDIRECT_URI || | |
| "http://localhost:3002/auth/google/callback" | |
| ); | |
| const authUrl = oauth2.generateAuthUrl({ | |
| access_type: "offline", | |
| scope: [ | |
| "https://www.googleapis.com/auth/spreadsheets", | |
| "https://www.googleapis.com/auth/drive.readonly", | |
| "https://www.googleapis.com/auth/gmail.send", | |
| "https://www.googleapis.com/auth/gmail.readonly", | |
| ], | |
| state: userId, | |
| prompt: "consent", | |
| }); | |
| return res.redirect(authUrl); | |
| } | |
| ); |
🤖 Prompt for AI Agents
In @apps/http-backend/src/routes/google_callback.ts around lines 8-36, The route
handler uses req.user?.id as state without verifying authentication; add an
explicit check after userMiddleware to ensure req.user exists (e.g., validate in
the googleAuth.get handler) and return an appropriate 401/403 response if not
authenticated so state is never undefined. Locate the googleAuth.get handler,
the userMiddleware usage, and the userId variable assignment in this file and
guard the call to oauth2.generateAuthUrl (and subsequent res.redirect) with that
check.
| message: "User is not Loggedin" | ||
| }) | ||
| } | ||
| const userId = req.user.sub; |
There was a problem hiding this comment.
Inconsistent userId extraction may cause authorization issues.
This route uses req.user.sub while all other routes in this file consistently use req.user.id (lines 81, 297, 346, 373, 404). This inconsistency could prevent users from retrieving their own credentials if the auth middleware populates req.user.id but not req.user.sub, or vice versa.
🔎 Proposed fix to use consistent userId extraction
- const userId = req.user.sub;
+ const userId = req.user.id;📝 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.
| const userId = req.user.sub; | |
| const userId = req.user.id; |
🤖 Prompt for AI Agents
In @apps/http-backend/src/routes/userRoutes/userRoutes.ts around line 217,
Replace the inconsistent extraction of the authenticated user's id by changing
the assignment that reads const userId = req.user.sub to use req.user.id instead
so it matches the other handlers in this module (keep the const userId variable
name and use req.user.id consistently across this file).
| }; | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknow error"}` |
There was a problem hiding this comment.
Typo in error message.
"Unknow error" should be "Unknown error".
🔎 Proposed fix
- `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknow error"}`
+ `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknown 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.
| `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknow error"}` | |
| `Failed to exchange code for token: ${error instanceof Error ? error.message : "Unknown error"}` |
🤖 Prompt for AI Agents
In @packages/nodes/src/common/google-oauth-service.ts around line 54, The error
message string in the token exchange catch uses a misspelled fallback "Unknow
error"; update the template literal used in the Failed to exchange code for
token logging (the string `Failed to exchange code for token: ${error instanceof
Error ? error.message : "Unknow error"}` in google-oauth-service.ts) to use the
correct fallback "Unknown error".
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.