From 625955e9442542e40f9e68e04a869e234f3e1d33 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 15:04:59 +0200 Subject: [PATCH] fix(frontmatter): accept a closing fence at literal end-of-file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review found a real regression: tightening Split's closing-fence match to require a trailing newline (closing the "\n---extra" bypass) also silently narrowed internal/frontmatter's own Lint/Fix behavior — a document whose closing fence is literally the last bytes of the file ("---\ntitle: x\n---", no body at all) used to parse fine via this package's private split, and now returns ok=false, contradicting this PR's own "zero behavior change for Lint/Fix" claim. internal/api and internal/mcp were unaffected (both already required the trailing newline pre-consolidation), but cmd/frontmatterlint's Lint/Fix relied on the old, looser acceptance. Split now distinguishes "fence followed by a newline" (real body follows) and "fence is the last bytes of the document" (no body, not a bypass attempt) — both valid — from "fence followed by anything else" (the actual bypass shape, still rejected). Regression test pins the literal-EOF case; flip-tested to confirm it's load-bearing. Co-Authored-By: Claude Sonnet 5 --- ingestion/internal/frontmatter/frontmatter.go | 17 +++++++++++++++-- .../internal/frontmatter/frontmatter_test.go | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/ingestion/internal/frontmatter/frontmatter.go b/ingestion/internal/frontmatter/frontmatter.go index 254d9db..8548e33 100644 --- a/ingestion/internal/frontmatter/frontmatter.go +++ b/ingestion/internal/frontmatter/frontmatter.go @@ -146,9 +146,22 @@ func Split(doc []byte) (block, rest []byte, ok bool) { } body := s[len(fence)+1:] - idx := strings.Index(body, "\n"+fence+"\n") + idx := strings.Index(body, "\n"+fence) if idx < 0 { return nil, nil, false } - return []byte(body[:idx+1]), []byte(body[idx+1+len(fence):]), true + // What follows the closing fence must be either a newline (a real body + // follows) or nothing at all (the fence is the literal end of the + // document — no body, not a bypass attempt). Anything else, e.g. + // "\n---extra", is not a valid close: an earlier version of this check + // accepted it, which is exactly the looser match hyperguild#138 closes. + after := idx + 1 + len(fence) + switch { + case after == len(body): + return []byte(body[:idx+1]), nil, true + case body[after] == '\n': + return []byte(body[:idx+1]), []byte(body[after:]), true + default: + return nil, nil, false + } } diff --git a/ingestion/internal/frontmatter/frontmatter_test.go b/ingestion/internal/frontmatter/frontmatter_test.go index 4945d57..bda0ff0 100644 --- a/ingestion/internal/frontmatter/frontmatter_test.go +++ b/ingestion/internal/frontmatter/frontmatter_test.go @@ -223,3 +223,18 @@ func TestSplit_RequiresANewlineAfterTheClosingFence(t *testing.T) { _, _, ok := Split([]byte(doc)) assert.False(t, ok, "a fence line not followed by a newline is not a valid closing fence") } + +// TestSplit_AcceptsAClosingFenceThatIsTheLastBytesOfTheDocument is a +// regression pin found in review of hyperguild#138: the fix for the +// "\n---extra" bypass above must not also reject the legitimate case of a +// closing fence with nothing after it at all (no trailing newline because +// there's no body, not because a bypass is being attempted) -- this +// package's own private split() accepted that shape before the +// consolidation, and cmd/frontmatterlint's Lint/Fix relied on it. +func TestSplit_AcceptsAClosingFenceThatIsTheLastBytesOfTheDocument(t *testing.T) { + doc := "---\ntitle: 'n'\n---" + block, rest, ok := Split([]byte(doc)) + require.True(t, ok, "a closing fence at literal end-of-file, with nothing after it, is valid -- there is no body to require a newline before") + assert.Contains(t, string(block), "title: 'n'") + assert.Equal(t, "", string(rest)) +}