From b67cfe3a97534cfdf37176815027c7008a0002b2 Mon Sep 17 00:00:00 2001 From: Doug Lance <4741454+douglance@users.noreply.github.com> Date: Thu, 26 Mar 2026 16:38:52 -0400 Subject: [PATCH 1/3] fix: YAML-quote frontmatter description to prevent parse errors Description values containing colons, backticks, and other YAML-special characters (e.g. "Run `app ping --help` for usage details.") break frontmatter parsing when left unquoted. This adds a `yamlQuote()` helper that wraps the value in double quotes when it contains any character that needs escaping, and strips quotes when reading the description back in index.json generation. --- src/Cli.ts | 3 ++- src/Skill.test.ts | 8 ++++---- src/Skill.ts | 10 +++++++++- src/e2e.test.ts | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Cli.ts b/src/Cli.ts index f7a2f82..e2bd89e 100644 --- a/src/Cli.ts +++ b/src/Cli.ts @@ -1566,9 +1566,10 @@ async function fetchImpl( const files = Skill.split(name, cmds, 1, groups) const skills = files.map((f) => { const descMatch = f.content.match(/^description:\s*(.+)$/m) + const rawDesc = descMatch?.[1] ?? '' return { name: f.dir || name, - description: descMatch?.[1] ?? '', + description: rawDesc.replace(/^"(.*)"$/, '$1'), files: ['SKILL.md'], } }) diff --git a/src/Skill.test.ts b/src/Skill.test.ts index 9410d23..50bc046 100644 --- a/src/Skill.test.ts +++ b/src/Skill.test.ts @@ -252,7 +252,7 @@ describe('split', () => { expect(files[0]!.content).toMatchInlineSnapshot(` "--- name: gh-auth - description: Authenticate with GitHub. Log in, Check status. Run \`gh auth --help\` for usage details. + description: "Authenticate with GitHub. Log in, Check status. Run \`gh auth --help\` for usage details." requires_bin: gh command: gh auth --- @@ -270,7 +270,7 @@ describe('split', () => { expect(files[1]!.content).toMatchInlineSnapshot(` "--- name: gh-pr - description: Manage pull requests. List PRs, Create PR. Run \`gh pr --help\` for usage details. + description: "Manage pull requests. List PRs, Create PR. Run \`gh pr --help\` for usage details." requires_bin: gh command: gh pr --- @@ -290,7 +290,7 @@ describe('split', () => { test('depth 1 without group descriptions uses child descriptions', () => { const files = Skill.split('gh', commands, 1) expect(files[0]!.content).toContain( - 'description: Log in, Check status. Run `gh auth --help` for usage details.', + 'description: "Log in, Check status. Run `gh auth --help` for usage details."', ) }) @@ -333,7 +333,7 @@ describe('split', () => { test('emits fallback description when no explicit descriptions exist', () => { const files = Skill.split('test', [{ name: 'ping' }], 1) - expect(files[0]!.content).toContain('description: Run `test ping --help` for usage details.') + expect(files[0]!.content).toContain('description: "Run `test ping --help` for usage details."') }) test('includes requires_bin in frontmatter', () => { diff --git a/src/Skill.ts b/src/Skill.ts index 3ec985f..131318a 100644 --- a/src/Skill.ts +++ b/src/Skill.ts @@ -133,7 +133,7 @@ function renderGroup( .replace(/-{2,}/g, '-') .replace(/^-|-$/g, '') const fm = ['---', `name: ${slug}`] - fm.push(`description: ${description}`) + fm.push(`description: ${yamlQuote(description)}`) fm.push(`requires_bin: ${cli}`) fm.push(`command: ${title}`, '---') @@ -234,6 +234,14 @@ function renderCommandBody(cli: string, cmd: CommandInfo, level = 1): string { return sections.join('\n\n') } +/** @internal YAML-quotes a string value if it contains characters that need escaping. */ +function yamlQuote(value: string): string { + if (/[:#\[\]{}&*!|>'"%@`]/.test(value) || value.startsWith('- ') || value.startsWith('? ')) { + return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"` + } + return value +} + /** Computes a deterministic hash of command structure for staleness detection. */ export function hash(commands: CommandInfo[]): string { const data = commands.map((cmd) => ({ diff --git a/src/e2e.test.ts b/src/e2e.test.ts index b8a6a84..e62a150 100644 --- a/src/e2e.test.ts +++ b/src/e2e.test.ts @@ -2927,7 +2927,7 @@ describe('.well-known/skills', () => { expect(result.body).toMatchInlineSnapshot(` "--- name: app-ping - description: Health check. Run \`app ping --help\` for usage details. + description: "Health check. Run \`app ping --help\` for usage details." requires_bin: app command: app ping --- From fd46f62099bb9115996e546926d3f69d64cc829a Mon Sep 17 00:00:00 2001 From: Doug Lance <4741454+douglance@users.noreply.github.com> Date: Thu, 26 Mar 2026 17:01:45 -0400 Subject: [PATCH 2/3] fix: use yaml library for frontmatter serialization/parsing The frontmatter description was built via string concatenation, which produces invalid YAML when the value contains colon-space sequences (e.g. "Use key: value"). Instead of hand-rolling quoting logic, use the `yaml` package (already a dependency) to serialize frontmatter and parse it back in index.json generation. This handles all YAML edge cases correctly. --- src/Cli.ts | 7 ++++--- src/Skill.test.ts | 16 ++++++++++++---- src/Skill.ts | 20 +++++++------------- src/e2e.test.ts | 2 +- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/Cli.ts b/src/Cli.ts index e2bd89e..e2462fa 100644 --- a/src/Cli.ts +++ b/src/Cli.ts @@ -2,6 +2,7 @@ import * as fs from 'node:fs/promises' import * as os from 'node:os' import * as path from 'node:path' import { estimateTokenCount, sliceByTokens } from 'tokenx' +import { parse as yamlParse } from 'yaml' import type { z } from 'zod' import * as Completions from './Completions.js' @@ -1565,11 +1566,11 @@ async function fetchImpl( if (segments[2] === 'index.json' && segments.length === 3) { const files = Skill.split(name, cmds, 1, groups) const skills = files.map((f) => { - const descMatch = f.content.match(/^description:\s*(.+)$/m) - const rawDesc = descMatch?.[1] ?? '' + const fmMatch = f.content.match(/^---\n([\s\S]*?)\n---/) + const meta = fmMatch ? (yamlParse(fmMatch[1]!) as Record) : {} return { name: f.dir || name, - description: rawDesc.replace(/^"(.*)"$/, '$1'), + description: meta.description ?? '', files: ['SKILL.md'], } }) diff --git a/src/Skill.test.ts b/src/Skill.test.ts index 50bc046..7ae0aa7 100644 --- a/src/Skill.test.ts +++ b/src/Skill.test.ts @@ -252,7 +252,7 @@ describe('split', () => { expect(files[0]!.content).toMatchInlineSnapshot(` "--- name: gh-auth - description: "Authenticate with GitHub. Log in, Check status. Run \`gh auth --help\` for usage details." + description: Authenticate with GitHub. Log in, Check status. Run \`gh auth --help\` for usage details. requires_bin: gh command: gh auth --- @@ -270,7 +270,7 @@ describe('split', () => { expect(files[1]!.content).toMatchInlineSnapshot(` "--- name: gh-pr - description: "Manage pull requests. List PRs, Create PR. Run \`gh pr --help\` for usage details." + description: Manage pull requests. List PRs, Create PR. Run \`gh pr --help\` for usage details. requires_bin: gh command: gh pr --- @@ -290,7 +290,7 @@ describe('split', () => { test('depth 1 without group descriptions uses child descriptions', () => { const files = Skill.split('gh', commands, 1) expect(files[0]!.content).toContain( - 'description: "Log in, Check status. Run `gh auth --help` for usage details."', + 'description: Log in, Check status. Run `gh auth --help` for usage details.', ) }) @@ -333,7 +333,7 @@ describe('split', () => { test('emits fallback description when no explicit descriptions exist', () => { const files = Skill.split('test', [{ name: 'ping' }], 1) - expect(files[0]!.content).toContain('description: "Run `test ping --help` for usage details."') + expect(files[0]!.content).toContain('description: Run `test ping --help` for usage details.') }) test('includes requires_bin in frontmatter', () => { @@ -341,6 +341,14 @@ describe('split', () => { expect(files[0]!.content).toContain('requires_bin: gh') }) + test('YAML-quotes description containing colon-space', () => { + const groups = new Map([['search', 'Search items. Use key: value for precision']]) + const files = Skill.split('app', [{ name: 'search list', description: 'List results' }], 1, groups) + expect(files[0]!.content).toContain( + 'description: "Search items. Use key: value for precision. List results. Run `app search --help` for usage details."', + ) + }) + test('no per-command frontmatter in split files', () => { const files = Skill.split('gh', commands, 1, groups) const afterFrontmatter = files[0]!.content.slice( diff --git a/src/Skill.ts b/src/Skill.ts index 131318a..495014d 100644 --- a/src/Skill.ts +++ b/src/Skill.ts @@ -1,5 +1,6 @@ import { createHash } from 'node:crypto' import type { z } from 'zod' +import { stringify as yamlStringify } from 'yaml' import * as Schema from './Schema.js' @@ -132,13 +133,14 @@ function renderGroup( .replace(/[^a-z0-9-]+/g, '-') .replace(/-{2,}/g, '-') .replace(/^-|-$/g, '') - const fm = ['---', `name: ${slug}`] - fm.push(`description: ${yamlQuote(description)}`) - fm.push(`requires_bin: ${cli}`) - fm.push(`command: ${title}`, '---') + const fm = yamlStringify( + { name: slug, description, requires_bin: cli, command: title }, + { lineWidth: 0 }, + ).trimEnd() + const fmBlock = `---\n${fm}\n---` const body = cmds.map((cmd) => renderCommandBody(cli, cmd)).join('\n\n---\n\n') - return `${fm.join('\n')}\n\n${body}` + return `${fmBlock}\n\n${body}` } /** @internal Renders a command's heading and sections without frontmatter. */ @@ -234,14 +236,6 @@ function renderCommandBody(cli: string, cmd: CommandInfo, level = 1): string { return sections.join('\n\n') } -/** @internal YAML-quotes a string value if it contains characters that need escaping. */ -function yamlQuote(value: string): string { - if (/[:#\[\]{}&*!|>'"%@`]/.test(value) || value.startsWith('- ') || value.startsWith('? ')) { - return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"` - } - return value -} - /** Computes a deterministic hash of command structure for staleness detection. */ export function hash(commands: CommandInfo[]): string { const data = commands.map((cmd) => ({ diff --git a/src/e2e.test.ts b/src/e2e.test.ts index e62a150..b8a6a84 100644 --- a/src/e2e.test.ts +++ b/src/e2e.test.ts @@ -2927,7 +2927,7 @@ describe('.well-known/skills', () => { expect(result.body).toMatchInlineSnapshot(` "--- name: app-ping - description: "Health check. Run \`app ping --help\` for usage details." + description: Health check. Run \`app ping --help\` for usage details. requires_bin: app command: app ping --- From b6b84203ca02230cb0a01cda75ee396d07a3aa79 Mon Sep 17 00:00:00 2001 From: tmm Date: Fri, 24 Apr 2026 11:36:51 -0400 Subject: [PATCH 3/3] refactor: parse skill frontmatter consistently --- AGENTS.md | 1 + src/SyncSkills.test.ts | 31 +++++++++++++++++++++++++++++++ src/SyncSkills.ts | 25 ++++++++++++++++--------- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8110dae..593e71e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,7 @@ ## Documentation Conventions - **JSDoc on all exports** — every exported function, type, and constant gets a JSDoc comment. Type properties get JSDoc too. Namespace types (e.g. `declare namespace create { type Options }`) get JSDoc too. Doc-driven development: write the JSDoc before or alongside the implementation, not after. +- **Parse structured frontmatter structurally** — when `SKILL.md` frontmatter is emitted as YAML, read it back with the YAML parser instead of regex-scraping individual fields. ## Testing Conventions diff --git a/src/SyncSkills.test.ts b/src/SyncSkills.test.ts index 28541cc..aa2166a 100644 --- a/src/SyncSkills.test.ts +++ b/src/SyncSkills.test.ts @@ -125,3 +125,34 @@ test('installed SKILL.md contains frontmatter', async () => { rmSync(tmp, { recursive: true, force: true }) }) + +test('sync returns unquoted descriptions from YAML frontmatter', async () => { + const tmp = join(tmpdir(), `clac-quoted-description-test-${Date.now()}`) + mkdirSync(tmp, { recursive: true }) + + const search = Cli.create('search', { description: 'Search items. Use key: value for precision' }) + search.command('list', { description: 'List results', run: () => ({}) }) + + const cli = Cli.create('app') + cli.command('search', search) + + const commands = Cli.toCommands.get(cli)! + const installDir = join(tmp, 'install') + mkdirSync(join(installDir, '.agents', 'skills'), { recursive: true }) + + const result = await SyncSkills.sync('app', commands, { + global: false, + cwd: installDir, + }) + + expect(result.skills).toMatchInlineSnapshot(` + [ + { + "description": "Search items. Use key: value for precision. Run \`app search --help\` for usage details.", + "name": "app-search", + }, + ] + `) + + rmSync(tmp, { recursive: true, force: true }) +}) diff --git a/src/SyncSkills.ts b/src/SyncSkills.ts index bf44b16..fec105a 100644 --- a/src/SyncSkills.ts +++ b/src/SyncSkills.ts @@ -2,6 +2,7 @@ import fsSync from 'node:fs' import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' +import { parse as yamlParse } from 'yaml' import { formatExamples } from './Cli.js' import * as Agents from './internal/agents.js' @@ -30,9 +31,8 @@ export async function sync( : path.join(tmpDir, 'SKILL.md') await fs.mkdir(path.dirname(filePath), { recursive: true }) await fs.writeFile(filePath, `${file.content}\n`) - const nameMatch = file.content.match(/^name:\s*(.+)$/m) - const descMatch = file.content.match(/^description:\s*(.+)$/m) - skills.push({ name: nameMatch?.[1] ?? (file.dir || name), description: descMatch?.[1] }) + const meta = parseFrontmatter(file.content) + skills.push({ name: meta.name ?? (file.dir || name), description: meta.description }) } // Include additional SKILL.md files matched by glob patterns @@ -42,16 +42,14 @@ export async function sync( for await (const match of fs.glob(globPattern, { cwd })) { try { const content = await fs.readFile(path.resolve(cwd, match), 'utf8') - const nameMatch = content.match(/^name:\s*(.+)$/m) + const meta = parseFrontmatter(content) const skillName = - pattern === '_root' ? (nameMatch?.[1] ?? name) : path.basename(path.dirname(match)) + pattern === '_root' ? (meta.name ?? name) : path.basename(path.dirname(match)) const dest = path.join(tmpDir, skillName, 'SKILL.md') await fs.mkdir(path.dirname(dest), { recursive: true }) await fs.writeFile(dest, content) - if (!skills.some((s) => s.name === skillName)) { - const descMatch = content.match(/^description:\s*(.+)$/m) - skills.push({ name: skillName, description: descMatch?.[1], external: true }) - } + if (!skills.some((s) => s.name === skillName)) + skills.push({ name: skillName, description: meta.description, external: true }) } catch {} } } @@ -147,6 +145,15 @@ function collectEntries( return result.sort((a, b) => a.name.localeCompare(b.name)) } +function parseFrontmatter(content: string): { description?: string | undefined; name?: string | undefined } { + const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/) + if (!match) return {} + + const meta = yamlParse(match[1]!) + if (!meta || typeof meta !== 'object') return {} + return meta as { description?: string | undefined; name?: string | undefined } +} + /** Resolves the package root from the executing bin script (`process.argv[1]`). Walks up from the bin's directory looking for `package.json`. Falls back to `process.cwd()`. */ function resolvePackageRoot(): string { const bin = process.argv[1]