chore: handle (re)subscribe in guide toolbar V2 when entering/exiting#943
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
7d120df to
4e39774
Compare
bdda172 to
0489df2
Compare
0489df2 to
deae866
Compare
4e39774 to
a9698fd
Compare
a9698fd to
ab5fe31
Compare
deae866 to
7e4dc86
Compare
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ab5fe31. Configure here.
| stopDebug(); | ||
| } | ||
|
|
||
| return () => stopDebug(); |
There was a problem hiding this comment.
Cleanup unconditionally calls stopDebug causing spurious side effects
Medium Severity
The cleanup function unconditionally calls stopDebug(), which always invokes client.unsetDebug() and client.subscribe()/client.unsubscribe(), even when the toolbar was never visible and debug mode was never entered. Since the V2 component is always rendered (returns null when not visible, but hooks still run), any change to readyToTarget or listenForUpdates triggers this cleanup. When listenForUpdates is true, the spurious subscribe() call disconnects and reconnects the websocket channel unnecessarily. Additionally, subscribe() calls this.knock.failIfNotAuthenticated() which throws if the user is unauthenticated — unlike the old code in setDebug/unsetDebug that guarded against this. During logout/unmount, this cleanup can throw an unhandled error. The cleanup needs a guard to only call stopDebug() when debug mode was actually entered.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ab5fe31. Configure here.
ab5fe31 to
63df698
Compare
7e4dc86 to
43c0e12
Compare



Description
** Describe what, why and how of the changes clearly and concisely. Add any additional useful context or info, as necessary. **
Todos
** List any todo items necessary before merging, if any. Delete if none. **
Checklist
Screenshots or videos
** Attach any screenshots or recordings to visually illustrate the changes, as necessary. Delete if not relevant. **