fix(git): tolerate noisy login-shell stdout in git binary probe#777
Open
hamman wants to merge 13 commits into
Open
fix(git): tolerate noisy login-shell stdout in git binary probe#777hamman wants to merge 13 commits into
hamman wants to merge 13 commits into
Conversation
The macOS git-binary detection probe spawns the user's login shell with
`-lc 'printf %s\n%s ...'` and parses the resulting stdout positionally:
line 1 is treated as the path to git and line 2 as PATH. Login shells
routinely print to stdout during startup -- ssh-agent's "Agent pid
12345" line, oh-my-zsh banners, plugin status lines -- which shifts the
parse so that `git_path` becomes "Agent pid ..." (filtered to None by
the `.exists()` guard) and the PATH slot is filled with what was meant
to be the git path. The cached GitLaunchConfig ends up with
program="git" and PATH=/usr/bin/git, so every subsequent
`Command::new("git")` lookup fails with "No such file or directory
(os error 2)" because the PATH list points at a single binary path
treated as a directory. Every git operation in Tolaria then fails
until the process is restarted, and the same broken probe runs again
on the next launch.
Replace positional parsing with sentinel-prefixed lines:
__TOLARIA_GIT_PROBE_PATH__=<git path>
__TOLARIA_GIT_PROBE_ENV__=<PATH>
The parser scans for these prefixes anywhere in stdout, so unrelated
output from the user's startup files is ignored regardless of where it
appears. Extract `parse_shell_git_config` as a pure function so the
parsing rules are unit-testable without spawning a real shell, and
move the `.exists()` filesystem check to the caller so the parser
stays side-effect-free. The shell probe script is built at runtime
from the sentinel constants so the prefixes have a single source of
truth.
Adds four unit tests covering: clean probe output, noisy login-shell
preamble (the regression), missing sentinels, and empty sentinel
values.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Bug
In some environments, every git operation in Tolaria fails with:
The error reproduces on every launch and persists across restarts, even though
gitworks fine from the user's terminal in the same vault. Symptoms: connecting a remote fails, commit/pull/push silently fail, status updates don't refresh.Root cause
src-tauri/src/git/mod.rs::shell_git_config_from_shellprobes the user's login shell with:/bin/zsh -lc "printf '%s\n%s' \"\$(command -v git 2>/dev/null || true)\" \"\$PATH\""…and parses the stdout positionally — line 1 is treated as the path to git, line 2 as
PATH.If the user's
~/.zprofileor~/.zshrcwrites anything to stdout during startup —eval \$(ssh-agent)printingAgent pid 12345, oh-my-zsh banners, plugin status lines, etc. — the parse shifts:Agent pid 98189(filtered toNoneby.exists())PATHenv var/usr/bin/git(the actual git path)The cached
GitLaunchConfigends up withprogram = "git"andPATH = "/usr/bin/git". SubsequentCommand::new("git")resolves git against that bogusPATH(treating the file path as a directory list), the kernel returnsENOENT, and the io::Error bubbles up asos error 2from every git operation in the app. Becausegit_launch_configisOnceLock-cached for the process lifetime, every call in the session is poisoned, and the deterministic shell startup means the same poisoning happens on every launch.Fix
Replace positional parsing with sentinel-prefixed lines. The probe now emits:
…and the parser scans for those prefixes anywhere in stdout, so unrelated lines emitted by the user's startup files are ignored regardless of where they appear.
Other small refactors carried by this change:
parse_shell_git_configis extracted as a pure&str -> Option<ShellGitConfig>function, so the parsing rules are unit-testable without spawning a real shell..exists()filesystem check moves to the caller, keeping the parser side-effect-free.Tests
Adds 4 unit tests in
src-tauri/src/git/mod.rs:test_parse_shell_git_config_extracts_sentinel_lines_when_clean— sanity check on clean probe output.test_parse_shell_git_config_ignores_login_shell_noise_before_sentinels— direct regression test for this bug, fed real-world noisy preamble (Agent pid 98189, oh-my-zsh banner) before the sentinel lines.test_parse_shell_git_config_returns_none_when_sentinels_missing— graceful fallback when a probe fails to emit sentinels at all.test_parse_shell_git_config_treats_empty_sentinel_value_as_missing— empty value (e.g.command -v gitreturned nothing) maps toNonerather thanSome("").All 966 Rust lib tests pass.
cargo fmt --checkandcargo clippy --all-targets -- -D warningsclean. Frontend tests, frontend coverage, Rust coverage, and Playwright smoke suite all pass via the project's pre-push gate.Manual verification
pnpm tauri devwith my own ssh-agent-printing zsh setup — every remote operation failed withos error 2.Scope
Single file changed:
src-tauri/src/git/mod.rs. No behavior change for users whose login shells produce clean stdout (the new sentinels-anywhere parser also handles the previously-tested clean case).