Skip to content

afx cleanup deletes an unmerged builder branch on the orphan path, unlike the path #123 fixed #126

Description

@pseudoseed

Found by running the cleanup, not by reading it.

#123 made --force on an unmerged live-row builder remove the directory but keep the branch:

await removeOrphanWorktree(workspaceRoot, worktreePath, force, { deleteBranch: merged });

That is the property that makes --force safe to hand to someone in a hurry — 900 MB of directory goes, committed work stays recoverable.

The orphan path does not pass the option:

// cleanupOrphan
await removeOrphanWorktree(config.workspaceRoot, worktreePath, force);

and removeOrphanWorktree defaults it on:

const deleteBranch = options?.deleteBranch !== false;

So afx cleanup -p <id> --force against a row-less worktree deletes the local branch whether or not it is merged, which is the exact behaviour CMAP flagged on #123 and the builder fixed — on one of the two paths.

Observed

Cleaning 17 orphans here, 12 of which reported Branch is not merged, took the local builder/* branch count from 19 to 2.

Nothing was lost: all 17 branches exist on origin, and I verified the specific worktree HEADs are reachable there —

pir-4          aee79bb2d   reachable from origin/builder/pir-4
spir-52        53bb76946   reachable from origin/builder/spir-52
spir-83        583e8d715   reachable from origin/builder/spir-83
air-78         9e6f845ca   reachable from origin/builder/air-78
air-98         b2d506721   reachable from origin/builder/air-98
experiment-39  6fec5e348   reachable from origin/builder/experiment-39

— so this was recoverable because every branch had been pushed. A builder that never pushed would have lost its commits outright, and the message would still have said Worktree removed.

Fix

Pass the same option the live-row path passes:

const merged = await isWorktreeMerged(config.workspaceRoot, worktreePath);
await removeOrphanWorktree(config.workspaceRoot, worktreePath, force, { deleteBranch: merged });

removeOrphanWorktree already computes merged internally for its own refusal check, so the cleaner fix is probably to have it decide deleteBranch from that rather than take it as a parameter at all — one source of truth instead of two call sites that must remember.

Worth a test that asserts the branch survives an unmerged --force on the orphan path specifically. #123's test covers the live-row path and passes, which is why this got through.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/towerTower, afx, terminals, messaging

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions