Open
Conversation
Introduces useTabCoordinator composable using BroadcastChannel API for focus-follows-leader tab coordination. Only the leader tab broadcasts heartbeats and state, preventing duplicate signals that caused avatar flickering and WebRTC connect/disconnect loops when multiple tabs were open. - New useTabCoordinator composable with automatic leader election - Guard signalling broadcasts behind leader check - Guard call joining with leadership claim and call-pinned conflict toast - Sync inCall state with coordinator for call pinning
Bug 1: call-pinned handler never reverted the optimistic leadership claim because currentLeaderTabId was set to our own tabId in becomeLeader(), making the !== check always false. Bug 2: claimLeadership was synchronous but the call-pinned rejection arrives asynchronously via BroadcastChannel. joinRoom checked the result before the rejection had a chance to arrive. Fixes: - call-pinned handler now always reverts leadership (own messages are already filtered at the top of handleMessage) - claimLeadership is now async with a 300ms grace period for rejections - joinRoom awaits the claim result properly
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for fluxdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough📝 Walkthrough🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/composables/useTabCoordinator.ts`:
- Around line 69-72: The heartbeat message sent from startLeaderHeartbeat omits
the inCall flag, so followers see heartbeats without msg.inCall and clear
otherTabInCall; update startLeaderHeartbeat to include the current inCall state
when calling post({ type: 'heartbeat' }) (e.g., post({ type: 'heartbeat', inCall
})) so receivers can check msg.inCall and preserve call-pinning; ensure the same
change is applied wherever heartbeats are emitted so otherTabInCall and reclaim
logic (lines reading msg.inCall) behave correctly.
- Around line 61-63: onBecomeLeader and onLoseLeadership are append-only arrays
that leak closures in the singleton coordinator; replace the raw Arrays with
Set<() => void> (or keep arrays but provide subscribe functions that return an
unsubscribe function) and update the subscription API in useTabCoordinator to
add callbacks via a subscribeBecomeLeader/subscribeLoseLeadership method that
returns a function to remove that callback, and ensure the leadership emitters
(where onBecomeLeader/onLoseLeadership are iterated) iterate the Set (or guarded
array) so removed callbacks are not called; also apply the same change to the
other occurrence using the same symbols to prevent accumulation of stale
handlers.
- Line 103: The forEach callbacks on onBecomeLeader and onLoseLeadership use
expression-bodied arrow functions which implicitly return a value and trigger
Biome's useIterableCallbackReturn error; change both callbacks to
statement-bodied arrow functions (e.g., onBecomeLeader.forEach(cb => { cb(); })
and onLoseLeadership.forEach(cb => { cb(); })) so they do not return a value
from the callback and the lint rule is satisfied, updating the callback
implementations in the useTabCoordinator composable where these symbols are
defined.
In `@app/src/stores/webrtcStore.ts`:
- Around line 77-78: The singleton tabCoordinator created via
useTabCoordinator() is never cleaned up, causing BroadcastChannel listeners and
heartbeat timers to leak when the store is recreated; update the store to call
tabCoordinator.destroy() on disposal by importing/using the scope cleanup hook
(onScopeDispose) and registering a callback that calls tabCoordinator.destroy();
also apply the same pattern to the similar useSignallingService() instance so
its destroy() is invoked on scope disposal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ccbc615-5580-4940-9450-d14d0cb402e7
📒 Files selected for processing (3)
app/src/composables/useSignallingService.tsapp/src/composables/useTabCoordinator.tsapp/src/stores/webrtcStore.ts
marvin-bot-coasys
approved these changes
Mar 4, 2026
marvin-bot-coasys
left a comment
marvin-bot-coasys
left a comment
There was a problem hiding this comment.
✅ Approved
The implementation is solid, the bug fixes are correct, and the architecture is sound. The focus-follows-leader pattern is the right approach for preventing multi-tab signalling conflicts.
Key Strengths
- Clean singleton pattern with proper BroadcastChannel coordination
- Well-documented focus-follows-leader semantics
- Proper call pinning (active call blocks leadership transfer)
- Correct async handling in claimLeadership (300ms grace period for rejections)
- Guards all broadcasts behind leader check
Bug Fixes Verified
Bug #1 (call-pinned): ✅ Correctly simplified to always revert optimistic claim
Bug #2 (async race): ✅ Properly awaits claim result before proceeding
Code Quality
- Proper lifecycle management (init/destroy)
- Good defensive coding (try/catch on window.focus)
- Clean integration into signalling service and WebRTC store
Optional Follow-ups (non-blocking)
- Add manual test checklist (open 2+ tabs, switch focus, join call conflicts, etc.)
- Consider random jitter (0-500ms) on visibility claim to reduce simultaneous claim collisions
No blocking issues. Ready to merge. 🎉
- Replace append-only arrays with Sets for leadership callbacks - Subscribe methods now return unsubscribe functions - stopSignalling calls unsubscribe to clean up closures - Heartbeat now includes inCall flag so followers preserve call-pinning - Use statement-bodied forEach callbacks to satisfy lint rules
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/composables/useSignallingService.ts`:
- Around line 304-309: The leadership hooks are being unsubscribed permanently
by calling the stored unsubBecomeLeader/unsubLoseLeadership in stopSignalling,
but startSignalling never re-subscribes; update the logic so that
tabCoordinator.onBecomeLeader and tabCoordinator.onLoseLeadership subscriptions
are created inside startSignalling (or a helper subscribeLeadership function)
and their unsubscribe functions (unsubBecomeLeader, unsubLoseLeadership) are
stored on a higher-scope variable, and then stopSignalling only calls those
stored unsub functions without clearing the subscription creation path; ensure
startSignalling checks whether subscriptions already exist to avoid
double-subscribe and reassigns unsubBecomeLeader/unsubLoseLeadership when
re-subscribing so subsequent restarts react to leader changes.
In `@app/src/composables/useTabCoordinator.ts`:
- Around line 83-86: leaderTimeoutTimer's timeout path can deadlock when
otherTabInCall is true because calling claimLeadership() hits the rejection
path; change the timeout handler to either clear or bypass otherTabInCall when
we are visible so a new leader can be elected — specifically, in the
leaderTimeoutTimer callback (using leaderTimeoutTimer, claimLeadership,
otherTabInCall) when document.visibilityState === 'visible' set otherTabInCall =
false or call claimLeadership with the forced option (e.g.,
claimLeadership(true) / claimLeadership({force: true}) depending on the function
signature) so the immediate rejection on lines ~125-130 is avoided and a new
leader is elected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2de6e307-c1bb-43fa-8c7d-cf2e24b61236
📒 Files selected for processing (2)
app/src/composables/useSignallingService.tsapp/src/composables/useTabCoordinator.ts
…therTabInCall on leader timeout - useSignallingService: move onBecomeLeader/onLoseLeadership subscriptions into a subscribeLeadership() helper called from startSignalling so they are re-established after a stop/start cycle; stopSignalling nulls out the unsub handles instead of permanently destroying them. - useTabCoordinator: clear otherTabInCall before claimLeadership() in the leader timeout handler so a dead leader's stale in-call flag doesn't deadlock new leader election.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: Multi-tab signalling coordination
Problem
When a user opens Flux in multiple browser tabs, each tab independently runs its own signalling heartbeat and broadcasts its own agent state. This causes two issues:
currentRoutefor the same DID, so remote peers see the agent's position oscillate between channels every heartbeat cycle.inCall/callRoutevalues, causing remote peers to repeatedly create and destroy peer connections for the same agent.The root cause is that every tab creates its own
Ad4mClient(separate WebSocket), registers its own signal handler, and runs its own heartbeat timer — with zero cross-tab coordination.Solution
Introduces a focus-follows-leader tab coordination pattern using the
BroadcastChannelAPI. Only one tab (the "leader") broadcasts heartbeats and state to the network. Leadership automatically follows user focus — whichever tab the user is looking at becomes the leader, with no manual interaction required.New file:
app/src/composables/useTabCoordinator.tsSingleton composable that manages tab leader election:
focus/visibilitychange, the active tab claims leadership and the previous leader yields silently.BroadcastChannel. If no heartbeat arrives for 15s, visible follower tabs auto-claim.beforeunloadbroadcasts a resign message so remaining tabs can claim immediately.claimLeadership()waits 300ms for potentialcall-pinnedrejections before confirming, avoiding race conditions.Modified:
app/src/composables/useSignallingService.tsonSignalhandlers and run the agent staleness evaluator (UI stays accurate everywhere).broadcastState().onBecomeLeader/onLoseLeadershipcallbacks auto-toggle broadcasting when leadership changes.Modified:
app/src/stores/webrtcStore.tsjoinRoom()now force-claims leadership before joining. If the claim is rejected (another tab is in a call), it shows a toast: "You are already in a call in another tab" and requests the leader tab to surface viawindow.focus().leaveRoom()resets the coordinator'sinCallflag so other tabs can claim leadership again.watch(inCall)keeps the coordinator in sync with the store's call state.How to test
Files changed
app/src/composables/useTabCoordinator.tsapp/src/composables/useSignallingService.tsapp/src/stores/webrtcStore.tsSummary by CodeRabbit
New Features
Bug Fixes