Skip to content

feat: rbac UI dashboard#2078

Open
qstearns wants to merge 21 commits intomainfrom
feat/rbac-ui-dashboard
Open

feat: rbac UI dashboard#2078
qstearns wants to merge 21 commits intomainfrom
feat/rbac-ui-dashboard

Conversation

@qstearns
Copy link
Copy Markdown
Contributor

@qstearns qstearns commented Apr 2, 2026

No description provided.

qstearns and others added 18 commits April 1, 2026 17:29
Org scopes (org:read, org:admin) are inherently tied to the current
org and don't need a resource picker dropdown.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix tab bar negative margin causing layout issues
- Fix breadcrumb display names per tab
- Add invite team members row to members table
- Widen action columns to prevent button overflow
- Replace static kebab icon with MoreActions dropdown on roles table

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements the Roles & Permissions settings page based on the RFC
"Gram RBAC — Scope & Permission Design". Adds frontend UI with mock
data and backend Goa service design.

Frontend:
- Roles tab with table listing roles, grants, and member counts
- Members tab with role assignments and change-role dialog
- Create Role side drawer with collapsible scope groups, per-scope
  resource pickers (project/MCP server allowlists), and member assignment
- 7 system-defined scopes across 3 resource types (org, project, mcp)

Backend:
- Goa API design for access service (CRUD roles, list scopes/members,
  update member roles)
- Stub implementation deferred until gen packages are created via
  mise gen:goa

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Gate sidebar nav item and /access route behind "gram-rbac" PostHog flag
- Replace mock data with real SDK hooks in MembersTab and RolesTab
- Add edit mode to CreateRoleDialog with pre-populated fields and grants
- Disable name/description for system roles, hide delete for system roles
- Show tab counts for roles and members
- Fix Radix DropdownMenu/Sheet pointer-events race condition
- UI polish: remove tab bg, tighten spacing, widen actions column

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reset selectedRole state in ChangeRoleDialog when member prop changes
  to prevent stale role assignment across dialog open/close cycles
- Move role id into updateRoleForm where the SDK type expects it
- Fix claims_test.go assertion to match actual code behavior (raw org ID
  instead of deterministicUUID)
- Remove unused Role type import

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The "Assign Members" section was shown in edit mode with interactive
checkboxes, but changes were silently discarded on save since the
UpdateRole API doesn't accept member IDs. Hide the section when
editing — member role changes are handled via the Members tab instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…k-data.ts

Scope definitions now come from the listScopes API instead of a local
mock file. Removes unused MOCK_PROJECTS, MOCK_MCP_SERVERS, and
redundant local type definitions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…roup

- Add "Get help" section to both org and project sidebars
- Remove Support/Docs/Changelog buttons from top header
- Remove billing from project sidebar (already in org sidebar)
- Add scroll indicator arrow when sidebar overflows viewport
- Tighten sidebar spacing (group and item gaps)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Seed script: use Postgres array literal '{id1,id2}' cast to text[]
  instead of ARRAY[:'org_ids'] which produced a single concatenated
  string for multi-org seeding
- Submit button: count only effective grants (unrestricted or with
  selected resources) so the button disables when all scopes have
  empty resource lists

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
System roles (admin, member) now have disabled checkboxes and hidden
resource pickers in the edit dialog, matching the existing behavior
of disabled name/description fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
deleteRole onSuccess was only invalidating the roles cache. When a
role is deleted, WorkOS reassigns members to a default role, so the
members cache must also be invalidated to avoid showing stale role
assignments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
useListRoles → useRoles, useListMembers → useMembers, and
restore auditLogs route lost during rebase.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@qstearns qstearns requested a review from a team as a code owner April 2, 2026 00:38
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gram-docs-redirect Ready Ready Preview, Comment Apr 2, 2026 3:58am

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 2, 2026

🦋 Changeset detected

Latest commit: 1df7db7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 5 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

assert.Equal(t, "My Company", orgs[0].Name)
assert.Equal(t, "my-company", orgs[0].Slug)
assert.Equal(t, deterministicUUID("org:org_123"), orgs[0].ID)
assert.Equal(t, "org_123", orgs[0].ID)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Test expects raw org ID but implementation still hashes it with deterministicUUID

The test assertion at mock-speakeasy-idp/claims_test.go:35 was changed from deterministicUUID("org:org_123") to the raw string "org_123", but the implementation at mock-speakeasy-idp/claims.go:32 was not changed and still returns deterministicUUID("org:" + claims.OrgID). This means the test will always fail because it compares a SHA256-derived UUID string against a plain string "org_123".

Prompt for agents
Either revert the test change in mock-speakeasy-idp/claims_test.go line 35 back to `assert.Equal(t, deterministicUUID("org:org_123"), orgs[0].ID)`, OR update the implementation in mock-speakeasy-idp/claims.go line 32 to use the raw org ID instead of hashing it: change `ID: deterministicUUID("org:" + claims.OrgID)` to `ID: claims.OrgID`. The choice depends on the intended behavior — whether org IDs from WorkOS should be used as-is or hashed.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +90 to +102
if (editingRole && !initialized) {
setName(editingRole.name);
setDescription(editingRole.description);
setGrants(grantsFromRole(editingRole));
const assignedIds = new Set(
members.filter((m) => m.roleId === editingRole.id).map((m) => m.id),
);
setSelectedMembers(assignedIds);
setInitialized(true);
}
if (!editingRole && initialized) {
setInitialized(false);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 CreateRoleDialog pre-populates members during render, may miss async data

In CreateRoleDialog.tsx:90-98, the component pre-populates selectedMembers from the members array when editingRole changes. However, useMembers() at line 68 may not have resolved yet when the dialog first opens with an editingRole. If members is still [] (empty fallback from line 69), the selectedMembers set will be empty, and initialized is set to true. When the members query later resolves, the initialization block won't re-run because initialized is already true. In practice this is mitigated because CreateRoleDialog is always mounted inside RolesTab (line 180), so useMembers() starts fetching on mount regardless of dialog state — but on slow networks or first load, the race is real.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +105 to +111
<DropdownMenuItem
className="text-destructive focus:text-destructive cursor-pointer"
onSelect={() => deleteRole.mutate({ request: { id: role.id } })}
>
Delete
</DropdownMenuItem>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 RolesTab delete has no confirmation dialog

In RolesTab.tsx:107, clicking 'Delete' on a non-system role immediately calls deleteRole.mutate() with no confirmation dialog. This is a destructive operation that removes a role and potentially reassigns or unassigns members. While not strictly a code bug (it works as written), it's a UX concern — accidental clicks could delete roles. The CONTRIBUTING.md guidelines emphasize building high-quality products, so a confirmation step would be appropriate here.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- Show auditLogs in sidebar when gram-rbac flag is off
- Remove billing route from project-level ROUTE_STRUCTURE (rebase artifact)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

orgRoutes.domains,
orgRoutes.logs,
orgRoutes.auditLogs,
...(isRbacEnabled ? [orgRoutes.access] : [orgRoutes.auditLogs]),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Feature flag hides audit logs from sidebar when RBAC is enabled

At client/dashboard/src/components/org-sidebar.tsx:48, the sidebar conditionally shows either orgRoutes.access or orgRoutes.auditLogs based on the gram-rbac feature flag. When RBAC is enabled, audit logs disappear entirely from the sidebar navigation. The audit logs route still exists in the route structure (client/dashboard/src/routes.tsx:558-563) so users can still access it via direct URL, but there's no sidebar link. This may be intentional (replacing audit logs with the access page), but worth confirming that audit logs should become undiscoverable in the UI when RBAC is on.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 No new tests for the RBAC UI components

Six new React components were added (Access.tsx, ChangeRoleDialog.tsx, CreateRoleDialog.tsx, MembersTab.tsx, RolesTab.tsx, ScopePickerPopover.tsx) without any corresponding test files. The CONTRIBUTING.md guideline states 'Add tests for all new contributions' with the note that 'the goal is not to hit 100% test coverage but to have higher and higher confidence.' This is a significant amount of new business logic (role CRUD, member assignment, scope permissions) that would benefit from test coverage.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +90 to +102
if (editingRole && !initialized) {
setName(editingRole.name);
setDescription(editingRole.description);
setGrants(grantsFromRole(editingRole));
const assignedIds = new Set(
members.filter((m) => m.roleId === editingRole.id).map((m) => m.id),
);
setSelectedMembers(assignedIds);
setInitialized(true);
}
if (!editingRole && initialized) {
setInitialized(false);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 CreateRoleDialog member initialization races with async member data loading

In CreateRoleDialog.tsx:90-98, when the dialog opens in edit mode, the initialization block runs immediately during render and populates selectedMembers from the current members array. If membersData hasn't loaded yet (e.g., the useMembers() query at line 68 is still pending), members is [] and selectedMembers will be an empty Set. The initialized flag is then set to true, so when membersData finally loads and the component re-renders, the initialization block is skipped (line 90: editingRole && !initialized is false). Existing role members won't be in selectedMembers, which could cause memberIds to be sent as undefined on save (CreateRoleDialog.tsx:211-214), potentially removing all member assignments depending on API behavior. This is partially mitigated by Access.tsx also calling useMembers() which warms the React Query cache, but the race exists if the cache is cold.

Prompt for agents
In client/dashboard/src/pages/access/CreateRoleDialog.tsx, the initialization block at lines 90-102 sets state during render using an `initialized` flag, but this races with the async `useMembers()` data fetch. The fix should make the initialization depend on `members` data being available. One approach: include `members` (or `membersData`) in the condition, e.g. change line 90 from `if (editingRole && !initialized)` to `if (editingRole && !initialized && membersData)`. Alternatively, convert the initialization to a `useEffect` with `editingRole` and `membersData` as dependencies, resetting `initialized` when `editingRole` changes (via a ref tracking the previous editingRole ID).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants