[Audit] Git integration & status buffer — 7 verified findings (7 medium, 0 low)

[cuttlefisch]

Batched findings for the git capability slice, from the pre-v0.15 codebase audit (epic #592).

Traced end-to-end across every crate the capability touches, then independently re-verified by a
reviewer briefed to refute each claim. Only survivors appear here, at the verifier's corrected severity.

7 findings — 7 medium, 0 low. Tick off individually; split any one out
if it turns out to need real design work.

---

1. GUI blame gutter is drawn at the wrong column — it paints over the source code at the window's LEFT edge instead of the right margin

bug · medium

Impact. SPC g b in the GUI (the primary backend per CLAUDE.md) blanks and overwrites the first 30 columns of every blamed line, hiding the code the user is trying to blame, and in a split it corrupts a neighbouring window's content. gutter_width = 30 and the take(10) author truncation (popup_render.rs:1424) are also silent magic numbers with no OptionRegistry entry and no written rationale.

Evidence

crates/gui/src/popup_render.rs:1417-1431:

    // Render blame annotations in the right margin.
    let gutter_width = 30;
    for row in 0..win_height {
        ...
            // Draw at the right side of the window.
            let col = win_col_offset.saturating_sub(gutter_width + 1);
            canvas.draw_rect_fill(win_row_offset + row, col, gutter_width, 1, bg);
            canvas.draw_text_at(win_row_offset + row, col, &display, blame_fg);

win_col_offset is the window's LEFT edge, not its right: crates/gui/src/lib.rs:795-801 sets let (win_col_off, win_row_off, win_w, win_h) = ... (r.x as usize, r.y as usize, r.width as usize, r.height as usize) from layout_rects, and lib.rs:902-911 passes win_col_off as the win_col_offset argument. The window WIDTH is never passed to render_blame_gutter at all (its params are canvas, editor, win_row_offset, win_col_offset, win_height, visible_start_line), so the function cannot compute a right edge.
Consequence by arithmetic: single full-width window (r.x = 0) → 0.saturating_sub(31) = 0, so a 30-column opaque rect + blame text is drawn starting at column 0, on top of the code. A right-hand vertical split at r.x = 60 → col 29, i.e. the blame text for the RIGHT window is painted inside the LEFT window.

Verification

Code reads as quoted. crates/gui/src/popup_render.rs:1417-1431: let gutter_width = 30; ... // Draw at the right side of the window. / let col = win_col_offset.saturating_sub(gutter_width + 1); then canvas.draw_rect_fill(win_row_offset + row, col, gutter_width, 1, bg) and draw_text_at(..., col, ...). The parameter really is the LEFT edge: crates/gui/src/lib.rs:795-801 destructures focused_rect as (r.x as usize, r.y as usize, r.width as usize, r.height as usize) into (win_col_off, win_row_off, win_w, win_h), and lib.rs:902-911 passes win_row_off, win_col_off, win_h, visible_start — win_w is never passed, and render_blame_gutter's signature (popup_render.rs:1401-1408) has no width parameter, so it structurally cannot compute a right edge. Arithmetic confirmed: r.x=0 -> 0usize.saturating_sub(31) = 0, so a 30-col opaque fill + text lands at column 0 over the code; r.x=60 -> col 29, inside the left window. gutter_width = 30 and chars().take(10) (popup_render.rs:1424) are indeed unregistered magic numbers — grep git crates/core/src/options.rs returns only two unrelated substring hits (a .github/copilot-instructions.md path in a doc string at options.rs:526 and the tool-category list at :545), so the zero-git-options claim is correct. Downgraded to medium: this is a non-destructive, toggleable read-only overlay (SPC g b toggles it off, git_ops.rs:1003-1009) — obscured text, not wrong or lost data.

Scope correction from verification: Accurate but GUI-only, and cosmetic rather than corrupting: the overlay draws over code / into a neighbouring split and is dismissed by pressing SPC g b again. Worth adding to the finding: grep -rn blame crates/renderer/src/ returns nothing at all — the TUI renders no blame overlay whatsoever, so SPC g b in the terminal backend runs git blame, sets blame_overlay, reports "Blame overlay active" and displays nothing. That is the larger principle-#8 gap here.

---

2. magit status parser mangles paths containing spaces and shows renames under their OLD path (porcelain=v2 split_whitespace)

bug · medium

Impact. Silent wrong-file operations in a destructive surface: the wrong path is displayed, and s/u/x act on that wrong path. The failure is a toast, not an error dialog, so a user staging a batch can easily miss it. Any repo with a space in a filename (assets, docs, vendored fixtures) or any renamed file hits this.

Evidence

crates/core/src/editor/git_ops.rs:177-190:

} else if line.starts_with("1 ") || line.starts_with("2 ") {
    let parts: Vec<&str> = line.split_whitespace().collect();
    if parts.len() >= 9 {
        let staging = parts[1];
        let path = parts[parts.len() - 1].to_string();

Verified against real git status --porcelain=v2 output in a throwaway repo:

1 .M N... 100644 100644 100644 <h> <h> my file.txt
2 R. N... 100644 100644 100644 <h> <h> R100 new.txt\told.txt

• Space case: split_whitespace yields 10 tokens, parts[9] = "file.txt". The buffer renders ▸ M file.txt; s then runs git add file.txt → fatal: pathspec 'file.txt' did not match any files (surfaced only as a transient status-line toast at git_ops.rs:985).
• Rename case (2 lines): the rename record is <path>\t<origPath>; TAB is whitespace, so parts.last() is the original path. A staged rename is listed as old.txt; u (git-unstage) runs git reset HEAD -- old.txt, unstaging only the deletion half and leaving a half-staged rename; Enter (git-status-open, dispatch/git.rs:63-79) tries to open old.txt, which no longer exists.
  The sibling parser in crates/core/src/file_tree.rs:196-200 DOES handle renames (if let Some(arrow) = path_str.find(" -> ")), so the magit buffer is the odd one out.

Verification

Code reads as quoted (crates/core/src/editor/git_ops.rs:177-182: } else if line.starts_with("1 ") || line.starts_with("2 ") { let parts: Vec<&str> = line.split_whitespace().collect(); if parts.len() >= 9 { let staging = parts[1]; let path = parts[parts.len() - 1].to_string();). I reproduced the porcelain v2 output in a throwaway repo and it matches the claim byte for byte: 1 .M N... 100644 100644 100644 <h> <h> my file.txt (10 whitespace tokens, parts.last() = file.txt) and 2 R. N... 100644 100644 100644 <h> <h> R100 new.txt\told.txt (TAB-separated <path><sep><origPath>, so parts.last() = old.txt, the ORIGINAL path). The sibling parser really does handle renames — crates/core/src/file_tree.rs:196-200 let path_str = if let Some(arrow) = path_str.find(" -> ") { &path_str[arrow + 4..] } else { path_str }; — so the magit buffer is the odd one out, as claimed. Downgraded to medium on consequence, not on facts: see correction.

Scope correction from verification: The space case is a loud failure, not a silent wrong-file operation: git_stage_file (git_ops.rs:977-987) surfaces git add failed: fatal: pathspec 'file.txt' did not match any files via set_status. It only becomes a wrong-file op if a second path that happens to equal the trailing token also exists. The genuinely silent half is renames: every 2 line is displayed under its ORIGINAL path, so u runs git reset HEAD -- old.txt (unstaging only the deletion half, leaving a half-staged rename) and Enter/git-status-open (dispatch/git.rs:63-79) tries to open a path that no longer exists.

---

3. git blame --porcelain parser mis-attributes every repeated commit — author, date and summary leak from the previously-seen commit

bug · medium

Impact. The blame overlay confidently attributes lines to the wrong person and the wrong date whenever a commit touches non-contiguous line ranges — the normal case in any real file. Blame output is used to assign responsibility; a wrong author is worse than no blame at all.

Evidence

crates/core/src/editor/git_ops.rs:1035-1080: current_author / current_timestamp / current_summary are only reassigned when an author /author-time /summary header line appears, and are cloned into each entry on the \t content line. But git's porcelain format emits those headers only the FIRST time a commit is seen. Verified on a real repo:

418d1f6... 1 1 1
author A
...
summary c1
	l1
135cf64... 2 2 1
author B
...
summary c2
	CHANGED
418d1f6... 3 3 1        <-- no author/summary headers at all
	l3

Line 3 belongs to commit A but the parser still holds current_author = "B", current_summary = "c2", current_timestamp = c2, so the entry is emitted with B's name/date/message under A's hash. current_hash IS updated (git_ops.rs:1063-1076), so the hash column is right while every other column is wrong.
The guard test parse_blame_porcelain_basic (git_ops.rs:1294-1334) hand-writes the full header block twice — output real git blame never produces — so it passes over the bug. This is precisely the "cherry-picked unicorn value" failure mode CLAUDE.md #14 names.

Verification

Confirmed against the source and against real git output. crates/core/src/editor/git_ops.rs:1034-1078: current_author/current_timestamp/current_summary are only reassigned inside strip_prefix("author ") / strip_prefix("author-time ") / strip_prefix("summary ") arms, cloned into the entry on the \t content line, and current_hash IS updated in the 40-hex-digit arm (:1063-1076). There is no per-hash cache. I ran git blame --porcelain on a two-commit fixture: the third record is a4bdd8cf... 3 3 1 followed immediately by \tl3 with NO author/author-time/summary headers, while the parser still holds author Bob / summary c2 from the preceding record — so line 3 is emitted with commit A's hash and commit B's author, date and message, exactly as claimed. The guard test at git_ops.rs:1294-1334 does hand-write a full header block for both commits (verified: committer John Doe ... summary Initial commit and committer Jane Smith ... summary Second commit), output real git never produces — a genuine principle-#14 unicorn fixture. Downgraded to medium: the only consumer is the GUI blame overlay, which is itself mis-positioned (finding 2) and absent entirely in the TUI, so the blast radius is a read-only informational overlay, not any mutation.

---

4. git-discard (x) destroys uncommitted work with no confirmation, in an editor that confirms file-delete and buffer-revert

bug · medium

Impact. Discarding uncommitted changes is the one git operation with no recovery path — magit prompts for exactly this reason. MAE prompts before deleting a file and before reverting a buffer, but not before destroying uncommitted work, and the behaviour is not configurable either way.

Evidence

crates/core/src/editor/dispatch/git.rs:57-59 → git_discard_at_cursor() (crates/core/src/editor/git_ops.rs:846-862) → either git_discard_hunk() (git apply --recount -R, git_ops.rs:646-652) or git_discard_file():

let (ok, _, stderr) = self.run_git_porcelain(&["checkout", "--", &path]);
if ok {
    self.set_status(format!("Discarded changes to {}", path));

No prompt, no undo, no option gate — a single keypress on x (modules/git-status/autoloads.scm:36) or SPC m x (line 73) irrecoverably discards the working-tree changes for the file/hunk under the cursor. Combined with finding 3, the path it discards may not even be the one displayed.
The confirmation infrastructure exists and is used for strictly less destructive actions: MiniDialog::confirm (crates/core/src/command_palette.rs:246) with MiniDialogContext::FileDelete and MiniDialogContext::RevertBuffer (command_palette.rs:132,153). There is no git_confirm_destructive-style option in crates/core/src/options.rs (zero git options exist at all).

Verification

Call path verified end to end: dispatch/git.rs:57-59 "git-discard" => { self.git_discard_at_cursor(); } -> git_ops.rs:846-862, which branches on GitLineKind::DiffHunk | DiffLine(_) to git_discard_hunk() (git_ops.rs:646-652, apply_hunk_patch(&["apply", "--recount", "-R"], ...)) and otherwise to git_discard_file() (:654+, run_git_porcelain(&["checkout", "--", &path])). No prompt, no MiniDialog, no option check anywhere on that path. Keybindings verified: modules/git-status/autoloads.scm:36 (define-key "git-status" "x" "git-discard") and :73 SPC m x. The comparison holds: MiniDialog::confirm exists (command_palette.rs:246-258) and MiniDialogContext::FileDelete / RevertBuffer are real variants (command_palette.rs:132,153). Zero git options exist in options.rs (verified independently — only two unrelated substring hits), so there is no gate either way.

---

5. Three divergent git status parsers and two different git-root resolvers across the same workspace

duplication · medium

Impact. The rename bug in finding 3 exists specifically because parser #1 did not reuse the rename handling parser #2 already had; each new git surface re-derives porcelain parsing and re-derives "what is the repo root", so a fix in one place silently leaves the other two wrong. CLAUDE.md #15 explicitly forbids adding a third parallel implementation instead of consolidating.

Evidence

Parsers:

1. crates/core/src/editor/git_ops.rs:148,174-193 — status --porcelain=v2 --branch, split_whitespace(), no rename handling (finding 3).
2. crates/core/src/file_tree.rs:178-200 — status --porcelain=v1, byte-slicing line[3..], DOES handle renames via find(" -> ").
3. crates/ai/src/tool_impls/git.rs:28-68 — status --porcelain --branch, byte-slicing plus a third, different XY→section mapping (match status { "M " | "A " | ... => staged, ... _ => unstaged }).
   Root resolvers:

• crates/core/src/editor/git_ops.rs:937-942 fn git_root() → active_project_root() or CWD — never ascends to find .git.
• crates/ai/src/tool_impls/git.rs:11-16 → editor.git_or_project_root() (crates/core/src/editor/project_ops.rs:224-240), which DOES walk up to the .git directory.
  So the human's magit buffer and the AI's git_status tool can be rooted at different directories in the same repo (e.g. a monorepo subcrate project root), and produce differently-shaped path strings for the same working tree.
  None of this lives in render_common/ or a shared git module; crates/core/src/render_common/git_status.rs only maps GitLineKind → theme key.

Verification

All five cited sites verified. Parser 1: git_ops.rs:148 run_git_porcelain(&["status", "--porcelain=v2", "--branch"]) + :177-193 split_whitespace, no rename handling. Parser 2: file_tree.rs:180-200, --porcelain=v1, byte-slices &line[3..], handles renames via find(" -> "). Parser 3: crates/ai/src/tool_impls/git.rs:29-58, --porcelain --branch, let status = &line[0..2]; let path = &line[3..]; with a third distinct XY mapping ("M " | "A " | "D " | "R " | "C " => staged, " M" | " D" | " R" | " C" => unstaged, "MM"|"AM"|"DM" => both, _ => unstaged) and no rename handling either. Root resolvers genuinely diverge: git_ops.rs:942-947 fn git_root() is active_project_root().or_else(|| current_dir().ok()).unwrap_or_default() with no .git ascent, while tool_impls/git.rs:11-16 calls editor.git_or_project_root() which is a real upward walk (project_ops.rs:226-240, loop { if dir.join(".git").exists() { return Some(dir); } if !dir.pop() { break; } }). Observable divergence: with a monorepo subcrate as the active project root, the human's magit buffer runs git in the subcrate while the AI's git_status runs it at the repo root, producing different path prefixes and a different file set for the same working tree. crates/core/src/render_common/git_status.rs is 188 lines and is only theme-key mapping (git_line_theme_key + compute_git_status_spans), so the claim that no shared git module exists holds.

---

6. git-branch-create / git-branch-delete are registered, keybound and KB-documented but unreachable from every human surface; the implementations are dead code

parity-gap · medium

Impact. Two advertised, keybound, help-documented commands do nothing, and the fix is not "type the ex form" because the ex form is equally dead. The AI can create and delete branches in the user's repo while the user cannot do either from the editor — a direct #3 inversion.

Evidence

Registered: crates/core/src/commands.rs:1159-1160 ("Create new branch (b n in git status)"). Keybound: modules/git-status/autoloads.scm:53-54 ((define-key "git-status" "b n" "git-branch-create")). Documented: crates/core/src/kb_seed/concepts.rs:26-27 (| \b n` | Create branch | [[cmd:git-branch-create]] |`). Dispatch, crates/core/src/editor/dispatch/git.rs:87-93:

"git-branch-create" => {
    // Uses command-line input; prompt via status
    self.set_status("Use :git-branch-create <name>");
}
"git-branch-delete" => {
    self.set_status("Use :git-branch-delete <name>");
}

But :git-branch-create foo reaches the ex-parser's final fallback (crates/core/src/editor/command.rs:1219-1224: if let Some(a) = args { self.vi.command_line = a.to_string(); } if self.dispatch_builtin(command) { return true; }) which re-enters the same arm and prints the same message — an instruction loop. grep -rn git_branch_create outside git_ops.rs finds no caller, so Editor::git_branch_create (git_ops.rs:761-773) and git_branch_delete (776-787) are dead code.
Meanwhile the AI peer CAN do both: git_checkout with create: true (crates/ai/src/tool_impls/git.rs:192-213) and git_branch_delete (crates/ai/src/tool_impls/git.rs:261-277).
The editor already has the input mechanism this needs — MiniDialog::single_input (crates/core/src/command_palette.rs:259) — and git-branch-switch already uses the palette (git_ops.rs:728-743).

Verification

Every cited site verified. commands.rs:1159-1160 registers both ("Create new branch (b n in git status)" / "Delete a branch (b d in git status)"); modules/git-status/autoloads.scm:53-54 binds b n / b d; kb_seed/concepts.rs:26-27 documents them. dispatch/git.rs:87-93 is verbatim "git-branch-create" => { // Uses command-line input; prompt via status\n self.set_status("Use :git-branch-create <name>"); } and the same for delete. A repo-wide grep -rn 'git_branch_create|git_branch_delete|git-branch-create|git-branch-delete' returns 17 hits and NONE is a call to Editor::git_branch_create/git_branch_delete — the only occurrences of those symbols in mae-core are their definitions at git_ops.rs:761 and :776. command.rs has no git-branch-* arm, so :git-branch-create foo reaches the final fallback (command.rs:1219-1224, if let Some(a) = args { self.vi.command_line = a.to_string(); } if self.dispatch_builtin(command) { return true; }) which re-enters the same dispatch arm and reprints the same instruction — a closed loop, confirmed. The AI counterpart exists: crates/ai/src/tools/shell_tools.rs:156 ToolDefBuilder::new("git_branch_delete", ...) -> tool_impls/git.rs:261, plus git_checkout with create. MiniDialog::single_input (command_palette.rs:259) and the existing git_branch_switch_palette (dispatch/git.rs:86) confirm the input mechanism already exists.

---

7. No end-to-end coverage of the magit buffer: git_status(), hunk patch construction, fold state and stash-ref parsing are entirely untested

test-gap · medium

Impact. Every bug in findings 1, 3, 4 and 6 is in code with zero test coverage, in a subsystem that mutates the user's repository. The stash fallback in particular is an unlogged silent default: if stash_index_at_cursor fails to find stash@{N} on the cursor line, z d drops stash@{0} and reports "Dropped stash@{0}" as success. A real-repo fixture (tempdir + git init, as the existing #79 test already demonstrates) would falsify all of them.

Evidence

The only tests in crates/core/src/editor/git_ops.rs (lines 1236-1377) cover parse_diff_hunks (5 cases), parse_blame_porcelain (1 unicorn-input case — see finding 4) and the #79 push-failure notification. grep -rn 'build_hunk_patch\|git_status()\|git_toggle_fold\|find_cursor_hunk\|stash_index_at_cursor' crates/core/src outside git_ops.rs returns only dispatch call sites — no test constructs a real repo and asserts the parsed GitStatusView, no test round-trips build_hunk_patch (git_ops.rs:581-593) through git apply --check, no test exercises the multi-level collapse state machine in git_toggle_fold (git_ops.rs:471-499), and no test covers stash_index_at_cursor (git_ops.rs:802-810) — whose failure mode is a silent fallback to stash@{0} (git_ops.rs:813-817), i.e. dropping/popping the WRONG stash when parsing fails.
The MCP self-test git category (crates/ai/src/executor/self_test.rs:635-677) has 4 tests, all read-only AI tools (git_status, git_log, git_diff, github_pr_status) graded success_only/json_field_exists — it never opens *git-status*, never stages, never discards. ls tests/ shows collab-e2e, collab-local, crdt, editor, fixtures, headless-e2e, kb-lifecycle — no git suite; the only Scheme-test grep hit for "git" is tests/headless-e2e/verify.sh.

Verification

Count verified: grep -n '#\[test\]' crates/core/src/editor/git_ops.rs returns exactly 7 (lines 1240, 1250, 1265, 1276, 1282, 1294, 1341) — 5 parse_diff_hunks, 1 parse_blame_porcelain (the unicorn fixture of finding 4), 1 push-failure notification. No test touches git_status(), build_hunk_patch (git_ops.rs:581-593), git_toggle_fold, find_cursor_hunk, or stash_index_at_cursor. The stash fallback is real and worse than described in one respect: stash_ref_at_cursor (git_ops.rs:812-817) is self.stash_index_at_cursor().map(...).unwrap_or_else(|| "stash@{0}".to_string()) and stash_op (:820-828) reports set_status(format!("{} {}", past_tense, idx_str)) on success — so z d pressed on any non-stash line (a file entry, a diff line, the header) drops stash@{0} and reports "Dropped stash@{0}" as success. The MCP self-test git category (crates/ai/src/executor/self_test.rs:634-676) is exactly the 4 read-only AI tools claimed (git_status/git_log/git_diff/github_pr_status, graded json_field_exists / min_count / success_only), never opening *git-status*. ls tests/ returns collab-e2e, collab-local, crdt, editor, fixtures, headless-e2e, kb-lifecycle — no git suite.

---

From the pre-v0.15 codebase audit (epic #592). Traced end-to-end across crates, then independently re-verified by a reviewer briefed to refute it; severity is the verifier's corrected value. In this batch, 8 of 11 claimed-high were downgraded.

[cuttlefisch]

Part of the pre-v0.15 codebase audit — tracking epic #592.