fix(command-generation): escape YAML frontmatter name in codebuddy/crush/lingma/qoder#1330
fix(command-generation): escape YAML frontmatter name in codebuddy/crush/lingma/qoder#1330seattled23 wants to merge 1 commit into
Conversation
The codebuddy, crush, lingma, and qoder adapters interpolated
`name: ${content.name}` raw into command frontmatter. Every OPSX command name
contains a colon ("OPSX: Explore", "OPSX: Apply", ...), so the emitted
frontmatter is `name: OPSX: Explore` — invalid YAML ("mapping values are not
allowed here"). On a default `openspec init` for any of those 4 tools, the
generated slash-commands ship broken to the user's repo.
Fix routes the field through the existing `escapeYamlValue` helper, matching
the claude/windsurf/trae adapters. The prior escapeYamlValue refactor
(Fission-AI#1204/Fission-AI#1205) wired the helper into pi/claude/qwen but did not reach these 4.
Adds tests that parse each adapter's frontmatter with the real YAML library and
assert the name round-trips.
Co-authored-by: Sōren Vale <soren@tessara.us>
📝 WalkthroughWalkthroughFour command-generation adapters (CodeBuddy, Crush, Lingma, Qoder) now import and apply ChangesYAML frontmatter escaping across adapters
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/core/command-generation/adapters.test.ts (1)
844-879: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider expanding coverage to other escaped fields.
The suite name says "colon-bearing fields" (plural) but only
nameis tested with a colon. Sincedescription,category, andtagsare also now routed throughescapeYamlValue, adding at least one case where those fields contain YAML-special characters (e.g., colons, brackets) would strengthen the round-trip guarantee and catch regressions if an adapter reverts to raw interpolation.♻️ Suggested additional test case
it(`${name} emits parseable YAML frontmatter when the name contains a colon`, () => { const contentWithColon: CommandContent = { ...sampleContent, name: 'OPSX: Explore', }; const output = adapter.formatFile(contentWithColon); const frontmatter = extractFrontmatter(output); // Must parse without throwing and preserve the exact name (colon intact). const parsed = parseYaml(frontmatter) as Record<string, unknown>; expect(parsed.name).toBe('OPSX: Explore'); }); + + it(`${name} preserves all fields when multiple contain YAML-special characters`, () => { + const contentWithSpecialChars: CommandContent = { + ...sampleContent, + name: 'OPSX: Explore', + description: 'Explore: think, plan & act', + category: 'Workflow: Core', + tags: ['opsx:explore', 'plan & act'], + }; + + const output = adapter.formatFile(contentWithSpecialChars); + const frontmatter = extractFrontmatter(output); + + const parsed = parseYaml(frontmatter) as Record<string, unknown>; + expect(parsed.name).toBe('OPSX: Explore'); + expect(parsed.description).toBe('Explore: think, plan & act'); + expect(parsed.category).toBe('Workflow: Core'); + expect(parsed.tags).toEqual(['opsx:explore', 'plan & act']); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/command-generation/adapters.test.ts` around lines 844 - 879, Expand the YAML frontmatter coverage in the adapters test by adding cases for other escaped fields besides `name`, since `description`, `category`, and `tags` also flow through `escapeYamlValue`. Update the `yamlFrontmatterAdapters`/`extractFrontmatter` test block to include at least one input where `description`, `category`, or `tags` contains YAML-special characters like colons or brackets, then parse the emitted frontmatter with `parseYaml` and assert the value round-trips correctly. Use the existing adapter helpers (`formatFile`, `parseYaml`, `extractFrontmatter`) so the new test stays aligned with the current colon-handling checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/core/command-generation/adapters.test.ts`:
- Around line 844-879: Expand the YAML frontmatter coverage in the adapters test
by adding cases for other escaped fields besides `name`, since `description`,
`category`, and `tags` also flow through `escapeYamlValue`. Update the
`yamlFrontmatterAdapters`/`extractFrontmatter` test block to include at least
one input where `description`, `category`, or `tags` contains YAML-special
characters like colons or brackets, then parse the emitted frontmatter with
`parseYaml` and assert the value round-trips correctly. Use the existing adapter
helpers (`formatFile`, `parseYaml`, `extractFrontmatter`) so the new test stays
aligned with the current colon-handling checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37644f05-a474-4dbf-942d-8d3ff54dc113
📒 Files selected for processing (5)
src/core/command-generation/adapters/codebuddy.tssrc/core/command-generation/adapters/crush.tssrc/core/command-generation/adapters/lingma.tssrc/core/command-generation/adapters/qoder.tstest/core/command-generation/adapters.test.ts
Problem
The
codebuddy,crush,lingma, andqoderadapters interpolatename: ${content.name}raw into command frontmatter. Every OPSX command name contains a colon (OPSX: Explore,OPSX: Apply, …), so the emitted frontmatter is:which is invalid YAML — parsers throw
mapping values are not allowed here(reproduced). On a defaultopenspec initfor any of those 4 tools, the generated slash-commands ship broken into the user's repo.Fix
Route the field through the existing
escapeYamlValuehelper — the same oneclaude/windsurf/traealready use.OPSX: Explore→"OPSX: Explore", valid YAML, value preserved.The prior
escapeYamlValuerefactor (#1204 / #1205) wired the helper intopi/claude/qwenbut didn't reach these 4 adapters.Test
Adds tests that feed a colon-bearing name through each of the 4 adapters, extract the frontmatter, parse it with the real
yamllibrary, and assert the name round-trips.Note on scope (follow-up, not in this PR)
This PR fixes the field that actually breaks today —
name, because it's the one carrying a colon. For completeness: ~11 other adapters still interpolatedescription: ${content.description}raw (amazon-q, antigravity, auggie, codex, continue, factory, github-copilot, iflow, junie, kiro, opencode), andiflowalso emitscategory: ${content.category}raw. None break today (no current description/category value contains a colon), so it's a latent instance of the same class rather than a live bug. Happy to follow up with a sweep if you'd like it in one pass.Co-authored-by: Sōren Vale soren@tessara.us
Summary by CodeRabbit