fix(issue-21): reviews 8f141362 N1 and 275fdc4f R4/R5 - clean refusal for a broken worktree, no init over existing files, check retained submodule repositories

- N1 (planner-reviewer-claude-01) worktree_remove.sh: when `git status` fails
  in the worktree (e.g. its directory was deleted), judge_tree returned before
  METHOD was set and the summary line died under set -u with exit 1. METHOD
  now has its default before the function; the refusal is exit 3 with
  "nothing was changed", plus a `git worktree prune` hint when the directory
  is gone. Same for --dry-run.
- R4 (planner-reviewer-codex-01) worktree_prepare.sh: a submodule path that is
  not initialised yet but already holds files made the clone fail, and the
  rollback's `deinit -f` then deleted those files. --submodules now refuses
  such a path up front (new exit 3, nothing changed), which also makes the
  rollback's premise true: after a failed attempt, everything under a
  not-yet-initialised path was created by that attempt.
- R5 (planner-reviewer-codex-01) lib_py/worktree.py: `submodule foreach`
  visits initialised checkouts only, but a deinit'd submodule keeps its
  repository - stash and unpushed commits included - under the worktree's
  modules/, which the forced removal deletes. Every repository under modules/
  (nested ones included) is now checked directly (`--git-dir`): commits on no
  remote (HEAD included) and refs/stash with its reflog. A directory that looks
  like a partial repository cannot be checked and refuses.
- Also: the rollback loops are C-style (`seq 0 -1` counts down on BSD), and
  prepare warns that --base is ignored for an existing branch.

Tests W6i (deleted worktree directory, with and without --dry-run), W6j and
W6j2 (stash / unpushed commit in a deinit'd submodule's repository) and W6k
(existing files in an uninitialised submodule path). Skill docs and the design
doc describe the new refusal and the wider repository check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
This commit is contained in:
GodopuandClaude Opus 5 committed 2026-09-14 18:14:13 +00:00
1 parent bbbbb95ef6
commit c7c559b7aa
7 files changed
+168 -8

No files matched your search

+88
View File
@@ -516,3 +516,91 @@ def test_w6h_failed_retry_keeps_the_earlier_submodule_state(ctl_sub):
assert (wt / "sm" / ".git").exists() and (wt / "sm" / "s.txt").exists()
assert _git(main, "config", "--get", "submodule.sm.url").stdout.strip() == url_before
assert _git(main, "config", "--get", "submodule.zbad.url", check=False).returncode != 0
# ---------------------------------------------------------------------------
# reviews 8f141362 (N1) and 275fdc4f (R4, R5)
# ---------------------------------------------------------------------------
def test_w6i_deleted_worktree_directory_is_refused_cleanly(ctl):
"""N1: `git status` fails in a worktree whose directory is gone - the
helper refuses (exit 3) instead of dying on an unset METHOD."""
main = ctl
wt = _wt(main)
assert _prepare(main, wt).returncode == 0
shutil.rmtree(wt)
for extra in ([], ["--dry-run"]):
res = _run(main, REM, "--path", str(wt), *extra)
assert res.returncode == 3, res.stdout + res.stderr
assert "worktree prune" in res.stderr
assert "nothing was changed" in res.stdout + res.stderr
assert "unbound variable" not in res.stderr
assert str(wt) in _worktrees(main), "nothing was pruned or removed"
def _deinit_after(wt: Path, make_state) -> str:
"""Put state into the initialised `sm`, then deinit it (no force); returns
the submodule's repository, which stays under the worktree's modules/."""
make_state(wt / "sm")
gd = _git(wt / "sm", "rev-parse", "--absolute-git-dir").stdout.strip()
_git(wt, "submodule", "deinit", "-q", "sm")
assert not (wt / "sm" / ".git").exists() and os.path.isdir(gd)
return gd
def test_w6j_stash_in_a_deinitialised_submodule_repository_is_kept(ctl_sub):
"""R5: foreach no longer visits a deinit'd submodule, but its repository -
and its stash - is still under modules/ and would be force-removed."""
main = ctl_sub
wt = _wt(main)
assert _prepare(main, wt, "--submodules", env=FILE_OK).returncode == 0
def stash(sm):
(sm / "s.txt").write_text("valuable stashed work\n")
_git(sm, "stash", "push", "-q", "-m", "keep this work")
gd = _deinit_after(wt, stash)
before = _git(gd, "rev-parse", "refs/stash").stdout.strip()
res = _run(main, REM, "--path", str(wt))
assert res.returncode == 3 and "'stashes'" in res.stderr, res.stdout + res.stderr
assert _git(gd, "rev-parse", "refs/stash").stdout.strip() == before
def test_w6j2_unpushed_commit_in_a_deinitialised_submodule_repository_is_kept(ctl_sub):
main = ctl_sub
wt = _wt(main)
assert _prepare(main, wt, "--submodules", env=FILE_OK).returncode == 0
local = []
def commit(sm):
(sm / "new.txt").write_text("n\n")
_git(sm, "add", "new.txt")
_git(sm, "commit", "-q", "-m", "local only")
local.append(_git(sm, "rev-parse", "HEAD").stdout.strip())
# record the new gitlink first: deinit refuses a submodule whose HEAD
# differs from it, and the top level must be clean for the scenario
_git(wt, "add", "sm")
_git(wt, "commit", "-q", "-m", "bump sm")
gd = _deinit_after(wt, commit)
assert _git(wt, "status", "--porcelain", "--ignore-submodules=none").stdout == ""
res = _run(main, REM, "--path", str(wt))
assert res.returncode == 3 and "'unpushed'" in res.stderr, res.stdout + res.stderr
assert _git(gd, "cat-file", "-e", local[0], check=False).returncode == 0
def test_w6k_files_in_an_uninitialised_submodule_path_refuse_the_init(ctl_sub):
"""R4: files that predate the attempt in a not-yet-initialised submodule
path refuse --submodules up front; nothing is changed or deleted."""
main = ctl_sub
wt = _wt(main)
assert _prepare(main, wt).returncode == 0
valuable = wt / "sm" / "user-work.txt"
valuable.write_text("existing user work\n")
res = _prepare(main, wt, "--submodules", env=FILE_OK)
assert res.returncode == 3 and "not empty" in res.stderr, res.stdout + res.stderr
assert valuable.read_text() == "existing user work\n"
gd = _git(wt, "rev-parse", "--absolute-git-dir").stdout.strip()
assert not os.path.exists(os.path.join(gd, "modules"))
assert _git(main, "config", "--get", "submodule.sm.url", check=False).returncode != 0