feat: Real-time multi-user collaboration and presence via WebSockets - #53
feat: Real-time multi-user collaboration and presence via WebSockets#53Dev1822 wants to merge 2 commits into
Conversation
|
@Dev1822 is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
✅ Thanks! This PR is linked to #47 and will close it automatically when merged. |
📝 WalkthroughWalkthroughAdds Socket.IO-based workspace synchronization for flowchart blocks, debounced client updates, and collaborative presence indicators for remote cursors and node selections. ChangesReal-time collaboration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CenterCanvas
participant App
participant SocketIO
participant Server
User->>CenterCanvas: Move cursor or select node
CenterCanvas->>SocketIO: Emit cursor-move or node-select
SocketIO->>Server: Relay presence event
Server-->>SocketIO: Broadcast cursor-update or selection-update
SocketIO-->>CenterCanvas: Render remote presence
User->>App: Add, update, delete, or clear block
App->>SocketIO: Emit block mutation
SocketIO->>Server: Update workspace state
Server-->>SocketIO: Broadcast synchronized block event
SocketIO-->>App: Apply remote workspace state
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.tsx (1)
138-177: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMove Socket.IO emits out of the state updaters. React StrictMode is enabled here, so these updater callbacks can run more than once in development and duplicate
add-block/update-blockevents on the server. Compute the block mutations first, then update state and emit once outside the updater; the same pattern in the delete flow has the same risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 138 - 177, Refactor the block-add flow around the setBlocks updater so it computes the updated blocks, newBlock, and modifiedActiveBlock without performing side effects inside the callback, then emit add-block and update-block exactly once after state preparation and outside setBlocks. Apply the same change to the delete flow: move its Socket.IO emits out of the state updater to prevent duplicate events under React StrictMode.
🧹 Nitpick comments (1)
package.json (1)
27-37: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
corsas a direct dependency.server.jsimports it, butpackage.jsononly gets it transitively viasocket.io. Declaring it here avoids relying on an implicit dependency chain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 27 - 37, Add cors as an explicit direct dependency in package.json alongside the other runtime dependencies, since server.js imports it directly. Keep the existing socket.io and other dependency declarations unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server.js`:
- Around line 10-15: Update the Server configuration to replace the wildcard
CORS origin with the application’s trusted-origin allowlist, and add connection
authorization that authenticates the client and validates membership in the
requested workspace before permitting socket.join(). Reject unauthorized
origins, unauthenticated clients, and non-members without joining the workspace.
- Around line 65-75: Update the `update-block` handler to require an existing
block and validate the incoming block’s revision/base version against the stored
block before applying it. Reject stale, mismatched, or unknown block IDs without
replacing or pushing them, and emit `block-updated` only after a valid
revisioned update.
- Around line 105-110: Update the full-sync handler to accept payloads only
during workspace initialization, reject subsequent sync attempts, and validate
that blocks is an array whose entries match the complete block schema before
assigning roomState.blocks or broadcasting full-sync-update. Preserve the
existing currentRoom and roomState checks while rejecting invalid payloads
without mutating workspace state.
In `@src/App.tsx`:
- Around line 62-64: Update emitUpdateDebounced and its corresponding usage
around the later update path so debounce state is maintained independently for
each block.id. Ensure editing one block does not cancel a pending update for
another block, while preserving the 300 ms delay and existing
socket.emit('update-block', block) behavior.
- Around line 76-77: Update the socket setup in App.tsx so the join-workspace
emit and initial-state synchronization run from a connect handler, including the
initial connection and every subsequent Socket.IO reconnect. Ensure the handler
is registered before or alongside socket.connect and preserves the existing wsId
and initialState processing.
- Around line 78-82: The initialization branch in the App initialization flow
must distinguish an explicitly empty workspace from an uninitialized room. Use
an explicit initialization flag in the initial state, or move default-demo
initialization exclusively into the server, so `blocks: []` restores the empty
workspace rather than emitting `full-sync` with `initialDemoBlocks`.
In `@src/components/CenterCanvas.tsx`:
- Around line 804-831: Move the remote cursor rendering block identified by the
“Render Remote Cursors” comment outside the blocks.length !== 0 conditional in
CenterCanvas, while preserving its existing positioning, styling, and
remoteCursors mapping so cursors remain visible when the workspace is empty or
newly cleared.
- Around line 115-117: Update the selection effect in CenterCanvas so
node-select is emitted only after join-workspace has completed, preserving the
initial selection instead of sending it before currentRoom is set. Also move or
duplicate remote-cursor rendering outside the non-empty-canvas branch so
collaborators remain visible when no blocks exist.
---
Outside diff comments:
In `@src/App.tsx`:
- Around line 138-177: Refactor the block-add flow around the setBlocks updater
so it computes the updated blocks, newBlock, and modifiedActiveBlock without
performing side effects inside the callback, then emit add-block and
update-block exactly once after state preparation and outside setBlocks. Apply
the same change to the delete flow: move its Socket.IO emits out of the state
updater to prevent duplicate events under React StrictMode.
---
Nitpick comments:
In `@package.json`:
- Around line 27-37: Add cors as an explicit direct dependency in package.json
alongside the other runtime dependencies, since server.js imports it directly.
Keep the existing socket.io and other dependency declarations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fdd28ee3-915a-45cf-862d-5e9a7792552f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonserver.jssrc/App.tsxsrc/components/CenterCanvas.tsxsrc/utils/socket.ts
| const io = new Server(httpServer, { | ||
| cors: { | ||
| origin: "*", | ||
| methods: ["GET", "POST"] | ||
| } | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Restrict origins and authorize workspace membership.
With origin: "*", any website can connect and mutate a known workspace ID; no authentication or room authorization is performed. Configure trusted origins and validate access before socket.join().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server.js` around lines 10 - 15, Update the Server configuration to replace
the wildcard CORS origin with the application’s trusted-origin allowlist, and
add connection authorization that authenticates the client and validates
membership in the requested workspace before permitting socket.join(). Reject
unauthorized origins, unauthenticated clients, and non-members without joining
the workspace.
| socket.on('update-block', (block) => { | ||
| if (!currentRoom) return; | ||
| const roomState = workspaces.get(currentRoom); | ||
| if (roomState) { | ||
| const idx = roomState.blocks.findIndex(b => b.id === block.id); | ||
| if (idx !== -1) { | ||
| roomState.blocks[idx] = block; | ||
| } else { | ||
| roomState.blocks.push(block); | ||
| } | ||
| socket.to(currentRoom).emit('block-updated', block); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Require an existing block and matching revision for updates.
Whole-block last-write-wins replacement loses concurrent changes. The upsert branch also lets a delayed update recreate a deleted block, while src/App.tsx ignores updates for unknown IDs, leaving server and clients divergent.
Include a revision/base version and reject stale or missing block updates rather than pushing them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server.js` around lines 65 - 75, Update the `update-block` handler to require
an existing block and validate the incoming block’s revision/base version
against the stored block before applying it. Reject stale, mismatched, or
unknown block IDs without replacing or pushing them, and emit `block-updated`
only after a valid revisioned update.
| const emitUpdateDebounced = useRef( | ||
| debounce((block: Block) => socket.emit('update-block', block), 300) | ||
| ).current; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Debounce updates independently per block.
This single timer cancels an earlier update when another block is edited within 300 ms, leaving the server with stale state. Key pending timers by block.id, or debounce at the editor instance level.
Also applies to: 189-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 62 - 64, Update emitUpdateDebounced and its
corresponding usage around the later update path so debounce state is maintained
independently for each block.id. Ensure editing one block does not cancel a
pending update for another block, while preserving the 300 ms delay and existing
socket.emit('update-block', block) behavior.
| socket.connect(); | ||
| socket.emit('join-workspace', wsId, (initialState: { blocks: Block[] }) => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Rejoin the workspace after every Socket.IO reconnect.
join-workspace runs only once. After a network reconnect, the server creates a new socket with currentRoom === null, so subsequent block and presence events are silently ignored. Register a connect handler that rejoins and resynchronizes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 76 - 77, Update the socket setup in App.tsx so the
join-workspace emit and initial-state synchronization run from a connect
handler, including the initial connection and every subsequent Socket.IO
reconnect. Ensure the handler is registered before or alongside socket.connect
and preserves the existing wsId and initialState processing.
| if (initialState && initialState.blocks.length > 0) { | ||
| setBlocks(initialState.blocks); | ||
| } else { | ||
| socket.emit('full-sync', initialDemoBlocks); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not treat an empty workspace as uninitialized.
A legitimately cleared room also returns blocks: [], causing every later join to upload initialDemoBlocks and undo the clear. Return an explicit initialization flag or initialize the room exclusively on the server.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 78 - 82, The initialization branch in the App
initialization flow must distinguish an explicitly empty workspace from an
uninitialized room. Use an explicit initialization flag in the initial state, or
move default-demo initialization exclusively into the server, so `blocks: []`
restores the empty workspace rather than emitting `full-sync` with
`initialDemoBlocks`.
| useEffect(() => { | ||
| socket.emit('node-select', { selectedId: selectedBlockId }); | ||
| }, [selectedBlockId]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FILES\n'
git ls-files 'src/components/CenterCanvas.tsx' '*CenterCanvas*' '*workspace*' '*socket*' | sed -n '1,200p'
printf '\nOUTLINE CenterCanvas\n'
ast-grep outline src/components/CenterCanvas.tsx --view expanded || true
printf '\nRELEVANT SEARCHES\n'
rg -n "join-workspace|node-select|selectedBlockId|socket.emit|currentRoom|remote cursor|cursor" src/components/CenterCanvas.tsx src -g '!**/node_modules/**' || trueRepository: itzzavdhesh/FlowCraft
Length of output: 12695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'OUTLINE App\n'
ast-grep outline src/App.tsx --view expanded || true
printf '\nOUTLINE socket util\n'
ast-grep outline src/utils/socket.ts --view expanded || true
printf '\nAPP RELEVANT LINES\n'
sed -n '60,95p' src/App.tsx
printf '\nSOCKET UTIL\n'
cat -n src/utils/socket.tsRepository: itzzavdhesh/FlowCraft
Length of output: 2833
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'SEARCH SERVER JOIN/CURRENTROOM\n'
rg -n "currentRoom|join-workspace|node-select|cursor-update|cursor-move|full-sync|selectedId" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
printf '\nCENTERCANVAS RENDER LINES\n'
sed -n '620,770p' src/components/CenterCanvas.tsxRepository: itzzavdhesh/FlowCraft
Length of output: 10372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'CENTERCANVAS STATE/EFFECTS\n'
sed -n '60,130p' src/components/CenterCanvas.tsx
printf '\nCENTERCANVAS EMPTY-STATE / CURSOR RENDER\n'
sed -n '500,640p' src/components/CenterCanvas.tsxRepository: itzzavdhesh/FlowCraft
Length of output: 8814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'CENTER CANVAS WITH LINE NUMBERS\n'
nl -ba src/components/CenterCanvas.tsx | sed -n '500,640p'
printf '\nREMOTE CURSOR RENDER SEARCH\n'
rg -n "remoteCursors|remoteSelections|userColors" src/components/CenterCanvas.tsxRepository: itzzavdhesh/FlowCraft
Length of output: 231
🌐 Web query:
React useEffect mount order parent child passive effects Socket.IO client buffers emits before connect ordering preserve order
💡 Result:
In React, the useEffect hooks in child components are committed and executed before the useEffect hooks of their parent components [1][2]. This occurs because React traverses the fiber tree during the commit phase in a depth-first manner, effectively processing the subtree of children before completing the commit for the parent [2][3]. Regarding Socket.IO client behavior, the library is designed to preserve the chronological order of events [4]. When a client is disconnected, calls to emit are buffered into a sendBuffer [5][6]. Upon reconnection, the Socket.IO client explicitly flushes the sendBuffer before emitting the internal connect event [4][7][6]. This ensures that events buffered while the client was offline are sent to the server before any new events triggered within a connect event handler [4][7]. To summarize the order of operations: 1. React Lifecycle: Child useEffects execute before Parent useEffects [1][8]. 2. Socket.IO Reconnection: Buffered events (from the sendBuffer) are sent before the connect event handler executes [7][6]. If you rely on specific ordering between these two systems (e.g., a component effect performing an emit), be aware that the child effect will run first, and if that effect triggers an emit while the socket is offline, it will be added to the sendBuffer and sent in the correct order once connectivity is restored [4][6].
Citations:
- 1: [Suggestion]: Adding documentation to note that Effects in children components are committed before their parent component reactjs/react.dev#7748
- 2: https://master.dev/blog/react-internals-which-useeffect-runs-first/
- 3: https://stackoverflow.com/questions/76768261/why-dose-reacts-child-components-useeffect-runs-before-its-parents-useeffect
- 4: socketio/socket.io-client@991eb0b
- 5: https://socket.io/docs/v4/client-offline-behavior/
- 6: https://github.com/socketio/socket.io-client/blob/master/lib/socket.ts
- 7:
sendBuffershould be emptied beforeemitsocketio/socket.io-client#1458 - 8: https://stackoverflow.com/questions/58352375/what-is-the-correct-order-of-execution-of-useeffect-in-react-parent-and-child-co
🌐 Web query:
React useEffect mount order parent child passive effects on mount
💡 Result:
In React, passive effects (useEffect) are executed in a child-first order during the mounting process [1][2]. Specifically, when a component tree is mounted, the effects of child components are committed and executed before the effects of their parent components [1][2]. This behavior occurs because of how React traverses the Fiber tree during the commit phase [1][2]. React performs a depth-first traversal of the component tree to flush effects, meaning it moves down to the leaves (children) of the tree and invokes their passive effects before moving back up to the parent [1][2]. Key points regarding this execution order: 1. Child-First Order: If a parent component renders a child component, the child's useEffect will run before the parent's useEffect [1][3][4]. 2. Internal Mechanism: This is a result of the recursive traversal performed by functions such as recursivelyTraversePassiveMountEffects within React's internal commit phase logic [1][2]. 3. Comparison with useLayoutEffect: Unlike useEffect (which is a passive effect that runs asynchronously after paint), useLayoutEffect runs synchronously after DOM mutations but before the browser paints [5]. While useLayoutEffect also follows a child-first order, it executes earlier in the lifecycle than useEffect [3][6]. 4. Consistency: This behavior is consistent across mounting and updating phases, ensuring that dependencies and child-parent relationships are handled in a predictable order [7]. If you require logic to run in the parent before the child, you cannot achieve this by default using useEffect because of this structural traversal [1]. If such an ordering is strictly necessary for your application, it is often a sign that you should re-evaluate the component architecture or consider if the logic truly depends on the child having already "mounted" its effect [8][9].
Citations:
- 1: [Suggestion]: Adding documentation to note that Effects in children components are committed before their parent component reactjs/react.dev#7748
- 2: https://master.dev/blog/react-internals-which-useeffect-runs-first/
- 3: useEffect firing in children before parent react/react#15281
- 4: https://stackoverflow.com/questions/64502739/useeffect-of-children-component-called-before-useeffect-of-parent
- 5: https://deepwiki.com/facebook/react/2.1-fiber-work-loop-and-scheduling
- 6: https://stackoverflow.com/questions/58352375/what-is-the-correct-order-of-execution-of-useeffect-in-react-parent-and-child-co
- 7: Bug(17.0.0-rc.1): useEffect cleanup functions not running in the same order as effect functions react/react#19866
- 8: https://react.dev/reference/react/useEffect
- 9: https://react.dev/learn/lifecycle-of-reactive-effects
🌐 Web query:
Socket.IO client emit before connect buffered order preserved
💡 Result:
Yes, Socket.IO preserves the order of events emitted while the client is disconnected [1]. When a client is disconnected, any events emitted via socket.emit() are added to an internal sendBuffer [2][3]. Upon reconnection, the Socket.IO client automatically processes and transmits these buffered events in the order they were initially emitted [4][3]. Specifically, the client ensures that the sendBuffer is emptied and transmitted to the server before or as part of the reconnection process, ensuring that buffered events are sent before any new events emitted after the connection is re-established [4][3][5]. This behavior was explicitly reinforced in version 4.0.2 to prevent potential out-of-order issues where events emitted within a "connect" handler might have previously been sent before the buffered items [4][5]. Key points regarding this behavior: - Buffered Order: Events are stored in a queue (the sendBuffer) and sent in FIFO (First-In, First-Out) order [3]. - Delivery Guarantees: While Socket.IO guarantees that buffered events are sent in order upon reconnection, it does not guarantee that they will be successfully received by the server if the connection is dropped during transmission [1]. By default, Socket.IO provides an "at most once" delivery guarantee [1]. - Volatile Events: If you do not want events to be buffered while the client is offline, you can use socket.volatile.emit(). These events are dropped if the client is not connected at the time of emission [2][6].
Citations:
- 1: https://socket.io/docs/v4/delivery-guarantees
- 2: https://socket.io/docs/v4/client-offline-behavior/
- 3: https://github.com/socketio/socket.io-client/blob/master/lib/socket.ts
- 4: socketio/socket.io-client@991eb0b
- 5:
sendBuffershould be emptied beforeemitsocketio/socket.io-client#1458 - 6: https://socket.io/docs/v4/emitting-events/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=500 && NR<=760 { printf "%4d %s\n", NR, $0 }' src/components/CenterCanvas.tsx | sed -n '1,260p'Repository: itzzavdhesh/FlowCraft
Length of output: 13899
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'CENTER CANVAS REMOTE-RELATED MATCHES\n'
rg -n "remote|selection-update|cursor-update|cursor-move|user-left" src/components/CenterCanvas.tsx
printf '\nTAIL OF CENTER CANVAS\n'
awk 'NR>=760 && NR<=940 { printf "%4d %s\n", NR, $0 }' src/components/CenterCanvas.tsx | sed -n '1,220p'Repository: itzzavdhesh/FlowCraft
Length of output: 9531
Emit selection after joining the room, and keep remote cursors visible on empty canvases.
node-selectcan fire beforejoin-workspace, so the first selection is dropped whilecurrentRoomis still unset. Move the emit into the join callback or gate it on joined state.- Remote cursors are only rendered in the non-empty branch, so collaborators disappear when the canvas is empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/CenterCanvas.tsx` around lines 115 - 117, Update the selection
effect in CenterCanvas so node-select is emitted only after join-workspace has
completed, preserving the initial selection instead of sending it before
currentRoom is set. Also move or duplicate remote-cursor rendering outside the
non-empty-canvas branch so collaborators remain visible when no blocks exist.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
17 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/CenterCanvas.tsx">
<violation number="1" location="src/components/CenterCanvas.tsx:91">
P3: Repeated disconnects/reconnects leave stale entries in `userColors`, so this component's presence state grows for the lifetime of the canvas. Remove the departing user's color with its cursor and selection.</violation>
<violation number="2" location="src/components/CenterCanvas.tsx:116">
P2: Peers joining after a node is already selected do not see its remote border until that peer changes selection. Include presence/selection state in the workspace join snapshot, or request/rebroadcast current selections after joining.</violation>
<violation number="3" location="src/components/CenterCanvas.tsx:805">
P3: Remote cursors disappear for empty or newly cleared workspaces because this render is gated by `blocks.length !== 0`. Render the presence overlay outside the empty-state conditional so collaborators remain visible before a block exists.</violation>
</file>
<file name="server.js">
<violation number="1" location="server.js:4">
P0: Missing dependency: `cors` is imported and used (`import cors from 'cors'`) but not listed in `package.json` `dependencies` or `devDependencies`. The project will fail at runtime after a clean install because `npm install` won't install `cors`.
Add `"cors": "^2.8.5"` to `dependencies` in `package.json`.</violation>
<violation number="2" location="server.js:12">
P2: CORS is configured with `origin: "*"` (all origins allowed) on the Socket.IO server. While this is convenient during development, it permits any website to connect to the WebSocket server and read/emit events for any workspace ID. Since the server has no authentication or origin validation, this is a security concern — especially for a collaborative editor where workspace data flows over WebSocket.
Recommend either:
1. Restricting `origin` to the frontend's actual URL in production (e.g., `http://localhost:3000` in dev, configured via env var), OR
2. Adding a token/authentication handshake on the `connection` or `join-workspace` event.
At minimum, for dev this should be configured through an environment variable so it can be tightened before deployment.</violation>
<violation number="3" location="server.js:29">
P2: Switching workspaces leaves the old room showing this user's cursor and selected-node border indefinitely. Emit `user-left` to `currentRoom` before leaving it.</violation>
<violation number="4" location="server.js:32">
P1: Workspace joins are accepted for any supplied `workspaceId` without an access check, so anyone who can guess an ID can join that room and emit mutating events. Consider validating membership/authorization before `socket.join(workspaceId)` and rejecting unauthorized joins.</violation>
<violation number="5" location="server.js:60">
P2: The `update-block` handler silently falls through to `push()` when a block ID is not found, turning a failed update into an insert. This means if a client sends an update for a block that was already deleted by another user (or that never existed), a ghost block is created instead of the client receiving feedback that the block doesn't exist. This can cause desync'd state and confusing phantom blocks on the canvas.
Recommend either:
1. Log a warning and skip the push (maintains clear update-vs-insert semantics), OR
2. Emit a 'block-not-found' error event so the client can handle it.</violation>
<violation number="6" location="server.js:105">
P1: The `full-sync` handler (line 64) replaces the entire `blocks` array with whatever the client sends, bypassing the atomic operational model that this PR describes as a key design goal. Any client can overwrite all blocks regardless of whether they have the latest state, which can cause data loss in a race between a stale full-sync and concurrent add/update/delete operations. The PR description explicitly says "Atomic operational events for add/update/delete blocks (no full state replacement) to prevent clobbering" — yet this handler does exactly that.
Recommend either removing this handler entirely in favor of atomic operations, or adding server-side validation that reconciles rather than replaces.</violation>
<violation number="7" location="server.js:109">
P1: `full-sync` assigns unvalidated client data directly to `roomState.blocks`. If a non-array payload is sent, later handlers call array methods on invalid data and can throw. Validate the payload type/schema before storing or broadcasting it.</violation>
</file>
<file name="src/utils/socket.ts">
<violation number="1" location="src/utils/socket.ts:8">
P1: After a transient disconnect, this client reconnects but no longer belongs to its workspace, so subsequent edits/presence are dropped or not received. Rejoin the current workspace from the socket `connect` handler (which also runs after reconnect) and resync there.</violation>
</file>
<file name="src/App.tsx">
<violation number="1" location="src/App.tsx:63">
P2: This shared debounced emitter drops updates across different blocks: a second edit inside the debounce window cancels the first pending send. Debounce should be scoped per block ID (or per editor instance) so unrelated edits do not overwrite each other in-flight.</violation>
<violation number="2" location="src/App.tsx:81">
P1: Joining a workspace that was intentionally cleared repopulates it with the demo flow and broadcasts that restore to collaborators. Seed defaults only when the server creates a workspace, not whenever its current block list is empty.</violation>
<violation number="3" location="src/App.tsx:115">
P2: Debounced socket emit for block updates is never flushed before socket disconnect. The `useEffect` cleanup disconnects the socket (`socket.disconnect()`), but `emitUpdateDebounced` has no flush/cancel capability. If a user edits a label (triggering the 300ms debounce) and then immediately navigates away or closes the tab, the debounced update never fires — the last edit is silently lost for other collaborators.
Add a `flush` method to the debounce utility, or use a ref to call the debounced function directly with `flush` during cleanup. Alternatively, use an immediate (non-debounced) emit for important final-state updates and reserve debouncing only for transient/in-flight events.</violation>
<violation number="4" location="src/App.tsx:172">
P1: Socket emits inside `setBlocks` state updater cause duplicate sends in React Strict Mode. In `handleAddBlock`, both `socket.emit('add-block', newBlock)` and `socket.emit('update-block', modifiedActiveBlock)` are called directly inside the `setBlocks(prev => { ... })` callback. Similarly in `handleDeleteBlock`, `socket.emit('update-block', next)` is called inside the `.map()` within the `setBlocks` updater. React 18+ calls state updater functions twice in development Strict Mode to detect impure side effects — this means each emit fires twice, flooding the server with duplicate events and causing unnecessary network traffic or even duplicate block processing on the receiver side.
Move all socket emits outside the `setBlocks` callback. Compute the new state first, then emit after the updater call.</violation>
<violation number="5" location="src/App.tsx:191">
P1: Deleting or clearing within 300 ms of an edit can resurrect the removed block when the pending debounced update arrives. Cancel pending updates for deleted/cleared IDs and have the server reject updates for missing blocks.</violation>
<violation number="6" location="src/App.tsx:191">
P1: Concurrent edits to different properties of one block lose data: each event replaces the entire block with a stale snapshot. Send field-level patches (or apply revision/conflict checks) instead of broadcasting full `Block` objects.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| import express from 'express'; | ||
| import { createServer } from 'http'; | ||
| import { Server } from 'socket.io'; | ||
| import cors from 'cors'; |
There was a problem hiding this comment.
P0: Missing dependency: cors is imported and used (import cors from 'cors') but not listed in package.json dependencies or devDependencies. The project will fail at runtime after a clean install because npm install won't install cors.
Add "cors": "^2.8.5" to dependencies in package.json.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server.js, line 4:
<comment>Missing dependency: `cors` is imported and used (`import cors from 'cors'`) but not listed in `package.json` `dependencies` or `devDependencies`. The project will fail at runtime after a clean install because `npm install` won't install `cors`.
Add `"cors": "^2.8.5"` to `dependencies` in `package.json`.</comment>
<file context>
@@ -0,0 +1,136 @@
+import express from 'express';
+import { createServer } from 'http';
+import { Server } from 'socket.io';
+import cors from 'cors';
+
+const app = express();
</file context>
| socket.leave(currentRoom); | ||
| } | ||
|
|
||
| socket.join(workspaceId); |
There was a problem hiding this comment.
P1: Workspace joins are accepted for any supplied workspaceId without an access check, so anyone who can guess an ID can join that room and emit mutating events. Consider validating membership/authorization before socket.join(workspaceId) and rejecting unauthorized joins.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server.js, line 32:
<comment>Workspace joins are accepted for any supplied `workspaceId` without an access check, so anyone who can guess an ID can join that room and emit mutating events. Consider validating membership/authorization before `socket.join(workspaceId)` and rejecting unauthorized joins.</comment>
<file context>
@@ -0,0 +1,136 @@
+ socket.leave(currentRoom);
+ }
+
+ socket.join(workspaceId);
+ currentRoom = workspaceId;
+
</file context>
| if (!currentRoom) return; | ||
| const roomState = workspaces.get(currentRoom); | ||
| if (roomState) { | ||
| roomState.blocks = blocks; |
There was a problem hiding this comment.
P1: full-sync assigns unvalidated client data directly to roomState.blocks. If a non-array payload is sent, later handlers call array methods on invalid data and can throw. Validate the payload type/schema before storing or broadcasting it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server.js, line 109:
<comment>`full-sync` assigns unvalidated client data directly to `roomState.blocks`. If a non-array payload is sent, later handlers call array methods on invalid data and can throw. Validate the payload type/schema before storing or broadcasting it.</comment>
<file context>
@@ -0,0 +1,136 @@
+ if (!currentRoom) return;
+ const roomState = workspaces.get(currentRoom);
+ if (roomState) {
+ roomState.blocks = blocks;
+ socket.to(currentRoom).emit('full-sync-update', blocks);
+ }
</file context>
| ? `http://${window.location.hostname}:3001` | ||
| : window.location.origin; | ||
|
|
||
| export const socket = io(SOCKET_URL, { |
There was a problem hiding this comment.
P1: After a transient disconnect, this client reconnects but no longer belongs to its workspace, so subsequent edits/presence are dropped or not received. Rejoin the current workspace from the socket connect handler (which also runs after reconnect) and resync there.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/socket.ts, line 8:
<comment>After a transient disconnect, this client reconnects but no longer belongs to its workspace, so subsequent edits/presence are dropped or not received. Rejoin the current workspace from the socket `connect` handler (which also runs after reconnect) and resync there.</comment>
<file context>
@@ -0,0 +1,19 @@
+ ? `http://${window.location.hostname}:3001`
+ : window.location.origin;
+
+export const socket = io(SOCKET_URL, {
+ autoConnect: false, // We will connect manually when we have the workspace ID
+});
</file context>
| // Update node details (Right panel) | ||
| const handleUpdateBlock = (updatedBlock: Block) => { | ||
| setBlocks((prev) => prev.map((b) => (b.id === updatedBlock.id ? updatedBlock : b))); | ||
| emitUpdateDebounced(updatedBlock); |
There was a problem hiding this comment.
P1: Concurrent edits to different properties of one block lose data: each event replaces the entire block with a stale snapshot. Send field-level patches (or apply revision/conflict checks) instead of broadcasting full Block objects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App.tsx, line 191:
<comment>Concurrent edits to different properties of one block lose data: each event replaces the entire block with a stale snapshot. Send field-level patches (or apply revision/conflict checks) instead of broadcasting full `Block` objects.</comment>
<file context>
@@ -122,6 +188,7 @@ export default function App() {
// Update node details (Right panel)
const handleUpdateBlock = (updatedBlock: Block) => {
setBlocks((prev) => prev.map((b) => (b.id === updatedBlock.id ? updatedBlock : b)));
+ emitUpdateDebounced(updatedBlock);
};
</file context>
| socket.off('block-updated', onBlockUpdated); | ||
| socket.off('block-deleted', onBlockDeleted); | ||
| socket.off('blocks-cleared', onBlocksCleared); | ||
| socket.off('full-sync-update', onFullSync); |
There was a problem hiding this comment.
P2: Debounced socket emit for block updates is never flushed before socket disconnect. The useEffect cleanup disconnects the socket (socket.disconnect()), but emitUpdateDebounced has no flush/cancel capability. If a user edits a label (triggering the 300ms debounce) and then immediately navigates away or closes the tab, the debounced update never fires — the last edit is silently lost for other collaborators.
Add a flush method to the debounce utility, or use a ref to call the debounced function directly with flush during cleanup. Alternatively, use an immediate (non-debounced) emit for important final-state updates and reserve debouncing only for transient/in-flight events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/App.tsx, line 115:
<comment>Debounced socket emit for block updates is never flushed before socket disconnect. The `useEffect` cleanup disconnects the socket (`socket.disconnect()`), but `emitUpdateDebounced` has no flush/cancel capability. If a user edits a label (triggering the 300ms debounce) and then immediately navigates away or closes the tab, the debounced update never fires — the last edit is silently lost for other collaborators.
Add a `flush` method to the debounce utility, or use a ref to call the debounced function directly with `flush` during cleanup. Alternatively, use an immediate (non-debounced) emit for important final-state updates and reserve debouncing only for transient/in-flight events.</comment>
<file context>
@@ -58,6 +59,64 @@ export default function App() {
+ socket.off('block-updated', onBlockUpdated);
+ socket.off('block-deleted', onBlockDeleted);
+ socket.off('blocks-cleared', onBlocksCleared);
+ socket.off('full-sync-update', onFullSync);
+ socket.disconnect();
+ };
</file context>
| const onUserLeft = (data: { userId: string }) => { | ||
| setRemoteCursors(prev => { | ||
| const next = { ...prev }; | ||
| delete next[data.userId]; | ||
| return next; | ||
| }); | ||
| setRemoteSelections(prev => { | ||
| const next = { ...prev }; | ||
| delete next[data.userId]; | ||
| return next; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
P3: Repeated disconnects/reconnects leave stale entries in userColors, so this component's presence state grows for the lifetime of the canvas. Remove the departing user's color with its cursor and selection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/CenterCanvas.tsx, line 91:
<comment>Repeated disconnects/reconnects leave stale entries in `userColors`, so this component's presence state grows for the lifetime of the canvas. Remove the departing user's color with its cursor and selection.</comment>
<file context>
@@ -55,6 +56,66 @@ export default function CenterCanvas({
+ getUserColor(data.userId);
+ };
+
+ const onUserLeft = (data: { userId: string }) => {
+ setRemoteCursors(prev => {
+ const next = { ...prev };
</file context>
| const onUserLeft = (data: { userId: string }) => { | |
| setRemoteCursors(prev => { | |
| const next = { ...prev }; | |
| delete next[data.userId]; | |
| return next; | |
| }); | |
| setRemoteSelections(prev => { | |
| const next = { ...prev }; | |
| delete next[data.userId]; | |
| return next; | |
| }); | |
| }; | |
| const onUserLeft = (data: { userId: string }) => { | |
| setRemoteCursors(prev => { | |
| const next = { ...prev }; | |
| delete next[data.userId]; | |
| return next; | |
| }); | |
| setRemoteSelections(prev => { | |
| const next = { ...prev }; | |
| delete next[data.userId]; | |
| return next; | |
| }); | |
| setUserColors(prev => { | |
| const next = { ...prev }; | |
| delete next[data.userId]; | |
| return next; | |
| }); | |
| }; |
| </div> | ||
|
|
||
| {/* Render Remote Cursors */} | ||
| {Object.entries(remoteCursors).map(([userId, pos]) => { |
There was a problem hiding this comment.
P3: Remote cursors disappear for empty or newly cleared workspaces because this render is gated by blocks.length !== 0. Render the presence overlay outside the empty-state conditional so collaborators remain visible before a block exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/CenterCanvas.tsx, line 805:
<comment>Remote cursors disappear for empty or newly cleared workspaces because this render is gated by `blocks.length !== 0`. Render the presence overlay outside the empty-state conditional so collaborators remain visible before a block exists.</comment>
<file context>
@@ -716,6 +801,35 @@ export default function CenterCanvas({
</div>
+ {/* Render Remote Cursors */}
+ {Object.entries(remoteCursors).map(([userId, pos]) => {
+ const color = userColors[userId] || '#6366f1';
+ return (
</file context>
ef443ee to
3ee5dd9
Compare
|
@Dev1822 Please resolve the branch conflicts |
3ee5dd9 to
dc5c84b
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.tsx (1)
240-275: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMove
socket.emitcalls out ofsetBlocksupdaters inhandleAddBlockandhandleDeleteBlock. Both handlers place side-effecting socket emissions inside a functionalsetBlocks(prev => ...)updater; React can invoke these updaters more than once, and neitheronBlockAddednoronBlockUpdateddedupe by id, so a replay would surface duplicate/incorrect state to collaborators.
src/App.tsx#L240-L275: computeupdated/modifiedActiveBlock/newBlockfrom theblocksclosure, callsetBlocks([...updated, newBlock])with a plain value, then emitadd-block/update-blockafter the call.src/App.tsx#L305-L317: compute the filtered/mapped array and collect blocks needing anupdate-blockemit outside the updater, callsetBlocks(updated), then emit the collected updates anddelete-blockafter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 240 - 275, Move all socket side effects out of the functional setBlocks updaters in handleAddBlock and handleDeleteBlock. In src/App.tsx lines 240-275, derive updated, modifiedActiveBlock, and newBlock from the blocks closure, call setBlocks with the resulting array, then emit add-block and update-block. In src/App.tsx lines 305-317, derive the filtered/mapped array and collect blocks requiring updates before calling setBlocks with the plain value, then emit those update-block events and delete-block afterward.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/App.tsx`:
- Around line 240-275: Move all socket side effects out of the functional
setBlocks updaters in handleAddBlock and handleDeleteBlock. In src/App.tsx lines
240-275, derive updated, modifiedActiveBlock, and newBlock from the blocks
closure, call setBlocks with the resulting array, then emit add-block and
update-block. In src/App.tsx lines 305-317, derive the filtered/mapped array and
collect blocks requiring updates before calling setBlocks with the plain value,
then emit those update-block events and delete-block afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 662e4b43-7cfd-4a86-926a-72fdc3c3879b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonserver.jssrc/App.tsxsrc/components/CenterCanvas.tsxsrc/utils/socket.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utils/socket.ts
- src/components/CenterCanvas.tsx
- server.js
|
@Itzzavdheshh Done |
|
@Itzzavdheshh any updates on the review ? |
2 participants
Description
This PR introduces real-time multi-user collaboration and presence to FlowCraft via WebSockets, transforming it from a single-user application into a collaborative document editor.
Related Issue
Closes #47
Type of Change
Testing Performed
npm install && npm run devstarts without errorsnpm run lintpasses with no TypeScript errors?workspace=idURL.npm run build.Checklist
npm run lintpasses).env.local,node_modules, or generated build outputfeat: add export to svg)Closes #issue-number// TODO: [description]commentsSummary by cubic
Adds real-time multi-user collaboration and presence via WebSockets so users in the same workspace see live edits, selections, and cursors. Implements the collaboration requirements in #47.
New Features
socket.io; room per workspace with in-memory blocks.update-blockand throttled cursor updates to cut network noise.?workspace=with auto-generated ID if missing; client util handles dev/prod URL with manual connect.Dependencies
socket.io,socket.io-client,concurrently,uuid,cors.devscript to run Vite and the WebSocket server together:concurrently "vite --port=3000 --host=0.0.0.0" "node server.js".Written for commit dc5c84b. Summary will update on new commits.
Summary by CodeRabbit