feat: expose skills repository navigation and access state - #4234
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds RbacResource.SkillsRepository, a guarded /workspace/skills-repo file route, sidebar "New" badge support and a gated "Skills Repository" menu item, and a new is_git_available config/schema flag. ChangesSkills Repository Feature with RBAC, Config, and Sidebar Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5Safe to merge — all UI changes follow established patterns and the RBAC guard is correctly wired The route guard, sidebar gating, enum addition, and schema update all follow patterns already in use throughout the workspace. No logic regressions were identified in the changed files. transports/go.mod adds go-git as a direct dependency; the backend source files that import it are not in this PR's diff and should be confirmed reviewed in the prior commits that introduced them Important Files Changed
Reviews (44): Last reviewed commit: "feat: expose skills repository navigatio..." | Re-trigger Greptile |
742573c to
089771f
Compare
2895869 to
ba8d9d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/components/sidebar.tsx`:
- Around line 562-566: The sub-item badge in the expanded submenu is missing the
data-new-badge attribute and the custom class prop used elsewhere; update the
badge rendering for subItem (the conditional that renders <Badge
className="ml-auto">New</Badge>) to include data-new-badge="true" and apply the
existing newBadgeClassName (merge with "ml-auto" or replace as appropriate) so
it matches the top-level and flyout sub-item badges and receives the shine
animation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0dae1f57-28bb-4b21-98d7-03bc6d1df734
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/contexts/rbacContext.tsxui/app/workspace/skills-repo/layout.tsxui/components/sidebar.tsxui/lib/types/config.tsui/lib/types/schemas.ts
089771f to
e260f62
Compare
ba8d9d7 to
bd75352
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/components/sidebar.tsx (1)
1132-1162: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMissing
hasFeatureFlagsAccessin useMemo dependency array.The
hasFeatureFlagsAccesshook is declared at line 739-742 and used in the items array at line 1127 for the Feature Flags sub-item, but it's not included in theuseMemodependency array. When this permission changes, the sidebar items won't recompute, potentially showing stale access state.Since this PR is already adding
hasSkillsRepositoryAccessto the dependency array, it's a good opportunity to include the missinghasFeatureFlagsAccessas well.♻️ Proposed fix to complete the dependency array
hasSettingsAccess, hasPromptRepositoryAccess, hasSkillsRepositoryAccess, hasAccessProfilesAccess, isDbConnected, + hasFeatureFlagsAccess, ], );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/sidebar.tsx` around lines 1132 - 1162, The useMemo that builds the sidebar items (the items array used in the sidebar component) is missing hasFeatureFlagsAccess in its dependency list, so update the useMemo dependency array to include hasFeatureFlagsAccess (alongside the already-added hasSkillsRepositoryAccess and the other permission flags) to ensure the Feature Flags sub-item recomputes when its permission changes; locate the useMemo that returns items and add hasFeatureFlagsAccess to that dependency array.
♻️ Duplicate comments (1)
ui/components/sidebar.tsx (1)
562-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent badge styling in expanded submenu sub-items.
This badge rendering is missing
data-new-badge="true"and thenewBadgeClassNamethat the other two badge locations (lines 316-326 for top-level items, lines 474-481 for flyout sub-items) include. Without these, the shine animation defined innewBadgeClassNamewon't apply to badges in the expanded submenu, creating visual inconsistency.🐛 Proposed fix for consistent badge styling
{subItem.new && ( - <Badge className="ml-auto"> + <Badge + data-new-badge="true" + className={cn("ml-auto", newBadgeClassName)} + > New </Badge> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/sidebar.tsx` around lines 562 - 566, The expanded submenu badge rendering for subItem (inside ui/components/sidebar.tsx) is missing the data attribute and class used elsewhere, so add data-new-badge="true" and apply the same newBadgeClassName used for top-level items and flyout sub-items to the Badge for expanded sub-items (the Badge rendered for subItem.new) so the shine animation is consistent; locate the Badge instance inside the expanded submenu rendering and update it to include data-new-badge="true" and the existing newBadgeClassName prop/variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ui/components/sidebar.tsx`:
- Around line 1132-1162: The useMemo that builds the sidebar items (the items
array used in the sidebar component) is missing hasFeatureFlagsAccess in its
dependency list, so update the useMemo dependency array to include
hasFeatureFlagsAccess (alongside the already-added hasSkillsRepositoryAccess and
the other permission flags) to ensure the Feature Flags sub-item recomputes when
its permission changes; locate the useMemo that returns items and add
hasFeatureFlagsAccess to that dependency array.
---
Duplicate comments:
In `@ui/components/sidebar.tsx`:
- Around line 562-566: The expanded submenu badge rendering for subItem (inside
ui/components/sidebar.tsx) is missing the data attribute and class used
elsewhere, so add data-new-badge="true" and apply the same newBadgeClassName
used for top-level items and flyout sub-items to the Badge for expanded
sub-items (the Badge rendered for subItem.new) so the shine animation is
consistent; locate the Badge instance inside the expanded submenu rendering and
update it to include data-new-badge="true" and the existing newBadgeClassName
prop/variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fe64f931-6456-45fa-be1c-53372dc6ad16
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/contexts/rbacContext.tsxui/app/workspace/skills-repo/layout.tsxui/components/sidebar.tsxui/lib/types/config.tsui/lib/types/schemas.ts
e260f62 to
fadd334
Compare
bd75352 to
6c2cbec
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/components/sidebar.tsx (1)
562-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBadge styling inconsistency still present (duplicate).
The expanded submenu sub-item badge is missing
data-new-badge="true"and thenewBadgeClassNamethat the other two badge locations (lines 316-326 for top-level items, lines 474-481 for flyout sub-items) include. The shine animation won't apply here.This was flagged in a previous review and marked as addressed, but the current code still shows the incomplete styling.
🐛 Proposed fix for consistent badge styling
{subItem.new && ( - <Badge className="ml-auto"> + <Badge + data-new-badge="true" + className={cn("ml-auto", newBadgeClassName)} + > New </Badge> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/sidebar.tsx` around lines 562 - 566, The expanded submenu's "New" Badge rendering (the block checking subItem.new) is missing the data-new-badge="true" attribute and the shared newBadgeClassName styling; update that Badge to include data-new-badge="true" and combine its className with newBadgeClassName (e.g., preserve "ml-auto" and merge newBadgeClassName) so it matches the other Badge instances; reference the Badge element inside the subItem.new conditional and the newBadgeClassName variable to locate and fix the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/components/sidebar.tsx`:
- Around line 562-566: The expanded submenu's "New" Badge rendering (the block
checking subItem.new) is missing the data-new-badge="true" attribute and the
shared newBadgeClassName styling; update that Badge to include
data-new-badge="true" and combine its className with newBadgeClassName (e.g.,
preserve "ml-auto" and merge newBadgeClassName) so it matches the other Badge
instances; reference the Badge element inside the subItem.new conditional and
the newBadgeClassName variable to locate and fix the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c08733c9-c8d9-42d4-977b-053ceb29ff7d
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/contexts/rbacContext.tsxui/app/workspace/skills-repo/layout.tsxui/components/sidebar.tsxui/lib/types/config.tsui/lib/types/schemas.ts
6c2cbec to
5f3cb4a
Compare
fadd334 to
3d05ddf
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/components/sidebar.tsx (1)
562-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpanded submenu badge still missing styling attributes (duplicate issue).
The Badge at lines 562-566 lacks
data-new-badge="true"andnewBadgeClassNamethat are present in the top-level badge (lines 318-321) and flyout sub-item badge (lines 476-478). Without these, the shine animation defined innewBadgeClassName(line 95-96) will not apply here, creating an inconsistent visual experience.This was flagged in a previous review and marked as addressed, but the current code still shows the issue.
🐛 Proposed fix for consistent badge styling
{subItem.new && ( - <Badge className="ml-auto"> + <Badge + data-new-badge="true" + className={cn("ml-auto", newBadgeClassName)} + > New </Badge> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/sidebar.tsx` around lines 562 - 566, The expanded submenu Badge rendering for subItem.new (in ui/components/sidebar.tsx) is missing the data-new-badge="true" attribute and the newBadgeClassName prop; update the JSX where Badge is rendered for subItem.new (the block containing {subItem.new && (<Badge ...>New</Badge>)}) to include data-new-badge="true" and pass newBadgeClassName (the same class prop used on the top-level badge and flyout sub-item badge) so the shine animation applies consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/components/sidebar.tsx`:
- Around line 562-566: The expanded submenu Badge rendering for subItem.new (in
ui/components/sidebar.tsx) is missing the data-new-badge="true" attribute and
the newBadgeClassName prop; update the JSX where Badge is rendered for
subItem.new (the block containing {subItem.new && (<Badge ...>New</Badge>)}) to
include data-new-badge="true" and pass newBadgeClassName (the same class prop
used on the top-level badge and flyout sub-item badge) so the shine animation
applies consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6fde5d2-4662-46a2-a934-cf91e1a417eb
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/contexts/rbacContext.tsxui/app/workspace/skills-repo/layout.tsxui/components/sidebar.tsxui/lib/types/config.tsui/lib/types/schemas.ts
5f3cb4a to
cf6ff45
Compare
3d05ddf to
6be3306
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/components/sidebar.tsx (1)
562-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent badge styling in expanded submenu sub-items (duplicate).
This badge rendering is missing
data-new-badge="true"andnewBadgeClassNamethat the other two badge locations (lines 316-326 for top-level items, lines 474-481 for flyout sub-items) include. The shine animation won't apply here.🐛 Proposed fix for consistent badge styling
{subItem.new && ( - <Badge className="ml-auto"> + <Badge + data-new-badge="true" + className={cn("ml-auto", newBadgeClassName)} + > New </Badge> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/sidebar.tsx` around lines 562 - 566, The expanded submenu sub-item badge rendering (when subItem.new is true) is missing the data-new-badge="true" attribute and the newBadgeClassName applied elsewhere; update the Badge usage inside the expanded submenu (the branch that checks subItem.new) to include data-new-badge="true" and pass the newBadgeClassName prop/class to match the other Badge usages (same pattern as the top-level Badge and flyout sub-item Badge), ensuring consistent styling and the shine animation is applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/components/sidebar.tsx`:
- Around line 562-566: The expanded submenu sub-item badge rendering (when
subItem.new is true) is missing the data-new-badge="true" attribute and the
newBadgeClassName applied elsewhere; update the Badge usage inside the expanded
submenu (the branch that checks subItem.new) to include data-new-badge="true"
and pass the newBadgeClassName prop/class to match the other Badge usages (same
pattern as the top-level Badge and flyout sub-item Badge), ensuring consistent
styling and the shine animation is applied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6988d5c1-6cde-48f2-8da8-6ef73106f25e
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/contexts/rbacContext.tsxui/app/workspace/skills-repo/layout.tsxui/components/sidebar.tsxui/lib/types/config.tsui/lib/types/schemas.ts
cf6ff45 to
a1b58fb
Compare
6be3306 to
63f309b
Compare
a1b58fb to
a5b8d1c
Compare
1ee7de5 to
8963ea7
Compare
59f72cf to
75bbbe9
Compare
8963ea7 to
57ede98
Compare
56d8d1b to
cccdcdd
Compare
57ede98 to
cc5edd6
Compare
cccdcdd to
fa1acae
Compare
3f0f938 to
961f759
Compare
7f7ae1e to
194aedd
Compare
7bee282 to
925e4fa
Compare
194aedd to
7162ea7
Compare
925e4fa to
80947df
Compare
7162ea7 to
7659eb1
Compare
80947df to
acdbac4
Compare
6b56312 to
7ee0f7b
Compare
38a17ec to
8c18056
Compare
7ee0f7b to
6221cb7
Compare
8c18056 to
02f20d3
Compare
f49b047 to
d47661c
Compare
Merge activity
|

Summary
Exposes Skills Repository in the workspace navigation and wires frontend access
state so the page is only visible/usable to users with
SkillsRepository:Viewpermission.
Changes
SkillsRepository:Viewguard for/workspace/skills-repo,rendering
NoPermissionViewwhen access is denied.section, gated by
is_db_connectedandSkillsRepository:View.is_git_available.Type of change
Affected areas
How to test
Validated with:
Expected outcome: the UI production build completes successfully.
Recommended manual UI verification before merge:
SkillsRepository:Viewand confirm the sidebaritem and page are visible.
SkillsRepository:Viewand confirm the sidebaritem is hidden and the route guard blocks access.
No new automated tests were added; this PR was validated through the UI build
and manual RBAC/navigation checks.
No new configs or environment variables are added in this PR.
Screenshots/Recordings
UI changes include the workspace sidebar entry and no-permission behavior for
Skills Repository. Screenshots should be added before submitting if required by
review.
Breaking changes
Related issues
N/A
Security considerations
This adds frontend gating only. Backend route protection remains enforced by the
server and enterprise RBAC middleware; the UI guard is not treated as the source
of authorization truth.
Checklist
docs/contributing/README.mdand followed the guidelines