Skip to content

Commit 87391a6

Browse files
fix(upgrade): harden auto-commit safety and improve test coverage
- _git_status_paths returns None on failure instead of empty set, preventing accidental commits of unrelated work when git status fails at baseline - _prepare_upgrade_commit_files skips auto-commit when baseline is None - Change allow_empty=True to allow_empty=False in safe_commit call - Initialize auto_commit_warning in both code paths (no-migrations and migrations-ran) - Add clarifying comment about rename handling (destination path used) - Add comprehensive unit tests for _git_status_paths parsing, _is_upgrade_commit_eligible edge cases, failure paths, and upgrade() function wiring (27 tests total) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1eeb3b6 commit 87391a6

File tree

2 files changed

+470
-9
lines changed

2 files changed

+470
-9
lines changed

src/specify_cli/cli/commands/upgrade.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,20 @@
1616
from specify_cli.git.commit_helpers import safe_commit
1717

1818

19-
def _git_status_paths(repo_path: Path) -> set[str]:
20-
"""Return git status paths for *repo_path* using porcelain -z output."""
19+
def _git_status_paths(repo_path: Path) -> set[str] | None:
20+
"""Return git status paths for *repo_path* using porcelain -z output.
21+
22+
Returns ``None`` when ``git status`` fails (e.g. not a git repo) so
23+
callers can distinguish "no dirty files" from "unable to determine".
24+
"""
2125
result = subprocess.run(
2226
["git", "status", "--porcelain", "-z"],
2327
cwd=repo_path,
2428
capture_output=True,
2529
check=False,
2630
)
2731
if result.returncode != 0:
28-
return set()
32+
return None
2933

3034
entries = result.stdout.decode("utf-8", errors="replace").split("\0")
3135
paths: set[str] = set()
@@ -40,7 +44,9 @@ def _git_status_paths(repo_path: Path) -> set[str]:
4044
status = entry[:2]
4145
path = entry[3:]
4246

43-
# With -z format, renames/copies include a second NUL-separated path.
47+
# With -z format, renames/copies include a second NUL-separated
48+
# path. We take the *destination* (new name); the source (old name)
49+
# is intentionally discarded because we care about "what exists now".
4450
if "R" in status or "C" in status:
4551
if i < len(entries) and entries[i]:
4652
path = entries[i]
@@ -75,10 +81,20 @@ def _is_upgrade_commit_eligible(path: str, project_path: Path) -> bool:
7581

7682
def _prepare_upgrade_commit_files(
7783
project_path: Path,
78-
baseline_paths: set[str],
84+
baseline_paths: set[str] | None,
7985
) -> list[Path]:
80-
"""Collect newly changed project-directory files after an upgrade run."""
86+
"""Collect newly changed project-directory files after an upgrade run.
87+
88+
Returns an empty list when *baseline_paths* is ``None`` (git status
89+
failed at baseline time) to avoid accidentally committing unrelated work.
90+
"""
91+
if baseline_paths is None:
92+
return []
93+
8194
current_paths = _git_status_paths(project_path)
95+
if current_paths is None:
96+
return []
97+
8298
new_paths = sorted(
8399
path
84100
for path in current_paths
@@ -91,7 +107,7 @@ def _auto_commit_upgrade_changes(
91107
project_path: Path,
92108
from_version: str,
93109
to_version: str,
94-
baseline_paths: set[str],
110+
baseline_paths: set[str] | None,
95111
) -> tuple[bool, list[str], str | None]:
96112
"""Auto-commit newly introduced project-directory upgrade changes."""
97113
files_to_commit = _prepare_upgrade_commit_files(project_path, baseline_paths)
@@ -105,7 +121,7 @@ def _auto_commit_upgrade_changes(
105121
repo_path=project_path,
106122
files_to_commit=files_to_commit,
107123
commit_message=commit_message,
108-
allow_empty=True,
124+
allow_empty=False,
109125
)
110126
committed_paths = [str(path).replace("\\", "/") for path in files_to_commit]
111127

@@ -292,6 +308,7 @@ def upgrade(
292308

293309
auto_committed = False
294310
auto_commit_paths: list[str] = []
311+
auto_commit_warning: str | None = None
295312
if result.success and not dry_run:
296313
auto_committed, auto_commit_paths, auto_commit_warning = _auto_commit_upgrade_changes(
297314
project_path=project_path,

0 commit comments

Comments
 (0)