Repository navigation
fix(nix): dedupe the dev shell's PATH so mbx stays under E2BIG - #7594
Conversation
Quest-Claim: 823a51bc1a53f93e
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
kixelated
changed the title
…'s E2BIG limit Each nested dev shell prepended another ~10 KB copy of its store paths. mbx 1.25-1.29 copies PATH into MBX_SESSION_PATH and MBX_SESSION_RESTORE at ~3.6x its size, so four nested shells pushed both past Linux's 131,072-byte per-string limit and every Cargo launch failed with E2BIG. Keeping each directory's first entry leaves lookups unchanged and holds PATH at ~10.8 KB. The repo-side goal is met and the maintainer chose not to report upstream, so the quest is deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
Finishing Touches
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: cb7b0e5
No actionable bugs found. Deduplicating PATH at the shared dev-shell boundary is a proportionate repo-side fix for repeated shell prefixes, and doing it after the mbx shim insertion preserves that shim's priority. It is simpler than changing each caller or bypassing mbx. The documented removal of a trailing empty PATH entry is a deliberate lookup change; this does not solve arbitrarily large, unique incoming PATHs or the upstream metadata expansion itself.
Verification: static review of the three-file diff, shell context, recent flake history, and the affected recipe test. No published prior review was present. I did not run Nix, tests, or the E2BIG reproduction; the PR's reported measurements and gate results were not independently reproduced.
(Written by OpenAI)
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- PATH dedup in the dev shell —
flake.nix'sshellHooknow drops later duplicate PATH entries after prepending the mbx Cargo shim, keeping each directory's first occurrence (awk -v RS=: '!seen[$0]++'). PATH lookup resolves against the first match, so removing later exact duplicates cannot change what any command resolves to. - Quest cleanup — deletes
quest/m0/ci/mbx-arg-max.mdand its line inquest/m0/ci/README.md; the test it tracked now passes and the maintainer declined the upstream mbx report, both recorded in the PR body and commit message.
I verified the awk dedup keeps first occurrences, preserves leading/middle empty entries, drops only an implicit-cwd trailing entry, and that the same !seen[...]++ idiom already runs in this repo (test/smoke/relays.sh:125, sh/rs/select.sh), so awk is a settled dev-shell dependency. No behavioral or portability concerns.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Problem
just cifailed locally inRecipeTest.test_unexecuted_bench_is_not_compiled(sh/rs/select.test.py) and elsewhere: mbx could not launch Cargo,Argument list too long (os error 7)(E2BIG).mbx 1.25.0 through 1.29.1 copies PATH into
MBX_SESSION_PATHandMBX_SESSION_RESTORE, serialized as byte arrays at ~3.6× PATH's size (record_overlay). Linux rejects any single env string over 131,072 bytes (MAX_ARG_STRLEN), so a PATH over ~36 KB breaks every Cargo launch through mbx.The repo-side cause is why PATH got that big. Each nested dev shell (direnv or an agent harness, then
nix develop, then anothernix developinside a gate) prepends another copy of the shell's ~160 store paths, about 10.4 KB per level. Four levels reach 42 KB. Only PATH andXDG_DATA_DIRSaccumulate across nesting; the other large Nix variables are reset each time.Approach
The dev shell's
shellHook(flake.nix) now keeps only the first occurrence of each PATH directory, after it puts the mbx Cargo shim in front. Command lookup takes the first matching directory, so dropping later duplicates can't change what any command resolves to. The fix lives in the shell that CI (nix develop --command …), direnv, and local gates all enter, so it covers CI and local runs alike. No test, assertion, or mbx path changes.Measurements
PATH by nesting depth (depth 1 is this session's own dev shell;
nix developfrom a clean environment gives 10,464 bytes):At depth 4,
strace -f -e trace=execveoncargo buildof a dependency-free crate through the mbx 1.25.0 shim (largest entry, including name,=, and NUL):At depth 4, the original test fails before the fix with the reported
mbx[error]: failed to run …/cargo: Argument list too long (os error 7). After the fix it passes.Decisions
Choices made without asking, for review:
quest/m0/ci/mbx-arg-max.md. Its goal (the test passes injust ci) is met. Its rewritten goal (pin an upstream-fixed mbx) depended on an upstream report the maintainer declined. The remaining exposure needs a PATH over ~26 KB from outside the dev shell. The deduped shell PATH (10.8 KB) leaves mbx about 3.4× headroom.XDG_DATA_DIRS. mbx doesn't copyXDG_DATA_DIRS. It grows ~7 KB per level and would need ~18 levels to reach the per-string limit.awkwithRS=:emits no final empty record. An implicit current-directory entry is a hazard anyway.Checks
quest gate nix develop --command just ci: exit 1, on two failures this change doesn't cause. Cargo launched through mbx with no E2BIG (obs_compile:mbx[cache]: 429 hits, 21 misses). Passed: obs_compile, js check, quest, obs, shell, large_files, toml, nix, justfile, markdown, gh check, gh test.check flake: thegithub-runnerVM image build fails withunshare: write failed /proc/self/uid_map: Operation not permitted. This session's host restricts rootless user namespaces (kernel.apparmor_restrict_unprivileged_userns = 1), andCI_STRICT=1turns the userns probe's skip into a failure (sh/dispatch.sh:331). Thegithub-runner-vmandgithub-runner-vm-imagecheck derivations are byte-identical toorigin/main's, so main fails the same way here. flake.nix PRs still land through the host gate, e.g. moq-dev/moq#7217.test js:@moq/neta peer close drains the overtaken group over moq-lite-02(js/net/src/lite/connection.test.ts:394) failed under host load ~34. Rerun alone,bun test --only-failures src/lite/connection.test.tspassed 57/57. PATH dedupe can't affect it.RecipeTestdoesn't run in this gate. It ran at depth 4 instead (next item).python3 sh/rs/select.test.py RecipeTest.test_unexecuted_bench_is_not_compiled): fails before, passes after.quest check --base origin/main: 629 documents ok.nix fmt -- --ci flake.nix: no changes.Review
quest review --base origin/main, round 1, session01a19aaa-2b48-70b0-b2ca-6827e6e947f5).21f8dbb70d16a5723d090227a2a487af13502a26.cb7b0e5cfaadds only a merge oforigin/mainwith no conflicts, which the guide exempts from re-review.Follow-ups
Unblocks the E2BIG failures in drafts moq-dev/moq#7593, moq-dev/moq#7596, moq-dev/moq#7599, moq-dev/moq#7600, moq-dev/moq#7602, and moq-dev/moq#7606 once they merge this base.
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)