[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.

Bugbot on 455e2010 (burn-down agent)

1. An inner #[cfg(test)] blanked the whole file: confirmed and fixed in 10c665c5. A top-level one still does; one inside an inline mod x { … } now blanks only that module's body, so production references after it stay visible to the download / BOM guards. Test: test_items_are_removed_in_place_not_truncated (extended). cargo test -p socket-patch-core --features test-fixtures --lib -- production_sources lock_pinned_locations_never_drive_a_download formats::text: 9 passed. Bugbot's re-run on 10c665c5 found no new issues.

---

Note

Medium Risk
Touches vendor revert/inventory paths and adds stricter failure modes around corrupted locks, so regressions could block reverts or mis-handle edge cases; download-reference guards are test-only but sensitive to future refactors.

Overview
Composer lockfiles and manifests with a UTF-8 BOM are now read consistently: one leading BOM is stripped via shared helpers (parse_json_text / parse_json_manifest, strip_bom), across lock inventory, vendored backend (parse, memo reseed, splice writes), discovery/crawler/upstream gates, and CLI vendored-package hints—so BOM-prefixed files behave like normal JSON instead of disappearing or failing spuriously.

Revert and stranded scans fail closed when composer.lock exists but is unreadable or unparseable (merge conflicts, double BOM, etc.) if the lock could still wire the artifact—preventing dangerous reverts that deleted vendor dirs while Composer still pointed at them; locks that never mention the UUID no longer block unrelated reverts.

Architecture guardrails add production_sources-based scans and paired core/CLI tests that pin production references to registry download/fetch_registry_bytes and recover_lock_entry, with docs clarifying that lock “pins” must not drive pristine downloads; supporting tests cover BOM round-trips, memo behavior, and inventory edge cases.

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

[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]

Stale Bugbot comment from a previous run.

[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

[Mikola Lysenko (mikolalysenko)]

Merge conflict resolved by the burn-down agent (head 455e2010; Ready for review removed until CI is green again).

Main's #2015 moved pristine_fetch_is_verifiable from the CLI's vendor.rs into core's vendor/refusals.rs. The merge keeps that move and carries this PR's intent over: the "never downloads a lock-pinned artifact" doc comment now sits on the core function, core's lock_pinned_locations_never_drive_a_download pins the recover_lock_entry call in vendor/refusals.rs, and the CLI guard now asserts the CLI has no recover_lock_entry reference. Both guard tests pass locally.

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[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 10c665c. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent).

• Head: 10c665c5e604a96090e3d4511e332f4d1fccd8e9. It is mergeable with no conflicts.
• CI: the CI and Audit GHA Workflows runs on this head are both green, with no failures.
• Bugbot: it found one issue on 455e2010, which is confirmed and fixed. An inner #[cfg(test)] inside an inline module blanked the whole file in the download-guard source scan. Now it blanks only that module's body, and the test is extended. Bugbot's re-run on 10c665c5 found no new issues.
• Auto-merge: I disabled it before pushing the fix. The final reviewer should re-arm it after looking at the new commit.
• Approval: the earlier approval (Tanmay182003) was on 22544e7b. Since then the branch has had a main merge and this one small, test-only fix in vendor/test_support/production_sources.rs.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Enqueued by the final reviewer (head 10c665c5). Tanmay182003's approval on 22544e7b is still valid. Since then the branch has had a main merge and a test-only fix to vendor/test_support/production_sources.rs, which Bugbot found clean. ci-ok and clippy are green, it is mergeable, and it has no unresolved threads.

---

Generated by Claude Code