Skip to content

Commit e559bee

Browse files
fix: handle YAML parsing edge cases in CustomModesManager (#5099)
* fix: handle YAML parsing edge cases in CustomModesManager - Add BOM (Byte Order Mark) stripping for UTF-8 and UTF-16 - Normalize invisible characters including non-breaking spaces - Replace fancy quotes and dashes with standard characters - Remove zero-width characters that can cause parsing issues - Add comprehensive test coverage for all edge cases This fixes the YAML parsing limitations documented in PR #237 by implementing proper preprocessing before parsing YAML content. * fix: address PR review comments - Fix BOM handling to correctly handle UTF-16 (all BOMs appear as \uFEFF when decoded) - Optimize cleanInvisibleCharacters with single regex pass for better performance - Prevent duplicate error messages by marking errors as already handled - Refactor test file to use mockFsReadFile helper function to reduce duplication - Fix YAML indentation in tests (use spaces instead of tabs) - Add ESLint disable comment for character class warning (regex is correct) * fix: prevent YAML line breaks by setting lineWidth to 0 - Added lineWidth: 0 option to all yaml.stringify() calls - Prevents automatic line wrapping at 80 characters - Improves readability of YAML output for long strings - Applied to CustomModesManager, SimpleInstaller, and migrateSettings * fix: add defaultStringType option to yaml.stringify calls - Added defaultStringType: 'PLAIN' to minimize formatting changes - This helps preserve plain scalars when possible - Works alongside lineWidth: 0 to prevent automatic line wrapping * refactor: extract problematic characters regex as a named constant - Move regex pattern to PROBLEMATIC_CHARS_REGEX static constant - Add comprehensive documentation for each character range - Improves maintainability and makes the pattern reusable * test: add comprehensive edge case tests for YAML parsing - Add test for mixed line endings (CRLF vs LF) - Add test for multiple BOMs in sequence - Add test for deeply nested structures with problematic characters - Ensures robustness across different real-world scenarios * feat(i18n): add error messages for custom modes in multiple languages * fix: update tests to expect i18n keys instead of hardcoded strings - Update CustomModesManager tests to expect translation keys - Fix YAML edge case tests to match new i18n error messages - All tests now pass with the i18n integration * refactor: use strip-bom package and fix error handling - Replace custom stripBOM method with existing strip-bom package - Fix duplicate error handling in parseYamlSafely by returning empty object instead of re-throwing - Addresses review comments from PR #5099 --------- Co-authored-by: Daniel Riccio <[email protected]>
1 parent 896e986 commit e559bee

File tree

23 files changed

+806
-26
lines changed

23 files changed

+806
-26
lines changed

src/core/config/CustomModesManager.ts

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as path from "path"
33
import * as fs from "fs/promises"
44

55
import * as yaml from "yaml"
6+
import stripBom from "strip-bom"
67

78
import { type ModeConfig, customModesSettingsSchema } from "@roo-code/types"
89

@@ -11,6 +12,7 @@ import { getWorkspacePath } from "../../utils/path"
1112
import { logger } from "../../utils/logging"
1213
import { GlobalFileNames } from "../../shared/globalFileNames"
1314
import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
15+
import { t } from "../../i18n"
1416

1517
const ROOMODES_FILENAME = ".roomodes"
1618

@@ -73,12 +75,88 @@ export class CustomModesManager {
7375
return exists ? roomodesPath : undefined
7476
}
7577

78+
/**
79+
* Regex pattern for problematic characters that need to be cleaned from YAML content
80+
* Includes:
81+
* - \u00A0: Non-breaking space
82+
* - \u200B-\u200D: Zero-width spaces and joiners
83+
* - \u2010-\u2015, \u2212: Various dash characters
84+
* - \u2018-\u2019: Smart single quotes
85+
* - \u201C-\u201D: Smart double quotes
86+
*/
87+
private static readonly PROBLEMATIC_CHARS_REGEX =
88+
// eslint-disable-next-line no-misleading-character-class
89+
/[\u00A0\u200B\u200C\u200D\u2010\u2011\u2012\u2013\u2014\u2015\u2212\u2018\u2019\u201C\u201D]/g
90+
91+
/**
92+
* Clean invisible and problematic characters from YAML content
93+
*/
94+
private cleanInvisibleCharacters(content: string): string {
95+
// Single pass replacement for all problematic characters
96+
return content.replace(CustomModesManager.PROBLEMATIC_CHARS_REGEX, (match) => {
97+
switch (match) {
98+
case "\u00A0": // Non-breaking space
99+
return " "
100+
case "\u200B": // Zero-width space
101+
case "\u200C": // Zero-width non-joiner
102+
case "\u200D": // Zero-width joiner
103+
return ""
104+
case "\u2018": // Left single quotation mark
105+
case "\u2019": // Right single quotation mark
106+
return "'"
107+
case "\u201C": // Left double quotation mark
108+
case "\u201D": // Right double quotation mark
109+
return '"'
110+
default: // Dash characters (U+2010 through U+2015, U+2212)
111+
return "-"
112+
}
113+
})
114+
}
115+
116+
/**
117+
* Parse YAML content with enhanced error handling and preprocessing
118+
*/
119+
private parseYamlSafely(content: string, filePath: string): any {
120+
// Clean the content
121+
let cleanedContent = stripBom(content)
122+
cleanedContent = this.cleanInvisibleCharacters(cleanedContent)
123+
124+
try {
125+
return yaml.parse(cleanedContent)
126+
} catch (error) {
127+
const errorMsg = error instanceof Error ? error.message : String(error)
128+
console.error(`[CustomModesManager] Failed to parse YAML from ${filePath}:`, errorMsg)
129+
130+
// Show user-friendly error message for .roomodes files
131+
if (filePath.endsWith(ROOMODES_FILENAME)) {
132+
const lineMatch = errorMsg.match(/at line (\d+)/)
133+
const line = lineMatch ? lineMatch[1] : "unknown"
134+
vscode.window.showErrorMessage(t("common:customModes.errors.yamlParseError", { line }))
135+
}
136+
137+
// Return empty object to prevent duplicate error handling
138+
return {}
139+
}
140+
}
141+
76142
private async loadModesFromFile(filePath: string): Promise<ModeConfig[]> {
77143
try {
78144
const content = await fs.readFile(filePath, "utf-8")
79-
const settings = yaml.parse(content)
145+
const settings = this.parseYamlSafely(content, filePath)
80146
const result = customModesSettingsSchema.safeParse(settings)
147+
81148
if (!result.success) {
149+
console.error(`[CustomModesManager] Schema validation failed for ${filePath}:`, result.error)
150+
151+
// Show user-friendly error for .roomodes files
152+
if (filePath.endsWith(ROOMODES_FILENAME)) {
153+
const issues = result.error.issues
154+
.map((issue) => `• ${issue.path.join(".")}: ${issue.message}`)
155+
.join("\n")
156+
157+
vscode.window.showErrorMessage(t("common:customModes.errors.schemaValidationError", { issues }))
158+
}
159+
82160
return []
83161
}
84162

@@ -89,8 +167,11 @@ export class CustomModesManager {
89167
// Add source to each mode
90168
return result.data.customModes.map((mode) => ({ ...mode, source }))
91169
} catch (error) {
92-
const errorMsg = `Failed to load modes from ${filePath}: ${error instanceof Error ? error.message : String(error)}`
93-
console.error(`[CustomModesManager] ${errorMsg}`)
170+
// Only log if the error wasn't already handled in parseYamlSafely
171+
if (!(error as any).alreadyHandled) {
172+
const errorMsg = `Failed to load modes from ${filePath}: ${error instanceof Error ? error.message : String(error)}`
173+
console.error(`[CustomModesManager] ${errorMsg}`)
174+
}
94175
return []
95176
}
96177
}
@@ -124,7 +205,12 @@ export class CustomModesManager {
124205
const fileExists = await fileExistsAtPath(filePath)
125206

126207
if (!fileExists) {
127-
await this.queueWrite(() => fs.writeFile(filePath, yaml.stringify({ customModes: [] })))
208+
await this.queueWrite(() =>
209+
fs.writeFile(
210+
filePath,
211+
yaml.stringify({ customModes: [] }, { lineWidth: 0, defaultStringType: "PLAIN" }),
212+
),
213+
)
128214
}
129215

130216
return filePath
@@ -147,13 +233,12 @@ export class CustomModesManager {
147233
await this.getCustomModesFilePath()
148234
const content = await fs.readFile(settingsPath, "utf-8")
149235

150-
const errorMessage =
151-
"Invalid custom modes format. Please ensure your settings follow the correct YAML format."
236+
const errorMessage = t("common:customModes.errors.invalidFormat")
152237

153238
let config: any
154239

155240
try {
156-
config = yaml.parse(content)
241+
config = this.parseYamlSafely(content, settingsPath)
157242
} catch (error) {
158243
console.error(error)
159244
vscode.window.showErrorMessage(errorMessage)
@@ -284,7 +369,7 @@ export class CustomModesManager {
284369

285370
if (!workspaceFolders || workspaceFolders.length === 0) {
286371
logger.error("Failed to update project mode: No workspace folder found", { slug })
287-
throw new Error("No workspace folder found for project-specific mode")
372+
throw new Error(t("common:customModes.errors.noWorkspaceForProject"))
288373
}
289374

290375
const workspaceRoot = getWorkspacePath()
@@ -318,7 +403,7 @@ export class CustomModesManager {
318403
} catch (error) {
319404
const errorMessage = error instanceof Error ? error.message : String(error)
320405
logger.error("Failed to update custom mode", { slug, error: errorMessage })
321-
vscode.window.showErrorMessage(`Failed to update custom mode: ${errorMessage}`)
406+
vscode.window.showErrorMessage(t("common:customModes.errors.updateFailed", { error: errorMessage }))
322407
}
323408
}
324409

@@ -329,20 +414,20 @@ export class CustomModesManager {
329414
content = await fs.readFile(filePath, "utf-8")
330415
} catch (error) {
331416
// File might not exist yet.
332-
content = yaml.stringify({ customModes: [] })
417+
content = yaml.stringify({ customModes: [] }, { lineWidth: 0, defaultStringType: "PLAIN" })
333418
}
334419

335420
let settings
336421

337422
try {
338-
settings = yaml.parse(content)
423+
settings = this.parseYamlSafely(content, filePath)
339424
} catch (error) {
340-
console.error(`[CustomModesManager] Failed to parse YAML from ${filePath}:`, error)
425+
// Error already logged in parseYamlSafely
341426
settings = { customModes: [] }
342427
}
343428

344429
settings.customModes = operation(settings.customModes || [])
345-
await fs.writeFile(filePath, yaml.stringify(settings), "utf-8")
430+
await fs.writeFile(filePath, yaml.stringify(settings, { lineWidth: 0, defaultStringType: "PLAIN" }), "utf-8")
346431
}
347432

348433
private async refreshMergedState(): Promise<void> {
@@ -373,7 +458,7 @@ export class CustomModesManager {
373458
const globalMode = settingsModes.find((m) => m.slug === slug)
374459

375460
if (!projectMode && !globalMode) {
376-
throw new Error("Write error: Mode not found")
461+
throw new Error(t("common:customModes.errors.modeNotFound"))
377462
}
378463

379464
await this.queueWrite(async () => {
@@ -392,23 +477,24 @@ export class CustomModesManager {
392477
await this.refreshMergedState()
393478
})
394479
} catch (error) {
395-
vscode.window.showErrorMessage(
396-
`Failed to delete custom mode: ${error instanceof Error ? error.message : String(error)}`,
397-
)
480+
const errorMessage = error instanceof Error ? error.message : String(error)
481+
vscode.window.showErrorMessage(t("common:customModes.errors.deleteFailed", { error: errorMessage }))
398482
}
399483
}
400484

401485
public async resetCustomModes(): Promise<void> {
402486
try {
403487
const filePath = await this.getCustomModesFilePath()
404-
await fs.writeFile(filePath, yaml.stringify({ customModes: [] }))
488+
await fs.writeFile(
489+
filePath,
490+
yaml.stringify({ customModes: [] }, { lineWidth: 0, defaultStringType: "PLAIN" }),
491+
)
405492
await this.context.globalState.update("customModes", [])
406493
this.clearCache()
407494
await this.onUpdate()
408495
} catch (error) {
409-
vscode.window.showErrorMessage(
410-
`Failed to reset custom modes: ${error instanceof Error ? error.message : String(error)}`,
411-
)
496+
const errorMessage = error instanceof Error ? error.message : String(error)
497+
vscode.window.showErrorMessage(t("common:customModes.errors.resetFailed", { error: errorMessage }))
412498
}
413499
}
414500

src/core/config/__tests__/CustomModesManager.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,7 @@ describe("CustomModesManager", () => {
754754

755755
await manager.deleteCustomMode("non-existent-mode")
756756

757-
expect(mockShowError).toHaveBeenCalledWith(expect.stringContaining("Write error"))
757+
expect(mockShowError).toHaveBeenCalledWith("customModes.errors.deleteFailed")
758758
})
759759
})
760760

0 commit comments

Comments
 (0)