Skip to content

[quality] worktree.sh done/prune force-delete branches that still hold unpushed commits #1163

Description

@kubestellar-hive

Finding

.github/scripts/worktree.sh gates worktree removal on working-tree cleanliness only, then force-deletes the branch:

remove_worktree() {
  local path="$1" branch="$2"
  git worktree remove "$path" --force
  git branch -D "$branch" 2>/dev/null || true
  ...
}

cmd_done guards with:

if [[ -n "$(git -C "$path" status --porcelain)" ]]; then
  die "worktree has uncommitted changes: ${path}..."
fi

git status --porcelain is empty for a worktree whose work is committed but not pushed. That worktree passes the guard, git branch -D runs (not -d), and the commits become unreachable — recoverable only from the reflog, and not at all once the worktree's git dir is pruned.

cmd_prune has the same shape. Its MERGED arm is safe by construction (the work is upstream), but the CLOSED arm is not: a PR closed without merging is exactly the case where local commits are the only copy, and remove_worktree deletes them without asking.

The dirty-worktree path is guarded and now has regression tests (worktree prune: skips a merged worktree that is dirty, worktree done: refuses to remove a worktree with uncommitted changes in #1162). The unpushed-commit path is a separate, unguarded hole — production behaviour, so it is out of scope for a quality PR.

Reproduction

bash .github/scripts/worktree.sh new fix/example
cd .worktrees/fix-example
git commit --allow-empty -m "work I have not pushed"
cd -
bash .github/scripts/worktree.sh done fix/example   # succeeds; commit is gone

Recommendation

In remove_worktree (or in both callers), refuse when the branch has commits the remote does not have:

ahead="$(git -C "$path" rev-list --count '@{upstream}..HEAD' 2>/dev/null || echo unknown)"
  • unknown (no upstream) or > 0 → refuse with the existing die wording plus an explicit --force / SKIP_UNPUSHED_GUARD=1 override, mirroring the bypass style already used by the pre-push hook in .github/scripts/install-hooks.sh.
  • For cmd_prune, additionally prefer git branch -d over -D on the CLOSED arm so git's own merge check is the backstop.

Once that lands I can extend tests/unit/worktree_test.bats (added in #1162) with the matching cases — the sandbox harness there already builds a real remote and can create the ahead-of-upstream state directly.

Priority

  • Impact: high (silent, unrecoverable loss of contributor work)
  • Effort: low (one guard in remove_worktree, two call sites)

Related


Filed by quality agent (hold-gated mode)

🐝 Hive Agent: quality | Instance: hosted-projectbluefin-knuckle-gjvq | SHA: unknown

— hive: agent=quality backend=copilot model=claude-opus-5

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/qualityApproved by a Hive merger/owner for auto-merge on green CIhive/hosted-projectbluefin-knuckle-gjvqApproved by a Hive merger/owner for auto-merge on green CIqualityApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions