GeezerVison is showing github.com — CI: share one bundler scrub across test helpers (#823) by mikolalysenko · Pull Request #1536 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #823 (slice 23)
Problem
Test children that run bundle each had their own environment scrub,
and the copies had drifted:
e2e_vendored_production, e2e_vendor_gem_build and
e2e_redirect_gem_build swept every BUNDLE_* and every
GEM_* by hand. Removing GEM_HOME/GEM_PATH goes against
tests/common/cache_env.rs's rule: rvm/chruby export them and
bundle resolves through them, so the suites could break a
developer's version-managed ruby. Two of the three also skipped the
SOCKET_* scrub.
e2e_hosted_production ran bundle through tool() with no bundler
scrub, so an ambient BUNDLE_PATH, BUNDLE_FROZEN or RUBYOPT reached it.
- The production suites'
socket-patch child got only Extra::Venv, so
an ambient BUNDLE_PATH/BUNDLE_GEMFILE or COMPOSER_VENDOR_DIR moved
the crawl off the fixture.
Change (test-only)
hermetic::Extra::Bundle also removes RUBYOPT (a bundle exec
shell sets -rbundler/setup, which pins child rubies to the outer
Gemfile). It keeps GEM_HOME/GEM_PATH, as before.
- New
hermetic::CRAWLER_EXTRAS (Venv, Bundle, Composer) for a
socket-patch child that crawls a real tree. The production suites'
run/run_socket and vex_e2e_common::run_vex use it. Env a test
passes is applied after the scrub, so it still wins.
- The three
bundle helpers use scrub_socket_vars +
Extra::Bundle. Both production suites' pm_command adds
Extra::Bundle, which covers the hosted tool("bundle") calls.
spawn_env_hygiene: the bundle test also checks RUBYOPT is removed
and GEM_PATH kept; a new test covers CRAWLER_EXTRAS; a new ratchet
no_private_bundler_sweeps rejects a hand-rolled
starts_with("BUNDLE_") sweep outside hermetic.rs.
- Review follow-up:
socket-patch children that crawl a real tree go
through hermetic::scrub_crawler_inputs (CRAWLER_EXTRAS plus
BUNDLE_USER_CONFIG pinned to a missing file, so the ruby crawler
can't fall back to a developer's ~/.bundle/config). The pin is not
in Extra::Bundle, because bundle children get their own
BUNDLE_USER_HOME from cache_env::isolate, which it would shadow.
Extra::Bundle also drops RUBYLIB (set by bundle exec). The gem
build suites' run_socket use the same helper.
Evidence
cargo test -p socket-patch-cli --all-features --test spawn_env_hygiene:
20 passed. On origin/main the new ratchet would flag the 3 helper
files (grep 'starts_with("BUNDLE_")' matches them there).
cargo test -p socket-patch-cli --all-features --test spawn_env_hygiene --test e2e_vendor_gem_build --test e2e_redirect_gem_build --test e2e_redirect_gem_stale_install --test apply -- --include-ignored
(ruby 3.3.6): 20 + 43 + 51 + 42 + 136 passed.
cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings (the CI lint) are clean. The production
suites compile (--no-run). They and e2e_gem need the live
patches API, which this sandbox can't reach ("tunnel error"), so they
run on the scheduled full tier.
- A full
cargo test --workspace ran out of disk in the sandbox. The
lean PR run covers it.
Code review (/code-review high)
Fixed: missing #[serial] on the new hygiene test; the
~/.bundle/config fallback for crawler spawns; RUBYLIB under
bundle exec; thin assertions in the new test; the stale
GEM_SPEC_CACHE doc; the gem build suites now use the shared helper.
Not fixed, with reasons:
- Apply the crawler scrub inside
hermetic::command for every spawn.
That changes ~20 more suites, so it belongs in a later Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823 slice
rather than this one.
- Widen
no_private_bundler_sweeps to other spellings. It is a ratchet
against copying the old pattern back. A fuzzier match would flag
legitimate code, and the env tests are what actually pin behavior.
- Share
pm_command between the two production suites. That duplicate
predates this PR and is out of scope here.
- Use
tool_command("bundle") for Windows bundle.bat. The gem build
suites only run on Linux.
CI cost
Test-only, no workflow changes. A lean PR run keeps the same job count
and minutes (about 28 Linux jobs, ~8 min wall), before and after.
🤖 Generated with Claude Code
https://claude.ai/code/session_018oK2ooQLZpyw2hVVqctE9C
Generated by Claude Code
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Three gem suites each swept BUNDLE_* and GEM_* by hand before running
`bundle`, and the hosted production suite ran `bundle` with no bundler
scrub at all. The GEM_* half went against cache_env.rs's rule that
rvm/chruby's GEM_HOME/GEM_PATH must stay, so the suites could break a
developer's version-managed ruby. The production suites' socket-patch
child also kept an ambient BUNDLE_PATH or COMPOSER_VENDOR_DIR, which
moves the crawl off the fixture.
Every bundle child now goes through scrub_socket_vars plus
hermetic::Extra::Bundle, which also drops RUBYOPT (a `bundle exec`
shell pins every child to the outer Gemfile). socket-patch children
that crawl a project (the production suites and run_vex) use the new
hermetic::CRAWLER_EXTRAS. A ratchet in spawn_env_hygiene rejects new
private BUNDLE_* sweeps.
Refs #823
Assisted-by: Claude Code:claude-opus-5-5
This was referenced Oct 14, 2026
Review fixes for the shared bundler scrub:
- socket-patch children that crawl a real tree now go through
hermetic::scrub_crawler_inputs, which also points
BUNDLE_USER_CONFIG at a missing file. Without it the ruby crawler
fell back to the developer's ~/.bundle/config, so a global `path`
or `gemfile` there could still move the gem crawl off the fixture.
- Extra::Bundle also drops RUBYLIB, which `bundle exec` sets to the
outer bundler's lib dir.
- The new hygiene test is #[serial] like every other with_ambient
caller, and it now checks the pins and the RUBYOPT/RUBYLIB removal.
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 7dfba36. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 14, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|