Route inline BOM strips through formats::text (#905)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #905 (step 3, slice 1). The issue stays open for the inline sites in files that open PRs change.

Summary

This PR moves ten production sites that each spelled out "drop one leading UTF-8 BOM" onto formats::text. It adds a guard test that fails when a new production file spells out its own BOM handling.

Why

• #905 step 3, register row E64 (living document Part 4, "CRLF/BOM/indent policy"). #909 added formats::text::{split_bom, strip_bom}, but about 50 inline strips remained, some with drifted counts (trim_start_matches strips any number of BOMs). #1057 added two more while this PR was open, which the guard caught.
• Leverage: B 0, U 0, D ≈ 10 (inline copies collapsed, plus a guard against new ones), S 0, R L. It is the top-scoring candidate that lies entirely in files no open PR changes.

What changed

• formats::text:
  • New strip_bom_bytes, for the byte decoders.
  • split_bom now returns its BOM half as &'static str. It is always "" or U+FEFF, so callers can store it.
• Re-pointed to the helpers:
  • patch/redirect/requirements.rs: 4 sites.
  • utils/requirements.rs: 2 sites.
  • manifest/operations.rs
  • hosted/npm_manifest.rs
  • gradle/dsl.rs: decode and the lexer's start offset.
  • policy/socket_yml.rs: decode.
  • formats/yarn/blocks.rs and formats/yarn/stanzas.rs (both from Move all yarn.lock parsing into formats/yarn and stop pinning non-registry copies #1057).
  • The CLI's scan/hosted/python.rs: Pipfile.lock for the stale-install remedy.
• production_bom_handling_goes_through_the_helpers guard:
  • It scans production code (test modules and test-support files are exempt).
  • PENDING_INLINE_BOMS lists the 26 files still changed by open PRs.
  • OWN_BOM_RULE lists 2 deliberate exceptions, each with a reason: sbt_version skips a BOM on any properties line, which its test pins, and the vlt sniff refuses a BOM lock because vlt can't read one.
  • The guard is one-sided: a pending file that loses its last inline BOM doesn't fail it.
• Ported from #1118: the ci.yml setup-php pin comment # v2 → # 2.37.2. The Audit GitHub Actions check (zizmor ref-version-mismatch) fails without it, and it no-ops once Label the setup-php pin in ci.yml with its real tag #1118 lands.
• Merged main (b96a785).

What was deleted

• Production: about +25 / −40, net ≈ −15. The removals are inline strip_prefix('\u{feff}')/starts_with copies and the hand-built bom selectors in the hosted requirements rewriter and the yarn stanza splitter.
• Tests: about +147: the guard, the strip_bom_bytes cases, and one-vs-two-BOM coverage for the hosted npm manifest decoder.

Behavior

None for zero or one leading BOM. The one exact change: the hosted Pipenv stale-install remedy read a Pipfile.lock with two leading BOMs as JSON, because it used trim_start_matches. It now treats the second BOM as content, as every other reader does and as #905 requires.

Test evidence

• CI on f810607: all checks green (192 passed, 8 skipped).
  • test-release first failed in api_retry_e2e::a_refused_connection_is_not_retried, a port-reuse race in that test that this PR doesn't touch. It passed on its one re-run. See the PR comments.
• cargo clippy --workspace --all-features -- -D warnings: clean, after the merge.
• cargo test -p socket-patch-core --lib: 5742 passed, 4 failed. The failures are the known root-only tests that also fail on main in this sandbox: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched and pypi_requirements::wire_failure_rolls_back_already_written_files.
• cargo test -p socket-patch-cli --all-features --lib: 875 passed.
• --test in_process_redirect_pipenv, policy_pypi_names, scan_requirements_lock_only, scan_vendor_requirements_unwired and e2e_socket_yml_policy: all pass.
• The guard works in CI. On the merge with main it caught formats/yarn/{blocks,stanzas}.rs, which are now migrated. Locally, removing a pending entry fails it as expected.
• Every former caller keeps its existing BOM test: the requirements lexers, gradle::dsl::bom_is_skipped, the manifest reader's BOM and double-BOM tests, the socket.yml BOM bytes test, and the yarn BOM round-trips. The new reads_past_one_bom_only covers the hosted npm decoder.

Risk

Low. The change is mechanical, apart from the double-BOM Pipfile.lock case above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PQTx5YVAZtPf1S2S5HYdQs

---

Note

Low Risk
Mostly mechanical refactors plus a guard test; the only intentional behavior change is double-BOM Pipfile.lock parsing in the hosted Python stale-install path.

Overview
Centralizes UTF-8 BOM handling in formats::text (#905 step 3): adds strip_bom_bytes, makes split_bom return a 'static BOM prefix for callers that store it, and replaces inline BOM strips across requirements/manifest/Gradle/policy/yarn/npm hosted paths and the CLI Pipenv stale-install scan with strip_bom / split_bom / strip_bom_bytes.

Adds production_bom_handling_goes_through_the_helpers, which fails CI if new production code spells out BOM bytes/literals instead of the helpers (with allowlists for files still migrating in other PRs and two deliberate exceptions).

Behavior: unchanged for zero or one leading BOM. Pipfile.lock parsing no longer uses trim_start_matches, so a second leading BOM is treated as content and JSON parse fails, matching other readers.

Also updates the setup-php pin comment in ci.yml (# v2 → # 2.37.2) for the zizmor ref-version check.

^{Reviewed by Cursor Bugbot for commit f810607. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] The Audit GitHub Actions check failed, and the cause isn't this PR. zizmor's ref-version-mismatch flags .github/workflows/ci.yml:1400: the setup-php pin f3e473d… is commented # v2, but that commit is tagged 2.37.2. This PR's diff doesn't touch that line.

I ported the one-line fix from #1118 as 592de6b. It no-ops once #1118 lands.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit f810607. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] test-release failed on f810607 in api_retry_e2e::a_refused_connection_is_not_retried, and the cause isn't this PR.

The test binds 127.0.0.1:0, drops the listener and expects a refused connection. In this run, another mock server in the same test binary picked up the freed port and answered with a retry-pkg-071 search result, so the call succeeded. That's a port-reuse race in the test itself. This PR doesn't touch api/, the retry code or that test, and every other check that ran the same suite passed.

No fix exists yet. A robust version would keep the listener bound and immediately close each accepted connection, or use a reserved unroutable address. That belongs in a separate PR, so I'm not widening this one. I'll re-run the failed job once when the run completes.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent).

• Head: f810607aa7085e640b05bc8b003489ccb730aca5
• CI: 454/454 check runs green (success/skipped/neutral) on this head, mergeable, no conflicts.
• Bugbot: reviewed this head (Cursor Bugbot check: success), no unresolved review threads.
• Changelog: untouched.

Nothing specific flagged for the reviewer beyond the PR description.

---

Generated by Claude Code