Conversation
- Added redux-persist to persist workflow state across sessions. - Updated workflow slice to include new state structure for managing nodes, edges, and trigger configurations. - Refactored WorkflowCanvas component to utilize Redux for state management, including auto-saving and node configuration. - Enhanced error handling and loading states during workflow execution and configuration. - Adjusted API interactions to align with new state structure and persist changes. - Updated Prisma schema to accommodate new fields for triggers and nodes. - Modified common validation schemas to include optional position and credential fields.
📝 WalkthroughWalkthroughThis PR introduces auto-save functionality for workflows with Redux state persistence. Key changes include restructuring the workflow state model with explicit change tracking, integrating redux-persist for session persistence, refactoring the workflow UI to use Redux dispatch instead of direct API calls, and updating backend routes to support new position and credential fields for triggers and nodes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant React as React Component
participant Hook as useAutoSave Hook
participant Redux as Redux Store
participant API as API Client
participant Backend as HTTP Backend
User->>React: Modifies workflow (drag node, edit config)
React->>Redux: dispatch(updateNodePosition/Config)
Redux->>React: State updated
React->>React: Marks isChanged flag
Note over Hook: 30-second interval elapses
Hook->>Hook: batchSave triggered
Hook->>Redux: Read current workflow state
Hook->>Redux: Set saveStatus to "saving"
alt Changes detected
Hook->>API: api.nodes.update (changed nodes)
Hook->>API: api.triggers.update (if trigger changed)
Hook->>API: api.workflows.put (if edges changed)
API->>Backend: POST/PUT requests
Backend->>Backend: Update database
Backend-->>API: Success response
API-->>Hook: Updates complete
Hook->>Redux: dispatch(markSynced)
Redux->>Redux: Clear isChanged flags
Hook->>React: saveStatus = "saved"
else No changes
Hook->>React: saveStatus remains "saved"
end
User->>React: Trigger beforeunload (page close)
React->>Hook: useAutoSave cleanup
Hook->>Hook: batchSave if unsaved changes
React->>User: Page unloads after save
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Pull request overview
This PR integrates redux-persist to persist workflow editing state in the web app, refactors workflow state management to Redux (nodes/edges/trigger config + autosave), and aligns backend + Prisma + shared Zod schemas with the updated workflow data shape.
Changes:
- Add
redux-persistand wirePersistGate+ persisted reducer to keep theworkflowslice across sessions. - Redesign the
workflowRedux slice to storetrigger/nodes/edgesplus “dirty” tracking and batched autosave. - Update backend routes, Prisma schema, and shared Zod validators to support optional trigger config/position and node credential fields.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds lock entries for redux-persist. |
| apps/web/package.json | Adds redux-persist dependency. |
| apps/web/store/index.ts | Wraps the root reducer with persistReducer and exports a persistor. |
| apps/web/app/components/providers.tsx | Wraps the app in PersistGate to rehydrate persisted Redux state. |
| apps/web/store/slices/workflowSlice.ts | Introduces new normalized workflow state + dirty tracking reducers. |
| apps/web/app/hooks/useAutoSave.ts | Adds interval-based + manual “batch save” to persist changed nodes/trigger/edges. |
| apps/web/app/workflows/[id]/page.tsx | Refactors workflow canvas to use Redux state + autosave and improved pre-execution validation. |
| apps/web/app/workflows/[id]/components/ConfigModal.tsx | Dispatches config updates directly into Redux and loads existing config from Redux. |
| apps/web/app/lib/api.ts | Tightens typings for workflow updates and node creation using shared Zod schemas. |
| packages/common/src/index.ts | Expands Zod schemas for trigger position + node credential id + optional config updates. |
| packages/db/prisma/schema.prisma | Makes trigger config optional and adds a trigger Position JSON field. |
| apps/http-backend/src/routes/userRoutes/userRoutes.ts | Aligns workflow fetch ordering, trigger/node creation & update handling with new fields. |
| apps/http-backend/src/index.ts | Changes Google OAuth router mount path. |
| packages/nodes/src/google-sheets/google-sheets.node.ts | Stops registering this node as an available trigger. |
| packages/nodes/src/gmail/gmail.node.ts | Stops registering this node as an available trigger. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
You can also share your feedback on Copilot code review. Take the survey.
| import { useAppDispatch, useAppSelector } from "@/app/hooks/redux"; | ||
| import { useAutoSave } from "@/app/hooks/useAutoSave"; | ||
| import { workflowActions } from "@/store/slices/workflowSlice"; | ||
| import { store } from "@/store"; |
There was a problem hiding this comment.
store is imported here but no longer used (remaining references are commented out). This will trigger ESLint no-unused-vars warnings in the web app lint config; please remove the unused import.
| const workflowId = params.id as string; | ||
| const dispatch = useAppDispatch() | ||
| const reduxWorkflow = useAppSelector(s => s.workflow) | ||
| const { saveStatus, batchSave, displayStatus } = useAutoSave(workflowId) |
There was a problem hiding this comment.
saveStatus is destructured from useAutoSave but not used in this component. Since the repo enables TypeScript-ESLint recommended rules, this will be reported as an unused variable; consider omitting it from the destructuring.
| export interface WorkflowSlice { | ||
| data: Workflow; | ||
| isChanged: { | ||
| trigger: boolean; | ||
| nodes: boolean; |
There was a problem hiding this comment.
WorkflowSlice state shape has been changed to { data, isChanged, ... }, but there are still app components selecting the old fields (e.g. s.workflow.workflow_id, s.workflow.trigger, s.workflow.nodes). This will break type-checking/runtime state reads unless those callers are updated or you provide a backwards-compatible adapter (e.g. keep deprecated fields or selectors).
| app.use("/user" , userRouter) | ||
| app.use('/node', sheetRouter) | ||
| app.use('/oauth/google', googleAuth) | ||
| app.use('/auth/google', googleAuth) |
There was a problem hiding this comment.
Changing the mounted OAuth router path to /auth/google appears to break existing OAuth URLs/redirect URIs that still use /oauth/google/... (e.g. apps/http-backend/src/routes/google_callback.ts defaults to http://localhost:3002/oauth/google/callback). Please update the redirect URI defaults and any frontend/back-end links to match the new mount path (or keep the old path as an alias).
| return ()=>{ | ||
| clearInterval(interval); | ||
| window.removeEventListener("beforeunload", handleBeforeUnload) | ||
| batchSave() | ||
| } |
There was a problem hiding this comment.
batchSave() is invoked in the effect cleanup; since batchSave calls setSaveStatus, this can trigger React warnings about setting state on an unmounted component. Consider avoiding state updates during unmount (e.g. guard with an isMounted ref, or split the persistence side-effect from the UI status updates).
| const updatedConfig = ({ ...config, [fieldName]: value }) | ||
| console.log(fieldName, " ", value, " ", nodeConfig) | ||
| console.log(config, "from handle field function - 1") | ||
| setConfig((prev) => ({ ...prev, [fieldName]: value })); | ||
| dispatchConfig(updatedConfig); |
There was a problem hiding this comment.
handleFieldChange builds updatedConfig from the stale config closure, but then updates state via the functional setConfig(prev => ...). This can cause Redux to receive a different config than the local state when changes happen quickly. Consider deriving a single nextConfig from prev and using that for both setConfig and dispatchConfig (and for dependent-field option fetching).
| useEffect(() => { | ||
| setConfig({}); | ||
| // We no longer set local credentials here; handled by useCredentials! | ||
| if (!selectedNode) { | ||
| setConfig({}); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When selectedNode changes, the effect loads loadedConfig but never resets dynamicOptions. This can leave dropdown options from the previous node in state and show incorrect options for the newly selected node. Consider clearing dynamicOptions (and any other per-node derived state like activeField/testResult) at the start of this effect.
|
|
||
| ...(dataSafe.data.position !== undefined ? { position: dataSafe.data.position } : {}), | ||
| ...(dataSafe.data.Config !== undefined ? { config: dataSafe.data.Config } : {}), | ||
| ...(dataSafe.data.Config?.credentialId ? { CredentialsID: dataSafe.data.Config.credentialId } : {}) | ||
| } |
There was a problem hiding this comment.
The updated node update logic only sets CredentialsID when Config.credentialId is truthy, but never clears it when credentials are removed/emptied in the config. This can leave a stale credential relationship in the DB. Consider explicitly setting CredentialsID to null when Config is provided and credentialId is missing/empty (or accept a top-level credential field for updates).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (2)
622-635:⚠️ Potential issue | 🟡 MinorIncorrect status code for update operation.
Line 632 returns
CREATED(201) for an update operation. This should returnOK(200) since no new resource is being created.Proposed fix
if (updatedTrigger) - return res.status(statusCodes.CREATED).json({ + return res.status(statusCodes.OK).json({ message: "Trigger updated", data: updatedTrigger, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts` around lines 622 - 635, The handler currently returns statusCodes.CREATED after updating an existing Trigger; locate the prismaClient.trigger.update call and the following if (updatedTrigger) branch and change the response status from statusCodes.CREATED to statusCodes.OK so an update returns HTTP 200 (OK) instead of 201 (Created) when sending the res.status(...).json({ message: "Trigger updated", data: updatedTrigger }).
581-595:⚠️ Potential issue | 🟡 MinorTwo issues: duplicate credential storage and incorrect status code.
Credential duplication: Line 587 extracts
credentialIdfrom insideConfigand stores it inCredentialsID. However, ifConfigis also stored (line 586), thecredentialIdwill exist in both theconfigJSON field and theCredentialsIDcolumn. Consider strippingcredentialIdfromConfigbefore saving, as the commented code on line 580 originally intended.Status code: Line 592 returns
CREATED(201) for an update operation. Updates should returnOK(200) instead.Proposed fix
const updateNode = await prismaClient.node.update({ where: { id: dataSafe.data.NodeId }, data: { - ...(dataSafe.data.position !== undefined ? { position: dataSafe.data.position } : {}), - ...(dataSafe.data.Config !== undefined ? { config: dataSafe.data.Config } : {}), - ...(dataSafe.data.Config?.credentialId ? { CredentialsID: dataSafe.data.Config.credentialId } : {}) + ...(dataSafe.data.Config !== undefined ? { + config: (() => { + const { credentialId, ...restConfig } = dataSafe.data.Config || {}; + return restConfig; + })() + } : {}), + ...(dataSafe.data.Config?.credentialId ? { CredentialsID: dataSafe.data.Config.credentialId } : {}), + ...(dataSafe.data.CredentialId !== undefined ? { CredentialsID: dataSafe.data.CredentialId } : {}) } }); if (updateNode) - return res.status(statusCodes.CREATED).json({ + return res.status(statusCodes.OK).json({ message: "Node updated", data: updateNode, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts` around lines 581 - 595, The update block currently writes credentialId both into the config JSON and into CredentialsID and returns a 201; change prismaClient.node.update usage so you remove/omit credentialId from dataSafe.data.Config before including it in the update (e.g., build a cleanedConfig that copies dataSafe.data.Config without credentialId and use that for the config field) while still setting CredentialsID to the extracted credentialId when present (referencing the existing updateNode and prismaClient.node.update usage and dataSafe.data.Config/CredentialsID symbols), and change the HTTP response to return statusCodes.OK instead of statusCodes.CREATED for the update response.apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
347-347:⚠️ Potential issue | 🟡 MinorRemove console.log in JSX render.
Line 347 has a
console.log(options)inside the JSX return, which will log on every render. This should be removed for production code.🧹 Remove debug log
- {console.log(options)} <option value="">Select {field.label.toLowerCase()}</option>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx at line 347, Remove the debug console.log by deleting the console.log(options) expression that was placed inside the JSX return of the ConfigModal component (the render for ConfigModal in ConfigModal.tsx). If you need to keep the log for local debugging, move it out of the JSX into a useEffect or temporary development-only block (e.g., useEffect(() => { console.log(options) }, [options])) so it doesn't execute on every render or ship to production.
🧹 Nitpick comments (13)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (2)
591-595: Missing fallback response if update returns falsy value.If
updateNodeis falsy, no response is sent and the request will hang. While Prisma'supdate()typically throws on failure (e.g., record not found), defensive coding suggests either removing the conditional check or adding an else branch.Proposed fix - remove unnecessary check
- if (updateNode) - return res.status(statusCodes.CREATED).json({ - message: "Node updated", - data: updateNode, - }); + return res.status(statusCodes.OK).json({ + message: "Node updated", + data: updateNode, + });The same pattern applies to the trigger update route (lines 631-635).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts` around lines 591 - 595, The handler currently only returns a response when updateNode is truthy, causing the request to hang if it's falsy; update the code in the route handler that calls updateNode (and the analogous handler calling updateTrigger) to always send a response: either remove the unnecessary conditional and always return the created/updated payload, or add an else branch that returns a proper fallback (e.g., res.status(404).json({ message: "Node not found" }) or res.status(500) for unexpected failures). Ensure the change is applied to the block around the updateNode response and the similar block around updateTrigger so every execution path ends with a res.* call.
465-474: Consider allowingnullinstead of defaulting to empty object.The Prisma schema defines
Position Json?as nullable. Defaulting to{}when Position is not provided stores an empty object instead ofnull, which may have different semantics when querying or checking for "no position set."Consider this alternative if null semantics are preferred
- Position: dataSafe.data.Position || {} + Position: dataSafe.data.Position ?? null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts` around lines 465 - 474, The code currently forces Position to an empty object when missing, but the Prisma field Position is nullable (Json?), so change the create payload in prismaClient.trigger.create to pass null (or omit the Position key) when dataSafe.data.Position is undefined to preserve null semantics; locate the prismaClient.trigger.create call and replace the unconditional Position: dataSafe.data.Position || {} with logic that sets Position to dataSafe.data.Position ?? null (or conditionally exclude Position from the data object) so missing positions are stored as null instead of {}.packages/common/src/index.ts (1)
2-2: Unused import.The
numberimport fromzod/v4is not used anywhere in this file.🧹 Proposed fix
-import { number } from "zod/v4";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/index.ts` at line 2, Remove the unused named import "number" from "zod/v4" in this module (the import statement that reads import { number } from "zod/v4"); simply delete the unused import so the file no longer imports the unused symbol "number" and run lint/tsc to ensure no other references remain.apps/web/app/workflows/[id]/components/ConfigModal.tsx (2)
65-73: Consider debouncing Redux dispatches on config changes.
dispatchConfigis called on every keystroke in text fields (Lines 370-374, 395-399). This causes frequent Redux state updates and potential performance issues, especially with redux-persist writing to localStorage.⚡ Proposed optimization with debounce
+import { useMemo } from "react"; +import { debounce } from "lodash"; // or implement a simple debounce const dispatchConfig = (newConfig: Record<string, any>) => { if (!selectedNode) return; const isTrigger = reduxWorkflow.trigger?.TriggerId === selectedNode.id; if (isTrigger) { dispatch(workflowActions.updateTriggerConfig({ config: newConfig })); } else { dispatch(workflowActions.updateNodeConfig({ nodeId: selectedNode.id, config: newConfig })); } }; +// Debounced version for text input changes +const debouncedDispatchConfig = useMemo( + () => debounce(dispatchConfig, 300), + [selectedNode?.id, reduxWorkflow.trigger?.TriggerId] +);Then use
debouncedDispatchConfigfor text/input fields and immediatedispatchConfigfor dropdowns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 65 - 73, dispatchConfig currently dispatches on every keystroke causing excessive Redux updates (see dispatchConfig, selectedNode, reduxWorkflow.trigger?.TriggerId, workflowActions.updateTriggerConfig and workflowActions.updateNodeConfig); implement a debounced wrapper (e.g., debouncedDispatchConfig) using a stable debounce utility (lodash.debounce or a custom hook like useDebouncedCallback) and use debouncedDispatchConfig for text/input change handlers while keeping the immediate dispatchConfig for dropdown/select changes so dropdowns remain responsive; ensure the debounced function is memoized/created once per component (useRef or useCallback) and cancelled on unmount to avoid stale updates.
26-26: Remove commented-out code.Multiple sections of code are commented out rather than removed (Lines 26, 35, 244-255, 598-606). This adds noise and makes the code harder to maintain. Since the
onSavecallback flow is being intentionally removed in favor of Redux-based persistence, these comments should be deleted.🧹 Clean up commented code
Remove all the commented-out sections:
- Line 26:
// onSave: (selectedNode: string, config: any, userId: string) => Promise<void>;- Line 35:
// onSave,- Lines 244-255: The entire commented
handleSavefunction- Lines 598-606: The commented Cancel button
This keeps the codebase clean and the git history already preserves the removed code if needed.
Also applies to: 35-35, 244-255, 598-606
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx at line 26, Remove all commented-out onSave-related code and UI comments to clean up the component: delete the commented prop declaration "// onSave: (selectedNode: string, config: any, userId: string) => Promise<void>;", the commented prop usage "// onSave,", the entire commented-out handleSave function block, and the commented Cancel button markup so only the active Redux-based persistence flow remains; locate these by looking for the identifiers "onSave", "handleSave", and the Cancel button JSX comment inside ConfigModal.tsx and remove those commented sections entirely.apps/web/app/components/providers.tsx (1)
38-51: Consider adding a loading indicator for PersistGate.Using
loading={null}means children render immediately without waiting for rehydration. This could cause a brief flash where the persisted workflow data isn't available yet. For a smoother UX, consider adding a minimal loading state.💡 Optional: Add loading indicator
- <PersistGate loading={null} persistor={persistor}> + <PersistGate + loading={<div className="min-h-screen" />} + persistor={persistor} + >Or for a more visible loading state:
loading={<div className="flex items-center justify-center min-h-screen">Loading...</div>}The current approach works but may cause a brief content flash on slow devices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/components/providers.tsx` around lines 38 - 51, Replace the PersistGate's loading={null} so the app shows a minimal loading indicator while Redux rehydrates; update the PersistGate component (PersistGate, persistor) to pass a small fallback UI (e.g., a centered spinner or simple loading div) via the loading prop so children don't render until rehydration completes and avoid content flash.apps/web/store/index.ts (1)
14-19: Dynamic require for storage is correct but consider async import.The dynamic
requireworks for SSR compatibility, but usingrequirein an ES module context (the package.json shows"type": "module") can be fragile. Consider using dynamicimport()for better ES module compatibility.♻️ Alternative using dynamic import (optional)
// If issues arise with require in ESM context, consider: let browserStorage: any = null; const createStorage = () => { if (typeof window !== 'undefined') { // Synchronous require works here because this code only runs client-side // after the module is already loaded return require('redux-persist/lib/storage').default; } return createNoopStorage(); }The current implementation should work, but monitor for any bundler warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/store/index.ts` around lines 14 - 19, The createStorage function uses a CommonJS require which can be fragile in ESM builds; change createStorage to load the browser storage via dynamic import (await import('redux-persist/lib/storage')) and return the imported default when typeof window !== 'undefined', otherwise return createNoopStorage(); make createStorage async (or provide an async getStorage wrapper) and update any callers to await it, or cache the imported module locally to avoid repeated imports; reference createStorage and createNoopStorage when making the change.apps/web/store/slices/workflowSlice.ts (2)
89-94: Unnecessary optional chaining onchangedNodeIds.
changedNodeIdsis initialized as[]ininitialState(line 71) and never set toundefined/null, so the optional chaining (state.changedNodeIds?.includes,state.changedNodeIds?.push) is unnecessary. The same pattern appears inupdateNodePosition(lines 103-104) andupdateNodeConfig(lines 114-115).Suggested cleanup
addWorkflowNode(state, action: PayloadAction<NodeItem>) { state.data.nodes.push(action.payload) state.isChanged.nodes = true - if(!state.changedNodeIds?.includes(action.payload.NodeId)) - state.changedNodeIds?.push(action.payload.NodeId) + if(!state.changedNodeIds.includes(action.payload.NodeId)) + state.changedNodeIds.push(action.payload.NodeId) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/store/slices/workflowSlice.ts` around lines 89 - 94, The optional chaining on state.changedNodeIds in addWorkflowNode, updateNodePosition, and updateNodeConfig is unnecessary because initialState initializes changedNodeIds as an empty array; replace uses of state.changedNodeIds?.includes(...) and state.changedNodeIds?.push(...) with direct calls state.changedNodeIds.includes(...) and state.changedNodeIds.push(...) in those functions (addWorkflowNode, updateNodePosition, updateNodeConfig) to simplify the code and rely on the guaranteed array initialization.
7-7: Consider typingConfigmore strictly thanany.Using
anyforConfig(lines 7 and 20) bypasses TypeScript's type checking. If the config structure varies by trigger/node type, consider using a discriminated union,Record<string, unknown>, or a generic parameter for better type safety.Also applies to: 20-20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/store/slices/workflowSlice.ts` at line 7, The Config field is currently typed as any (symbol: Config) which disables type safety; replace it with a stricter type by introducing a Config type alias or generic for the slice (e.g., a discriminated-union for different trigger/node shapes, or at minimum Record<string, unknown>), update the declarations that reference Config (the occurrences shown at lines with "Config: any;") to use the new alias or generic parameter, and adjust any places that read/write Config to use narrow type guards or casts so the compiler enforces the expected properties.apps/web/app/workflows/[id]/page.tsx (3)
159-253: Significant code duplication between Redux and API branches.The two branches in
loadWorkflows(lines 159-253 for Redux data and lines 257-406 for API fetch) share nearly identical logic for transforming triggers, nodes, and edges into React Flow format. Consider extracting a shared helper function to reduce duplication and maintenance burden.Example extraction
function buildFlowNodes( trigger: { TriggerId: string; name: string; Config: any; position?: any }, nodes: Array<{ NodeId: string; name: string; Config: any; position?: any; AvailableNodeID: string }>, handleNodeConfigure: (node: any) => void, setActionOpen: (open: boolean) => void, checkIsConfigure: (name: string, config: any) => boolean ) { const triggerPosition = ensurePosition(trigger.position, DEFAULT_TRIGGER_POSITION); const triggerNode = { id: trigger.TriggerId, type: "customNode", position: triggerPosition, data: { label: trigger.name || "Trigger", icon: "⚡", nodeType: "trigger", isConfigured: checkIsConfigure(trigger.name, trigger.Config), onConfigure: () => handleNodeConfigure({ id: trigger.TriggerId, name: trigger.name }), }, }; // ... rest of transformation logic return { triggerNode, transformedNodes, actionPlaceholder, edges }; }Also applies to: 257-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/page.tsx around lines 159 - 253, The Redux and API branches in loadWorkflows duplicate the flow construction logic (trigger/node/placeholder/edges); extract a helper (e.g., buildFlowNodes) that accepts trigger, nodes, edges plus utilities (ensurePosition, DEFAULT_TRIGGER_POSITION, handleNodeConfigure, checkIsConfigure, setActionOpen) and returns { finalNodes, newEdges, errorState } or similar; replace both branches to call buildFlowNodes and then setNodes(finalNodes), setEdges(newEdges), setError(null) (or setError from returned errorState) to remove duplication while preserving existing IDs/placeholder logic and the e-action-... edge creation.
429-459: Remove commented-out code.The commented-out API calls in
nodeChangeDb(and similar blocks throughout the file at lines 497-502, 573-579, 677-691, 779-822) should be removed. If this code is needed for reference, preserve it in version control history or a separate documentation file rather than leaving it inline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/page.tsx around lines 429 - 459, Remove the commented-out API calls inside nodeChangeDb and the other similar blocks referenced (the commented await api.triggers.update and await api.nodes.update calls) so the function only contains active logic: the try/catch with dispatch calls to workflowActions.updateTriggerPosition and workflowActions.updateNodePosition and the setError error handling; do the same for the other blocks at the mentioned locations (where commented await api.* calls appear) so only live code remains and history is preserved in VCS rather than inline comments.
79-79: Remove debugconsole.logstatements before merging.Multiple
console.logcalls (lines 79, 128, 264, 494) should be removed or converted to proper logging for production code.Also applies to: 128-128, 264-264, 494-494
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/workflows/`[id]/page.tsx at line 79, Remove the debug console.log calls in this component: delete the specific console.log("This is from the Execute Button", data) and the other ad-hoc console.log statements in the same file, and if persistent tracing is needed replace them with a proper logging utility (e.g., useLogger or processLogger) at appropriate levels; search for remaining console.log occurrences in the page component and either remove them or convert them to structured logger calls (keeping sensitive data out of logs).apps/web/app/hooks/useAutoSave.ts (1)
13-13: Misleading variable name.
hasUnChangedimplies "has no changes" but actually means "has unsaved changes". Rename tohasUnsavedChangesfor clarity.Suggested rename
- const hasUnChanged = isChangedState.trigger || isChangedState.edges || isChangedState.nodes; + const hasUnsavedChanges = isChangedState.trigger || isChangedState.edges || isChangedState.nodes;Then update line 17:
- hasUnChanged ? 'Unsaved Changes' : 'Saved' + hasUnsavedChanges ? 'Unsaved Changes' : 'Saved'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/hooks/useAutoSave.ts` at line 13, Rename the misleading variable hasUnChanged to hasUnsavedChanges inside useAutoSave (where it is defined from isChangedState) and update all local references to the new name (e.g., the usage at the boolean expression using isChangedState.trigger || isChangedState.edges || isChangedState.nodes and any subsequent reads/writes) so the identifier accurately reflects that it means "has unsaved changes" rather than "unchanged".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/http-backend/src/index.ts`:
- Line 37: The Google OAuth redirect URI is mismatched because the route was
changed to use app.use('/auth/google', googleAuth) but the environment value
used in google_callback.ts (GOOGLE_REDIRECT_URI) still points to
http://localhost:3002/oauth/google/callback; update the GOOGLE_REDIRECT_URI to
http://localhost:3002/auth/google/callback (and update any .env or config
loading code that populates GOOGLE_REDIRECT_URI) and confirm the same redirect
URI is registered in the Google Cloud Console for the OAuth client so the
callback handled by the googleAuth route (and its callback handler in
google_callback.ts) receives the OAuth response.
In `@apps/web/app/hooks/useAutoSave.ts`:
- Around line 74-79: The cleanup effect currently calls batchSave() without
awaiting it and omits batchSave from the deps, risking aborted in-flight saves
and stale closures; modify the hook so batchSave is memoized with useCallback
(including workflowId and any other captured state) and track in-flight requests
with a ref (e.g., isSavingRef) so the cleanup handler (that clears interval and
removes handleBeforeUnload) can either await completion of the in-flight save
before returning or explicitly document that the final save is best-effort;
update the effect dependency array to include the memoized batchSave and ensure
handleBeforeUnload/interval teardown only proceeds after checking the in-flight
ref or awaiting the promise when feasible.
- Around line 64-69: The beforeunload handler (handleBeforeUnload) currently
calls the async batchSave() which won't complete before the page unload; change
the handler to avoid awaiting async work: detect unsaved changes via
store.getState().workflow.isChanged and either (A) send a synchronous
fire-and-forget save using navigator.sendBeacon with the minimal payload instead
of batchSave(), or (B) remove the async call and simply set
e.preventDefault()/e.returnValue to show a confirmation dialog and let the user
choose to stay and trigger batchSave elsewhere; update handleBeforeUnload to use
navigator.sendBeacon when available and fall back to the confirmation-only
behavior so data isn't lost.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 216-240: The effect uses reduxWorkflow and fetchOptionsMap but
only depends on selectedNode; to fix, derive the minimal pieces from
reduxWorkflow (e.g., const triggerId = reduxWorkflow.trigger?.TriggerId and
const nodesSnapshot = reduxWorkflow.nodes) using useMemo or local variables and
include those derived values in the useEffect dependency array instead of the
whole reduxWorkflow, and ensure fetchOptionsMap is stable (wrap it in a ref or
memoize it) and reference fetchOptionsMapRef.current in the effect; update the
useEffect dependency array to [selectedNode, triggerId, nodesSnapshot,
fetchOptionsMapRef.current] and keep the existing logic that calls
getNodeConfig, fetchFn and setDynamicOptions.
In `@apps/web/app/workflows/`[id]/page.tsx:
- Around line 138-154: The useEffect updating nodes uses setNodes and the
render-scoped checkIsConfigure but doesn't include them in the dependency array;
wrap checkIsConfigure in useCallback (or move it outside the component) so it is
stable, then add checkIsConfigure and setNodes (if not stable) to the useEffect
deps, or explicitly disable the eslint rule with a comment and rationale; update
references to the function name checkIsConfigure in the effect and ensure the
new useCallback signature captures only safe dependencies (or has an empty deps
array) so the linter is satisfied.
In `@packages/common/src/index.ts`:
- Line 29: Update the schema field names to use camelCase to match
backend/database expectations: in TriggerSchema and TriggerUpdateSchema replace
the Position field with position (e.g., Position: z.object({...}).optional() ->
position: z.object({...}).optional()) and normalize credential fields to
credentialId (replace any CredentialId or CredentialID occurrences) so they
align with the database mapping (CredentialsID); ensure any parsing/validation
keys and exported types referencing TriggerSchema or TriggerUpdateSchema are
updated accordingly to avoid serialization mismatches.
In `@packages/db/prisma/schema.prisma`:
- Around line 36-39: The Prisma schema changes made the field config optional
and added the Position field on the Trigger model (with workflowId and
AvailableTriggerID present); generate and apply a migration to update the DB by
running: npx prisma migrate dev --name add_trigger_position_and_optional_config,
verify the generated SQL in packages/db/prisma/migrations/*/migration.sql and
commit that migration file, then run prisma db push or the migrate apply flow to
ensure the database reflects the changes; optionally plan a follow-up to
standardize the Position identifier (e.g., rename to position) for consistency
with Node.position.
In `@packages/nodes/src/gmail/gmail.node.ts`:
- Around line 33-36: The static async register method currently only calls
NodeRegistry.register(this.definition) which prevents the gmail trigger from
being persisted; restore the trigger registration by calling and awaiting
NodeRegistry.registerTrigger(this.definition) in static async register (i.e.,
re-enable the commented line so both NodeRegistry.register(this.definition) and
await NodeRegistry.registerTrigger(this.definition) are invoked using
this.definition).
In `@packages/nodes/src/google-sheets/google-sheets.node.ts`:
- Around line 34-37: The trigger registration for the Google Sheets node was
commented out so its trigger ("google_sheet") isn't upserted into
availableTrigger; restore or relocate that call so triggers are registered
during startup. Re-enable the call to NodeRegistry.registerTrigger in the static
async register method of the GoogleSheets node (or move equivalent logic to the
global startup path where NodeRegistry.registerAll runs) so that
NodeRegistry.registerTrigger(this.definition) executes and the
/getAvailableTriggers endpoint can read the google_sheet entry from the
availableTrigger table.
---
Outside diff comments:
In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts`:
- Around line 622-635: The handler currently returns statusCodes.CREATED after
updating an existing Trigger; locate the prismaClient.trigger.update call and
the following if (updatedTrigger) branch and change the response status from
statusCodes.CREATED to statusCodes.OK so an update returns HTTP 200 (OK) instead
of 201 (Created) when sending the res.status(...).json({ message: "Trigger
updated", data: updatedTrigger }).
- Around line 581-595: The update block currently writes credentialId both into
the config JSON and into CredentialsID and returns a 201; change
prismaClient.node.update usage so you remove/omit credentialId from
dataSafe.data.Config before including it in the update (e.g., build a
cleanedConfig that copies dataSafe.data.Config without credentialId and use that
for the config field) while still setting CredentialsID to the extracted
credentialId when present (referencing the existing updateNode and
prismaClient.node.update usage and dataSafe.data.Config/CredentialsID symbols),
and change the HTTP response to return statusCodes.OK instead of
statusCodes.CREATED for the update response.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Line 347: Remove the debug console.log by deleting the console.log(options)
expression that was placed inside the JSX return of the ConfigModal component
(the render for ConfigModal in ConfigModal.tsx). If you need to keep the log for
local debugging, move it out of the JSX into a useEffect or temporary
development-only block (e.g., useEffect(() => { console.log(options) },
[options])) so it doesn't execute on every render or ship to production.
---
Nitpick comments:
In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts`:
- Around line 591-595: The handler currently only returns a response when
updateNode is truthy, causing the request to hang if it's falsy; update the code
in the route handler that calls updateNode (and the analogous handler calling
updateTrigger) to always send a response: either remove the unnecessary
conditional and always return the created/updated payload, or add an else branch
that returns a proper fallback (e.g., res.status(404).json({ message: "Node not
found" }) or res.status(500) for unexpected failures). Ensure the change is
applied to the block around the updateNode response and the similar block around
updateTrigger so every execution path ends with a res.* call.
- Around line 465-474: The code currently forces Position to an empty object
when missing, but the Prisma field Position is nullable (Json?), so change the
create payload in prismaClient.trigger.create to pass null (or omit the Position
key) when dataSafe.data.Position is undefined to preserve null semantics; locate
the prismaClient.trigger.create call and replace the unconditional Position:
dataSafe.data.Position || {} with logic that sets Position to
dataSafe.data.Position ?? null (or conditionally exclude Position from the data
object) so missing positions are stored as null instead of {}.
In `@apps/web/app/components/providers.tsx`:
- Around line 38-51: Replace the PersistGate's loading={null} so the app shows a
minimal loading indicator while Redux rehydrates; update the PersistGate
component (PersistGate, persistor) to pass a small fallback UI (e.g., a centered
spinner or simple loading div) via the loading prop so children don't render
until rehydration completes and avoid content flash.
In `@apps/web/app/hooks/useAutoSave.ts`:
- Line 13: Rename the misleading variable hasUnChanged to hasUnsavedChanges
inside useAutoSave (where it is defined from isChangedState) and update all
local references to the new name (e.g., the usage at the boolean expression
using isChangedState.trigger || isChangedState.edges || isChangedState.nodes and
any subsequent reads/writes) so the identifier accurately reflects that it means
"has unsaved changes" rather than "unchanged".
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 65-73: dispatchConfig currently dispatches on every keystroke
causing excessive Redux updates (see dispatchConfig, selectedNode,
reduxWorkflow.trigger?.TriggerId, workflowActions.updateTriggerConfig and
workflowActions.updateNodeConfig); implement a debounced wrapper (e.g.,
debouncedDispatchConfig) using a stable debounce utility (lodash.debounce or a
custom hook like useDebouncedCallback) and use debouncedDispatchConfig for
text/input change handlers while keeping the immediate dispatchConfig for
dropdown/select changes so dropdowns remain responsive; ensure the debounced
function is memoized/created once per component (useRef or useCallback) and
cancelled on unmount to avoid stale updates.
- Line 26: Remove all commented-out onSave-related code and UI comments to clean
up the component: delete the commented prop declaration "// onSave:
(selectedNode: string, config: any, userId: string) => Promise<void>;", the
commented prop usage "// onSave,", the entire commented-out handleSave function
block, and the commented Cancel button markup so only the active Redux-based
persistence flow remains; locate these by looking for the identifiers "onSave",
"handleSave", and the Cancel button JSX comment inside ConfigModal.tsx and
remove those commented sections entirely.
In `@apps/web/app/workflows/`[id]/page.tsx:
- Around line 159-253: The Redux and API branches in loadWorkflows duplicate the
flow construction logic (trigger/node/placeholder/edges); extract a helper
(e.g., buildFlowNodes) that accepts trigger, nodes, edges plus utilities
(ensurePosition, DEFAULT_TRIGGER_POSITION, handleNodeConfigure,
checkIsConfigure, setActionOpen) and returns { finalNodes, newEdges, errorState
} or similar; replace both branches to call buildFlowNodes and then
setNodes(finalNodes), setEdges(newEdges), setError(null) (or setError from
returned errorState) to remove duplication while preserving existing
IDs/placeholder logic and the e-action-... edge creation.
- Around line 429-459: Remove the commented-out API calls inside nodeChangeDb
and the other similar blocks referenced (the commented await api.triggers.update
and await api.nodes.update calls) so the function only contains active logic:
the try/catch with dispatch calls to workflowActions.updateTriggerPosition and
workflowActions.updateNodePosition and the setError error handling; do the same
for the other blocks at the mentioned locations (where commented await api.*
calls appear) so only live code remains and history is preserved in VCS rather
than inline comments.
- Line 79: Remove the debug console.log calls in this component: delete the
specific console.log("This is from the Execute Button", data) and the other
ad-hoc console.log statements in the same file, and if persistent tracing is
needed replace them with a proper logging utility (e.g., useLogger or
processLogger) at appropriate levels; search for remaining console.log
occurrences in the page component and either remove them or convert them to
structured logger calls (keeping sensitive data out of logs).
In `@apps/web/store/index.ts`:
- Around line 14-19: The createStorage function uses a CommonJS require which
can be fragile in ESM builds; change createStorage to load the browser storage
via dynamic import (await import('redux-persist/lib/storage')) and return the
imported default when typeof window !== 'undefined', otherwise return
createNoopStorage(); make createStorage async (or provide an async getStorage
wrapper) and update any callers to await it, or cache the imported module
locally to avoid repeated imports; reference createStorage and createNoopStorage
when making the change.
In `@apps/web/store/slices/workflowSlice.ts`:
- Around line 89-94: The optional chaining on state.changedNodeIds in
addWorkflowNode, updateNodePosition, and updateNodeConfig is unnecessary because
initialState initializes changedNodeIds as an empty array; replace uses of
state.changedNodeIds?.includes(...) and state.changedNodeIds?.push(...) with
direct calls state.changedNodeIds.includes(...) and
state.changedNodeIds.push(...) in those functions (addWorkflowNode,
updateNodePosition, updateNodeConfig) to simplify the code and rely on the
guaranteed array initialization.
- Line 7: The Config field is currently typed as any (symbol: Config) which
disables type safety; replace it with a stricter type by introducing a Config
type alias or generic for the slice (e.g., a discriminated-union for different
trigger/node shapes, or at minimum Record<string, unknown>), update the
declarations that reference Config (the occurrences shown at lines with "Config:
any;") to use the new alias or generic parameter, and adjust any places that
read/write Config to use narrow type guards or casts so the compiler enforces
the expected properties.
In `@packages/common/src/index.ts`:
- Line 2: Remove the unused named import "number" from "zod/v4" in this module
(the import statement that reads import { number } from "zod/v4"); simply delete
the unused import so the file no longer imports the unused symbol "number" and
run lint/tsc to ensure no other references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32d813fb-b941-4b9c-baa1-0a6469c67435
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/http-backend/src/index.tsapps/http-backend/src/routes/userRoutes/userRoutes.tsapps/web/app/components/providers.tsxapps/web/app/hooks/useAutoSave.tsapps/web/app/lib/api.tsapps/web/app/workflows/[id]/components/ConfigModal.tsxapps/web/app/workflows/[id]/page.tsxapps/web/package.jsonapps/web/store/index.tsapps/web/store/slices/workflowSlice.tspackages/common/src/index.tspackages/db/prisma/schema.prismapackages/nodes/src/gmail/gmail.node.tspackages/nodes/src/google-sheets/google-sheets.node.ts
| app.use("/user" , userRouter) | ||
| app.use('/node', sheetRouter) | ||
| app.use('/oauth/google', googleAuth) | ||
| app.use('/auth/google', googleAuth) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for old route references in the codebase
rg -n 'oauth/google' --type ts --type tsx --type js
# Search for new route references
rg -n 'auth/google' --type ts --type tsx --type jsRepository: Dev-Pross/BuildFlow
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Search for old route references in the codebase (broader search)
echo "=== Searching for 'oauth/google' references ==="
rg -n 'oauth/google' -t ts -t js
echo -e "\n=== Searching for 'auth/google' references ==="
rg -n 'auth/google' -t ts -t js
echo -e "\n=== Checking the index.ts file context ==="
head -50 apps/http-backend/src/index.tsRepository: Dev-Pross/BuildFlow
Length of output: 3051
🏁 Script executed:
#!/bin/bash
# Check the googleAuth handler to see what routes it exposes
echo "=== Content of google_callback.ts ==="
cat -n apps/http-backend/src/routes/google_callback.ts
echo -e "\n=== Checking for any callback endpoint definitions ==="
rg -n 'callback|router\.' apps/http-backend/src/routes/google_callback.tsRepository: Dev-Pross/BuildFlow
Length of output: 11672
Critical: Route change breaks OAuth callback flow—update GOOGLE_REDIRECT_URI environment variable.
The Google OAuth route was moved from /oauth/google to /auth/google, but the default GOOGLE_REDIRECT_URI in google_callback.ts (line 22) still points to http://localhost:3002/oauth/google/callback. This causes a mismatch: Google will attempt to redirect to /oauth/google/callback, but the server callback endpoint is now at /auth/google/callback.
Update the GOOGLE_REDIRECT_URI environment variable to http://localhost:3002/auth/google/callback and ensure it matches the redirect URI registered in your Google Cloud Console OAuth application settings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/http-backend/src/index.ts` at line 37, The Google OAuth redirect URI is
mismatched because the route was changed to use app.use('/auth/google',
googleAuth) but the environment value used in google_callback.ts
(GOOGLE_REDIRECT_URI) still points to
http://localhost:3002/oauth/google/callback; update the GOOGLE_REDIRECT_URI to
http://localhost:3002/auth/google/callback (and update any .env or config
loading code that populates GOOGLE_REDIRECT_URI) and confirm the same redirect
URI is registered in the Google Cloud Console for the OAuth client so the
callback handled by the googleAuth route (and its callback handler in
google_callback.ts) receives the OAuth response.
| const handleBeforeUnload = (e: BeforeUnloadEvent) =>{ | ||
| const { isChanged } = store.getState().workflow | ||
| if(isChanged.edges || isChanged.nodes || isChanged.trigger){ | ||
| e.preventDefault() | ||
| batchSave(); | ||
| } |
There was a problem hiding this comment.
Async batchSave() in beforeunload won't complete before page unload.
beforeunload fires synchronously, and calling an async function here doesn't block the browser from navigating away. While e.preventDefault() shows a confirmation dialog, the batchSave() promise won't resolve in time, potentially losing user data.
Consider using navigator.sendBeacon for fire-and-forget persistence, or warn users about unsaved changes instead of attempting a save.
Alternative: warn only, don't attempt async save
const handleBeforeUnload = (e: BeforeUnloadEvent) =>{
const { isChanged } = store.getState().workflow
if(isChanged.edges || isChanged.nodes || isChanged.trigger){
e.preventDefault()
- batchSave();
+ // Browser will show "unsaved changes" dialog
+ // Async save won't complete before unload; rely on interval/manual save
}
}📝 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 handleBeforeUnload = (e: BeforeUnloadEvent) =>{ | |
| const { isChanged } = store.getState().workflow | |
| if(isChanged.edges || isChanged.nodes || isChanged.trigger){ | |
| e.preventDefault() | |
| batchSave(); | |
| } | |
| const handleBeforeUnload = (e: BeforeUnloadEvent) =>{ | |
| const { isChanged } = store.getState().workflow | |
| if(isChanged.edges || isChanged.nodes || isChanged.trigger){ | |
| e.preventDefault() | |
| // Browser will show "unsaved changes" dialog | |
| // Async save won't complete before unload; rely on interval/manual save | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/hooks/useAutoSave.ts` around lines 64 - 69, The beforeunload
handler (handleBeforeUnload) currently calls the async batchSave() which won't
complete before the page unload; change the handler to avoid awaiting async
work: detect unsaved changes via store.getState().workflow.isChanged and either
(A) send a synchronous fire-and-forget save using navigator.sendBeacon with the
minimal payload instead of batchSave(), or (B) remove the async call and simply
set e.preventDefault()/e.returnValue to show a confirmation dialog and let the
user choose to stay and trigger batchSave elsewhere; update handleBeforeUnload
to use navigator.sendBeacon when available and fall back to the
confirmation-only behavior so data isn't lost.
| return ()=>{ | ||
| clearInterval(interval); | ||
| window.removeEventListener("beforeunload", handleBeforeUnload) | ||
| batchSave() | ||
| } | ||
| }, [workflowId]) |
There was a problem hiding this comment.
Cleanup effect calls async batchSave() which won't complete.
When the component unmounts, React won't wait for the batchSave() promise to resolve. This means the final save attempt may be aborted mid-flight. Consider tracking in-flight requests with a ref and only cleaning up after completion, or accept that cleanup saves are best-effort.
Additionally, batchSave is not in the dependency array. Since batchSave is defined inside the component and captures workflowId in its closure, if workflowId changed without re-running the effect, stale closures could occur. However, since workflowId is in the deps, the effect re-runs correctly. Consider wrapping batchSave in useCallback with proper deps for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/hooks/useAutoSave.ts` around lines 74 - 79, The cleanup effect
currently calls batchSave() without awaiting it and omits batchSave from the
deps, risking aborted in-flight saves and stale closures; modify the hook so
batchSave is memoized with useCallback (including workflowId and any other
captured state) and track in-flight requests with a ref (e.g., isSavingRef) so
the cleanup handler (that clears interval and removes handleBeforeUnload) can
either await completion of the in-flight save before returning or explicitly
document that the final save is best-effort; update the effect dependency array
to include the memoized batchSave and ensure handleBeforeUnload/interval
teardown only proceeds after checking the in-flight ref or awaiting the promise
when feasible.
| useEffect(() => { | ||
| setConfig({}); | ||
| // We no longer set local credentials here; handled by useCredentials! | ||
| if (!selectedNode) { | ||
| setConfig({}); | ||
| return; | ||
| } | ||
| // Load existing saved config from Redux instead of starting empty | ||
| const isTrigger = reduxWorkflow.trigger?.TriggerId === selectedNode.id; | ||
| const loadedConfig = isTrigger ? (reduxWorkflow.trigger?.Config || {}) : (reduxWorkflow.nodes.find(n => n.NodeId === selectedNode.id)?.Config || {}) | ||
|
|
||
| setConfig(loadedConfig) | ||
|
|
||
| const nodeConfig = getNodeConfig(selectedNode.name || selectedNode.actionType); | ||
| if(nodeConfig?.fields){ | ||
| for(const field of nodeConfig.fields){ | ||
| if(field.fetchOptions && field.dependsOn && loadedConfig[field.dependsOn]){ | ||
| const fetchFn = fetchOptionsMap[field.fetchOptions]; | ||
| if(fetchFn){ | ||
| fetchFn(loadedConfig) | ||
| .then((option: any[])=> setDynamicOptions(prev => ({...prev, [field.name]: option}))) | ||
| .catch(()=> {}) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, [selectedNode]); |
There was a problem hiding this comment.
Missing dependencies in useEffect.
The useEffect hook references reduxWorkflow and fetchOptionsMap but they're not included in the dependency array. This could lead to stale closures where the effect uses outdated values.
However, adding reduxWorkflow would cause the effect to run on every Redux state change, which isn't desired. Consider using a ref or restructuring the logic.
🔧 Proposed fix using selective dependency
useEffect(() => {
if (!selectedNode) {
setConfig({});
return;
}
- // Load existing saved config from Redux instead of starting empty
- const isTrigger = reduxWorkflow.trigger?.TriggerId === selectedNode.id;
- const loadedConfig = isTrigger ? (reduxWorkflow.trigger?.Config || {}) : (reduxWorkflow.nodes.find(n => n.NodeId === selectedNode.id)?.Config || {})
+ // Extract config once when selectedNode changes
+ const trigger = reduxWorkflow.trigger;
+ const nodes = reduxWorkflow.nodes;
+ const isTrigger = trigger?.TriggerId === selectedNode.id;
+ const loadedConfig = isTrigger
+ ? (trigger?.Config || {})
+ : (nodes.find(n => n.NodeId === selectedNode.id)?.Config || {});
setConfig(loadedConfig)
// ... rest of effect
- }, [selectedNode]);
+ }, [selectedNode, reduxWorkflow.trigger?.TriggerId, reduxWorkflow.nodes]);Alternatively, memoize the config lookup outside the effect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 216 -
240, The effect uses reduxWorkflow and fetchOptionsMap but only depends on
selectedNode; to fix, derive the minimal pieces from reduxWorkflow (e.g., const
triggerId = reduxWorkflow.trigger?.TriggerId and const nodesSnapshot =
reduxWorkflow.nodes) using useMemo or local variables and include those derived
values in the useEffect dependency array instead of the whole reduxWorkflow, and
ensure fetchOptionsMap is stable (wrap it in a ref or memoize it) and reference
fetchOptionsMapRef.current in the effect; update the useEffect dependency array
to [selectedNode, triggerId, nodesSnapshot, fetchOptionsMapRef.current] and keep
the existing logic that calls getNodeConfig, fetchFn and setDynamicOptions.
| useEffect(()=>{ | ||
| setNodes(prev => prev.map(node => { | ||
| if(node.data?.isPlaceholder) return node; | ||
| if(node.data?.nodeType === 'trigger'){ | ||
| const reduxConfig = reduxWorkflow.data.trigger?.Config; | ||
| const name = reduxWorkflow.data.trigger?.name || ""; | ||
| return { ...node, data: { ...node.data, isConfigured: checkIsConfigure(name, reduxConfig)}} | ||
| } | ||
|
|
||
| if(node.data.nodeType === 'action'){ | ||
| const reduxNode = reduxWorkflow.data.nodes.find(n=> n.NodeId === node.id); | ||
| if(!reduxNode) return node; | ||
| return { ...node, data: { ...node.data, isConfigured: checkIsConfigure(reduxNode.name, reduxNode.Config)}}; | ||
| } | ||
| return node; | ||
| })); | ||
| }, [reduxWorkflow.data.nodes, reduxWorkflow.data.trigger]) |
There was a problem hiding this comment.
Missing dependencies in useEffect.
This effect calls setNodes and checkIsConfigure but neither is included in the dependency array. While setNodes from useNodesState is stable and checkIsConfigure is defined in render scope, the linter rule exists to catch subtle bugs. Wrap checkIsConfigure in useCallback and include both in deps, or disable the lint rule with an explanation.
Suggested fix
- }, [reduxWorkflow.data.nodes, reduxWorkflow.data.trigger])
+ }, [reduxWorkflow.data.nodes, reduxWorkflow.data.trigger, setNodes])And move checkIsConfigure outside the component or wrap in useCallback:
const checkIsConfigure = useCallback((nodeName: string, config: any): boolean => {
const nodeConfig = getNodeConfig(nodeName);
if(!nodeConfig || !nodeConfig.fields) return true;
const requiredFields = nodeConfig.fields.filter((f: any) => f.required);
if(requiredFields.length === 0) return true;
return requiredFields.every((f: any) => config?.[f.name] !== undefined && config?.[f.name] !== '');
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/workflows/`[id]/page.tsx around lines 138 - 154, The useEffect
updating nodes uses setNodes and the render-scoped checkIsConfigure but doesn't
include them in the dependency array; wrap checkIsConfigure in useCallback (or
move it outside the component) so it is stable, then add checkIsConfigure and
setNodes (if not stable) to the useEffect deps, or explicitly disable the eslint
rule with a comment and rationale; update references to the function name
checkIsConfigure in the effect and ensure the new useCallback signature captures
only safe dependencies (or has an empty deps array) so the linter is satisfied.
| Config: z.any().optional(), | ||
| WorkflowId: z.string(), | ||
| TriggerType: z.string().optional(), | ||
| Position: z.object({x: z.number(), y: z.number()}).optional() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how Position/position and CredentialId/CredentialID are used in backend routes
rg -n -C3 'Position|position|CredentialId|CredentialID' --type ts apps/http-backend/Repository: Dev-Pross/BuildFlow
Length of output: 4399
Standardize schema field naming to camelCase to prevent serialization bugs.
Frontend schemas have inconsistent casing that causes silent failures when backend routes map data to database operations:
- Position field:
TriggerSchemaandTriggerUpdateSchemausePosition(PascalCase), but backend node creation expectsposition(camelCase) when persisting to the database. - CredentialId field: Inconsistent as
CredentialIdvsCredentialIDacross schemas, while backend always maps toCredentialsIDin the database.
Change both fields to camelCase to match database column names and ensure consistent data flow.
🔧 Proposed fix
export const TriggerSchema = z.object({
Name: z.string(),
AvailableTriggerID: z.string(),
Config: z.any().optional(),
WorkflowId: z.string(),
TriggerType: z.string().optional(),
- Position: z.object({x: z.number(), y: z.number()}).optional()
+ position: z.object({x: z.number(), y: z.number()}).optional()
});
export const TriggerUpdateSchema = z.object({
TriggerId: z.string(),
Config: z.any().optional(),
- Position: z.object({ x: z.number(), y: z.number()}).optional(),
- CredentialID: z.string().optional()
+ position: z.object({ x: z.number(), y: z.number()}).optional(),
+ CredentialId: z.string().optional()
});Also applies to: Lines 38-43, 59-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/src/index.ts` at line 29, Update the schema field names to
use camelCase to match backend/database expectations: in TriggerSchema and
TriggerUpdateSchema replace the Position field with position (e.g., Position:
z.object({...}).optional() -> position: z.object({...}).optional()) and
normalize credential fields to credentialId (replace any CredentialId or
CredentialID occurrences) so they align with the database mapping
(CredentialsID); ensure any parsing/validation keys and exported types
referencing TriggerSchema or TriggerUpdateSchema are updated accordingly to
avoid serialization mismatches.
| config Json? | ||
| workflowId String? @unique | ||
| AvailableTriggerID String | ||
| Position Json? |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for recent migration files
fd -e sql . packages/db/prisma/migrations --changed-within 7d
# Check migration status
cat packages/db/prisma/migrations/*/migration.sql 2>/dev/null | tail -50Repository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Check if migrations directory exists and list its contents
find packages/db/prisma/migrations -type f -name "*.sql" 2>/dev/null | head -20
# If no migrations, check the directory structure
ls -la packages/db/prisma/ 2>/dev/null
# Check git status for any migration files
git ls-files packages/db/prisma/migrations/ 2>/dev/null | head -20Repository: Dev-Pross/BuildFlow
Length of output: 346
🏁 Script executed:
# Get the schema file to verify the actual changes mentioned in the review
cat -n packages/db/prisma/schema.prisma | sed -n '30,80p'Repository: Dev-Pross/BuildFlow
Length of output: 2329
🏁 Script executed:
# Check git diff or git log to see what actually changed in this commit
git diff HEAD~1 packages/db/prisma/schema.prisma 2>/dev/null || git show HEAD:packages/db/prisma/schema.prisma 2>/dev/null | sed -n '30,80p'Repository: Dev-Pross/BuildFlow
Length of output: 1972
Generate and apply a Prisma migration for these schema changes.
No migration files were found in packages/db/prisma/migrations/. The schema changes (making config optional and adding the Position field) require a migration to update the database. Generate and commit the migration before deployment:
npx prisma migrate dev --name add_trigger_position_and_optional_config
Then verify the generated migration file in packages/db/prisma/migrations/*/migration.sql is committed.
Note: The naming inconsistency remains (Trigger.Position in PascalCase vs Node.position in camelCase). Consider standardizing to lowercase for consistency in a follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/prisma/schema.prisma` around lines 36 - 39, The Prisma schema
changes made the field config optional and added the Position field on the
Trigger model (with workflowId and AvailableTriggerID present); generate and
apply a migration to update the DB by running: npx prisma migrate dev --name
add_trigger_position_and_optional_config, verify the generated SQL in
packages/db/prisma/migrations/*/migration.sql and commit that migration file,
then run prisma db push or the migrate apply flow to ensure the database
reflects the changes; optionally plan a follow-up to standardize the Position
identifier (e.g., rename to position) for consistency with Node.position.
| static async register(){ | ||
| await NodeRegistry.register(this.definition) | ||
| // console.log(`✅ Registered node: ${this.definition.name}`); | ||
| await NodeRegistry.registerTrigger(this.definition) | ||
| // await NodeRegistry.registerTrigger(this.definition) |
There was a problem hiding this comment.
This drops Gmail from the available trigger catalog.
Without NodeRegistry.registerTrigger(this.definition), the gmail trigger is never persisted during startup. The backend later serves available triggers from availableTrigger, so users will no longer be able to create Gmail-triggered workflows unless another registration path was added.
Suggested fix
static async register(){
await NodeRegistry.register(this.definition)
- // await NodeRegistry.registerTrigger(this.definition)
+ await NodeRegistry.registerTrigger(this.definition)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nodes/src/gmail/gmail.node.ts` around lines 33 - 36, The static
async register method currently only calls
NodeRegistry.register(this.definition) which prevents the gmail trigger from
being persisted; restore the trigger registration by calling and awaiting
NodeRegistry.registerTrigger(this.definition) in static async register (i.e.,
re-enable the commented line so both NodeRegistry.register(this.definition) and
await NodeRegistry.registerTrigger(this.definition) are invoked using
this.definition).
| static async register(){ | ||
| await NodeRegistry.register(this.definition) | ||
| // console.log(`✅ Registered node: ${this.definition.name}`); | ||
| await NodeRegistry.registerTrigger(this.definition) | ||
| // await NodeRegistry.registerTrigger(this.definition) |
There was a problem hiding this comment.
Re-enable trigger registration or move it to another startup path.
Commenting this out stops google_sheet from being upserted into availableTrigger. NodeRegistry.registerAll() only registers webhook explicitly, and /getAvailableTriggers reads from that table, so this trigger will disappear from the frontend after server start.
Suggested fix
static async register(){
await NodeRegistry.register(this.definition)
- // await NodeRegistry.registerTrigger(this.definition)
+ await NodeRegistry.registerTrigger(this.definition)
}📝 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.
| static async register(){ | |
| await NodeRegistry.register(this.definition) | |
| // console.log(`✅ Registered node: ${this.definition.name}`); | |
| await NodeRegistry.registerTrigger(this.definition) | |
| // await NodeRegistry.registerTrigger(this.definition) | |
| static async register(){ | |
| await NodeRegistry.register(this.definition) | |
| // console.log(`✅ Registered node: ${this.definition.name}`); | |
| await NodeRegistry.registerTrigger(this.definition) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nodes/src/google-sheets/google-sheets.node.ts` around lines 34 - 37,
The trigger registration for the Google Sheets node was commented out so its
trigger ("google_sheet") isn't upserted into availableTrigger; restore or
relocate that call so triggers are registered during startup. Re-enable the call
to NodeRegistry.registerTrigger in the static async register method of the
GoogleSheets node (or move equivalent logic to the global startup path where
NodeRegistry.registerAll runs) so that
NodeRegistry.registerTrigger(this.definition) executes and the
/getAvailableTriggers endpoint can read the google_sheet entry from the
availableTrigger table.
Vamsi-o
left a comment
Vamsi-o
left a comment
There was a problem hiding this comment.
Critical: Route change breaks OAuth callback flow—update GOOGLE_REDIRECT_URI environment variable.
The Google OAuth route was moved from /oauth/google to /auth/google, but the default GOOGLE_REDIRECT_URI in google_callback.ts (line 22) still points to http://localhost:3002/oauth/google/callback. This causes a mismatch: Google will attempt to redirect to /oauth/google/callback, but the server callback endpoint is now at /auth/google/callback.
Update the GOOGLE_REDIRECT_URI environment variable to http://localhost:3002/auth/google/callback and ensure it matches the redirect URI registered in your Google Cloud Console OAuth application settings.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor