fix(frontmatter,verify,devcontract): preserve spec line endings

[pbean]

Part 1 of #357.

The defect

All four spec frontmatter writers read with Path.read_text. Universal-newline
handling means a CRLF spec arrives in memory as LF — in its entirety, not
just the line being edited. Writing that back relays every line ending in the
file, from functions whose contract is "a minimal in-place line replacement so
the spec's formatting, comments, and field order survive — only the value
changes". Two of the four compounded it with Path.write_text, whose
newline=None default translates \n to os.linesep, so on Windows an all-LF
spec came back all-CRLF.

Writer	Read	Write
frontmatter.set_frontmatter_status	read_text ❌	write_text ❌
verify.set_frontmatter_field	read_text ❌	write_text ❌
devcontract.reset_spec_status	read_text ❌	_atomic_write_spec ✅
devcontract.strip_auto_run_result	read_text ❌	_atomic_write_spec ✅

devcontract's write side was already byte-preserving, and the comment above
_atomic_write_spec had already reasoned this out for append_auto_run_result
— which reads bytes. Its two siblings never got the same read.

The fix

read_bytes().decode("utf-8") on all four; write_bytes(....encode("utf-8"))
on the two frontmatter writers. frontmatter.py's module docstring bars
importing platform_util (it drags in subprocess), so this cannot borrow
atomic_replace — the write stays non-atomic, which is a separate concern from
a byte-preserving one and out of scope here.

_replace_value now carries the edited line's own terminator rather than
re-emitting a flat "\n", which would have left a CRLF spec with exactly one
bare-LF line — the very line the writer was asked to touch. splitlines(keepends=True)
guarantees at most one terminator per line, so \r\n, \n, a bare \r, and a
final line with no terminator each round-trip as authored.

devcontract._render_status_line needed no change: _FM_STATUS_RE's
(?P<rest>.*)$ already captures the \r of a raw CRLF line (verified before
touching it, not assumed).

Side effect: this makes _edit_frontmatter_block's insert branch reachable for
the first time — before the byte-level read, the block was always LF by the time
it got there. It is now covered on both writers that can insert.

Second commit, flagged for the maintainer as a scope call. Making that branch
live exposed that it hardcoded "\r\n" if block.endswith("\r\n") else "\n",
which agrees with the line it follows for CRLF and LF but not for a bare \r —
so an insert into a CR-only spec appended the file's only LF line. That is the
same hardcoded-\n defect as _replace_value's, in the sibling branch of the
same function, and leaving it would have shipped a writer where one branch
carries endings and the other invents them. _last_line_ending reads the
terminator off the block's last line instead. Happy to split this out if you'd
rather part 1 stay strictly to the four read/write swaps.

Behavior delta, pinned rather than fixed

_FRONTMATTER_RE and AUTO_RUN_HEADING_RE are line-oriented on \r?\n. A
CR-only spec used to be normalized by read_text on the way in, so the edit
landed (and the file was rewritten as LF, which was the defect). Reading the
bytes as authored, it finds no frontmatter block / no heading and returns a
silent False.

frontmatter.set_frontmatter_status is splitlines-based and still rewrites
that same shape, so the two siblings now disagree on CR-only input. That
asymmetry is documented at both writers and pinned by
test_reset_status_no_ops_on_a_cr_only_spec / test_strip_auto_run_result_no_ops_on_a_cr_only_spec,
which assert the no-op and that the sibling rewrites the identical bytes.
Widening the patterns to \r would change what counts as a line for every
reader keyed on them (and ^ in MULTILINE would not follow anyway) — a larger
contract than this fix owns, and no BMAD tool authors CR-only files.

Tests

New coverage, all byte-exact:

• tests/test_frontmatter.py — a line endings section: CRLF whole-file
  (only the status line changes, plus "\n" not in text.replace("\r\n", "")),
  bare-CR round-trip, and a mixed-ending file where the status line is the odd
  one out on purpose, so a whole-file nl detection fails it.
• tests/test_resolve.py — set_frontmatter_field CRLF replace, CRLF insert, and
  a CR-only insert that pins "the inserted line is never the file's odd one out".
• tests/test_devcontract.py — reset_spec_status CRLF replace + CRLF insert,
  strip_auto_run_result CRLF preservation, an append→strip CRLF byte-for-byte
  round-trip, and the two CR-only characterizations.

_as_written (the os.linesep-modelling helper) is deleted and its ten call
sites reverted to bare byte-exact comparisons. It existed to keep the byte-exact
gate green on Windows by describing the relay; with the relay gone, those bare
comparisons are what fails on Windows if the write half regresses.

Ablations

Every gate was reverted individually by a scripted patch → run → restore, with
the restore sha256-verified against the original (no git checkout <file>).

Ablation	Result
frontmatter read_bytes → read_text	CRLF + bare-CR + mixed all fail ✅
_replace_value per-line ending → bare "\n"	CRLF + bare-CR fail; plain-LF flip still passes ✅
_replace_value → CRLF/LF only (bare CR dropped)	bare-CR fails; CRLF still passes ✅
_replace_value → hardcoded "\r\n"	mixed fails; CRLF still passes ✅
verify read_bytes → read_text	both new resolve tests fail ✅
devcontract.reset_spec_status read revert	all three of its tests fail ✅
devcontract.strip_auto_run_result read revert	all three of its tests fail ✅
_last_line_ending → the old block.endswith("\r\n") branch	CR-only insert fails; CRLF insert still passes ✅

The "still passes" rows matter as much as the failures: they show each test
discriminates a specific wrong implementation rather than firing on any change.

One honest limit. Reverting write_bytes → write_text keeps the whole
suite green on Linux — os.linesep is \n there, so the write half is
POSIX-invisible. The Windows CI leg is its only oracle, via the reverted
byte-exact assertions in the characterization half. That is recorded in a
comment in tests/test_frontmatter.py so the next reader does not conclude the
write change is untested.

Verification

• uv run pytest -q -n auto — 3671 passed, 24 skipped
• trunk check --no-fix — no issues
• uvx pyright@1.1.411 — 0 errors

Out of scope, staying in #357: the trailing-inline-comment drop (part 2), moving
set_frontmatter_status's tests out of tests/test_resolve.py, and making these
writes atomic.

[coderabbitai]

Warning

Review limit reached

@pbean, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 22506320-5219-4804-b597-7d6c7808f53c

📥 Commits

Reviewing files that changed from the base of the PR and between aec88f5 and 718d647.

📒 Files selected for processing (7)

• CHANGELOG.md
• src/bmad_loop/devcontract.py
• src/bmad_loop/frontmatter.py
• src/bmad_loop/verify.py
• tests/test_devcontract.py
• tests/test_frontmatter.py
• tests/test_resolve.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pbean]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[greptile-apps]

Greptile Summary

This PR fixes a line-ending relay defect in four spec frontmatter writers (set_frontmatter_status, set_frontmatter_field, reset_spec_status, strip_auto_run_result). All four previously used read_text, whose universal-newline translation silently converted every line ending in a CRLF spec to LF on read; two of them then wrote back through write_text, which on Windows additionally converted all LF to CRLF — both directions violated the contract that "only the status value changes."

• All four writers now use read_bytes().decode("utf-8") so the file is decoded without any newline translation; the two frontmatter writers also move to write_bytes on the write side.
• _replace_value now extracts each line's own terminator via rstrip("\r\n") instead of re-emitting a flat "\n", and the new _last_line_ending helper propagates the block's last-line terminator to any inserted key — so neither a replacement nor an insert can introduce a foreign line ending.
• The old _as_written test helper (which modelled the relay rather than failing on it) is removed and replaced with direct byte-exact assertions; a CR-only behaviour asymmetry between the two writers is characterised and pinned rather than fixed.

Confidence Score: 5/5

Safe to merge. The changes are narrowly scoped to the read/write path of four spec writers, and each code path is covered by byte-exact tests that fail on the right regression.

The fix is mechanically straightforward (four read_text to read_bytes().decode swaps and two write_text to write_bytes swaps), and the logic in _replace_value and _last_line_ending is correct for every terminator variant (CRLF, LF, bare-CR, no terminator). The one observable behaviour delta — CR-only specs becoming a no-op through the regex-based writers — is explicitly characterised and pinned in tests rather than silently accepted. The test suite is comprehensive: each new test uses dual assertions (byte-exact equality plus a no bare-LF introduced check) that discriminate specific wrong implementations.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/bmad_loop/frontmatter.py	Core fix: read_bytes().decode + write_bytes replaces read_text/write_text; _replace_value now carries each line's own terminator; new _last_line_ending helper ensures inserts cannot introduce a foreign ending. Logic is correct for LF, CRLF, CR, and mixed-ending specs.
src/bmad_loop/devcontract.py	reset_spec_status and strip_auto_run_result fixed to read_bytes().decode. _render_status_line was already CRLF-safe (captures \r in rest group); only the read side was wrong. _atomic_write_spec write side was already byte-preserving.
src/bmad_loop/verify.py	set_frontmatter_field moved to read_bytes().decode + write_bytes, consistent with the frontmatter.py sibling change.
tests/test_frontmatter.py	_spec helper changed to write_bytes; _as_written deleted; new line-endings section adds CRLF, CR-only, and mixed-ending tests with dual assertions.
tests/test_devcontract.py	New tests cover reset_spec_status CRLF replace + insert, strip_auto_run_result CRLF preservation, append→strip CRLF round-trip, and the two CR-only characterisation pins.
tests/test_resolve.py	New tests cover set_frontmatter_field CRLF replace, CRLF insert, and CR-only insert discriminating the old block.endswith approach from the new _last_line_ending.

_{Reviews (1): Last reviewed commit: "fix(frontmatter): carry the block's endi..." | Re-trigger Greptile}