GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix pnpm lock/workspace readers missing a BOM (#903, #904, #905) by mikolalysenko · Pull Request #909 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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:

Hosted pnpm scan skips the trustLockfile: true auto-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 no lockfileVersion, so it skipped the trustLockfile: true auto-config and pnpm 11/12 frozen installs failed. rollback / remove / list then 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.
pnpm-workspace.yaml with a UTF-8 BOM: hosted and vendored miss the first top-level key and append a duplicate trustLockfile / overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan #904: a BOM-prefixed first key in pnpm-workspace.yaml (trustLockfile: or overrides:) wasn't recognised. Hosted and vendored then appended a duplicate key, which pnpm refuses to parse, so every pnpm command broke. An explicit trustLockfile: false was also overridden.

Root cause

formats::pnpm had no BOM handling. head_lock_version, lock_versions (behind lock_version_major and may_need_store_flag), is_pnpm_lock_text, unsupported_early_shrinkwrap and workspace::top_level_key all 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

New 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.
The pnpm lock sniffs read strip_bom(text). A new formats::pnpm::is_shrinkwrap_lock replaces the engine's inline shrinkwrapVersion: check.
workspace::top_level_key skips a leading BOM, and so does block_insert_point on line 0. The splices keep the original line, so the BOM stays byte-exact on write and on revert.
governing_root::workspace_lockfile_dir now reads through top_level_key, which deletes its private key grammar (Consolidate remaining BOM stripping after the pnpm reader failures were fixed #905 step 2).
The npm/PyPI/gem wrappers need no change; this is Rust-internal.
CI port: cherry-picked Route Gradle digests through utils::digest #878's 659ac2c (7515fa7) so that coverage stops failing production_digests_go_through_the_helpers, which is red on main. It becomes a no-op once Route Gradle digests through utils::digest #878 merges.

Per-issue test mapping (red on `main`, green here)

Issue	Test	Without fix
#903	`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`)	FAILED
#903	`vendor::pnpm_lock::tests::bom_lock_vendors_and_reverts_byte_exact`	FAILED: `Refused vendor_lockfile_version_unsupported: … has no lockfileVersion in its head`
#903, #904	CLI `in_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 BOM `trustLockfile: false/true` is kept and not duplicated)	n/a (added after the fix; it covers the same readers)
#904	`formats::pnpm::workspace::tests::top_level_key_skips_a_leading_bom`	FAILED
#904	`hosted::guidance::tests::workspace_trust_plan_reads_a_bom_first_key`	FAILED
#904	`vendor::pnpm_lock::tests::bom_workspace_override_inserted_beside_existing_not_duplicated`	FAILED
#905	`formats::text::tests::*`, `hosted::governing_root::tests::workspace_lockfile_dir_reads_the_top_level_key` (extended)	refactor coverage

Verification

CI on 7515fa7: all 547 check runs completed (541 success, 6 skipped, none failed or pending). Bugbot's review of 7515fa7 found no new issues, and there are no review threads.
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 on origin/main in this sandbox and pass in CI.
cargo test -p socket-patch-cli --all-features --lib and the in_process_redirect_pnpm, in_process_rollback_hosted, hosted_memory_{engine,parity,rollout}, e2e_safety_pnpm, e2e_vendor_pnpm_build and e2e_npm suites: all pass locally.
cargo fmt --all -- --check already fails on main with this toolchain, and CI doesn't gate on it. I formatted only the changed lines and left unrelated files alone.

Follow-ups

Consolidate remaining BOM stripping after the pnpm reader failures were fixed #905 step 3: re-point the ~50 inline BOM strips to formats::text.

🤖 Generated with Claude Code

https://claude.ai/code/session_0167b49WbzczWBJxsnUhCNEy

Note

Medium Risk
Changes lock/workspace detection and trust-config editing paths used by hosted scan and vendored revert; behavior is narrowly scoped to BOM-prefixed files but affects install-critical YAML edits.

Overview
Fixes BOM-blind pnpm lock/workspace parsing so Windows “UTF-8 with signature” files behave like pnpm does (#903, #904).

Adds shared formats::text::{split_bom, strip_bom} and routes existing BOM helpers (serde, Gradle DSL, Yarn, Cargo manifest) through it. pnpm lock sniffing (lockfileVersion, shrinkwrap detection, is_pnpm_lock_text) and workspace top-level key parsing now strip a leading BOM before matching column-0 text, while splices keep the BOM byte-exact on write/revert. workspace_lockfile_dir reads lockfileDir via the same top_level_key grammar.

Hosted/vendored behavior: BOM locks get trustLockfile: true when appropriate, rollback works, explicit BOM trustLockfile: false is not duplicated, and overrides merge beside existing sections instead of appending duplicate keys.

New unit, vendor, and CLI integration tests cover BOM lock/workspace scenarios.

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

Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #903, #904, #905

adda011

Assisted-by: Claude Code:claude-opus-5-5

This was referenced Oct 6, 2026

Hosted pnpm scan skips the trustLockfile: true auto-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

Closed

Consolidate remaining BOM stripping after the pnpm reader failures were fixed #905

Open

Claude (claude) added 2 commits

October 6, 2026 01:33


          Read BOM-prefixed pnpm locks and workspace files

f81d6c1

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


          Test hosted pnpm scan/rollback on BOM files

7235dc3

Covers the #903 and #904 user flows end to end: a BOM lock gets the
trustLockfile auto-config and rolls back byte-exact, and a BOM first
trustLockfile key in pnpm-workspace.yaml is respected instead of
duplicated.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 6, 2026 01:50

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Route Gradle digests through utils::digest

7515fa7

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)

cursor Bot reviewed

Oct 6, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] coverage failed on 7235dc3 in utils::digest::tests::production_digests_go_through_the_helpers, an architecture guard that is red on main too: three Gradle files from #646 hash inline, but #865's guard requires them to use the utils::digest helpers. That isn't this PR's change. The fix is #878, so I cherry-picked its commit (659ac2c → 7515fa7) here. It becomes a no-op once #878 lands. The guard test passes locally after the port.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 6, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request

Oct 6, 2026


          refactor: re-rank queue, #905 taken by #909

c835c39

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 6, 2026

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Copy link

Copy Markdown

Collaborator Author

[burn-down agent] Ready for review at head 7515fa7 (0 behind main).

CI: 547/547 check runs completed, 0 failed/pending (541 success, 6 skipped).
Bugbot: reviewed 7515fa7, no new issues; no unresolved review threads.
Reviewer note: includes a cherry-pick of Route Gradle digests through utils::digest #878's Gradle digest fix (7515fa7) to keep coverage green; it no-ops once Route Gradle digests through utils::digest #878 merges.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request

Oct 6, 2026


          audit-ecosystems: file E22 (#920, #922) and E65 (#923); E64 in PR #909

d87c994

Assisted-by: Claude Code:claude-opus-5-5

Tanmay Singla (Tanmay182003) approved these changes

Oct 6, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 7, 2026

This was referenced Oct 7, 2026

Fix 15 open pnpm issues across hosted, vendored and agent modes #1007

Merged

Tracking: split patch/redirect/mod.rs into per-ecosystem modules and sibling test files #1010

Open


          Merge origin/main into agent/fix-pnpm-bom-readers

6743dd5

Bring the BOM-reader fix up to date with main (33 commits, including
sbt/Mill/scala-cli support and the Bun lock text reader).

- formats/mod.rs: keep both new modules, `sbt` from main and `text`
  from this branch.
- vendor/bun_lock_text.rs (new on main) called the
  `utils::serde::strip_bom` copy this branch deletes; point it at the
  shared `formats::text::strip_bom` so the merge builds.

Co-Authored-By: Claude <[email protected]>

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026

Copy link

Copy Markdown

Collaborator Author

bugbot run

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 7, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Merge branch 'main' into agent/fix-pnpm-bom-readers

1607f5d

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026

Copy link

Copy Markdown

Collaborator Author

bugbot run

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 7, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 1607f5d. Configure here.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) merged commit 4b4b15d into main

Oct 7, 2026

608 of 629 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-pnpm-bom-readers branch

October 7, 2026 15:25

Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request

Oct 7, 2026


          Merge origin/main into agent/fix-yarn-pnp-linker-detect

1a6636b

Conflict in formats/pnpm/mod.rs: took main's is_shrinkwrap_lock helper
(#909); the branch side was only a rustfmt reflow. Main moved
yarnrc_scalar to formats::yarn::berry_gates (#657), so the PnP linker
readers in pkg_managers and the in-memory view now call it there.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 7, 2026

Compare purls through one PurlKey type #1045

Merged

Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request

Oct 7, 2026


          Merge origin/main into agent/fix-pnpm-open-issues

dcae255

Conflict resolutions keep both sides' behavior:

- scan/vendor_flow.rs: preview_vendor_json takes both the run's
  patch-server origins (the hosted pnpm takeover lock-text prediction,
  #853) and main's caller-resolved takeover refusals (the gem preflight).
  preflight_refused_purls takes GlobalArgs (main) and derives the origins
  itself, folding in both the pnpm and the gem takeover refusals.
- scan/mod.rs, get.rs: callers resolve the gem takeover refusals and pass
  the origins too; get's lock_text_refusals_for keeps main's HostedPin
  list (the gem takeover needs the pins).
- hosted/engine.rs: union of both guidance imports.
- vendor/mod.rs: main's manifest_pins_yarn_classic helper, now reading the
  pin through utils::package_manager::pinned_version as this branch did.
- CLI_CONTRACT.md: word-level merge; the pnpm would_refuse preview text
  and main's gem preflight paragraph both kept.

Main (#909) removed utils::serde::strip_bom in favor of
formats::text::strip_bom; this branch's callers (pnpm version pins,
pnpm workspace globs, npm upstream restore, pnpm multi-document split)
now use main's helper.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 8, 2026

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

Merged

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pnpm-workspace.yaml with a UTF-8 BOM: hosted and vendored miss the first top-level key and append a duplicate trustLockfile / overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan Hosted pnpm scan skips the trustLockfile: true auto-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

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Changes

Per-issue test mapping (red on main, green here)

Verification

Follow-ups

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Per-issue test mapping (red on `main`, green here)

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.