fix(cc): remove rustc wrapper directories left behind by in-place mbx upgrades

[jdx]

https://entire.io/gh/jdx/mr-boxington/trails/25

Problem

mbx installs the persistent RUSTC_WRAPPER at <shims_dir>/rust/<binary identity>/mbx-rustc, where the identity hashes the executable's path, size and mtime. Every in-place upgrade or rebuild of mbx at the same path creates a new rust/<id> directory. The old directory's symlink still resolves (to the new binary at that path), so the collector added in #516 (which only removes directories whose links all dangle) never removes it. One development machine had accumulated 206 of these directories.

Collecting them only by age isn't safe. A session that started before an upgrade, such as a long-lived mbx exec shell, keeps using its old wrapper and can outlive the 7-day unused window.

Fix

• Every session now holds a lease on its rust/<id> directory for as long as it runs: a locked file under rust/<id>/.mbx-leases/, removed when the session ends. A lease file whose lock can be taken belongs to a session that is gone, and collection deletes it.
• On Unix, a session start now also removes a rust/<id> directory when its mbx-rustc link resolves to a binary whose identity no longer matches the directory name (i.e. the binary was upgraded in place), no session has started with it for 7 days, and no running session holds a lease on it. Only directories installed by a lease-aware mbx are judged this way. A directory from an older mbx may belong to a session that is still running and can't hold a lease, so it is still removed only once all its links dangle.
• The lease check also applies to the existing dangling-link collection of rust/ directories, so a session in another container sharing the cache keeps its wrapper even if it runs longer than 7 days.
• Sessions create and lease their directory under a lock (rust/.registrar), and collection judges and removes each rust/ directory under the same lock, so a directory can't be removed between a session creating it and taking its lease.

native/ directories are unchanged. They are keyed by install path on Unix, so upgrades already reuse them.

No configuration changes. Windows shims are still never collected.

Limitations: rust/ directories left by mbx versions before this change aren't removed by the upgrade rule, so directories that have already piled up (like the 206 above) stay until deleted by hand. Leases use advisory file locks. On a shared filesystem where locks don't reach other hosts (some NFS setups), a session on another host is protected only by the 7-day window, as before.

Validation

• New unit test superseded_rustc_shims_go_once_no_session_holds_them: a superseded, unleased wrapper directory is removed. Directories are kept while a session holds a lease, while they were used recently, when they belong to the binary the path currently holds, and when a lease is held even though their links dangle. A crashed session's leftover lease file doesn't protect its directory, and a superseded directory that no session ever leased is kept. After a leased session drops, its lease file is gone and the next collection removes the directory.
• only_shim_directories_nothing_can_use_are_collected now keeps its superseded rustc directory because a session holds a lease on it, not because the collector skipped superseded directories. Both tests fail when the lease check is removed.
• mise run ci passes locally (macOS).

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes shim lifecycle and concurrent collection around persistent RUSTC_WRAPPER paths; mistakes could delete wrappers still in use or leave leaks, though registrar locking and lease checks aim to prevent races.

Overview
Fixes persistent RUSTC_WRAPPER directories under shims/rust/<binary identity>/ that never went away after in-place mbx upgrades, because old symlinks still resolve to the new binary and the prior collector only removed dirs whose links all dangled.

Each CacheSession now holds a ShimLease for its wrapper directory for the whole session (advisory lock file under .mbx-leases/, dropped on session end). install_session_shims creates the per-binary dir and takes the lease under a shared rust/.registrar lock, and remove_stranded_binary_shims uses the same lock when judging rust/ dirs so nothing is deleted between create and lease.

On Unix, collection may also remove a rust/ dir when its mbx-rustc target’s identity no longer matches the directory name (superseded by upgrade), the dir has been unused for the configured age, and no live session holds a lease—including long-running pre-upgrade sessions and shared-cache containers with dangling links. native/ behavior is unchanged; Windows still skips collection.

Tests cover lease retention, stale lease cleanup, superseded removal, and updated stranded-shim scenarios (including mbx-rustc symlinks).

^{Reviewed by Cursor Bugbot for commit f244ef5. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes
  • Improved session cleanup for compiler shim directories.
  • Prevented shim directories from being removed while an active session still depends on them.
  • Added cleanup for superseded or abandoned shims once they are no longer in use.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: d2d3ab3f-4751-4d40-8296-f5aab7abdca8

📥 Commits

Reviewing files that changed from the base of the PR and between 88c0960 and e1645b9.

📒 Files selected for processing (3)

• crates/mbx/src/session.rs
• crates/mbx/src/session/shims.rs
• crates/mbx/src/session_tests.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The change adds per-session leases for binary shim directories. CacheSession retains the lease, and cleanup checks leases and superseded Rust shim identities before removal. Tests cover active, stale, legacy, current, and dangling shim directories.

Changes

Session shim lease lifecycle

Layer / File(s)	Summary
Lease lifecycle and cleanup crates/mbx/src/session/shims.rs	install_session_shims acquires a ShimLease. Lease files track active sessions and remove stale leases. Rust shim cleanup now considers superseded binary identities and active leases.
Session lease ownership crates/mbx/src/session.rs	CacheSession stores the lease obtained during start_with_jobs until the session ends.
Lease cleanup tests crates/mbx/src/session_tests.rs	Tests verify retention and removal for held, abandoned, crashed, legacy, recent, current, and dangling Rust shim directories.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SessionStartup
  participant CacheSession
  participant ShimLease
  participant ShimCollector

  SessionStartup->>ShimLease: acquire Rust shim directory lease
  ShimLease-->>SessionStartup: return lease
  SessionStartup->>CacheSession: store lease
  ShimCollector->>ShimLease: check live leases
  ShimLease-->>ShimCollector: return lease state
  ShimCollector->>ShimCollector: remove eligible superseded directory

Loading

Suggested reviewers: jbellis

Merge Risk: ⚪ Minimal · up to e1645

The lease lifecycle and cleanup behavior are internally consistent, with no concrete merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing rustc wrapper directories left behind by in-place mbx upgrades.

• Fix all pre-merge checks with AI

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; no outstanding correctness, security, or repository-rule violations were identified.

Summary

Adds lifecycle-managed leases for persistent Rust compiler shim directories and serializes lease registration against collection.

• Removes superseded in-place-upgrade shim directories only after the unused window and when no active session holds a lease.
• Preserves pre-lease directories whose potentially active sessions cannot be detected.
• Releases lease locks before deleting lease files, fixing Windows cleanup behavior.
• Extends tests for active, crashed, legacy, superseded, and cross-container session scenarios.

_{Reviews (3) · Last reviewed commit: "fix(cc): keep wrappers of pre-lease sess..."}

[cursor]

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

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 06956bf. Configure here.}

[github-actions]

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
mbx_help	▁▁▁▇▇▇▆▆▆▆▆▆██▇	1,050,006 → 1,046,921	-0.29%	1.39 → 1.37ms	-1.60%
startup_control	███▁▁▁▁▁▁▁▁▁▁▁▇	218,611 → 220,203	+0.73%	0.76 → 0.60ms	-20.36%

No instruction-count regression above 1%.

_{Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.}

_{Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.}

_{e1645b9c3c53 vs 88c09606cdc0 · measured on jdx-perf-v1.}