From 695e90257ad8a0e1f3573919903715d80d10d9db Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 10:10:10 +0200 Subject: [PATCH] fix(api): frontmatter parser must not misparse indented list items as keys hyperguild#128, filed while fixing the same bug class in the sibling internal/wiki package's parseFrontmatterBlock (hyperguild#126 review round 3). This package was the original parseFrontmatter/set()/render() shape that reimplementation was modeled on, and had the same gap: an indented YAML block-list item containing a colon (e.g. a title with a subtitle inside an aliases: list) got cut at the colon and treated as a fake top-level key, since strings.Cut doesn't know a line is a continuation just because TrimSpace(key) happens to be non-empty. Re-rendering then stripped the item's indentation, de-indenting it out of the list it belongs to. This package backs UpdateNote/writeHallNote, so it's on the live brain_update write path. Fixed the same way as the wiki package: check indentation before calling Cut at all -- a line starting with a space or tab is a continuation, full stop, regardless of what it contains. Second, related bug found via the fix's own test fixture, not speculatively: render() wrote "key: " (trailing space) for a key with an empty value (the same "aliases:" block-list header line) instead of bare "key:" -- a different byte sequence than the original, breaking this type's whole documented purpose (a byte-preserving line view, not a YAML re-marshal). No current caller sets an empty-value key via set(), so this hadn't shown up in a set()-driven path, but it fires on every ordinary parse-then-render round-trip of a note that already has one -- confirmed directly via this fix's own test output before writing a fix for it. Flip-tested both independently: disabling either fix turns its corresponding test red, the other stays green. Co-Authored-By: Claude Sonnet 5 --- ingestion/internal/api/frontmatter.go | 23 +++++++++++++-- ingestion/internal/api/frontmatter_test.go | 34 ++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/ingestion/internal/api/frontmatter.go b/ingestion/internal/api/frontmatter.go index 6569f15..6177161 100644 --- a/ingestion/internal/api/frontmatter.go +++ b/ingestion/internal/api/frontmatter.go @@ -40,6 +40,15 @@ func parseFrontmatter(src string) (frontmatter, string) { body := rest[end+len("\n---\n"):] for _, line := range strings.Split(block, "\n") { + // An indented line is a continuation (a YAML block-list item, a + // wrapped value) never a top-level key, regardless of whether it + // happens to contain a colon — " - 'Chapter 3: The Reckoning'" is + // not a key named "- 'Chapter 3" (hyperguild#128). Checked before + // Cut so a colon inside one never reaches the key/value path. + if line != "" && (line[0] == ' ' || line[0] == '\t') { + fm.lines = append(fm.lines, fmLine{raw: line}) + continue + } key, val, ok := strings.Cut(line, ":") key = strings.TrimSpace(key) if !ok || key == "" || strings.HasPrefix(strings.TrimSpace(line), "#") { @@ -83,9 +92,19 @@ func (f *frontmatter) render() string { var b strings.Builder b.WriteString("---\n") for _, l := range f.lines { - if l.key == "" { + switch { + case l.key == "": b.WriteString(l.raw) - } else { + case l.value == "": + // A key with nothing after the colon (the YAML block-list + // shape: "aliases:" followed by indented "- item" lines on + // their own raw lines) must stay bare — "key: " with a + // trailing space is a different byte sequence than the + // original, and this type's whole purpose (see the package + // doc) is a byte-preserving round-trip. + b.WriteString(l.key) + b.WriteByte(':') + default: b.WriteString(l.key) b.WriteString(": ") b.WriteString(l.value) diff --git a/ingestion/internal/api/frontmatter_test.go b/ingestion/internal/api/frontmatter_test.go index 6bcb2e3..b125f25 100644 --- a/ingestion/internal/api/frontmatter_test.go +++ b/ingestion/internal/api/frontmatter_test.go @@ -59,3 +59,37 @@ func TestFrontmatterRenderRoundTrips(t *testing.T) { fm, _ := parseFrontmatter(src) assert.Equal(t, src, fm.render()) } + +func TestParseFrontmatterIndentedListItemWithColonIsNotMisparsedAsKey(t *testing.T) { + // hyperguild#128: an indented YAML block-list item that happens to + // contain a colon (e.g. a title with a subtitle) is a continuation + // line, never a top-level key — strings.Cut doesn't know that just + // because TrimSpace(key) happens to be non-empty. Misparsing it as a + // key/value pair and re-rendering strips its indentation, de-indenting + // it out of the list it belongs to. Same bug class already found and + // fixed in the sibling internal/wiki package's parseFrontmatterBlock + // (hyperguild#126 review round 3) — this package was the original the + // reimplementation was modeled on and had the gap too. + src := "---\naliases:\n - 'Chapter 3: The Reckoning'\ntitle: 'X'\n---\nbody\n" + fm, _ := parseFrontmatter(src) + fm.set("updated_at", "2026-09-23T00:00:00Z") + + out := fm.render() + assert.Contains(t, out, " - 'Chapter 3: The Reckoning'", + "the indented list item must survive verbatim, not get re-emitted as a de-indented fake key") + assert.NotContains(t, out, "\n- 'Chapter 3", + "must not be split into a fake key/value pair at column 0 (as opposed to surviving indented, asserted above)") +} + +func TestFrontmatterRenderEmptyValueKeyStaysBareNoTrailingSpace(t *testing.T) { + // Companion to the indented-list-item fix: "aliases:" (a key with + // nothing after the colon, the YAML block-list header) must round-trip + // bare, not as "aliases: " with a trailing space — a different byte + // sequence than the original, breaking this type's whole reason for + // being a line-preserving view rather than a YAML re-marshal. + // render() returns only the frontmatter block (see + // TestFrontmatterRenderRoundTrips), no trailing body. + src := "---\naliases:\n - one\n - two\n---\n" + fm, _ := parseFrontmatter(src) + assert.Equal(t, src, fm.render()) +} -- 2.54.0