feat: add custom AI sub-agents support (Phase 19)#8
Conversation
Add full support for custom AI sub-agents (.claude/agents/*.md): Core module (packages/core/src/agents/): - types.ts: Agent types, schemas, AGENT_DISCOVERY_PATHS for 17 AI agents - parser.ts: Parse agent markdown with YAML frontmatter - discovery.ts: Discover agents across all supported agent directories - translator.ts: Translate agents between Claude, Cursor, and universal formats - index.ts: Export all agent functionality CLI commands (skillkit agent): - list: List all installed agents (project + global) - show: Show agent details - create: Create a new agent from template - translate: Translate agents between formats - sync: Sync agents to target AI coding agent - validate: Validate agent definitions Features: - Agent discovery across 17 AI coding agents - Frontmatter support: name, description, model, permissionMode, disallowedTools, allowedTools, hooks, skills, context, tags, author, version - Translation between claude-agent, cursor-agent, and universal formats - 36 new tests (706 total passing) Closes #5
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughIntroduces a comprehensive agent management system to skillkit with CLI commands for creating, discovering, validating, and translating custom AI agents. Adds core agent types, parsing, discovery, and translation modules to support multiple agent formats (claude-agent, cursor-agent, universal) with corresponding test coverage. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant CLI as CLI Layer
participant Discovery as Discovery
participant Parser as Parser
participant Translator as Translator
participant FS as File System
User->>CLI: Run agent command
alt List Agents
CLI->>Discovery: discoverAgents()
Discovery->>FS: Scan .claude/agents dirs
FS-->>Discovery: Agent paths
Discovery->>Parser: parseAgentFile()
Parser->>FS: Read markdown
FS-->>Parser: Frontmatter + Content
Parser-->>Discovery: CustomAgent[]
Discovery-->>CLI: Formatted agent list
else Create Agent
CLI->>FS: Write agent template
FS-->>CLI: Success confirmation
else Translate Agent
CLI->>Discovery: findAgent()
Discovery->>FS: Search directories
FS-->>Discovery: Agent path
Discovery->>Parser: parseAgentFile()
Parser-->>Discovery: CustomAgent
CLI->>Translator: translateAgent()
Translator->>Parser: toCanonicalAgent()
Parser-->>Translator: CanonicalAgent
Translator->>Translator: generateFormat()
Translator->>FS: Write translated file
FS-->>Translator: Success
end
Translator-->>CLI: Result with status
CLI-->>User: Formatted output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
pnpm automatically converts workspace:* to actual versions when publishing, but uses local workspace packages during development and CI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@apps/skillkit/src/cli.ts`:
- Around line 42-49: The import in apps/skillkit/src/cli.ts references
AgentCommand, AgentListCommand, AgentShowCommand, AgentCreateCommand,
AgentTranslateCommand, AgentSyncCommand, and AgentValidateCommand but
`@skillkit/cli` does not export them causing TS2305; either re-export these
symbols from the package entrypoint (or update the package's "exports" map) so
they are available from '@skillkit/cli', or change this import to the exact
module that defines them (e.g., import from the internal file that actually
declares AgentCommand and the other Agent* symbols). Ensure you update the
package index export or the import statement to reference the concrete module
where functions/classes AgentCommand, AgentListCommand, AgentShowCommand,
AgentCreateCommand, AgentTranslateCommand, AgentSyncCommand, and
AgentValidateCommand are declared.
In `@packages/cli/src/commands/agent.ts`:
- Around line 422-424: The split-and-cast of this.agent into targetAgents is
unsafe; mirror the validation used in AgentTranslateCommand by checking each
parsed string against the allowed AgentType values instead of force-casting.
Replace the direct map(a => a.trim() as AgentType) with logic that trims each
entry, verifies it exists in the AgentType set/enum, collects only valid
AgentType entries (or raises a clear error if invalid), and use that validated
array for targetAgents so the code no longer relies on an unchecked cast.
- Around line 315-318: The code casts this.to directly to AgentType in execute()
without validating it, which can make CUSTOM_AGENT_FORMAT_MAP[targetAgent] and
AGENT_DISCOVERY_PATHS[targetAgent] undefined for bad input; fix by validating
the --to value before casting (e.g., check that this.to is one of the allowed
AgentType enum values or call AgentType.parse() via Zod) and return an
error/throw if invalid so subsequent lookups of CUSTOM_AGENT_FORMAT_MAP and
AGENT_DISCOVERY_PATHS use a guaranteed valid targetAgent.
In `@packages/core/src/agents/discovery.ts`:
- Around line 198-217: The global agent search paths are inconsistent between
discoverGlobalAgents and findAgent: update the globalPaths array used in
findAgent (the variable named globalPaths in discovery.ts) to include the same
"~/.config/skillkit/agents" path that discoverGlobalAgents uses so agents
discovered in ~/.config/skillkit/agents can be found by name; ensure the new
path is constructed with join(homedir(), '.config', 'skillkit', 'agents') and
keep the existing checks that look for both a `${name}.md` file and a directory
(parseAgentFile/parseAgentDir).
In `@packages/core/src/agents/parser.ts`:
- Line 355: Replace the inline CommonJS call require('node:fs').statSync with
the ESM-imported statSync: add statSync to the top-level fs import alongside
other fs functions, then change the line that currently declares const stats =
require('node:fs').statSync(agentPath); to use the imported statSync (e.g.,
const stats = statSync(agentPath)); keep the existing agentPath reference
unchanged.
- Around line 244-249: The serializer in parser.ts writes the key as
"allowed-tools" (kebab-case) but the AgentFrontmatter schema expects
"allowedTools" (camelCase), causing re-parsing to miss the field; update the
serialization to use the camelCase key by changing the output label for
canonical.allowedTools to "allowedTools" to match how disallowedTools is
serialized and the AgentFrontmatter type so round-trip parsing populates
allowedTools correctly.
In `@packages/core/src/agents/translator.ts`:
- Around line 123-150: The function parseAgentFromContent currently uses runtime
require() for extractAgentFrontmatter, extractAgentContent and AgentFrontmatter;
replace those require() calls by adding top-level ESM imports for
extractAgentFrontmatter and extractAgentContent from './parser.js' and
AgentFrontmatter from './types.js' (remove the require lines inside
parseAgentFromContent) so the function uses the imported symbols directly;
ensure imports use correct named imports and keep the rest of
parseAgentFromContent logic unchanged.
In `@packages/core/src/agents/types.ts`:
- Around line 63-68: AgentFrontmatter and toCanonicalAgent currently only handle
allowedTools while the generator emits allowed-tools, causing data loss; update
the schema and normalization so both keys are accepted and coerced into the
canonical allowedTools field before constructing a CanonicalAgent. Specifically,
modify AgentFrontmatter parsing (and any zod schema around allowedTools) to
accept a kebab-case alias "allowed-tools" (or accept both z.union/object shape)
and then in toCanonicalAgent normalize/merge the values from "allowed-tools"
into allowedTools (prefer explicit allowedTools when both present) so the
resulting CanonicalAgent always has the tools list under allowedTools. Ensure
any downstream code that reads allowedTools (e.g., validation or generator
tests) uses the normalized field.
🧹 Nitpick comments (4)
packages/core/src/agents/types.ts (1)
239-259: DeriveALL_AGENT_DISCOVERY_PATHSfrom the source map to avoid drift.Keeping a separate hand-maintained list risks omissions when new agents/paths are added.
♻️ Suggested refactor
-export const ALL_AGENT_DISCOVERY_PATHS = [ - 'agents', - '.agents', - '.claude/agents', - '.cursor/agents', - '.codex/agents', - '.gemini/agents', - '.opencode/agents', - '.antigravity/agents', - '.amp/agents', - '.clawdbot/agents', - '.factory/agents', - '.github/agents', - '.github/copilot/agents', - '.goose/agents', - '.kilocode/agents', - '.kiro/agents', - '.roo/agents', - '.trae/agents', - '.windsurf/agents', -]; +export const ALL_AGENT_DISCOVERY_PATHS = Array.from( + new Set(Object.values(AGENT_DISCOVERY_PATHS).flat()) +);packages/core/src/agents/__tests__/discovery.test.ts (2)
22-33: Test setup usesDate.now()which could collide in parallel runs.While unlikely in practice, using
Date.now()alone for temp directory naming could cause collisions if tests run in parallel. Consider using a more robust unique identifier.♻️ Suggested improvement
beforeEach(() => { // Create a temporary test directory - testDir = join(tmpdir(), `agent-test-${Date.now()}`); + testDir = join(tmpdir(), `agent-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); mkdirSync(testDir, { recursive: true }); });
283-314: Consider adding test coverage for disabled agents.The
getAgentStatstest only verifies the enabled path. Since the function tracksdisabledcount, consider adding a test that creates an agent withenabled: falsein its metadata to verify the disabled counting logic works correctly.packages/core/src/agents/translator.ts (1)
375-380: YAML escaping may miss edge cases.The
escapeYamlStringfunction handles common special characters but doesn't escape backslashes in the input string. If the input contains literal backslashes (e.g.,C:\path), the output could produce invalid YAML.♻️ Suggested improvement
function escapeYamlString(str: string): string { if (/[:\{\}\[\],&*#?|\-<>=!%@`]/.test(str) || str.includes('\n')) { - return `"${str.replace(/"/g, '\\"').replace(/\n/g, '\\n')}"`; + return `"${str.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n')}"`; } return str; }
| AgentCommand, | ||
| AgentListCommand, | ||
| AgentShowCommand, | ||
| AgentCreateCommand, | ||
| AgentTranslateCommand, | ||
| AgentSyncCommand, | ||
| AgentValidateCommand, | ||
| } from '@skillkit/cli'; |
There was a problem hiding this comment.
Fix missing public exports in @skillkit/cli (TS2305 build failure).
CI shows Module '"@skillkit/cli"' has no exported member 'AgentCommand', so this import currently breaks the build. Ensure the package entrypoint (and exports map, if used) re‑exports the new Agent* commands or change the import to the module that actually exports them.
🧰 Tools
🪛 GitHub Actions: CI
[error] 42-42: tsc --noEmit failed: TS2305: Module '"@skillkit/cli"' has no exported member 'AgentCommand'.
🤖 Prompt for AI Agents
In `@apps/skillkit/src/cli.ts` around lines 42 - 49, The import in
apps/skillkit/src/cli.ts references AgentCommand, AgentListCommand,
AgentShowCommand, AgentCreateCommand, AgentTranslateCommand, AgentSyncCommand,
and AgentValidateCommand but `@skillkit/cli` does not export them causing TS2305;
either re-export these symbols from the package entrypoint (or update the
package's "exports" map) so they are available from '@skillkit/cli', or change
this import to the exact module that defines them (e.g., import from the
internal file that actually declares AgentCommand and the other Agent* symbols).
Ensure you update the package index export or the import statement to reference
the concrete module where functions/classes AgentCommand, AgentListCommand,
AgentShowCommand, AgentCreateCommand, AgentTranslateCommand, AgentSyncCommand,
and AgentValidateCommand are declared.
| async execute(): Promise<number> { | ||
| const searchDirs = [process.cwd()]; | ||
| const targetAgent = this.to as AgentType; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the agent.ts file around the disputed lines
cat -n packages/cli/src/commands/agent.ts | sed -n '310,330p'Repository: rohitg00/skillkit
Length of output: 778
🏁 Script executed:
# Find the AgentType definition
rg -n "type AgentType|interface AgentType|enum AgentType" packages/ -A 5Repository: rohitg00/skillkit
Length of output: 5859
🏁 Script executed:
# Look at the AgentTranslateCommand class definition to understand how `this.to` is defined
rg -n "class AgentTranslateCommand" packages/cli/src/commands/agent.ts -A 30 | head -50Repository: rohitg00/skillkit
Length of output: 1123
🏁 Script executed:
# Find the full AgentType schema definition in types.ts
cat -n packages/core/src/types.ts | head -30Repository: rohitg00/skillkit
Length of output: 832
🏁 Script executed:
# Search for AgentType schema definition (before the type export)
rg -n "const AgentType|export const AgentType" packages/core/src/types.ts -B 2 -A 5Repository: rohitg00/skillkit
Length of output: 207
🏁 Script executed:
# Check the exact AgentType definition - look for z.enum or similar
rg -n "AgentType\s*=" packages/core/src/types.ts -B 5 -A 15 | head -50Repository: rohitg00/skillkit
Length of output: 1111
🏁 Script executed:
# Check how translateAgent function is called and if it validates
rg -n "translateAgent" packages/cli/src/commands/agent.ts -A 3 -B 1Repository: rohitg00/skillkit
Length of output: 661
🏁 Script executed:
# Find the translateAgent function to see if it validates input
rg -n "export.*translateAgent|function translateAgent" packages/ -A 10 | head -60Repository: rohitg00/skillkit
Length of output: 2391
🏁 Script executed:
# Check the actual implementation of translateCanonicalAgent to see how it uses targetAgent
cat -n packages/core/src/agents/translator.ts | sed -n '35,80p'Repository: rohitg00/skillkit
Length of output: 1849
🏁 Script executed:
# Check if there's any validation in the translate flow or if it just uses targetAgent directly
rg -n "switch.*targetAgent|case.*targetAgent|\[targetAgent\]" packages/core/src/agents/translator.tsRepository: rohitg00/skillkit
Length of output: 301
🏁 Script executed:
# Find CUSTOM_AGENT_FORMAT_MAP definition to understand what happens with invalid keys
rg -n "CUSTOM_AGENT_FORMAT_MAP\s*=" packages/core/src/agents/ -A 20 | head -40Repository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
# Also check AGENT_DISCOVERY_PATHS since it's also accessed with targetAgent
rg -n "AGENT_DISCOVERY_PATHS\s*=" packages/core/src/agents/ -A 15 | head -35Repository: rohitg00/skillkit
Length of output: 1011
🏁 Script executed:
# Search more broadly for CUSTOM_AGENT_FORMAT_MAP
rg -n "CUSTOM_AGENT_FORMAT_MAP" packages/ -B 2 -A 20Repository: rohitg00/skillkit
Length of output: 9340
🏁 Script executed:
# Also search for where these maps are defined
fd -t f "types.ts" packages/core/src/agents/ -exec cat {} \; | grep -n "CUSTOM_AGENT_FORMAT_MAP\|AGENT_DISCOVERY_PATHS" -A 15 -B 2Repository: rohitg00/skillkit
Length of output: 207
🏁 Script executed:
# Check if clipanion supports enum validation for Option.String
rg -n "Option.String.*--to" packages/cli/src/commands/agent.ts -B 2 -A 5Repository: rohitg00/skillkit
Length of output: 312
🏁 Script executed:
# Look for how clipanion defines options and if there's enum support
rg -n "class.*Command.*extends Command" packages/cli/src/commands/ | head -5Repository: rohitg00/skillkit
Length of output: 484
🏁 Script executed:
# Check if there's enum/choice validation in clipanion usage elsewhere
rg -n "Option.String.*enum|OptionValues" packages/cli/src/commands/ -B 2 -A 2 | head -30Repository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
# Check if AgentType.parse is used elsewhere in the codebase for validation
rg -n "AgentType\.parse|z\.parse" packages/cli/src/commands/ -B 2 -A 2Repository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
# Check settings.ts where VALID_AGENTS is defined to see the pattern
cat -n packages/cli/src/commands/settings.ts | sed -n '1,20p'Repository: rohitg00/skillkit
Length of output: 968
🏁 Script executed:
# Look for how validateAgent is implemented
rg -n "export.*validateAgent|function validateAgent" packages/core/src/agents/ -A 10 | head -40Repository: rohitg00/skillkit
Length of output: 852
Add runtime validation for --to argument before casting to AgentType.
Line 317 casts this.to directly to AgentType without validation. The clipanion Option.String does not enforce enum constraints, so invalid input like --to invalid-agent passes through unchecked. This causes CUSTOM_AGENT_FORMAT_MAP[targetAgent] and AGENT_DISCOVERY_PATHS[targetAgent] to return undefined, resulting in confusing errors downstream.
Validate the input against the valid agent types before use:
Suggested fix
async execute(): Promise<number> {
const searchDirs = [process.cwd()];
+
+ // Validate target agent type
+ const validAgents = Object.keys(CUSTOM_AGENT_FORMAT_MAP);
+ if (!validAgents.includes(this.to)) {
+ console.log(chalk.red(`Invalid target agent: ${this.to}`));
+ console.log(chalk.dim(`Valid options: ${validAgents.join(', ')}`));
+ return 1;
+ }
const targetAgent = this.to as AgentType;Alternatively, use Zod's AgentType.parse() to validate and infer the type simultaneously.
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/agent.ts` around lines 315 - 318, The code casts
this.to directly to AgentType in execute() without validating it, which can make
CUSTOM_AGENT_FORMAT_MAP[targetAgent] and AGENT_DISCOVERY_PATHS[targetAgent]
undefined for bad input; fix by validating the --to value before casting (e.g.,
check that this.to is one of the allowed AgentType enum values or call
AgentType.parse() via Zod) and return an error/throw if invalid so subsequent
lookups of CUSTOM_AGENT_FORMAT_MAP and AGENT_DISCOVERY_PATHS use a guaranteed
valid targetAgent.
| const targetAgents = this.agent | ||
| ? this.agent.split(',').map(a => a.trim() as AgentType) | ||
| : ['claude-code' as AgentType]; |
There was a problem hiding this comment.
Same unsafe cast issue as in AgentTranslateCommand.
The --agent option is split and cast to AgentType[] without validation. Apply the same validation pattern suggested for AgentTranslateCommand.
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/agent.ts` around lines 422 - 424, The
split-and-cast of this.agent into targetAgents is unsafe; mirror the validation
used in AgentTranslateCommand by checking each parsed string against the allowed
AgentType values instead of force-casting. Replace the direct map(a => a.trim()
as AgentType) with logic that trims each entry, verifies it exists in the
AgentType set/enum, collects only valid AgentType entries (or raises a clear
error if invalid), and use that validated array for targetAgents so the code no
longer relies on an unchecked cast.
| // Check global agents | ||
| const home = homedir(); | ||
| const globalPaths = [ | ||
| join(home, '.claude', 'agents'), | ||
| join(home, '.skillkit', 'agents'), | ||
| ]; | ||
|
|
||
| for (const agentsDir of globalPaths) { | ||
| if (!existsSync(agentsDir)) continue; | ||
|
|
||
| const agentFile = join(agentsDir, `${name}.md`); | ||
| if (existsSync(agentFile)) { | ||
| return parseAgentFile(agentFile, 'global'); | ||
| } | ||
|
|
||
| const agentDir = join(agentsDir, name); | ||
| if (existsSync(agentDir) && statSync(agentDir).isDirectory()) { | ||
| return parseAgentDir(agentDir, 'global'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Global paths inconsistency between findAgent and discoverGlobalAgents.
discoverGlobalAgents searches ~/.config/skillkit/agents (line 120), but findAgent only searches ~/.claude/agents and ~/.skillkit/agents. An agent discovered globally might not be found by name.
🔧 Proposed fix
// Check global agents
const home = homedir();
const globalPaths = [
join(home, '.claude', 'agents'),
join(home, '.skillkit', 'agents'),
+ join(home, '.config', 'skillkit', 'agents'),
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check global agents | |
| const home = homedir(); | |
| const globalPaths = [ | |
| join(home, '.claude', 'agents'), | |
| join(home, '.skillkit', 'agents'), | |
| ]; | |
| for (const agentsDir of globalPaths) { | |
| if (!existsSync(agentsDir)) continue; | |
| const agentFile = join(agentsDir, `${name}.md`); | |
| if (existsSync(agentFile)) { | |
| return parseAgentFile(agentFile, 'global'); | |
| } | |
| const agentDir = join(agentsDir, name); | |
| if (existsSync(agentDir) && statSync(agentDir).isDirectory()) { | |
| return parseAgentDir(agentDir, 'global'); | |
| } | |
| } | |
| // Check global agents | |
| const home = homedir(); | |
| const globalPaths = [ | |
| join(home, '.claude', 'agents'), | |
| join(home, '.skillkit', 'agents'), | |
| join(home, '.config', 'skillkit', 'agents'), | |
| ]; | |
| for (const agentsDir of globalPaths) { | |
| if (!existsSync(agentsDir)) continue; | |
| const agentFile = join(agentsDir, `${name}.md`); | |
| if (existsSync(agentFile)) { | |
| return parseAgentFile(agentFile, 'global'); | |
| } | |
| const agentDir = join(agentsDir, name); | |
| if (existsSync(agentDir) && statSync(agentDir).isDirectory()) { | |
| return parseAgentDir(agentDir, 'global'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/agents/discovery.ts` around lines 198 - 217, The global
agent search paths are inconsistent between discoverGlobalAgents and findAgent:
update the globalPaths array used in findAgent (the variable named globalPaths
in discovery.ts) to include the same "~/.config/skillkit/agents" path that
discoverGlobalAgents uses so agents discovered in ~/.config/skillkit/agents can
be found by name; ensure the new path is constructed with join(homedir(),
'.config', 'skillkit', 'agents') and keep the existing checks that look for both
a `${name}.md` file and a directory (parseAgentFile/parseAgentDir).
| if (canonical.allowedTools && canonical.allowedTools.length > 0) { | ||
| lines.push('allowed-tools:'); | ||
| for (const tool of canonical.allowedTools) { | ||
| lines.push(` - ${tool}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the AgentFrontmatter schema to see the expected key name
rg -A 10 "interface AgentFrontmatter|type AgentFrontmatter" packages/core/src/agents/types.tsRepository: rohitg00/skillkit
Length of output: 351
🏁 Script executed:
# Check if there's YAML parsing or conversion logic in the parser
rg -n "allowedTools|allowed-tools" packages/core/src/agents/parser.ts | head -20Repository: rohitg00/skillkit
Length of output: 781
🏁 Script executed:
# Look for any YAML metadata handling or parsing functions
rg -n "frontmatter|yaml|YAML" packages/core/src/agents/ -i | head -30Repository: rohitg00/skillkit
Length of output: 2593
🏁 Script executed:
# Get the full AgentFrontmatter schema definition
sed -n '50,86p' packages/core/src/agents/types.tsRepository: rohitg00/skillkit
Length of output: 1460
🏁 Script executed:
# Also check if allowedTools is referenced anywhere in the schema
rg -B 5 -A 5 "allowedTools" packages/core/src/agents/types.ts | head -40Repository: rohitg00/skillkit
Length of output: 887
Fix key name inconsistency in serialization: allowed-tools should be allowedTools.
Line 245 outputs allowed-tools (kebab-case), but the AgentFrontmatter schema expects allowedTools (camelCase). This is inconsistent with:
- The schema definition in
types.tsline 65 - The serialization of
disallowedToolson line 238 (correctly camelCase)
When agent files are re-parsed, the allowedTools field will not populate because the YAML key doesn't match the schema's expected key name.
🤖 Prompt for AI Agents
In `@packages/core/src/agents/parser.ts` around lines 244 - 249, The serializer in
parser.ts writes the key as "allowed-tools" (kebab-case) but the
AgentFrontmatter schema expects "allowedTools" (camelCase), causing re-parsing
to miss the field; update the serialization to use the camelCase key by changing
the output label for canonical.allowedTools to "allowedTools" to match how
disallowedTools is serialized and the AgentFrontmatter type so round-trip
parsing populates allowedTools correctly.
| disallowedTools: z.array(z.string()).optional(), | ||
| /** Allowed tools for this agent */ | ||
| allowedTools: z.union([ | ||
| z.array(z.string()), | ||
| z.string(), // YAML-style list in frontmatter | ||
| ]).optional(), |
There was a problem hiding this comment.
Normalize allowedTools vs allowed-tools to avoid data loss.
AgentFrontmatter only accepts allowedTools, but the generator emits allowed-tools and toCanonicalAgent reads only allowedTools. This breaks round‑trip (tools dropped) and likely fails validation for generated agents. Align the key or accept both and normalize before building CanonicalAgent.
🔧 Minimal schema-side fix (also normalize in parser)
/** Allowed tools for this agent */
allowedTools: z.union([
z.array(z.string()),
z.string(), // YAML-style list in frontmatter
]).optional(),
+ /** Legacy/alt key emitted by formatter */
+ 'allowed-tools': z.union([
+ z.array(z.string()),
+ z.string(),
+ ]).optional(),🤖 Prompt for AI Agents
In `@packages/core/src/agents/types.ts` around lines 63 - 68, AgentFrontmatter and
toCanonicalAgent currently only handle allowedTools while the generator emits
allowed-tools, causing data loss; update the schema and normalization so both
keys are accepted and coerced into the canonical allowedTools field before
constructing a CanonicalAgent. Specifically, modify AgentFrontmatter parsing
(and any zod schema around allowedTools) to accept a kebab-case alias
"allowed-tools" (or accept both z.union/object shape) and then in
toCanonicalAgent normalize/merge the values from "allowed-tools" into
allowedTools (prefer explicit allowedTools when both present) so the resulting
CanonicalAgent always has the tools list under allowedTools. Ensure any
downstream code that reads allowedTools (e.g., validation or generator tests)
uses the normalized field.
- parser.ts: Import statSync instead of using require('node:fs')
- translator.ts: Add ESM imports for extractAgentFrontmatter, extractAgentContent, AgentFrontmatter
- agent.ts: Add validation for agent name format (lowercase alphanumeric with hyphens)
Addresses review comments from Devin.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| async execute(): Promise<number> { | ||
| const searchDirs = [process.cwd()]; | ||
| const targetAgent = this.to as AgentType; |
There was a problem hiding this comment.
🟡 Invalid AgentType not validated before use in CLI commands
The --to option in AgentTranslateCommand and --agent option in AgentSyncCommand are cast directly to AgentType without validation.
Click to expand
When an invalid agent type is provided (e.g., skillkit agent translate --to invalid-agent), the value is cast to AgentType at line 325 and 431. This causes:
CUSTOM_AGENT_FORMAT_MAP[targetAgent]returnsundefined(packages/core/src/agents/translator.ts:46)- The switch statement falls through to default case (
packages/core/src/agents/translator.ts:81-82) AGENT_DISCOVERY_PATHS[targetAgent]returnsundefined, causing files to be written to a fallbackagentsdirectory (packages/core/src/agents/translator.ts:368-372)- The function still returns
success: true
Actual behavior: Files are silently written to wrong directory with no error.
Expected behavior: Command should validate the agent type and display an error for invalid values.
Recommendation: Add validation before casting: const validAgents = ['claude-code', 'cursor', 'codex', ...]; if (!validAgents.includes(this.to)) { console.log(chalk.red(Invalid agent type: ${this.to})); return 1; }
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Generate frontmatter | ||
| lines.push('---'); | ||
| lines.push(`name: ${canonical.name}`); | ||
| lines.push(`description: ${canonical.description}`); |
There was a problem hiding this comment.
🟡 Description not escaped in fromCanonicalAgent causing potentially invalid YAML
The fromCanonicalAgent function does not escape the description field, while other generator functions (generateClaudeAgent, generateCursorAgent, generateUniversalAgent) properly use escapeYamlString.
Click to expand
At line 227 in packages/core/src/agents/parser.ts:
lines.push(`description: ${canonical.description}`);Compare with generateClaudeAgent at packages/core/src/agents/translator.ts:167:
lines.push(`description: ${escapeYamlString(canonical.description)}`);If the description contains YAML special characters like :, #, {, }, [, ], etc., the generated YAML could be invalid or parsed incorrectly.
Actual behavior: Descriptions with special characters produce invalid YAML.
Expected behavior: Description should be escaped like in other generator functions.
Recommendation: Import and use the escapeYamlString function from translator.ts, or extract it to a shared utility, and apply it: lines.push(description: ${escapeYamlString(canonical.description)});
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
.claude/agents/*.md)skillkit agent list/show/create/translate/sync/validateChanges
Core module (
packages/core/src/agents/):types.ts: Agent types, schemas, AGENT_DISCOVERY_PATHS for 17 AI agentsparser.ts: Parse agent markdown with YAML frontmatterdiscovery.ts: Discover agents across all supported agent directoriestranslator.ts: Translate agents between Claude, Cursor, and universal formatsCLI commands (
skillkit agent):listshow <name>create <name>translate --to <agent>sync --agent <agent>validate [path]Test plan
pnpm build)pnpm test)skillkit agent listworksskillkit agent createcreates agent fileCloses #5
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.