feat(permissions): MCP settings page#2978
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds MCP settings and permission management: new MCP settings pages and components, permission services/types/fetchers, mapping utilities and a permissions hook, UI permission controls (cards, selectors, switches), RBAC review API, and a Radix-based Switch dependency. Changes
Sequence DiagramssequenceDiagram
participant User
participant MCP_UI as "MCP UI (Pages/Components)"
participant PermissionsHook as "useMcpResourcePermissions"
participant PermissionAPI as "Permission API Client"
participant Backend
User->>MCP_UI: open MCP tab / select resource / toggle override / open selector
MCP_UI->>PermissionsHook: read resources + permissions (init)
PermissionsHook->>PermissionAPI: GET permissions (mcp:run / mcp:use), GET subjects
PermissionAPI->>Backend: HTTP GET /permissions, /subjects
Backend-->>PermissionAPI: permissions[], subjects[]
PermissionAPI-->>PermissionsHook: data
PermissionsHook-->>MCP_UI: permissionsByResource, globalOverrideByResource, preselectedSubjectIds
User->>MCP_UI: toggle global override / apply subject selection
MCP_UI->>PermissionsHook: setGlobalOverride / allowSelectiveAccess
PermissionsHook->>PermissionAPI: POST/PATCH/DELETE /permissions
PermissionAPI->>Backend: HTTP POST/PATCH/DELETE /permissions
Backend-->>PermissionAPI: success
PermissionAPI-->>PermissionsHook: result
PermissionsHook->>MCP_UI: refetch/update state
MCP_UI->>User: updated UI
Possibly related PRs
Suggested Reviewers
🚥 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)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
e9ae835 to
95a60f2
Compare
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
6f4e30c to
dd849e5
Compare
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
dd849e5 to
57196ee
Compare
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
57196ee to
373ef27
Compare
- Fix re-render loop: init selected state only on open transition via prevOpenRef, read preselectedSubjectIds through a stable ref so parent re-renders don't wipe user selections - Fix incomplete onAllow payload: add fetchPermissionSubjectsByIds and seed selectedSubjects upfront so preselected subjects on other pages are always included - Allow applying empty selection (remove last subject) by disabling Apply only when selection is unchanged from initial, not when count is zero - Debounce search input with useDebouncedValue(300ms) to avoid per-keystroke API calls - Remove unnecessary useMemo around selectedCount - Add PAGE_SIZE to query key - Replace Avatar with type-appropriate icon for team/group subjects - Replace typeLabel function with a TYPE_LABELS record map - Wrap subjects derivation in useMemo to fix exhaustive-deps lint warning
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
373ef27 to
1c133c0
Compare
adityathebe
force-pushed
the
feat/permissions-bulk-selection-ui
branch
from
b612f75 to
e0ef0f4
Compare
Closed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@src/pages/Settings/mcp/McpOverviewPage.tsx`:
- Around line 208-215: The icon-only button that opens the MCP setup modal (the
button with key "setup-mcp" that calls setIsSetupMcpModalOpen(true) and renders
AiFillPlusCircle) lacks an explicit accessible name; add an aria-label (e.g.,
aria-label="Setup MCP") to the button element so screen readers announce the
action consistently while keeping the existing title prop and onClick handler
unchanged.
- Around line 138-188: The mutation currently only calls refetchPermissions() in
onSuccess so partial failures (Promise.all rejecting) leave the UI stale; update
the mutation callbacks to call refetchPermissions() in onSettled (or both
onSuccess and onError) so it always runs after the Promise.all of
deletePermission/updatePermission completes or fails—look for the mutation
definition using onSuccess/onError here (and the Promise.all logic with
primaryPermission, duplicatePermissionIds, deletePermission, updatePermission)
and move or add the refetchPermissions() call into an onSettled handler to
guarantee a refresh regardless of partial failures.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 969b4d8a-54bc-41ba-810c-5d5f2c58f6db
📒 Files selected for processing (7)
src/components/MCP/UserList.tsxsrc/components/Permissions/ResourceAccessCard.tsxsrc/components/Permissions/SubjectAccessCard.tsxsrc/components/Tokens/Add/CreateTokenForm.tsxsrc/pages/Settings/mcp/McpOverviewPage.tsxsrc/pages/Settings/mcp/McpPlaybooksPage.tsxsrc/pages/Settings/mcp/McpViewsPage.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/Tokens/Add/CreateTokenForm.tsx
- src/components/Permissions/SubjectAccessCard.tsx
- src/pages/Settings/mcp/McpPlaybooksPage.tsx
- src/pages/Settings/mcp/McpViewsPage.tsx
- src/components/MCP/UserList.tsx
- src/components/Permissions/ResourceAccessCard.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Permissions/PermissionAccessCheckModal.tsx`:
- Around line 152-183: The async checkAccess call in
PermissionAccessCheckModal.tsx can update state after the modal closes; guard
against stale updates by tracking cancellation and only calling setResult /
setRequestError / helpers.setSubmitting when the modal is still open. Wrap the
try/catch/finally body with a local flag (e.g., let cancelled = false) or an
AbortController tied to the modal's close handler, set cancelled = true when the
modal is closed, and before each state update check !cancelled (or
controller.signal.aborted) to avoid applying results after close; ensure the
cancel flag is set where the modal close logic runs so in-flight promises won't
mutate state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea958173-2c9f-4fab-b1e0-b3ea3b57dbbf
📒 Files selected for processing (5)
src/api/services/rbac.tssrc/components/Forms/Formik/FormikResourceSelectorDropdown.tsxsrc/components/Permissions/PermissionAccessCheckModal.tsxsrc/components/Permissions/PermissionsView.tsxsrc/components/Playbooks/Settings/PlaybookPermissionsModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/services/rbac.ts
Extract ResourceList, ResourceRow, and EffectiveAccessBadge from ResourceSelectorPanel. Add independent bulk access switches for Playbooks and Views.
Move sort state and sorting logic into each ResourceList so sorting one list does not rerender the sibling list. Memoize ResourceList and ResourceRow and stabilize bulk handlers in the parent.
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.
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests