Conversation
WalkthroughThis pull request introduces action node support to the workflow builder. It adds new React components for rendering and selecting action nodes, a custom hook for fetching available actions from the backend, updates the CreateWorkFlow component to handle action nodes alongside triggers, fixes an import path for NodeRegistry, expands CORS origins, and updates build metadata with renamed file paths. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CWF as CreateWorkFlow
participant ASB as ActionSidebar
participant Hook as useActions Hook
participant API as Backend API
participant UI as Action Selection UI
User->>CWF: Click placeholder node
alt Trigger exists
CWF->>ASB: Open ActionSidebar (isOpen=true)
else No trigger
CWF->>ASB: Open ActionSidebar (isOpen=true)
end
ASB->>Hook: useActions({shouldFetch: true})
Hook->>API: GET /user/getAvailableNodes
API-->>Hook: Return actions array
Hook-->>ASB: actions, loading, error state
ASB->>UI: Render Select dropdown with actions
User->>UI: Select action from dropdown
UI->>ASB: onSelectAction callback
ASB->>CWF: onSelectAction(id, name, type, icon)
CWF->>CWF: handleSelectAction: create node + edges
CWF->>CWF: Update nodes, edges, and positions
CWF->>UI: Re-render with new ActionNode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
apps/http-backend/src/index.ts (1)
14-18: CORS config: consider env-driven origins + explicit handling for missingOriginheader.
Hardcoding localhost ports is fine for dev, but it’s easy to accidentally ship. Also,corscan reject requests that have noOrigin(some tools/health checks) depending on how it’s used. Consider a callback that (a) reads allowed origins from env, and (b) allows!originwhen appropriate.-const allowedOrigins = ['http://localhost:3000' , 'http://localhost:3001' ]; -app.use(cors({ - origin: allowedOrigins, - credentials: true, -})); +const allowedOrigins = (process.env.CORS_ORIGINS ?? "http://localhost:3000,http://localhost:3001") + .split(",") + .map(s => s.trim()) + .filter(Boolean); + +app.use(cors({ + origin(origin, cb) { + if (!origin) return cb(null, true); // decide if you want to allow non-browser calls + return cb(null, allowedOrigins.includes(origin)); + }, + credentials: true, +}));apps/web/app/components/Actions/ActionNode.tsx (1)
3-24: Avoid renderingundefinedfor action title/subtitle (fallback tolabel).
data.name/data.typeare optional but rendered directly; consider falling back todata.labeland hiding the subtitle when missing.export const ActionNode = ({ data }: ActionNodeProps) => { return ( <div className="flex flex-col items-center justify-center p-4 bg-gray-800 border-2 border-blue-500 rounded-lg min-w-[150px]"> <Handle type="target" position={Position.Left} /> <span className="text-3xl mb-2">{data.icon || "⚙️"}</span> - <span className="text-white font-semibold">{data.name}</span> - <span className="text-gray-400 text-sm">{data.type}</span> + <span className="text-white font-semibold">{data.name ?? data.label}</span> + {data.type ? <span className="text-gray-400 text-sm">{data.type}</span> : null} <Handle type="source" position={Position.Right} /> </div> ); };apps/web/app/types/workflow.types.ts (1)
24-30: Config type change looks right; consider aligning node config types too.
AvailabeAction.config: Record<string, unknown>is a better fit thanJSON, but yourNodeType.data.configstill appears asJSON(same file). Consider making them consistent to avoid casts downstream.apps/web/app/components/Actions/ActionSidebar.tsx (2)
22-26: Normalize response inuseActions(and removeanyhere).
getAvailableActionsis compensating for multiple payload shapes; that’s better handled in the hook so UI can stay strongly typed (AvailabeAction[]).Also applies to: 28-31
59-70: Prefer a disabledSelectItemfor “No actions available” (avoid raw<div>).
Some Select implementations expect only items and may mis-handle focus/keyboard navigation with arbitrary children.apps/web/app/components/nodes/CreateWorkFlow.tsx (2)
106-175: Brittle placeholder/edge targeting + unused variable.
previousNodeId(Line 122-124) is computed but unused—please remove.- Rewiring uses
target.startsWith("placeholder"); if you ever end up with >1 placeholder, this can rewire the wrong edge. Consider tracking the “active placeholder id” fromonNodeClickand using that id explicitly.
48-53: Nice: nodeTypes extended cleanly; consider memoizingnodeTypes.
Not required, butuseMemocan avoid recreating the object each render (helps avoid subtle ReactFlow re-init behaviors).Also applies to: 179-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/http-backend/src/index.ts(1 hunks)apps/http-backend/tsconfig.tsbuildinfo(1 hunks)apps/web/app/components/Actions/ActionNode.tsx(1 hunks)apps/web/app/components/Actions/ActionSidebar.tsx(1 hunks)apps/web/app/components/nodes/CreateWorkFlow.tsx(1 hunks)apps/web/app/hooks/useActions.tsx(1 hunks)apps/web/app/page.tsx(2 hunks)apps/web/app/types/workflow.types.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/app/components/nodes/CreateWorkFlow.tsx (5)
apps/web/app/types/workflow.types.ts (1)
NodeType(1-11)apps/web/app/components/nodes/TriggerNode.tsx (1)
TriggerNode(12-21)apps/web/app/components/Actions/ActionNode.tsx (1)
ActionNode(12-24)apps/web/app/components/nodes/TriggerSidebar.tsx (1)
TriggerSideBar(22-66)apps/web/app/components/Actions/ActionSidebar.tsx (1)
ActionSideBar(28-77)
apps/web/app/hooks/useActions.tsx (2)
apps/web/app/types/workflow.types.ts (1)
AvailabeAction(24-30)packages/common/src/index.ts (1)
BACKEND_URL(4-4)
🔇 Additional comments (5)
apps/web/app/page.tsx (1)
6-6: Formatting normalization looks good.The whitespace and trailing space removals are minor cleanup with no functional impact.
Also applies to: 9-9
apps/http-backend/src/index.ts (1)
7-7: Import path fix looks right; verify the workspace export/path actually exists.
This should resolve the broken import, but it’s worth confirming@repo/nodes/nodeClientis the correct published/tsconfig-path entry and that it exportsNodeRegistry.package.json (1)
18-21: pnpm version bump: ensure CI/dev are aligned (Corepack).
If CI or contributors rely oncorepack, confirm it’s enabled/pinned so the repo consistently usespnpm@10.25.0(and lockfile behavior doesn’t drift).apps/web/app/components/Actions/ActionNode.tsx (1)
1-2: Double-check Handle requirements/ids for @xyflow/react v12.9.3.
If you ever add multiple handles per node (or custom connection logic), you’ll likely want explicitids and/orisConnectable/validation.Also applies to: 15-21
apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
54-104: Trigger selection currently resets the graph; confirm that’s intended UX.
handleSelectTriggerreturns[triggerNode, newPlaceholder]and replacesedgeswith a single edge—this will wipe any existing action chain if users can re-pick a trigger.
| export const useActions = ({ shouldFetch }: userActionsPrps) => { | ||
| const [actions, setActions] = useState<AvailabeAction[]>([]); | ||
| const [loading, setLoading] = useState<boolean>(false); | ||
| const [error, setError] = useState(""); | ||
|
|
||
| useEffect(() => { | ||
| if (!shouldFetch) return; | ||
| const fetchData = async () => { | ||
| try { | ||
| setLoading(true); | ||
| const Data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | ||
| withCredentials: true, | ||
| }); | ||
| console.log(JSON.stringify(Data.data)); | ||
| setActions(Data.data); | ||
| setLoading(false); | ||
| } catch (e: any) { | ||
| setError(e); | ||
| console.log("Error while fetching Data from Triggers", e); | ||
| } | ||
| }; | ||
| fetchData(); | ||
| }, [shouldFetch]); | ||
|
|
||
| return { actions, loading, error }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "useActions.tsx" -o -name "useActions.ts" | head -20Repository: Dev-Pross/BuildFlow
Length of output: 98
🏁 Script executed:
cat -n ./apps/web/app/hooks/useActions.tsxRepository: Dev-Pross/BuildFlow
Length of output: 1327
🏁 Script executed:
rg "useActions" --type tsx --type ts -A 3 -B 1 | head -100Repository: Dev-Pross/BuildFlow
Length of output: 90
🏁 Script executed:
rg "useActions" -A 3 -B 1 | head -150Repository: Dev-Pross/BuildFlow
Length of output: 1338
🏁 Script executed:
cat -n ./apps/web/app/components/Actions/ActionSidebar.tsx | head -80Repository: Dev-Pross/BuildFlow
Length of output: 3083
🏁 Script executed:
rg "AvailabeAction" -A 5 -B 2 | head -50Repository: Dev-Pross/BuildFlow
Length of output: 1456
🏁 Script executed:
rg "setError" -B 5 -A 2 apps/web/app/Repository: Dev-Pross/BuildFlow
Length of output: 6690
Fix loading/error state handling and normalize API response shape in the hook.
The loading state remains true on fetch errors since setLoading(false) is only called in the success path. The error state is typed string but receives the entire error object. Additionally, the hook doesn't normalize the API response shape (which can be either an array or {Data: array}), leaving this responsibility to consumers like ActionSideBar via getAvailableActions(). Move normalization into the hook for cleaner separation of concerns, and use a finally block to ensure proper cleanup.
export const useActions = ({ shouldFetch }: userActionsPrps) => {
const [actions, setActions] = useState<AvailabeAction[]>([]);
const [loading, setLoading] = useState<boolean>(false);
const [error, setError] = useState("");
useEffect(() => {
if (!shouldFetch) return;
const fetchData = async () => {
try {
setLoading(true);
- const Data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, {
+ setError("");
+ const resp = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, {
withCredentials: true,
});
- console.log(JSON.stringify(Data.data));
- setActions(Data.data);
- setLoading(false);
+ const data = resp.data;
+ const normalized: AvailabeAction[] = Array.isArray(data)
+ ? data
+ : Array.isArray(data?.Data)
+ ? data.Data
+ : [];
+ setActions(normalized);
} catch (e: any) {
- setError(e);
- console.log("Error while fetching Data from Triggers", e);
+ setError(e?.message ?? "Failed to fetch actions");
+ setActions([]);
+ console.log("Error while fetching actions", e);
+ } finally {
+ setLoading(false);
}
};
fetchData();
}, [shouldFetch]);
return { actions, loading, error };
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useActions = ({ shouldFetch }: userActionsPrps) => { | |
| const [actions, setActions] = useState<AvailabeAction[]>([]); | |
| const [loading, setLoading] = useState<boolean>(false); | |
| const [error, setError] = useState(""); | |
| useEffect(() => { | |
| if (!shouldFetch) return; | |
| const fetchData = async () => { | |
| try { | |
| setLoading(true); | |
| const Data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | |
| withCredentials: true, | |
| }); | |
| console.log(JSON.stringify(Data.data)); | |
| setActions(Data.data); | |
| setLoading(false); | |
| } catch (e: any) { | |
| setError(e); | |
| console.log("Error while fetching Data from Triggers", e); | |
| } | |
| }; | |
| fetchData(); | |
| }, [shouldFetch]); | |
| return { actions, loading, error }; | |
| }; | |
| export const useActions = ({ shouldFetch }: userActionsPrps) => { | |
| const [actions, setActions] = useState<AvailabeAction[]>([]); | |
| const [loading, setLoading] = useState<boolean>(false); | |
| const [error, setError] = useState(""); | |
| useEffect(() => { | |
| if (!shouldFetch) return; | |
| const fetchData = async () => { | |
| try { | |
| setLoading(true); | |
| setError(""); | |
| const resp = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | |
| withCredentials: true, | |
| }); | |
| const data = resp.data; | |
| const normalized: AvailabeAction[] = Array.isArray(data) | |
| ? data | |
| : Array.isArray(data?.Data) | |
| ? data.Data | |
| : []; | |
| setActions(normalized); | |
| } catch (e: any) { | |
| setError(e?.message ?? "Failed to fetch actions"); | |
| setActions([]); | |
| console.log("Error while fetching actions", e); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| fetchData(); | |
| }, [shouldFetch]); | |
| return { actions, loading, error }; | |
| }; |
🤖 Prompt for AI Agents
In apps/web/app/hooks/useActions.tsx around lines 9 to 34, the hook leaves
loading true on errors, stores the entire error object into a string-typed error
state, and pushes response-shape normalization to consumers; fix by wrapping the
fetch in try/catch/finally and call setLoading(false) in the finally block,
setError to a string (e.g. e?.message || String(e)) in the catch, and normalize
the response before setting actions (if response.data is an array use it, else
if response.data.Data use that array) so consumers always receive an array of
AvailabeAction; update any local typings if needed to reflect error as string
and actions as AvailabeAction[].
There was a problem hiding this comment.
Pull request overview
This PR implements the functionality to select and add action nodes to a workflow, building upon the existing trigger node selection feature. The changes enable users to add action nodes after triggers in a visual workflow builder using React Flow.
Key Changes:
- Added action node support to the workflow builder with corresponding UI components (ActionNode, ActionSidebar)
- Created a new custom hook
useActionsto fetch available actions from the backend - Updated type definitions to use proper TypeScript types instead of JSON type
- Fixed import path spelling error in the backend
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated pnpm package manager version from 10.24.0 to 10.25.0 |
| apps/web/app/types/workflow.types.ts | Changed config type from JSON to Record<string, unknown> for better type safety |
| apps/web/app/page.tsx | Removed trailing whitespace and empty lines (formatting cleanup) |
| apps/web/app/hooks/useActions.tsx | New custom hook to fetch available actions from backend API |
| apps/web/app/components/nodes/CreateWorkFlow.tsx | Added action node selection logic, conditional sidebar handling, and action node type support |
| apps/web/app/components/Actions/ActionSidebar.tsx | New sidebar component for selecting actions from a dropdown list |
| apps/web/app/components/Actions/ActionNode.tsx | New React Flow node component to display action nodes in the workflow |
| apps/http-backend/tsconfig.tsbuildinfo | Updated build info to reflect corrected import paths |
| apps/http-backend/src/index.ts | Fixed spelling error in import path (nodeClinet → nodeClient) and added localhost:3001 to CORS allowed origins |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const edgeToPlaceholder = currentEdges.find((e) => | ||
| e.target.startsWith("placeholder") | ||
| ); |
There was a problem hiding this comment.
The logic to find the edge to update is fragile. It finds any edge where the target starts with "placeholder", but there could be multiple placeholder nodes. This should specifically match the clicked placeholder node's ID instead. Consider using e.target === placeholderNode.id where placeholderNode is captured in the closure.
| interface userActionsPrps { | ||
| shouldFetch: boolean; | ||
| } | ||
| export const useActions = ({ shouldFetch }: userActionsPrps) => { |
There was a problem hiding this comment.
The interface name "userActionsPrps" contains a spelling error. It should be "userActionsProps" (with "Props" correctly spelled).
| interface userActionsPrps { | |
| shouldFetch: boolean; | |
| } | |
| export const useActions = ({ shouldFetch }: userActionsPrps) => { | |
| interface userActionsProps { | |
| shouldFetch: boolean; | |
| } | |
| export const useActions = ({ shouldFetch }: userActionsProps) => { |
| const Data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | ||
| withCredentials: true, | ||
| }); | ||
| console.log(JSON.stringify(Data.data)); | ||
| setActions(Data.data); |
There was a problem hiding this comment.
The variable name "Data" uses PascalCase, which is unconventional for a local variable. It should be lowercase "data" to follow standard JavaScript/TypeScript naming conventions.
| const Data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | |
| withCredentials: true, | |
| }); | |
| console.log(JSON.stringify(Data.data)); | |
| setActions(Data.data); | |
| const data = await axios.get(`${BACKEND_URL}/user/getAvailableNodes`, { | |
| withCredentials: true, | |
| }); | |
| console.log(JSON.stringify(data.data)); | |
| setActions(data.data); |
| onSelectAction: (action: { id: string; name: string; type: string; icon?: string }) => void; | ||
| } | ||
|
|
||
| const getAvailableActions = (actions: any): Array<any> => { |
There was a problem hiding this comment.
Using 'any' type defeats the purpose of TypeScript's type safety. Since the 'AvailabeAction' type is already defined in workflow.types.ts, use that type instead of 'any' for proper type checking.
| </SelectTrigger> | ||
| <SelectContent> | ||
| {availableActions.length ? ( | ||
| availableActions.map((action: any) => ( |
There was a problem hiding this comment.
Using 'any' type defeats the purpose of TypeScript's type safety. Since the 'AvailabeAction' type is already defined in workflow.types.ts, use that type instead of 'any' for proper type checking.
| setActions(Data.data); | ||
| setLoading(false); | ||
| } catch (e: any) { | ||
| setError(e); |
There was a problem hiding this comment.
Setting the error state to the exception object directly is problematic. The error state is typed as a string, but you're setting it to an error object. This should extract the error message using e.message or convert it to a string.
| setError(e); | |
| setError(e && typeof e === "object" && "message" in e ? String(e.message) : String(e)); |
| setLoading(false); | ||
| } catch (e: any) { | ||
| setError(e); | ||
| console.log("Error while fetching Data from Triggers", e); |
There was a problem hiding this comment.
The loading state is set to true but never reset to false in the error case. This will cause the loading indicator to remain active indefinitely if an error occurs. Add setLoading(false) in the catch block or use a finally block.
| setLoading(false); | |
| } catch (e: any) { | |
| setError(e); | |
| console.log("Error while fetching Data from Triggers", e); | |
| } catch (e: any) { | |
| setError(e); | |
| console.log("Error while fetching Data from Triggers", e); | |
| } finally { | |
| setLoading(false); |
| <p className="text-red-500">Error fetching actions</p> | ||
| ) : ( | ||
| <Select onValueChange={(value) => { | ||
| const selected = availableActions.find((a: any) => String(a.id) === value); |
There was a problem hiding this comment.
Using 'any' type defeats the purpose of TypeScript's type safety. Since the 'AvailabeAction' type is already defined in workflow.types.ts, use that type instead of 'any' for proper type checking.
| id: selected.id, | ||
| name: selected.name, | ||
| type: selected.type, | ||
| icon: 'icon' in selected && selected.icon ? selected.icon : '⚡', |
There was a problem hiding this comment.
The default icon '⚡' (lightning bolt) is inconsistent with the action context. In ActionNode.tsx and handleSelectAction, the default icon is '⚙️' (gear). This inconsistency should be resolved by using the same default icon throughout.
| const previousNodeId = edges.find( | ||
| (e) => e.target === placeholderNode.id | ||
| )?.source; | ||
|
|
There was a problem hiding this comment.
Unused variable previousNodeId.
| const previousNodeId = edges.find( | |
| (e) => e.target === placeholderNode.id | |
| )?.source; |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.