Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/fix-yaml-frontmatter-escaping/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-06-20
32 changes: 32 additions & 0 deletions openspec/changes/fix-yaml-frontmatter-escaping/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## Context

The `writeMemoryFile` function in `src/memory/writer.js` generates YAML frontmatter for memory files. String values (title and frontmatter fields) are wrapped in double quotes without escaping special characters. When a string contains double quotes, backslashes, or newlines, the resulting YAML is malformed and cannot be parsed correctly by the memory reader in `src/memory/index.js`.

## Goals / Non-Goals

**Goals:**
- Add a dedicated `escapeYamlString()` helper that properly escapes backslashes, double quotes, and newlines for YAML double-quoted strings
- Apply escaping to the title field and all string frontmatter values
- Update the test helper in `tests/unit/memory.test.js` to mirror the escaping logic
- Add unit tests covering special character edge cases

**Non-Goals:**
- Changes to the YAML parsing logic in `src/memory/index.js` (it already handles unescaping via quote stripping)
- Changes to memory file naming or directory structure
- Adding a YAML library dependency

## Decisions

1. **Use string replacement rather than a YAML library** — The escaping requirement is simple (backslashes, quotes, newlines). Importing a library would add unnecessary dependencies for a focused fix.

2. **Escape order: backslashes first, then quotes, then newlines** — Backslashes must be escaped first to avoid double-escaping. If we escaped quotes first, the backslashes added by quote escaping would themselves need escaping.

3. **Internal helper, not exported** — The `escapeYamlString()` function is only needed for frontmatter generation within writer.js. Exporting it would expose an implementation detail with no external consumers.

4. **Double-quoted YAML strings** — The existing code uses double quotes for all string values. We maintain this convention rather than switching to single quotes or block scalars, minimizing the scope of change.

## Risks / Trade-offs

- [Risk] Escape order could produce incorrect output if not implemented carefully → Mitigation: Unit tests verify all three escape types independently and in combination
- [Risk] Existing tests don't cover special characters → Mitigation: New tests added specifically for quotes, backslashes, and newlines
- [Trade-off] Simple string replacement may not handle all YAML edge cases (e.g., tab characters) → Acceptable: The most common problematic characters are covered; future enhancements can expand if needed
26 changes: 26 additions & 0 deletions openspec/changes/fix-yaml-frontmatter-escaping/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
## Why

The `writeMemoryFile` function in `src/memory/writer.js` writes YAML frontmatter with string values wrapped in double quotes, but does not escape special characters within those strings. This causes YAML parsing errors when titles or frontmatter values contain double quotes, backslashes, or newlines. This is a medium-severity bug that can corrupt memory files and prevent them from being read correctly.

## What Changes

- Add an `escapeYamlString()` helper function that properly escapes backslashes, double quotes, and newlines for YAML double-quoted strings
- Apply escaping to the `title` field in the frontmatter output
- Apply escaping to all string frontmatter values before writing
- Update the test helper `buildMemoryContent` in `tests/unit/memory.test.js` to mirror the escaping logic
- Add unit tests for strings containing quotes, backslashes, and newlines

## Capabilities

### New Capabilities
<!-- No new capabilities — this is a bug fix -->

### Modified Capabilities
- `memory`: The memory writer now properly escapes special characters in YAML frontmatter string values, ensuring valid YAML output for titles and frontmatter containing quotes, backslashes, or newlines.

## Impact

- Affected code: `src/memory/writer.js` (escape helper + application to title and string frontmatter values)
- Affected tests: `tests/unit/memory.test.js` (update test helper + add new escaping tests)
- No API or dependency changes
- No breaking changes — existing valid YAML output remains unchanged
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## MODIFIED Requirements

### Requirement: Conversation Persistence
The system SHALL persist every conversation exchange (user input, LLM output, tool results) as a timestamped markdown file in the `memory/` directory. YAML frontmatter metadata (timestamp, provider, model, title, and any additional fields) SHALL be properly escaped to ensure valid YAML output when values contain special characters such as double quotes, backslashes, or newlines.

#### Scenario: System persists a conversation message
- **WHEN** the user sends a message or the LLM generates a response
- **THEN** a new markdown file is written to `memory/` containing the exchange with YAML frontmatter for metadata (timestamp, provider, model)

#### Scenario: System reads conversation history from memory
- **WHEN** the user reopens the TUI after exiting
- **THEN** the system loads the latest conversation file from `memory/` and displays the history in the conversation panel

#### Scenario: Frontmatter with special characters is valid YAML
- **WHEN** a title or frontmatter value contains double quotes, backslashes, or newlines
- **THEN** the generated YAML frontmatter is properly escaped and can be parsed without errors
19 changes: 19 additions & 0 deletions openspec/changes/fix-yaml-frontmatter-escaping/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
## 1. Implement escapeYamlString helper

- [ ] 1.1 Add escapeYamlString() function to src/memory/writer.js that escapes backslashes (\\ → \\\\), double quotes (" → \\"), and newlines (\\n → \\\\n) in the correct order
- [ ] 1.2 Apply escapeYamlString() to the title field on line 27 of writeMemoryFile
- [ ] 1.3 Apply escapeYamlString() to string frontmatter values on line 31 of writeMemoryFile

## 2. Update tests

- [ ] 2.1 Update the buildMemoryContent test helper in tests/unit/memory.test.js to use the same escapeYamlString logic
- [ ] 2.2 Add unit test for title containing double quotes
- [ ] 2.3 Add unit test for title containing backslashes
- [ ] 2.4 Add unit test for title containing newlines
- [ ] 2.5 Add unit test for frontmatter string values with special characters

## 3. Verify

- [ ] 3.1 Run full test suite (npm run test) and verify all tests pass
- [ ] 3.2 Run lint (npm run lint) and verify no lint errors
- [ ] 3.3 Verify application starts without crashing (npm start)
16 changes: 13 additions & 3 deletions src/memory/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@ import { join } from "node:path";

const FRONTMATTER_DELIMITER = "---";

/**
* Escape a string value for safe inclusion in a YAML double-quoted scalar.
* Escapes backslashes first, then double quotes, then newlines.
* @param {string} str - The string to escape
* @returns {string} The escaped string
*/
function escapeYamlString(str) {
return str.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n");
}

/**
* Write a memory file with YAML frontmatter.
* Creates a timestamped markdown file in the specified directory.
Expand All @@ -24,11 +34,11 @@ export function writeMemoryFile(directory, title, frontmatter, body = "") {

const lines = [
FRONTMATTER_DELIMITER,
`title: "${title}"`,
`timestamp: "${timestamp}"`,
`title: "${escapeYamlString(title)}"`,
`timestamp: "${escapeYamlString(timestamp)}"`,
...Object.entries(frontmatter).map(([k, v]) => {
if (v == null) return `${k}:`;
if (typeof v === "string") return `${k}: "${v}"`;
if (typeof v === "string") return `${k}: "${escapeYamlString(v)}"`;
if (typeof v === "boolean") return `${k}: ${v}`;
if (typeof v === "number") return `${k}: ${v}`;
return `${k}: ${JSON.stringify(v)}`;
Expand Down
33 changes: 30 additions & 3 deletions tests/unit/memory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ describe("frontmatter parsing", () => {
});

describe("memory file writer logic", () => {
/**
* Escape a string value for safe inclusion in a YAML double-quoted scalar.
*/
function escapeYamlString(str) {
return str.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n");
}

/**
* Replicate the core logic of writeMemoryFile without filesystem access.
*/
Expand All @@ -97,11 +104,11 @@ describe("memory file writer logic", () => {
.replace(/^-|-$/g, "");
const lines = [
"---",
`title: "${title}"`,
`timestamp: "${timestamp}"`,
`title: "${escapeYamlString(title)}"`,
`timestamp: "${escapeYamlString(timestamp)}"`,
...Object.entries(frontmatter).map(([k, v]) => {
if (v == null) return `${k}:`;
if (typeof v === "string") return `${k}: "${v}"`;
if (typeof v === "string") return `${k}: "${escapeYamlString(v)}"`;
if (typeof v === "boolean") return `${k}: ${v}`;
if (typeof v === "number") return `${k}: ${v}`;
return `${k}: ${JSON.stringify(v)}`;
Expand Down Expand Up @@ -149,6 +156,26 @@ describe("memory file writer logic", () => {
assert.ok(content.includes("extra:"));
});

it("escapes double quotes in title", () => {
const content = buildMemoryContent('Title with "quotes"', {});
assert.ok(content.includes('title: "Title with \\"quotes\\""'));
});

it("escapes backslashes in title", () => {
const content = buildMemoryContent("C:\\path\\to\\file", {});
assert.ok(content.includes('title: "C:\\\\path\\\\to\\\\file"'));
});

it("escapes newlines in title", () => {
const content = buildMemoryContent("Line1\nLine2", {});
assert.ok(content.includes('title: "Line1\\nLine2"'));
});

it("escapes special characters in frontmatter string values", () => {
const content = buildMemoryContent("Test", { note: 'He said "hello\\there"' });
assert.ok(content.includes('note: "He said \\"hello\\\\there\\""'));
});

describe("memory index search logic", () => {
function searchIndex(entries, query) {
if (!query) return [];
Expand Down