fix: improve skill discovery and list output#17
Open
KirioXX wants to merge 8 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughExpands discovery to include canonical (~/.skills, ./.skills) and agent-specific directories, deduplicates discovered skills by metadata.name, exposes agent discovery helpers, integrates merged search paths into ChangesSkill Discovery Path Expansion and Deduplication
Sequence DiagramsequenceDiagram
participant User
participant ListCmd as registerListCommand
participant SearchPaths as getListSearchPaths
participant Discovery as discoverSkills
participant Output as formatters
User->>ListCmd: skills list [--verbose]
ListCmd->>SearchPaths: getListSearchPaths()
SearchPaths->>Discovery: DEFAULT_SKILL_PATHS + getAllAgentDiscoveryPaths()
Discovery->>Discovery: glob **/SKILL.md (per-root), load metadata, dedupe by metadata.name
Discovery-->>ListCmd: SkillRef[] (deduped)
ListCmd->>Output: getListJsonEntry() / getListDisplayLines()
Output-->>ListCmd: formatted entries (include sourcePath)
ListCmd-->>User: JSON/table/default output
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-06-01-skill-discovery-paths.md (1)
44-61: 💤 Low valuePlan test code differs from actual implementation.
The test structure in the plan uses paths like
join(root, 'home', '.skills', 'demo-skill')andjoin(root, 'repo', '.agent', 'skills', 'demo-skill')(lines 46-47, 55), but the actual implementation shown in context snippets uses simpler paths likejoin(root, 'canonical', 'demo-skill')andjoin(root, 'agent', 'demo-skill').While the test logic is equivalent, the discrepancy between plan and implementation could confuse readers referencing this document.
📝 Optional update to match actual implementation
- const canonical = join(root, 'home', '.skills', 'demo-skill'); - const agentDir = join(root, 'repo', '.agent', 'skills', 'demo-skill'); + const canonical = join(root, 'canonical', 'demo-skill'); + const agentDir = join(root, 'agent', 'demo-skill'); await mkdir(canonical, { recursive: true }); await mkdir(agentDir, { recursive: true }); @@ -52,7 +52,7 @@ await writeFile(join(agentDir, 'SKILL.md'), skillMd); const skills = await discoverSkills({ - searchPaths: [join(root, 'home', '.skills'), join(root, 'repo', '.agent', 'skills')], + searchPaths: [join(root, 'canonical'), join(root, 'agent')], maxDepth: 3, });🤖 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 `@docs/superpowers/plans/2026-06-01-skill-discovery-paths.md` around lines 44 - 61, Test path literals in the plan differ from the actual implementation which uses simpler directories; update the plan's example test to match the implementation by replacing occurrences of join(root, 'home', '.skills', 'demo-skill') and join(root, 'repo', '.agent', 'skills', 'demo-skill') with the actual canonical/agent layout used by discoverSkills (e.g., join(root, 'canonical', 'demo-skill') and join(root, 'agent', 'demo-skill')), and likewise adjust the created directory variables (canonical, agentDir) and the searchPaths passed to discoverSkills so the example matches the real code paths.
🤖 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 `@docs/superpowers/plans/2026-06-01-skill-discovery-paths.md`:
- Around line 99-107: The plan's getDefaultSkillPaths implementation is missing
two entries present in the real loader: add join(home, ".gemini", "antigravity",
"skills") and the ".agent/skills" path to the returned array in
getDefaultSkillPaths so the plan matches the actual implementation; locate the
getDefaultSkillPaths function and update its returned list to include these two
paths in the appropriate order to mirror the loader.
- Around line 1-12: Add an H2 heading "## Implementation Tasks" immediately
after the existing H1 "Skill Discovery Paths Implementation Plan" and before the
current Task 1 section (the first H3), so the document follows proper Markdown
heading hierarchy; update any references or anchors that assume the previous
structure if needed.
In `@README.md`:
- Around line 251-252: The discovery note block (the quoted line starting with
"**Discovery note:** `skills list`...") was inserted between table rows and
breaks the Markdown table; move that entire quote out of the table body and
place it either immediately above or below the table (for example after the
table closure) so the table rows (including the OpenCode row and subsequent
agent information) render correctly; update the README.md by removing the quote
from inside the table and inserting it as a standalone paragraph outside the
table boundaries.
In `@src/cli/commands/list.ts`:
- Around line 63-69: The printed skill-paths are hardcoded; update the block in
src/cli/commands/list.ts to print the actual computed search paths instead.
Locate where the skill discovery array is built (e.g., variable or function name
like searchPaths, skillSearchPaths, or getSkillSearchPaths) and replace the
fixed console.log lines with code that logs the header ("Skills are searched
in:") then iterates over that array and logs each path with chalk.gray. Ensure
you reference the existing computed array name
(searchPaths/skillSearchPaths/getSkillSearchPaths) so the output always reflects
the real discovery locations.
In `@src/core/loader.test.ts`:
- Around line 17-21: The test is using process.env.HOME which can differ from
the OS homedir used in production; update the assertion to use homedir() instead
of process.env.HOME so DEFAULT_SKILL_PATHS is compared against join(homedir(),
".skills"); ensure you import homedir from 'os' at the top of the test file if
it's not already imported and keep the rest of the assertions (e.g.,
join(process.cwd(), ".skills")) unchanged.
In `@src/core/loader.ts`:
- Line 63: The discovery config currently sets maxDepth using the
falsy-coalescing pattern (config.maxDepth || 3) which ignores an explicit 0;
change this to use an undefined-only fallback so 0 is respected (e.g., use the
nullish coalescing operator or an explicit undefined check). Update the location
where maxDepth is assigned (the object property maxDepth in loader/discovery
setup that references config.maxDepth) to use config.maxDepth ?? 3 or
(config.maxDepth !== undefined ? config.maxDepth : 3) so an explicit maxDepth: 0
is honored.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-06-01-skill-discovery-paths.md`:
- Around line 44-61: Test path literals in the plan differ from the actual
implementation which uses simpler directories; update the plan's example test to
match the implementation by replacing occurrences of join(root, 'home',
'.skills', 'demo-skill') and join(root, 'repo', '.agent', 'skills',
'demo-skill') with the actual canonical/agent layout used by discoverSkills
(e.g., join(root, 'canonical', 'demo-skill') and join(root, 'agent',
'demo-skill')), and likewise adjust the created directory variables (canonical,
agentDir) and the searchPaths passed to discoverSkills so the example matches
the real code paths.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dcd1f777-149d-4405-bbbb-0fd75524954a
📒 Files selected for processing (7)
README.mddocs/superpowers/plans/2026-06-01-skill-discovery-paths.mdsrc/cli/agents.tssrc/cli/commands/list.test.tssrc/cli/commands/list.tssrc/core/loader.test.tssrc/core/loader.ts
…nstallPath docs Add proper JSDoc documentation to functions in list.ts: - getListSearchPaths - formatSkillSourcePath - getListJsonEntry - getListDisplayLines Improve getInstallPath docstring in agents.ts with param and return documentation. Fixes CodeRabbit docstring coverage warning (70.59% → 80%+ threshold)
Add h2 '## Implementation Tasks' section heading between h1 and Task 1 (h3) to fix markdownlint heading hierarchy violation. Fixes CodeRabbit minor issue: heading jump from h1 to h3
…tion Update the getDefaultSkillPaths code snippet in the implementation plan to match the actual code: - Add missing join(home, '.gemini', 'antigravity', 'skills') path - Add missing '.agent/skills' path - Add clarifying comments for path categories Fixes CodeRabbit minor issue: plan/implementation mismatch
The discovery note was inserted between table rows (after Antigravity, before OpenCode), which broke the Markdown table structure. Moved the note outside the table after the '+32 more agents' line. Fixes CodeRabbit major issue: broken table rendering
Replace the hardcoded list of search paths in the 'No skills found' message with a dynamic loop over config.searchPaths. This ensures output always reflects the actual discovery locations including agent-specific directories. Fixes CodeRabbit minor issue: stale hardcoded paths
Use the homedir() function from the 'os' module instead of process.env.HOME to align test path resolution with production code. This prevents environment-dependent test failures. Fixes CodeRabbit minor issue: environment-dependent path resolution
Change maxDepth fallback from || (falsy) to ?? (nullish) so that an explicit maxDepth: 0 is respected instead of being replaced with 3. Fixes CodeRabbit minor issue: falsy-coalescing ignored explicit 0
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.
Description
Fix skill discovery so
skills listcan find skills installed viaskills installacross canonical storage (~/.skills,./.skills) and agent-specific directories. Also addssource paths to
skills listoutput for easier inspection, and updates docs/tests accordingly.Type of Change
Testing
npm run buildpassesskills --helpChecklist
Fixes #15
Summary by CodeRabbit
New Features
Tests
Documentation