Skip to content

Fix gem stale-install guard home selection (#1001, #729) - #1002

Merged
Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
agent/fix-gem-stale-guard-roots
Oct 8, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
agent/fix-gem-stale-guard-roots

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #1001
Fixes #729

Summary

scan --mode hosted (and get --mode hosted) now judges stale gem installs only in the gem homes bundle install actually installs into or reuses, and it decides whether a home belongs to the project the same way however --cwd is spelled.

Root cause

The guard (gem_stale_install_warnings in crates/socket-patch-cli/src/commands/scan/hosted.rs) answered two questions with lexical tests on RubyCrawler::get_gem_paths, a flat path list built for agent apply:

  1. Which homes does Bundler reuse? get_gem_paths appends the gem env homes whenever the default vendor/bundle has no store. apply needs that for default gems. But with an explicit path, Bundler's use_system_gems? is false: it fetches non-default gems into the path and never reuses a system copy.
  2. Is a home project-local? The guard used dir.starts_with(cwd) against the raw --cwd. The crawler hands back vendor/bundle/... for the default ., and Path::starts_with(".") doesn't match that.

Fix

  • bundler_sets_explicit_path follows Bundler's Settings#path. The first tier (app config, env, global config) that sets path, path.system or disable_shared_gems decides alone, and its path is explicit only when it's non-empty, path.system isn't true and disable_shared_gems isn't false. Both flags go through Bundler's to_bool (bundler_truthy, from Fix gem crawl ignoring Bundler path.system (#915) #916). A higher tier's path.system: true therefore beats a lower tier's path (a Bugbot finding, fixed in 5ed25cd). An empty path counts as not explicit, so when it's unclear the system homes are still judged.
  • RubyCrawler::bundler_install_homes returns the install stores, the refused out-of-tree config root (Hosted gem VEX attests not_affected for an unpatched install when .bundle/config sets an out-of-tree path (absolute or ~/…), because the skipped bundle root counts as "nothing installed" #709), and the gem env homes only when Bundler uses system gems, i.e. no deployment store and no explicit path. Each home comes back tagged project-local by bundler_gem_homes_from, which compares absolute, lexically normalized paths, as the containment guard already does.
  • The guard uses that API. get_gem_paths and apply are unchanged.
  • CLI_CONTRACT.md's "Gem stale-install guard" section now describes both rules.

No wrapper (npm/, pypi/, gem/) changes are needed. This is Rust-side discovery logic.

Tests (red → green)

Issue Test Without the fix With the fix
#1001 e2e_redirect_gem_stale_install::gem_hosted_explicit_bundle_path_ignores_system_home_copy (local config and env BUNDLE_PATH; fake gem on PATH points at a home holding an unpatched copy) FAILED: redirect_gem_stale_install for the system copy, no_applicable_patches ok: no warning, VEX written with the purl, exit 0
#1001 (control) gem_hosted_system_install_still_flags_system_home_copy (no path set) ok ok: still warns, shared flavor, not attested
#1001 ruby_crawler::tests::bundler_sets_explicit_path_follows_settings_tiers (14 tier and to_bool combinations) doesn't compile (new fn) ok
Bugbot gem_hosted_system_install_still_flags_system_home_copy, arm with a local path.system: true over env BUNDLE_PATH FAILED on 9dd72b4 (no warning) ok on 5ed25cd
#729 gem_hosted_default_cwd_keeps_project_local_remedy (no --cwd, and --cwd ., with a committed vendor/cache archive) FAILED: "shared gem home" plus a separate cache warning ok: one project-local warning with the archive in its delete list
#729 ruby_crawler::tests::bundler_gem_homes_from_tags_project_local_by_absolute_path doesn't compile (new fn) ok

Commands run locally

On d6a1466 (main merged in, 33 commits including #916; d65c5c4 only adds the digest-list port, and the digest tests pass):

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test --workspace --all-features --lib: CLI 861/861. Core 5559 passed, 5 failed: the same root-only permission tests and the digest guard listed below, none in files this PR touches.
  • e2e_redirect_gem_stale_install: 35/35. e2e_redirect_gem_build --include-ignored with real Bundler 4.0.18: 31/31.
  • e2e_gem --include-ignored (the live-API smoke suite): 3 tests fail only because this sandbox can't reach patches-api.socket.dev (tunnel error). CI runs it with network.

Earlier, the full suite on 9dd72b4 (CI was green on that head in every workflow):

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --all -- --check: every hunk this PR touches is clean. main itself isn't rustfmt-clean with the pinned 1.93.1 toolchain (122 files differ), and CI doesn't run a fmt check, so I left the unrelated files alone.
  • cargo test --workspace --all-features, run in batches of 10 test binaries because one full build exceeds this sandbox's disk:
    • all 35 e2e_redirect_gem_stale_install tests pass, plus every other suite, except as noted below;
    • socket-patch-core --lib: 5247 passed, 5 failed. 4 are permission-injection tests (copy_tree, vlt_heal, pypi_poetry, pypi_requirements) that can't fail a write when the sandbox runs as root. The 5th is utils::digest::tests::production_digests_go_through_the_helpers, the known main failure that Route Gradle digests through utils::digest #878 fixes;
    • CLI covgap_commands_vendor (3), in_process_redirect (3) and repair (2): all write- or remove-failure injection tests, also defeated by running as root. None of them touch gem code.
  • SOCKET_PATCH_BUNDLER_E2E_REQUIRED=1 SOCKET_PATCH_BUNDLER_E2E_VERSION=4.0.18 cargo test -p socket-patch-cli --all-features --test e2e_redirect_gem_build -- --ignored (real Ruby 3.3.6 / Bundler 4.0.18): 16/16 pass.

Notes

🤖 Generated with Claude Code


Note

Medium Risk
Changes hosted scan/VEX behavior for Ruby projects with Bundler path settings—fewer false stale warnings but different warning text and VEX inclusion when system gem copies are ignored.

Overview
Hosted-mode gem stale-install probing no longer walks every path from agent apply's get_gem_paths (which always included gem env homes). It now uses RubyCrawler::bundler_install_homes, which limits checks to stores Bundler actually installs into or reuses, and only adds machine gem env homes when Bundler is on system gems (no explicit install path / deployment store), via new bundler_sets_explicit_path tier logic aligned with Bundler's Settings#path.

Each home is tagged project-local vs shared with bundler_gem_homes_from (absolute, normalized containment), so default --cwd . still treats vendor/bundle as project-local and picks the delete-list remedy instead of the shared-home caveat. CLI_CONTRACT.md documents the rules; e2e and ruby_crawler unit tests cover explicit BUNDLE_PATH, path.system precedence, and default-cwd behavior.

Reviewed by Cursor Bugbot for commit aa42c4a. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
The hosted stale-install guard needs to know which gem homes
`bundle install` installs into or reuses, and which of those belong
to the project. The crawler's flat get_gem_paths list can't say: it
keeps the `gem env` homes for apply's default-gem fallback even when
the project sets its own Bundler `path`, and it gives relative dirs
for the default `--cwd .`.

Record in BundleStoreDiscovery whether an explicit install path is
configured (app config, env BUNDLE_PATH or global config), and add
RubyCrawler::bundler_install_homes. It returns the install stores,
the refused out-of-tree config root, and the `gem env` homes only
when Bundler uses system gems. Each home is tagged project-local by
comparing absolute, normalized paths.

Assisted-by: Claude Code:claude-opus-5-5
`scan --mode hosted` flagged an unpatched copy in the machine's gem
home as stale even when the project sets a Bundler `path`. Bundler
never reuses that copy, but the warning dropped the gem from the
same run's VEX, so `scan --mode hosted --vex` failed with
no_applicable_patches on every fresh checkout (#1001).

With the default `--cwd .`, a stale copy in the project's own
vendor/bundle was called a "shared gem home" with a remedy that does
nothing, and the committed vendor/cache archive was left out of the
delete list (#729).

The guard now uses RubyCrawler::bundler_install_homes, so it judges
only the homes Bundler uses and takes the project-local tag from the
crawler instead of a lexical starts_with(cwd).

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 7, 2026 12:15
@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.

Comment thread crates/socket-patch-core/src/crawlers/ruby_crawler.rs Outdated
The stale-install guard skipped the machine's gem homes whenever
any tier set a Bundler install path. But Bundler takes `path`,
`path.system` and `disable_shared_gems` from the first tier (local,
env, global) that sets any of them. So a local `path.system: true`
puts Bundler back on system gems even when BUNDLE_PATH is set in
the environment. In that setup a stale system copy wasn't warned
about, and the same run's VEX could attest it.

Decide this the way Bundler's Settings#path does, with
bundler_sets_explicit_path, and drop the discovery flag that
ignored tier order.

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.

Stale Bugbot comment from a previous run.

Resolve ruby_crawler.rs by keeping the tests from both sides.

Main's #916 added bundler_truthy, Bundler's to_bool coercion, so
bundler_sets_explicit_path now reads path.system and
disable_shared_gems through it ("1" and "yes" count as true, "no"
and "0" as false), the same as #916's path.system tier check.

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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/crawlers/ruby_crawler.rs
The digest guard test is red on main: #955 added
crawlers/gradle_cache.rs, patch/jvm_jar.rs and
patch/sidecars/maven.rs to the pending list, and #690 had already
moved them onto the utils::digest helpers. This ports the same
three-line change as #1016, so it becomes a no-op once #1016 lands.

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

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

[agent] coverage failed on d6a1466 in utils::digest::tests::production_digests_go_through_the_helpers. This PR didn't cause it: the same guard is red on main (for example coverage on 1c6c509). #955 added crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs to the pending list after #690 had already moved them onto the utils::digest helpers. #1016 fixes it on main. I ported its identical three-line change in d65c5c4 so this PR can go green, and it becomes a no-op once #1016 merges.


Generated by Claude Code

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

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Blocked: waiting on CI to finish on d65c5c4 and on #1016 to merge.

Generated by Claude Code

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

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent). Head d65c5c4. CI: all latest check runs green (532 success, 6 skipped, 0 failing). Bugbot reviewed d65c5c4: no new issues; earlier findings fixed (5ed25cd) or refuted in-thread; no unresolved review threads. 1 commit behind main (#1016, already ported here), mergeable. Reviewer focus: bundler_sets_explicit_path tier precedence vs Bundler's Settings#path; #968 touches discover_bundle_stores_impl too, so the second to land needs a small merge.


Generated by Claude Code

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

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Blocked: waiting on CI for aa42c4a.

  • Merged current origin/main into the branch (no conflicts). Pushed as aa42c4a. It now has the ci-ok merge-queue job.
  • Local: ruby_crawler unit tests (93) and e2e_redirect_gem_stale_install (36) pass. Linux-equivalent clippy is unaffected. macOS clippy flags an unused unix_default in python_crawler.rs, which comes from main and is outside this PR.
  • CI on aa42c4a when this pass ended: 4 success, 2 in progress, 66 queued, 0 failing.
  • Bugbot: bugbot run requested on aa42c4a at 18:50Z. No result yet. Both earlier Bugbot threads already have replies.
  • I removed the Ready for review label until ci-ok is green on this head.

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 aa42c4a. Configure here.

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

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

[agent] Ready for review at aa42c4a375f7c066bdd9d205b10526ab6a35635d.

  • CI: required ci-ok green. 523 success / 7 skipped / 0 failing of 558 check runs. 28 non-required macOS/Windows native / install-proof legs are still queued on the runner backlog.
  • Bugbot reviewed aa42c4a375 with no unresolved findings; no open review threads.
  • Mergeable, no conflicts.
  • Reviewer focus: gem-home selection in gem_stale_install_warnings (scan/hosted.rs) and the Bundler path precedence.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment