feat(09-05): @claude-ops/sdk v1.0.0 + marketplace polish#96
feat(09-05): @claude-ops/sdk v1.0.0 + marketplace polish#96auroracapital merged 1 commit intomainfrom
Conversation
- sdk/ package with SkillManifest, AgentManifest, PluginManifest, HooksConfig TypeScript types - create-ops-skill CLI scaffolder (Node 18+ ESM, no third-party deps, parseArgs) - skill + agent templates with placeholder values (Rule 0 compliant) - marketplace.json: updated description (25 skills, 13 agents) + screenshots array - plugin.json: description updated to 25 skills / 13 agents - CHANGELOG.md: Phase 9 additions prepended to [1.0.0] section - README.md: skills/agents badges updated to 25/13
|
Mention Blocks like a regular teammate with your question or request: @blocks review this pull request Run |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
| const key = line.slice(0, colonIdx).trim(); | ||
| const raw = line.slice(colonIdx + 1).trim(); | ||
| // Parse arrays: [a, b, c] | ||
| if (raw.startsWith('[') && raw.endsWith(']')) { |
There was a problem hiding this comment.
Bug: The parseSkillFrontmatter function fails to parse YAML block sequences for allowed-tools, causing it to be read as an empty string and fail validation.
Severity: CRITICAL
Suggested Fix
Replace the custom line-by-line YAML parsing logic in parseSkillFrontmatter with a robust, standard YAML parsing library that correctly handles block sequence syntax. This will ensure that multi-line lists like allowed-tools are correctly parsed into arrays.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sdk/src/helpers.ts#L23
Potential issue: The `parseSkillFrontmatter` function uses a line-by-line parser that
does not support YAML block sequences (multi-line lists). When parsing a `SKILL.md`
file, it encounters the `allowed-tools:` key but then skips the following lines that
define the list items. This results in `frontmatter['allowed-tools']` being assigned an
empty string. A subsequent call to `isValidSkillManifest` fails because it expects
`allowed-tools` to be an array, causing validation to fail for all existing and newly
created skills that use the standard block sequence format.
Also affects:
sdk/templates/skill/SKILL.md.template:10~13
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 463579968a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (raw.startsWith('[') && raw.endsWith(']')) { | ||
| frontmatter[key] = raw | ||
| .slice(1, -1) | ||
| .split(',') | ||
| .map((s) => s.trim().replace(/^["']|["']$/g, '')) |
There was a problem hiding this comment.
Accept block-list syntax in frontmatter arrays
The parser only recognizes inline arrays like [a, b], but the SDK’s own templates (and existing skill/agent manifests) use YAML block lists (allowed-tools:\n - Bash). In this code path, allowed-tools is parsed as an empty string and each - item line is ignored, so parseSkillFrontmatter cannot successfully parse scaffolded manifests and isValidSkillManifest will reject them.
Useful? React with 👍 / 👎.
| if (Array.isArray(v)) { | ||
| lines.push(`${k}: [${v.map(String).join(', ')}]`); | ||
| } else { | ||
| lines.push(`${k}: ${String(v)}`); |
There was a problem hiding this comment.
Quote scalar strings when serializing frontmatter
Serializing strings with String(v) emits unescaped YAML scalars, which changes types for common values (for example "[args]" becomes a YAML list, and "true"/"123" become boolean/number). This means manifests written via serializeSkillFrontmatter can be re-read with different types than the original object and fail schema expectations such as argument-hint being a string.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit 4635799. Configure here.
| } else { | ||
| frontmatter[key] = raw.replace(/^["']|["']$/g, ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
Frontmatter parser can't handle YAML block-sequence arrays
High Severity
parseSkillFrontmatter only handles inline arrays like [Bash, Read], but every actual skill file in the repo and both SDK templates use YAML block-sequence format (allowed-tools:\n - Bash\n - Read). When parsing this format, the key line allowed-tools: has an empty value after the colon, so the parser stores an empty string "" instead of an array. The subsequent - Bash lines have no colon and are silently skipped. This means isValidSkillManifest will always reject files scaffolded by the SDK's own CLI, since Array.isArray(m['allowed-tools']) will be false.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 4635799. Configure here.
| } | ||
| lines.push('---', '', body); | ||
| return lines.join('\n'); | ||
| } |
There was a problem hiding this comment.
Serializer corrupts string values resembling YAML types on roundtrip
Medium Severity
serializeSkillFrontmatter doesn't quote string values that look like YAML arrays, booleans, or numbers. A parse-modify-serialize roundtrip silently corrupts data. For example, the template's argument-hint: "[args]" correctly parses to the string [args], but serializing it produces argument-hint: [args] (unquoted), which re-parses as the single-element array ['args'] instead of a string. The same corruption affects string values like "true" or "42".
Reviewed by Cursor Bugbot for commit 4635799. Configure here.
| for (const line of yaml.split('\n')) { | ||
| const colonIdx = line.indexOf(':'); | ||
| if (colonIdx === -1) continue; | ||
| const key = line.slice(0, colonIdx).trim(); | ||
| const raw = line.slice(colonIdx + 1).trim(); | ||
| // Parse arrays: [a, b, c] | ||
| if (raw.startsWith('[') && raw.endsWith(']')) { | ||
| frontmatter[key] = raw | ||
| .slice(1, -1) | ||
| .split(',') | ||
| .map((s) => s.trim().replace(/^["']|["']$/g, '')) | ||
| .filter(Boolean); | ||
| } else if (raw === 'true') { | ||
| frontmatter[key] = true; | ||
| } else if (raw === 'false') { | ||
| frontmatter[key] = false; | ||
| } else if (!isNaN(Number(raw)) && raw !== '') { | ||
| frontmatter[key] = Number(raw); | ||
| } else { | ||
| frontmatter[key] = raw.replace(/^["']|["']$/g, ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 parseSkillFrontmatter cannot parse multi-line YAML arrays used by all existing skills and the SDK's own templates
The frontmatter parser in parseSkillFrontmatter only handles inline YAML arrays like allowed-tools: [Bash, Read], but every single skill in the repository — and the SDK's own generated templates — uses multi-line YAML array syntax:
allowed-tools:
- Bash
- ReadA grep for the inline format (allowed-tools: [) across all skills returns zero matches. When the parser encounters allowed-tools: with nothing after the colon, it stores an empty string. The subsequent - Bash lines have no colon and are silently skipped. This means parseSkillFrontmatter will produce { 'allowed-tools': '' } instead of an array, causing isValidSkillManifest (sdk/src/validators.ts:10) to reject every skill since Array.isArray('') is false. The same issue affects tools: and disallowedTools: in agent templates (sdk/templates/agent/AGENT.md.template:7-13).
Prompt for agents
The parseSkillFrontmatter parser in sdk/src/helpers.ts only handles inline YAML arrays (e.g. `key: [a, b, c]`) but all existing skills and the SDK's own templates use multi-line YAML list syntax:
allowed-tools:
- Bash
- Read
The parser loop at lines 17-38 processes each line independently and looks for `key: value` pairs. When it encounters `allowed-tools:` with an empty value, it stores an empty string. The continuation lines ` - Bash` have no colon and are skipped.
To fix: after parsing a `key:` line with an empty value, check if subsequent lines match the pattern /^\s+-\s+(.+)/ and collect them into an array. When you encounter a line that doesn't match that pattern, stop collecting and continue normal parsing. This needs careful handling of the iteration state — either switch to an index-based loop or use a state variable to track whether you're currently collecting list items.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
sdk/package (@claude-ops/sdk) with TypeScript types (SkillManifest,AgentManifest,PluginManifest,HooksConfig) andcreate-ops-skillCLI scaffoldermarketplace.jsonupdated with full v1.0.0 description (25 skills, 13 agents) + screenshots arrayplugin.jsondescription updated to 25 skills / 13 agentsCHANGELOG.mdPhase 9 additions prepended to[1.0.0]sectionREADME.mdskills/agents badges updated (22→25, 12→13)Test plan
node sdk/bin/create-ops-skill.mjs --helpruns without errornode sdk/bin/create-ops-skill.mjs test-skillcreatesskills/test-skill/SKILL.mdjq .plugins[0].version .claude-plugin/marketplace.json→1.0.0jq .version claude-ops/.claude-plugin/plugin.json→1.0.0grep "\[1.0.0\]" claude-ops/CHANGELOG.md→ entry existssdk/src/are valid (strict mode, no anys except intentionalstring & {})🤖 Generated with Claude Code
Note
Low Risk
Low risk: mostly documentation/metadata updates plus a new standalone SDK package and CLI scaffolder that do not affect the plugin runtime behavior.
Overview
Adds a new
sdk/npm package (@claude-ops/sdk) that ships TypeScript manifest types, minimal frontmatter parse/serialize helpers, runtime validators, and acreate-ops-skillCLI to scaffold skill/agent markdown from templates.Polishes marketplace and plugin metadata: expands descriptions to reflect 25 skills / 13 agents and adds
screenshotsto.claude-plugin/marketplace.json; updates README badges and prepends new v1.0.0 “Added” changelog entries.Reviewed by Cursor Bugbot for commit 4635799. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation