feat(agents): add skill-to-subagent converter command#34
Conversation
Add `skillkit agent from-skill` command to convert SkillKit skills into Claude Code native subagent format (.md files in .claude/agents/). Features: - Reference mode (default): generates subagent with `skills: [skill-name]` - Inline mode (--inline): embeds full skill content in system prompt - Options: --model, --permission, --global, --output, --dry-run New files: - packages/core/src/agents/skill-converter.ts - packages/core/src/agents/__tests__/skill-converter.test.ts (23 tests) Closes #22
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an AgentFromSkill CLI command and a skill-to-subagent conversion library, plus tests and type updates, enabling generation of Claude Code subagent markdown from SkillKit skills with inline/reference modes and frontmatter-driven metadata. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Core
participant FS
User->>CLI: run `skillkit agent from-skill --skillName=...`
CLI->>Core: discoverSkills()
Core-->>CLI: skills list
CLI->>Core: validate & select skill
CLI->>FS: read skill file
FS-->>CLI: skill content
CLI->>Core: generateSubagentFromSkill(skill, content, options)
Core->>Core: skillToSubagent() (parse frontmatter, choose inline/reference, merge tools)
Core->>Core: generateSubagentMarkdown()
Core-->>CLI: markdown string
CLI->>FS: write .md (or dry-run)
FS-->>CLI: write success
CLI-->>User: success message / usage guidance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 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 |
| return allowedToolsStr | ||
| .split(',') | ||
| .map(t => t.trim()) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
🔴 parseAllowedTools only splits by comma, missing space-separated format support
The parseAllowedTools function in skill-converter.ts only splits the allowed-tools string by comma (,), but the codebase supports both comma-separated and space-separated formats for this field.
Click to expand
Evidence of space-separated format usage
The test suite and e2e tests show that allowed-tools can be space-separated:
tests/skills.test.ts:428:allowed-tools: Bash(git:*) Read Writee2e/core-commands.e2e.test.ts:195:allowed-tools: Bash(npm:*) Read Write Edit
The bug
In packages/core/src/agents/skill-converter.ts:90-93:
return allowedToolsStr
.split(',')
.map(t => t.trim())
.filter(Boolean);This only splits by comma. When a skill uses space-separated tools like Bash(git:*) Read Write, the function returns ['Bash(git:*) Read Write'] (a single element array) instead of ['Bash(git:*)', 'Read', 'Write'].
Expected behavior
The function should handle both comma-separated and space-separated formats, similar to how packages/core/src/agents/parser.ts:187 handles it:
allowedTools = fm.allowedTools.split(/[,\n]/).map(t => t.trim()).filter(Boolean);Or even better, split by comma, newline, or whitespace to handle all cases.
Impact
When converting skills that use space-separated allowed-tools to subagents, the generated agent will have incorrect tool restrictions - a single malformed tool name instead of multiple valid tools.
Recommendation: Change the split pattern to handle both comma-separated and space-separated formats. Consider using a regex like /[,\s]+/ or /[,\n\s]+/ to split by comma, newline, or whitespace, while being careful to preserve tool arguments like Bash(git:*).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/cli/src/commands/agent.ts`:
- Around line 870-876: The CLI option help for permission is out of sync with
accepted values; update the Option.String description for permission (the
permission = Option.String(...) declaration) to list all valid modes including
"inherit" and "bypassPermissions" in addition to "default, plan, auto-edit,
full-auto" so the help text matches actual supported values; also scan the model
= Option.String(...) declaration to confirm its description lists the actual
supported model names and update if any additional models exist.
- Around line 936-942: The filename construction allows a malicious --output to
escape the agents directory; sanitize and validate the output stem before
joining into outputPath: when this.output is set, ensure you extract only a safe
basename/stem (no path separators or absolute paths, reject or strip leading ../
or /, allow only a limited character set) and append the .md extension, then
build outputPath from targetDir and that validated filename; update the logic
around the filename variable (used with generateSubagentFromSkill, targetDir,
outputPath) to perform this validation/sanitization and error out or fallback if
the provided output is unsafe.
In `@packages/core/src/agents/skill-converter.ts`:
- Around line 77-94: parseAllowedTools currently assumes
frontmatter['allowed-tools'] is a string and calls .split(','), which fails for
YAML-parsed arrays; update parseAllowedTools (function name: parseAllowedTools,
types: SkillFrontmatter/frontmatter) to accept both string and string[]: if
optionTools present return it; then check if allowedToolsStr is an array
(Array.isArray) — normalize by mapping/trimming/filtering and return; if it's a
string, split by ',' then trim/filter as before; ensure the function returns
string[] | undefined in all branches.
🧹 Nitpick comments (2)
packages/core/src/agents/skill-converter.ts (1)
148-195: Escape list items and author/tags in YAML.
Tools/tags/author values with special characters can break frontmatter. ReuseescapeYamlStringfor list items and scalar fields.♻️ Suggested hardening
if (canonical.author) { - lines.push(`author: ${canonical.author}`); + lines.push(`author: ${escapeYamlString(canonical.author)}`); } if (canonical.tags && canonical.tags.length > 0) { - lines.push(`tags: [${canonical.tags.join(', ')}]`); + lines.push(`tags: [${canonical.tags.map(escapeYamlString).join(', ')}]`); } if (canonical.userInvocable !== undefined) { lines.push(`user-invocable: ${canonical.userInvocable}`); } lines.push('---', '', canonical.content); return lines.join('\n'); } function appendYamlList(lines: string[], key: string, items?: string[]): void { if (!items || items.length === 0) return; lines.push(`${key}:`); for (const item of items) { - lines.push(` - ${item}`); + lines.push(` - ${escapeYamlString(item)}`); } }packages/cli/src/commands/agent.ts (1)
953-959: Consider warning on same-name agents across scopes.
If a global agent exists while creating a project agent (or vice versa), users can end up with ambiguous behavior. A warning aligns with the scope-handling objective.💡 Optional warning
const outputPath = join(targetDir, filename); + const otherDir = this.global + ? join(process.cwd(), '.claude', 'agents') + : join(homedir(), '.claude', 'agents'); + const otherPath = join(otherDir, filename); + if (existsSync(otherPath)) { + console.log( + chalk.yellow(`Note: a ${this.global ? 'project' : 'global'} agent with the same name exists at ${otherPath}`) + ); + } + if (this.dryRun) { console.log(chalk.cyan('Preview (dry run):\n'));
- Update CLI option descriptions to list all valid values (model: inherit, permission: bypassPermissions) - Add filename sanitization to prevent path traversal attacks via --output - Fix parseAllowedTools to handle both string and YAML array formats - Support space-separated tool lists in allowed-tools - Add tests for array and space-separated formats
Add
skillkit agent from-skillcommand to convert SkillKit skills into Claude Code native subagent format (.md files in .claude/agents/).Features:
skills: [skill-name]New files:
Closes #22
Summary by CodeRabbit
New Features
Public API
Tests
✏️ Tip: You can customize this high-level summary in your review settings.