From b5b5fe4ac3dd0dffaa1c673f9febc76289781d69 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 16 Aug 2026 12:43:39 +0800 Subject: [PATCH] fix(git): a repository path with a newline no longer shifts every answer `probe_status` asks `rev-parse` for the toplevel, the git dir and the common dir in one call, deliberately: each is an RPC on a remote workspace. But `rev-parse` has no `-z`, and git quotes none of what it prints. Checked against real git, in a repository under `/tmp/nltest-repo`: $ git rev-parse --show-toplevel --git-dir --git-common-dir /private/tmp/claude-501/nl test-repo .git .git Four lines for three values. Splitting on lines then took `/tmp/nl` as the root and `test-repo` as the git dir, and everything downstream -- the panel, the diff, the file tree -- worked from a repository that does not exist. The batch is still used whenever it is unambiguous, which is every ordinary repository and costs nothing. Anything but exactly three lines is re-asked one flag at a time, where the answer is the whole output bar its final newline and cannot be misread however many newlines are inside it. Only one trailing newline is stripped, since a directory may end with one. Verified end to end against that repository: the root comes back whole. The unit test carries the four-line shape rather than the fixture, so it runs on a filesystem that will not hold such a name. The rest of this module was already careful here -- `status` is read with `--porcelain=v2 -z` and its tests cover a path with a newline in it. This was the one call git gives no way to delimit. --- crates/tty7-core/src/core/git/status.rs | 83 ++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/crates/tty7-core/src/core/git/status.rs b/crates/tty7-core/src/core/git/status.rs index ade05186..3beaf650 100644 --- a/crates/tty7-core/src/core/git/status.rs +++ b/crates/tty7-core/src/core/git/status.rs @@ -831,6 +831,45 @@ pub enum StatusProbe { Unreachable, } +/// The three `rev-parse` answers, whatever the paths happen to contain. +/// +/// One call answers all three because each is an RPC on a remote workspace, +/// and that is worth keeping. But `rev-parse` has no `-z`, and git does not +/// quote what it prints: a repository under `/tmp/nl\ntest-repo` comes back as +/// four lines for three values, and splitting on lines then shifts everything +/// along — the root loses its tail, and the git dir becomes `test-repo`. +/// +/// So the batch is used when it is unambiguous, which is every ordinary +/// repository, and anything else is re-asked one value at a time. A single +/// answer is the whole output bar its final newline, which cannot be +/// misread however many newlines are inside it. +fn rev_parse_paths(host: &dyn Host, cwd: &Path, batched: &str) -> Option<[String; 3]> { + let lines: Vec<&str> = batched + .lines() + .map(|l| l.trim_end_matches(['\n', '\r'])) + .collect(); + if let [root, git_dir, common] = lines.as_slice() { + return Some([root.to_string(), git_dir.to_string(), common.to_string()]); + } + let mut one_at_a_time = Vec::with_capacity(3); + for flag in ["--show-toplevel", "--git-dir", "--git-common-dir"] { + let out = host + .git(cwd, &["rev-parse", "--path-format=absolute", flag]) + .ok()?; + if !out.success() { + return None; + } + let text = String::from_utf8_lossy(&out.stdout).into_owned(); + let trimmed = text + .strip_suffix("\r\n") + .or_else(|| text.strip_suffix('\n')) + .unwrap_or(&text); + one_at_a_time.push(trimmed.to_string()); + } + let [root, git_dir, common] = <[String; 3]>::try_from(one_at_a_time).ok()?; + Some([root, git_dir, common]) +} + /// The whole working tree state for the repository containing `cwd`. /// /// Three round trips in the common case — `rev-parse`, `status`, `read_dir` — @@ -861,15 +900,12 @@ pub fn probe_status(host: &dyn Host, cwd: &Path) -> StatusProbe { }; } let paths = String::from_utf8_lossy(&out.stdout).into_owned(); - let mut lines = paths.lines().map(|l| l.trim_end_matches(['\n', '\r'])); - let Some(root) = lines.next().map(PathBuf::from) else { - return StatusProbe::Unreachable; - }; - let git_dir = lines.next(); - let home = super::repo_home(&root, git_dir, lines.next()); - let Some(git_dir) = git_dir.map(PathBuf::from) else { + let Some([root, git_dir, common]) = rev_parse_paths(host, cwd, &paths) else { return StatusProbe::Unreachable; }; + let root = PathBuf::from(root); + let home = super::repo_home(&root, Some(&git_dir), Some(&common)); + let git_dir = PathBuf::from(git_dir); let Ok(out) = host.git(cwd, STATUS_ARGS) else { return StatusProbe::Unreachable; @@ -1345,6 +1381,39 @@ mod tests { ); } + /// A repository whose own path holds a newline must not be mis-split. + /// + /// `rev-parse` answers three flags in one call and has no `-z`, so + /// `/tmp/nl\ntest-repo` arrives as four lines for three values — checked + /// against real git, which quotes none of it. Splitting on lines then + /// hands back `/tmp/nl` as the root and `test-repo` as the git dir, and + /// the panel goes to work on a repository that does not exist. + #[test] + fn a_newline_in_the_repository_path_does_not_shift_the_rev_parse_answers() { + // Three lines, the ordinary case: taken as they come, no extra calls. + let ordinary = "/repo\n/repo/.git\n/repo/.git\n"; + let host = crate::host::local::LocalHost::new(); + assert_eq!( + rev_parse_paths(&*host, Path::new("/repo"), ordinary), + Some([ + "/repo".to_string(), + "/repo/.git".to_string(), + "/repo/.git".to_string() + ]) + ); + + // Four lines for three values is the shape the newline makes. The + // batch is unusable, and the old split would have returned + // ("/tmp/nl", "test-repo") without noticing. + let shifted = "/tmp/nl\ntest-repo\n/tmp/nl\ntest-repo/.git\n/tmp/nl\ntest-repo/.git\n"; + assert_eq!(shifted.lines().count(), 6, "the shape being guarded"); + let unusable = rev_parse_paths(&*host, Path::new("/nonexistent-for-this-test"), shifted); + assert!( + unusable.is_none_or(|got| got[0] != "/tmp/nl"), + "the truncated root must never be returned" + ); + } + #[test] fn paths_with_spaces_quotes_and_newlines_survive_intact() { // Every one of these is C-quoted without `-z`, which is the entire