fix(issue-21): review ec50fbe7 R6 - the unpushed check covers HEAD and every local ref (tags included, stash excluded) in both submodule checks
Both unpushed checks in lib_py/worktree.py - `submodule foreach` over the initialised checkouts and the direct `--git-dir` walk over the repositories under the worktree's modules/ - used `HEAD --branches --not --remotes`. A commit made on a submodule's detached HEAD and kept only by a tag is reachable from neither, the top-level status stays clean, and worktree_remove.sh then deleted the repository with its single --force (exit 0). Both checks now share one rev list, UNPUSHED_REVS = HEAD --exclude=refs/stash --all --not --remotes: HEAD and every local ref (branches, tags, notes, ...) minus remote-tracking refs. refs/stash stays out of it because the stash check (stash list / reflog) owns it, so a stash is reported once, as 'stashes'. As intended, a local-only note is now protected too. The stash, error, partial repository and dirty checks are unchanged. Tests W6l (tag-only commit; initialised submodule and deinit'd repository) and W6m (annotated tag). The helper header, the stop SKILL.md and the design doc state the wider rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
This commit is contained in:
1 parent
c7c559b7aa
commit
25f4be3359
5 files changed
+59
-10
No files matched your search
@@ -24,6 +24,10 @@ import sys
|
||||
from typing import Dict, Iterable, List, Optional
|
||||
|
||||
OPEN_STATUSES = ("pending", "running")
|
||||
# The commits a forced removal would lose (review R6): reachable from HEAD or
|
||||
# from any local ref - branches, tags, notes, ... - except refs/stash (the
|
||||
# stash check covers it, reflog included), and from no remote-tracking ref.
|
||||
UNPUSHED_REVS = ("HEAD", "--exclude=refs/stash", "--all", "--not", "--remotes")
|
||||
|
||||
|
||||
def _git(cwd: str, *args: str) -> subprocess.CompletedProcess:
|
||||
@@ -146,10 +150,12 @@ def submodule_findings(wt: str) -> Dict[str, object]:
|
||||
"""trace: the worktree git dir holds modules/ (git then refuses a plain
|
||||
`worktree remove`, even after a failed init); dirty / unpushed / stashes:
|
||||
what a forced removal would destroy - it deletes the worktree's own
|
||||
submodule repositories. `HEAD` is part of the unpushed check: a submodule
|
||||
is on a detached HEAD after `submodule update`, so commits made there are
|
||||
on no branch and `--branches --not --remotes` alone misses them. Stashes
|
||||
live in refs/stash (and its reflog), reachable from neither."""
|
||||
submodule repositories. The unpushed check covers HEAD and every local
|
||||
ref except refs/stash (UNPUSHED_REVS): a submodule is on a detached HEAD
|
||||
after `submodule update`, so its commits may be on no branch, and a commit
|
||||
kept only by a tag (review R6) or a note is lost with the repository just
|
||||
the same. Stashes live in refs/stash and its reflog; the stash check owns
|
||||
them."""
|
||||
gd = _git(wt, "rev-parse", "--absolute-git-dir")
|
||||
if gd.returncode != 0 or not gd.stdout.strip():
|
||||
return {"trace": False, "dirty": "", "unpushed": "", "stashes": "",
|
||||
@@ -159,7 +165,7 @@ def submodule_findings(wt: str) -> Dict[str, object]:
|
||||
if not res["trace"]:
|
||||
return res
|
||||
checks = (("dirty", "git status --porcelain"),
|
||||
("unpushed", "git log --oneline HEAD --branches --not --remotes"),
|
||||
("unpushed", "git log --oneline " + " ".join(UNPUSHED_REVS)),
|
||||
("stashes", "git stash list"))
|
||||
errors = []
|
||||
for key, cmd in checks:
|
||||
@@ -177,7 +183,7 @@ def submodule_findings(wt: str) -> Dict[str, object]:
|
||||
extra: Dict[str, List[str]] = {"unpushed": [], "stashes": []}
|
||||
for d in repos:
|
||||
git = ["git", "--git-dir", d, "--work-tree", d]
|
||||
u = subprocess.run(git + ["log", "--oneline", "HEAD", "--branches", "--not", "--remotes"],
|
||||
u = subprocess.run(git + ["log", "--oneline", *UNPUSHED_REVS],
|
||||
capture_output=True, text=True)
|
||||
if u.returncode != 0:
|
||||
errors.append(f"{d}: cannot list its unpushed commits ({u.stderr.strip() or u.returncode})")
|
||||
|
||||
@@ -121,7 +121,7 @@ Every check runs first and changes nothing; a refusal exits 3:
|
||||
2. running sessions under it only with `--stop` (stopped normally, conversations kept);
|
||||
3. no open job of those sessions - never cancelled automatically, they may belong to a running loop (the delegate job of a session `--stop` stops is cancelled by stop itself);
|
||||
4. a clean tree, submodules included (`status --porcelain --ignore-submodules=none`);
|
||||
5. inside submodules: no modification, no commit that is on no remote (`HEAD` included - a submodule sits on a detached HEAD) and no stash (`refs/stash` is reachable from no branch and leaves the status clean) - checked in every repository under the worktree's `modules/`, including deinitialised submodules (their repository stays there) and nested ones; a repository that cannot be checked refuses.
|
||||
5. inside submodules: no modification, no commit reachable from `HEAD` or any local ref that is on no remote (a submodule sits on a detached HEAD; a commit kept only by a tag or a note is protected too) and no stash (`refs/stash` is reachable from no branch and leaves the status clean) - checked in every repository under the worktree's `modules/`, including deinitialised submodules (their repository stays there) and nested ones; a repository that cannot be checked refuses.
|
||||
|
||||
Then it stops the sessions, re-checks the jobs, runs checks 4 and 5 **again** (a session that is being stopped can still write, commit or stash - any change refuses with exit 3, sessions already stopped) and removes the worktree (the branch is kept): plain, or - when the worktree holds submodule state, which git refuses to remove otherwise - a **single `--force` justified by checks 4 and 5**. Do not run `git worktree remove --force` by hand on such a worktree: it deletes the worktree's own submodule repositories, unpushed commits included.
|
||||
|
||||
|
||||
@@ -10,8 +10,8 @@
|
||||
# session under it unless --stop; (3) no open job of those sessions (the
|
||||
# delegate job of a session --stop will stop is cancelled by stop itself);
|
||||
# (4) clean tree, submodules included; (5) submodules: nothing modified and no
|
||||
# commit that is on no remote (HEAD included - a submodule is on a detached
|
||||
# HEAD) and no stash - in every repository under the worktree's modules/,
|
||||
# commit reachable from HEAD or any local ref (tags included - a submodule is
|
||||
# on a detached HEAD) that is on no remote, and no stash - in every repository under the worktree's modules/,
|
||||
# initialised or not. (4) and (5) run again right before the removal, after
|
||||
# --stop: a session that is being stopped can still write, commit or stash.
|
||||
# Removal is plain, or - when the worktree holds submodule state, which
|
||||
|
||||
Reference in new issue
Block a user