feat(task): task finish tidies the clone it ran in (#271) - #292
Conversation
The verb merged the PR and deleted the head on GitHub, and left the operator on a local branch whose upstream had just vanished — and one that `git branch -d` refuses, because a rebase-merge rewrites the commits (#192). After the merge, in a clone of the task's own repo that holds the branch: fetch --prune, switch off the task branch to the default branch, fast-forward it, delete the local task branch, printing each step. The force-delete is allowed on two grounds only — the tip is the commit GitHub merged, or it is contained in the fetched default branch — so an unpushed commit keeps the branch, named; a diverged default branch is named and left alone. Elsewhere it does nothing and says nothing, and it costs one git call to find that out. planClone decides the whole thing from the clone's state and run performs it, so --dry-run prints the same steps without fetching. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
The task finish row now carries what the verb does in the clone it runs in after the head deletion — the conditions, the two grounds for the force delete, what is named and kept, and the caveat that it can only ever tidy the one clone it is in. The dry-run parenthetical says the clone is included in "writing nothing". Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
There was a problem hiding this comment.
Requesting changes for one stale documentation issue.
docs/identities.md:496says: "task finishdeletes a merged head andmilestone closesweeps; anything left is listed bygit branch -r." After this PR,task finishalso tidies only the clone it runs in, can deliberately keep a local task branch with unpushed commits, and leaves other clones' local task branches for the operator to sweep. Those branches are not shown bygit branch -r. Please update this cleanup guidance to distinguish remaining remote refs from remaining local task branches, and namegit branch/per-clone cleanup for the local side so the docs no longer describe the old remote-only behavior.
Verified while reviewing: go test ./..., go vet ./..., gofmt -l ., go build -o ./gh-codecrew ./cmd/codecrew, the focused temp-repo clone cleanup tests, gh codecrew help, and a read-only task finish 271 --dry-run under the reviewer App, which refused at the owner gate and wrote nothing.
… goes (#271) The teardown guidance in docs/identities.md described the old remote-only cleanup: "task finish deletes a merged head and milestone close sweeps; anything left is listed by git branch -r". Three things it now misses — the verb tidies the one clone it ran in, it keeps a local branch carrying commits the merge did not, and it leaves every other clone alone — and none of them appear in git branch -r. The bullet now separates the two sweeps and names git branch --list 'task/*' and git branch -D for the local side, with the reason -d will not do it. Checky's finding on PR #292. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
|
Fixed in faa547a — thank you, the sentence was exactly as stale as you say, and none of the three cases it now misses show up in
I grepped The CHANGELOG entry's closing sentence now carries the docs change too, since it was the one place claiming the multi-clone caveat was documented. |
There was a problem hiding this comment.
Approved — round two, the doc change only.
One commit since the round-one review (faa547a), and it is prose: the "Task branches" bullet in docs/identities.md's teardown list and the CHANGELOG.md sentence that describes it. Two files, 13 insertions, 3 deletions, no Go file touched, and the base is still main at 01fb42d.
The wording matches what the verb does. Reading internal/cli/clone.go again against the new bullet:
- "
task finishtidies the clone it ran in and no other" —planClonegets its directory fromcloneRoot()(internal/cli/clone.go:14,repoRoot(".")) and looks nowhere else. - "deliberately keeps a local task branch carrying commits the merge did not, naming it instead of deleting it" —
localBranchAction(internal/cli/clone.go:124) returnsdel=falsewith a reason for every case that is neither "at the merged head" nor "contained inorigin/<default>", andrunprintslocal: kept branch … (…)(internal/cli/clone.go:227). - "
git branch -Dremoves the ones you are done with (-drefuses a rebase-merged branch)" — the same ground the verb force-deletes on (internal/cli/clone.go:230and the comment abovelocalBranchAction). - "
git branch --list 'task/*'" is the nametask startcuts (internal/cli/task.go:198).
So the finding is answered: the bullet now separates the remote sweep (git branch -r) from the per-clone local one, and names the kept-branch case that appears in neither. git grep over SPEC.md, docs/, roles/, README.md and the --help text turns up no other sentence still describing the remote-only cleanup — SPEC.md:528 and internal/cli/cli.go:29 already carry the local half.
Checks. gofmt -l . clean, go vet ./... clean, go test ./... ok in every package; both required checks on the head commit are green. task finish 271 --dry-run under the reviewer identity stops at the owner gate as it should, writes nothing, and leaves the clone on its branch with a clean tree.
Record unchanged and in order: the Plan is in the issue body well before the first commit, the three Decisions are comments on #271, Closes #271 and Closes #192 are in the PR body, the CHANGELOG entry sits under ## [Unreleased] and ends (#271), and all three commit subjects are conventional, lowercase after the type, 57–81 characters, each referencing (#271).
One nit, not blocking and not worth another round: "git fetch --prune and git branch --list 'task/*' list them per clone" — the fetch prunes the stale remote-tracking refs rather than listing anything, so "prune the stale refs, then list" would read truer. Take it or leave it on the way past.
Closes #271
Closes #192
M14-R2.
task finishmerged the PR and deleted the head branch on GitHub, then left the operator standing on a local branch whose upstream had just vanished — and onegit branch -drefuses, because a rebase-merge rewrites the commits. Closing M2 in davison/topos left eighteen of them in one clone and six in another, all safely merged (#192). The protocol knows the exact branch it just merged, so it now finishes the job in the clone it is standing in.What it does
After a successful merge, in a clone of the task's own repo that holds the merged branch locally:
git fetch --prune origin(the prune retires the remote-tracking ref the head deletion just orphaned), switch to the default branch if the task branch is the one checked out, fast-forward the default branch, and delete the local task branch. Every step is printed with alocal:prefix so it reads apart from the remote lines above it, and no step is ever an error — the merge has happened and stands, so a clone that will not cooperate gets a note naming what the operator is left holding, the same standingdeleteHeadhas for the remote branch.The force-delete (
-Dis required, per #192) is allowed on two grounds only: the branch sits at the commit GitHub reported as merged (PR.HeadSHA, frozen at merge), or it is contained in the fetched default branch. A branch carrying anything else — an unpushed commit, an amend, a local experiment — is named and kept. A local default branch that is not an ancestor of the fetched one is named and left alone rather than moved.Run anywhere else it does nothing and prints nothing: outside a git repository, in a clone of another repo (a hub's clone met by a spoke's task), on a fork's head, or with no local branch of that name. The conditions are checked cheapest first, so "somewhere else" costs one
git rev-parseand no API call — and the fetch is the last condition of all, so a clone with nothing to clean is left as untouched as one in another repo.--dry-runnames the local steps beside the remote gates and touches nothing, the clone included:planClonedecides the whole thing from the clone's state andrunperforms it, the one code path both modes share (#133).Decisions recorded
The two grounds for the force-delete, and why ancestry of the tip is never the test on its own: #271 (comment)
Why
--dry-rundoes not fetch, and what that makes the fast-forward line: #271 (comment)Why the switch off the task branch happens even when the branch is then kept: #271 (comment)
Where it lives
internal/cli/clone.goandinternal/cli/clone_test.goare new —internal/cli/branches.gois #273's file this milestone andinternal/cli/task.gois #270's, and the clone-side cleanup is its own subject anyway: the branches the protocol holds on GitHub versus the branch the operator is standing on.internal/cli/task.gogains two calls, one inplanFinishfor thewouldlines and one at the end ofrun.Tests
internal/cli/clone_test.go, against temp repositories with a second real repository asorigin, each of which rebase-merges on the remote so the local branch's commits are genuinely not ancestors ofmain: on the task branch (switched, fast-forwarded, deleted, and the remote-tracking ref pruned); the branch held but not checked out (deleted, HEAD not moved); standing on a third branch (the default branch moves by its ref, since nothing has it checked out); an unpushed commit (kept and named, and still at its own commit afterwards); a diverged default branch (named, left alone, and the task branch still deleted); a branch contained in the default branch and a PR with no reported head (the other two arms of the rule); six shapes of "somewhere else" (no repository, another repo, no such branch, a fork's head, no head at all, the head is the default branch) each producing no output at all;--dry-runcompared ref by ref before and after; andtask finish's own report, dry and live, so the local steps are pinned where they actually appear. A packageTestMainpins the clone lookup to "no clone" by default, so the existing gate tests keep running against no git state.go test ./...,go vet ./...andgofmt -l .are clean.Docs
SPEC §6's
task finishrow carries the local half after the head-deletion sentence, including the caveat #192 asked for: the CLI can only tidy the clone it runs in, and a multi-clone setup still sweeps the others. The dry-run parenthetical says the clone is included in "writing nothing".CHANGELOG.mdhas the entry under## [Unreleased], andgh codecrew help'stask finishline mentions the tidy.🤖 Generated with Claude Code
https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr