chore(prune): add --adopt and --prune-detached, fix NameError in main() - #853
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved safety and correctness issues remain in detached-worktree pruning and branch adoption.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds opt-in branch adoption and detached-worktree pruning, while fixing stale variable references and ancestry checks.
Changes:
- Adds
--adoptvalidation and classification. - Adds
--prune-detachedwith--onlysupport. - Fixes reporting and centralizes ancestry checks.
File summaries
| File | Review findings |
|---|---|
scripts/prune_agent_worktrees.py |
Critical (3 votes): Detached pruning can delete uncommitted work. Moderate (3 votes): The current branch is not protected during adoption validation. Nit (3 votes): Dry-run wording incorrectly says entries are pruned. Moderate (1 vote): Abbreviated SHAs can cause ancestry checks to fail. |
Review details
Suppressed comments (1)
scripts/prune_agent_worktrees.py:264
- The detached entries passed here contain only the first eight characters of each HEAD (see
scan_worktrees()), which is a display abbreviation rather than a guaranteed object name. If that abbreviation is ambiguous,merge-base --is-ancestorfails and this already-landed detached worktree is incorrectly skipped. Keep the full SHA for ancestry checks and abbreviate it only when printing.
for path, sha in detached:
if not in_main(sha):
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+267
to
+269
| out = subprocess.run( | ||
| ["git", "worktree", "remove", "--force", str(path)], | ||
| cwd=REPO_ROOT, |
| if unknown: | ||
| print(f"error: --adopt names no such local branch: {', '.join(unknown)}") | ||
| return 2 | ||
| protected = [b for b in args.adopt if b in PROTECTED] |
Comment on lines
215
to
+217
| if detached: | ||
| print(f"\n=== DETACHED worktrees ({len(detached)}) - not pruned, review by hand ===") | ||
| note = "pruned below" if prune_detached else "not pruned, review by hand" | ||
| print(f"\n=== DETACHED worktrees ({len(detached)}) - {note} ===") |
The initial script deliberately never touched UNLANDED branches, which left no path for the abandoned-experiment case: decafclaw still had 5 branches holding work that landed via a sibling branch (verified by artifact, not by ancestry) plus 2 detached worktrees sitting at commits already in origin/main. - --adopt BRANCH prunes one named UNLANDED branch. It takes a branch name rather than a blanket --include-unlanded switch, so abandoning work stays a per-branch human decision and cannot happen in bulk by accident. Mistyped or protected names exit 2 instead of silently pruning nothing. - --prune-detached removes detached worktrees whose HEAD is already an ancestor of origin/main (no unique commits by definition). Detached checkouts with unmerged HEADs are still skipped and reported. Also fixes one bug in the original: the detached report ignored --only. The origin/main ancestor test is now one in_main() helper shared by the report and the detached pruner, so they agree by construction. Defaults are unchanged: a plain run still reports 5 UNLANDED / 2 detached and prunes nothing. Verified dry-run and both guard paths (exit 2).
lmorchard
force-pushed
the
chore/prune-adopt
branch
from
September 15, 2026 23:34
0e7f6a6 to
be3be9f
Compare
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #852. The prune script deliberately never touched UNLANDED
branches, which left no path for the abandoned-experiment case -- so the last
5 branches and 2 detached worktrees in this clone had nowhere to go.
Two escape hatches, both off by default
--adopt BRANCHprunes one named UNLANDED branch. It takes a branch namerather than a blanket
--include-unlandedswitch, so abandoning work stays aper-branch human decision and cannot happen in bulk by accident. A mistyped or
protected name exits 2 rather than silently pruning nothing.
--prune-detachedremoves detached worktrees whose HEAD is already anancestor of
origin/main-- those hold no unique commits by definition.Detached checkouts with unmerged HEADs are still skipped and reported.
One bug in the original
The detached-worktree report ignored
--only, so a filtered run still printedevery detached entry. Fixed here.
(An earlier draft of this description claimed
main()shipped aNameErrorin#852. That was wrong -- #852 as merged is internally consistent and its
make prune-worktreesrun pruned 25 branches successfully. Thelen(landed)mismatch was introduced by the
landed->prunablerename in this branchand fixed before the commit.)
The
origin/mainancestor test is now a singlein_main()helper shared by thereport and the detached pruner, so the two agree by construction instead of by
duplicated subprocess calls.
Verification
Defaults unchanged -- a plain run still reports 5 UNLANDED / 2 detached and
prunes nothing (
0 branch(es) would be pruned). Both guard paths exit 2(mistyped
--adopt pr-80, and--adopt main).--only issue-31-wtnownarrows the detached list from 2 to 1. Dry run with all five adopted correctly
reclassifies them ADOPTED / 0 UNLANDED.
ruff checkandruff formatclean.Recovery record
These commits have no remote copy once the branches are deleted, so recording
them here deliberately:
Detached worktree HEADs, both already in
origin/main:f84907d3(
issue-31-wt),e2dec038(.Codex/worktrees/fix-779).Each branch's work was verified present in main via a sibling branch:
#779 ->
src/decafclaw/inbox.py, #777 ->tests/test_tool_output_store.py,#757 ->
repsin the eval runner.feat-788-textual-tuiis an abandonedPython-Textual alternative to the ink TUI that shipped in #847, and is also on
origin.
fix-confirmation-widgetswas pushed to origin in #852's session, soit survives regardless.