feat: implement token refresh scheduler and enhance Google OAuth hand… #48
Conversation
…ling - Added TokenScheduler to manage periodic token refreshes. - Introduced tokenRefreshService for handling token refresh logic. - Enhanced Google OAuth callback with detailed logging and error handling. - Implemented update functionality for nodes and triggers in user routes. - Updated workflow slice to accommodate new ID structures. - Improved error handling and logging across various components. - Refactored Google Sheets service to conditionally include refresh tokens. - Added debug endpoints for credential management and token status.
📝 WalkthroughWalkthroughThis PR introduces automated token refresh infrastructure for Google OAuth credentials. It adds a periodic scheduler and refresh service to detect and refresh expiring tokens, creates backend endpoints for updating node and trigger configurations, and refactors frontend workflow components to support configuration updates with revised ID formats. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as TokenScheduler
participant Service as TokenRefreshService
participant Sheets as GoogleSheetsService
participant OAuth as GoogleOAuthService
participant DB as Database
rect rgb(200, 220, 255)
Note over Scheduler,DB: Periodic Refresh Flow (every 60 minutes)
end
Scheduler->>Service: refreshAllExpiringTokens()
activate Service
Service->>DB: fetch all google_oauth credentials
DB-->>Service: credentials list
loop For each expiring credential
rect rgb(220, 240, 220)
Note over Service,DB: Token Refresh for Single Credential
end
Service->>Sheets: refreshAccessToken(currentTokens)
activate Sheets
Sheets-->>Sheets: call Google API
Sheets-->>Service: { access_token, refresh_token?, expiry_date }
deactivate Sheets
Service->>OAuth: updateCredentials(credId, newTokens)
activate OAuth
OAuth->>OAuth: filter truthy token fields
OAuth->>DB: update credential record
DB-->>OAuth: updated
OAuth-->>Service: success
deactivate OAuth
end
rect rgb(255, 200, 200)
Note over Service,DB: Error Handling: invalid_grant
end
alt token refresh fails with invalid_grant
Service->>OAuth: credential is invalid
OAuth->>DB: delete credential
OAuth->>DB: clear credId from triggers & nodes
DB-->>OAuth: references cleared
Service-->>Service: log deletion
else other error
Service-->>Service: log error, continue
end
Service-->>Scheduler: { total, refreshed, failed, deleted }
deactivate Service
Scheduler->>Scheduler: log results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @TejaBudumuru3. * #48 (comment) The following files were modified: * `apps/http-backend/src/index.ts` * `apps/web/app/components/nodes/CreateWorkFlow.tsx` * `apps/web/app/components/nodes/GoogleSheetFormClient.tsx` * `apps/web/app/components/nodes/actions.ts` * `apps/web/app/components/ui/app-sidebar.tsx`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
155-223: RemoveworkflowIdanddispatchfrom the dependency array.The second
useEffectrebuilds nodes/edges from Redux state (existingNodes,existingTrigger). NeitherworkflowIdnordispatchare used in this effect's body. Additionally, the first effect already fetches workflow data whenworkflowIdchanges and updates Redux state, so the second effect will re-run indirectly through its actual dependencies. IncludingworkflowIddirectly causes unnecessary re-renders. Remove both from[existingNodes, existingTrigger, dispatch, workflowId]to[existingNodes, existingTrigger].
🧹 Nitpick comments (8)
packages/common/src/index.ts (1)
37-45: Consider stricter typing for Config fields.Both schemas use
z.any()for theConfigfield, which bypasses validation. If the config structure is known or partially known, consider usingz.record(z.unknown())or a more specific schema for better type safety and runtime validation.💡 Example alternative
export const NodeUpdateSchema = z.object({ NodeId: z.string(), Config: z.record(z.unknown()) // or z.object({ ... }) if structure is known }) export const TriggerUpdateSchema = z.object({ TriggerId: z.string(), Config: z.record(z.unknown()) })apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
50-51: Remove debug console logs before merging.These console logs appear to be for debugging purposes and should be removed or wrapped in a development-only conditional before merging to production.
apps/web/app/components/ui/app-sidebar.tsx (2)
32-32: Unused import:workflowReducer.
workflowReduceris imported but not used in this file.🔎 Proposed fix
-import { workflowActions, workflowReducer } from '@/store/slices/workflowSlice' +import { workflowActions } from '@/store/slices/workflowSlice'
41-41: Remove debug console log.This console log should be removed before merging.
apps/http-backend/src/scheduler/token-scheduler.ts (1)
55-61:triggerManualRefreshpropagates errors fromrunRefreshJob.While
runRefreshJobcatches errors internally,triggerManualRefreshcalls it withawaitand could still throw if something unexpected happens before the try-catch. Consider wrapping it or documenting that callers should handle errors.🔎 Proposed fix
async triggerManualRefresh(): Promise<void> { console.log('\n🔧 Manual token refresh triggered...'); - await this.runRefreshJob(); + try { + await this.runRefreshJob(); + } catch (error) { + console.error('❌ Manual token refresh failed:', error); + throw error; + } }apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
20-20: Unused prop:avlNodeis declared but never used.The
avlNodeprop is added to the interface and component signature but is not referenced anywhere in the component body.If this is for future use, consider removing it until needed to avoid confusion. If it's intended to be used, the implementation appears incomplete.
apps/http-backend/src/services/token-refresh.service.ts (2)
4-10: Consider importingOAuthTokensfrom@repo/nodesinstead of duplicating.Per the relevant snippets,
OAuthTokensis already exported from@repo/nodes. Duplicating the interface can lead to drift if the original is updated.🔎 Proposed fix
import { prismaClient } from "@repo/db/client"; -import { GoogleSheetsService, GoogleOAuthService } from "@repo/nodes"; - -interface OAuthTokens { - access_token: string; - refresh_token: string; - token_type: string; - expiry_date: number; - scope?: string; -} +import { GoogleSheetsService, GoogleOAuthService, OAuthTokens } from "@repo/nodes";
168-174: Fragile string matching for detecting deleted credentials.The check
result.error?.includes('deleted')relies on the specific wording of the error message set at line 123. If that message changes, this logic breaks silently.Consider using a structured result type instead:
🔎 Proposed fix
interface RefreshResult { credentialId: string; success: boolean; error?: string; + deleted?: boolean; } // In refreshToken, when returning after deletion: return { credentialId, success: false, + deleted: true, error: 'Credential deleted. Please reconnect your Google account.' }; // In refreshAllExpiringTokens: -} else if (result.error?.includes('deleted')) { +} else if (result.deleted) { deleted++;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/http-backend/src/index.tsapps/http-backend/src/routes/google_callback.tsapps/http-backend/src/routes/userRoutes/userRoutes.tsapps/http-backend/src/scheduler/token-scheduler.tsapps/http-backend/src/services/token-refresh.service.tsapps/web/app/components/nodes/CreateWorkFlow.tsxapps/web/app/components/nodes/GoogleSheetFormClient.tsxapps/web/app/components/nodes/actions.tsapps/web/app/components/ui/app-sidebar.tsxapps/web/app/workflow/lib/config.tsapps/web/store/slices/workflowSlice.tspackages/common/src/index.tspackages/nodes/src/common/google-oauth-service.tspackages/nodes/src/google-sheets/google-sheets.service.tspackages/nodes/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (8)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (4)
apps/http-backend/src/routes/userRoutes/userMiddleware.ts (2)
userMiddleware(18-42)AuthRequest(8-10)packages/common/src/index.ts (2)
NodeUpdateSchema(37-40)TriggerUpdateSchema(42-45)apps/web/app/workflow/lib/config.ts (1)
updateNode(183-208)packages/db/src/index.ts (1)
prismaClient(17-18)
packages/nodes/src/common/google-oauth-service.ts (1)
packages/nodes/src/index.ts (1)
OAuthTokens(7-7)
apps/http-backend/src/index.ts (1)
apps/http-backend/src/scheduler/token-scheduler.ts (1)
tokenScheduler(65-65)
apps/http-backend/src/services/token-refresh.service.ts (4)
packages/nodes/src/common/google-oauth-service.ts (2)
OAuthTokens(161-161)GoogleOAuthService(160-160)packages/nodes/src/index.ts (3)
OAuthTokens(7-7)GoogleOAuthService(9-9)GoogleSheetsService(11-11)packages/nodes/src/google-sheets/google-sheets.service.ts (1)
GoogleSheetsService(138-138)packages/db/src/index.ts (1)
prismaClient(17-18)
apps/web/app/workflow/lib/config.ts (1)
packages/common/src/index.ts (1)
BACKEND_URL(5-5)
apps/web/app/components/ui/app-sidebar.tsx (3)
apps/web/app/hooks/redux.ts (1)
useAppDispatch(6-6)apps/web/app/workflow/lib/config.ts (3)
getAllWorkflows(64-76)getAllCredentials(48-61)getworkflowData(270-286)apps/web/store/slices/workflowSlice.ts (1)
workflowActions(61-61)
apps/web/app/components/nodes/actions.ts (1)
apps/web/app/workflow/lib/config.ts (2)
updateTrigger(157-181)updateNode(183-208)
apps/web/app/components/nodes/GoogleSheetFormClient.tsx (5)
apps/web/app/hooks/redux.ts (1)
useAppSelector(7-7)apps/web/app/hooks/useCredential.ts (1)
useCredentials(5-34)packages/common/src/index.ts (1)
BACKEND_URL(5-5)apps/web/app/components/nodes/actions.ts (2)
handleUpdateConfig(76-106)handleSaveConfig(29-74)apps/web/store/slices/workflowSlice.ts (1)
workflowActions(61-61)
🔇 Additional comments (11)
packages/nodes/src/common/google-oauth-service.ts (1)
151-151: LGTM: Helpful debug logging.The success log aids in tracing credential updates during token refresh operations.
packages/nodes/src/index.ts (1)
11-11: LGTM: Exposes GoogleSheetsService for token refresh integration.The new export enables the token refresh service (apps/http-backend/src/services/token-refresh.service.ts) to access GoogleSheetsService methods for refreshing credentials.
packages/common/src/index.ts (1)
58-58: LGTM: Valid HTTP status code addition.HTTP 302 FOUND is correctly added for redirect scenarios.
packages/nodes/src/google-sheets/google-sheets.service.ts (1)
133-133: LGTM: Typo correction.Fixed "Falied" → "Failed" in error message.
apps/web/app/components/nodes/actions.ts (1)
76-106: LGTM! Clean implementation of the update flow.The
handleUpdateConfigfunction follows the same pattern ashandleSaveConfig, maintains good code consistency, and properly delegates to the appropriate update function based on type.apps/web/app/workflow/lib/config.ts (1)
183-208: LGTM! Correct implementation.The
updateNodefunction correctly implements the update operation with proper error handling and accurate logging.apps/web/store/slices/workflowSlice.ts (2)
3-9: All references todbIdhave been successfully updated.The field rename from
dbIdtoidin theTriggerinterface is complete. No remaining references todbIdwere found in the codebase, and all code consuming this interface is using the newidfield.
11-18: No action required. All references tonode.dbIdhave been successfully updated. The field rename fromdbIdtoidin theNodeIteminterface is complete, with no remaining references to the old field name found in the codebase.apps/http-backend/src/scheduler/token-scheduler.ts (1)
14-27: LGTM on the core scheduler logic.The scheduler implementation is straightforward and follows a reasonable pattern: immediate execution on start, then periodic runs via
setInterval. The interval conversion and cleanup are handled correctly.apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
157-188: Nested fetch logic is complex but handles restoration well.The restoration logic for preserving previously selected document and sheet when credential changes is thorough. However, consider extracting the nested fetch into a separate function for readability.
apps/http-backend/src/services/token-refresh.service.ts (1)
36-58: Token refresh logic is well-structured.The refresh flow correctly:
- Uses existing services (
GoogleSheetsService,GoogleOAuthService)- Falls back to the old refresh token if a new one isn't provided (line 52)
- Properly updates all token fields in the credential store
| async function startServer() { | ||
| await NodeRegistry.registerAll() | ||
|
|
||
| tokenScheduler.start(); |
There was a problem hiding this comment.
Add error handling and graceful shutdown for the token scheduler.
The scheduler starts without error handling, and there's no cleanup on server shutdown. If the scheduler fails to start, the server continues anyway, potentially leaving tokens unrefreshed. Additionally, without graceful shutdown, the scheduler may continue running after the server stops.
🔎 Recommended improvements
async function startServer() {
await NodeRegistry.registerAll()
- tokenScheduler.start();
+ try {
+ tokenScheduler.start();
+ console.log('✅ Token scheduler started');
+ } catch (error) {
+ console.error('❌ Failed to start token scheduler:', error);
+ // Decide: should this be fatal? For now, log and continue
+ }
+
app.listen(PORT, () => {
console.log(`Server running on port ${PORT}`);
})
+
+ // Graceful shutdown
+ process.on('SIGTERM', () => {
+ console.log('SIGTERM received, shutting down gracefully');
+ tokenScheduler.stop();
+ process.exit(0);
+ });
+
+ process.on('SIGINT', () => {
+ console.log('SIGINT received, shutting down gracefully');
+ tokenScheduler.stop();
+ process.exit(0);
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tokenScheduler.start(); | |
| async function startServer() { | |
| await NodeRegistry.registerAll() | |
| try { | |
| tokenScheduler.start(); | |
| console.log('✅ Token scheduler started'); | |
| } catch (error) { | |
| console.error('❌ Failed to start token scheduler:', error); | |
| // Decide: should this be fatal? For now, log and continue | |
| } | |
| app.listen(PORT, () => { | |
| console.log(`Server running on port ${PORT}`); | |
| }) | |
| // Graceful shutdown | |
| process.on('SIGTERM', () => { | |
| console.log('SIGTERM received, shutting down gracefully'); | |
| tokenScheduler.stop(); | |
| process.exit(0); | |
| }); | |
| process.on('SIGINT', () => { | |
| console.log('SIGINT received, shutting down gracefully'); | |
| tokenScheduler.stop(); | |
| process.exit(0); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/http-backend/src/index.ts around line 44, tokenScheduler.start() is
invoked with no error handling or shutdown cleanup; wrap the start call in a
try/catch (await if start is async) and log the error and stop server or exit if
it fails to start, and register graceful shutdown handlers (process
SIGINT/SIGTERM and server close) that call tokenScheduler.stop() (or
cancel/cleanup) to ensure the scheduler is stopped before exit; finally ensure
start/stop calls are idempotent and errors during shutdown are caught and
logged.
| // DEBUG: Log tokens received from Google | ||
| console.log('\n🔐 Google OAuth Callback - Tokens received:'); | ||
| console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' expiry_date:', tokens.expiry_date); | ||
| console.log(' token_type:', tokens.token_type); | ||
| console.log(' scope:', tokens.scope); | ||
|
|
||
| if (!tokens.refresh_token) { | ||
| console.warn('⚠️ WARNING: No refresh_token received! User may have already authorized this app.'); | ||
| console.warn(' To force new refresh_token, user needs to revoke access at: https://myaccount.google.com/permissions'); | ||
| } |
There was a problem hiding this comment.
Remove or sanitize debug logging in production.
The debug logging exposes detailed OAuth token information to console logs. In production environments, this could leak sensitive data to log aggregation systems.
Consider:
- Using a debug flag to conditionally enable verbose logging
- Sanitizing logs to avoid exposing token presence/length details
- Moving these logs behind a proper debugging configuration
🔎 Suggested approach with environment-based logging
+ const isDebug = process.env.NODE_ENV === 'development';
+
- // DEBUG: Log tokens received from Google
- console.log('\n🔐 Google OAuth Callback - Tokens received:');
- console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing');
- console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing');
- console.log(' expiry_date:', tokens.expiry_date);
- console.log(' token_type:', tokens.token_type);
- console.log(' scope:', tokens.scope);
+ if (isDebug) {
+ console.log('\n🔐 Google OAuth Callback - Tokens received:');
+ console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing');
+ console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing');
+ console.log(' expiry_date:', tokens.expiry_date);
+ }
if (!tokens.refresh_token) {
- console.warn('⚠️ WARNING: No refresh_token received! User may have already authorized this app.');
- console.warn(' To force new refresh_token, user needs to revoke access at: https://myaccount.google.com/permissions');
+ if (isDebug) {
+ console.warn('⚠️ WARNING: No refresh_token received!');
+ }
}📝 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.
| // DEBUG: Log tokens received from Google | |
| console.log('\n🔐 Google OAuth Callback - Tokens received:'); | |
| console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing'); | |
| console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing'); | |
| console.log(' expiry_date:', tokens.expiry_date); | |
| console.log(' token_type:', tokens.token_type); | |
| console.log(' scope:', tokens.scope); | |
| if (!tokens.refresh_token) { | |
| console.warn('⚠️ WARNING: No refresh_token received! User may have already authorized this app.'); | |
| console.warn(' To force new refresh_token, user needs to revoke access at: https://myaccount.google.com/permissions'); | |
| } | |
| const isDebug = process.env.NODE_ENV === 'development'; | |
| if (isDebug) { | |
| console.log('\n🔐 Google OAuth Callback - Tokens received:'); | |
| console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing'); | |
| console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing'); | |
| console.log(' expiry_date:', tokens.expiry_date); | |
| } | |
| if (!tokens.refresh_token) { | |
| if (isDebug) { | |
| console.warn('⚠️ WARNING: No refresh_token received!'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around lines 26 to 37, remove
or guard the DEBUG console.log/console.warn calls that print OAuth token
details; instead wrap verbose token logs behind a configurable debug flag (e.g.,
process.env.DEBUG_OAUTH === 'true') or the app logger's debug level, and if you
must log, only emit non-sensitive, sanitized info (e.g., token existence as
boolean or masked/length-only indicators) while never printing raw tokens,
refresh_token, or expiry values to production logs. Ensure the warning about
missing refresh_token remains but is also routed through the logger at an
appropriate level and sanitized, and update any related tests/config to set the
debug flag during local dev only.
| // Debug endpoint to check stored credentials | ||
| googleAuth.get('/debug/credentials', async(req: Request, res: Response)=>{ | ||
| try { | ||
| const { prismaClient } = await import('@repo/db/client'); | ||
|
|
||
| const credentials = await prismaClient.credential.findMany({ | ||
| where: { type: 'google_oauth' }, | ||
| select: { | ||
| id: true, | ||
| userId: true, | ||
| type: true, | ||
| config: true | ||
| } | ||
| }); | ||
|
|
||
| const debugInfo = credentials.map(cred => { | ||
| const config = cred.config as any; | ||
| return { | ||
| id: cred.id, | ||
| userId: cred.userId, | ||
| hasAccessToken: !!config?.access_token, | ||
| hasRefreshToken: !!config?.refresh_token, | ||
| refreshTokenLength: config?.refresh_token?.length || 0, | ||
| expiryDate: config?.expiry_date, | ||
| expiresIn: config?.expiry_date ? Math.round((config.expiry_date - Date.now()) / 1000 / 60) + ' minutes' : 'N/A', | ||
| isInvalid: config?.invalid || false, | ||
| scope: config?.scope | ||
| }; | ||
| }); | ||
|
|
||
| console.log('\n📋 Stored Credentials Debug:'); | ||
| console.table(debugInfo); | ||
|
|
||
| return res.json({ credentials: debugInfo }); | ||
| } catch (err) { | ||
| return res.status(500).json({ error: err instanceof Error ? err.message : 'Unknown error' }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
CRITICAL: Debug endpoint exposes credentials without authentication.
The /debug/credentials endpoint returns sensitive OAuth credential information without any authentication middleware. This allows anyone to access stored credentials including token metadata and expiry information.
🔎 Required fix: Add authentication and restrict access
-// Debug endpoint to check stored credentials
-googleAuth.get('/debug/credentials', async(req: Request, res: Response)=>{
+// Debug endpoint to check stored credentials (protected)
+googleAuth.get('/debug/credentials', userMiddleware, async(req: AuthRequest, res: Response)=>{
try {
+ // Only allow in development or for admin users
+ if (process.env.NODE_ENV === 'production') {
+ return res.status(403).json({ error: 'Debug endpoints disabled in production' });
+ }
+
+ if (!req.user) {
+ return res.status(401).json({ error: 'Authentication required' });
+ }
+
const { prismaClient } = await import('@repo/db/client');
const credentials = await prismaClient.credential.findMany({
- where: { type: 'google_oauth' },
+ where: {
+ type: 'google_oauth',
+ userId: req.user.id // Only show user's own credentials
+ },
select: {
id: true,
userId: true,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around lines 56 to 93, the
/debug/credentials endpoint exposes sensitive OAuth credential data without any
authentication; secure it by removing or disabling the public debug route and/or
wrapping it with the application’s auth middleware, verifying the requester is
authenticated and has an admin or ops role before continuing, and ensure
returned data omits secrets (no access_token, refresh_token, or full config) —
only return non-sensitive metadata (e.g., id, userId, hasTokens true/false,
expiryDate) for authorized users; if you keep the endpoint, log full details
only server-side and never send tokens to clients.
| router.put('/update/node', userMiddleware, async(req: AuthRequest, res: Response)=>{ | ||
| try{ | ||
| if(!req.user){ | ||
| return res.status(statusCodes.BAD_REQUEST).json({ | ||
| message: "User is not logged in ", | ||
| }); | ||
| } | ||
| const data = req.body; | ||
| const dataSafe = NodeUpdateSchema.safeParse(data) | ||
|
|
||
| if(!dataSafe.success) { | ||
| return res.status(statusCodes.BAD_REQUEST).json({ | ||
| message: "Invalid input" | ||
| }) | ||
| } | ||
|
|
||
| const updateNode = await prismaClient.node.update({ | ||
| where: {id: dataSafe.data.NodeId}, | ||
| data:{ | ||
| config: dataSafe.data.Config | ||
| } | ||
| }) | ||
|
|
||
| if(updateNode) | ||
| return res.status(statusCodes.CREATED).json({ | ||
| message: "Node updated", | ||
| data: updateNode | ||
| }) | ||
|
|
||
| }catch(e){ | ||
| console.log("This is the error from Node updating", e); | ||
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | ||
| message: "Internal server Error from Node Updation.", | ||
| }); | ||
| } | ||
| }) |
There was a problem hiding this comment.
Use HTTP 200 instead of 201 for update operations.
The endpoint returns 201 CREATED on successful update (line 501), but 201 is semantically for resource creation. For update operations, 200 OK is the correct status code.
🔎 Proposed fix
if(updateNode)
- return res.status(statusCodes.CREATED).json({
+ return res.status(statusCodes.OK).json({
message: "Node updated",
data: updateNode
})📝 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.
| router.put('/update/node', userMiddleware, async(req: AuthRequest, res: Response)=>{ | |
| try{ | |
| if(!req.user){ | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "User is not logged in ", | |
| }); | |
| } | |
| const data = req.body; | |
| const dataSafe = NodeUpdateSchema.safeParse(data) | |
| if(!dataSafe.success) { | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "Invalid input" | |
| }) | |
| } | |
| const updateNode = await prismaClient.node.update({ | |
| where: {id: dataSafe.data.NodeId}, | |
| data:{ | |
| config: dataSafe.data.Config | |
| } | |
| }) | |
| if(updateNode) | |
| return res.status(statusCodes.CREATED).json({ | |
| message: "Node updated", | |
| data: updateNode | |
| }) | |
| }catch(e){ | |
| console.log("This is the error from Node updating", e); | |
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | |
| message: "Internal server Error from Node Updation.", | |
| }); | |
| } | |
| }) | |
| router.put('/update/node', userMiddleware, async(req: AuthRequest, res: Response)=>{ | |
| try{ | |
| if(!req.user){ | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "User is not logged in ", | |
| }); | |
| } | |
| const data = req.body; | |
| const dataSafe = NodeUpdateSchema.safeParse(data) | |
| if(!dataSafe.success) { | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "Invalid input" | |
| }) | |
| } | |
| const updateNode = await prismaClient.node.update({ | |
| where: {id: dataSafe.data.NodeId}, | |
| data:{ | |
| config: dataSafe.data.Config | |
| } | |
| }) | |
| if(updateNode) | |
| return res.status(statusCodes.OK).json({ | |
| message: "Node updated", | |
| data: updateNode | |
| }) | |
| }catch(e){ | |
| console.log("This is the error from Node updating", e); | |
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | |
| message: "Internal server Error from Node Updation.", | |
| }); | |
| } | |
| }) |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/userRoutes/userRoutes.ts around lines 477 to
512, the handler for PUT /update/node currently returns statusCodes.CREATED
(201) on successful update; change this to statusCodes.OK (200) to correctly
indicate an update operation; update the response line to use statusCodes.OK
while keeping the response body the same.
| router.put('/update/trigger', userMiddleware, async(req: AuthRequest, res: Response)=>{ | ||
| try{ | ||
| if(!req.user){ | ||
| return res.status(statusCodes.BAD_REQUEST).json({ | ||
| message: "User is not logged in ", | ||
| }); | ||
| } | ||
| const data = req.body; | ||
| const dataSafe = TriggerUpdateSchema.safeParse(data) | ||
|
|
||
| if(!dataSafe.success) | ||
| return res.status(statusCodes.BAD_REQUEST).json({ | ||
| message: "Invalid input" | ||
| }) | ||
|
|
||
| const updatedTrigger = await prismaClient.trigger.update({ | ||
| where:{id: dataSafe.data.TriggerId}, | ||
| data:{ | ||
| config: dataSafe.data.Config | ||
| } | ||
| }) | ||
|
|
||
| if(updatedTrigger) | ||
| return res.status(statusCodes.CREATED).json({ | ||
| message: "Trigger updated", | ||
| data: updatedTrigger | ||
| }) | ||
| }catch(e){ | ||
| console.log("This is the error from Trigger updating", e); | ||
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | ||
| message: "Internal server Error from Trigger Updation", | ||
| }); | ||
| } | ||
| }) |
There was a problem hiding this comment.
Use HTTP 200 instead of 201 for update operations.
Similar to the node update endpoint, this trigger update endpoint returns 201 CREATED (line 537) but should return 200 OK for successful updates.
🔎 Proposed fix
if(updatedTrigger)
- return res.status(statusCodes.CREATED).json({
+ return res.status(statusCodes.OK).json({
message: "Trigger updated",
data: updatedTrigger
})📝 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.
| router.put('/update/trigger', userMiddleware, async(req: AuthRequest, res: Response)=>{ | |
| try{ | |
| if(!req.user){ | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "User is not logged in ", | |
| }); | |
| } | |
| const data = req.body; | |
| const dataSafe = TriggerUpdateSchema.safeParse(data) | |
| if(!dataSafe.success) | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "Invalid input" | |
| }) | |
| const updatedTrigger = await prismaClient.trigger.update({ | |
| where:{id: dataSafe.data.TriggerId}, | |
| data:{ | |
| config: dataSafe.data.Config | |
| } | |
| }) | |
| if(updatedTrigger) | |
| return res.status(statusCodes.CREATED).json({ | |
| message: "Trigger updated", | |
| data: updatedTrigger | |
| }) | |
| }catch(e){ | |
| console.log("This is the error from Trigger updating", e); | |
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | |
| message: "Internal server Error from Trigger Updation", | |
| }); | |
| } | |
| }) | |
| router.put('/update/trigger', userMiddleware, async(req: AuthRequest, res: Response)=>{ | |
| try{ | |
| if(!req.user){ | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "User is not logged in ", | |
| }); | |
| } | |
| const data = req.body; | |
| const dataSafe = TriggerUpdateSchema.safeParse(data) | |
| if(!dataSafe.success) | |
| return res.status(statusCodes.BAD_REQUEST).json({ | |
| message: "Invalid input" | |
| }) | |
| const updatedTrigger = await prismaClient.trigger.update({ | |
| where:{id: dataSafe.data.TriggerId}, | |
| data:{ | |
| config: dataSafe.data.Config | |
| } | |
| }) | |
| if(updatedTrigger) | |
| return res.status(statusCodes.OK).json({ | |
| message: "Trigger updated", | |
| data: updatedTrigger | |
| }) | |
| }catch(e){ | |
| console.log("This is the error from Trigger updating", e); | |
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | |
| message: "Internal server Error from Trigger Updation", | |
| }); | |
| } | |
| }) |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/userRoutes/userRoutes.ts around lines 514 to
547, the trigger update handler returns HTTP 201 CREATED on success; change it
to return HTTP 200 OK instead. Update the response that currently sends
statusCodes.CREATED to use statusCodes.OK and keep the same message and payload;
ensure the rest of the logic and error handling remain unchanged.
| if(!creds) getCreds() | ||
| if(!workflow) getWorkflows() | ||
| },[selectedWorkflow]) | ||
| if(!workflow) { | ||
| getWorkflows() | ||
| createNewWorkflow() | ||
| } | ||
| getWorkflowData() | ||
| },[flow.workflow_id, dispatch]) |
There was a problem hiding this comment.
Potential race condition and missing state guards.
When !workflow, both getWorkflows() and createNewWorkflow() are called simultaneously. createNewWorkflow() may call getWorkflows() again in its else-branch (line 128), leading to duplicate fetches. Additionally, getWorkflowData() runs on every effect trigger, even when flow.workflow_id hasn't changed meaningfully.
Consider:
- Using a ref or state flag to prevent duplicate initialization
- Separating concerns into distinct effects (credentials, workflows, workflow data)
- Adding cleanup logic for unmounted components
🔎 Suggested approach
+ const [initialized, setInitialized] = useState(false);
+
useEffect(()=>{
// ... existing code
- if(!workflow) {
+ if(!workflow && !initialized) {
+ setInitialized(true);
getWorkflows()
createNewWorkflow()
}
- getWorkflowData()
},[flow.workflow_id, dispatch])
+
+ // Separate effect for workflow data
+ useEffect(() => {
+ if (flow.workflow_id) {
+ getWorkflowData()
+ }
+ }, [flow.workflow_id])🤖 Prompt for AI Agents
In apps/web/app/components/ui/app-sidebar.tsx around lines 89-95, the effect may
trigger duplicate/racing workflow initialization because getWorkflows() and
createNewWorkflow() can run concurrently and getWorkflowData() runs on every
trigger; refactor into three focused effects (1) credentials: run getCreds()
once on mount with a mounted-ref cleanup, (2) workflows: on mount or when a
guarded-initialize ref/state indicates "not initialized" call getWorkflows() OR
createNewWorkflow() but never both—use a useRef boolean (e.g., initializedRef)
or local state to ensure only one initialization path runs and avoid calling
getWorkflows() from createNewWorkflow() if already initializing, and (3)
workflow data: run getWorkflowData() only when flow.workflow_id actually changes
(compare prev id via ref or include flow.workflow_id in deps and short-circuit
if unchanged). Also add a cleanup flag to ignore results for unmounted
components to prevent state updates after unmount.
| export const updateTrigger = async(context: updateContext)=>{ | ||
| try{ | ||
| const res = await axios.put(`${BACKEND_URL}/user/update/trigger`, | ||
| { | ||
| TriggerId: context.id, | ||
| Config: context.config | ||
| },{ | ||
| headers: {"Content-Type": "application/json"}, | ||
| withCredentials: true | ||
| } | ||
| ) | ||
| const trigger = (res.data) | ||
| console.log('trigger created: ', trigger); | ||
| return { | ||
| success: true, | ||
| data: trigger | ||
| } | ||
| }catch(e){ | ||
| console.error("Error in updating Trigger:", e); | ||
| return { | ||
| success: false, | ||
| data: e instanceof Error ? e.message : `unknown error ${e}` | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix misleading log message.
Line 169 logs 'trigger created: ' but this function updates an existing trigger. The message should say "trigger updated" to accurately reflect the operation.
🔎 Proposed fix
const trigger = (res.data)
- console.log('trigger created: ', trigger);
+ console.log('trigger updated: ', trigger);
return {
success: true,
data: trigger📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const updateTrigger = async(context: updateContext)=>{ | |
| try{ | |
| const res = await axios.put(`${BACKEND_URL}/user/update/trigger`, | |
| { | |
| TriggerId: context.id, | |
| Config: context.config | |
| },{ | |
| headers: {"Content-Type": "application/json"}, | |
| withCredentials: true | |
| } | |
| ) | |
| const trigger = (res.data) | |
| console.log('trigger created: ', trigger); | |
| return { | |
| success: true, | |
| data: trigger | |
| } | |
| }catch(e){ | |
| console.error("Error in updating Trigger:", e); | |
| return { | |
| success: false, | |
| data: e instanceof Error ? e.message : `unknown error ${e}` | |
| } | |
| } | |
| } | |
| export const updateTrigger = async(context: updateContext)=>{ | |
| try{ | |
| const res = await axios.put(`${BACKEND_URL}/user/update/trigger`, | |
| { | |
| TriggerId: context.id, | |
| Config: context.config | |
| },{ | |
| headers: {"Content-Type": "application/json"}, | |
| withCredentials: true | |
| } | |
| ) | |
| const trigger = (res.data) | |
| console.log('trigger updated: ', trigger); | |
| return { | |
| success: true, | |
| data: trigger | |
| } | |
| }catch(e){ | |
| console.error("Error in updating Trigger:", e); | |
| return { | |
| success: false, | |
| data: e instanceof Error ? e.message : `unknown error ${e}` | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/workflow/lib/config.ts around lines 157 to 181, the console.log
message incorrectly says 'trigger created' inside updateTrigger; change that
message to 'trigger updated' (optionally include the trigger id or brief
context) so the log accurately reflects the operation and aids debugging.
| @@ -1,4 +1,5 @@ | |||
| import z from "zod"; | |||
| import { string } from "zod/v4"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for all imports from zod/v4
echo "=== Checking zod/v4 imports ==="
rg -n "from ['\"]zod" packages/common/src/index.ts
# Check zod version in package.json
echo -e "\n=== Checking zod in package.json ==="
fd -e json "package.json" | head -5 | xargs grep -H "zod" 2>/dev/null || true
# View the entire file to check if string is used
echo -e "\n=== Entire file content ==="
cat -n packages/common/src/index.ts
# Check if string identifier is used anywhere in the file
echo -e "\n=== Search for 'string' usage in file ==="
rg -n "string" packages/common/src/index.tsRepository: Dev-Pross/BuildFlow
Length of output: 2653
🏁 Script executed:
#!/bin/bash
# Check if zod package has any exports for subpaths
echo "=== Checking zod package exports/structure ==="
fd -e json "package.json" | xargs grep -l "zod" | head -1 | xargs cat | grep -A 20 "dependencies\|devDependencies" | grep "zod"
# Check if there are any other zod/v4 imports in the entire codebase
echo -e "\n=== Searching for zod/v4 imports across codebase ==="
rg "from ['\"]zod/v4['\"]" --type=ts --type=jsRepository: Dev-Pross/BuildFlow
Length of output: 225
Remove the unused import from line 2.
The string import from "zod/v4" is never used in this file. All schema definitions correctly use z.string() from the standard zod import on line 1. Additionally, "zod/v4" is not a valid import path for the installed zod version (3.25.76).
Proposed fix
import z from "zod";
-import { string } from "zod/v4";📝 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.
| import { string } from "zod/v4"; | |
| import z from "zod"; |
🤖 Prompt for AI Agents
In packages/common/src/index.ts around line 2, remove the unused and invalid
import "import { string } from \"zod/v4\""; it is not referenced anywhere and
the project uses zod v3 where "zod/v4" is not a valid path—delete that line so
only the correct zod import remains and run typecheck to ensure no remaining
references to that symbol.
| // Filter out empty/falsy values to prevent overwriting valid tokens | ||
| const filteredTokens: Partial<OAuthTokens> = {}; | ||
| 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; |
There was a problem hiding this comment.
Critical: Falsy check incorrectly rejects expiry_date = 0.
The truthy check if (tokens.expiry_date) treats 0 as falsy and excludes it from filteredTokens. Since expiry_date is a Unix timestamp (number), 0 is a valid value (epoch time) that should be preserved. This bug could cause tokens to lose their expiry metadata.
🔎 Proposed fix: Use explicit undefined/null checks
- // Filter out empty/falsy values to prevent overwriting valid tokens
const filteredTokens: Partial<OAuthTokens> = {};
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.expiry_date !== undefined && tokens.expiry_date !== null) {
+ filteredTokens.expiry_date = tokens.expiry_date;
+ }
if (tokens.scope) filteredTokens.scope = tokens.scope;🤖 Prompt for AI Agents
In packages/nodes/src/common/google-oauth-service.ts around lines 129 to 135,
the current truthy checks (e.g., if (tokens.expiry_date)) incorrectly drop valid
numeric values such as 0; change those checks to explicit null/undefined checks
(e.g., if (tokens.expiry_date !== undefined && tokens.expiry_date !== null)) so
expiry_date (and any other numeric fields) are preserved even when zero, thereby
preventing accidental loss of token expiry metadata.
| const result: GoogleSheetsCredentials = { | ||
| access_token: credentials.access_token || '', | ||
| refresh_token: credentials.refresh_token || '', | ||
| refresh_token: '', // Will be set below if present | ||
| 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; | ||
| } |
There was a problem hiding this comment.
Returning empty string for missing refresh_token creates inconsistency.
When Google doesn't return a new refresh_token, the current code returns an empty string (''). This creates problems:
- The
GoogleSheetsCredentialsinterface requiresrefresh_token: string, but semantically an empty string is ambiguous. - The filtering logic in
google-oauth-service.ts(lines 132-135) treats empty strings as falsy and won't persist them, potentially causing the existing refresh_token to be lost. - Consumers checking
if (creds.refresh_token)will getfalsefor empty strings.
🔎 Recommended solutions
Option 1 (Preferred): Make refresh_token optional and use undefined
interface GoogleSheetsCredentials{
access_token: string,
- refresh_token: string,
+ refresh_token?: string,
token_type: string,
expiry_date: number
} const result: GoogleSheetsCredentials = {
access_token: credentials.access_token || '',
- refresh_token: '', // Will be set below if present
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;
}Option 2: Preserve the existing refresh_token from constructor
Store the original refresh_token in the constructor and return it when Google doesn't provide a new one:
class GoogleSheetsService{
private sheets : sheets_v4.Sheets;
private auth: OAuth2Client;
private drive: drive_v3.Drive;
+ private originalRefreshToken: string;
constructor(credentials: GoogleSheetsCredentials){
+ this.originalRefreshToken = credentials.refresh_token;
// ... rest of constructor
}
async refreshAccessToken(): Promise <GoogleSheetsCredentials>{
try{
const {credentials} = await this.auth.refreshAccessToken();
const result: GoogleSheetsCredentials = {
access_token: credentials.access_token || '',
- refresh_token: '',
+ refresh_token: credentials.refresh_token || this.originalRefreshToken,
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;
- }
return result;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/nodes/src/google-sheets/google-sheets.service.ts around lines 118 to
128, the code currently sets refresh_token to an empty string when Google
doesn't return one; change this so that refresh_token is optional/undefined
instead of an empty string by updating the GoogleSheetsCredentials type to make
refresh_token?: string and return result without setting refresh_token (or
explicitly set it to undefined) when credentials.refresh_token is absent; ensure
callers that check for creds.refresh_token use truthy checks and update any
typings where needed so no empty-string sentinel is used.
…ling
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.