Migrate to SKILL.md agent skills standard#7
Conversation
Replace flat .md files with proper SKILL.md format in skill directories.
Source of truth is ~/.clkx/ with skills/{name}/SKILL.md structure.
- Agent enum: skillsDir replaces skillDir/skillExtension/useSymlinks
- SkillGenerator writes SKILL.md files to ~/.clkx/skills/{name}/
- Agent defs written to ~/.clkx/agents/, symlinked to project dirs
- Copilot gets copies instead of symlinks (JetBrains limitation)
- Project-level: symlinks from .claude/skills/, .github/skills/, etc.
- .agents/skills/ shared path for cross-tool compatibility
- All gitignores use opsx* srcx* wrkx* patterns
- ~/.clkx/.gitignore ignores everything with *
- CleanTask removes from all new locations
- All 465 tests pass
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 37 minutes and 59 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR refactors the skill and agent distribution architecture: skills now organize in per-skill subdirectories ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt (1)
61-63:⚠️ Potential issue | 🟡 MinorSkill count logic doesn't match directory-based layout.
The count uses
it.name.endsWith(".md"), but with the new directory-based skill layout (~/.clkx/skills/{name}/SKILL.md), skills are directories, not.mdfiles at the sourceDir level. This will always report 0 skills.Compare with
CleanTask.cleanSourceSkills()which correctly counts directories:sourceDir.listFiles().orEmpty().count { it.isDirectory }.🐛 Proposed fix
- val fileCount = sourceDir.listFiles()?.count { it.name.endsWith(".md") } ?: 0 + val fileCount = sourceDir.listFiles()?.count { it.isDirectory } ?: 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt` around lines 61 - 63, The skill count in SyncTask (variable fileCount) incorrectly counts .md files in sourceDir but skills are directories; update the logic to mirror CleanTask.cleanSourceSkills by using sourceDir.listFiles().orEmpty().count { it.isDirectory } (referencing SyncTask, fileCount and sourceDir) so the logged skill count reflects directory-based skills; keep agentSummary generation unchanged.
🧹 Nitpick comments (3)
src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt (1)
88-104: Consider also cleaning broken symlinks during sync.
SyncTask.cleanAgentSymlinks()only removes symlinks that point intosourceDir, but doesn't handle broken symlinks. In contrast,CleanTask.cleanSymlinksInDir()(seesrc/main/kotlin/zone/clanker/opsx/task/CleanTask.kt:65-69) filters byisOpsxSymlink(it, sourceDir) || isBrokenSymlink(it).If a skill is removed between syncs, its symlink becomes broken and would persist across subsequent sync operations.
♻️ Proposed fix to also remove broken symlinks
private fun cleanAgentSymlinks( root: File, sourceDir: File, ) { // Clean ALL agent dirs, not just configured ones — removes stale symlinks from removed agents val dirs = (Agent.allSkillsDirs + SkillGenerator.SHARED_SKILLS_DIR) .map { File(root, it) } .distinct() dirs.filter { it.exists() }.forEach { dir -> dir .listFiles() .orEmpty() - .filter { isOpsxSymlink(it, sourceDir) } + .filter { isOpsxSymlink(it, sourceDir) || isBrokenSymlink(it) } .forEach { it.delete() } } } + + private fun isBrokenSymlink(file: File): Boolean { + val path = file.toPath() + return Files.isSymbolicLink(path) && !Files.exists(path) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt` around lines 88 - 104, cleanAgentSymlinks currently only deletes symlinks that point into sourceDir, so broken symlinks persist; update SyncTask.cleanAgentSymlinks to also remove broken symlinks by changing the filter to include isBrokenSymlink(it) in addition to isOpsxSymlink(it, sourceDir) (use the same isBrokenSymlink helper used by CleanTask), ensuring the loop over dirs (built from Agent.allSkillsDirs + SkillGenerator.SHARED_SKILLS_DIR) deletes entries that are either opsx symlinks or broken.src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt (2)
92-106: Silent failure increateSymlinkmay hide issues.When
Files.exists(link)is true but it's not a symlink (line 101-102), the function silently returns without creating the symlink or logging a warning. Additionally,runCatchingswallows all exceptions. This could make debugging difficult if symlink creation fails for unexpected reasons.Consider logging a warning when skipping due to existing non-symlink files, or when
createSymbolicLinkthrows.♻️ Optional: Add logging for transparency
private fun createSymlink( target: File, source: File, ) { val link = target.toPath() val sourcePath = source.toPath() - runCatching { - if (Files.isSymbolicLink(link)) { - Files.delete(link) - } else if (Files.exists(link)) { - return@runCatching - } - Files.createSymbolicLink(link, sourcePath) + if (Files.isSymbolicLink(link)) { + Files.delete(link) + } else if (Files.exists(link)) { + // Non-symlink exists; skip to avoid overwriting user data + return } + runCatching { Files.createSymbolicLink(link, sourcePath) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt` around lines 92 - 106, The createSymlink function currently swallows all errors with runCatching and silently returns when a non-symlink file exists; update createSymlink to (1) detect the case Files.exists(link) && !Files.isSymbolicLink(link) and emit a warning (use the class logger or suitable logger) explaining the existing path blocks symlink creation, and (2) replace runCatching with explicit try/catch that logs any exception from Files.createSymbolicLink (including the exception message/stack) and rethrows or returns a failure indicator so callers can react; reference createSymlink, Files.isSymbolicLink, Files.exists, and Files.createSymbolicLink when making the changes.
503-507: Remove the unusedagentparameter instead of deprecating.The
agentparameter is unused (and suppressed), but external non-test callers do not exist. Tests do call this method withagentarguments (lines 343, 350 in SkillGeneratorAgentTest.kt), but the implementation ignores them. Rather than deprecating, simply remove the parameter from both the method signature and the test calls, since this is a utility helper with no external dependents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt` around lines 503 - 507, Remove the unused agent parameter from the generatedDirs function and update its callers in tests: change fun generatedDirs(agent: Agent? = null): List<File> to fun generatedDirs(): List<File> (in SkillGenerator.kt) and remove the agent argument from calls in SkillGeneratorAgentTest (the calls around the previous lines ~343 and ~350); ensure imports and any suppress annotations are cleaned up since the parameter and `@Suppress`("UnusedParameter") are no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt`:
- Around line 204-208: The current primary-agent selection uses
agents.firstOrNull { it.agentDir != null } which makes frontmatter dependent on
the agents list order; instead, choose a deterministic preferred primary (e.g.,
prefer Agent.CLAUDE, then Agent.COPILOT, then any with agentDir) before calling
writeAgentDefinition/buildAgentDefinition so the YAML frontmatter is stable.
Update the selection logic around agents.firstOrNull to explicitly pick
Agent.CLAUDE if present (fallback to Agent.COPILOT, then any agent with non-null
agentDir, then return), so writeAgentDefinition(primaryAgent, homeAgentsDir) and
buildAgentDefinition use a consistent primary agent.
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.kt`:
- Around line 78-84: Wrap the System.setProperty("user.home", ...) manipulation
in a small reusable helper and use it in SkillGeneratorAgentTest around the
SkillGenerator(tempDir, ...).generateAgentDefinitions() calls so the original
property is always restored via try/finally and to avoid cross-test pollution;
e.g., add a withSystemProperty helper (used from SkillGeneratorAgentTest) that
accepts a key, value, and lambda, sets the property, runs the lambda, and
restores the previous value, or alternatively enable Kotest's
IsolationMode.InstancePerLeaf for the test class to ensure per-test JVM property
isolation; replace the direct System.setProperty/restore blocks around
SkillGenerator and the generateAgentDefinitions() calls with calls to this
helper.
---
Outside diff comments:
In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt`:
- Around line 61-63: The skill count in SyncTask (variable fileCount)
incorrectly counts .md files in sourceDir but skills are directories; update the
logic to mirror CleanTask.cleanSourceSkills by using
sourceDir.listFiles().orEmpty().count { it.isDirectory } (referencing SyncTask,
fileCount and sourceDir) so the logged skill count reflects directory-based
skills; keep agentSummary generation unchanged.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt`:
- Around line 92-106: The createSymlink function currently swallows all errors
with runCatching and silently returns when a non-symlink file exists; update
createSymlink to (1) detect the case Files.exists(link) &&
!Files.isSymbolicLink(link) and emit a warning (use the class logger or suitable
logger) explaining the existing path blocks symlink creation, and (2) replace
runCatching with explicit try/catch that logs any exception from
Files.createSymbolicLink (including the exception message/stack) and rethrows or
returns a failure indicator so callers can react; reference createSymlink,
Files.isSymbolicLink, Files.exists, and Files.createSymbolicLink when making the
changes.
- Around line 503-507: Remove the unused agent parameter from the generatedDirs
function and update its callers in tests: change fun generatedDirs(agent: Agent?
= null): List<File> to fun generatedDirs(): List<File> (in SkillGenerator.kt)
and remove the agent argument from calls in SkillGeneratorAgentTest (the calls
around the previous lines ~343 and ~350); ensure imports and any suppress
annotations are cleaned up since the parameter and `@Suppress`("UnusedParameter")
are no longer needed.
In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt`:
- Around line 88-104: cleanAgentSymlinks currently only deletes symlinks that
point into sourceDir, so broken symlinks persist; update
SyncTask.cleanAgentSymlinks to also remove broken symlinks by changing the
filter to include isBrokenSymlink(it) in addition to isOpsxSymlink(it,
sourceDir) (use the same isBrokenSymlink helper used by CleanTask), ensuring the
loop over dirs (built from Agent.allSkillsDirs +
SkillGenerator.SHARED_SKILLS_DIR) deletes entries that are either opsx symlinks
or broken.
🪄 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
Run ID: 914ced95-c734-4b2d-a377-6119e30a6b25
📒 Files selected for processing (10)
src/main/kotlin/zone/clanker/opsx/model/Agent.ktsrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/main/kotlin/zone/clanker/opsx/task/CleanTask.ktsrc/main/kotlin/zone/clanker/opsx/task/SyncTask.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginSyncTest.ktsrc/test/kotlin/zone/clanker/opsx/model/AgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.ktsrc/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/SyncTaskTest.kt
No more nulls in Agent enum. Codex uses AGENTS.md + .agents/ dir. OpenCode uses AGENTS.md + .opencode/agents/ dir. Agent definitions now generated for all 4 platforms.
Primary agent for frontmatter prefers CLAUDE then COPILOT instead of depending on list order. Test helper withHome() wraps System.setProperty in try/finally to prevent cross-test pollution.
1 participant
Summary
Replace flat .md file generation with the Agent Skills Standard SKILL.md format, adopted by Claude Code, Copilot, Codex, OpenCode, and 30+ other tools.
Key changes
~/.clkx/skills/{name}/SKILL.md(source of truth)~/.clkx/agents/{name}.md.claude/skills/,.github/skills/,.codex/skills/,.opencode/skills/.agents/skills/shared path all tools read~/.clkx/.gitignoreignores everythingopsx* srcx* wrkx*gitignore patternsTest plan
./gradlew buildpasses (465 tests, detekt, ktlint, kover)Change proposed via opsx
opsx-propose+opsx-apply(9/12 tasks auto-completed, 3 fixed manually).Summary by CodeRabbit
New Features
Refactor