feat(skill-loader): support skills.paths config for plugin skill discovery#2840
feat(skill-loader): support skills.paths config for plugin skill discovery#2840ventsislav-georgiev wants to merge 7 commits intocode-yeongyu:devfrom
Conversation
…overy Read skills.paths from opencode.json/jsonc and scan configured directories for SKILL.md files. This allows plugins to ship their own skills that are discoverable via the standard config mechanism. Scans project root, .opencode/, and global config paths for the setting. Skills from configured paths get 'config' scope priority (between opencode and project scopes).
|
All contributors have signed the CLA. Thank you! ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
4 issues found across 3 files
Confidence score: 3/5
- There is concrete cross-platform risk in
src/features/opencode-skill-loader/opencode-config-skills-paths.ts: hardcoding~/.config/opencode(severity 8/10, confidence 10/10) can miss the real global config location on Windows and other OS setups. src/features/opencode-skill-loader/loader.tsmay not wire newly discoveredskills.pathsinto command registration, so skills could be found but not become executable commands (user-facing behavior gap).- The remaining findings are test-focused but still meaningful:
src/features/opencode-skill-loader/opencode-config-skills-paths.test.tshas Windows JSONC escaping fragility and environment leakage into real global config, which can cause flaky/platform-dependent failures. - Pay close attention to
src/features/opencode-skill-loader/opencode-config-skills-paths.ts,src/features/opencode-skill-loader/loader.ts, andsrc/features/opencode-skill-loader/opencode-config-skills-paths.test.ts- path resolution and command wiring need validation, and tests should be made platform-safe and isolated.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/opencode-skill-loader/opencode-config-skills-paths.ts">
<violation number="1" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:13">
P1: Custom agent: **Opencode Compatibility**
Do not hardcode `~/.config/opencode` for OpenCode's global configuration directory. Use the shared `getOpenCodeConfigPaths` utility to ensure OS-specific paths (e.g., `%APPDATA%` on Windows, `Library/Application Support` on macOS) and environment variables like `OPENCODE_CONFIG_DIR` are respected, maintaining compatibility with OpenCode.</violation>
</file>
<file name="src/features/opencode-skill-loader/loader.ts">
<violation number="1" location="src/features/opencode-skill-loader/loader.ts:49">
P2: New `skills.paths` discovery is not wired into command registration, so those skills may not become executable commands.</violation>
</file>
<file name="src/features/opencode-skill-loader/opencode-config-skills-paths.test.ts">
<violation number="1" location="src/features/opencode-skill-loader/opencode-config-skills-paths.test.ts:72">
P2: Test is not environment-isolated: it can read real global `~/.config/opencode` config and fail on machines with `skills.paths` configured.</violation>
<violation number="2" location="src/features/opencode-skill-loader/opencode-config-skills-paths.test.ts:101">
P2: Raw Windows path interpolation creates invalid JSONC string escapes, making the JSONC test platform-dependent and likely to fail on Windows.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Use getOpenCodeConfigDir() instead of hardcoded ~/.config/opencode for cross-platform and OPENCODE_CONFIG_DIR env support - Wire loadConfigPathsSkills into command-config-handler for proper command registration of skills.paths entries - Isolate tests from real global config via OPENCODE_CONFIG_DIR env - Fix Windows path escaping in JSONC test using JSON.stringify
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/plugin-handlers/command-config-handler.ts">
<violation number="1" location="src/plugin-handlers/command-config-handler.ts:63">
P2: configPathsSkills is merged too early, giving it lower precedence than user/project/opencode skills; this conflicts with the documented priority (config paths should outrank project/user) and can cause config-path skills to be overridden.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Move configPathsSkills after project commands/skills in the spread merge so it has higher precedence, matching the documented priority: opencode-project > opencode > config-paths > project > user.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/plugin-handlers/command-config-handler.ts">
<violation number="1">
P1: Spread precedence regression: `systemCommands` now overrides project commands/skills due to reordered merge order.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Move opencodeGlobal(Cmds+Skills) and systemCommands back before project commands, matching dev branch ordering. Fixes precedence regression where systemCommands was overriding project commands/skills.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/plugin-handlers/command-config-handler.ts">
<violation number="1">
P2: Spread order now gives project/config-path commands higher precedence than opencode global/system, contradicting documented priority where opencode should override config paths/project. This behavior change can cause global/system commands to be silently overridden on name collisions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@ventsislav-georgiev I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 4 files
Confidence score: 3/5
- There is some risk here because the config resolution logic in
src/features/opencode-skill-loader/opencode-config-skills-paths.tsdiverges from OpenCode behavior, which can load the wrongskills.pathsand affect skill discovery. - The current implementation returns only the first
skills.pathslist and ignores later config layers, so additive/merged configs may silently drop entries and change runtime behavior. - Relative
skills.pathsfrom global configs are resolved against the project directory, which can point to incorrect locations when global configs are used. - Pay close attention to
src/features/opencode-skill-loader/opencode-config-skills-paths.ts- config precedence/merging and relative path resolution.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/opencode-skill-loader/opencode-config-skills-paths.ts">
<violation number="1" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:1">
P1: Custom agent: **Opencode Compatibility**
Config search precedence is reversed relative to native OpenCode, which can load the wrong `skills.paths` when both project and `.opencode` configs exist.</violation>
<violation number="2" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:35">
P2: Custom agent: **Opencode Compatibility**
Returns the first `skills.paths` list and ignores the rest of the config layers, which conflicts with OpenCode’s merged/additive config behavior.</violation>
<violation number="3" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:38">
P2: Relative `skills.paths` are resolved against the project directory even when they come from global config files, so global relative entries point to the wrong location.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/opencode-skill-loader/opencode-config-skills-paths-precedence.test.ts">
<violation number="1" location="src/features/opencode-skill-loader/opencode-config-skills-paths-precedence.test.ts:31">
P2: Incorrect expected path for `.opencode`-scoped relative `skills.paths`; it should resolve under `.opencode`, not the workspace root.</violation>
</file>
<file name="src/features/opencode-skill-loader/opencode-config-skills-paths.ts">
<violation number="1" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:21">
P2: Preferring only one config file per directory means valid `skills.paths` in `opencode.json` are skipped whenever `opencode.jsonc` exists but has no usable paths.</violation>
<violation number="2" location="src/features/opencode-skill-loader/opencode-config-skills-paths.ts:61">
P1: Relative paths from `.opencode/opencode.json` are resolved against `.opencode/` instead of the project root, causing configured skill directories to be missed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Additive changes for skill discovery via config. Robust path resolution, comprehensive tests for precedence/JSONC, and no modifications to existing core logic ensure zero regressions.
Summary
skills.pathsfromopencode.json/opencode.jsoncconfig files.opencode/, and global~/.config/opencode/config pathsProblem
OpenCode plugins can register tools via the plugin API, but there's no mechanism for plugins to ship discoverable skills (SKILL.md files). Users had to manually copy or symlink skill files into
.opencode/skills/directories.OpenCode natively supports
skills.pathsin config, but oh-my-opencode's skill loader override didn't read this setting, causing a gap where configured skill paths were silently ignored.Solution
Added
readOpenCodeSkillsPaths()that readsskills.pathsfrom opencode config files (checking project root,.opencode/, and global config in precedence order). Wired into bothdiscoverSkills()anddiscoverAllSkills()with "config" scope priority between opencode and project scopes.Config example
{ "skills": { "paths": [ "/absolute/path/to/plugin/skills", "relative/path/to/skills", "~/home-relative/path" ] } }Changes
opencode-config-skills-paths.ts— reads and resolvesskills.pathsfrom configloader.ts— addeddiscoverConfigPathsSkills(), wired into both discovery flowsopencode-config-skills-paths.test.ts— 7 tests covering absolute/relative/JSONC/filtering/integrationTesting
/skillscommandSummary by cubic
Adds support for
skills.pathsin OpenCode config so pluginSKILL.mdfolders are auto-discovered with the right precedence and registered as commands automatically.skills.pathsfromopencode.jsonc(preferred) andopencode.jsonin the project root,.opencode/, and the global config dir (respectsOPENCODE_CONFIG_DIR).~/paths;.opencodeentries resolve against the project root, global entries against the global config dir; skips missing dirs.configscope with priority: opencode-project > opencode > config paths > project > user.discoverConfigPathsSkills(),loadConfigPathsSkills(), andreadOpenCodeSkillsPaths(); integrated intodiscoverSkills,discoverAllSkills, and the command config handler.Written for commit 549aaf7. Summary will update on new commits.