Skip to content

Fix gem lockfile DSL call ignored (#1477) - #1479

Merged
Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-gem-lockfile-dsl
Oct 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-gem-lockfile-dsl

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #1477

Summary

Bundler 4 lets the Gemfile name its own lock with lockfile "custom.lock",
and that call wins over BUNDLE_LOCKFILE from the environment and both
config files. socket-patch only read the settings, so a hosted scan of such
a project pinned the leftover Gemfile.lock, rewrote the Gemfile, left
custom.lock (the lock bundler reads) untouched, reported no refusal, and
every frozen / deployment bundle install afterwards failed. In-run VEX
could attest a lock that was never pinned.

The scan now refuses such a project with
redirect_gem_bundle_lockfile_unsupported and writes nothing, in hosted and
vendored mode alike, with "remove the lockfile call from the Gemfile" as
the remedy. A call naming the pair's own lock (lockfile "Gemfile.lock")
changes nothing and still wires the project.

Root cause

LoadedManifest::with_lockfile (formats/gem/manifest.rs) was the only
lockfile resolver and was fed only the env / app-config / global-config
values. Bundler 4's Dsl#to_definition (lockfile = @lockfile unless @lockfile.nil?) lets the Gemfile call override all of them. Every gem lock
reader (hosted rewriter, lock inventory, VEX, vendored backend) asks the one
resolver in crawlers/ruby_crawler.rs, so the fix lives there.

What changed

  • formats/gem/manifest.rs: gemfile_lockfile reads the call from Ruby
    source. Only lockfile "<plain literal>" at the start of a statement is a
    path. lockfile false / lockfile nil (bundler 4 then reads no lock),
    computed or interpolated values, modifiers, calls later in a line, and
    calls that disagree all fail closed. Comments, strings, heredoc bodies,
    =begin/=end, __END__ and local variables named lockfile are
    skipped. with_gemfile_lockfile gives the call top precedence (new
    GemfileSetting::GemfileCall). eval_gemfile_paths lists literal
    eval_gemfile targets.
  • crawlers/ruby_crawler.rs: the settings probe now returns a
    BundlerSettings value, and resolve_in reads the loaded manifest's
    call through the ProjectView (disk, snapshot overlay or memory). It
    decodes lossily (legacy encodings, BOM) and follows literal
    eval_gemfiles inside the project, at most 32 files. The in-memory
    absolute-path rule moved into the lexical comparison, so it covers both
    lockfile sources.
  • npm / pypi / gem wrappers only dispatch to the binary, so they need no
    change.

Test evidence

Red→green: with gemfile_lockfile stubbed to return None (the old
behavior) the new tests fail, and with the fix they pass:

  • crawlers::ruby_crawler::tests::loaded_manifest_reads_the_gemfile_lockfile_call FAILED → ok
  • vendor::gem::tests::a_bundler4_gemfile_lockfile_call_is_refused FAILED → ok
  • hosted_memory_engine::a_bundler4_gemfile_lockfile_call_is_refused FAILED → ok
  • e2e_redirect_gem_build::gem_hosted_bundler4_gemfile_lockfile_call_redirects_nothing
    (real bundler 4.0.18, the issue's exact repro: frozen install after the
    scan) FAILED at the refusal assertion → ok

Unit tests: gemfile_lockfile_reads_the_dsl_call (each spelling, the
fail-closed shapes, and the non-calls), eval_gemfile_paths_reads_literal_paths,
and with_gemfile_lockfile_accepts_only_the_pairs_own_lock.

Local runs:

  • cargo fmt --all -- --check: ok
  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo test --workspace --all-features: everything passes except
    13 tests that fail only in this sandbox. Ten rely on chmod 0555
    blocking writes, which root ignores (covgap_commands_vendor ×3,
    in_process_redirect ×3, repair ×2, plus the core lib's
    copy_tree / vlt_heal / pypi_poetry / pypi_requirements tests). One
    (e2e_bun_lockb) needs patches-api.socket.dev, which the sandbox
    proxy blocks. None touch gem code.
  • e2e_redirect_gem_build + e2e_vendor_gem_build --ignored on bundler
    4.0.18: 30 + 8 passed.

Per-issue checklist

Review

I ran /code-review high and fixed every confirmed finding in 7c215c9:
lockfile nil, mid-line calls, eval_gemfiled calls, encoding/BOM,
heredoc/__END__/variable false positives, reading the manifest through
the snapshot overlay, and the duplicated memory-view absolute-path check.
I didn't add a refusal for an unreadable Gemfile (permission denied): the
scan can't wire such a project anyway.

Follow-ups

  • eval_gemfile with a computed path or a path outside the project is not
    followed. A lockfile call hidden there is still missed. That is rare,
    and refusing every computed eval_gemfile would block common setups.

🤖 Generated with Claude Code

https://claude.ai/code/session_017y6beSAab1H495Uwtdu5dU


Generated by Claude Code


Note

Medium Risk
Changes shared Bundler manifest/lock resolution for all gem scan modes; mistakes in DSL parsing could refuse valid projects or miss rare eval_gemfile paths, though behavior is intentionally fail-closed.

Overview
Fixes #1477: hosted and vendored gem flows now treat Bundler 4’s in-Gemfile lockfile "…" call as the authoritative lock source, above BUNDLE_LOCKFILE and .bundle/config.

Previously only env/config lock settings were resolved, so a project with e.g. lockfile "custom.lock" could get Gemfile.lock rewritten while bundler still read custom.lock, with no refusal and broken frozen installs / misleading VEX.

manifest.rs adds static parsing (gemfile_lockfile, eval_gemfile_paths, GemfileSetting::GemfileCall) and with_gemfile_lockfile so non-default locks fail closed with guidance to remove the call; a call naming the pair’s own default lock is a no-op.

ruby_crawler.rs refactors manifest loading through BundlerSettings::resolve_in, reading the DSL from the loaded manifest via ProjectView (including literal eval_gemfile chains, bounded depth).

E2E, in-memory hosted, and vendored tests cover the DSL path alongside the existing custom-lockfile config case.

Reviewed by Cursor Bugbot for commit 582f898. Configure here.

Assisted-by: Claude Code:claude-opus-5-5
Bundler 4 lets the Gemfile pick its lock with `lockfile "custom.lock"`,
and that call wins over BUNDLE_LOCKFILE from the environment and the
config files. socket-patch only read those settings, so a hosted scan
pinned the leftover Gemfile.lock, left custom.lock untouched, reported
no refusal, and every frozen install afterwards failed (#1477).

The shared gem resolver now reads the call from the manifest bundler
loads (Gemfile or gems.rb, on disk and in memory) before the settings.
A call naming the pair's own lock changes nothing; any other call,
`lockfile false`, or a value that cannot be read statically is refused
with redirect_gem_bundle_lockfile_unsupported, in hosted and vendored
mode alike, with "remove the `lockfile` call" as the remedy.

Tests: DSL reader and precedence unit tests, a vendored refusal test,
a memory-engine test, and a bundler 4 e2e twin of the #749 test.

Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the Gemfile `lockfile` call (#1477):

- `lockfile nil` drops the lock bundler would read, so it is refused
  like `lockfile false` instead of being read as no call.
- A call later in a line (`group(:ci) { lockfile "x" }`, `a; lockfile
  "x"`, `cond && lockfile("x")`) is refused instead of missed.
- Calls in files the Gemfile `eval_gemfile`s by a literal path are
  read too, since bundler evaluates them in the same builder.
- A Gemfile in a legacy encoding or with a BOM is still scanned.
- Heredoc bodies, the __END__ data section and local variables named
  `lockfile` (`||=`, `==`, `.foo`) no longer cause false refusals.
- In the hosted engine the manifest is read through the project view,
  so the overlay of a just-rewritten Gemfile is what gets scanned and
  recorded; the in-memory absolute-path rule moved into the lexical
  comparison, so it covers both lockfile sources.

Assisted-by: Claude Code:claude-opus-5-5
Clippy flagged the duplicated escape/interpolation test in the gem
lockfile reader; one helper now decides whether a quoted Gemfile
argument is a plain literal for both the `lockfile` and `eval_gemfile`
readers. No behavior change.

Assisted-by: Claude Code:claude-opus-5-5
The repo's BOM guard test requires every production BOM strip to go
through formats::text (#905); the gem lockfile reader now does.

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but none of the reported issues met your autofix severity threshold.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 582f898. Configure here.

Merged via the queue into main with commit 4493714 Oct 13, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-gem-lockfile-dsl branch October 13, 2026 14:15
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Oct 13, 2026
Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-merge-queue Bot pushed a commit that referenced this pull request Oct 13, 2026
* ci(release): publish npm from release.yml, not publish-npm.yml

Every socket-patch npm package's trusted publisher is registered against
release.yml, and npm allows only one publisher per package. The 5.0.0
release dispatched publish-npm.yml, whose OIDC token named that file, so
npm rejected it with E401 before staging anything.

Move the npm leg back inline as release.yml's npm-publish job (same steps
as publish-npm.yml) and delete publish-npm.yml, so the existing
registration keeps working. Add an `npm-retry-version` input that skips
the version gate, build, tag, GitHub release and crates.io jobs and
re-stages npm from an existing release's assets; this retries 5.0.0 now
and replaces the manual Publish npm dispatch. crates.io stays dispatched
through publish-cargo.yml, since crates.io allows several publishers per
crate.

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

* chore(release): fold 22 more PRs into the 5.0.0 notes

Cover the PRs merged to main after #1432's base (1779317..4cb3643):
Cargo source replacement and registry = "crates-io" (#338/#455, #386),
pnpm lockfile=false (#1074), berry removed descriptors (#1435), yarnrc
${VAR} reads (#1100), CRLF .npmrc append (#1196), Composer vendor-dir
cascade and global home (#439/#686, #586), Go +incompatible vex (#484),
gem moved BUNDLE_GEMFILE and commented Gemfile revert (#952, #988),
Poetry source = "pypi" (#1282) and Hatch path characters (#547).

No entries for #1411 (macOS fix to code #1393 added in this release;
the #1393 entry already describes the intended behavior), #1426 (perf
regression introduced and fixed within 5.0.0) or the CI/test/bench PRs.

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

* chore(release): cover #1419, #1434, #1440, #1442, #1444 in the 5.0.0 notes

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

* chore(release): 5.0.1

Cut [Unreleased] into [5.0.1] — 2026-10-13 and stamp 5.0.1 with
scripts/version-sync.sh. The 5.0.1 notes cover main through 501c6d9
(#1449), i.e. every PR after the v5.0.0 tag (6ad6add): the npm
publish fix (5.0.0 never reached npm or crates.io, 5.0.1 is the first
v5 there), #1465, #759, #1460, #1462, #1445, #869, #1447, #1450,
#1467, and #1449 under Maintenance. CI/test-only PRs (#1448, #1452,
#1454, #1457, #1459) have no entries.

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

* chore(release): 5.0.1 notes for PRs merged since the cut

Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

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

---------

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

None yet

Projects

None yet

3 participants