agent skill fixes#258
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
🤖 My Senior Dev — Analysis Complete👤 For @khaliqgant📁 Expert in View your contributor analytics → 📊 17 files reviewed • 2 high risk • 3 need attention 🚨 High Risk:
🚀 Open Interactive Review →The full interface unlocks features not available in GitHub:
💬 Chat here: 📖 View all 12 personas & slash commandsYou can interact with me by mentioning In PR comments or on any line of code:
Slash commands:
AI Personas (mention to get their perspective):
For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews. |
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| if (subtype === 'skill' && (frontmatter.name || frontmatter['allowed-tools'] || frontmatter.compatibility || frontmatter.license)) { | ||
| metadataSection.data.agentSkills = { | ||
| name: frontmatter.name, | ||
| license: frontmatter.license, | ||
| compatibility: frontmatter.compatibility, | ||
| allowedTools: frontmatter['allowed-tools'], | ||
| metadata: frontmatter.metadata, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Suggestion: The canonical agentSkills.allowedTools field is documented and used elsewhere as a space-delimited list per the Agent Skills spec, but here it is populated directly from Claude frontmatter allowed-tools, which in this codebase is treated as a comma-separated list; this mismatch will produce invalid allowed-tools values in generated SKILL.md files and break roundtrip parsing where consumers expect space-delimited tool names. [logic error]
Severity Level: Major ⚠️
- ⚠️ Agent Skills SKILL.md uses non-spec comma-separated allowed-tools.
- ⚠️ Roundtrip Claude→Agent Skills misparses allowed tools list.
- ⚠️ Downstream tooling relying on schema may reject skills.| if (subtype === 'skill' && (frontmatter.name || frontmatter['allowed-tools'] || frontmatter.compatibility || frontmatter.license)) { | |
| metadataSection.data.agentSkills = { | |
| name: frontmatter.name, | |
| license: frontmatter.license, | |
| compatibility: frontmatter.compatibility, | |
| allowedTools: frontmatter['allowed-tools'], | |
| metadata: frontmatter.metadata, | |
| }; | |
| } | |
| if (subtype === 'skill' && (frontmatter.name || frontmatter['allowed-tools'] || frontmatter.compatibility || frontmatter.license)) { | |
| const allowedToolsField = frontmatter['allowed-tools']; | |
| const normalizedAllowedTools = | |
| typeof allowedToolsField === 'string' | |
| ? allowedToolsField.replace(/,/g, ' ').replace(/\s+/g, ' ').trim() | |
| : undefined; | |
| metadataSection.data.agentSkills = { | |
| name: frontmatter.name, | |
| license: frontmatter.license, | |
| compatibility: frontmatter.compatibility, | |
| allowedTools: normalizedAllowedTools, | |
| metadata: frontmatter.metadata, | |
| }; | |
| } |
Steps of Reproduction ✅
1. Use the PRPM CLI convert flow implemented in
`packages/cli/src/commands/convert.ts:338`, which calls `fromClaude(content, metadata)`
(defined in `packages/converters/src/from-claude.ts:30`) to convert a Claude markdown file
to canonical format.
2. Supply a Claude skill markdown file whose YAML frontmatter includes Agent Skills fields
and a comma-separated `allowed-tools` value, for example:
- `name: pdf-processing`
- `description: Extracts and processes PDF documents`
- `license: MIT`
- `compatibility: Requires pdftotext`
- `allowed-tools: Bash(pdftotext:*) Read, Write`
This mirrors how `fromClaude` already treats `frontmatter['allowed-tools']` as a
comma-separated list for tools extraction at `from-claude.ts:54-60`.
3. When `fromClaude` runs, subtype detection at
`packages/converters/src/from-claude.ts:121-122` classifies the package as a `skill`, and
the block at `126-132` executes:
- It assigns `metadataSection.data.agentSkills.allowedTools =
frontmatter['allowed-tools']`, preserving the comma-separated string `Bash(pdftotext:*)
Read, Write` without normalization, even though the canonical type defines
`agentSkills.allowedTools` as a **space-delimited** list in
`packages/converters/src/types/canonical.ts:148-153`.
4. Convert this canonical package to an Agent Skills SKILL.md using any of the outbound
converters that read `agentSkills.allowedTools`:
- `toCopilot` skill conversion in `packages/converters/src/to-copilot.ts:166-175` and
`218-220` writes `allowed-tools: "<agentSkillsData.allowedTools>"` into SKILL.md.
- `toOpencode` skill conversion in `packages/converters/src/to-opencode.ts:72-104`
writes `frontmatter['allowed-tools'] = agentSkillsData.allowedTools`.
- `toCodex` skill conversion in `packages/converters/src/to-codex.ts:217-218` writes
`frontmatter['allowed-tools'] = skillsData.allowedTools`.
The generated SKILL.md frontmatter now contains a comma-separated `allowed-tools` value
(e.g., `allowed-tools: "Bash(pdftotext:*) Read, Write"`), which contradicts the Agent
Skills spec comments in `from-codex.ts:31-40` and `from-opencode.ts:59-63` that
describe `allowed-tools` as a **space-delimited** list.
5. Parse that generated SKILL.md back through the Codex parser `fromCodex` in
`packages/converters/src/from-codex.ts`:
- `parseFrontmatter` returns the string `Bash(pdftotext:*) Read, Write` as
`fm['allowed-tools']` (`from-codex.ts:48-65`).
- `parseAllowedTools` at `from-codex.ts:78-90` splits on whitespace only, producing
tokens like `["Bash(pdftotext:*)", "Read,", "Write"]`, so the second tool becomes
`Read,` (including the comma), an invalid tool name.
- The tools section built at `from-codex.ts:130-139` will therefore contain an
incorrect tool identifier (`'Read,'`), and the original comma-separated string is also
preserved into `metadataSection.data.agentSkills.allowedTools` at
`from-codex.ts:119-126`, breaking the assumption (documented in
`types/canonical.ts:148-153`) that this field is space-delimited.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/converters/src/from-claude.ts
**Line:** 126:134
**Comment:**
*Logic Error: The canonical `agentSkills.allowedTools` field is documented and used elsewhere as a space-delimited list per the Agent Skills spec, but here it is populated directly from Claude frontmatter `allowed-tools`, which in this codebase is treated as a comma-separated list; this mismatch will produce invalid `allowed-tools` values in generated SKILL.md files and break roundtrip parsing where consumers expect space-delimited tool names.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 10 files
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="packages/converters/schemas/codex-agent-role.schema.json">
<violation number="1" location="packages/converters/schemas/codex-agent-role.schema.json:23">
P2: sandbox_mode enum values don’t match Codex docs ("read-only"/"workspace-write"[/"danger-full-access"]). The schema currently rejects valid agent-role configs and permits unsupported values.</violation>
</file>
<file name="packages/cli/src/commands/install.ts">
<violation number="1" location="packages/cli/src/commands/install.ts:1103">
P2: Non-Codex agent installs now fall through to the manifest-file path because the agent branch only matches Codex. This changes the install destination for `agents.md`/`claude.md`/`gemini.md` agent packages and breaks expected progressive-disclosure behavior.</violation>
</file>
<file name="packages/converters/src/from-claude.ts">
<violation number="1" location="packages/converters/src/from-claude.ts:132">
P2: frontmatter.metadata is not parsed as an object by parseFrontmatter, so assigning it directly to agentSkills.metadata can produce malformed metadata output (e.g., Object.entries on a string). Guard against non-object values or parse the metadata map before storing it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1 participant
CodeAnt-AI Description
Add native Codex agent role (TOML) support and preserve Agent Skills frontmatter
What Changed
Impact
✅ Codex agent roles install as .codex/agents/<name>.toml✅ Roundtrip fidelity for Codex agent configurations✅ Preserves Agent Skills frontmatter for SKILL.md roundtrips💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.