Repository navigation
Fix .bundle/config values keeping a trailing # comment (#951) - #953
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Bundler cuts a .bundle/config value at its first `#` (RubyGems and Bundler 2.5.6+), but socket-patch kept the comment as part of the value. A hand-commented `BUNDLE_PATH: .gems # note` therefore made agent apply patch the system gem copy, and vex attest not_affected, while Bundler loaded the unpatched .gems copy. A commented BUNDLE_CACHE_PATH also hid stale archives from the hosted stale-install guard, and commented path.system / gemfile values were misread too. The shared config value reader now strips the comment the way Bundler's config loader does. Older Bundler keeps the comment in the value, so for directory settings (path, cache_path) the legacy reading is used when only its directory exists on disk. Fixes #951 Assisted-by: Claude Code:claude-opus-5-5
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] On the start commit Generated by Claude Code |
|
BugBot review Generated by Claude Code |
A `BUNDLE_PATH__SYSTEM: true # note` line makes Bundler 2.5.6+ ignore the recorded BUNDLE_PATH and load system gems. The era fallback added for #951 could still pick the recorded path when a leftover directory existed there, so apply would patch a copy Bundler never loads. The legacy reading now only competes with another directory reading; an unset current reading always stands. Assisted-by: Claude Code:claude-opus-5-5
|
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 52542db. Configure here.
|
[agent] On Generated by Claude Code |
|
[agent] The Generated by Claude Code |
|
[agent] Ready for review at
Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #951
Root cause
Every
.bundle/configreader goes throughunquote_bundle_config_valueincrawlers/ruby_crawler.rs. That coversparse_bundle_config_path(the app and globalpath/path.system),bundle_config_setting*(cache_path, the tier-presence check) andformats::gem::manifest::config_gemfile. The helper trims and unquotes the value, but it never applies Bundler'sYAMLSerializer#strip_comment, which cuts the value at its first#unless the value starts with#. SoBUNDLE_PATH: .gems # noteresolved to a directory literally named.gems # note. Agentapplythen patched thegem envcopy andvexattestednot_affected, while Bundler loaded the unpatched.gemscopy. A commentedBUNDLE_CACHE_PATHlikewise hid stale archives from the hosted stale-install guard.Fix
unquote_bundle_config_valuenow matches Bundler's config loader (Gem::YAMLSerializer, which Bundler 2.4+ uses when present, and Bundler's own copy from 2.5.6). It trims and unwraps one matching quote pair that closes the line, then strips the comment. This was checked against the real loader:"a#b"→a,x#y→x,#zstays whole,"vendor/bundle" # ckeeps its quotes.strip_commentfirst shipped in RubyGems/Bundler 3.5.6/2.5.6 (I checked the published gems: 3.5.5 doesn't have it, 3.5.6 does). Bundler < 2.4, or 2.4–2.5.5 on RubyGems < 3.5.6, keeps the comment and installs into.gems # note, which the old code happened to handle correctly. The installed Bundler's era isn't known to the crawler, so for the two directory settings (pathfrom the app and global config, andcache_path),bundle_config_dir_readinguses the current reading unless only the legacy reading's directory exists. Bundler creates the directory it uses, so this follows whichever copy is actually installed. A value without a comment reads the same in both eras. An unset current reading, such as a commentedpath.system: true, always stands; a leftover directory at the recorded path doesn't bring that path back (Bugbot finding).path.systemandgemfileuse the current reading only, because a boolean or a file name has no directory to tell the eras apart. On a legacy Bundler a commentedgemfilestill fails closed (redirect_gem_bundle_gemfile_unsupported).Tests (red on
main, green here)ruby_crawler::tests::bundle_config_values_drop_a_trailing_comment_like_bundlerpath,path.system,cache_path,gemfileruby_crawler::tests::commented_bundle_config_settings_follow_bundler.gems # commentstore discoveryruby_crawler::tests::commented_app_config_bundle_path_discovers_the_bundler_storebundle install+ agentapplypatches the copybundle execloadsin_process_alternate_installers::bundler_commented_config_path_apply_patches_loaded_gemBUNDLE_CACHE_PATHe2e_redirect_gem_stale_install::gem_hosted_stale_archive_at_configured_cache_path_warns_and_is_not_attested(newapp-config-commentedrow)path.system: true+ leftover recorded dir stays on system gemsruby_crawler::tests::commented_path_system_true_ignores_a_leftover_recorded_pathruby_crawler::tests::commented_bundle_path_keeps_the_legacy_bundler_store(passes before and after)Every test above except the legacy guard fails with the
maincrawler (verified locally by swappingruby_crawler.rsback tomain) and passes with the fix. The Bugbot test also fails against the first version of the era fallback. The legacy guard passes on both, by design. The e2e ran against Bundler 4.0.18 / RubyGems 3.5.22 (Ruby 3.3.6), andbundle execconfirmed the patched file is the one Bundler loads.Local checks
cargo clippy --workspace --all-features -- -D warnings: clean.cargo test --workspace --all-features --no-fail-fast: 226 suites pass. 12 tests in 4 targets fail locally only because this sandbox runs as root, which ignores the read-onlychmodthose tests depend on (covgap_commands_vendor×3,in_process_redirect×3,repair×2, and core lib ×4:copy_tree::relax_loop…,vlt_heal::an_unremovable…,pypi_poetry::wire_write_failure…,pypi_requirements::wire_failure_rolls_back…). They are all permission-mode tests, none touch gem code, and CI runs them as a normal user.e2e_redirect_gem_build -- --ignored(16 passed) ande2e_vendor_gem_build -- --ignored(8 passed);in_process_alternate_installersbundler legs pass.3f8e1c1, cherry-picked with-x) to fix theproduction_digests_go_through_the_helpersfailure that is already red onmain. It becomes a no-op once Route Gradle digests through utils::digest #878 lands.CI
All 412 check runs on
52542dbare green (406 success, 6 skipped), and the merge state is clean. Two jobs needed one re-run each after hitting theirtimeout-minuteswith no failing test:coveragepassed in 14 minutes on re-run, andgradle 8.14.3 / jdk 21 / hosted / macos-latestpassed on re-run. Both had passed on3f8e1c1, which runs the same tests. Bugbot reviewed52542dband found no new issues, and its one finding on3f8e1c1is fixed and resolved.Notes: the npm, PyPI and gem wrappers only dispatch to the binary, so none of them needs a change.
cargo fmt --all -- --checkreports ~466 pre-existing diffs onmainitself (CI has no fmt gate). The three files this PR touches are rustfmt-clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01CcuTzWw24ikXbTnMaqW4JJ