Skip to content

feat(cluster): reap orphaned dev clusters on worktree removal - #3128

Open
daryllimyt wants to merge 1 commit into
mainfrom
daryl/cluster-orphan-reaper
Open

feat(cluster): reap orphaned dev clusters on worktree removal#3128
daryllimyt wants to merge 1 commit into
mainfrom
daryl/cluster-orphan-reaper

Conversation

@daryllimyt

@daryllimyt daryllimyt commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Problem

Removing a git worktree left its Docker Compose stack running indefinitely. The
containers keep their ports and volumes, and nothing ever cleans them up — a
stack was recently found still running 23h after its worktree had been pruned,
alongside ~80 orphaned volumes from long-dead worktrees.

Worse, an orphan cannot be traced back to an owner. WORKTREE_ID is derived from
the branch name, and git rev-parse --abbrev-ref HEAD returns HEAD in a
detached worktree, which lowercases to head. Every detached checkout therefore
collapses into a shared head namespace:

tracecat-head-1 … tracecat-head-5   # five different worktrees, indistinguishable

Worktrees for this repo are created by several tools, so a hook on any single one
of them cannot cover every removal path.

What changed

Ownership registry. cluster up records the owning worktree's absolute path
under the shared git common dir, so the entry outlives the worktree it describes.
TRACECAT_CLUSTER_REGISTRY_DIR overrides the location for tests.

cluster reap [--dry-run]. Tears down any cluster whose recorded worktree is
gone, including volumes left behind by an earlier down, unregisters the
portless alias, and clears the entry. It runs quietly at the start of up so
freed cluster numbers get reused.

cluster rm --all. Non-interactive teardown of every cluster owned by the
current checkout, wired to a worktrunk pre-remove hook. The plain rm path
could not be used: with more than one cluster it falls through to an interactive
selector that reads from /dev/tty, which has no TTY during hook execution, and
it would only ever have removed one of them.

cluster list now resolves clusters by project root and prints the owning
worktree, which addresses the "tracecat-head-1 tells you nothing" problem
directly.

Safety properties

  • Only a project whose registered worktree path is missing is ever destroyed.
  • Running clusters with no registry entry are reported and left alone, so
    stacks started before this lands are never touched.
  • No docker volume prune, system prune, or any unfiltered bulk removal.
  • The long-lived shared tracecat-infra stack is explicitly excluded.
  • The hook exits 0 when nothing is running, so it can never block removal.

Verification

  • 9/9 in tests/unit/test_cluster_script.py, shellcheck -S warning clean,
    bash -n clean, ruff and basedpyright clean.
  • Confirmed against a live Docker daemon with two unrelated clusters running:
    reap --dry-run correctly refused to touch them (unregistered → warn only).
  • Isolation checked in both directions behind a fake docker binary that logs
    instead of destroying: rm --all from one checkout performed zero
    mutations while foreign clusters were running, and correctly targeted only the
    cluster whose config files pointed at the current checkout.
  • Confirmed empirically that worktrunk runs hooks with cwd set to the worktree
    being removed, so the relative ./scripts/cluster path resolves.

Review guide

  • scripts/cluster — start at reap_clusters, then the rm --all handler and
    the registry read/write helpers. The bulk of the remaining diff is the
    cluster-resolution refactor that makes list/auto-select key off project root
    instead of the branch-derived id; that is the part most worth a careful look.
  • One set -u subtlety: on macOS bash 3.2, "${arr[@]}" on an empty array is an
    unbound-variable error, so array expansions are guarded on ${#arr[@]}.

Notes

  • .config/wt.toml is tracked, so existing worktrees only pick up the
    pre-remove hook once their branch includes this commit. The reaper is the
    load-bearing half and works regardless.
  • The registry only covers clusters started after this lands; clusters already
    running stay unregistered (and safely skipped) until they are restarted.

Summary by cubic

Prevents orphaned dev clusters by recording their owning worktree and adding an automatic reaper. Also adds a non-interactive cleanup on worktree removal and safer cluster selection across worktrees.

  • New Features

    • Ownership registry: up records WORKTREE_PATH, WORKTREE_ID, and cluster number under the shared git common dir (override with TRACECAT_CLUSTER_REGISTRY_DIR).
    • cluster reap [--dry-run]: Removes clusters whose recorded worktree is gone, deletes labelled volumes, unregisters any portless alias, and clears the registry entry. Runs quietly before up. Skips unregistered stacks and excludes tracecat-infra.
    • cluster rm --all: Non-interactive teardown of all clusters owned by the current checkout; used by the worktrunk pre-remove hook.
    • Safer selection/listing: A bare ./cluster lists all Tracecat clusters across worktrees and marks “this worktree.” Status commands (e.g. ports, ps) resolve across worktrees; mutating commands stay in this worktree unless you pass an explicit cluster number. Explicit numbers target the owning worktree.
  • Migration

    • No action required. Existing clusters without a registry entry are reported and left alone until restarted.
    • To clean old orphans once, run: ./scripts/cluster reap --dry-run then ./scripts/cluster reap.
    • The worktrunk pre-remove hook now calls ./scripts/cluster rm --all; it applies to worktrees once they include this change.

Written for commit 5711062. Summary will update on new commits.

Review in cubic

Removing a worktree left its Docker Compose stack running forever. The
cluster name could not be traced back to an owner either, because
WORKTREE_ID is derived from the branch name and `rev-parse --abbrev-ref
HEAD` returns `HEAD` in a detached worktree, collapsing every detached
checkout into a shared `head` namespace.

Record the owning worktree path at `up` time in a registry under the
shared git common dir, so the entry survives deletion of the worktree
itself. Add `cluster reap [--dry-run]`, which tears down any cluster
whose recorded worktree is gone, including volumes left behind by an
earlier `down`, then clears the entry. Reap runs quietly before `up` so
freed cluster numbers are reused.

Add a non-interactive `cluster rm --all` that removes every cluster owned
by the current checkout, and call it from a worktrunk `pre-remove` hook.
The plain `rm` path cannot be used here: with more than one cluster it
falls through to an interactive selector that reads from /dev/tty, which
has no TTY during hook execution, and it would only ever remove one of
them.

Registry lookups only ever destroy a project whose recorded worktree path
is missing. Running clusters with no registry entry are reported and left
alone, so stacks started before this change are never touched.

Also make the existing cluster script tests derive the expected worktree
id instead of hardcoding `main`, so the suite passes from a linked
worktree rather than only from the primary checkout.
@daryllimyt daryllimyt added enhancement New feature or request infrastructure Infra updates and fixes tests Changes to unit and integration tests labels Jul 24, 2026
@zeropath-ai

zeropath-ai Bot commented Jul 24, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 5711062.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► .config/wt.toml
Add pre-remove hook to tear down clusters via cluster script
Enhancement ► scripts/cluster
Major rewrite with extensive new functionality for cluster lifecycle, registry, port management, and cross-worktree behavior
Enhancement ► tests/unit/test_cluster_script.py
Add unit tests for cluster script behavior and registry interactions

@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: 57110629cd

ℹ️ 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 .config/wt.toml
# `rm --all` is non-interactive and exits 0 when nothing is running, so it can
# never block worktree removal.
[pre-remove]
cluster = "./scripts/cluster rm --all || true"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not delete volumes from the pre-remove hook

Whenever a developer removes a worktree with wt, this hook invokes rm --all, whose implementation runs docker compose ... down --volumes without asking whether the worktree's database and other persisted state may be destroyed. Worktree removal is not explicit confirmation of volume data loss, so keep the hook non-destructive or require a separate confirmed cleanup action.

AGENTS.md reference: AGENTS.md:L65-L67

Useful? React with 👍 / 👎.

Comment thread scripts/cluster
# Quietly reclaim orphaned registered clusters before allocating a number.
# Cleanup failures must never prevent a new cluster from starting.
if [[ "$COMMAND" == "up" ]]; then
reap_clusters false true || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make startup orphan reaping opt-in

Every cluster up now silently calls the non-dry reaper, which executes down --volumes and explicit docker volume rm operations for registered clusters whose worktree is missing. Thus starting an unrelated development cluster can permanently erase an orphan's PostgreSQL and other persisted data without the user requesting cleanup or confirming data loss; restrict destructive reaping to an explicit, confirmed command.

AGENTS.md reference: AGENTS.md:L65-L67

Useful? React with 👍 / 👎.

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

Labels

enhancement New feature or request infrastructure Infra updates and fixes tests Changes to unit and integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant