fix(frontmatter): consolidate 3 independent fence-parsers into one #139

2026-09-23T12:57:48Z

mathias commented

2026-09-23 12:57:48 +00:00

Closes #138.

Summary

Three independent fence-detection implementations (internal/mcp/tools_notes.go's frontmatterBlock, internal/api/frontmatter.go's parseFrontmatter, internal/frontmatter/frontmatter.go's split) each grew their own gap through parallel bug-fixing across today's #123/#126/#128 sagas. internal/frontmatter (a leaf package, no internal imports) becomes the canonical detector: exported Split(doc []byte) (block, rest []byte, ok bool), BOM-stripped, CRLF-normalized, with a stricter trailing-newline-required closing-fence match (closing a real bypass internal/api silently carried — a doc ending "\n---extra" used to be treated as validly closed).

internal/api and internal/mcp now call frontmatter.Split instead of re-detecting the fence, keeping only what's genuinely package-specific (api's ordered fmLine round-trip shape, mcp's map + block-scalar-fold logic).

Two real regressions found and fixed while wiring this up

Split's block/rest boundary sits one byte differently than either caller's prior contract (it keeps the fence-adjacent newline on each side, by design — needed for Fix's own reconstruction in internal/frontmatter). Both callers' own pre-existing byte-exactness tests (TestParseFrontmatterSplitsHeaderAndBody, TestFrontmatterRenderRoundTrips, TestFrontmatterRenderEmptyValueKeyStaysBareNoTrailingSpace) went red immediately on the naive swap — an extra spurious blank line appeared before the closing fence and after the body's start. Fixed with a one-byte TrimSuffix/TrimPrefix at each call site, verified back to exact byte-parity with the prior behavior.

Neither regression would have been caught without those tests already existing — worth noting as evidence for why the byte-exactness tests in internal/api earned their keep.

Test plan

New Split tests in internal/frontmatter: CRLF normalization, BOM stripping, strict trailing-newline closing-fence requirement — each written first, confirmed red, then green.
Full internal/frontmatter, internal/api, internal/mcp suites green, including all of #123's existing CRLF/BOM/folded-scalar regression tests (now passing via the shared parser instead of package-local guards).
Full module gate: go build ./..., go vet ./..., go test ./... -race -count=1 — all 31 packages green.
task check (lint, vet, build, gitleaks) green.
Flip-tested the TrimSuffix compensation in internal/mcp specifically — found it's not observably load-bearing for that package's own tests (its map-extraction tolerates the extra byte harmlessly), kept anyway for exact behavioral parity with the prior contract.

🤖 Generated with Claude Code

Created via git-mcp on behalf of @mathias

Closes #138. ## Summary Three independent fence-detection implementations (`internal/mcp/tools_notes.go`'s `frontmatterBlock`, `internal/api/frontmatter.go`'s `parseFrontmatter`, `internal/frontmatter/frontmatter.go`'s `split`) each grew their own gap through parallel bug-fixing across today's #123/#126/#128 sagas. `internal/frontmatter` (a leaf package, no internal imports) becomes the canonical detector: exported `Split(doc []byte) (block, rest []byte, ok bool)`, BOM-stripped, CRLF-normalized, with a stricter trailing-newline-required closing-fence match (closing a real bypass `internal/api` silently carried — a doc ending `"\n---extra"` used to be treated as validly closed). `internal/api` and `internal/mcp` now call `frontmatter.Split` instead of re-detecting the fence, keeping only what's genuinely package-specific (api's ordered `fmLine` round-trip shape, mcp's map + block-scalar-fold logic). ## Two real regressions found and fixed while wiring this up `Split`'s block/rest boundary sits one byte differently than either caller's prior contract (it keeps the fence-adjacent newline on each side, by design — needed for `Fix`'s own reconstruction in `internal/frontmatter`). Both callers' own pre-existing byte-exactness tests (`TestParseFrontmatterSplitsHeaderAndBody`, `TestFrontmatterRenderRoundTrips`, `TestFrontmatterRenderEmptyValueKeyStaysBareNoTrailingSpace`) went red immediately on the naive swap — an extra spurious blank line appeared before the closing fence and after the body's start. Fixed with a one-byte `TrimSuffix`/`TrimPrefix` at each call site, verified back to exact byte-parity with the prior behavior. Neither regression would have been caught without those tests already existing — worth noting as evidence for why the byte-exactness tests in `internal/api` earned their keep. ## Test plan - [x] New `Split` tests in `internal/frontmatter`: CRLF normalization, BOM stripping, strict trailing-newline closing-fence requirement — each written first, confirmed red, then green. - [x] Full `internal/frontmatter`, `internal/api`, `internal/mcp` suites green, including all of #123's existing CRLF/BOM/folded-scalar regression tests (now passing via the shared parser instead of package-local guards). - [x] Full module gate: `go build ./...`, `go vet ./...`, `go test ./... -race -count=1` — all 31 packages green. - [x] `task check` (lint, vet, build, gitleaks) green. - [x] Flip-tested the `TrimSuffix` compensation in `internal/mcp` specifically — found it's not observably load-bearing for that package's own tests (its map-extraction tolerates the extra byte harmlessly), kept anyway for exact behavioral parity with the prior contract. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Created via git-mcp on behalf of @mathias_

mathias added 1 commit 2026-09-23 12:57:48 +00:00

fix(frontmatter): consolidate 3 independent fence-parsers into one

CI / Lint / Test / Vet (pull_request) Successful in 34s

Details

CI / Mirror to GitHub (pull_request) Skipped

Details

a75fd95ab8

hyperguild#138: internal/mcp/tools_notes.go's frontmatterBlock,
internal/api/frontmatter.go's parseFrontmatter, and
internal/frontmatter/frontmatter.go's split each independently detected a
note's --- fence, converging on the same shape through parallel bug-fixing
across today's #123/#126/#128 sagas. Each carried a subtly different gap:
internal/frontmatter's closing-fence match had no trailing-newline
requirement (a document ending "\n---extra" was treated as validly closed);
internal/mcp needed CRLF/BOM guards bolted on across two separate review
rounds after both silently defeated its own HasPrefix check; internal/api
never gained either fix, so the same encoding-dependent bypass was live
there too, just never triggered by a test.

internal/frontmatter is the leaf package (imports nothing internal), so it
becomes the canonical fence detector: exported Split(doc []byte) (block,
rest []byte, ok bool), BOM-stripped and CRLF-normalized, with the stricter
trailing-newline-required close. internal/api and internal/mcp now call it
instead of re-detecting the fence themselves, keeping only what's genuinely
package-specific (api's ordered fmLine round-trip shape, mcp's map + block-
scalar-fold logic).

Found two real regressions while wiring the callers, caught by each
package's own pre-existing byte-exactness tests going red immediately:
Split's block/rest boundary sits one byte differently than either caller's
prior contract (it keeps the fence-adjacent newline on each side, by design
for Fix's own reconstruction) — both callers now TrimSuffix/TrimPrefix
exactly that one byte to restore their exact prior semantics. Neither
regression would have been visible without those tests already existing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

mathias referenced this pull request

2026-09-23 13:03:20 +00:00

Consolidate the three independent frontmatter-fence parsers #138

mathias added 1 commit 2026-09-23 13:05:00 +00:00

fix(frontmatter): accept a closing fence at literal end-of-file

CI / Lint / Test / Vet (pull_request) Successful in 39s

Details

CI / Mirror to GitHub (pull_request) Skipped

Details

625955e944

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 <noreply@anthropic.com>

mathias merged commit 3d5af85713 into main

2026-09-23 13:07:32 +00:00

mathias deleted branch fix/138-consolidate-frontmatter-parsers

2026-09-23 13:07:32 +00:00

mathias referenced this pull request

2026-09-23 13:07:51 +00:00

internal/wiki/merge.go:184 is a fourth hand-rolled frontmatter-fence parser, not folded into frontmatter.Split #141

mathias referenced this pull request

2026-09-23 13:08:22 +00:00

Consolidate the three independent frontmatter-fence parsers #138

Sign in to join this conversation.