Skip to content

fix(sources): use shared yamlEscape in buildFrontmatter#34

Merged
harlan-zw merged 1 commit intomainfrom
fix/frontmatter-yaml-escaping
Mar 17, 2026
Merged

fix(sources): use shared yamlEscape in buildFrontmatter#34
harlan-zw merged 1 commit intomainfrom
fix/frontmatter-yaml-escaping

Conversation

@oritwoen
Copy link
Copy Markdown
Collaborator

@oritwoen oritwoen commented Mar 17, 2026

Description

buildFrontmatter in github-common.ts had its own inline YAML quoting logic that only triggered on 4 characters (: " [ ]). Issue and discussion titles containing #, ', \, newlines, or other YAML metacharacters went through unquoted or with incomplete escaping - backslashes weren't escaped before quotes, so foo\nbar would be misread as a newline by any YAML parser consuming the frontmatter.

Replaced the inline logic with yamlEscape() from core/yaml.ts which already handles all 18+ special characters correctly. One import, one line changed.

Linked Issues

Additional context

8 new unit tests for buildFrontmatter covering colons, hash signs, backslashes, newlines, single quotes, double quotes, simple values, and undefined skipping.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced YAML metadata formatting to properly handle special characters and ensure consistent quoting behavior.
  • Tests

    • Added test coverage for metadata formatting edge cases, including special characters and escape sequences.

buildFrontmatter had its own inline YAML quoting that only checked for
4 special characters (: " [ ]). Titles containing #, ', \, newlines,
or other YAML metacharacters were written unquoted or with incomplete
escaping. Replaced with yamlEscape() which already handles all of these.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73fc8dbe-baba-4563-9f68-d315c14dfd07

📥 Commits

Reviewing files that changed from the base of the PR and between 216d48e and f281d1d.

📒 Files selected for processing (2)
  • src/sources/github-common.ts
  • test/unit/github-common-api.test.ts

📝 Walkthrough

Walkthrough

A refactor that centralizes YAML escaping logic by introducing a yamlEscape import and updating the buildFrontmatter function to consistently apply YAML escaping to string values, accompanied by comprehensive test coverage for edge cases involving special characters and undefined values.

Changes

Cohort / File(s) Summary
YAML Escaping Consolidation
src/sources/github-common.ts
Replaces inline string escaping logic with centralized yamlEscape function import from core/yaml.ts. Ensures consistent YAML escaping and quoting behavior for all string values in frontmatter generation.
Test Coverage for YAML Escaping
test/unit/github-common-api.test.ts
Adds comprehensive test suite for buildFrontmatter covering simple values, strings with special characters (colons, hashes, backslashes, newlines, quotes), and handling of undefined values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • harlan-zw

Poem

🐰 Escaping strings with vim and grace,
YAML fears no more we face,
Colons, quotes, and hashes too,
One function rules them all, it's true! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(sources): use shared yamlEscape in buildFrontmatter' directly and clearly describes the main change: replacing inline YAML-quoting logic with a shared yamlEscape function. It is specific, concise, and accurately reflects the primary modification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/frontmatter-yaml-escaping
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@harlan-zw harlan-zw merged commit a90affa into main Mar 17, 2026
3 checks passed
@oritwoen oritwoen deleted the fix/frontmatter-yaml-escaping branch March 17, 2026 09:59
oritwoen added a commit that referenced this pull request Mar 19, 2026
formatRelease was hand-rolling YAML escaping for the name field
(only escaping double quotes) and emitting tag/version fields
unescaped. Release names with backslashes, newlines, or colons
would produce corrupt frontmatter.

Same class of bug fixed in buildFrontmatter (#34).
oritwoen added a commit that referenced this pull request Mar 19, 2026
formatBlogRelease hand-rolled YAML escaping for the title field
and left version/url completely unescaped. Blog post titles with
backslashes, newlines, or colons would produce corrupt frontmatter.

Third instance of the same bug class (after buildFrontmatter #34
and formatRelease #37).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants