fix(git): keep a repository root that ends in a carriage return whole

`strip_output_terminator` stripped `\r?\n$`, and a carriage return is as
legal a byte in a POSIX directory name as a newline is. For a repository
rooted at a directory named `trailing\r`, `rev-parse --show-toplevel`
prints the path's own `0d` and then its own `0a` terminator, and a strip
tolerant of an optional preceding carriage return cannot tell those two
bytes apart --- so it took both. The root resolved as `trailing`, and
every command afterwards ran with a `-C` and a cwd naming a directory
that does not exist: the same defect the previous commit at this call
site fixed, one byte over.

Exactly one trailing `\n` is now removed, by an explicit last-byte test
rather than an anchored pattern. Both of this function's bugs lived in a
pattern, and the third answer to the same question should not be a
cleverer one.

`-z` was CHECKED against the installed git rather than assumed, and must
NOT be used. `git rev-parse` has no `-z` option at all on git 2.55: it
is absent from the manual, `--parseopt -z` errors with "unknown switch",
and in ordinary mode `rev-parse` treats `-z` as an unrecognized flag
argument and echoes a literal `-z\n` onto stdout AHEAD of the toplevel
--- exit code 0, corrupted output, silent. `--show-toplevel` applies no
C quoting either, not even under `core.quotePath=true`. There is
therefore no unambiguous output representation to prefer over removing
the one byte git appended.

`first_line` is untouched, for the reason the previous commit recorded:
its three callers all feed the single-line status band, where taking the
first line is right.

Witnessed by `g6_14d` end to end, not at the parser: the fixture really
creates directories named `trailing\r` and `nl\nand-trailing\r`, the
real `git` resolves them, and the assertion is on the cwd of the status
spawn the module actually made, plus real rows in the panel and a RET
that opens the file the row names. The second case sends both hazards in
together because a root may hold both and neither fix may mask the
other. `g6_14c` now reaches that chain through the shared
`assert_root_resolves_whole` rather than keeping a second copy of it,
and the helper binds `pmacs.project.set_search_boundary` to the fixture
through `open_panel` --- R8's lesson, and a root fixture is exactly that
hazard's shape.

Mutation-verified: restoring `\r?\n$` fails `g6_14d` at `<tmp>/trailing`
against `<tmp>/trailing\r` while `g6_14c` still passes, which is exactly
the byte that separates the two fixes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai

builtin/runtime/git.lua

@@ -314,11 +314,33 @@ end
 --- has three other callers, all of them status-band text, and folding
 --- the two together would fix this one and break those.
 ---
---- Exactly one trailing newline is stripped, with an optional preceding
---- carriage return, because that is what git emits. A second newline
---- would be output, not a terminator.
+--- Exactly ONE trailing `\n` is stripped, and NOTHING may ride along
+--- with it --- not a carriage return, not a second newline. A carriage
+--- return is as legal a POSIX path byte as a newline is, so a repository
+--- rooted at `/tmp/a\r` makes git print `/tmp/a` `0d` `0a`: the path's
+--- own CR, then git's LF terminator. A strip tolerant of `\r?\n$` cannot
+--- tell those two bytes apart and takes both, resolving the root as
+--- `/tmp/a` --- the same defect this function was written to fix, one
+--- byte over. A second newline is output, not a terminator, for the same
+--- reason.
+---
+--- There is no unambiguous output representation to prefer instead,
+--- which was CHECKED against git 2.55 rather than assumed: `-z` is not
+--- an option of `git rev-parse` at all. It is absent from the manual,
+--- `--parseopt -z` errors with "unknown switch", and in ordinary mode
+--- `rev-parse` treats `-z` as an unrecognized FLAG ARGUMENT and echoes a
+--- literal `-z\n` onto stdout AHEAD of the toplevel --- so asking for it
+--- would corrupt the very output it was meant to disambiguate, silently
+--- and with exit code 0. `--show-toplevel` applies no C quoting either,
+--- not even under `core.quotePath=true`. Removing the one byte git
+--- appended is therefore the whole of the correct answer.
+---
+--- Written as an explicit last-byte test rather than a pattern: an
+--- anchored Lua pattern is where both of this function's bugs lived.
 local function strip_output_terminator(text)
-  return ((text or ""):gsub("\r?\n$", ""))
+  text = text or ""
+  if text:sub(-1) == "\n" then return text:sub(1, -2) end
+  return text
 end
 
 --- A one-line description of why a git invocation failed.
@@ -815,9 +837,11 @@ function pmacs.git._deliver_root(request, res)
     end
     return
   end
-  -- The WHOLE output, minus its terminator --- never the first line. A
-  -- repository root may contain a newline, and truncating one here would
-  -- point every command that follows at a directory that does not exist.
+  -- The WHOLE output, minus its terminator --- never the first line, and
+  -- never a byte more than git appended. A repository root may contain a
+  -- newline and may END in a carriage return, and losing either here
+  -- would point every command that follows at a directory that does not
+  -- exist.
   local root = strip_output_terminator(res.stdout)
   if root == "" then
     pmacs.editor.set_status("git: rev-parse returned no worktree root")

tests/git_status_stage1_acceptance.rs

@@ -1654,32 +1654,27 @@ fn g6_14_the_root_rule_works_where_project_kind_is_not_git() {
     );
 }
 
-/// A repository root containing a **newline** resolves **whole**, and
-/// the status command really runs there.
-///
-/// A newline is a legal byte in a POSIX path — the fixture below builds
-/// one and `git rev-parse --show-toplevel` prints it, terminator and
-/// all — so parsing that output with a first-line match truncates
-/// `/tmp/…/nl\nroot` to `/tmp/…/nl`, and every command this module runs
-/// afterwards gets a `-C` and a cwd naming a directory that does not
-/// exist. The right answer is to strip git's final terminator and
-/// nothing else.
+/// A repository rooted at a directory literally named `leaf` resolves
+/// **whole**, and the status command really runs there.
 ///
 /// End to end, not at the parser: the directory really is created, the
 /// real `git` really resolves it, and the assertion is on the cwd of the
 /// spawn the module actually made. `_last_spawn` is the status
 /// invocation here, since `rev-parse` runs first and carries no cwd of
-/// its own.
+/// its own. A root that lost a byte would still SPAWN — with a `-C` and
+/// a cwd naming a directory that does not exist — so the panel is
+/// checked for a failure row as well as for real ones.
 ///
-/// It rides beside the one-line-status rule rather than replacing it:
-/// the helper this uses is deliberately **separate** from `first_line`,
-/// whose other three callers all feed the single-line status band and
-/// would be corrupted by a multi-line message.
-#[test]
-fn g6_14c_a_root_containing_a_newline_is_not_truncated() {
+/// `open_panel` binds `pmacs.project.set_search_boundary` to the fixture
+/// (R8's lesson), which matters most here: these leaf names are exactly
+/// the shape that makes a detection walk out of the tempdir hard to
+/// read when it goes wrong.
+fn assert_root_resolves_whole(leaf: &str) {
     let (_dir, base) = tempdir();
-    let root = base.join("nl\nroot");
-    std::fs::create_dir_all(&root).expect("a newline is a legal POSIX path byte");
+    let root = base.join(leaf);
+    std::fs::create_dir_all(&root).unwrap_or_else(|e| {
+        panic!("a root named {leaf:?} must be creatable — every byte in it is a legal POSIX path byte: {e}")
+    });
     mixed_repo(&root);
 
     let mut s = editor();
@@ -1689,7 +1684,7 @@ fn g6_14c_a_root_containing_a_newline_is_not_truncated() {
     assert_eq!(
         cwd,
         root.display().to_string(),
-        "the resolved root must be the WHOLE path, newline included"
+        "the resolved root must be the WHOLE path, every byte of {leaf:?} included"
     );
 
     let text = panel_text(&s);
@@ -1714,6 +1709,47 @@ fn g6_14c_a_root_containing_a_newline_is_not_truncated() {
     );
 }
 
+/// A repository root containing a **newline** resolves **whole**, and
+/// the status command really runs there.
+///
+/// A newline is a legal byte in a POSIX path — the fixture builds one
+/// and `git rev-parse --show-toplevel` prints it, terminator and all —
+/// so parsing that output with a first-line match truncates
+/// `/tmp/…/nl\nroot` to `/tmp/…/nl`, and every command this module runs
+/// afterwards gets a `-C` and a cwd naming a directory that does not
+/// exist. The right answer is to strip git's final terminator and
+/// nothing else.
+///
+/// It rides beside the one-line-status rule rather than replacing it:
+/// the helper this uses is deliberately **separate** from `first_line`,
+/// whose other three callers all feed the single-line status band and
+/// would be corrupted by a multi-line message.
+#[test]
+fn g6_14c_a_root_containing_a_newline_is_not_truncated() {
+    assert_root_resolves_whole("nl\nroot");
+}
+
+/// A repository root ending in a **carriage return** resolves whole too
+/// — the byte the newline fix's own strip still ate.
+///
+/// `\r` is as legal in a POSIX directory name as `\n` is, and it is the
+/// byte that makes `\r?\n$` ambiguous: for a root named `trailing\r`,
+/// git prints `…/trailing` `0d` `0a`, where the `0d` is the PATH and
+/// only the `0a` is the terminator. A strip tolerant of an optional
+/// preceding carriage return cannot tell those apart and takes both,
+/// resolving the root as `…/trailing` — a directory that does not
+/// exist. Only git's final `\n` may be removed.
+///
+/// The second case sends the two hazards in together, because a root may
+/// hold both and neither fix may mask the other: an embedded newline
+/// (which forbids a first-line read) ahead of a trailing carriage return
+/// (which forbids an over-eager terminator strip).
+#[test]
+fn g6_14d_a_root_ending_in_a_carriage_return_is_not_truncated() {
+    assert_root_resolves_whole("trailing\r");
+    assert_root_resolves_whole("nl\nand-trailing\r");
+}
+
 /// A directory outside any repository reports it, rather than opening
 /// an empty panel or saying nothing.
 #[test]