Conversation
brush tracks its working directory purely as internal shell state (mirrored into $PWD) and never calls chdir(2) on the shell process itself; children are spawned with the correct directory explicitly via Command::current_dir, so the shell's own commands and prompt are unaffected either way. But every external tool that inspects a process's cwd out-of-band (terminal multiplexers, session managers, debuggers, ps/lsof-style tooling) reads /proc/<pid>/cwd (or platform equivalents) directly, and sees the directory the shell was launched in, forever, no matter how many times it cd's. Add an opt-in CreateOptions/RuntimeOptions flag, sync_process_cwd, defaulting to false. When enabled, set_working_dir best-effort syncs the real process cwd via std::env::set_current_dir after a successful cd/pushd/popd. The flag is gated to the root shell only (depth() == 0 / !is_subshell()). Subshells, command substitutions, background jobs, and function calls run as concurrent in-process clones of Shell sharing one OS process (see openfiles.rs); letting any of them mutate the process-global cwd would race with, and corrupt, every other clone's own view of "its" directory. Verified this stays correct: a command substitution that cd's internally no longer leaks into, or gets clobbered by, the real process cwd, and the root shell's own cwd is unaffected by a subshell's cd, matching bash. Defaults to off at the brush-core library level because brush-core may be embedded to run multiple independent root shells within a single host process (e.g. a multi-threaded test binary), where each enabling this would race to own the same process-global cwd. The brush-shell binary enables it explicitly, since a CLI invocation is always the sole owner of its process. Fixes reubeno#1303. Testing: - New isolated integration test (brush-core/tests/process_cwd_sync_tests.rs) covering: option off leaves the real cwd untouched, option on syncs it for the root shell, and a subshell never touches it even with the option on. - cargo test --workspace --lib --bins: all pass (pre-existing, unrelated fuzz_arithmetic failure confirmed present on main before this change too). - cargo test -p brush-core --test kill_on_drop_tests --test process_cwd_sync_tests -p brush-shell --lib: all pass. - Compat suite filtered to cd/pwd/pushd/popd/dirs against the real bash oracle: 20 passed, 2 pre-existing known-fail, 0 regressions. - Manual repro against the built binary (bypassing reedline's cursor- position query via --input-backend minimal): cd now moves the real process cwd (verified via /proc/<pid>/cwd from outside the process); a command substitution's internal cd continues to leave the real process cwd, and the root shell's own $PWD, untouched. Assisted-by: Claude (Anthropic), via an omp coding-agent session
Wire aarch64-unknown-linux-gnu to aarch64-linux-gnu-gcc so release builds of brush-shell can be produced on x86_64 hosts for void.
Cover bind -x host-command delivery on reedline >= 0.48 and ensure a single unanswered DSR cursor query is retried instead of killing the shell.
Map Signal::HostCommand from reedline >= 0.48 to bound-command reads and retry cursor-position DSR timeouts a bounded number of times instead of tearing down the interactive shell on a single late terminal response.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
cd/pushd/popdnever actuallychdir(2)the shell process — they only update brush's internal working-directory state (mirrored into$PWD). This makes the shell fully self-consistent for its own use (builtins, prompt, and spawned children all get the right directory), but leaves the real process cwd frozen at wherever the shell was launched, forever, invisible to every external tool that reads/proc/<pid>/cwd(terminal multiplexers, session managers, debuggers,ps/lsof-based tooling).Full repro and root-cause analysis: #1303.
Fix
New opt-in
sync_process_cwdoption (CreateOptions→RuntimeOptions), off by default. When on,set_working_dirbest-effort callsstd::env::set_current_dirafter a successfulcd/pushd/popd— but only for the root shell (!is_subshell(), i.e.depth() == 0).That gate matters: subshells, command substitutions, background jobs, and function calls all run as concurrent in-process clones of
Shellsharing one OS process (per the existing comment inopenfiles.rs), not viafork(2). If any of them synced the process-global cwd, they'd race with and corrupt every other clone's own view of "its" directory — e.g.echo $(cd /a && pwd) & echo $(cd /b && pwd)running concurrently. Verified this stays correct: a command substitution's internalcdnever leaks into, or gets clobbered by, the real process cwd, and the root shell's own directory is unaffected by its subshell'scd— matching bash.Defaults to
falseat thebrush-corelibrary level because it can be embedded to run multiple independent root shells within a single host process (e.g. a multi-threaded test binary —brush-shell's owncompletion_tests.rsconstructs many shells and callsset_working_diragainst temp dirs; several run concurrently undercargo test, and each syncing the process-global cwd would race).brush-shell's binary enables it explicitly, since a CLI invocation always solely owns its process.Testing
brush-core/tests/process_cwd_sync_tests.rs— deliberately its own file/process (not sharing a test binary with anything else that touchesstd::env::set_current_dir), single test function (all assertions run sequentially, avoiding intra-process races on process-global state). Covers: option off leaves the real cwd untouched; option on syncs it for the root shell; a subshell never touches it even with the option on.cargo test --workspace --lib --bins: all pass. (One pre-existing, unrelatedfuzz_arithmeticfailure — confirmed present onmainbefore this change viagit stash, not a regression.)cargo test -p brush-core --test kill_on_drop_tests --test process_cwd_sync_tests -p brush-shell --lib: all pass.cd/pwd/pushd/popd/dirsagainst the real bash oracle: 20 passed, 2 pre-existing known-fail, 0 regressions.brushbinary (using--input-backend minimalto sidestepreedline's terminal cursor-position query, which a bare test pty doesn't answer):cdnow moves the real process cwd, verified via/proc/<pid>/cwdread from outside the process;$(cd /tmp && pwd)still correctly leaves the root shell's real cwd and its own$PWDuntouched.Disclosure
This PR was significantly assisted by Claude (Anthropic) via an agentic coding session — investigation, implementation, and testing were AI-driven under my direction and review, per the project's AI Policy. This is also my first Rust contribution anywhere, so extra scrutiny very welcome — opening as a draft per the contributing guide's suggested first-timer flow.
Fixes #1303.