Repository navigation
Add bdt cleanup: prune merged worktrees and orphaned pgdevkit test DBs - #22
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports a personal script's git-worktree + pgdevkit test-DB cleanup logic into bdt as a proper, tested subcommand: `bdt cleanup worktrees` prunes worktrees merged into main/test (and their pgdevkit test DB(s)), and `bdt cleanup orphaned-dbs` sweeps for test DBs whose worktree is already gone. Both are non-interactive (require --yes), matching `bdt issue delete`'s pattern since bdt is also invoked by AI-agent callers. Per-repo sibling/nested DB conventions (e.g. CCMT2's _onetrade, MDMApp's _akeneo/akeneo_editor) are configured via `[tool.bdt.worktree]` in that repo's own pyproject.toml rather than hardcoded here, so other repos can add their own later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd messaging - psql calls now fail with a friendly "not found on PATH" message (matching the existing git/az/gh pattern) instead of a raw FileNotFoundError. - --pg-user's "current OS user" default now actually resolves via getpass.getuser() when PGUSER/USER/LOGNAME are all unset, instead of silently substituting the literal role "postgres". - `bdt cleanup orphaned-dbs` now distinguishes "no repos found under root" from "repos scanned, no orphans found", matching `cleanup worktrees`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aersam
force-pushed
the
cleanup-worktrees-testdb
branch
from
September 14, 2026 17:14
0a447d4 to
aba4017
Compare
…ementing it testdb.py's slugify/workspace_db_name/expected_db_names and worktree.py's own live-worktree-vs-Postgres diffing reimplemented pgdevkit's naming algorithm "without ever importing pgdevkit itself" (per testdb.py's old module docstring) -- a drift risk called out explicitly when that code was written. pgdevkit 0.7.1 now exposes find_orphaned_dbs()/ workspace_db_names() directly, so this replaces the reimplementation with real calls into pgdevkit: - pgdevkit>=0.7.1 (with the [db] extra, needed to import pgdevkit.testdb at all -- see testdb.py's comment) is now a real dependency. Requires bumping devtools' own requires-python to >=3.14 (pgdevkit's own minimum) and the CI Python matrix accordingly. - [tool.bdt.worktree].db_sibling_suffixes is gone; that's now [tool.pgdevkit].extra_db_suffixes in the *consuming* repo (pgdevkit's own config key, folded into its own naming/orphan-detection). db_nested_projects stays bdt's own concern (pgdevkit has no notion of a wholly separate nested project); verified against the real MDMApp checkout that pgdevkit's load_config() falls back to the directory name when a nested project's own pyproject.toml has no [tool.pgdevkit] section at all -- exactly akeneo_editor's actual current state -- so no changes needed there for the nested lookup itself to work. - bdt cleanup orphaned-dbs no longer takes --remote (pgdevkit's orphan detection only cares about live-worktree existence, not merge status); its per-repo diffing is entirely pgdevkit.testdb.find_orphaned_dbs() now, fanned out across discovered repos (and configured nested projects) instead of bdt's own by-hand diff. - --pg-port/--pg-user now default to pgdevkit's own test-container port/user instead of Postgres' standard 5432/the OS user: dropping is still bdt's own psql-based DROP DATABASE (pgdevkit has no public "drop this arbitrary already-known name" call, and its plain clean_testdb() doesn't fold in extra_db_suffixes), but the listing half now always goes through pgdevkit's own PGDEVKIT_TESTDB_*-driven connection -- defaulting the two halves differently risked silently targeting different Postgres instances. - Fixed a latent bug surfaced along the way: has_pgdevkit_project (nee read_pgdevkit_project) treated an entirely empty but present [tool.pgdevkit] section as "no section" (empty dict is falsy); fixed to check for absence explicitly. - require_psql() (cli_tools.py) was dead code with testdb.py's _run_psql reimplementing the identical check inline; wired it in instead. Tests reworked throughout: what tested the removed naming/diffing reimplementation is gone; what's left covers bdt's own remaining logic (db_nested_projects discovery, caution-DB flagging, CLI option wiring) plus a few real (unmocked) calls into pgdevkit's naming for the nested-project nuance specifically, since that's the part most likely to silently drift again. Temporary: [tool.uv.sources] pins pgdevkit to its own testdb-cli-orphaned-followups branch, since 0.7.1 isn't on PyPI yet (that PR is open but unmerged). Remove once it's merged and published. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… branch /code-review flagged the branch-name pin as fragile: if testdb-cli-orphaned-followups gets force-pushed, rebased, or deleted (e.g. after its PR merges and the branch is cleaned up), uv sync/CI here would start failing or silently resolve different code with no way to reproduce what was actually reviewed/tested. Pinning to the exact commit reviewed here avoids that; still temporary until pgdevkit 0.7.1 publishes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… DROP, SQL escaping, and caution-check slugging
Fixes four issues surfaced by an independent code-review pass on the
pgdevkit-delegation refactor, none of which the reviewing agent's own
"finding fixed" summary actually addressed in the pushed code:
- `--pg-user`'s envvar list included USER/LOGNAME, which are set on
virtually every real shell, so `pg_user or pgdevkit_constants.USER`'s
fallback to pgdevkit's own test-container user never fired in practice.
Now only PGUSER feeds it.
- `drop_database`/`_run_psql` never passed -h or PGPASSWORD, so it defaulted
to unix-socket peer auth instead of the TCP+password path pgdevkit's own
find_orphaned_dbs()/workspace_db_names() use -- the listing and dropping
halves could target different auth paths (or just fail outright). Added
a --pg-host option (default pgdevkit's own HOST) and PGPASSWORD env.
- `drop_database` interpolated the DB name into `DROP DATABASE IF EXISTS
"{name}"` unescaped; a name containing a literal `"` (from a live
pg_database listing, not a validated slug) could break out of the quoted
identifier. Now escapes embedded `"` per Postgres identifier quoting.
- `is_caution_db` compared db_name's prefix against the raw project name,
but db names are always pgdevkit's *slugified* name -- a project name
with uppercase/special characters (e.g. "MDMApp") never matched its own
DBs ("mdmapp_main"), so the caution flag silently never fired for them.
All four verified against the real sandbox Postgres, not just mocks.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rce pin pgdevkit#35 (workspace_db_names()) merged and 0.7.1 is now published on PyPI, so the commit-SHA [tool.uv.sources] override this PR added as a stopgap is no longer needed -- the >=0.7.1 constraint now resolves for real. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aersam
marked this pull request as ready for review
September 14, 2026 18:51
…ocal hosts `bdt cleanup db` drops the current worktree's own pgdevkit test DB(s) in place (unlike `cleanup worktrees`, it never touches the worktree itself), guarded by an interactive confirmation unless --confirm is passed. Every cleanup command that drops a database (`worktrees`, `orphaned-dbs`, `db`) now also requires an unconditional, un-overridable interactive confirmation whenever --pg-host isn't localhost/127.0.0.1/::1 -- --yes/ --confirm alone can no longer drop anything on a remote/shared Postgres instance. Also bumps the package version to 0.17.0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiiWxLhjWygyfEs7H2WnVr
…ktree + its db Unlike `cleanup worktrees` (bulk, merged-into-main/test only), this tears down one specific still-live worktree on demand -- run from inside it, default path '.' -- plus its pgdevkit test DB(s), gated by the same --confirm/interactive-prompt pattern as `cleanup db`. Refuses the main checkout, a protected branch, a locked or dirty worktree. Removal reuses `remove_worktree()`, so a worktree containing submodules is handled the same way as the bulk command (git's blanket refusal retried once with --force) -- covered by a real submodule integration test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiiWxLhjWygyfEs7H2WnVr
…back claim Documents the two new single-worktree cleanup commands and the non-local --pg-host confirmation gate, and corrects a stale claim that --pg-user falls back to \$USER/\$LOGNAME -- it deliberately never did (found by /code-review). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiiWxLhjWygyfEs7H2WnVr
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.
Summary
bdt cleanup worktreescommand: finds git worktrees merged into main/test across a directory tree and offers removal, dropping each worktree's pgdevkit test DB(s) along with itbdt cleanup orphaned-dbscommand: finds pgdevkit-managed test DBs whose worktree no longer exists, and offers removalbdt cleanup dbcommand: drops the current worktree's own pgdevkit test DB(s) in place, without touching the worktree itself -- run from inside the worktree, gated by an interactive confirmation unless--confirmis passedbdt cleanup worktreecommand: removes one specific still-live worktree (regardless of merge status) plus its pgdevkit test DB(s) in one go -- also run from inside it, same--confirm/interactive-prompt gate; refuses the main checkout, a protected branch (main/test), a locked worktree, or a dirty one (submodules included); removal reuses the same--forceretry ascleanup worktreesso a worktree containing submodules is handled correctly, covered by a real submodule integration testworktrees,orphaned-dbs,db,worktree): a non-local--pg-host(anything other thanlocalhost/127.0.0.1/::1) now always requires a separate interactive "type yes" confirmation that no flag can bypass, not even--yes/--confirm-- a script/agent can never drop a database on a shared/remote Postgres instance without a human actually confirming it. Non-interactive input (no stdin, e.g. CI) fails closed.workspace_db_names()/find_orphaned_dbs(), see Add find_orphaned_dbs() / clean_testdb(orphaned=True) pgdevkit#34/Enforce conventional commits #35) instead of reimplementing the naming algorithm here -- the one thing this PR started out doing that we specifically wanted to avoid duplicatingdb_nested_projects(e.g. MDMApp'sakeneo_editor/sharing its parent worktree's branch) stays bdt's own concern via[tool.bdt.worktree], since pgdevkit has no notion of a wholly separate nested projectworktrees,orphaned-dbs) stay non-interactive by design (--yesto act, otherwise dry-run print), matching this codebase's existingbdt issue delete --yesconvention; the two single-target commands (db,worktree) instead default to an interactive confirmation, since they're meant to be run by hand against one worktree at a time (--confirmopts back into non-interactive use)Notes
[tool.uv.sources]) until pgdevkit#35 (workspace_db_names()) merges and ships as 0.7.1 on PyPI -- remove that override once it does.requires-pythonmoved>=3.11to>=3.14since pgdevkit requires 3.14; pgdevkit'stestdbmodule also unconditionally pulls in itsdbextra's heavy dependency chain (psycopg-pool/pydantic) even though this PR only needs the DB-naming/testdb pieces -- noted upstream, not fixed here.Test plan
uv run -m pytest -q), including a real git submodule integration test forcleanup worktreeruff checkandty checkclean/code-reviewpasses addressed, including a broken--pg-userenv fallback, missing host/password on the DROP step, unescaped SQL interpolation of DB names, and a slugification mismatch in the caution-DB heuristic -- all verified against this sandbox's real Postgres, not just mocks🤖 Generated with Claude Code