fix(#1779): emit valid YAML for unsafe scalars in reconstructFrontmatter (#1807)

* fix(#1779): emit valid YAML for unsafe scalars in reconstructFrontmatter

reconstructFrontmatter wraps a scalar or block-array item in double quotes
when it contains a YAML indicator (`:`/`#`, plus `[`/`{` for top-level
scalars), but interpolated the raw value with no escaping. A value carrying
both an indicator and a literal `"` (or `\`) serialized to invalid YAML, e.g.
`upstream: "https://x (Tom; "Git. Ship. Done")"`, which fails under any strict
parser (js-yaml, PyYAML) and corrupts the whole block on the next
syncStateFrontmatter whole-block regen.

- escapeDoubleQuoted() escapes `\`, `"`, and control chars (newline/tab/CR/C0
  controls + DEL → YAML `\n`/`\t`/`\r`/`\xHH`) so any wrapped value is valid.
- scalarNeedsDoubleQuoting() routes values that mis-parse or round-trip lossily
  when bare through that escaped form: the empty string (bare `k:` reloads as
  null), an embedded `"`/`\` or control char, a leading YAML indicator
  (quote / `&`*`!` anchor-alias-tag / `|`>` block scalar / flow `[]{},` / `#` /
  reserved `%`@`backtick / `-`?`:` before a space), or leading/trailing space.
  Applied at all four wrap sites.

Scope stays on serialization correctness; it does NOT broaden the lossy
object-list handling deferred to #1572/#1660. Known limitation: lone UTF-16
surrogates are still lossy through UTF-8 encoding (out of scope, extremely
unlikely in frontmatter values).

Regression test asserts strict js-yaml round-trip across all four wrap sites
plus backslash, control-char (incl. NUL), empty-string, leading-indicator, and
leading/trailing-whitespace classes; test-the-test confirms each fails on the
unescaped output. Frontmatter suite 549/549 green, golden-install-parity 16/16
unchanged (no serialization churn).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(#1779): add changeset for frontmatter quote-escaping fix

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

.changeset/humble-tunas-munch.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 1807
+---
+reconstructFrontmatter now emits valid YAML for scalars and block-array items that were previously serialized unescaped. Values carrying a YAML indicator plus a literal quote/backslash, embedded control characters, the empty string, a leading YAML indicator, or leading/trailing whitespace are now routed through a properly escaped double-quoted form, so frontmatter round-trips through strict parsers (js-yaml, PyYAML) instead of corrupting the block on the next state sync.

src/frontmatter.cts

@@ -133,6 +133,47 @@ function extractFrontmatter(content: string): Frontmatter {
   return frontmatter;
 }
 
+/**
+ * Escape a string for emission inside a YAML double-quoted scalar (#1779).
+ * Backslash must be escaped first so the backslashes added for embedded quotes
+ * (and control chars) are not themselves doubled. Without this, a value
+ * carrying an indicator (`:`/`#`) that also contains a literal `"` serializes
+ * to invalid YAML, e.g. `upstream: "https://x (Tom; "Git. Ship. Done")"`. A
+ * literal newline/tab/control char inside the quotes likewise breaks (or
+ * silently alters) the scalar, so those are escaped to their YAML forms too.
+ */
+function escapeDoubleQuoted(s: string): string {
+  return s
+    .replace(/\\/g, '\\\\')
+    .replace(/"/g, '\\"')
+    .replace(/\n/g, '\\n')
+    .replace(/\t/g, '\\t')
+    .replace(/\r/g, '\\r')
+    // Remaining C0 controls + DEL → \xHH (a valid YAML double-quoted escape).
+    .replace(/[\u0000-\u001f\u007f]/g, (c) => `\\x${c.charCodeAt(0).toString(16).padStart(2, '0')}`);
+}
+
+/**
+ * A plain (unquoted) scalar that would mis-parse or round-trip lossily when
+ * emitted bare must instead go through the double-quoted + escaped form
+ * (#1779): the empty string (bare `k:` reloads as null), an embedded `"`/`\`
+ * or control char, a leading YAML indicator (quote, `&`/`*`/`!` anchor/alias/
+ * tag, `|`/`>` block scalar, flow `[]{},`, `#`, reserved `%`/`@`/backtick, or
+ * `-`/`?`/`:` before a space), or leading/trailing whitespace. This helper is
+ * the correctness complement of `escapeDoubleQuoted`: it broadens the *trigger*
+ * for quoting without broadening the lossy object-list handling deferred to
+ * #1572/#1660.
+ */
+function scalarNeedsDoubleQuoting(s: string): boolean {
+  if (s === '') return true;
+  if (/["\\\u0000-\u001f\u007f]/.test(s)) return true;
+  // Always-unsafe leading indicators, or leading/trailing whitespace.
+  if (/^[,[\]{}#&*!|>'"%@`]/.test(s) || /^\s|\s$/.test(s)) return true;
+  // `-` `?` `:` only start a plain scalar safely when NOT followed by a space.
+  if (/^[-?:](\s|$)/.test(s)) return true;
+  return false;
+}
+
 function reconstructFrontmatter(obj: Frontmatter): string {
   const lines: string[] = [];
   for (const [key, value] of Object.entries(obj)) {
@@ -145,7 +186,7 @@ function reconstructFrontmatter(obj: Frontmatter): string {
       } else {
         lines.push(`${key}:`);
         for (const item of value) {
-          lines.push(`  - ${typeof item === 'string' && (item.includes(':') || item.includes('#')) ? `"${item}"` : item}`);
+          lines.push(`  - ${typeof item === 'string' && (item.includes(':') || item.includes('#') || scalarNeedsDoubleQuoting(item)) ? `"${escapeDoubleQuoted(item)}"` : item}`);
         }
       }
     } else if (typeof value === 'object') {
@@ -160,7 +201,7 @@ function reconstructFrontmatter(obj: Frontmatter): string {
           } else {
             lines.push(`  ${subkey}:`);
             for (const item of subval) {
-              lines.push(`    - ${typeof item === 'string' && (item.includes(':') || item.includes('#')) ? `"${item}"` : item}`);
+              lines.push(`    - ${typeof item === 'string' && (item.includes(':') || item.includes('#') || scalarNeedsDoubleQuoting(item)) ? `"${escapeDoubleQuoted(item)}"` : item}`);
             }
           }
         } else if (typeof subval === 'object') {
@@ -184,13 +225,13 @@ function reconstructFrontmatter(obj: Frontmatter): string {
         } else {
           // eslint-disable-next-line @typescript-eslint/no-base-to-string
           const sv = String(subval);
-          lines.push(`  ${subkey}: ${sv.includes(':') || sv.includes('#') ? `"${sv}"` : sv}`);
+          lines.push(`  ${subkey}: ${sv.includes(':') || sv.includes('#') || scalarNeedsDoubleQuoting(sv) ? `"${escapeDoubleQuoted(sv)}"` : sv}`);
         }
       }
     } else {
       const sv = String(value);
-      if (sv.includes(':') || sv.includes('#') || sv.startsWith('[') || sv.startsWith('{')) {
-        lines.push(`${key}: "${sv}"`);
+      if (sv.includes(':') || sv.includes('#') || sv.startsWith('[') || sv.startsWith('{') || scalarNeedsDoubleQuoting(sv)) {
+        lines.push(`${key}: "${escapeDoubleQuoted(sv)}"`);
       } else {
         lines.push(`${key}: ${sv}`);
       }

tests/frontmatter.property.test.cjs

@@ -21,6 +21,7 @@
 const { describe, test } = require('node:test');
 const assert = require('node:assert/strict');
 const fc = require('./helpers/fast-check-setup.cjs');
+const yaml = require('js-yaml');
 
 const {
   extractFrontmatter,
@@ -263,3 +264,32 @@ describe('frontmatter: prohibitions parse ↔ splice bijection (#644)', () => {
     );
   });
 });
+
+// #1779 — reconstructFrontmatter must emit YAML that a STRICT parser accepts and
+// that preserves string values. The bijective contract is
+//   ∀ s: yaml.load(reconstructFrontmatter({ k: s })).k === s
+// over the documented safe-input subset. Two classes are out of scope and
+// excluded here, not silently passed:
+//   - lone UTF-16 surrogates (lossy through UTF-8 encoding) — filtered via
+//     fc.pre(s.isWellFormed());
+//   - numeric/boolean/null-looking BARE strings (e.g. "42", "true", "-5") that a
+//     YAML loader resolves to a non-string type — a separate pre-existing bug
+//     class (valid YAML, wrong type), so we assert equality only when the value
+//     loads back AS a string. An escaping defect (invalid YAML) still fails
+//     loudly because yaml.load() throws.
+describe('frontmatter: reconstructFrontmatter strict-YAML property (#1779)', () => {
+  test('property: every string value serializes to valid YAML and string-round-trips', () => {
+    fc.assert(
+      fc.property(fc.string({ maxLength: 200 }), (s) => {
+        fc.pre(s.isWellFormed());
+        // Throws → reconstructFrontmatter emitted invalid YAML → property fails
+        // (fast-check shrinks + prints the replay seed automatically).
+        const loaded = yaml.load(reconstructFrontmatter({ k: s }));
+        if (typeof loaded.k === 'string') {
+          assert.equal(loaded.k, s,
+            `value did not round-trip through strict YAML: ${JSON.stringify(s)}`);
+        }
+      })
+    );
+  });
+});

tests/frontmatter.unit.test.cjs

@@ -15,6 +15,7 @@
 
 const { describe, test } = require('node:test');
 const assert = require('node:assert/strict');
+const yaml = require('js-yaml');
 
 const {
   extractFrontmatter,
@@ -1125,6 +1126,81 @@ describe('reconstructFrontmatter: round-trip', () => {
   });
 });
 
+// #1779 — reconstructFrontmatter must emit valid YAML for scalars and block-
+// array items that carry an embedded `"`/`\`, a control char, an empty string,
+// a leading YAML indicator, or surrounding whitespace. The project's own lossy
+// `extractFrontmatter` tolerates the broken output, which is exactly why these
+// assert round-trip through a STRICT parser (js-yaml) instead — the strict path
+// fails on the unescaped/bare emission and passes only once every wrap site
+// escapes and every unsafe-bare value is routed through the quoted form.
+describe('reconstructFrontmatter: strict-YAML round-trip (#1779)', () => {
+  // Serialize via the production path, then load with a strict YAML parser.
+  const strictRoundTrip = (obj) => yaml.load(reconstructFrontmatter(obj));
+
+  test('top-level scalar with indicator + embedded quotes (the reported case)', () => {
+    const obj = { upstream: 'https://x (Tom; "Git. Ship. Done")' };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('nested scalar with indicator + embedded quotes', () => {
+    const obj = { meta: { note: 'see: "the docs"' } };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('top-level block-array item with indicator + embedded quotes', () => {
+    // 4 items forces block form (the inline `[a, b]` branch caps at 3).
+    const obj = { tags: ['a: "1"', 'b: "2"', 'c: "3"', 'd: "4"'] };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('nested block-array item with indicator + embedded quotes', () => {
+    const obj = { meta: { tags: ['a: "1"', 'b: "2"', 'c: "3"', 'd: "4"'] } };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('embedded backslash is escaped (not just quotes)', () => {
+    const obj = { path: 'a:\\b\\c "x"' };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('control chars in a wrapped value are escaped (newline, tab, NUL)', () => {
+    const obj = { note: 'line1: a\nline2\twith\x00nul' };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('CRLF round-trips (locks the \\r escape for Windows-authored values)', () => {
+    const obj = { note: 'line1: a\r\nline2' };
+    assert.deepEqual(strictRoundTrip(obj), obj);
+  });
+
+  test('empty string round-trips as "" (bare `k:` would reload as null)', () => {
+    assert.deepEqual(strictRoundTrip({ k: '' }), { k: '' });
+  });
+
+  test('embedded/leading quote with NO indicator still round-trips', () => {
+    assert.deepEqual(strictRoundTrip({ k: '"x' }), { k: '"x' });
+    assert.deepEqual(strictRoundTrip({ k: "'leading single" }), { k: "'leading single" });
+    assert.deepEqual(strictRoundTrip({ k: 'say "hi" there' }), { k: 'say "hi" there' });
+  });
+
+  test('leading YAML indicators with no `:`/`#` still round-trip', () => {
+    for (const v of ['>x', '|x', '&anchor', '*alias', '!tag', '[flow', '{map', '%pct', '@reserved', '`tick']) {
+      assert.deepEqual(strictRoundTrip({ k: v }), { k: v }, `value ${JSON.stringify(v)}`);
+    }
+  });
+
+  test('leading/trailing whitespace is preserved (bare would be trimmed)', () => {
+    assert.deepEqual(strictRoundTrip({ k: '  leading' }), { k: '  leading' });
+    assert.deepEqual(strictRoundTrip({ k: 'trailing  ' }), { k: 'trailing  ' });
+  });
+
+  test('plain values without indicators are unaffected (no spurious quoting)', () => {
+    const obj = { name: 'simple-value', label: 'plain' };
+    assert.equal(reconstructFrontmatter(obj), 'name: simple-value\nlabel: plain');
+    assert.deepEqual(yaml.load(reconstructFrontmatter(obj)), obj);
+  });
+});
+
 describe('extractFrontmatter: boundary — dash at start of file', () => {
   test('--- at byte 0 is treated as frontmatter', () => {
     const result = extractFrontmatter('---\nkey: val\n---\n');