fix(adapters): escape YAML frontmatter values consistently across all command adapters#1447
Open
hsusul wants to merge 1 commit into
Open
fix(adapters): escape YAML frontmatter values consistently across all command adapters#1447hsusul wants to merge 1 commit into
hsusul wants to merge 1 commit into
Conversation
… command adapters
Contributor
📝 WalkthroughWalkthroughChangesShared YAML helpers now handle empty values and tag arrays. Command-generation adapters use centralized escaping for YAML frontmatter fields, replacing several local helpers and raw interpolations. Adapter tests validate empty descriptions, normalized quoting, and parsing of descriptions containing YAML-sensitive characters. YAML frontmatter generation
Estimated code review effort: 3 (Moderate) | ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@test/core/command-generation/adapters.test.ts`:
- Around line 18-21: Update the YAML adapter matrix in the adapter tests to
include kiroAdapter and lingmaAdapter alongside the existing YAML-emitting
adapters. Keep kilocodeAdapter excluded from yamlAdapters because its dedicated
coverage verifies that it does not emit YAML frontmatter.
- Around line 977-997: Update escapeYamlValue, used by the adapter formatFile
flow, to quote values that YAML would interpret as non-string scalars, including
true, null, and numeric values, and to exclude indicator-prefixed text such as
“- starts here” from plain scalars. Extend the existing YAML frontmatter tests
with round-trip cases asserting these values parse back as the original strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b1b1ec2-de80-4b21-8370-9916276171d4
📒 Files selected for processing (23)
src/core/command-generation/adapters/amazon-q.tssrc/core/command-generation/adapters/antigravity.tssrc/core/command-generation/adapters/auggie.tssrc/core/command-generation/adapters/bob.tssrc/core/command-generation/adapters/claude.tssrc/core/command-generation/adapters/codebuddy.tssrc/core/command-generation/adapters/continue.tssrc/core/command-generation/adapters/costrict.tssrc/core/command-generation/adapters/crush.tssrc/core/command-generation/adapters/factory.tssrc/core/command-generation/adapters/github-copilot.tssrc/core/command-generation/adapters/iflow.tssrc/core/command-generation/adapters/junie.tssrc/core/command-generation/adapters/kiro.tssrc/core/command-generation/adapters/lingma.tssrc/core/command-generation/adapters/opencode.tssrc/core/command-generation/adapters/qoder.tssrc/core/command-generation/adapters/qwen.tssrc/core/command-generation/adapters/trae.tssrc/core/command-generation/adapters/windsurf.tssrc/core/command-generation/adapters/zcode.tssrc/core/command-generation/yaml.tstest/core/command-generation/adapters.test.ts
Comment on lines
+18
to
+21
| import { junieAdapter } from '../../../src/core/command-generation/adapters/junie.js'; | ||
| import { kilocodeAdapter } from '../../../src/core/command-generation/adapters/kilocode.js'; | ||
| import { kiroAdapter } from '../../../src/core/command-generation/adapters/kiro.js'; | ||
| import { lingmaAdapter } from '../../../src/core/command-generation/adapters/lingma.js'; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files | rg '(^|/)adapters\.(test|ts)$|src/core/command-generation/adapters/(junie|kilocode|kiro|lingma)\.js$' || true
printf '\nTarget adapter test outline:\n'
ast-grep outline test/core/command-generation/adapters.test.ts || true
printf '\nImports and relevant sections:\n'
sed -n '1,35p;920,985p' test/core/command-generation/adapters.test.ts
printf '\nAdapter files excerpts:\n'
for f in src/core/command-generation/adapters/junie.js src/core/command-generation/adapters/kilocode.js src/core/command-generation/adapters/kiro.js src/core/command-generation/adapters/lingma.js; do
if [ -f "$f" ]; then
echo "--- $f: $(wc -l < "$f") lines ---"
sed -n '1,220p' "$f"
fi
done
printf '\nSearch for adapter names and yamlAdapters declarations/usages:\n'
rg -n "yamlAdapters|kilocodeAdapter|junieAdapter|kiroAdapter|lingmaAdapter|command-generation|frontmatter|yml|yaml|\\.yml|\\.yaml|```" test/core/command-generation/adapters.test.ts src/core/command-generation/adapters -SRepository: Fission-AI/OpenSpec
Length of output: 5596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Adapter imports:\n'
sed -n '1,30p' test/core/command-generation/adapters.test.ts
printf '\nYAML frontmatter test section:\n'
sed -n '930,1015p' test/core/command-generation/adapters.test.ts
printf '\nAdapter files existence and frontmatter-related outputs:\n'
python3 - <<'PY'
from pathlib import Path
import re
files = [
Path('src/core/command-generation/adapters/junie.js'),
Path('src/core/command-generation/adapters/kilocode.js'),
Path('src/core/command-generation/adapters/kiro.js'),
Path('src/core/command-generation/adapters/lingma.js'),
]
for p in files:
print(f'--- {p} exists={p.exists()} lines={sum(1 for _ in open(p)) if p.exists() else None} ---')
if p.exists():
text = p.read_text()
imports = [line for line in text.splitlines() if 'from' in line or 'import' in line]
print('imports:')
for line in imports[:80]:
print(line)
print('frontmatter-like tokens:')
for pat in ['name:', 'description:', 'category:', 'tags:', 'file:', 'output:', '---', 'type:', 'command-generation', '--output', 'command_generation']:
for i, line in enumerate(text.splitlines(), 1):
if pat in line:
print(f'{i}: {line}')
PY
printf '\nAll adapter files containing output markers:\n'
rg -n "junie|kilocode|kiro|lingma|command-generation|---\\n|name:|description:|category:|tags:|type:|output" src/core/command-generation/adapters/*.js src/core/command-generation -SRepository: Fission-AI/OpenSpec
Length of output: 5857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository tree excerpt:\n'
git ls-files test/core/command-generation/adapters.test.ts src/core/command-generation/adapters | sed -n '1,120p'
printf '\nYAML adapter test continuation:\n'
sed -n '970,1020p' test/core/command-generation/adapters.test.ts
printf '\nSearch exact adapter names and output tokens:\n'
rg -n 'junieAdapter|kilocodeAdapter|kiroAdapter|lingmaAdapter|junie|kilocode|kiro|lingma|command-generation|toolId|formatFile|getFilePath|name:|description:|category:|tags:|type:|output' test/core/command-generation/adapters.test.ts src/core/command-generation -SRepository: Fission-AI/OpenSpec
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in src/core/command-generation/adapters/junie.ts src/core/command-generation/adapters/kilocode.ts src/core/command-generation/adapters/kiro.ts src/core/command-generation/adapters/lingma.ts; do
echo "--- $f ($(wc -l < "$f")) lines ---"
sed -n '1,220p' "$f"
done
printf '\nAdapters exporting YAML frontmatter:\n'
python3 - <<'PY'
from pathlib import Path
for p in Path('src/core/command-generation/adapters').glob('*.ts'):
text = p.read_text()
lines = text.splitlines()
if 'formatFile' in text:
start = next((i for i,l in enumerate(lines) if 'formatFile' in l), None)
if start is not None:
snippet = '\n'.join(lines[start:start+25]) if start is not None else ''
has_frontmatter = '---' in text
print(f'{p}:{start+1} toolId={next((l.strip() for l in lines if "toolId:" in l), "")} frontmatter={has_frontmatter}')
PYRepository: Fission-AI/OpenSpec
Length of output: 6049
Add kiroAdapter and lingmaAdapter to the YAML frontmatter matrix.
Both adapters emit YAML frontmatter, but neither is in yamlAdapters, so their escaping is not covered by this regression. Add them here while keeping kilocodeAdapter out because it has dedicated coverage that it does not emit ---.
🤖 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 18 - 21, Update
the YAML adapter matrix in the adapter tests to include kiroAdapter and
lingmaAdapter alongside the existing YAML-emitting adapters. Keep
kilocodeAdapter excluded from yamlAdapters because its dedicated coverage
verifies that it does not emit YAML frontmatter.
Comment on lines
+977
to
+997
| it.each(yamlAdapters)('$toolId formats valid YAML frontmatter when description contains colons and quotes', (adapter) => { | ||
| const specialContent: CommandContent = { | ||
| id: 'explore', | ||
| name: 'OpenSpec: Explore', | ||
| description: 'Explore mode: "thinking" & planning (e.g. feature: dark-mode)', | ||
| category: 'Workflow: Core', | ||
| tags: ['workflow:core', 'explore:mode'], | ||
| body: 'Body text', | ||
| }; | ||
|
|
||
| const fileContent = adapter.formatFile(specialContent); | ||
| const frontmatterMatch = fileContent.match(/^---\n([\s\S]*?)\n---/); | ||
| expect(frontmatterMatch).not.toBeNull(); | ||
|
|
||
| // Ensure frontmatter is valid YAML that parses without throwing | ||
| const { parse: parseYaml } = require('yaml'); | ||
| expect(() => parseYaml(frontmatterMatch![1])).not.toThrow(); | ||
|
|
||
| const parsed = parseYaml(frontmatterMatch![1]); | ||
| expect(parsed.description).toBe(specialContent.description); | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
node -e "const { parse } = require('yaml'); for (const v of ['true','null','123']) console.log(v, typeof parse('description: ' + v).description)"Repository: Fission-AI/OpenSpec
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the adapter test plus any escapeYamlValue implementation/usages.
printf 'Files matching adapters.test.ts:\n'
fd -a 'adapters\.test\.ts$' . | sed 's#^\./##'
printf '\nRelevant test section:\n'
test -f test/core/command-generation/adapters.test.ts && sed -n '940,1010p' test/core/command-generation/adapters.test.ts
printf '\nSymbol usages:\n'
rg -n "escapeYamlValue|yamlAdapters|formatFile|CommandContent" -S .
printf '\nPotential helper implementations:\n'
rg -n "function escapeYamlValue|const escapeYamlValue|escapeYamlValue\\(" tests ts src . --glob '!**/node_modules/**' --glob '!**/.next/**' 2>/dev/null || trueRepository: Fission-AI/OpenSpec
Length of output: 46370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'yaml helper source:\n'
sed -n '1,130p' src/core/command-generation/yaml.ts
printf '\nyaml helper existing tests:\n'
sed -n '1,120p' test/core/command-generation/yaml.test.ts
printf '\nBehavioral probe for escaped scalars generated by current helper:\n'
node - <<'JS'
const fs = require('fs');
const src = fs.readFileSync('src/core/command-generation/yaml.ts', 'utf8');
const fn = new Function('matchOnlyHexInBlockComment', src + '\nreturn escapeYamlValue;');
const escape = fn(false);
const yaml = require('yaml');
const values = ['true', 'null', '123', '1.23', '- starts here', '`@starts` here', '{starts here'];
for (const value of values) {
const emitted = `description: ${escape(value)}`;
try {
const parsed = yaml.parse(emitted);
const preserved = parsed.description === value;
const type = typeof parsed.description;
console.log(JSON.stringify({ emitted, value, parsed: parsed.description, type, preserved }));
} catch (err) {
console.log(JSON.stringify({ emitted, value, error: err.message }));
}
}
JSRepository: Fission-AI/OpenSpec
Length of output: 5433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Copy escapeYamlValue into a standalone JS probe and compare parser outcomes:\n'
node - <<'JS'
const yaml = require('yaml');
function escapeYamlValue(value) {
if (value === '') {
return '""';
}
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
if (needsQuoting) {
const escaped = value
.replace(/\\/g, '\\\\')
.replace(/"/g, '\\"')
.replace(/\n/g, '\\n')
.replace(/\r/g, '\\r');
return `"${escaped}"`;
}
return value;
}
const values = ['true', 'null', '123', '1.23', 'boolean', 'number', '- starts here', '`@starts` here', '{starts here', '<start: with colon'];
for (const value of values) {
const emitted = `description: ${escapeYamlValue(value)}`;
try {
const parsed = yaml.parse(emitted);
const preserved = parsed.description === value;
const type = typeof parsed.description;
console.log(JSON.stringify({ emitted, value, parsed: parsed.description, type, preserved }));
} catch (err) {
console.log(JSON.stringify({ emitted, value, error: err.message }));
}
}
JSRepository: Fission-AI/OpenSpec
Length of output: 1401
Quote YAML-tagged scalars in escapeYamlValue.
Current logic leaves values like true, null, and 123 unquoted, so the generated frontmatter can round-trip as non-string values. Add round-trip coverage and ensure these scalar tag values are quoted; also exclude indicators like - starts here from plain scalars.
🤖 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 977 - 997, Update
escapeYamlValue, used by the adapter formatFile flow, to quote values that YAML
would interpret as non-string scalars, including true, null, and numeric values,
and to exclude indicator-prefixed text such as “- starts here” from plain
scalars. Extend the existing YAML frontmatter tests with round-trip cases
asserting these values parse back as the original strings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Centralize and enforce YAML frontmatter value escaping across all tool command adapters.
Root Cause
Command adapters emitting YAML frontmatter (
amazon-q,antigravity,auggie,bob,codebuddy,continue,costrict,crush,factory,github-copilot,iflow,junie,kiro,lingma,opencode,qoder,qwen,trae,zcode) either omitted string escaping or re-implemented local un-escaped/duplicate variants ofescapeYamlValue. When command descriptions, names, categories, or tags contained YAML special characters (e.g. colons:, quotes", newlines\n, carriage returns\r, or brackets[]), generated command files contained invalid YAML frontmatter that failed YAML parsers.Fix Details
escapeYamlValueandformatTagsArrayfromsrc/core/command-generation/yaml.tsas the single authoritative helper module.escapeYamlValueto return""when given an empty string so empty fields produce valid YAML scalars instead of unquoted trailing whitespace.amazon-q,antigravity,auggie,bob,codebuddy,continue,costrict,crush,factory,github-copilot,iflow,junie,kiro,lingma,opencode,qoder,qwen,trae,windsurf,zcode) to import and use sharedescapeYamlValueandformatTagsArrayhelpers.test/core/command-generation/adapters.test.tsverifying that frontmatter emitted across all YAML adapters parses cleanly withyaml.parse.Verification
pnpm test(112 test files passed, 2275 tests passed)pnpm run build(Clean build completion)pnpm run lint(0 ESLint errors)pnpm tsc --noEmit(0 type errors)git diff --check(Clean formatting, no trailing whitespace)Summary by CodeRabbit
"".