fix(qwen): generate Markdown commands instead of deprecated TOML format#1191
fix(qwen): generate Markdown commands instead of deprecated TOML format#1191mc856 wants to merge 1 commit into
Conversation
Qwen Code deprecated TOML custom commands in favor of Markdown files with YAML frontmatter. Update the Qwen adapter to emit .qwen/commands/opsx-<id>.md and register the old opsx-*.toml files as legacy artifacts so they are cleaned up on update. Closes Fission-AI#838 Generated with Claude (Cowork) using claude-fable-5; tested with the full vitest suite (1663 tests passing).
📝 WalkthroughWalkthroughThis PR migrates Qwen command generation from deprecated TOML format to Markdown with YAML frontmatter, aligning with Qwen Code's current recommendations. The adapter gains a YAML-safe value escaper, legacy TOML artifacts are marked for cleanup detection, and tests are updated to validate the new format and proper character escaping. ChangesQwen Markdown Command Format Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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
🤖 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 `@src/core/command-generation/adapters/qwen.ts`:
- Around line 18-27: The escapeYamlValue function correctly detects values
needing quotes but doesn't escape carriage returns; update the escaping step in
escapeYamlValue to also replace '\r' with '\\r' (in addition to existing
backslash, double-quote, and newline escapes) so double-quoted YAML strings
conform to YAML 1.2; modify the replace chain inside escapeYamlValue
accordingly.
🪄 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: c1c9ab7b-d581-4492-929b-bc9c6e993f95
📒 Files selected for processing (7)
.changeset/qwen-markdown-commands.mddocs/supported-tools.mdsrc/core/command-generation/adapters/qwen.tssrc/core/legacy-cleanup.tstest/core/command-generation/adapters.test.tstest/core/legacy-cleanup.test.tstest/core/update.test.ts
| function escapeYamlValue(value: string): string { | ||
| // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) | ||
| const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); | ||
| if (needsQuoting) { | ||
| // Use double quotes and escape internal double quotes and backslashes | ||
| const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); | ||
| return `"${escaped}"`; | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Add carriage return escape sequence for complete YAML compliance.
The regex on line 20 correctly detects \r as requiring quotes, but line 23 doesn't escape it. Per the YAML 1.2 specification, carriage returns inside double-quoted strings must be escaped. While carriage returns are rare in description strings, adding the escape ensures valid YAML output.
🛡️ Proposed fix to add carriage return escaping
// Use double quotes and escape internal double quotes and backslashes
- const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
+ const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n').replace(/\r/g, '\\r');
return `"${escaped}"`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function escapeYamlValue(value: string): string { | |
| // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) | |
| const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); | |
| if (needsQuoting) { | |
| // Use double quotes and escape internal double quotes and backslashes | |
| const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); | |
| return `"${escaped}"`; | |
| } | |
| return value; | |
| } | |
| function escapeYamlValue(value: string): string { | |
| // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) | |
| const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); | |
| if (needsQuoting) { | |
| // Use double quotes and escape internal double quotes and backslashes | |
| const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n').replace(/\r/g, '\\r'); | |
| return `"${escaped}"`; | |
| } | |
| return value; | |
| } |
🤖 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 `@src/core/command-generation/adapters/qwen.ts` around lines 18 - 27, The
escapeYamlValue function correctly detects values needing quotes but doesn't
escape carriage returns; update the escaping step in escapeYamlValue to also
replace '\r' with '\\r' (in addition to existing backslash, double-quote, and
newline escapes) so double-quoted YAML strings conform to YAML 1.2; modify the
replace chain inside escapeYamlValue accordingly.
There was a problem hiding this comment.
Good catch on the detect/escape asymmetry. This function is an intentional verbatim copy of the existing escapeYamlValue in pi.ts and claude.ts (current convention is one copy per adapter), so the same gap exists in both siblings. Since descriptions come from internal command templates, a literal CR can't occur in practice. Rather than diverging this copy from the other two or expanding this PR's scope, I'd prefer a follow-up PR that extracts escapeYamlValue into a shared util and adds \r escaping in one place. Happy to fix it here instead if maintainers prefer.
There was a problem hiding this comment.
@mc856, that context makes sense — the detect/escape asymmetry is a pre-existing gap shared across all three adapters, and fixing it consistently via a shared utility is the right approach. Happy to leave this PR as-is.
Would you like me to open a follow-up GitHub issue to track:
- Extracting
escapeYamlValueinto a shared YAML utility - Adding
\r → \\rescaping in that shared location - Updating
pi.ts,claude.ts, andqwen.tsto consume it
Just let me know and I'll file it.
✏️ Learnings added
Learnt from: mc856
Repo: Fission-AI/OpenSpec PR: 1191
File: src/core/command-generation/adapters/qwen.ts:18-27
Timestamp: 2026-06-09T19:17:10.070Z
Learning: In the OpenSpec repository, `escapeYamlValue` is intentionally duplicated per adapter file (e.g., `src/core/command-generation/adapters/pi.ts`, `claude.ts`, `qwen.ts`) — one copy per adapter is the current convention. There is a known detect/escape asymmetry where `\r` is detected as needing quoting but is not escaped in the replace chain; this gap exists in all three siblings and is tracked for a future refactor that would extract the helper into a shared util.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
Summary
Qwen Code has deprecated TOML custom commands in favor of Markdown files with YAML frontmatter (docs). This PR updates the Qwen adapter to emit
.qwen/commands/opsx-<id>.mdand registers the oldopsx-*.tomlfiles as legacy artifacts so they are cleaned up onupdate.Closes #838
Changes
qwenadapter now generates Markdown withdescriptionfrontmatter (YAML values escaped, same as thepi/claudeadapters)legacy-cleanupgains a.qwen/commands/opsx-*.tomlpattern, following the junie/opencode precedent.mdfiles are not flagged as legacydocs/supported-tools.mdand changeset updatedDesign decisions
{{args}}, but the old TOML adapter relied on Qwen's default argument appending, so this PR intentionally preserves that behavior for parity. Explicit{{args}}injection could be a follow-up..qwen/commands/opsx-*.tomlfiles are cleaned up via the same legacy-cleanup mechanism used for the junie/opencode migrations (interactive confirm onupdate, auto with--force). Non-interactive runs without--forcewarn and leave the old files in place, matching existing behavior.Testing
npx vitest run— full suite passes (1664 tests)npx eslintclean on changed filesGenerated with Claude (Cowork) using claude-fable-5.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation