[Mikola Lysenko (mikolalysenko)]

Summary

Composer lock with a UTF-8 BOM. Every composer.lock reader that parsed strictly now reads past exactly one leading BOM, using the shared helpers lockfile discovery already uses (vendor::common::parse_json_manifest / parse_json_text, formats::text::strip_bom):

• the lock inventory (vendor/lock_inventory/composer.rs)
• the vendored backend's plan/prelude, --revert restore, and stranded scan (vendor/composer_lock.rs)
• the CLI's vendored reinstall hints (commands/composer_hints.rs)

The vendored splice (lock_bytes_with_entry) and its whole-document fallback put the BOM back on write, so vendor followed by --revert restores the lock byte for byte. composer.json readers are BOM-tolerant too: discovery's preferred-install, the upstream restore's repository gates, and the crawler's installer-paths / vendor-dir.

Revert fails closed on an unreadable composer.lock. The stranded scan used to read a lock it could not parse as "nothing wired". That covered a BOM lock, but also a merge conflict, a trailing comma, a second BOM, or a FIFO. Take a vendor --revert of an entry with no pre-vendor fragment, such as one rebuilt by repair. That entry drift-skipped. Then revert::finish's drift-keep ran a raw-text contains(".socket/vendor/composer/<uuid>"), which misses a \/-escaped path. So the revert deleted the artifact that composer.lock still installs from.

Now a one-BOM lock parses. Any other lock that exists but cannot be read or parsed refuses the revert, in both dry and wet runs, with the reason and a fix-it hint. Tests: revert_refuses_to_strand_a_bom_lock_entry, revert_refuses_over_an_unparseable_lock (merge markers).

Other GC paths are not affected. The scan --prune GC and hosted-record liveness use discovery, which already strips the BOM; the inventory only adds negative evidence there, so a dropped lock could only make them keep more. The orphan sweep matches raw text and handles both slash spellings.

Pristine fetch / repair (verified, no fix needed). No vendored or repair path downloads what a lock entry pins:

• Vendored backends and repair take their bytes from the patch service by patch uuid (service_fetch::fetch_verified_archive).
• recover_lock_entry's only production caller is pristine_fetch_is_verifiable, a yes/no refusal gate. Hosted-pinned purls are filtered out before that gate.
• The hosted → vendored takeover restores the upstream entry from registry metadata before the backend records original.

lock_pinned_locations_never_drive_a_download (core) and its CLI twin pin every production reference to registry_fetch::download, maven_repo::fetch_registry_bytes and recover_lock_entry. I also corrected the stale docs that said vendor fetches the lock's resolution.

Review findings (/code-review high)

1. Fail-closed stranded scan: confirmed and fixed. stranded_wired_packages now returns Result; an unreadable or unparseable lock refuses the revert. revert_refuses_over_an_unparseable_lock failed before the fix, and passes after it.
2. Guard truncated at the first #[cfg(test)]: confirmed and fixed. The new vendor::test_support::production_sources normalizes CRLF and blanks each #[cfg(test)] / cfg(all(test, ..)) item in place: inline modules, mod x;, helpers, uses, and #[cfg(test)] files. It never truncates, and line numbers are kept. It is lexical, so it skips comments and string / raw-string / char literals while matching brackets.
3. Guard evadable through use, aliases or growth in allowlisted files: confirmed and fixed. References are now matched by identifier name in comment- and literal-blind code: calls by any path, method calls, use and as aliases (an alias must name the original once), re-exports, and the function passed as a value. Each one is pinned by file and line text.
   • Still not caught: a new caller of a wrapper an already-pinned line hands out (for example a new caller of upstream::client's own methods); macro-generated calls; and an edit that keeps a pinned line's text while changing what feeds it.
   • Name collisions: the self-update module's download names are pinned explicitly.
4. Core walked ../socket-patch-cli with unwrap(): confirmed and fixed. The core guard scans only its own src/. A twin test in the CLI crate (commands::vendor::lock_download_guard_tests) pins the CLI's recover_lock_entry reference. The two download functions are pub(crate) in core, so the compiler already keeps the CLI from calling them. A by-name scan there would only hit the CLI's own download locals.
5. Strict composer.json parsing: confirmed and fixed in the 5 production readers named in the Summary. User patches already strip the BOM. Discovery's veto test a_bom_composer_json_still_decides_the_preference failed before the fix, and passes after it.
6. Write copied the whole lock / BOM locks lost the parse memo: confirmed and fixed. Without a BOM the bytes are returned as is. reseed_memo compares the render against the bytes past the BOM, so a BOM lock keeps the memo (the memo test is extended).
7. Guard read every file once per needle: fixed. Each file is read and split once, then all needles are checked.
8. Shared helper: done. The BOM-helper guard in formats/text.rs and both download guards use production_sources().
9. Module doc split the bullet list: fixed. The paragraph now follows the list.

Tests

• Failing before the change, passing after: bom_lock_vendors_keeps_its_bom_and_reverts_byte_identical, whole_document_rewrite_keeps_the_bom, stranded_scan_reads_a_bom_lock, revert_refuses_to_strand_a_bom_lock_entry, revert_refuses_over_an_unparseable_lock, composer_lock_with_a_bom_inventories_its_entries, a_bom_composer_json_still_decides_the_preference.
• BOM cases added to the composer.json gate / vendor-dir unit tests and to memo_is_reseeded_only_with_the_docs_own_render. Unit tests for production_sources. Guard pins in core and CLI.
• cargo test -p socket-patch-core --lib -- vendor:: vex::discover:: formats::composer formats::gem patch::redirect:: formats::text crawlers::composer_crawler: 4310 passed. The golden corpus test passes with no golden changes.
• CLI: lib composer_hints / commands::vendor:: / repair (80 passed); mode_migration_composer and e2e_vendor_composer_crlf (7 + 7 passed).
• clippy (--workspace --all-features -D warnings) and fmt are clean.

Not done

• any_live_file_references (the drift-keep probe) still misses \/-escaped paths; draft Keep vendored artifacts a lock references through escaped paths #2022 fixes that probe. Here the stranded refusal (now fail-closed) covers the composer case.

Review findings, round 2 (/code-review high on 75b6ca3, cloud routine)

Fixed in the commit after 75b6ca3. Each behavior test failed on 75b6ca3.

1. The source scan overran a test-only list item: confirmed and fixed. item_end ignored a depth-0 , and the enclosing }, so a #[cfg(test)] field, variant, arm or parameter blanked everything up to the next block, including a whole following impl. That hid production download references from the guard. An item now ends at its , or at the bracket that encloses it. Generic <…> count as brackets, so Result<(), ()> and HashMap<K, V> don't end it early. Test: list_items_end_at_their_comma_or_closer.
2. code_only failed open on a UTF-8 slip: fixed. It now uses expect, like strip_test_items.
3. The module docs overclaimed: fixed. They now say the file an out-of-line test module points at is still read unless it's named *tests.rs.
4. The composer revert refused over any unparseable lock: confirmed and fixed. It still refuses when the broken lock names the entry's uuid, which keeps the fail-closed fix from round 1. A broken lock that never mentions the uuid can't route composer at the uuid dir, so it no longer blocks the revert. Tests: revert_refuses_over_an_unparseable_lock and test_stranded_scan_degrades_on_unreadable_or_odd_shapes, both extended.
5. The BOM memo assertion was vacuous: fixed. The test now re-seeds with two BOMs before asserting nothing was stored.
6. The CLI guard didn't sort its expected list: fixed.

Declined:

• The bare download name matches updater lines. Renaming registry_fetch::download is a wider refactor. The pinned list makes any change visible.
• Other guards still truncate at the first #[cfg(test)]. They're outside this PR. Moving them to production_sources is a good follow-up.
• The core and CLI collection loops are duplicated, and the scanner lives in vendor::test_support. Both are layering cleanups and stay out of scope here.
• The four-arm match in lock_bytes_with_entry. Style only.

Tests (cloud run): cargo test -p socket-patch-core --features test-fixtures --lib -- vendor::composer formats::text production_sources lock_inventory:: vex::discover:: and -p socket-patch-cli --lib vendor pass. clippy and fmt are clean.

---

Note

Medium Risk
Changes composer vendoring, revert safety, and lock parsing paths where incorrect behavior could strand installs or delete wired artifacts; mitigated by extensive tests and fail-closed revert logic.

Overview
Composer locks and manifests with a single leading UTF-8 BOM are now parsed through shared helpers (parse_json_text / parse_json_manifest, strip_bom) across lock inventory, the vendored backend, CLI hints, crawler, upstream restore, and discovery. Lock rewrites preserve the BOM on disk, and the parse memo treats BOM-prefixed bytes as equivalent to the canonical render.

vendor --revert no longer treats an unreadable composer.lock as “nothing wired.” The stranded-package scan returns an error for missing parse (merge conflicts, double BOM, etc.) and refuses revert instead of drift-skipping and deleting artifacts the lock still references—a regression that also affected BOM locks before the scan parsed them.

Adds production_sources test support and architecture tests (lock_pinned_locations_never_drive_a_download in core + CLI) that pin every production use of registry downloads and recover_lock_entry, plus refactors the inline-BOM guard to use the same scanner. Module docs clarify that lock inventory / recovery are gates only, not pristine download sources.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: dc39f60ecaa0348b0951c045563068c29325ff45
• CI: 80/82 checks green; the only red is the e2e_vex_build pdm leg (+ its ci-ok rollup), which is main-wide: hishel 2.0.0 (released 2026-11-05) breaks pdm lock on every PR. Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it; merge main in after it lands.
• Bugbot: reviewed this head, no issues found.
• Mergeable, no CHANGELOG.md change.
• Slack announcement not sent this run (no Slack send tool available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Every composer.lock reader now skips one leading UTF-8 BOM before parsing. That covers the lock inventory, the vendored plan/prelude, --revert restore, the stranded scan and the CLI hints. When the lock is rewritten, the BOM comes off first and goes back on afterwards, so vendor followed by revert leaves the lock byte-identical. The rest of the PR is docs plus one architecture test that pins which call sites may download a lock entry.

Risk: low. About 10 lines of production code change: four strict serde_json parses now go through the existing BOM-aware helpers, and one function puts the BOM back. The memo cache stays correct, because reseed_memo only reuses the cached parse when the bytes match the canonical render. The one destructive path, the stranded refusal during revert, only becomes stricter. Before this PR, a BOM lock parsed as "nothing wired", so revert could delete an artifact the lock still referenced.

Look here

• composer_lock.rs:829: lock_bytes_with_entry splits the BOM off and puts it back, on both the splice path and the whole-document path
• composer_lock.rs:868: stranded_wired_packages, the fix that matters for safety
• composer_lock.rs:4413: revert_refuses_to_strand_a_bom_lock_entry, which would fail on the old code
• lock_inventory/composer.rs:44 and composer_hints.rs:52: the other readers
• lock_inventory/tests.rs:3783: the architecture test

Verified

• Read the full diff against the head versions of the touched files and the BOM helpers in formats/text.rs.
• Grepped for other composer.lock readers. The textual redirect code doesn't parse JSON, so a BOM can't break it.
• CHANGELOG.md is untouched and there's no debug code left in.
• CI: ci-ok is red only on the known pdm e2e (e2e_vex_build pdm, hishel 2.x). That failure is red on every PR right now and Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it. clippy is green.
• My local run of the core tests didn't finish (the build ran out of memory on the review box). The tests themselves pass in CI.

Open questions (non-blocking)

• The architecture test matches literal call strings. It would miss a bare download( brought in by use, and code after a file's first #[cfg(test)]. Treat it as a tripwire, not the guarantee the description implies.
• Out of scope, as the description says: any_live_file_references still misses \/-escaped paths.

Auto-merge is not armed: ci-ok has to go green first (after #2028 lands and main is merged in). Once it's green it gets armed, and approving then sends it straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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 75b6ca3. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review re-confirmed at the current head (burn-down agent).

• Head: 75b6ca30
• CI: 47/49 green or skipped. The 2 red checks are main-wide: e2e (ubuntu-latest, e2e_vex_build, … pdm:: …) fails with hishel 2.x (module 'hishel' has no attribute 'SyncSqliteStorage'), the same failure as pdm e2e on main; ci-ok is red only because of that job. The fix is Fix pdm e2e red on every PR: pin hishel<2 #2028.
• Bugbot: reviewed 75b6ca30 with no findings.
• Mergeable: yes, no CHANGELOG change.
• Slack: not announced; this run had no Slack send tool.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (updated for 75b6ca3)

What it does. Every composer.json/composer.lock reader now skips one leading UTF-8 BOM. vendor --revert now refuses to delete vendored artifacts when an existing composer.lock can't be read or parsed (a missing lock still proceeds). The new commit also replaces a text-matching guard with a comment/string-aware source scan that pins every production reference to the registry download paths, so a lock-pinned (possibly Socket-hosted) URL can never drive a download.

Risk: low. Production changes are small, fail closed, and touch only composer handling; most of the diff is test-only (test_support/production_sources.rs is gated on cfg(any(test, feature = "test-fixtures"))).

Look here

• composer_lock.rs:524: revert refuses over an unreadable/unparseable lock (user-visible; message says how to fix)
• composer_lock.rs:883: stranded_wired_packages now returns Result; only NotFound means "nothing wired"
• composer_lock.rs:827: reseed_memo / lock_bytes_with_entry keep the parse cache for BOM locks
• production_sources.rs:38:`` the new source scanner behind the download guard
• tests.rs:3790: PINNED_REFERENCES

Verified

• Read the new commit and the full PR diff; CHANGELOG.md untouched; no debug leftovers or unrelated edits.
• New tests (revert_refuses_over_an_unparseable_lock, stranded-scan unwrap_err cases, BOM cases) would fail without the change.
• Local test run not possible this hour (runner disk full); relying on CI. CI: clippy green; ci-ok red only on the shared pdm e2e (e2e_vex_build pdm, hishel 2.x), which Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes.

Nits (non-blocking)

• is_test_cfg (production_sources.rs:242) doesn't recognise cfg_attr or reordered all(..., test); documented.
• The core guard matches the bare word download, so a new unrelated local named download will trip the test (noisy but fails safe).

Auto-merge is not armed yet: ci-ok must be green first (after #2028 lands and the branch picks it up).

---

Generated by Claude Code