Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions tests/tools/test_subagent_worktree.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
from tools import subagent_worktree as sw # noqa: E402


def _git(args, cwd):
def _git(args, cwd, check=True):
return subprocess.run(
["git", *args], cwd=cwd, capture_output=True, text=True, check=True
["git", *args], cwd=cwd, capture_output=True, text=True, check=check
)


Expand Down Expand Up @@ -132,6 +132,38 @@ def test_finalize_keeps_dirty_worktree(self):
self.assertTrue(payload["dirty"])
self.assertTrue(os.path.isdir(info["path"]))

def test_finalize_keeps_worktree_when_git_inspection_fails(self):
"""#88113: a non-zero git status exit must not be read as "clean".

Corrupting the index makes the real `git status --porcelain` probe
exit 128. The old code kept the payload defaults (commits=0,
dirty=False) and pruned on them — permanently deleting the child's
uncommitted work. A destructive cleanup requires affirmative proof
of a clean tree."""
repo = _make_repo(self.tmp)
info = sw.create_subagent_worktree(str(repo), "inspect-fail1")
assert info is not None
wt = Path(info["path"])
(wt / "UNCOMMITTED-WORK.txt").write_text(
"irreplaceable\n", encoding="utf-8"
)

git_dir = Path(_git(["rev-parse", "--git-dir"], wt).stdout.strip())
if not git_dir.is_absolute():
git_dir = (wt / git_dir).resolve()
(git_dir / "index").write_bytes(b"not-a-valid-git-index\n")
# Sanity: the probe really fails now.
broken = _git(["status", "--porcelain"], wt, check=False)
self.assertNotEqual(broken.returncode, 0)

payload = sw.finalize_subagent_worktree(info)

self.assertFalse(payload["pruned"])
self.assertTrue(os.path.isdir(info["path"]))
self.assertTrue((wt / "UNCOMMITTED-WORK.txt").exists())
branches = _git(["branch", "--list", info["branch"]], repo).stdout
self.assertNotEqual(branches.strip(), "")

def test_finalize_missing_path_reports_pruned(self):
payload = sw.finalize_subagent_worktree(
{"path": str(self.tmp / "gone"), "branch": "b", "repo_root": "",
Expand Down
20 changes: 20 additions & 0 deletions tools/subagent_worktree.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,41 @@ def finalize_subagent_worktree(
payload["pruned"] = True # nothing on disk to review
return payload

inspection_ok = True
try:
if base_commit:
counted = _run_git(
["rev-list", "--count", f"{base_commit}..HEAD"], cwd=path
)
if counted.returncode == 0:
payload["commits"] = int(counted.stdout.strip() or 0)
else:
inspection_ok = False
status = _run_git(["status", "--porcelain"], cwd=path)
if status.returncode == 0:
payload["dirty"] = bool(status.stdout.strip())
else:
inspection_ok = False
except Exception as exc:
logger.debug("subagent worktree: finalize inspection failed: %s", exc)
# Unknown state — keep the worktree rather than risk deleting work.
return payload

if not inspection_ok:
# Fail-safe (#88113): a non-zero git exit proves nothing about the
# tree — the payload defaults (0 commits, clean) were never
# overwritten, and pruning on them permanently deleted uncommitted
# child work. A destructive cleanup requires affirmative proof of
# "zero commits + clean tree"; otherwise keep the worktree and
# branch for manual inspection.
logger.warning(
"subagent worktree: git inspection failed (rev-list/status "
"non-zero) — keeping %s (branch %s) for manual review",
path,
branch,
)
return payload

if prune and payload["commits"] == 0 and not payload["dirty"]:
try:
removed = _run_git(
Expand Down
Loading