Skip to content

Fix requirements files read as absent when not UTF-8 (#1120, #1119) - #1152

Merged
Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-decode
Oct 9, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-requirements-decode

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1120
Fixes #1119

Root cause

The requirements-file readers had no single pip-compatible
decode-or-refuse entry point. utils::requirements::decode (#724)
handled byte-order marks only, with no PEP 263 coding: step (#1119).
Several call sites skipped it entirely and read strict UTF-8:
vendor/pypi.rs requirements_pins_target (the #612 probe), the
shared -r include walk (requirements_include_names) and
vex/discover/pypi_other.rs extract_requirements (#1120). Each site
then mapped a file it couldn't decode to "absent", so scans,
vendor --check and vex all wired around a file pip or uv still
installs from.

Fix

No wrapper changes are needed (npm/, pypi/, gem/ only dispatch to
the binary).

Per-issue tests (red without the fix, green with it)

Issue Test
#1119 utils::requirements::tests::decode_follows_pips_pep_263_coding_line
#1119 vendor::lock_inventory::tests::requirements_pep_263_files_are_inventoried (root file and -r include, disk and in-memory)
#1119 CLI scan_requirements_lock_only::lock_only_scan_discovers_pep_263_pins (hosted and vendored lock-only scan, root and include)
#1120 vendor::pypi::tests::non_utf8_requirements_beside_the_governing_lock_is_a_loud_loser (UTF-16 LE/BE, PEP 263, undecodable)
#1120 vex::discover::pypi_other::tests::requirements_files_decode_as_pip_does (UTF-16 root and include, PEP 263 include, undecodable diagnosed)
#1120 vendor::pypi_requirements::tests::requirements_include_names_decodes_as_pip_does
#1120 CLI mode_migration_pypi::uv_requirements_export_contests_the_wiring_in_any_encoding (vendor warns pypi_multiple_lockfiles, vendor --check fails "wiring contested", for UTF-8, UTF-16 LE and UTF-16 BE)

To show red, I temporarily reverted only the non-test fix hunks. All 5
new core tests and both new CLI tests then failed.

Local evidence

  • cargo clippy --workspace --all-features -- -D warnings: clean
    (before and after merging main).
  • cargo test -p socket-patch-core --all-features: 5788 passed, 4
    failed. The 4 are read-only-permission tests (relax_loop_must_not_traverse_symlinked_root,
    an_unremovable_hidden_lock_keeps_every_store_entry,
    wire_write_failure_maps_error_and_leaves_lock_untouched,
    wire_failure_rolls_back_already_written_files) that can't fail a
    write as root, and this sandbox runs as uid 0. They are unrelated to
    this diff and pass in CI.
  • e2e_vendor_pypi_build -- --include-ignored with real uv 0.11.32:
    36/36 passed.
  • PyPI CLI suites (scan_requirements_lock_only, mode_migration_pypi,
    scan_vendor_requirements_unwired, in_process_vendor_pypi_takeover,
    in_process_redirect_pipenv, hosted_superseding_pypi,
    in_process_get_hosted_ecosystems, hosted_memory_engine,
    e2e_redirect_uv_build): all green except
    pipenv_hosted_to_vendored_names_the_unpatched_requirements. That
    test fetches https://pypi.org/pypi/six/1.16.0/json live, and the
    sandbox's TLS-inspecting proxy breaks that request because the client
    trusts only bundled roots (Every HTTPS call fails behind a TLS-inspecting proxy because the clients trust only bundled webpki roots #1107). It passes in CI.
  • A full cargo test --workspace didn't fit in the sandbox's disk
    allowance (hundreds of CLI test binaries). CI covers it.

CI

🤖 Generated with Claude Code

https://claude.ai/code/session_01YTFXaxBRSh6oHYt8J9yegn


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A requirements.txt that pip or uv installs from was read as absent
whenever it was not plain UTF-8:

- A file saved as Latin-1 or cp1252 with a PEP 263 coding line
  ("# -*- coding: latin-1 -*-") was skipped by lock-only discovery, so
  a fresh-checkout scan said "No pypi packages found" and exited 0
  while pip installed the unpatched release (#1119).
- A UTF-16 requirements.txt exported beside uv.lock (what
  `uv export > requirements.txt` writes in Windows PowerShell 5.1) was
  ignored by the vendored routing, `vendor --check` and `vex`. The scan
  gave no pypi_multiple_lockfiles warning, the check passed and VEX
  attested not_affected while `uv pip install -r requirements.txt`
  installed the unpatched release (#1120).

pip's decoder now lives in one place. utils::requirements::decode
follows pip's auto_decode: byte-order marks first, then a PEP 263
coding line in the first two lines (UTF-8, ASCII, Latin-1 and cp1252
and their Python aliases), then UTF-8. A codec it does not model is
reported as unreadable, never guessed.

Every reader that only observes requirements files now goes through
it: the -r include walk, the vendored #612 probe beside a tool lock,
and VEX discovery. The probe also counts a requirements.txt it cannot
decode as one that may pin the package, so the warning fails closed.
The vendored planner still refuses files that are not UTF-8, since it
rewrites them byte for byte.

Fixes #1119
Fixes #1120

Assisted-by: Claude Code:claude-opus-5-5
Adds the CLI-level regression for #1120: a requirements.txt exported
beside uv.lock (UTF-8, UTF-16 LE and UTF-16 BE) makes `vendor` warn
pypi_multiple_lockfiles and `vendor --check` fail on the contested
wiring, and is left byte-for-byte as the user wrote it. Before the
decoder change, both UTF-16 cases passed the check as if the file were
absent.

Refs #1120

Assisted-by: Claude Code:claude-opus-5-5
The formats::text guard test requires every UTF-8 BOM check to go
through the shared helpers (#905). The PEP 263 step checked for the
mark inline; it now asks strip_bom_bytes instead. No behavior change.

Refs #1119

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 8, 2026 18:11
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

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.

Resolves the conflict with #1091 in VEX requirements discovery: keep
its install_tree grouping of the -r include tree and read each file
through the pip-compatible decoder.

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 576706e. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] CI is green on 576706e. Three jobs failed once in code this PR doesn't touch. Each passed on its single re-run:

  • gradle 9.8.0 / jdk 21 / vendor / configuration-cache (3095cf5): Maven Central reported "Could not find org.apache.commons:commons-text:1.10.0" during fixture resolution.
  • e2e (e2e_redirect_maven_build, maven 3.8.9) (3095cf5): Maven Central returned HTTP 429 during fixture warm-up. Fix sbt docker e2e flake on Maven Central blips #1148 targets these Central blips.
  • test (windows-latest, 2) (576706e): e2e_redirect_gem_stale_install::gem_hosted_global_gemfile_setting_is_refused failed once on Windows. It passes on Linux (37/37), passed on Windows for 3095cf5 and on main, and passed on the re-run. This PR only changes PyPI requirements readers, so nothing to port here. The test may be intermittent on Windows and is worth a separate look.

Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 8, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 576706e.

  • CI: all check suites on the head are success (465 check runs, ci-ok success); no main-wide failures.
  • Bugbot: reviewed this head (Cursor check success); no unresolved review threads.
  • Mergeable, no CHANGELOG.md change.
  • Slack announcement not sent this run (Slack send tool unavailable); the next run will retry.

Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 8, 2026
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] The merge queue dropped this PR at 22:25 UTC because of e2e (ubuntu-latest, e2e_sbt_vendor_build, sbt, 1.13.0). The job failed before any test ran: scripts/sbt-warm-seed.sh couldn't download org.scala-sbt:sbt:1.13.0 ("not found" on repo1.maven.org and repo.scala-sbt.org).

The failure isn't caused by this PR. The same job passed on this PR's own CI at 576706e. It also passed in the queue runs for #1108 and #1147 during the same window, and this diff doesn't touch sbt or JVM code. There's no fix to port, so I re-queued the PR (22:26 UTC).


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 8, 2026
Resolve the pypi_requirements.rs conflict with #1168, which moved the
requirements walk onto ProjectView: keep this PR's per-caller decode
step (strict UTF-8 for the planner, pip's decoding for the include
listing) by reading bytes through view.read_bytes and decoding them,
and drop the now-unused read_regular_to_bytes import.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[final reviewer] The queue dropped this at 23:18 UTC because #1168 merged first and conflicts with it in crates/socket-patch-core/src/vendor/pypi_requirements.rs. Merged main at 9e8d605a:

  • Fix PyPI revert deleting a wheel used by a subdir requirements file (#1167) #1168 moved walk_requirements_tree from a disk root onto ProjectView (view.read_text(rel)); this PR gave the walk a per-caller decode step. The merged walk takes both: walk_requirements_tree(view, decode, visit) reads view.read_bytes(&rel) and runs decode on the bytes, so the planner still refuses non-UTF-8 files (strict utf8) and requirements_include_names_in still decodes as pip does (decode_requirements), now for disk, snapshot and in-memory views alike. Dropped the now-unused read_regular_to_bytes import.

Tanmay Singla (@Tanmay182003) this is a merge commit only, but the conflict resolution touches the walk itself, so a quick look at the merge diff of pypi_requirements.rs is worth it. Locally: cargo clippy --locked -p socket-patch-core -p socket-patch-cli --all-features -- -D warnings clean; socket-patch-core lib 5838 passed (4 failures are chmod/symlink tests that can't fail as root in this sandbox), scan_requirements_lock_only 7/7, mode_migration_pypi 44/45 (the 1 failure needs pypi.org, which this sandbox can't reach). I'll send it back to the merge queue once CI is green on this head.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] The queue dropped this at 00:26 UTC because e2e-macos (macos-latest, e2e_vex_build, pdm:: --ignored, 2.29.2) failed. No test ran: the job died while unpacking the prebuilt test binaries (zstd -d … | tar -xf - → zstd: /*stdout*\: Broken pipe) on the macOS runner. That's a runner failure, not this PR, and the same suite passed on this PR's own CI at 9e8d605. Re-queued.


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] The queue dropped this again at 00:34 UTC on e2e (ubuntu-latest, e2e_vendor_jvm_build, maven, 4.0.0-rc-6, --ignored maven_reactor). Maven Central reported maven-dependency-plugin's dependencies as absent (commons-io 2.11.0, plexus-archiver 4.6.0, plexus-utils 3.5.0, …) during fixture resolution. That's the same Central flakiness #1148 targets, in JVM code this PR doesn't touch. Re-queued.


Generated by Claude Code

Merged via the queue into main with commit 793edd4 Oct 9, 2026
465 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-requirements-decode branch October 9, 2026 00:55
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants