From 7edc36d4a4224cbc8a0effa60fbd9cdfd1b467d4 Mon Sep 17 00:00:00 2001 From: Amar K P Date: Wed, 9 Sep 2026 14:36:48 +0530 Subject: [PATCH 1/2] fix: filename quoting, nested globs, and small display bugs - Decode git C-style quoting (backslash, quotes, octal) so quoted patch and status keys align with raw -z numstat keys - Parse quoted diff --git headers by splitting between quoted strings - Match dir-prefix ignore globs at any depth for monorepos - LC_ALL=C for deterministic English git errors; clean GitCommandError on non-UTF-8 output - config show prints 0.0 budget instead of none; ignore tool-dropped .agents/ dir --- .gitignore | 3 + src/gitmate/cli.py | 4 +- src/gitmate/diff_extractor.py | 114 +++++++++++++++++++++++++++++----- tests/test_cli.py | 10 +++ tests/test_diff_extractor.py | 50 +++++++++++++++ 5 files changed, 165 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index 9ddf47e..3eb0f44 100644 --- a/.gitignore +++ b/.gitignore @@ -66,6 +66,9 @@ Thumbs.db *.log *.tmp +# Agent tooling droppings (not part of this project) +.agents/ + #md files plan.md diff --git a/src/gitmate/cli.py b/src/gitmate/cli.py index ebc2424..40b4741 100644 --- a/src/gitmate/cli.py +++ b/src/gitmate/cli.py @@ -125,7 +125,9 @@ def config_show() -> None: table.add_row("config file", str(config_mod.config_path())) table.add_row("model", cfg.model) table.add_row("commit_style", cfg.commit_style) - table.add_row("budget_cap_usd", str(cfg.budget_cap_usd) if cfg.budget_cap_usd else "none") + table.add_row( + "budget_cap_usd", str(cfg.budget_cap_usd) if cfg.budget_cap_usd is not None else "none" + ) table.add_row("ignore_globs", ", ".join(cfg.ignore_globs) if cfg.ignore_globs else "none") table.add_row("api_key", "set" if key_set else "not set") console.print(table) diff --git a/src/gitmate/diff_extractor.py b/src/gitmate/diff_extractor.py index dde9f27..bf7f192 100644 --- a/src/gitmate/diff_extractor.py +++ b/src/gitmate/diff_extractor.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import subprocess from collections.abc import Iterable from dataclasses import dataclass @@ -72,11 +73,15 @@ def run(self, args: list[str], cwd: Path) -> str: text=True, timeout=self._timeout, check=False, + # Deterministic English errors regardless of host locale. + env={**os.environ, "LC_ALL": "C"}, ) except FileNotFoundError as exc: raise GitCommandError("git binary not found on PATH.") from exc except subprocess.TimeoutExpired as exc: raise GitCommandError(f"git {' '.join(args)} timed out.") from exc + except UnicodeDecodeError as exc: + raise GitCommandError(f"git {' '.join(args)} output was not valid UTF-8.") from exc if proc.returncode != 0: err = proc.stderr.strip() if "not a git repository" in err: @@ -85,17 +90,25 @@ def run(self, args: list[str], cwd: Path) -> str: return proc.stdout +def _dir_match(posix: str, dirname: str) -> bool: + """True when dirname is the path itself or any parent directory in it.""" + return posix == dirname or posix.startswith(dirname + "/") or f"/{dirname}/" in f"/{posix}" + + def matches_any(path: str, patterns: Iterable[str]) -> bool: - """Gitignore-flavored match: dir prefixes, bare basenames, else fnmatch.""" + """Gitignore-flavored match: dir prefixes, bare basenames, else fnmatch. + + Directory patterns (`dist/`, `dist/*`) match at any depth, so monorepo + `packages/ui/dist/` output is filtered just like root-level `dist/`. + """ posix = PurePosixPath(path).as_posix() name = PurePosixPath(path).name for pat in patterns: if pat.endswith("/*"): - prefix = pat[:-1] - if posix == prefix[:-1] or posix.startswith(prefix): + if _dir_match(posix, pat[:-2]): return True elif pat.endswith("/"): - if posix == pat[:-1] or posix.startswith(pat): + if _dir_match(posix, pat[:-1]): return True elif "/" not in pat: if fnmatchcase(name, pat): @@ -174,18 +187,92 @@ def _parse_name_status(output: str) -> dict[str, tuple[FileStatus, str | None]]: if len(parts) != 3: raise GitCommandError(f"cannot parse rename/copy row: {line!r}.") _, old, new = parts - statuses[new] = (status, old) + statuses[_decode_quoted(new)] = (status, _decode_quoted(old)) else: if len(parts) != 2: raise GitCommandError(f"cannot parse status row: {line!r}.") - statuses[parts[1]] = (status, None) + statuses[_decode_quoted(parts[1])] = (status, None) return statuses -def _strip_git_prefix(path: str) -> str: - """Strip git's a//b/ prefixes and optional C-style quoting.""" +def _split_header_fallback(rest: str) -> tuple[str | None, str | None]: + """Split `a/X b/Y` on the last ` b/`; prefixes are pinned by our git flags. + + Correct for any spaces inside X or Y. Only a filename literally containing + ` b/` defeats it; the rename/copy metadata and ---/+++ re-keying backstop + even that pathological case. + """ + if rest.startswith('"'): + # Both paths quoted: "a/X" "b/Y" — split between the quoted strings. + idx = rest.find('" "') + if idx == -1: + return None, None + return _strip_git_prefix(rest[: idx + 1]), _strip_git_prefix(rest[idx + 1 :].strip()) + idx = rest.rfind(" b/") + if idx < 2: + return None, None + return _strip_git_prefix(rest[:idx]), _strip_git_prefix(rest[idx + 1 :]) + + +def _unescape_c_quoting(text: str) -> str: + """Decode git's C-style path escapes (`\\\\`, `\\"`, `\\n`, `\\t`, octal). + + Only meaningful on paths git quoted; unquoted output (e.g. `-z --numstat`) + is never munged and must pass through untouched. Octal runs decode as + UTF-8 bytes because git escapes non-ASCII per byte, not per character. + """ + out: list[str] = [] + raw = bytearray() + + def flush_octal() -> None: + if raw: + out.append(bytes(raw).decode("utf-8", errors="surrogateescape")) + raw.clear() + + i, n = 0, len(text) + while i < n: + ch = text[i] + if ch != "\\" or i + 1 >= n: + flush_octal() + out.append(ch) + i += 1 + continue + nxt = text[i + 1] + if nxt in "01234567": + digits = text[i + 1 : i + 4] + length = 0 + while length < 3 and length < len(digits) and digits[length] in "01234567": + length += 1 + raw.extend(int(digits[:length], 8).to_bytes(1, "big")) + i += 1 + length + continue + flush_octal() + if nxt == "n": + out.append("\n") + elif nxt == "t": + out.append("\t") + elif nxt == "\\": + out.append("\\") + elif nxt == '"': + out.append('"') + else: + out.append(ch) + out.append(nxt) + i += 2 + flush_octal() + return "".join(out) + + +def _decode_quoted(path: str) -> str: + """Remove git's C-style quoting when present; bare paths pass through.""" if len(path) >= 2 and path.startswith('"') and path.endswith('"'): - path = path[1:-1] + return _unescape_c_quoting(path[1:-1]) + return path + + +def _strip_git_prefix(path: str) -> str: + """Strip git's a//b/ prefixes, unescaping C-style quoting when present.""" + path = _decode_quoted(path) if path.startswith(("a/", "b/")): return path[2:] return path @@ -224,13 +311,9 @@ def flush() -> None: minus = _strip_git_prefix(rest[2:mid]) current = _strip_git_prefix(rest[mid + 3 :]) else: - halves = rest.rsplit(" ", 1) - minus = _strip_git_prefix(halves[0].strip()) if len(halves) == 2 else None - current = _strip_git_prefix(halves[1].strip()) if len(halves) == 2 else None + minus, current = _split_header_fallback(rest) else: - halves = rest.rsplit(" ", 1) - minus = _strip_git_prefix(halves[0].strip()) if len(halves) == 2 else None - current = _strip_git_prefix(halves[1].strip()) if len(halves) == 2 else None + minus, current = _split_header_fallback(rest) chunk = [line] elif current is None and minus is None: continue @@ -248,6 +331,7 @@ def flush() -> None: for prev in text.splitlines(): if prev.startswith("--- "): key = _strip_git_prefix(prev[4:].strip()) + break else: key = plus break diff --git a/tests/test_cli.py b/tests/test_cli.py index 87cc2b1..386e992 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -58,6 +58,16 @@ def test_config_set_field_persists(tmp_config_dir: Path) -> None: assert load_config().model == "my-model" +def test_config_show_displays_zero_budget( + tmp_config_dir: Path, mem_store: InMemorySecretStore +) -> None: + result = runner.invoke(cli.app, ["config", "set", "budget_cap_usd", "0"]) + assert result.exit_code == 0 + result = runner.invoke(cli.app, ["config", "show"]) + assert result.exit_code == 0 + assert "0.0" in result.output + + def test_config_set_unknown_field_fails(tmp_config_dir: Path) -> None: result = runner.invoke(cli.app, ["config", "set", "nope", "x"]) assert result.exit_code != 0 diff --git a/tests/test_diff_extractor.py b/tests/test_diff_extractor.py index 2f5951a..103b367 100644 --- a/tests/test_diff_extractor.py +++ b/tests/test_diff_extractor.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import subprocess from pathlib import Path @@ -88,6 +89,9 @@ def test_matcher_rules() -> None: assert matches_any("a/b/c.min.js", ["*.min.js"]) assert not matches_any("src/app.py", ["dist/*", "*.min.js"]) assert matches_any("out/log.txt", ["out/"]) + assert matches_any("packages/ui/dist/bundle.js", ["dist/*"]) + assert matches_any("packages/ui/dist/bundle.js", ["dist/"]) + assert not matches_any("src/distraction.py", ["dist/*"]) def test_branch_comparison(git_repo: Path) -> None: @@ -178,3 +182,49 @@ def test_pure_rename_with_spaces(git_repo: Path) -> None: def test_copy_status_row() -> None: statuses = _parse_name_status("C100\told.py\tnew.py\n") assert statuses["new.py"] == ("added", "old.py") + + +def test_nested_monorepo_dist_filtered(git_repo: Path) -> None: + stage_file(git_repo, "packages/ui/dist/bundle.js", b"var x;\n") + stage_file(git_repo, "src/ok.py", b"ok\n") + (diff,) = DiffExtractor(git_repo).staged() + assert diff.path == "src/ok.py" + + +def test_deleted_file_with_dash_comments_keeps_patch(git_repo: Path) -> None: + commit_file(git_repo, "query.sql", "SELECT 1;\n-- a comment\n", "add sql") + subprocess.run(["git", "rm", "query.sql"], cwd=git_repo, check=True) + (diff,) = DiffExtractor(git_repo).staged() + assert (diff.path, diff.status) == ("query.sql", "deleted") + assert "diff --git a/query.sql b/query.sql" in diff.patch_text + assert "-- a comment" in diff.patch_text + + +def test_backslash_filename_single_entry(git_repo: Path) -> None: + # A literal backslash is always C-quoted by git, even with quotepath=false; + # the entry must align with the raw -z numstat key, not duplicate. + stage_file(git_repo, "back\\slash.py", b"x = 1\n") + (diff,) = DiffExtractor(git_repo).staged() + assert diff.path == "back\\slash.py" + assert (diff.status, diff.additions) == ("added", 1) + assert "+x = 1" in diff.patch_text + + +def test_double_quote_filename_single_entry(git_repo: Path) -> None: + stage_file(git_repo, 'we"ird.py', b"x = 1\n") + (diff,) = DiffExtractor(git_repo).staged() + assert diff.path == 'we"ird.py' + assert diff.additions == 1 + + +def test_non_utf8_git_output_clean_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + # macOS refuses to create non-UTF-8 filenames, so fake the git binary + # itself emitting invalid bytes instead. + bindir = tmp_path / "bin" + bindir.mkdir() + fake = bindir / "git" + fake.write_bytes(b"#!/bin/sh\nprintf 'x\\377y'\n") + fake.chmod(0o755) + monkeypatch.setenv("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}") + with pytest.raises(GitCommandError): + DiffExtractor(tmp_path).staged() From bf649ffcef28c9862166ff5b1e5a5e030fbdefbd Mon Sep 17 00:00:00 2001 From: Amar K P Date: Wed, 9 Sep 2026 15:00:26 +0530 Subject: [PATCH 2/2] fix: mixed quoted headers and rename-to prefix stripping - Split diff --git headers across all four quoted/unquoted combinations - Stop stripping a//b/ from rename/copy from/to lines (never prefixed by git) - Regression tests: plain<->quoted renames with edits, pure rename into a/ dir --- src/gitmate/diff_extractor.py | 21 +++++++++++---------- tests/test_diff_extractor.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/gitmate/diff_extractor.py b/src/gitmate/diff_extractor.py index bf7f192..ebd93ca 100644 --- a/src/gitmate/diff_extractor.py +++ b/src/gitmate/diff_extractor.py @@ -196,18 +196,17 @@ def _parse_name_status(output: str) -> dict[str, tuple[FileStatus, str | None]]: def _split_header_fallback(rest: str) -> tuple[str | None, str | None]: - """Split `a/X b/Y` on the last ` b/`; prefixes are pinned by our git flags. + """Split `a/X b/Y` across quoted/unquoted combinations on either side. - Correct for any spaces inside X or Y. Only a filename literally containing - ` b/` defeats it; the rename/copy metadata and ---/+++ re-keying backstop - even that pathological case. + Our git flags pin the prefixes, so Y always begins `b/` or `"b/`, and + rfind lets the true boundary win over interior lookalikes. Rename/copy + metadata and ---/+++ re-keying backstop anything stranger still. """ - if rest.startswith('"'): - # Both paths quoted: "a/X" "b/Y" — split between the quoted strings. - idx = rest.find('" "') + if rest.endswith('"'): + idx = rest.rfind(' "b/') if idx == -1: return None, None - return _strip_git_prefix(rest[: idx + 1]), _strip_git_prefix(rest[idx + 1 :].strip()) + return _strip_git_prefix(rest[:idx]), _strip_git_prefix(rest[idx + 1 :]) idx = rest.rfind(" b/") if idx < 2: return None, None @@ -289,11 +288,13 @@ def _parse_patch(output: str) -> dict[str, str]: def flush() -> None: nonlocal current, minus if current is not None or minus is not None: + # Rename/copy lines carry bare paths (never a//b/ prefixed), so + # decode quoting only; stripping would eat real a//b/ directories. for cl in chunk: if cl.startswith(("rename to ", "copy to ")): - current = _strip_git_prefix(cl.split(" ", 2)[2].rstrip("\n")) + current = _decode_quoted(cl.split(" ", 2)[2].rstrip("\n")) elif cl.startswith(("rename from ", "copy from ")): - minus = _strip_git_prefix(cl.split(" ", 2)[2].rstrip("\n")) + minus = _decode_quoted(cl.split(" ", 2)[2].rstrip("\n")) key = current if current is not None and current != "/dev/null" else minus if key is not None: if key not in chunks: diff --git a/tests/test_diff_extractor.py b/tests/test_diff_extractor.py index 103b367..3c44e95 100644 --- a/tests/test_diff_extractor.py +++ b/tests/test_diff_extractor.py @@ -228,3 +228,33 @@ def test_non_utf8_git_output_clean_error(tmp_path: Path, monkeypatch: pytest.Mon monkeypatch.setenv("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}") with pytest.raises(GitCommandError): DiffExtractor(tmp_path).staged() + + +def test_rename_plain_to_quoted_keeps_patch(git_repo: Path) -> None: + commit_file(git_repo, "plain.txt", "line 1\n", "add plain") + subprocess.run(["git", "mv", "plain.txt", 'weird"name.txt'], cwd=git_repo, check=True) + (git_repo / 'weird"name.txt').write_text("line 1\nline 2\n", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True) + (diff,) = DiffExtractor(git_repo).staged() + assert (diff.path, diff.old_path, diff.status) == ('weird"name.txt', "plain.txt", "renamed") + assert "+line 2" in diff.patch_text + + +def test_rename_quoted_to_plain_keeps_patch(git_repo: Path) -> None: + commit_file(git_repo, 'weird"name.txt', "line 1\n", "add weird") + subprocess.run(["git", "mv", 'weird"name.txt', "plain.txt"], cwd=git_repo, check=True) + (git_repo / "plain.txt").write_text("line 1\nline 2\n", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True) + (diff,) = DiffExtractor(git_repo).staged() + assert (diff.path, diff.old_path, diff.status) == ("plain.txt", 'weird"name.txt', "renamed") + assert "+line 2" in diff.patch_text + + +def test_pure_rename_into_a_dir_keeps_patch(git_repo: Path) -> None: + commit_file(git_repo, "old.py", "line 1\n", "add old") + (git_repo / "a").mkdir() + subprocess.run(["git", "mv", "old.py", "a/foo.py"], cwd=git_repo, check=True) + subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True) + (diff,) = DiffExtractor(git_repo).staged() + assert (diff.path, diff.old_path, diff.status) == ("a/foo.py", "old.py", "renamed") + assert "rename to a/foo.py" in diff.patch_text