v0.4.0: stale branch detection and pruning - #4
Open
sven42xyz wants to merge 10 commits into
Open
Conversation
Detects two classes of stale local branches via libgit2 in phase 1: gone — upstream tracking ref no longer exists merged — fully reachable from the repo's default branch (main/master) Output is a per-repo grouped listing; --prune and age filter follow in subsequent commits (see #3). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
--prune deletes the listed stale branches via libgit2's git_branch_delete. --yes skips the confirmation prompt. Safety: * STR_MERGED branches are always safe to delete (already verified merged). * STR_GONE branches are re-checked against the default branch tip; if they carry local commits not reachable from default, deletion is refused with a clear "unmerged local commits" marker. Per-branch outcome (deleted/refused/error) is rendered by display.c via the new print_prune_results() function, followed by a summary line. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Covers: * no stale branches → "No stale branches found" * merged branch listed * gone-upstream branch listed (uses bare+clone+push-delete) * invalid combinations rejected (stale + -s, --prune without stale) * --prune --yes deletes merged branch * --prune --yes refuses gone+unmerged (data-loss guard) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new reason STR_SQUASHED is reported when a branch's cumulative diff (merge-base → tip) matches the patch-id of any commit on the default branch. This catches the common GitHub/GitLab "Squash & merge" flow where the per-branch commits collapse into a single mainline commit and git_graph_descendant_of can no longer recognise the branch as merged. For each repo we walk up to 500 commits of the default branch once, compute their first-parent patch-ids, and compare each candidate branch's single cumulative patch-id against that set. Branches with unmerged work past the squash point keep their unique patch-id and correctly do not match. Squashed branches are treated as safe to delete in --prune. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new ~/.gitlsrc key 'protected_branches=develop,staging,...' lists local branch names that must never be flagged by 'stale' detection, regardless of merge or upstream status. Parsing mirrors skip_dirs (comma-separated, cap of 64). Additionally main and master are always implicitly protected. The existing default-branch and current-HEAD skips already covered the common case, but a repo using master as its default could still have a separate 'main' branch flagged after a non-FF merge into master. Treating both names as reserved closes that gap. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Restricts the listing (and therefore --prune) to branches whose tip commit is older than the given duration. Useful for "list stale branches that have been dormant for at least a month" workflows where recently-merged branches may still be active locally. Duration suffixes: s, h, d, w, m (≈30d), y (≈365d). Examples: gitls stale --older-than 30d gitls stale --prune --yes --older-than 6m The filter applies before classification, so no patch-id work is done for branches that are too young to be candidates anyway. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a dedicated "Stale branches" section to the README covering the three detection categories (gone / merged / squash), the safety rules applied by --prune (current HEAD, default branch, main/master and protected_branches always excluded; gone+unmerged refused), and the --older-than age filter with its supported duration suffixes. Also wires protected_branches into the Config table and the example gitlsrc, and brings the synopsis / examples in line with the new subcommand surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a ChangesStale Branch Detection and Pruning
sequenceDiagram
participant User
participant CLI as main.c
participant Detector as repo.c
participant Pruner as repo.c
participant Display as display.c
participant Git as libgit2
User->>CLI: gitls stale [--prune] [--older-than N]
CLI->>Detector: process_repo_local() calls fill_stale_branches()
Detector->>Git: resolve default branch
Detector->>Git: compute patch-ids on default branch commits
Detector->>Git: check upstream tracking refs for gone branches
Detector->>Git: test reachability from default branch
Detector->>Detector: classify branches: STR_GONE/STR_MERGED/STR_SQUASHED
CLI->>Display: print_stale_summary()
Display->>User: display stale branches and counts
alt --prune flag set
CLI->>User: prompt for confirmation (unless --yes)
User-->>CLI: confirm
CLI->>Pruner: prune_stale_branches()
Pruner->>Git: reopen repos and re-verify safety
Pruner->>Git: delete eligible branches
Pruner->>Display: print_prune_results()
Display->>User: deleted/refused/error counts
end
CLI->>CLI: cleanup & exit
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/integration.sh (1)
487-503: ⚡ Quick winAdd one integration case for interactive
stale --pruneconfirmation (without--yes).Current prune coverage is good, but it only exercises the non-interactive path. Please add a prompt-path test (accept/decline) to lock down the safety rail behavior.
Suggested test shape
+printf "\nstale --prune: confirmation prompt\n" +D="$WORK/prune-confirm"; mkmain "$D" +git -C "$D" checkout -q -b f-confirm +git -C "$D" commit -q --allow-empty -m "merged work" +git -C "$D" checkout -q main +git -C "$D" merge -q --no-ff f-confirm -m "merge" + +# decline prune +out=$(printf 'n\n' | "$GITLS" --no-color stale --prune "$D" 2>&1) +if git -C "$D" rev-parse --verify -q f-confirm >/dev/null; then + printf " ok prune declined keeps branch\n"; passed=$((passed + 1)) +else + printf "FAIL prune declined keeps branch\n got: %s\n" "$out" + failed=$((failed + 1)) +fiAlso applies to: 568-585, 624-649
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration.sh` around lines 487 - 503, Add an interactive prune test next to the existing non-interactive case: after creating D via mkmain and creating/merging branch f-merged, run the CLI without --yes and feed it a simulated response using printf; e.g. capture out=$(printf 'y\n' | "$GITLS" --no-color stale --prune "$D" 2>&1) and assert the branch is deleted (similar checks used currently), then add a second case that feeds 'n\n' (out=$(printf 'n\n' | ...)) and assert the branch still exists; reuse the same test variables ($GITLS, mkmain, D, passed, failed) and mirror the existing success/failure reporting format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config.c`:
- Around line 123-133: The parsing loop that splits protected_branches (using
strtok on copy with tok, idx, arr, count) stores tokens verbatim so entries like
"develop, staging" keep spaces; update the loop to trim leading and trailing
whitespace from each tok before strdup and storing in arr (either by adding an
in-place trim routine or calling a reusable helper like trim_whitespace(tok) to
strip spaces/tabs/newlines), then strdup the trimmed token and proceed with the
same error handling and cleanup logic.
In `@main.c`:
- Around line 149-162: The CLI allows invalid/ignored flags: disallow
non-positive durations and require --yes only with --prune; in the argument
parsing around parse_duration and variables opt_older_than_secs and opt_yes,
treat secs <= 0 as invalid (emit an error for "0d" or negative durations)
instead of accepting, and after parsing all args validate that if opt_yes is
true then opt_prune must also be true (otherwise print a clear error and return
non-zero). Update the error messages to mention the contract (e.g.,
"--older-than must be >0" and "--yes is only valid with --prune") so callers get
immediate feedback.
In `@README.md`:
- Around line 229-240: The fenced code block showing the stale-output example
(the block beginning with the line "gitls stale ~/projects") lacks a language
tag and triggers MD040; update that fence to include a language identifier
(e.g., add "text" so it becomes ```text) so the markdown linter recognizes it;
locate the example block by the unique snippet "gitls stale ~/projects" in
README.md and add the language tag to the opening triple-backtick only.
In `@repo.c`:
- Around line 539-549: Resolve the repository's actual default branch from HEAD
first (e.g., call git_repository_head(&default_ref, repo) and obtain the branch
name via git_branch_name or git_reference_shorthand), store that name into
r->default_branch using strncpy, and only if HEAD cannot be resolved fall back
to checking "main" then "master" via git_branch_lookup; also ensure any
previously allocated git_reference (default_ref) is freed before reassigning.
Use the same symbols from the diff (default_ref, git_branch_lookup,
r->default_branch) so the change replaces the hard-coded main/master resolution
with HEAD-based resolution plus the existing fallbacks.
---
Nitpick comments:
In `@tests/integration.sh`:
- Around line 487-503: Add an interactive prune test next to the existing
non-interactive case: after creating D via mkmain and creating/merging branch
f-merged, run the CLI without --yes and feed it a simulated response using
printf; e.g. capture out=$(printf 'y\n' | "$GITLS" --no-color stale --prune "$D"
2>&1) and assert the branch is deleted (similar checks used currently), then add
a second case that feeds 'n\n' (out=$(printf 'n\n' | ...)) and assert the branch
still exists; reuse the same test variables ($GITLS, mkmain, D, passed, failed)
and mirror the existing success/failure reporting format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93adbd10-f809-4301-bd4f-1d84b3391b80
📒 Files selected for processing (9)
README.mdconfig.cdisplay.cgitlsrc.examplegitools.hmain.crepo.ctests/integration.shtests/unit.c
* config.c: trim leading/trailing whitespace in protected_branches tokens so "develop, release" matches the literal branch name instead of silently storing " release". * main.c: reject --older-than 0/negative durations (they would silently disable the filter while looking active on the CLI), and require --prune when --yes is given (it has no effect otherwise and the help text documents it as prune-only). * repo.c: resolve the repo's actual default branch from refs/remotes/origin/HEAD first, falling back to main/master only when the remote-published default is unavailable. Repos using develop, trunk, etc. as their default now get correct merged / squash detection against the right base. * README.md: language tag on the stale-output fence (MD040). * integration tests: new coverage for the validation gates, the origin/HEAD default-branch resolution, and whitespace-trimming in protected_branches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Restricts the listing (and therefore --prune) to a subset of detection reasons. Useful for "prune only the gone branches, leave merged ones for review" workflows. gitls stale --only gone gitls stale --prune --yes --only gone,merged Accepts a comma-separated list of: gone, merged, squash (also accepts "squashed" as a synonym). Whitespace around tokens is trimmed. The filter is applied at the append site so call sites stay clean and the existing reason-precedence (a branch is classified once, with GONE > MERGED > SQUASHED) is preserved. To match a branch that is both gone and merged under either reason, pass --only gone,merged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.c (1)
61-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
parse_duration()against signed overflow.
parse_duration()multiplies a user-controlledlong nby unit constants without bounding the product. On 32-bitlong, inputs like70ycan overflow, yielding undefined behavior and potentially causingopt_older_than_secsto become non-positive (disabling the--older-thanfilter inrepo.c).Suggested fix
static long parse_duration(const char *s) { if (!s || !*s) return -1; char *end; errno = 0; long n = strtol(s, &end, 10); /* Reject zero and negative durations — they would silently disable the * filter while still appearing on the command line. */ if (errno || n <= 0 || end == s || *end == '\0' || *(end + 1) != '\0') return -1; - switch (*end) { - case 's': return n; - case 'h': return n * 3600L; - case 'd': return n * 86400L; - case 'w': return n * 7L * 86400L; - case 'm': return n * 30L * 86400L; - case 'y': return n * 365L * 86400L; - default: return -1; - } + long mult; + switch (*end) { + case 's': mult = 1L; break; + case 'h': mult = 3600L; break; + case 'd': mult = 86400L; break; + case 'w': mult = 7L * 86400L; break; + case 'm': mult = 30L * 86400L; break; + case 'y': mult = 365L * 86400L; break; + default: return -1; + } + if (n > LONG_MAX / mult) return -1; + return n * mult; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.c` around lines 61 - 70, parse_duration() can overflow when multiplying the parsed long n by unit constants (e.g. in cases 'y','m','w','d','h','s'), leading to undefined behavior and incorrect opt_older_than_secs; add explicit overflow checks before each multiplication (or compute using unsigned long long or use bounds like LONG_MAX / multiplier) and return -1 on overflow. Locate parse_duration and the switch on *end and for each branch validate that n <= (LONG_MAX / multiplier) (or cast to wider type and compare against LONG_MAX) before performing n * multiplier; if the check fails, return -1 so callers (e.g., opt_older_than_secs handling) do not receive a wrapped/negative value. Ensure errno or error path is set consistently when returning -1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@main.c`:
- Around line 61-70: parse_duration() can overflow when multiplying the parsed
long n by unit constants (e.g. in cases 'y','m','w','d','h','s'), leading to
undefined behavior and incorrect opt_older_than_secs; add explicit overflow
checks before each multiplication (or compute using unsigned long long or use
bounds like LONG_MAX / multiplier) and return -1 on overflow. Locate
parse_duration and the switch on *end and for each branch validate that n <=
(LONG_MAX / multiplier) (or cast to wider type and compare against LONG_MAX)
before performing n * multiplier; if the check fails, return -1 so callers
(e.g., opt_older_than_secs handling) do not receive a wrapped/negative value.
Ensure errno or error path is set consistently when returning -1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a63abfb-4355-468c-b549-27cf988d114b
📒 Files selected for processing (7)
README.mdconfig.cgitools.hmain.crepo.ctests/integration.shtests/unit.c
✅ Files skipped from review due to trivial changes (1)
- README.md
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
1 participant
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.
Closes #3.
What
New
stalesubcommand that lists local branches likely safe to clean up across all scanned repos, with optional pruning.Three detection categories:
gonemergedsquashSurface
Plus
~/.gitlsrc:Safety
HEADbranch and the default branch are always excluded.mainandmasterare implicitly protected even when not the resolved default.gonebranches are re-verified against the default branch on prune. Local-only commits not reachable from default cause deletion to be refused, not silently deleted.mergedandsquashbranches are safe by construction.Implementation highlights
git_diff_patchid: walk the default branch (cap 500 commits), build a patch-id set, then compare each candidate branch's cumulative diff vs merge-base. Branches with unmerged work past the squash point keep a unique patch-id and correctly do not match.git_branch_deletefrom the main thread after user confirmation.s/h/d/w/m/ysuffixes.Tests
--prune(delete merged/squash, refuse gone+unmerged),--older-than(recent filtered, old retained, invalid rejected),protected_branchesconfig, implicit main/master protection. 52 total.Docs
protected_branchesin config table.gitlsrc.example: newprotected_branchesentry.--help: subcommand + dedicated "Stale options" block.Summary by CodeRabbit
New Features
stalesubcommand to identify/manage local branches (gone, merged, squash-merged)--pruneto remove eligible branches,--yes/-yto skip prompts,--older-thanage filterprotected_branchesconfig (defaults include main/master)Documentation
Tests