Skip to content
Open
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
60 changes: 60 additions & 0 deletions scripts/tests/test_worktree_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,30 @@ def test_healthy_teardown_removes_volumes_and_succeeds(
assert stub.calls_of("worktree_remove")[0][:3] == ["git", "-C", "/repo"]
assert stub.calls_of("worktree_prune")[0][:3] == ["git", "-C", "/repo"]

def test_unreadable_branch_refuses_cleanup_instead_of_reporting_success(
self,
monkeypatch: pytest.MonkeyPatch,
capsys: pytest.CaptureFixture[str],
fake_worktree: Path,
deallocated: list[str],
) -> None:
"""A `.git` marker without a readable branch is not cleanup evidence."""
stub = install(
monkeypatch,
RunStub(rev_parse=(128, "", "fatal: invalid gitfile format")),
)

exit_code = _run_main(monkeypatch, fake_worktree)

err = capsys.readouterr().err
assert exit_code == cleanup.EXIT_FAILED
assert "Failed to derive a branch" in err
assert "keeping the worktree and slot" in err
assert "Cleaned up worktree" not in err
assert stub.calls_of("volume_ls") == []
assert stub.calls_of("worktree_remove") == []
assert deallocated == []

def test_renamed_branch_still_tears_down_the_pinned_project(
self,
monkeypatch: pytest.MonkeyPatch,
Expand Down Expand Up @@ -617,6 +641,42 @@ def test_incomplete_docker_cleanup_does_not_report_success(
assert stub.calls_of("worktree_remove") != []
assert deallocated == [str(fake_worktree)]

def test_missing_git_marker_is_incomplete_not_success(
self,
monkeypatch: pytest.MonkeyPatch,
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
deallocated: list[str],
) -> None:
"""PP-ew10: `.git` absent means the volumes were never queried.

Partial removal, `rm -rf` without the hook, or a Web-sandbox session
leaves a directory with no `.git` marker (the PP-qlzu path). Without a
branch there is no project_id, so Docker is never asked — the volume state
is UNKNOWN. A run that never touched Docker used to exit 0 ("Cleaned up
worktree") anyway; it must instead read as incomplete, the same
success-without-evidence class as PP-omz3 and PP-3w4g.
"""
worktree = (tmp_path / "agent-alpha").resolve()
worktree.mkdir() # exists on disk, but with no `.git` inside
stub = install(monkeypatch, RunStub())

exit_code = _run_main(monkeypatch, worktree)

err = capsys.readouterr().err
assert exit_code == cleanup.EXIT_DOCKER_UNKNOWN
# The slot is still reclaimed, so the sweep can find any leaked volumes by
# their Docker label — a delayed leak, not a permanent one.
assert deallocated == [str(worktree)]
assert "git worktree prune" in err
assert "worktree_orphan_sweep.py --apply" in err
assert "INCOMPLETE" in err
assert "no .git marker" in err
assert "Cleaned up worktree" not in err
# No branch, so no Supabase/Docker phase ran, and no git worktree removal
# either — nothing was queried or reclaimed beyond the slot deallocation.
assert stub.calls == []

def test_docker_not_installed_still_reports_success(
self,
monkeypatch: pytest.MonkeyPatch,
Expand Down
77 changes: 60 additions & 17 deletions scripts/worktree_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
`worktree_orphan_sweep.py`). A failed `docker volume ls` used to collapse
into `volumes = []`, indistinguishable from "no volumes exist", after which
the worktree was removed and the slot deallocated while the volumes leaked.
An unqueryable Docker now yields an explicit unknown and a non-zero exit.
An unqueryable Docker now yields an explicit unknown and a non-zero exit — as
does a `.git`-less worktree, whose missing branch leaves no project_id to query
with, so the volumes are never counted at all (PP-ew10, the PP-qlzu path).

Targeting matters as much as reporting: the Supabase project id comes from the
worktree's pinned `supabase/config.toml`, not from its current branch name
Expand Down Expand Up @@ -58,8 +60,12 @@
#: manifest entry, a git worktree registration, or an unreadable source of
#: truth for either). Nothing was reclaimed, so this must not read as success.
EXIT_STALE_TARGET = 3
#: Cleanup ran but Docker could not be enumerated, so the Supabase volumes were
#: neither counted nor removed. `worktree_orphan_sweep.py` is the backstop.
#: Cleanup ran but the Supabase volumes were neither counted nor removed —
#: either Docker could not be enumerated, or it was never queried because no
#: branch yielded a project_id (the `.git`-less PP-qlzu path, PP-ew10). Either
#: way `worktree_orphan_sweep.py` is the backstop. The `.git`-less case must
#: prune any stale Git registration first so the sweep no longer treats that
#: project's retained config as active.
#: NOTE: `worktree_orphan_sweep.py` spells its equivalent `EXIT_DOCKER_UNKNOWN = 1`
#: — the same name with a different value, deliberately. In that script 1 is free;
#: here it already means "failed", and callers distinguish these codes per script.
Expand Down Expand Up @@ -365,17 +371,27 @@ def cleanup_worktree(worktree_path: Path) -> int:
# Claude in Web sandbox sessions), we can't derive the branch and therefore
# can't safely target the Supabase project_id. Skip the Docker/Supabase
# phase but still deallocate the slot — otherwise the manifest entry leaks
# forever. worktree_orphan_sweep.py picks up any leaked Docker resources.
# forever. After any stale Git registration is pruned,
# worktree_orphan_sweep.py picks up any leaked Docker resources.
#
# PP-ew10: skipping that phase means the volumes were never queried, so their
# state is UNKNOWN — the same "success without evidence" shape as PP-omz3 and
# PP-3w4g, reached from a third direction. Record it as unknown here so the run
# returns EXIT_DOCKER_UNKNOWN and points at the sweep, instead of a false
# EXIT_OK for a teardown that never touched Docker.
git_marker_present = git_marker.is_file()

volumes_unknown_reason: str | None = None
if not git_marker_present:
volumes_unknown_reason = (
"no .git marker, so no branch to derive the Supabase project_id"
)
print(
f"Warning: {worktree_path} has no .git marker — skipping Supabase/Docker "
"cleanup (no branch to derive project_id from); deallocating slot only.",
file=sys.stderr,
)

volumes_unknown_reason: str | None = None

branch = ""
if git_marker_present:
try:
Expand All @@ -386,9 +402,21 @@ def cleanup_worktree(worktree_path: Path) -> int:
check=True,
)
branch = result.stdout.strip()
except subprocess.CalledProcessError:
except (OSError, subprocess.CalledProcessError):
branch = ""

# A present marker is not enough evidence to continue: without a branch,
# cleanup cannot target this worktree's Supabase project. Keep both the
# worktree and its slot so a transient or corrupt Git state cannot turn
# into another successful-looking resource leak.
if not branch:
print(
f"Failed to derive a branch for {worktree_path} — refusing cleanup "
"and keeping the worktree and slot manifest entry for investigation.",
file=sys.stderr,
)
return EXIT_FAILED

if branch:
project_id = resolve_project_id(worktree_path, branch)

Expand Down Expand Up @@ -515,16 +543,31 @@ def cleanup_worktree(worktree_path: Path) -> int:
deallocate_slot(str(worktree_path))

if volumes_unknown_reason is not None:
# The worktree and slot are gone, so the sweep (which matches on the
# Docker label, not the manifest) will still find any leaked volumes —
# a delayed leak, not a permanent one. But this run did NOT finish the
# job, so it must not report success to the WorktreeRemove hook.
print(
f"Removed worktree {worktree_path} and deallocated its slot, but Supabase "
"volume state was UNKNOWN — cleanup is INCOMPLETE. Run "
f"`{SWEEP_HINT}` once Docker is reachable.",
file=sys.stderr,
)
# The slot is deallocated and (when we had a .git marker) the worktree
# removed, so the sweep — which matches on the Docker label, not the
# manifest — will still find any leaked volumes: a delayed leak, not a
# permanent one. But this run did NOT finish the job, so it must not
# report success to the WorktreeRemove hook.
if git_marker_present:
print(
f"Removed worktree {worktree_path} and deallocated its slot, but Supabase "
"volume state was UNKNOWN — cleanup is INCOMPLETE. Run "
f"`{SWEEP_HINT}` once Docker is reachable.",
file=sys.stderr,
)
else:
# PP-ew10: the .git-absent path never queried Docker, so word it for
# that case rather than borrowing the "once Docker is reachable" story.
# We never looked, so we can't claim volumes exist — only that any that
# do are unreclaimed. The reason string set above is surfaced here.
print(
f"Deallocated the slot for {worktree_path} but never queried Docker "
f"({volumes_unknown_reason}) — cleanup is INCOMPLETE; any Supabase "
"volumes that exist are still on disk. First remove any stale Git "
"registration with `git worktree prune`, then reclaim them with "
f"`{SWEEP_HINT}`.",
file=sys.stderr,
)
return EXIT_DOCKER_UNKNOWN

print(f"Cleaned up worktree: {worktree_path}", file=sys.stderr)
Expand Down
Loading