Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
review pls |
|
The PR description image depicts a "Share 'ws1' Workspace" modal within the MCPJam Inspector UI. It features:
On it — kicking off the review now! 🐇 ✅ Actions performedReview triggered.
|
WalkthroughAdds an "Invite team members" CTA to the sidebar (shown for authenticated users) and wires a ShareWorkspaceDialog into the sidebar. ShareWorkspaceDialog now supports selecting among multiple workspaces and drives member/invite/visibility/mutation logic from the selected workspace. Many components’ onWorkspaceShared callbacks were changed to accept an optional sourceWorkspaceId. The workspace state hook (use-workspace-state) was updated to accept and resolve sourceWorkspaceId and to preserve local-active-workspace behavior during remote mapping. New tests cover the sidebar CTA, expanded dialog flows, billing gates, and workspace-sharing/handoff behaviors. 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 |
|
The PR description image shows the MCPJam sidebar with a new "Invite team members" CTA at the bottom left, and the resulting "Share 'ws1' Workspace" modal — complete with a workspace picker dropdown, email invite field with role selector, a billing upsell card ("Need more members?"), access-settings dropdown, and the current member list. Looks sharp, ✅ Actions performedReview triggered.
|
dosubot
bot
added
size:L
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 e5afc95. Configure here.
| dispatch({ | ||
| type: "UPDATE_WORKSPACE", | ||
| workspaceId: appState.activeWorkspaceId, | ||
| workspaceId: sourceWorkspaceId ?? appState.activeWorkspaceId, |
There was a problem hiding this comment.
Authenticated path ignores sourceWorkspaceId on first share
Medium Severity
handleWorkspaceShared completely ignores the new sourceWorkspaceId parameter in the isAuthenticated branch, always calling setConvexActiveWorkspaceId(convexWorkspaceId). This contradicts the PR's stated goal of not switching the app to the wrong workspace. If an authenticated user in local-fallback mode shares a non-active workspace via the new dialog picker, the app switches to the newly created remote workspace instead of staying on the current one. The sourceWorkspaceId parameter was introduced to fix exactly this class of bug but is only wired up in the else (non-authenticated) branch.
Reviewed by Cursor Bugbot for commit e5afc95. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx (1)
1225-1256: Strengthen the “active workspace unchanged” proof by seeding prior storage state.The
localStorageassertion currently passes even if nothing happened, because storage is cleared in setup. Seed a known active ID first, then assert it is preserved after sharing a different workspace.Proposed test hardening
it("keeps the active workspace unchanged when sharing a different local workspace", async () => { + localStorage.setItem("convex-active-workspace-id", "workspace-a"); const appState = createAppState({ "workspace-a": createLocalWorkspace("workspace-a", { name: "Workspace A", organizationId: "org-owner", }), @@ - expect(localStorage.getItem("convex-active-workspace-id")).toBeNull(); + expect(localStorage.getItem("convex-active-workspace-id")).toBe("workspace-a"); + expect(result.current.effectiveActiveWorkspaceId).toBe("workspace-a");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx` around lines 1225 - 1256, Seed localStorage with a known active ID before invoking the hook so the test actually verifies preservation: in the test that calls renderUseWorkspaceState and result.current.handleWorkspaceShared, set localStorage.setItem("convex-active-workspace-id", "existing-active-id") (or similar) prior to the act(), then after handleWorkspaceShared assert that localStorage.getItem("convex-active-workspace-id") still equals that seeded value; update references to renderUseWorkspaceState and handleWorkspaceShared in the test and keep the existing assertions for dispatch and logger.info.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx`:
- Around line 1225-1256: Seed localStorage with a known active ID before
invoking the hook so the test actually verifies preservation: in the test that
calls renderUseWorkspaceState and result.current.handleWorkspaceShared, set
localStorage.setItem("convex-active-workspace-id", "existing-active-id") (or
similar) prior to the act(), then after handleWorkspaceShared assert that
localStorage.getItem("convex-active-workspace-id") still equals that seeded
value; update references to renderUseWorkspaceState and handleWorkspaceShared in
the test and keep the existing assertions for dispatch and logger.info.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c96327e-e0f8-436e-b5ef-aa05d9d7fd3f
📒 Files selected for processing (2)
mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsxmcpjam-inspector/client/src/hooks/use-workspace-state.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- mcpjam-inspector/client/src/hooks/use-workspace-state.ts
2 participants
Summary
This PR implements the sidebar invite flow from Linear
M-263throughM-271.What changed:
Invite team membersbutton to the sidebar footer for signed-in usersNotes
We followed the Linear behavior and scope.
The only thing we did not change is the backend invite result naming. The behavior matches what Linear asked for, but we kept the existing result names instead of renaming them.
Testing
use-workspace-state.test.tsx: 30/30 passedShareWorkspaceDialog.test.tsx+sidebar-invite-cta.test.tsx: 15/15 passednpm run buildpassedScreen.Recording.2026-04-13.at.10.37.15.PM.mov
Screen.Recording.2026-04-13.at.10.39.04.PM.mov
Note
Medium Risk
Adds a new sidebar-triggered invite/share entrypoint and refactors the share dialog to support selecting different workspaces, which could affect workspace membership/visibility flows and billing-gated invite behavior if selection or IDs are mishandled.
Overview
Adds an “Invite team members” CTA to the sidebar footer for authenticated users, opening
ShareWorkspaceDialogfor the active workspace and passing throughactiveOrganizationNameplus a workspace list scoped to the active org.Upgrades
ShareWorkspaceDialogto support multi-workspace invite targeting via an in-dialog workspace picker (without switching the app’s active workspace), adds email format validation, and ensures billing gating/mutations run against the selected workspace’sorganizationId/sharedWorkspaceId.Extends
onWorkspaceSharedacross call sites to include an optionalsourceWorkspaceId, and updatesuse-workspace-stateto correctly attach the newsharedWorkspaceIdto the originating local workspace when sharing is first created. Includes new/updated tests covering the sidebar CTA, workspace picker behavior, and gating/validation edge cases.Reviewed by Cursor Bugbot for commit e5afc95. Bugbot is set up for automated code reviews on this repo. Configure here.