diff --git a/anchor.go b/anchor.go index 7d360fd0..c0c22f49 100644 --- a/anchor.go +++ b/anchor.go @@ -8,4 +8,5 @@ type anchorCmd struct { Create anchorCreateCmd `cmd:"" aliases:"c" help:"Create a new worktree anchored at a branch"` List anchorListCmd `cmd:"" aliases:"ls" help:"List anchors and their worktrees"` Track anchorTrackCmd `cmd:"" aliases:"tr" help:"Track an existing worktree as an anchor"` + Rm anchorRmCmd `cmd:"" aliases:"rm" help:"Remove an anchor worktree and dissolve its anchor"` } diff --git a/anchor_rm.go b/anchor_rm.go new file mode 100644 index 00000000..2571b93f --- /dev/null +++ b/anchor_rm.go @@ -0,0 +1,145 @@ +package main + +import ( + "cmp" + "context" + "errors" + "fmt" + "slices" + + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" +) + +type anchorRmCmd struct { + Path string `arg:"" help:"Path of the anchor worktree to remove"` + Force bool `help:"Remove the worktree even if it has uncommitted changes"` +} + +func (*anchorRmCmd) Help() string { + return text.Dedent(` + Removes an anchor worktree and dissolves its anchor. + + The anchor's direct child branches are retargeted onto the + anchor's base (the canonical trunk for a root anchor, or the + pinned branch for an internal anchor); they are left needing a + restack. The anchor branch is then deleted and the worktree + directory removed. + + Use this when a worktree's work is done. Run it from a different + worktree (for example, the primary checkout): a worktree cannot + remove itself. + `) +} + +func (cmd *anchorRmCmd) Run( + ctx context.Context, + log *silog.Logger, + repo *git.Repository, + store *state.Store, + svc *spice.Service, + wt *git.Worktree, +) error { + // Resolve the path to a worktree root and find its anchor. + target, err := repo.OpenWorktree(ctx, cmd.Path) + if err != nil { + return fmt.Errorf("open worktree %q: %w", cmd.Path, err) + } + rootDir := target.RootDir() + + anchor, found := findAnchorForWorktree(ctx, store, target, rootDir) + if !found { + return fmt.Errorf("%v is not an anchor worktree", cmd.Path) + } + + // A worktree cannot remove itself: Git refuses to remove the current + // working tree, so refuse early, before mutating any state. + if wt.RootDir() == rootDir { + return errors.New("a worktree cannot remove itself; " + + "run 'gs anchor rm' from a different worktree") + } + + // The base onto which the anchor's children are retargeted: + // the pinned branch for an internal anchor, else the canonical trunk. + base := cmp.Or(anchor.Base, store.Trunk()) + + // Collect the anchor's direct children before mutating anything. + graph, err := svc.BranchGraph(ctx, nil) + if err != nil { + return fmt.Errorf("load branch graph: %w", err) + } + children := slices.Collect(graph.Aboves(anchor.Branch)) + + // Remove the worktree first. This is the only step that can fail for + // a reason outside our control (a dirty tree without --force), so do + // it before any state mutation: if it fails, nothing has changed and + // the command is safe to retry. Refs are left untouched. + if err := repo.WorktreeRemove(ctx, git.WorktreeRemoveRequest{ + Path: rootDir, + Force: cmd.Force, + }); err != nil { + return fmt.Errorf("remove worktree: %w", err) + } + log.Infof("Removed worktree %s", cmd.Path) + + // Retarget each direct child onto the anchor's base. Retarget-only + // updates state without rebasing, so the children are left needing + // a restack rather than risking a conflict. + for _, child := range children { + if err := svc.BranchOnto(ctx, &spice.BranchOntoRequest{ + Branch: child, + Onto: base, + Mode: spice.BranchOntoRetargetOnly, + }); err != nil { + return fmt.Errorf("retarget %q onto %q: %w", child, base, err) + } + log.Infof("%v: retargeted onto %v", child, base) + } + + // Tear out the anchor: delete its pointer branch and drop its + // registration. + if err := repo.DeleteBranch(ctx, anchor.Branch, git.BranchDeleteOptions{ + Force: true, + }); err != nil { + return fmt.Errorf("delete anchor branch %q: %w", anchor.Branch, err) + } + + if err := store.UnregisterAnchor(ctx, anchor.Branch); err != nil { + return fmt.Errorf("unregister anchor: %w", err) + } + log.Infof("Dissolved anchor %s", anchor.Branch) + + return nil +} + +// findAnchorForWorktree resolves the anchor owned by a worktree. +// +// It matches the registry's recorded worktree path first, then falls +// back to the anchor branch checked out in the worktree: the recorded +// path is advisory and can be stale if the worktree moved. +func findAnchorForWorktree( + ctx context.Context, + store *state.Store, + target *git.Worktree, + rootDir string, +) (state.Anchor, bool) { + anchors := store.Anchors() + for _, a := range anchors { + if a.Worktree == rootDir { + return a, true + } + } + + if branch, err := target.CurrentBranch(ctx); err == nil { + for _, a := range anchors { + if a.Branch == branch { + return a, true + } + } + } + + return state.Anchor{}, false +} diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index ea768de3..3b3e5344 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -316,9 +316,11 @@ Before merging, the stack is checked for branches whose base PR was already merged on the forge. Use --no-branch-check to skip this validation. -Before each merge, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait -before failing if checks are not ready. +Before each merge, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait +before failing if merge readiness is not reached. By default, a branch failure skips that branch's upstack descendants, but independent sibling branches continue. @@ -327,12 +329,12 @@ Use --fail-fast to stop the queue after the first branch failure. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--no-branch-check`: Skip stale base validation before merging. * `--fail-fast`: Stop the merge queue after the first branch failure. * `--branch=NAME`: Branch whose stack to merge -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice stack restack {#gs-stack-restack} @@ -630,7 +632,7 @@ This command acts as a local merge queue: it merges one Change Request, waits for that merge to finish, restacks and updates the next Change Request, -waits for its CI checks to pass, +waits for merge readiness on the updated Change Request, and then repeats the process. For a stack like this: @@ -648,13 +650,15 @@ Before merging, the downstack is checked for branches whose base PR was already merged on the forge. Use --no-branch-check to skip this validation. -Before each merge, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait +Before each merge, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait (default: 30m, 0 means fail immediately if not ready). Between merges, the command waits for each merge to complete, restacks and updates the next PR, -waits for CI checks on the updated PR, +waits for merge readiness on the updated PR, and syncs merged branch cleanup. Use --no-wait for single branch merging @@ -664,12 +668,12 @@ when you don't want to wait for the merge to propagate. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--no-wait`: Skip polling for a single branch merge to propagate. * `--no-branch-check`: Skip stale base validation before merging. * `--branch=NAME`: Branch to start merging from -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice downstack edit {#gs-downstack-edit} @@ -1139,16 +1143,18 @@ Use --branch to merge a different branch. The branch must be based directly on trunk. To merge a stacked branch, use 'gs downstack merge'. -Before merging, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait. +Before merging, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--branch=NAME`: Branch to merge -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice branch submit {#gs-branch-submit} @@ -1452,6 +1458,34 @@ the worktree updates it from the remote. * `--name=BRANCH`: Branch to register as this worktree's anchor (defaults to the branch checked out in the current worktree) +### git-spice anchor rm {#gs-anchor-rm} + +``` +gs anchor rm (rm) [flags] +``` + +Remove an anchor worktree and dissolve its anchor + +Removes an anchor worktree and dissolves its anchor. + +The anchor's direct child branches are retargeted onto the +anchor's base (the canonical trunk for a root anchor, or the +pinned branch for an internal anchor); they are left needing a +restack. The anchor branch is then deleted and the worktree +directory removed. + +Use this when a worktree's work is done. Run it from a different +worktree (for example, the primary checkout): a worktree cannot +remove itself. + +**Arguments** + +* `path`: Path of the anchor worktree to remove + +**Flags** + +* `--force`: Remove the worktree even if it has uncommitted changes + ## Rebase ### git-spice rebase continue {#gs-rebase-continue} diff --git a/internal/handler/sync/mocks_test.go b/internal/handler/sync/mocks_test.go index bc8f1b22..5092e04f 100644 --- a/internal/handler/sync/mocks_test.go +++ b/internal/handler/sync/mocks_test.go @@ -820,17 +820,17 @@ func (m *MockRestackHandler) EXPECT() *MockRestackHandlerMockRecorder { } // RestackBranch mocks base method. -func (m *MockRestackHandler) RestackBranch(ctx context.Context, branch string) error { +func (m *MockRestackHandler) RestackBranch(ctx context.Context, branch string, opts *restack.Options) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RestackBranch", ctx, branch) + ret := m.ctrl.Call(m, "RestackBranch", ctx, branch, opts) ret0, _ := ret[0].(error) return ret0 } // RestackBranch indicates an expected call of RestackBranch. -func (mr *MockRestackHandlerMockRecorder) RestackBranch(ctx, branch any) *MockRestackHandlerRestackBranchCall { +func (mr *MockRestackHandlerMockRecorder) RestackBranch(ctx, branch, opts any) *MockRestackHandlerRestackBranchCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), ctx, branch) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), ctx, branch, opts) return &MockRestackHandlerRestackBranchCall{Call: call} } @@ -846,13 +846,13 @@ func (c *MockRestackHandlerRestackBranchCall) Return(arg0 error) *MockRestackHan } // Do rewrite *gomock.Call.Do -func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/testdata/help/anchor_rm.txt b/testdata/help/anchor_rm.txt new file mode 100644 index 00000000..6c2e4444 --- /dev/null +++ b/testdata/help/anchor_rm.txt @@ -0,0 +1,26 @@ +Usage: gs anchor rm (rm) [flags] + +Remove an anchor worktree and dissolve its anchor + +Removes an anchor worktree and dissolves its anchor. + +The anchor's direct child branches are retargeted onto the anchor's base (the +canonical trunk for a root anchor, or the pinned branch for an internal anchor); +they are left needing a restack. The anchor branch is then deleted and the +worktree directory removed. + +Use this when a worktree's work is done. Run it from a different worktree (for +example, the primary checkout): a worktree cannot remove itself. + +Arguments: + Path of the anchor worktree to remove + +Flags: + --force Remove the worktree even if it has uncommitted changes + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/gs.txt b/testdata/help/gs.txt index 1a532fed..8db9ac89 100644 --- a/testdata/help/gs.txt +++ b/testdata/help/gs.txt @@ -73,6 +73,7 @@ Anchor anchor create (c) Create a new worktree anchored at a branch anchor list (ls) List anchors and their worktrees anchor track (tr) Track an existing worktree as an anchor + anchor rm (rm) Remove an anchor worktree and dissolve its anchor Rebase rebase (rb) continue (c) Continue an interrupted operation diff --git a/testdata/script/anchor_rm.txt b/testdata/script/anchor_rm.txt new file mode 100644 index 00000000..1caa5240 --- /dev/null +++ b/testdata/script/anchor_rm.txt @@ -0,0 +1,66 @@ +# 'gs anchor rm' removes a worktree and dissolves its anchor: +# direct child branches are retargeted onto the anchor's base +# (left needing a restack), the anchor branch is deleted, and the +# worktree directory is removed. + +as 'Test ' +at '2026-03-29T14:00:00Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# An anchor worktree with a child stack: dev -> b1 -> b2. +gs anchor create $WORK/wtB --name dev +cd $WORK/wtB +cp $WORK/b1.txt b1.txt +git add b1.txt +gs bc b1 -m 'Add b1' +cp $WORK/b2.txt b2.txt +git add b2.txt +gs bc b2 -m 'Add b2' + +# A worktree cannot remove itself: refused before any state changes. +cd $WORK/wtB +! gs anchor rm $WORK/wtB +stderr 'cannot remove itself' + +# A dirty worktree is refused without --force, and because removal is +# attempted before retargeting, nothing is mutated: the anchor is still +# registered and the worktree still exists. +cp $WORK/b1.txt $WORK/wtB/dirty.txt +cd $WORK/repo +! gs anchor rm $WORK/wtB +stderr 'remove worktree' +gs anchor ls +stderr 'dev' +exists $WORK/wtB + +# Clean up the dirt, then remove the anchor worktree for real. +rm $WORK/wtB/dirty.txt +gs anchor rm $WORK/wtB +stderr 'b1: retargeted onto main' +stderr 'Removed worktree' + +# The worktree directory is gone. +! exists $WORK/wtB + +# The anchor branch is dissolved; its direct child now roots at main, +# and the rest of the stack is preserved above it. +gs ls -a +stderr 'b1' +stderr 'b2' +! stderr 'dev' + +# Removing a path that is not an anchor worktree is an error. +! gs anchor rm $WORK/repo +stderr 'not an anchor worktree' + +-- repo/dummy.txt -- +dummy +-- b1.txt -- +b1 +-- b2.txt -- +b2