Repository navigation
Fix pnpm lock/workspace readers missing a BOM (#903, #904, #905) - #909
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
A pnpm-lock.yaml or pnpm-workspace.yaml saved with a UTF-8 BOM (as Windows editors do with "UTF-8 with signature") is read normally by pnpm, but socket-patch's pnpm readers matched column-0 text and missed the first line: - hosted scan skipped the trustLockfile auto-config, so frozen pnpm 11/12 installs failed; rollback, remove and list then refused the lock the scan had just pinned, and vendored mode refused it as having no lockfileVersion (#903) - a BOM first key in pnpm-workspace.yaml was missed, so hosted and vendored appended a duplicate trustLockfile / overrides key that pnpm refuses to parse (#904) Add formats::text with one split_bom/strip_bom pair, replace the four named copies with it, and make the pnpm lock sniffs and the workspace key reader skip one leading BOM. Splices keep the original line, so the BOM stays byte-exact. lockfileDir is now read through the same workspace key reader (#905). Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c)
|
[agent] Generated by Claude Code |
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 7515fa7. Configure here.
Assisted-by: Claude Code:claude-opus-5-5
|
[burn-down agent] Ready for review at head
Generated by Claude Code |
Assisted-by: Claude Code:claude-opus-5-5
LLM Description written by Claude Code:claude-opus-5-5
Fixes #903
Fixes #904
Refs #905 (this PR does steps 1 and 2 of its proposed change; step 3, re-pointing the ~50 inline
strip_prefix('\u{feff}')sites, stays open as the issue's follow-up slice)Summary
Windows editors that save "UTF-8 with signature" put a BOM at the start of
pnpm-lock.yaml/pnpm-workspace.yaml. pnpm reads those files normally, but socket-patch's pnpm readers matched column-0 text and missed the first line:trustLockfile: trueauto-config when pnpm-lock.yaml starts with a UTF-8 BOM, so pnpm 11/12 frozen installs fail with ERR_PNPM_TARBALL_URL_MISMATCH after a successful scan #903: on a BOM lock, the hosted trust gate saw nolockfileVersion, so it skipped thetrustLockfile: trueauto-config and pnpm 11/12 frozen installs failed.rollback/remove/listthen refused the lock the scan had just pinned ("not a pnpm lockfile"), and vendored mode refused it with a false "no lockfileVersion … re-lock" remedy.trustLockfile/overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan #904: a BOM-prefixed first key inpnpm-workspace.yaml(trustLockfile:oroverrides:) wasn't recognised. Hosted and vendored then appended a duplicate key, which pnpm refuses to parse, so every pnpm command broke. An explicittrustLockfile: falsewas also overridden.Root cause
formats::pnpmhad no BOM handling.head_lock_version,lock_versions(behindlock_version_majorandmay_need_store_flag),is_pnpm_lock_text,unsupported_early_shrinkwrapandworkspace::top_level_keyall match a column-0 literal. The crate has no shared BOM helper (#905): four named copies plus ~50 inline strips, and each new reader decides for itself.Changes
formats::text::{split_bom, strip_bom}, which treats exactly one leading BOM as encoding. The four named copies (utils::serde::strip_bom,gradle::dsl::strip_bom,formats::yarn::strip_bom,cargo_manifest::split_bom) are deleted and their callers re-pointed.strip_bom(text). A newformats::pnpm::is_shrinkwrap_lockreplaces the engine's inlineshrinkwrapVersion:check.workspace::top_level_keyskips a leading BOM, and so doesblock_insert_pointon line 0. The splices keep the original line, so the BOM stays byte-exact on write and on revert.governing_root::workspace_lockfile_dirnow reads throughtop_level_key, which deletes its private key grammar (pnpm lock and workspace readers don't skip a leading BOM, because BOM handling has no shared helper (4 named copies, ~50 inline strips) #905 step 2).coveragestops failingproduction_digests_go_through_the_helpers, which is red onmain. It becomes a no-op once Route Gradle digests through utils::digest #878 merges.Per-issue test mapping (red on
main, green here)formats::pnpm::tests::bom_lock_reads_like_its_plain_twin(9.0, 6.0, 5.4, 5.2 and shrinkwrap twins:is_pnpm_lock,sniff_lock_grammar,lock_version_major,may_need_store_flag,check_v9_lock_version,entries)vendor::pnpm_lock::tests::bom_lock_vendors_and_reverts_byte_exactRefused vendor_lockfile_version_unsupported: … has no lockfileVersion in its headin_process_redirect_pnpm::hosted_bom_lock_and_workspace_read_like_their_plain_twins(scan creates the trust scaffold, rollback restores the BOM lock byte-exact, a BOMtrustLockfile: false/trueis kept and not duplicated)formats::pnpm::workspace::tests::top_level_key_skips_a_leading_bomhosted::guidance::tests::workspace_trust_plan_reads_a_bom_first_keyvendor::pnpm_lock::tests::bom_workspace_override_inserted_beside_existing_not_duplicatedformats::text::tests::*,hosted::governing_root::tests::workspace_lockfile_dir_reads_the_top_level_key(extended)Verification
cargo clippy --workspace --all-features -- -D warnings: clean.cargo test -p socket-patch-core --all-features: the only local failures are 4 permission-denial tests that can't fail as uid 0. They fail identically onorigin/mainin this sandbox and pass in CI.cargo test -p socket-patch-cli --all-features --liband thein_process_redirect_pnpm,in_process_rollback_hosted,hosted_memory_{engine,parity,rollout},e2e_safety_pnpm,e2e_vendor_pnpm_buildande2e_npmsuites: all pass locally.cargo fmt --all -- --checkalready fails onmainwith this toolchain, and CI doesn't gate on it. I formatted only the changed lines and left unrelated files alone.Follow-ups
formats::text.🤖 Generated with Claude Code
https://claude.ai/code/session_0167b49WbzczWBJxsnUhCNEy
Generated by Claude Code