fix(git-core): make worktree-teardown deny message and doctor reclaim actually work - #493
Conversation
… actually work Two defects that together dead-ended an agent cleaning up a stale worktree. The teardown deny message told the agent to run `agentflare git worktree audit --prune`, which does not parse: the subcommand is `audit`, not `worktree audit`, and `prune` is a subcommand rather than a flag. Following the message verbatim produced a clap error -- exactly the dead-end the message was added to remove. The suggestion now lives in a constant (`classify::WORKTREE_PRUNE_COMMAND` = `agentflare git audit prune --all`), parsed against the real clap definition by a test in the binary crate so a later rename fails loudly instead of rotting the message again. `doctor::reclaim` ran `git worktree prune` only after a successful `remove_dir_all`. A lane whose directory was already gone -- precisely what `HealthFlag::MissingWorktree` describes -- was reported reclaimed with its `.git/worktrees/<name>` admin entry left dangling, so `git worktree list` kept calling it prunable and `git branch -d` kept failing with "used by worktree", with no way out because the shim denies plain `git worktree prune`. The prune now runs once at the end of a reclaim pass whenever any lane was reclaimed. A lane whose directory could not be deleted is still not reported as reclaimed and does not trigger a prune. Agentflare-Agent: claude-code Agentflare-Branch: task/469-worktree-teardown-dead-ends-shim-names-a Agentflare-Item: 469
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
getappz
changed the title
worktree prune for already-deleted dirs
doctor::reclaim's new needs_prune-gated `git worktree prune` call duplicated the identical batch-prune-at-end-of-loop pattern already used by worktree::gc_orphans. Extract shell::prune_worktree_metadata_if so both call sites share one guarded prune invocation. Agentflare-Agent: claude-code Agentflare-Branch: task/469-worktree-teardown-dead-ends-shim-names-a Agentflare-Item: 469
#529) agentflare git doctor now reads git.jsonl and flags scope-check crashes that hit 3+ of the last 10 commit/push invocations as a violation (nonzero exit, same as existing dirty/stale thresholds) -- an isolated crash (e.g. mid-upgrade binary swap) stays silent, since blocking on every one-off would just retrain agents to ignore doctor output. Agentflare-Agent: claude-code Agentflare-Branch: task/493-git-shim-alert-on-sustained-scopechecker Agentflare-Item: 493
1 participant
Both fixes are committed on
task/469-worktree-teardown-dead-ends-shim-names-a(commit720b790).What changed
Bug 1 — deny message named a non-existent command (
crates/flare-git-core/src/classify.rs)Verified the cited line said exactly what the brief claimed. Rather than inlining a corrected string (which would rot the same way on the next CLI rename), the suggestion is now a constant the test can parse:
The teardown deny reason is built with
format!from it; theitem(action="doctor", reclaim=true)alternative is untouched.Tests:
classify_tests.rs's existing assertion changed from the string literal"audit --prune"toreason.contains(WORKTREE_PRUNE_COMMAND), and a newworktree_teardown_deny_names_a_parsable_cliinsrc/cli/git.rsfeeds the constant throughCli::try_parse_fromand destructures the result down toGitCommand::Audit(… Prune(prune)), assertingprune.all. The parse test has to live in the binary crate —src/cliis declared inmain.rs, notlib.rs, so neithertests/nor flare-git-core can reach the clap definition.Bug 2 —
reclaimskippedworktree prunefor already-deleted dirs (crates/flare-git-core/src/doctor.rs)Replaced the in-
Ok(())prune with aneeds_pruneflag set on both the successful-delete and the!path.exists()paths, pruning once after the loop. TheErrarm still neither reports the lane reclaimed nor sets the flag — a lane whose directory couldn't be deleted has a live admin entry and isn't reclaimed.New test
reclaim_prunes_the_admin_entry_of_an_already_deleted_worktreecreates a linked worktree,remove_dir_alls its directory, runs reclaim, then assertsgit worktree list --porcelainreports noprunableentry andgit branch -d linked-branchsucceeds. I confirmed it's not vacuous by temporarily disabling the fix — it fails withprunable gitdir file points to non-existent location.Gate
cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic— clean.cargo fmt --all --check— clean.cargo test --workspace— all green except 3 pre-existing failures incrates/flare-git-shim/tests/shim_test.rs(protected_branch_checkout_…,push_of_default_branch_…,canonical_repo_detach_…). These are unrelated to this change and fail identically on the stashed clean tree — I verified that explicitly. They fail on the "…but passes through for a human" half of each assertion because the test process inherits this session's agent-detect environment, so the shim classifies the simulated human invocation as an agent. CI, running without those vars, is unaffected.Parenting — not set, and still blocked
Checked
origin/master(caf5845) directly:item_updatestill constructsUpdateItem { name, description, priority, state_id, assignee_agent, sort_order, metadata }with noparent_idfield, so #377 has not landed anditem(action="update", parent_id=...)would still silently discard it. Per the brief I left the parent unset rather than working around it — this item still needs re-parenting under EPIC #337 (X3evrzjhE4UFwug9Bktg-) once #377 ships.I did not push or open a PR, and did not mark the item done — the branch is committed and ready for whichever of those you want next.