From f957fcad206b6534c9bf89064d015ae941c4b6cf Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 10 Apr 2026 19:09:55 -0700 Subject: [PATCH] Fix: quote YAML scalars that contain colon-space in frontmatter generateYamlFrontmatter only re-quoted values starting with `[` or `{`, but parseFrontmatter strips surrounding quotes on input. Descriptions containing `: ` (e.g. "Also handles: critique...") round-tripped into unquoted plain scalars that YAML parsers reject. Added a yamlNeedsQuoting check covering colon-space, space-hash, YAML indicator chars, reserved keywords, and number-like strings, plus regression tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/lib/utils.js | 44 ++++++++++++++++++++++++++++++++++++----- tests/lib/utils.test.js | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index 3ea386c32..3f672e826 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -456,6 +456,41 @@ export function replacePlaceholders(content, provider, commandNames = [], allSki return result; } +/** + * Decide whether a YAML scalar string value must be quoted to survive parsing. + * + * Plain (unquoted) YAML scalars cannot contain `: ` or ` #`, cannot start with + * a YAML indicator character, cannot look like a boolean/null/number, and + * cannot carry leading/trailing whitespace. parseFrontmatter strips surrounding + * quotes on input, so we must re-detect the need to quote on output — otherwise + * descriptions like "Handles: critique/review..." round-trip into invalid YAML. + */ +function yamlNeedsQuoting(value) { + if (typeof value !== 'string') return false; + if (value === '') return true; + // Leading or trailing whitespace + if (/^\s|\s$/.test(value)) return true; + // Starts with a YAML flow/indicator character + if (/^[\[\]{},&*!|>'"%@`#]/.test(value)) return true; + // Starts with `?`, `:`, or `-` followed by space or end of string + if (/^[?:-](\s|$)/.test(value)) return true; + // Contains `: ` (ends plain scalar) or ` #` (starts comment), or ends with `:` + if (/: |\s#|:$/.test(value)) return true; + // Reserved keywords that YAML 1.1 parsers coerce to boolean/null + if (/^(true|false|null|yes|no|on|off|~)$/i.test(value)) return true; + // Looks like a number + if (/^-?\d+(\.\d+)?([eE][+-]?\d+)?$/.test(value)) return true; + return false; +} + +function formatYamlScalar(value) { + if (typeof value !== 'string') return String(value); + if (yamlNeedsQuoting(value)) { + return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`; + } + return value; +} + /** * Generate YAML frontmatter string */ @@ -467,18 +502,17 @@ export function generateYamlFrontmatter(data) { lines.push(`${key}:`); for (const item of value) { if (typeof item === 'object') { - lines.push(` - name: ${item.name}`); - if (item.description) lines.push(` description: ${item.description}`); + lines.push(` - name: ${formatYamlScalar(item.name)}`); + if (item.description) lines.push(` description: ${formatYamlScalar(item.description)}`); if (item.required !== undefined) lines.push(` required: ${item.required}`); } else { - lines.push(` - ${item}`); + lines.push(` - ${formatYamlScalar(item)}`); } } } else if (typeof value === 'boolean') { lines.push(`${key}: ${value}`); } else { - const needsQuoting = typeof value === 'string' && /^[\[{]/.test(value); - lines.push(`${key}: ${needsQuoting ? `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"` : value}`); + lines.push(`${key}: ${formatYamlScalar(value)}`); } } diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index 89cebd52d..7a0664005 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -166,6 +166,42 @@ describe('generateYamlFrontmatter', () => { expect(parsed.frontmatter.description).toBe(original.description); expect(parsed.frontmatter['argument-hint']).toBe(''); }); + + test('should quote strings containing colon-space (breaks plain scalars)', () => { + const data = { + name: 'impeccable', + description: 'Design fluency. Also handles: critique, audit. Commands: craft, polish.' + }; + + const result = generateYamlFrontmatter(data); + // Must be wrapped in quotes so YAML parsers don't mis-read the inner `: ` as a mapping + expect(result).toContain('description: "Design fluency. Also handles: critique, audit. Commands: craft, polish."'); + + // Roundtrip through our parser should recover the original string intact + const parsed = parseFrontmatter(`${result}\n\nbody`); + expect(parsed.frontmatter.description).toBe(data.description); + }); + + test('should quote strings starting with YAML flow indicators', () => { + const data = { + name: 'test', + 'argument-hint': '[command] [target]' + }; + + const result = generateYamlFrontmatter(data); + expect(result).toContain('argument-hint: "[command] [target]"'); + }); + + test('should not quote plain strings without special chars', () => { + const data = { + name: 'simple', + description: 'A plain description with no colons or hashes' + }; + + const result = generateYamlFrontmatter(data); + expect(result).toContain('description: A plain description with no colons or hashes'); + expect(result).not.toContain('"A plain'); + }); }); describe('ensureDir', () => {