Skip to content

fix(worktree): do not report EXIT_OK when .git is absent (PP-ew10) - #2094

Open
timothyfroehlich wants to merge 4 commits into
mainfrom
claude/vibrant-faraday-4v5h67
Open

fix(worktree): do not report EXIT_OK when .git is absent (PP-ew10)#2094
timothyfroehlich wants to merge 4 commits into
mainfrom
claude/vibrant-faraday-4v5h67

Conversation

@timothyfroehlich

@timothyfroehlich timothyfroehlich commented Sep 10, 2026

Copy link
Copy Markdown
Owner

What

Closes the remaining worktree_cleanup.py paths that could exit 0 without evidence (PP-ew10).

  • When a worktree directory has no .git marker — partial removal, rm -rf without the hook, or a Web-sandbox session (the PP-qlzu path) — cleanup cannot derive a branch or target a Supabase project_id. It used to skip Docker, deallocate the slot, and return EXIT_OK despite never querying volumes.
  • During adoption review, the same invariant gap was found when .git exists but git rev-parse cannot report a branch: cleanup could likewise skip Docker and continue toward a successful teardown.

Fix

  • Record the volume state as UNKNOWN in the .git-absent path, returning EXIT_DOCKER_UNKNOWN (4). The diagnostic now gives the complete recovery sequence: prune the stale Git registration, then run worktree_orphan_sweep.py --apply. The slot is still deallocated.
  • If .git exists but the branch cannot be read, fail closed with EXIT_FAILED and retain both the worktree and slot manifest entry for investigation. Nothing is queried or removed.
  • Keep the existing .git-present Docker-unavailable wording unchanged, and give the .git-absent path its own honest summary.
  • Broaden the EXIT_DOCKER_UNKNOWN documentation to cover volumes that were never queried as well as volumes that could not be enumerated.

Tests

  • test_missing_git_marker_is_incomplete_not_success proves the PP-qlzu path returns 4, deallocates only the slot, never invokes Docker or Git removal, carries both recovery steps, and never prints a clean-success message.
  • test_unreadable_branch_refuses_cleanup_instead_of_reporting_success proves branch lookup failure returns 1 while retaining the worktree and slot and avoiding Docker/removal calls.
  • pnpm run check passes (pre-existing warnings only).
  • pnpm run check:python passes: 580 tests.
  • scripts/tests/test_worktree_cleanup.py passes: 43 tests.

Scope / follow-up

No behavior change to healthy cleanup, missing-Docker, Docker-unavailable, stale-target, or main-worktree paths. One adjacent cosmetic inaccuracy remains deliberately out of scope: worktree_reap.py's exit-4 message says "removed," which is slightly off for the slot-only .git-absent path; it is recorded on the bead.

Bead: PP-ew10.

This PR was opened by the unattended nightly bead routine and adopted by Codex-PRBurndown2 for lifecycle completion.

Generated with Claude Code

https://claude.ai/code/session_01JdLKkQtCQRzaTa3gJZnkAu


Generated by Claude Code

When a worktree directory has no `.git` marker (partial removal, `rm -rf`
without the hook, or a Web-sandbox session — the PP-qlzu path),
`cleanup_worktree()` skipped the whole Docker/Supabase phase because it
can't derive a branch, and therefore can't target a Supabase project_id.
It then deallocated the slot and returned EXIT_OK ("Cleaned up worktree")
— a false success, since the volumes were never queried at all. That is
the same "success without evidence" shape as PP-omz3 and PP-3w4g, reached
from a third direction: the WorktreeRemove hook reads that 0 as "clean"
while the Supabase volumes may still be on disk.

Record the volume state as UNKNOWN in that path so the run returns
EXIT_DOCKER_UNKNOWN (4) with the `worktree_orphan_sweep.py --apply` hint,
matching the honest-reporting contract the exit code already describes.
The slot is still deallocated, so the sweep (which matches on the Docker
label, not the manifest) remains the backstop — a delayed leak, not a
permanent one. The final message is split so the .git-absent case tells
its own story rather than borrowing the "once Docker is reachable" wording,
which is wrong here (no worktree was git-removed, and Docker was never
contacted).

Adds a regression test alongside the PP-omz3/PP-3w4g cases; verified it
fails on the pre-fix source (returned 0, now 4).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JdLKkQtCQRzaTa3gJZnkAu
@timothyfroehlich timothyfroehlich added the ownerless Opened by an unattended agent (routine, Dependabot, Renovate); needs pickup label Sep 10, 2026 — with Claude
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
pin-point Ready Ready Preview Sep 10, 2026 3:16pm UTC

Request Review

@timothyfroehlich timothyfroehlich removed the ownerless Opened by an unattended agent (routine, Dependabot, Renovate); needs pickup label Sep 10, 2026
@timothyfroehlich timothyfroehlich changed the title fix(worktree): don't report EXIT_OK when .git is absent (PP-ew10) fix(worktree): do not report EXIT_OK when .git is absent (PP-ew10) Sep 10, 2026
@timothyfroehlich

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-10T14:11:32.533652Z c48d5d5 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c48d5d5a31

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/worktree_cleanup.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants