Skip to content

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

Merged
timothyfroehlich merged 5 commits into
mainfrom
claude/vibrant-faraday-4v5h67
Sep 11, 2026
Merged

fix(worktree): do not report EXIT_OK when .git is absent (PP-ew10)#2094
timothyfroehlich merged 5 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: inspect and remove the residual directory after preserving anything needed, 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 9:03pm 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-10T21:10:57.564297Z d529860 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
@timothyfroehlich

Copy link
Copy Markdown
Owner Author

@codex review

@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: 526ca721db

ℹ️ 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
@timothyfroehlich

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: d529860d97

ℹ️ 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".

@timothyfroehlich timothyfroehlich added the ready-for-review PR passed CI and has no unresolved review comments label Sep 10, 2026
@timothyfroehlich
timothyfroehlich merged commit 4923a1f into main Sep 11, 2026
22 checks passed
@timothyfroehlich
timothyfroehlich deleted the claude/vibrant-faraday-4v5h67 branch September 11, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review PR passed CI and has no unresolved review comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants