feat: add Skills management tab to dashboard#57
Merged
Conversation
Add a new Skills tab to the PPG macOS dashboard for managing Claude Code
skills (~/.claude/skills/{name}/SKILL.md) directly from the UI.
Features:
- List, create, edit, delete Claude Code skills
- Import from existing ppg prompts and swarms
- Manage reference files in skills' references/ subdirectory
- YAML frontmatter parsing (name, description, user-invocable)
- Kebab-case name validation with directory rename on save
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an Agent Config feature (UI + controller wiring), a CLAUDE.md editor, PPG agents editor, and a full Skills management UI; integrates Agent Config and Skills into sidebar, dashboard, and content view controllers with lifecycle and single-pane consistency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar as SidebarViewController
participant Dashboard as DashboardSplitViewController
participant Content as ContentTabViewController
participant AgentConfig as AgentConfigView
participant Child as (ClaudeMd|Skills|Agents)
User->>Sidebar: Clicks Agent Config tab
Sidebar->>Dashboard: onAgentConfigClicked()
Dashboard->>Content: showAgentConfigView()
Content->>Content: instantiate/attach AgentConfigView (if needed)
Content->>AgentConfig: configure(projects:)
AgentConfig->>Child: showChild(at: 0..2)
Child-->>User: display chosen editor (ClaudeMd / Skills / Agents)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Owner
Author
Code reviewFound 4 issues:
ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 798 to 815 in c76e70d ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 872 to 875 in c76e70d
ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 991 to 997 in c76e70d
ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 724 to 730 in c76e70d ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 852 to 858 in c76e70d
ppg-cli/PPG CLI/PPG CLI/SkillsView.swift Lines 754 to 755 in c76e70d 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
… clipping, path traversal Fix import type switcher by storing dialog state as instance properties so importTypeChanged repopulates the item list when switching Prompt/Swarm. Preserve unsaved body edits when navigating to/from reference files via pendingBodyText. Add fileExists guards to prevent silent skill overwrites in new/import/rename flows. Increase import dialog accessory height from 130pt to 160pt to prevent name field clipping. Add validateFilename() to reject path traversal (../, /) in skill names and reference filenames. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PPG` CLI/PPG CLI/ContentTabViewController.swift:
- Line 87: isShowingSkillsView currently checks presence via
skillsView?.superview != nil which can remain true after
restoreGrid(forEntryId:) because restoreGrid doesn't remove or detach
skillsView; update restoreGrid(forEntryId:) to explicitly remove or hide
skillsView (e.g. skillsView?.removeFromSuperview() or set skillsView?.isHidden =
true) and/or update any refresh logic to call a new helper (e.g.
hideSkillsViewIfNeeded()) so that isShowingSkillsView accurately reflects the UI
state after grid restoration.
In `@PPG` CLI/PPG CLI/SkillsView.swift:
- Around line 969-1031: The add/remove reference handlers (addRefClicked /
removeRefClicked) call configure() and loadSkillDetail(at:) which reload
SKILL.md from disk and overwrite any unsaved editor edits; before calling
configure()/loadSkillDetail(at:) detect unsaved SKILL.md changes in the current
skill editor (compare editor text to model or track an isDirty flag), and either
prompt the user to Save/Discard/Cancel or programmatically save the current
editor contents (invoke the existing save routine or add a
saveCurrentSkillEdits() helper) so unsaved edits are preserved or the operation
is aborted; apply the same change to the counterpart handler(s) that manage
references so adding/removing references no longer silently discards edits.
- Around line 887-892: The guard that silently returns when
itemPopup.indexOfSelectedItem is < 0 should instead provide user feedback; in
SkillsView.swift replace the silent early-return with a check that, if itemIdx <
0, presents an informative alert (e.g., NSAlert with a message like "No source
items exist for the selected type") or otherwise disables the Create action, and
do not clear importTypePopup/importItemPopup/importNameInput; update the code
around isPrompt, typePopup, itemPopup handling so the UI either shows the alert
or prevents Create when no source items exist.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
PPG CLI/PPG CLI/ContentTabViewController.swiftPPG CLI/PPG CLI/DashboardSplitViewController.swiftPPG CLI/PPG CLI/SidebarViewController.swiftPPG CLI/PPG CLI/SkillsView.swift
…r and ppg agents Replace the single-purpose Skills sidebar tab with a comprehensive Agent Config tab containing a segmented control that switches between three views: - CLAUDE.md: Editor for all CLAUDE.md locations (project root, .claude/, CLAUDE.local.md, .claude/rules/*.md, ~/.claude/CLAUDE.md, ~/.claude/rules/*) - Skills: Enhanced skill editor now scanning both personal (~/.claude/skills/) and project-level (.claude/skills/) skills, with full frontmatter support including argument-hint, disable-model-invocation, allowed-tools, model, context, and agent fields - Agents: Raw YAML editor for .ppg/config.yaml agent definitions Rename plumbing throughout sidebar, content controller, and split controller from skills -> agentConfig. New skills default to user-invocable: true matching Claude Code's official default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dit preservation - Add agentConfigView/schedulesView removal in restoreGrid to keep isShowingAgentConfigView accurate after grid restoration - Show informative alert when import dialog has no source items instead of failing silently - Preserve unsaved SKILL.md body edits when adding/removing reference files by capturing draft text before configure()/loadSkillDetail() reload Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
PPG CLI/PPG CLI/ContentTabViewController.swift (1)
479-483:⚠️ Potential issue | 🟠 Major
restoreGriddoes not detach all single-pane views.
schedulesViewandagentConfigVieware not removed here, so pane state can stay stale while grid is active.💡 Suggested fix
homeDashboardView?.removeFromSuperview() worktreeDetailView?.removeFromSuperview() promptsView?.removeFromSuperview() swarmsView?.removeFromSuperview() + schedulesView?.removeFromSuperview() + agentConfigView?.removeFromSuperview()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PPG` CLI/PPG CLI/ContentTabViewController.swift around lines 479 - 483, In restoreGrid, you currently remove homeDashboardView, worktreeDetailView, promptsView, swarmsView and terminalViews from containerView but miss schedulesView and agentConfigView, leaving stale single-pane state; update the restoreGrid implementation to also check and call removeFromSuperview() for schedulesView and agentConfigView (using the same containerView === check pattern used for terminalViews or the existing direct removeFromSuperview calls) so all single-pane views are detached when switching to grid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PPG` CLI/PPG CLI/ClaudeMdEditorView.swift:
- Around line 29-35: The initial selection can pick the leading separator
placeholder when there are no project entries; in configure(projects:), after
populateFileSwitcher() ensure you find the first valid file entry (skip
separator placeholders) instead of blindly selecting index 0 — e.g., check
fileEntries for the first non-separator entry and call
fileSwitcher.selectItem(at:) and loadFile(at:) with that index; apply the same
change to the other block around loadFile(at:) / fileSwitcher selection
referenced (lines ~167-173) so both places skip separator items when choosing
the initial selection.
- Around line 202-215: In fileSwitcherChanged(_:) the Cancel path still falls
through to loadFile(at:), so abort the switch when the user cancels: after
creating the NSAlert and calling alert.runModal(), check the returned value and
if it is not .alertFirstButtonReturn (i.e., the user cancelled) immediately
return from fileSwitcherChanged; otherwise proceed to call loadFile(at:
sender.indexOfSelectedItem). Ensure this logic references isDirty,
alert.runModal(), and loadFile(at:) so the method only calls loadFile when it’s
safe to switch.
In `@PPG` CLI/PPG CLI/PpgAgentsView.swift:
- Around line 181-193: The projectSwitcherChanged(_:) handler leaves the
NSPopUpButton selection on the new project when the user cancels, causing
subsequent saves to write to the wrong config; capture the current selection
index before prompting (e.g., prevIndex = sender.indexOfSelectedItem or store
selected item), and if the alert is cancelled revert the popup to prevIndex
before returning; apply the same pattern to the other project-switching handler
referenced around lines 197-200 so any canceled switch restores the UI selection
to the original project.
In `@PPG` CLI/PPG CLI/SkillsView.swift:
- Around line 833-847: The rename currently always builds newDir from the
personal skills root (skillsDir) which will move project-scoped skills out of
their project; change the destination logic used before calling
fm.moveItem(atPath:toPath:) so it preserves the skill's current scope: determine
the correct skills root (if the Skill instance indicates a project-scoped skill
use the project skills directory or the skill's current parent directory,
otherwise use the personal "~/.claude/skills"), then build newDir from that root
and newName (replace use of skillsDir variable when computing newDir); ensure
subsequent paths like newPath ((newDir as
NSString).appendingPathComponent("SKILL.md")) continue to use this scope-aware
newDir so project skills remain inside their project.
---
Duplicate comments:
In `@PPG` CLI/PPG CLI/ContentTabViewController.swift:
- Around line 479-483: In restoreGrid, you currently remove homeDashboardView,
worktreeDetailView, promptsView, swarmsView and terminalViews from containerView
but miss schedulesView and agentConfigView, leaving stale single-pane state;
update the restoreGrid implementation to also check and call
removeFromSuperview() for schedulesView and agentConfigView (using the same
containerView === check pattern used for terminalViews or the existing direct
removeFromSuperview calls) so all single-pane views are detached when switching
to grid.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
PPG CLI/PPG CLI/AgentConfigView.swiftPPG CLI/PPG CLI/ClaudeMdEditorView.swiftPPG CLI/PPG CLI/ContentTabViewController.swiftPPG CLI/PPG CLI/DashboardSplitViewController.swiftPPG CLI/PPG CLI/PpgAgentsView.swiftPPG CLI/PPG CLI/SidebarViewController.swiftPPG CLI/PPG CLI/SkillsView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- PPG CLI/PPG CLI/DashboardSplitViewController.swift
Owner
Author
|
All 4 issues from the code review were addressed in 1b0dd7a, and the 3 CodeRabbit inline review comments have now been resolved in 396bcc9:
|
…skip, rename path - ClaudeMdEditorView: track currentFileIndex, skip separator in initial selection, restore popup on cancel, save uses tracked index - PpgAgentsView: track currentConfigIndex, restore popup on cancel, save uses tracked index to prevent writing to wrong config file - SkillsView: rename uses skill.skillDir parent instead of hardcoded ~/.claude/skills, preventing project-scoped skills from being moved Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 tasks
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
1 participant
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
~/.claude/skills/{name}/SKILL.md)references/subdirectory)Test plan
~/.claude/skills/{name}/SKILL.mdreferences/subdirectory/slash-commandsin Claude Code🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UX