Surface provider skills in the slash command menu#759
Merged
Conversation
Provider skills were hidden behind provider-specific filesystem rules. That made the backend and UI unable to offer one discovery path for skills. Add a normalized skills contract, provider service, and provider skills API. Keep provider-specific lookup rules inside adapters so routes and UI stay generic. Claude needs plugin handling because enabled plugins resolve through installed_plugins.json. Plugin folders can expose commands or skills, so Claude scans both forms. Claude plugin commands are namespaced to avoid collisions with user and project skills. Codex, Gemini, and Cursor adapters map their expected skill roots into the same contract. The slash menu now shows skills beside built-in and custom commands for discovery. The menu avoids mid-message activation, duplicate rows, loose namespace matches, and input overlap. Provider tests cover discovery locations and Claude plugin edge cases.
Contributor
📝 WalkthroughWalkthroughAdds provider-skill types and utilities, a shared SkillsProvider base, provider-specific skill implementations (Claude/Codex/Cursor/Gemini), a service + GET /:provider/skills route and index export, frontend hook and CommandMenu integration for skill commands, supporting frontmatter/command-parser updates, tests, and documentation. ChangesProvider Skills Discovery & Integration
Possibly related PRs
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/chat/hooks/useSlashCommands.ts (1)
197-236:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let provider-skills fetch failures blank the whole slash menu.
/api/providers/${provider}/skillsis additive here, but any thrown error from that request currently lands in the outercatchand clears even the built-in/custom commands that were already fetched successfully. A flaky skills response should degrade to[], not take down the entire menu.Suggested fix
- const skillsResponse = await authenticatedFetch( - `/api/providers/${encodeURIComponent(provider)}/skills${skillsParams.toString() ? `?${skillsParams.toString()}` : ''}`, - ); - const skillsData = skillsResponse.ok - ? ((await skillsResponse.json()) as ProviderSkillsResponse) - : null; + let skillsData: ProviderSkillsResponse | null = null; + try { + const skillsResponse = await authenticatedFetch( + `/api/providers/${encodeURIComponent(provider)}/skills${skillsParams.toString() ? `?${skillsParams.toString()}` : ''}`, + ); + skillsData = skillsResponse.ok + ? ((await skillsResponse.json()) as ProviderSkillsResponse) + : null; + } catch (error) { + console.warn('Error fetching provider skills:', error); + } const skillCommands = dedupeProviderSkills(skillsData?.data?.skills || []) .map(mapSkillToSlashCommand);🤖 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 `@src/components/chat/hooks/useSlashCommands.ts` around lines 197 - 236, The skills fetch currently lives inside the main try and any error from authenticatedFetch(`/api/providers/${encodeURIComponent(provider)}/skills...`) bubbles out and clears all commands; isolate that fetch so failures degrade to an empty skills list instead of aborting the whole flow: wrap the provider skills request/parse (the logic around skillsResponse, skillsData, skillCommands and dedupeProviderSkills/mapSkillToSlashCommand) in its own try/catch (or handle non-ok responses) and set skillCommands = [] on error, then continue building allCommands and calling setSlashCommands; keep the rest of the logic (sorting, readCommandHistory, setSlashCommands) unchanged so built-in/custom commands still show when the skills endpoint is flaky.
🧹 Nitpick comments (2)
server/modules/providers/list/claude/claude-skills.provider.ts (2)
146-161: ⚡ Quick winPlugins with both
commands/andskills/only discover commands.The
continuestatement at line 150 skipsskills/discovery whencommands/exists. This precedence behavior means a plugin containing both folders will only surface commands, not skills. If this is intentional, consider adding a comment explaining the design choice. If not, both folders should be processed.📝 Option 1: Add clarifying comment
const commandsPath = path.join(pluginFolder, 'commands'); if (await pathExistsAsDirectory(commandsPath)) { + // Only process commands/ if present; skills/ is ignored when commands/ exists skills.push( ...(await this.listPluginCommandSkills(commandsPath, pluginId, pluginName)), ); continue; }♻️ Option 2: Process both folders
const commandsPath = path.join(pluginFolder, 'commands'); if (await pathExistsAsDirectory(commandsPath)) { skills.push( ...(await this.listPluginCommandSkills(commandsPath, pluginId, pluginName)), ); - continue; } const skillsPath = path.join(pluginFolder, 'skills'); - if (!(await pathExistsAsDirectory(skillsPath))) { - continue; - } - - skills.push( - ...(await this.listPluginSkillMarkdowns(pluginFolder, pluginId, pluginName)), - ); + if (await pathExistsAsDirectory(skillsPath)) { + skills.push( + ...(await this.listPluginSkillMarkdowns(pluginFolder, pluginId, pluginName)), + ); + }🤖 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 `@server/modules/providers/list/claude/claude-skills.provider.ts` around lines 146 - 161, The current loop uses a continue after discovering commands which prevents also discovering skills when a plugin has both folders; update the logic in the loop around pathExistsAsDirectory(commandsPath), listPluginCommandSkills, listPluginSkillMarkdowns, commandsPath and skillsPath so both folders are processed: either remove the continue and let execution fall through to check skillsPath and call listPluginSkillMarkdowns (ensuring duplicates aren’t added to skills) or explicitly call both listPluginCommandSkills(...) and listPluginSkillMarkdowns(...) when both paths exist; if the original precedence was intentional, add a clear comment explaining why the continue short-circuits discovery.
195-197: ⚡ Quick winConsider logging errors for debugging plugin discovery issues.
The error handlers at lines 195-197, 199-201, and 242-244 silently ignore malformed or unreadable plugin files to ensure resilient discovery. While this prevents one bad file from blocking others, it may make troubleshooting plugin configuration issues difficult for users. Consider logging these errors at debug or warning level.
📊 Proposed enhancement to add logging
For example, at lines 195-197:
- } catch { + } catch (error) { // Malformed command markdown should not block sibling plugin commands. + console.debug(`Skipping malformed command file ${sourcePath}:`, error); }Apply similar changes to other catch blocks for improved observability.
Also applies to: 199-201, 242-244
🤖 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 `@server/modules/providers/list/claude/claude-skills.provider.ts` around lines 195 - 197, The catch blocks that silently swallow errors (the one with the comment "Malformed command markdown should not block sibling plugin commands." and the other similar catches at the locations you flagged) should log the caught error at debug or warn level to aid troubleshooting; update the error handlers inside ClaudeSkillsProvider (the plugin discovery/loading routine) to call the module logger (e.g., this.logger.debug or this.logger.warn) and include the error object plus contextual details like the plugin filename or command name, then continue to swallow the error as before so discovery remains resilient—apply the same change to the other two empty catch blocks you noted (the ones handling unreadable/malformed plugin files).
🤖 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 `@server/modules/providers/list/claude/claude-skills.provider.ts`:
- Around line 23-26: getClaudePluginName currently returns an empty string for
inputs like '' or '@', leading to malformed command strings; update
getClaudePluginName to validate pluginId: if pluginId is falsy, equals '@', or
the part before '@' is empty, throw a clear Error (or return null) instead of
returning an empty string, and update the callers that construct commands (the
code that assembles "/${pluginName}:${commandName}") to either catch this error
or skip the plugin prefix when the function returns null so no "/:command" is
emitted. Ensure you modify the function named getClaudePluginName and the
command-construction sites to handle the new validation behavior.
In `@server/modules/providers/list/codex/codex-skills.provider.ts`:
- Around line 87-91: The hardcoded admin path passed to addUniqueSource
(sources, seenRootDirs, { rootDir: path.join('/etc','codex','skills'), scope:
'admin', ... }) is platform-unsafe; change it to derive adminDir from an
environment variable with a default (e.g. adminDir =
process.env.CODEX_ADMIN_SKILLS_DIR ?? path.join('/etc','codex','skills')) and
only call addUniqueSource when either process.platform !== 'win32' or the env
var is explicitly set (so Windows users must opt-in), updating the
addUniqueSource invocation to use adminDir and preserving scope/commandPrefix.
In `@src/components/chat/hooks/useSlashCommands.ts`:
- Around line 313-328: selectCommandFromKeyboard currently inserts every command
into the input before executing, causing keyboard Enter behavior to differ from
mouse clicks and leaving stale text on failures; change it so only skill
commands call insertCommandIntoInput(command) (and return early), while
non-skill commands call onExecuteCommand(command) directly and then trigger the
same menu reset/close logic used by the click selection path so the menu is
cleared even if execution fails; keep the existing promise handling
(isPromiseLike) for onExecuteCommand results.
---
Outside diff comments:
In `@src/components/chat/hooks/useSlashCommands.ts`:
- Around line 197-236: The skills fetch currently lives inside the main try and
any error from
authenticatedFetch(`/api/providers/${encodeURIComponent(provider)}/skills...`)
bubbles out and clears all commands; isolate that fetch so failures degrade to
an empty skills list instead of aborting the whole flow: wrap the provider
skills request/parse (the logic around skillsResponse, skillsData, skillCommands
and dedupeProviderSkills/mapSkillToSlashCommand) in its own try/catch (or handle
non-ok responses) and set skillCommands = [] on error, then continue building
allCommands and calling setSlashCommands; keep the rest of the logic (sorting,
readCommandHistory, setSlashCommands) unchanged so built-in/custom commands
still show when the skills endpoint is flaky.
---
Nitpick comments:
In `@server/modules/providers/list/claude/claude-skills.provider.ts`:
- Around line 146-161: The current loop uses a continue after discovering
commands which prevents also discovering skills when a plugin has both folders;
update the logic in the loop around pathExistsAsDirectory(commandsPath),
listPluginCommandSkills, listPluginSkillMarkdowns, commandsPath and skillsPath
so both folders are processed: either remove the continue and let execution fall
through to check skillsPath and call listPluginSkillMarkdowns (ensuring
duplicates aren’t added to skills) or explicitly call both
listPluginCommandSkills(...) and listPluginSkillMarkdowns(...) when both paths
exist; if the original precedence was intentional, add a clear comment
explaining why the continue short-circuits discovery.
- Around line 195-197: The catch blocks that silently swallow errors (the one
with the comment "Malformed command markdown should not block sibling plugin
commands." and the other similar catches at the locations you flagged) should
log the caught error at debug or warn level to aid troubleshooting; update the
error handlers inside ClaudeSkillsProvider (the plugin discovery/loading
routine) to call the module logger (e.g., this.logger.debug or this.logger.warn)
and include the error object plus contextual details like the plugin filename or
command name, then continue to swallow the error as before so discovery remains
resilient—apply the same change to the other two empty catch blocks you noted
(the ones handling unreadable/malformed plugin files).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 94323d6d-8b30-4d89-a29c-ba761e18c57d
📒 Files selected for processing (20)
server/modules/providers/index.tsserver/modules/providers/list/claude/claude-skills.provider.tsserver/modules/providers/list/claude/claude.provider.tsserver/modules/providers/list/codex/codex-skills.provider.tsserver/modules/providers/list/codex/codex.provider.tsserver/modules/providers/list/cursor/cursor-skills.provider.tsserver/modules/providers/list/cursor/cursor.provider.tsserver/modules/providers/list/gemini/gemini-skills.provider.tsserver/modules/providers/list/gemini/gemini.provider.tsserver/modules/providers/provider.routes.tsserver/modules/providers/services/skills.service.tsserver/modules/providers/shared/base/abstract.provider.tsserver/modules/providers/shared/skills/skills.provider.tsserver/modules/providers/tests/skills.test.tsserver/shared/interfaces.tsserver/shared/types.tsserver/shared/utils.tssrc/components/chat/hooks/useChatComposerState.tssrc/components/chat/hooks/useSlashCommands.tssrc/components/chat/view/subcomponents/CommandMenu.tsx
Claude plugin ids come from local settings and installed plugin metadata. Invalid ids such as empty strings or @ should not become command namespaces. Skip plugin folders when no safe plugin name can be derived. This prevents malformed slash commands like /:command from reaching the UI. Add regression coverage for empty and @ plugin ids. Keyboard selection in the slash menu should match mouse selection. Only skills are inserted into the composer because they are provider invocations. Built-in and custom commands execute directly and close the menu on success or failure.
This was
linked to
issues
May 11, 2026
Move frontmatter parsing into server/shared/frontmatter.ts so every backend caller uses the same gray-matter configuration instead of importing gray-matter directly. The goal is to keep executable JS and JSON frontmatter engines disabled for all markdown discovered from the filesystem, not only command routes. Provider skills and shared skill metadata now go through parseFrontMatter too. That closes the gap where plugin or provider markdown could regain default gray-matter behavior simply because it lived outside the original command path. Classify the new parser in backend boundaries so modules can depend on the safe shared API without reaching into legacy utility paths.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/modules/providers/README.md (1)
314-321: ⚡ Quick winAdd
npm run buildto the validation checklist.The checklist currently includes lint/typecheck but not the required build verification step.
Suggested doc patch
## Validation After adding or changing a provider, run the relevant checks: ```bash +npm run build npx eslint server/modules/providers/**/*.ts server/shared/types.ts server/shared/interfaces.ts npx tsc --noEmit -p server/tsconfig.json</details> As per coding guidelines, "Ensure the build passes before PR (run npm run build)." <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@server/modules/providers/README.mdaround lines 314 - 321, Update the
"Validation" checklist under the Validation heading to include running the
project build; specifically add the command npm run build before or alongside
the existing npx eslint ... and npx tsc --noEmit -p server/tsconfig.json steps
so PR authors run the build verification as part of provider changes.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@server/modules/providers/README.md:
- Line 1: Remove the UTF-8 BOM at the start of the README by editing the file
content that begins with the heading "Providers Module Guide" so the first
character is the '#' character (no invisible BOM prefix); ensure the file's
encoding is UTF-8 without BOM and save the change so diffs and tooling no longer
show the BOM before the heading.
Nitpick comments:
In@server/modules/providers/README.md:
- Around line 314-321: Update the "Validation" checklist under the Validation
heading to include running the project build; specifically add the command npm
run build before or alongside the existing npx eslint ... and npx tsc --noEmit
-p server/tsconfig.json steps so PR authors run the build verification as part
of provider changes.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Repository UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `186d7bd4-5390-45f8-b7a7-7e95ce9855c3` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between aabf331e91fb78e9a4104f4893decc19c1c40cb5 and 7e92de7cb7d86521198295356014580a07be8b8b. </details> <details> <summary>📒 Files selected for processing (7)</summary> * `eslint.config.js` * `server/modules/providers/README.md` * `server/modules/providers/list/claude/claude-skills.provider.ts` * `server/routes/commands.js` * `server/shared/frontmatter.ts` * `server/shared/utils.ts` * `server/utils/commandParser.js` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * server/modules/providers/list/claude/claude-skills.provider.ts </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @@ -0,0 +1,346 @@ | |||
| # Providers Module Guide | |||
Contributor
There was a problem hiding this comment.
Remove the BOM at file start.
Line 1 includes a UTF-8 BOM character before the heading, which can cause noisy diffs/tooling inconsistencies.
🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Ensure spelling is correct
Context: # Providers Module Guide This file documents the current provider...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@server/modules/providers/README.md` at line 1, Remove the UTF-8 BOM at the
start of the README by editing the file content that begins with the heading
"Providers Module Guide" so the first character is the '#' character (no
invisible BOM prefix); ensure the file's encoding is UTF-8 without BOM and save
the change so diffs and tooling no longer show the BOM before the heading.
viper151
approved these changes
May 12, 2026
stud20
pushed a commit
to stud20/claudecodeui
that referenced
this pull request
May 25, 2026
* feat(providers): surface skills in slash command menu Provider skills were hidden behind provider-specific filesystem rules. That made the backend and UI unable to offer one discovery path for skills. Add a normalized skills contract, provider service, and provider skills API. Keep provider-specific lookup rules inside adapters so routes and UI stay generic. Claude needs plugin handling because enabled plugins resolve through installed_plugins.json. Plugin folders can expose commands or skills, so Claude scans both forms. Claude plugin commands are namespaced to avoid collisions with user and project skills. Codex, Gemini, and Cursor adapters map their expected skill roots into the same contract. The slash menu now shows skills beside built-in and custom commands for discovery. The menu avoids mid-message activation, duplicate rows, loose namespace matches, and input overlap. Provider tests cover discovery locations and Claude plugin edge cases. * fix(providers): guard invalid skill command namespaces Claude plugin ids come from local settings and installed plugin metadata. Invalid ids such as empty strings or @ should not become command namespaces. Skip plugin folders when no safe plugin name can be derived. This prevents malformed slash commands like /:command from reaching the UI. Add regression coverage for empty and @ plugin ids. Keyboard selection in the slash menu should match mouse selection. Only skills are inserted into the composer because they are provider invocations. Built-in and custom commands execute directly and close the menu on success or failure. * fix(security): centralize safe frontmatter parsing Move frontmatter parsing into server/shared/frontmatter.ts so every backend caller uses the same gray-matter configuration instead of importing gray-matter directly. The goal is to keep executable JS and JSON frontmatter engines disabled for all markdown discovered from the filesystem, not only command routes. Provider skills and shared skill metadata now go through parseFrontMatter too. That closes the gap where plugin or provider markdown could regain default gray-matter behavior simply because it lived outside the original command path. Classify the new parser in backend boundaries so modules can depend on the safe shared API without reaching into legacy utility paths. * feat(providers): add comprehensive guide for provider module setup and usage
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.
Closes #456
Closes #494
Summary
This PR adds a unified provider skills system and exposes skills in the chat slash command popup. It lets the backend fetch skills from Claude, Codex, Gemini, and Cursor using each provider’s native filesystem rules, then normalizes them into one API response that the frontend can render as slash commands.
Why
Skills are stored differently across providers, and Claude plugin skills/commands have extra namespace and install-path rules. Without a normalized backend contract, the UI would need provider-specific filesystem knowledge or users would need to remember commands manually.
This also improves the slash command UX so skills feel discoverable and safe to use: duplicates are removed, namespaced filtering is precise,
/only opens commands at the start of input, and the popup no longer covers what the user is typing.Backend Changes
SKILL.mdfiles and parsing frontmatter.skills.service.tsto keep route logic thin.GET /providers/:provider/skills.SkillsProviderbase logic for providers withSKILL.mdfolder discovery.Provider Discovery
settings.json.installed_plugins.json.installPath, scans direct child folders, and reads.claude-plugin/plugin.jsonwhen present.commands/*.mdfirst when available.skills/**/SKILL.mdwhen nocommandsfolder exists./PluginName:command.$skill-namecommands..agentsskill locations.Frontend Changes
useSlashCommands.tsnow fetches provider skills and merges them with built-in/custom commands./is the first input character./Notion:findonly shows matching Notion commands.CommandMenu.tsxwas redesigned with icons, section counts, better selected states, truncation, and improved placement above the composer.Tests
Key Files
abstract.provider.tsskills.provider.tsskills.service.tsprovider.routes.tsclaude-skills.provider.tscodex-skills.provider.tsgemini-skills.provider.tscursor-skills.provider.tsskills.test.tsinterfaces.tstypes.tsutils.tsuseSlashCommands.tsuseChatComposerState.tsCommandMenu.tsxOut of Scope
Summary by CodeRabbit