fix: use double quotes in skill templates to prevent YAML parse errors#1903
fix: use double quotes in skill templates to prevent YAML parse errors#1903nikolasdehor wants to merge 1 commit intobmad-code-org:mainfrom
Conversation
Default skill templates used single-quoted YAML values for name and description fields. When descriptions contain apostrophes (e.g., "agent's workflow"), the generated YAML frontmatter would be invalid, causing parse failures in IDEs like Antigravity. Switches templates to double quotes and escapes any double quotes in the description during template rendering. Fixes bmad-code-org#1744
📝 WalkthroughWalkthroughThe pull request updates the Antigravity template generator to use double quotes for YAML frontmatter placeholders and escapes double quotes within the description field to prevent YAML parsing errors when descriptions contain single quotes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tools/cli/installers/lib/ide/_config-driven.js (3)
467-472:⚠️ Potential issue | 🟡 MinorPlease add regression cases beyond apostrophes.
This needs explicit coverage for embedded
", backslashes,:,#, and multiline descriptions. Those are the sequences most likely to break or silently truncate Antigravity frontmatter.Based on learnings:
:and#are confirmed parser traps unless the description is safely quoted.
467-472:⚠️ Potential issue | 🟠 MajorUnescaped descriptions in sibling YAML generators leave parse failures open.
agent-command-generator.js:80andcompiler.js:27both interpolate descriptions directly into YAML frontmatter without escaping double quotes or other special characters. These match the vulnerability fixed in_config-driven.js:471and must be hardened using the same escaping logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, The YAML frontmatter in agent-command-generator.js (near the description interpolation at/around line 80) and in compiler.js (around line 27) currently injects artifact/command descriptions unescaped; update those interpolation sites to escape double quotes the same way as in _config-driven.js (i.e., replace occurrences of " in the description with \" before inserting into YAML) so that descriptions are safe for YAML parsing—apply the same replaceAll('"', String.raw`\"`) (or a shared helper used by _config-driven.js) to the description value before interpolation.
467-472:⚠️ Potential issue | 🟠 MajorEscaping only
"is insufficient for YAML double-quoted scalars inrenderTemplate().Backslashes, newlines, and tabs are still emitted unescaped, causing YAML to misinterpret them. For example, descriptions containing Windows paths like
C:\new\toolswill be corrupted when parsed back from YAML.In YAML double-quoted strings, the sequence
\must be escaped as\\, actual newlines must be escaped as\\n, and tabs as\\t. The current code on line 471 only escapes", leaving these characters to be interpreted as YAML escape sequences.Add proper escaping to handle backslashes, newlines, tabs, and carriage returns in addition to quotes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, The description replacement in renderTemplate currently only escapes double quotes, which breaks YAML double-quoted scalars; update the code that builds the description replacement (the expression using artifact.description in the render block) to first escape backslashes, carriage returns, newlines and tabs (\\ -> \\\\, \r -> \\r, \n -> \\n, \t -> \\t) and then escape quotes (\"), so that the final substituted string is safe for YAML double-quoted scalars; locate the renderTemplate / the variable `rendered` where `.replaceAll('{{description}}', (artifact.description ...` is performed and apply these additional replaceAll steps to the description value before substitution.
🧹 Nitpick comments (5)
tools/cli/installers/lib/ide/_config-driven.js (5)
467-472: Validate rendered frontmatter before writing it.Right now flat-file installs trust raw string substitution. A cheap parse round-trip in tests—or a guard around generated frontmatter in this path—would catch invalid YAML before it ships again.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, After performing the template substitutions into the rendered string (the variable rendered which replaces '{{name}}','{{module}}','{{path}}','{{description}}','{{workflow_path}}'), validate the generated frontmatter before writing: extract the YAML frontmatter block (the leading --- ... ---), parse it with a YAML parser (e.g., js-yaml/YAML.parse) to ensure it round-trips or is syntactically valid, and fail loudly (throw or log and skip write) if parsing fails; add a unit/integration test that runs the same substitution and asserts the frontmatter parses successfully to guard future regressions.
467-472: Extract a named YAML-scalar escape helper.Burying the policy inside one chained
.replaceAll()is how this kind of bug reappears in the next emitter. A dedicated helper makes the contract obvious and reusable for bothnameanddescription.Based on learnings: a future PR should add an
escapeYamlDoubleQuotehelper intools/cli/installers/lib/ide/_config-driven.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, Extract a small helper named escapeYamlDoubleQuote that takes a string and returns it with double quotes escaped (e.g. replacing " with \" ), then replace the inline escaping logic in the replacement chain by calling that helper for both artifact.name and artifact.description; update the chain that currently mutates rendered (the .replaceAll('{{name}}', artifact.name || '') and .replaceAll('{{description}}', (artifact.description || `${artifact.name} ${artifact.type || ''}`).replaceAll('"', String.raw`\"`)) to call escapeYamlDoubleQuote(artifact.name || '') and escapeYamlDoubleQuote(artifact.description || `${artifact.name} ${artifact.type || ''}`) respectively so the escaping policy is centralized and reusable.
403-429: Stop hand-writing YAML frontmatter here.This string template now depends on every future edit remembering YAML quoting rules for
name,description, and any new keys. Serialize the frontmatter object withyaml.stringifyand keep only the body as literal template text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 403 - 429, Replace the hand-written YAML frontmatter in getDefaultTemplate with a serialized object using yaml.stringify: build a frontmatter object (include keys like name, description and conditionally disable-model-invocation when artifactType === 'agent'), call yaml.stringify(frontmatter), then concatenate that string with the remaining literal body text (the activation block for 'agent' and the LOAD/execute line for defaults). Import/require the yaml module (e.g., yaml.stringify) at the top, and ensure the template uses placeholders ({{name}}, {{description}}, {{bmadFolderName}}, {{path}}) only in the body portion while the frontmatter is produced from the object so future keys are safely quoted.
467-472: The escaping policy is already inconsistent across generators.
tools/cli/installers/lib/ide/shared/task-tool-command-generator.js:175-182still uses single-quote doubling, while this path now uses backslash escapes. Different emitters producing the same frontmatter field with different rules is brittle; centralize it by output format instead of per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, The frontmatter escaping is inconsistent: replace the per-file ad hoc escaping in _config-driven.js (the rendered.replaceAll chain using backslash escapes and variables rendered, artifact, pathToUse) by calling a shared utility like escapeFrontmatterValue(format, value) that encodes quotes according to the output format; implement this helper (e.g., in a new/common utils module) and update tools/cli/installers/lib/ide/_config-driven.js to use it for name, module, path, description, workflow_path and also change tools/cli/installers/lib/ide/shared/task-tool-command-generator.js to use the same helper so both generators use the same escape rules.
467-472: YAML escaping does not belong in a format-agnostic renderer.
renderTemplate()feeds.md,.toml,.yaml, and.ymltemplates. Hardcoding YAML-style backslashes into{{description}}means any non-YAML or non-frontmatter use of that placeholder will get corrupted output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 467 - 472, The renderer is applying YAML-specific escaping to the description placeholder (the .replaceAll('"', String.raw`\"`) in the renderTemplate output for '{{description}}'), which corrupts non-YAML outputs; change renderTemplate (or the code path that builds rendered) so it does NOT hardcode YAML escapes into artifact.description — either remove the .replaceAll call for '{{description}}' and leave raw text, or add an explicit format flag/parameter (e.g., format or fileExt) and only apply the escape when that flag indicates YAML/YML (and apply the appropriate escaping function there); target the replacement that uses '{{description}}' and artifact.description in _config-driven.js.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 406-407: The YAML frontmatter uses name: "{{name}}" without
escaping, so update the code path that calls renderTemplate() (the place that
injects artifact.name) to escape double-quotes and backslashes the same way
description is escaped: replace " with \" and \ with \\ before passing
artifact.name into the template. Locate the renderTemplate() call or the
variable that supplies artifact.name in _config-driven.js and apply the same
escaping function used for description (or add a small helper) so names like
C:\tools or Bob "QA" produce valid YAML.
---
Outside diff comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 467-472: The YAML frontmatter in agent-command-generator.js (near
the description interpolation at/around line 80) and in compiler.js (around line
27) currently injects artifact/command descriptions unescaped; update those
interpolation sites to escape double quotes the same way as in _config-driven.js
(i.e., replace occurrences of " in the description with \" before inserting into
YAML) so that descriptions are safe for YAML parsing—apply the same
replaceAll('"', String.raw`\"`) (or a shared helper used by _config-driven.js)
to the description value before interpolation.
- Around line 467-472: The description replacement in renderTemplate currently
only escapes double quotes, which breaks YAML double-quoted scalars; update the
code that builds the description replacement (the expression using
artifact.description in the render block) to first escape backslashes, carriage
returns, newlines and tabs (\\ -> \\\\, \r -> \\r, \n -> \\n, \t -> \\t) and
then escape quotes (\"), so that the final substituted string is safe for YAML
double-quoted scalars; locate the renderTemplate / the variable `rendered` where
`.replaceAll('{{description}}', (artifact.description ...` is performed and
apply these additional replaceAll steps to the description value before
substitution.
---
Nitpick comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 467-472: After performing the template substitutions into the
rendered string (the variable rendered which replaces
'{{name}}','{{module}}','{{path}}','{{description}}','{{workflow_path}}'),
validate the generated frontmatter before writing: extract the YAML frontmatter
block (the leading --- ... ---), parse it with a YAML parser (e.g.,
js-yaml/YAML.parse) to ensure it round-trips or is syntactically valid, and fail
loudly (throw or log and skip write) if parsing fails; add a unit/integration
test that runs the same substitution and asserts the frontmatter parses
successfully to guard future regressions.
- Around line 467-472: Extract a small helper named escapeYamlDoubleQuote that
takes a string and returns it with double quotes escaped (e.g. replacing " with
\" ), then replace the inline escaping logic in the replacement chain by calling
that helper for both artifact.name and artifact.description; update the chain
that currently mutates rendered (the .replaceAll('{{name}}', artifact.name ||
'') and .replaceAll('{{description}}', (artifact.description ||
`${artifact.name} ${artifact.type || ''}`).replaceAll('"', String.raw`\"`)) to
call escapeYamlDoubleQuote(artifact.name || '') and
escapeYamlDoubleQuote(artifact.description || `${artifact.name} ${artifact.type
|| ''}`) respectively so the escaping policy is centralized and reusable.
- Around line 403-429: Replace the hand-written YAML frontmatter in
getDefaultTemplate with a serialized object using yaml.stringify: build a
frontmatter object (include keys like name, description and conditionally
disable-model-invocation when artifactType === 'agent'), call
yaml.stringify(frontmatter), then concatenate that string with the remaining
literal body text (the activation block for 'agent' and the LOAD/execute line
for defaults). Import/require the yaml module (e.g., yaml.stringify) at the top,
and ensure the template uses placeholders ({{name}}, {{description}},
{{bmadFolderName}}, {{path}}) only in the body portion while the frontmatter is
produced from the object so future keys are safely quoted.
- Around line 467-472: The frontmatter escaping is inconsistent: replace the
per-file ad hoc escaping in _config-driven.js (the rendered.replaceAll chain
using backslash escapes and variables rendered, artifact, pathToUse) by calling
a shared utility like escapeFrontmatterValue(format, value) that encodes quotes
according to the output format; implement this helper (e.g., in a new/common
utils module) and update tools/cli/installers/lib/ide/_config-driven.js to use
it for name, module, path, description, workflow_path and also change
tools/cli/installers/lib/ide/shared/task-tool-command-generator.js to use the
same helper so both generators use the same escape rules.
- Around line 467-472: The renderer is applying YAML-specific escaping to the
description placeholder (the .replaceAll('"', String.raw`\"`) in the
renderTemplate output for '{{description}}'), which corrupts non-YAML outputs;
change renderTemplate (or the code path that builds rendered) so it does NOT
hardcode YAML escapes into artifact.description — either remove the .replaceAll
call for '{{description}}' and leave raw text, or add an explicit format
flag/parameter (e.g., format or fileExt) and only apply the escape when that
flag indicates YAML/YML (and apply the appropriate escaping function there);
target the replacement that uses '{{description}}' and artifact.description in
_config-driven.js.
🪄 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
Run ID: 72cc5d7a-53dd-4287-8546-e17db0e5f5c1
📒 Files selected for processing (1)
tools/cli/installers/lib/ide/_config-driven.js
| name: "{{name}}" | ||
| description: "{{description}}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the relevant lines in _config-driven.js
head -n 480 tools/cli/installers/lib/ide/_config-driven.js | tail -n 85Repository: bmad-code-org/BMAD-METHOD
Length of output: 2675
🏁 Script executed:
# Let's also check what the renderTemplate method does with name and description
rg -A 20 "renderTemplate.*{" tools/cli/installers/lib/ide/_config-driven.js | head -n 50Repository: bmad-code-org/BMAD-METHOD
Length of output: 166
🏁 Script executed:
# Let's search for how name is used throughout the file
rg "artifact\.name|{{name}}" tools/cli/installers/lib/ide/_config-driven.js -B 2 -A 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 182
🏁 Script executed:
# Check if there are similar escaping patterns in related generator files mentioned in learnings
rg "description.*replace|escapeYaml" tools/cli/installers/lib/ide/ -r --type jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 318
🏁 Script executed:
# Verify if name can contain special characters and trace how it's populated
rg -B 5 "artifact\.name\s*=" tools/cli/installers/lib/ide/_config-driven.js | head -n 40Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Check constructor and parameter sources to understand if name is user-controlled
rg "new\s+ConfigDrivenIdeSetup|\.name\s*=" tools/cli/installers/lib/ide/ -A 2 | head -n 50Repository: bmad-code-org/BMAD-METHOD
Length of output: 1721
🏁 Script executed:
# Verify the escaping is indeed incomplete by checking what the current description escaping handles
cat tools/cli/installers/lib/ide/_config-driven.js | grep -A 1 "description.*replaceAll"Repository: bmad-code-org/BMAD-METHOD
Length of output: 257
Escape {{name}} in double-quoted YAML frontmatter.
renderTemplate() injects artifact.name without escaping while the default templates now wrap it in double quotes (lines 406–407, 421–422). A name containing " or \ (e.g., C:\tools or Bob "QA") breaks YAML parsing and prevents the command/workflow/task/agent from loading.
Apply the same escaping logic as {{description}} (at minimum, replace " with \" and \ with \\ per YAML double-quote rules).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 406 - 407, The
YAML frontmatter uses name: "{{name}}" without escaping, so update the code path
that calls renderTemplate() (the place that injects artifact.name) to escape
double-quotes and backslashes the same way description is escaped: replace "
with \" and \ with \\ before passing artifact.name into the template. Locate the
renderTemplate() call or the variable that supplies artifact.name in
_config-driven.js and apply the same escaping function used for description (or
add a small helper) so names like C:\tools or Bob "QA" produce valid YAML.
1 participant
What
Switch default skill templates from single-quoted to double-quoted YAML values and escape descriptions during rendering.
Why
The
getDefaultTemplate()method in_config-driven.jswraps{{description}}in single quotes. When a workflow description contains an apostrophe (e.g., "agent's workflow"), the generated YAML frontmatter becomes invalid, causing parse failures in IDEs like Antigravity. This is the same class of issue fixed for Gemini templates in #1746.Fixes #1744
How
name: '{{name}}'→name: "{{name}}"in both agent and default templatesdescription: '{{description}}'→description: "{{description}}"in both templatesreplaceAll('"', String.raw\\"`)to description escaping inrenderTemplate()` for safetyTesting