diff --git a/bin/install.js b/bin/install.js index d05fc6b02..1ad6f43b6 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1137,7 +1137,11 @@ function convertClaudeCommandToCopilotSkill(content, skillName, isGlobal = false } // Reconstruct frontmatter in Copilot format - let fm = `---\nname: ${skillName}\ndescription: ${description}\n`; + // #2876: descriptions starting with a YAML flow indicator (`[BETA] …`, + // `{ … }`, `*ref`, `&anchor`, etc.) parse as flow sequences/mappings and + // crash gh-copilot's frontmatter loader. Always quote so any leading + // character is parser-safe. + let fm = `---\nname: ${skillName}\ndescription: ${yamlQuote(description)}\n`; if (argumentHint) fm += `argument-hint: ${yamlQuote(argumentHint)}\n`; if (agent) fm += `agent: ${agent}\n`; if (toolsLine) fm += `allowed-tools: ${toolsLine}\n`; @@ -1223,8 +1227,10 @@ function convertClaudeAgentToCopilotAgent(content, isGlobal = false) { ? "['" + uniqueTools.join("', '") + "']" : '[]'; - // Reconstruct frontmatter in Copilot format - let fm = `---\nname: ${name}\ndescription: ${description}\ntools: ${toolsArray}\n`; + // Reconstruct frontmatter in Copilot format. Quote description (#2876) + // so a leading YAML flow indicator (`[BETA] …`, `{ … }`, etc.) doesn't + // crash the Copilot frontmatter loader. + let fm = `---\nname: ${name}\ndescription: ${yamlQuote(description)}\ntools: ${toolsArray}\n`; if (color) fm += `color: ${color}\n`; fm += '---'; @@ -1277,7 +1283,9 @@ function convertClaudeCommandToAntigravitySkill(content, skillName, isGlobal = f const name = skillName || extractFrontmatterField(frontmatter, 'name') || 'unknown'; const description = extractFrontmatterField(frontmatter, 'description') || ''; - const fm = `---\nname: ${name}\ndescription: ${description}\n---`; + // #2876: quote description so YAML flow indicators in the source + // (e.g. `[BETA] …`) don't break downstream frontmatter parsers. + const fm = `---\nname: ${name}\ndescription: ${yamlQuote(description)}\n---`; return `${fm}\n${body}`; } @@ -1299,7 +1307,8 @@ function convertClaudeAgentToAntigravityAgent(content, isGlobal = false) { const claudeTools = toolsRaw.split(',').map(t => t.trim()).filter(Boolean); const mappedTools = claudeTools.map(t => convertGeminiToolName(t)).filter(Boolean); - let fm = `---\nname: ${name}\ndescription: ${description}\ntools: ${mappedTools.join(', ')}\n`; + // #2876: quote description for the same reason as the skill variant. + let fm = `---\nname: ${name}\ndescription: ${yamlQuote(description)}\ntools: ${mappedTools.join(', ')}\n`; if (color) fm += `color: ${color}\n`; fm += '---'; @@ -1799,7 +1808,9 @@ function convertClaudeCommandToTraeSkill(content, skillName) { } description = toSingleLine(description); const shortDescription = description.length > 180 ? `${description.slice(0, 177)}...` : description; - return `---\nname: ${yamlIdentifier(skillName)}\ndescription: ${shortDescription}\n---\n${body}`; + // #2876: quote so YAML flow indicators (`[BETA] …`) don't break Trae's + // frontmatter parser. + return `---\nname: ${yamlIdentifier(skillName)}\ndescription: ${yamlQuote(shortDescription)}\n---\n${body}`; } function convertClaudeAgentToTraeAgent(content) { @@ -1852,7 +1863,9 @@ function convertClaudeCommandToCodebuddySkill(content, skillName) { } description = toSingleLine(description); const shortDescription = description.length > 180 ? `${description.slice(0, 177)}...` : description; - return `---\nname: ${yamlIdentifier(skillName)}\ndescription: ${shortDescription}\n---\n${body}`; + // #2876: quote so YAML flow indicators (`[BETA] …`) don't break + // CodeBuddy's frontmatter parser. + return `---\nname: ${yamlIdentifier(skillName)}\ndescription: ${yamlQuote(shortDescription)}\n---\n${body}`; } function convertClaudeAgentToCodebuddyAgent(content) { diff --git a/tests/antigravity-install.test.cjs b/tests/antigravity-install.test.cjs index 5e44e3a5e..10775a33e 100644 --- a/tests/antigravity-install.test.cjs +++ b/tests/antigravity-install.test.cjs @@ -12,7 +12,7 @@ const assert = require('node:assert/strict'); const path = require('path'); const os = require('os'); const fs = require('fs'); -const { createTempDir, cleanup } = require('./helpers.cjs'); +const { createTempDir, cleanup, parseFrontmatter } = require('./helpers.cjs'); const { getDirName, @@ -187,13 +187,11 @@ Initialize new project at ~/.claude/get-shit-done/workflows/new-project.md test('produces name and description only in frontmatter', () => { const result = convertClaudeCommandToAntigravitySkill(claudeCommand, 'gsd-new-project', false); - assert.ok(result.startsWith('---\n'), result); - assert.ok(result.includes('name: gsd-new-project'), result); - assert.ok(result.includes('description: Initialize a new GSD project'), result); - // No allowed-tools in output - assert.ok(!result.includes('allowed-tools'), result); - // No argument-hint in output - assert.ok(!result.includes('argument-hint'), result); + const fm = parseFrontmatter(result); + assert.equal(fm.name, 'gsd-new-project', result); + assert.equal(fm.description, 'Initialize a new GSD project with requirements and roadmap', result); + assert.ok(!('allowed-tools' in fm), 'no allowed-tools field'); + assert.ok(!('argument-hint' in fm), 'no argument-hint field'); }); test('applies path replacement in body', () => { @@ -242,8 +240,9 @@ Execute plans from ~/.claude/get-shit-done/workflows/execute-phase.md test('preserves name and description', () => { const result = convertClaudeAgentToAntigravityAgent(claudeAgent, false); - assert.ok(result.includes('name: gsd-executor'), result); - assert.ok(result.includes('description: Executes GSD plans'), result); + const fm = parseFrontmatter(result); + assert.equal(fm.name, 'gsd-executor', result); + assert.equal(fm.description, 'Executes GSD plans with atomic commits', result); }); test('maps Claude tools to Gemini tool names', () => { @@ -344,9 +343,10 @@ Body text. test('SKILL.md has minimal frontmatter (name + description only)', () => { copyCommandsAsAntigravitySkills(srcDir, skillsDir, 'gsd', false); const content = fs.readFileSync(path.join(skillsDir, 'gsd-new-project', 'SKILL.md'), 'utf8'); - assert.ok(content.includes('name: gsd-new-project'), content); - assert.ok(content.includes('description: Initialize a new project'), content); - assert.ok(!content.includes('allowed-tools'), content); + const fm = parseFrontmatter(content); + assert.equal(fm.name, 'gsd-new-project', content); + assert.equal(fm.description, 'Initialize a new project', content); + assert.ok(!('allowed-tools' in fm), 'no allowed-tools field'); }); test('SKILL.md body has paths converted for local install', () => { diff --git a/tests/bug-2876-skill-frontmatter-quote.test.cjs b/tests/bug-2876-skill-frontmatter-quote.test.cjs new file mode 100644 index 000000000..70946bc7d --- /dev/null +++ b/tests/bug-2876-skill-frontmatter-quote.test.cjs @@ -0,0 +1,191 @@ +/** + * Bug #2876: SKILL.md frontmatter parse failure when `description` begins + * with a YAML flow indicator like `[BETA]`. + * + * description: [BETA] Offload plan phase to Claude Code's ultraplan… + * + * YAML 1.2 treats a leading `[` as the start of a flow sequence, so any + * downstream parser (gh-copilot, JetBrains' kit, etc.) fails with + * "Unexpected scalar at node end". The Copilot/Antigravity/Trae/Codebuddy + * skill+agent converters in `bin/install.js` re-emit the description + * unquoted; the Claude variant `yamlQuote(...)`s it. Bring the others + * in line so any value is round-trip-safe regardless of leading char. + * + * The test is structural: it parses each emitted frontmatter into lines + * and asserts the `description` value is a quoted YAML scalar (double or + * single quoted) when the source description starts with a flow indicator. + * It does not regex the bytes for substrings. + */ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const fs = require('node:fs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const pkg = JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'package.json'), 'utf-8')); +const installPath = path.resolve(REPO_ROOT, pkg.bin['get-shit-done-cc']); +const install = require(installPath); + +// Build a minimal Claude command source whose description starts with the +// reporter's exact flow-indicator prefix. Apostrophe in the body forces +// any naive single-quoting to also escape correctly — the canonical +// safe form is `JSON.stringify(...)` (used by yamlQuote). +const REPORTER_DESCRIPTION = + "[BETA] Offload plan phase to Claude Code's ultraplan cloud — drafts remotely while terminal stays free, review in browser with inline comments, import back via /gsd-import. Claude Code only."; + +// Use unquoted description in the source frontmatter — that's exactly the +// shape that ships in commands/gsd/*.md when authors paste a description +// without quoting it (see commands/gsd/ultraplan-phase.md). The bug is +// triggered when the converter re-emits this same value to the destination +// runtime without quoting. `extractFrontmatterField` strips a single outer +// quote pair but does not unescape internal characters, so quoting the +// fixture input would actually mask the bug. +function buildClaudeCommand(description) { + return [ + '---', + 'name: gsd:ultraplan-phase', + `description: ${description}`, + 'argument-hint: "[phase-number]"', + 'allowed-tools:', + ' - Read', + ' - Bash', + '---', + '', + '# body', + '', + ].join('\n'); +} + +function buildClaudeAgent(description) { + return [ + '---', + 'name: gsd-extract-learnings', + `description: ${description}`, + 'tools: Read, Bash', + '---', + '', + '# body', + '', + ].join('\n'); +} + +function extractFrontmatter(content) { + // Leading delimiter is `---\n`; closing is the next standalone `---` + // on its own line. Tests parse line-structurally so the assertion + // doesn't drift on whitespace/order changes (per project test-rigor). + assert.ok(content.startsWith('---'), `output must begin with frontmatter, got: ${content.slice(0, 40)}`); + const lines = content.split('\n'); + let openIdx = -1; + let closeIdx = -1; + for (let i = 0; i < lines.length; i += 1) { + if (lines[i] === '---') { + if (openIdx === -1) openIdx = i; + else { closeIdx = i; break; } + } + } + assert.ok(openIdx !== -1 && closeIdx !== -1, `output must have a closed frontmatter block, got:\n${content}`); + return lines.slice(openIdx + 1, closeIdx); +} + +function findDescriptionLine(frontmatterLines) { + for (const line of frontmatterLines) { + if (line.startsWith('description:')) return line; + } + assert.fail(`no description line found in frontmatter:\n${frontmatterLines.join('\n')}`); + return ''; // unreachable +} + +function isQuotedYamlScalar(valueText) { + // YAML safe-quoted scalar: starts with `"` and ends with `"`, OR + // starts with `'` and ends with `'`. This is what `yamlQuote()` + // (JSON.stringify) and the Claude variant of these converters emit. + const trimmed = valueText.trim(); + if (trimmed.startsWith('"') && trimmed.endsWith('"')) return true; + if (trimmed.startsWith("'") && trimmed.endsWith("'")) return true; + return false; +} + +function parseQuotedYamlValue(valueText) { + const trimmed = valueText.trim(); + if (trimmed.startsWith('"')) return JSON.parse(trimmed); + if (trimmed.startsWith("'")) return trimmed.slice(1, -1).replace(/''/g, "'"); + return trimmed; +} + +function assertDescriptionRoundTrips(emitted, expected, label) { + const fmLines = extractFrontmatter(emitted); + const descLine = findDescriptionLine(fmLines); + const valueText = descLine.slice('description:'.length); + assert.ok( + isQuotedYamlScalar(valueText), + `(${label}) description must be a quoted YAML scalar (parser-safe for leading flow indicators). Got line: ${descLine}`, + ); + assert.strictEqual( + parseQuotedYamlValue(valueText), + expected, + `(${label}) description must round-trip through YAML quoting unchanged.`, + ); +} + +const COMMAND_CONVERTERS = [ + { label: 'convertClaudeCommandToCopilotSkill', fn: (src) => install.convertClaudeCommandToCopilotSkill(src, 'gsd-ultraplan-phase') }, + { label: 'convertClaudeCommandToAntigravitySkill', fn: (src) => install.convertClaudeCommandToAntigravitySkill(src, 'gsd-ultraplan-phase') }, + { label: 'convertClaudeCommandToTraeSkill', fn: (src) => install.convertClaudeCommandToTraeSkill(src, 'gsd-ultraplan-phase') }, + { label: 'convertClaudeCommandToCodebuddySkill', fn: (src) => install.convertClaudeCommandToCodebuddySkill(src, 'gsd-ultraplan-phase') }, +]; + +const AGENT_CONVERTERS = [ + { label: 'convertClaudeAgentToCopilotAgent', fn: (src) => install.convertClaudeAgentToCopilotAgent(src) }, + { label: 'convertClaudeAgentToAntigravityAgent', fn: (src) => install.convertClaudeAgentToAntigravityAgent(src) }, +]; + +// A grab-bag of leading characters that all break unquoted YAML scalar +// parsing per YAML 1.2 §7.3.3 / §6.9. The reporter's case is `[`; the +// rest defend against neighbouring drift. +const FLOW_HOSTILE_PREFIXES = ['[', '{', '*', '&', '!', '|', '>', '%', '@', '`']; + +// Some converters (Trae, CodeBuddy) deliberately rewrite "Claude Code" +// in body content to their target runtime name, and the rewrite cuts +// across the description too. That's correct behavior — out of scope for +// the YAML-quoting fix — so for the reporter case we assert only the +// quoting requirement, not byte-equality of the round-tripped value. +function assertDescriptionIsQuoted(emitted, label) { + const fmLines = extractFrontmatter(emitted); + const descLine = findDescriptionLine(fmLines); + const valueText = descLine.slice('description:'.length); + assert.ok( + isQuotedYamlScalar(valueText), + `(${label}) description must be a quoted YAML scalar (parser-safe for leading flow indicators). Got line: ${descLine}`, + ); +} + +describe('bug-2876: skill+agent converters emit YAML-quoted description', () => { + for (const { label, fn } of COMMAND_CONVERTERS) { + test(`${label}: reporter's "[BETA] ..." description is quoted`, () => { + const out = fn(buildClaudeCommand(REPORTER_DESCRIPTION)); + assertDescriptionIsQuoted(out, label); + }); + for (const prefix of FLOW_HOSTILE_PREFIXES) { + test(`${label}: leading ${JSON.stringify(prefix)} is quoted`, () => { + // Avoid leading/trailing `'` or `"` in the payload — `extractFrontmatterField` + // strips a single outer quote char of either kind regardless of whether + // the value was actually quoted, which would obscure the round-trip + // assertion. Pre-existing behavior, out of scope for #2876. + const desc = `${prefix} edge-case payload — flow indicator at start`; + const out = fn(buildClaudeCommand(desc)); + assertDescriptionRoundTrips(out, desc, `${label} prefix=${prefix}`); + }); + } + } + + for (const { label, fn } of AGENT_CONVERTERS) { + test(`${label}: reporter-shape "[BETA] ..." description is quoted`, () => { + const out = fn(buildClaudeAgent(REPORTER_DESCRIPTION)); + assertDescriptionIsQuoted(out, label); + }); + } +}); diff --git a/tests/copilot-install.test.cjs b/tests/copilot-install.test.cjs index d563cb5d5..6966e4ccc 100644 --- a/tests/copilot-install.test.cjs +++ b/tests/copilot-install.test.cjs @@ -20,6 +20,7 @@ const assert = require('node:assert/strict'); const path = require('path'); const os = require('os'); const fs = require('fs'); +const { parseFrontmatter } = require('./helpers.cjs'); const { getDirName, @@ -368,9 +369,10 @@ allowed-tools: Body content here referencing ~/.claude/foo and gsd:health.`; const result = convertClaudeCommandToCopilotSkill(input, 'gsd-health'); - assert.ok(result.startsWith('---\nname: gsd-health\n'), 'name uses param'); - assert.ok(result.includes('description: Diagnose planning directory health'), 'description preserved'); - assert.ok(result.includes('argument-hint: "[--repair]"'), 'argument-hint double-quoted'); + const fm = parseFrontmatter(result); + assert.equal(fm.name, 'gsd-health', 'name uses param'); + assert.equal(fm.description, 'Diagnose planning directory health', 'description preserved (quoted per #2876)'); + assert.equal(fm['argument-hint'], '[--repair]', 'argument-hint round-trips'); assert.ok(result.includes('allowed-tools: Read, Bash, Write, AskUserQuestion'), 'tools comma-separated'); assert.ok(result.includes('.github/foo'), 'CONV-06 applied to body (local mode default)'); assert.ok(result.includes('gsd-health'), 'CONV-07 applied to body'); @@ -386,9 +388,10 @@ description: Show available GSD commands Help content.`; const result = convertClaudeCommandToCopilotSkill(input, 'gsd-help'); - assert.ok(result.includes('name: gsd-help'), 'name set'); - assert.ok(result.includes('description: Show available GSD commands'), 'description preserved'); - assert.ok(!result.includes('allowed-tools:'), 'no allowed-tools line'); + const fm = parseFrontmatter(result); + assert.equal(fm.name, 'gsd-help', 'name set'); + assert.equal(fm.description, 'Show available GSD commands', 'description preserved'); + assert.ok(!('allowed-tools' in fm), 'no allowed-tools line'); }); test('handles skill without argument-hint', () => { @@ -530,9 +533,10 @@ color: yellow Body.`; const result = convertClaudeAgentToCopilotAgent(input); - assert.ok(result.includes('name: gsd-executor'), 'name preserved'); - assert.ok(result.includes('description: Executes GSD plans with atomic commits'), 'description preserved'); - assert.ok(result.includes('color: yellow'), 'color preserved'); + const fm = parseFrontmatter(result); + assert.equal(fm.name, 'gsd-executor', 'name preserved'); + assert.equal(fm.description, 'Executes GSD plans with atomic commits', 'description preserved'); + assert.equal(fm.color, 'yellow', 'color preserved'); }); test('handles mcp__context7__ tools', () => { @@ -659,13 +663,17 @@ describe('copyCommandsAsCopilotSkills', () => { assert.ok(fs.existsSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md')), 'gsd-autonomous/SKILL.md exists'); const skillContent = fs.readFileSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md'), 'utf8'); + const fm = parseFrontmatter(skillContent); // Frontmatter: name converted from gsd:autonomous to gsd-autonomous - assert.ok(skillContent.startsWith('---\nname: gsd-autonomous\n'), 'name is gsd-autonomous'); - assert.ok(skillContent.includes('description: Run all remaining phases autonomously'), - 'description preserved'); - // argument-hint present and double-quoted - assert.ok(skillContent.includes('argument-hint: "[--from N] [--to N] [--only N] [--interactive]"'), 'argument-hint present and quoted'); + assert.equal(fm.name, 'gsd-autonomous', 'name is gsd-autonomous'); + assert.equal( + fm.description, + 'Run all remaining phases autonomously — discuss→plan→execute per phase', + 'description preserved (round-trips through #2876 yamlQuote)', + ); + // argument-hint round-trips + assert.equal(fm['argument-hint'], '[--from N] [--to N] [--only N] [--interactive]', 'argument-hint round-trips'); // allowed-tools comma-separated assert.ok(skillContent.includes('allowed-tools: Read, Write, Bash, Glob, Grep, AskUserQuestion, Task'), 'allowed-tools is comma-separated'); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index cc07a8e8c..0289252a2 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -107,4 +107,60 @@ function cleanup(tmpDir) { fs.rmSync(tmpDir, { recursive: true, force: true }); } -module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, TOOLS_PATH }; +/** + * Parse a Markdown frontmatter block into a flat key→value map. + * + * Handles the YAML scalar forms emitted by the install converters: + * key: "json-encoded value" → JSON.parse + * key: 'value with ''escape'' → strip quotes, unescape '' + * key: bare value → trimmed string + * + * Multi-line and block scalars are out of scope — every converter in + * `bin/install.js` emits single-line scalars only. Throws if the content + * has no closed `---` block so a regression in the emitter shape fails + * loudly rather than silently returning {}. + * + * Tests use this helper instead of `result.includes('key: value')` to + * follow the project's "tests parse, never grep" convention. + * + * @param {string} content - Full file content beginning with `---`. + * @returns {Record} Map of frontmatter keys to decoded values. + */ +function parseFrontmatter(content) { + if (!content.startsWith('---')) { + throw new Error(`parseFrontmatter: content must start with '---', got: ${content.slice(0, 40)}`); + } + // CRLF tolerance: a Windows-authored file split on `\n` would leave a + // trailing `\r` on every line, making `lines[i] === '---'` fail to + // recognize delimiters. Same goes for whitespace-padded delimiter lines. + // Normalize via a CRLF-aware split + trimmed comparison. + const lines = content.split(/\r?\n/); + let openIdx = -1; + let closeIdx = -1; + for (let i = 0; i < lines.length; i += 1) { + if (lines[i].trim() === '---') { + if (openIdx === -1) openIdx = i; + else { closeIdx = i; break; } + } + } + if (openIdx === -1 || closeIdx === -1) { + throw new Error('parseFrontmatter: no closed --- block'); + } + const fields = {}; + for (const line of lines.slice(openIdx + 1, closeIdx)) { + const match = line.match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(.*)$/); + if (!match) continue; // skip block-list items, blank lines, comments + const [, key, rawValue] = match; + const value = rawValue.trim(); + if (value.startsWith('"') && value.endsWith('"') && value.length >= 2) { + fields[key] = JSON.parse(value); + } else if (value.startsWith("'") && value.endsWith("'") && value.length >= 2) { + fields[key] = value.slice(1, -1).replace(/''/g, "'"); + } else { + fields[key] = value; + } + } + return fields; +} + +module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, TOOLS_PATH };