From 7510187243df3764d6ed8a679988f1c118c239d3 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 19 Sep 2026 16:11:59 +1000 Subject: [PATCH] agentws: check out --branch from origin when it exists `agentws new --branch` always forked from the base branch, so a worktree for a branch that already existed on origin started at the base's HEAD with none of the branch's commits, and callers had to reset --hard afterwards to recover. - Check out an existing origin/ and set the branch to track it - Fast-forward a stale local branch onto origin, keeping unpushed commits - Fork from --from or the remote default only when origin lacks the branch - Name which of the two paths was taken in the command's output - Ignore --from, with a note, when the branch is already on origin --- README.md | 18 +++- cmd/agentws/main.go | 78 ++++++++++++++-- cmd/agentws/new_test.go | 192 ++++++++++++++++++++++++++++++++++++++++ internal/agent/git.go | 31 +++++++ 4 files changed, 309 insertions(+), 10 deletions(-) create mode 100644 cmd/agentws/new_test.go diff --git a/README.md b/README.md index c407371..3308a1d 100644 --- a/README.md +++ b/README.md @@ -97,10 +97,10 @@ checkout too; the worktrees themselves live under the **worktree root** ```bash # Clone unkin/argocd-apps into ~/src/prodenv if missing, then add a worktree for -# a new branch off the remote default branch. Prints the worktree path. +# the branch. Prints the worktree path. agentws new argocd-apps --branch benvin/my-change -# Branch off a specific base instead of the remote default +# Branch off a specific base instead of the remote default (new branches only) agentws new argocd-apps --branch benvin/hotfix --from release-1.2 # List managed worktrees (repo, branch, path) @@ -124,6 +124,20 @@ agentws clean agentws token ``` +`agentws new` fetches first, then takes one of two paths and names the one it +took on its last output line. A branch that **already exists on origin** is +checked out at `origin/` and set to track it, so the worktree starts on +the branch's own commits (`branch (tracking origin/ at )`); a local +branch left from an earlier run is fast-forwarded onto it. A branch origin does +**not** have is created from `--from`, or from the remote's default branch when +`--from` is absent (`branch (new, from origin/)`) — the default is read +from `origin/HEAD`, so a repo on `master` forks from `master`. `--from` is +ignored, with a note, when the branch is already on origin. + +The one case the worktree does not land on `origin/` is a local branch +carrying commits origin has never seen. Those commits exist nowhere else, so the +checkout is left on them and the output says how many. + ### prune `agentws prune` finds worktrees two ways and merges the results: the managed diff --git a/cmd/agentws/main.go b/cmd/agentws/main.go index cbfeead..f15bd5f 100644 --- a/cmd/agentws/main.go +++ b/cmd/agentws/main.go @@ -161,13 +161,24 @@ func newNewCmd() *cobra.Command { return err } - // c. Base branch: --from or the remote default. + // c. A branch origin already has is work in progress and must be + // checked out as it stands; only a branch origin does not have is + // forked from a base. + onRemote := agent.GitRemoteBranchExists(srcDir, "origin", branch) + startPoint := "origin/" + branch base := from - if base == "" { - base, err = agent.GitRemoteDefaultBranch(srcDir, "origin") - if err != nil { - return err + if onRemote { + if from != "" { + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "note: --from %s ignored, origin/%s already exists\n", from, branch) } + } else { + if base == "" { + base, err = agent.GitRemoteDefaultBranch(srcDir, "origin") + if err != nil { + return err + } + } + startPoint = "origin/" + base } // d. Create the worktree FROM the source checkout so the branch is @@ -176,7 +187,7 @@ func newNewCmd() *cobra.Command { if _, statErr := os.Stat(wtPath); statErr == nil { return fmt.Errorf("worktree already exists at %s", wtPath) } - if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, "origin/"+base); err != nil { + if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, startPoint); err != nil { return err } @@ -199,9 +210,19 @@ func newNewCmd() *cobra.Command { return err } - // f. Report the worktree path and branch. + // f. A local branch left from an earlier run may sit behind origin, + // so reusing it is not enough on its own. + summary := fmt.Sprintf("branch %s (new, from origin/%s)", branch, base) + if onRemote { + summary, err = alignToRemote(cmd.OutOrStdout(), wtPath, branch) + if err != nil { + return err + } + } + + // g. Report the worktree path and which of the two paths was taken. _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", wtPath) - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "branch %s (from origin/%s)\n", branch, base) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", summary) return nil }, } @@ -212,6 +233,47 @@ func newNewCmd() *cobra.Command { return cmd } +// alignToRemote puts the worktree on origin/ and reports what that took. +// A reused local branch can be stale, and a fast-forward is the only move that +// adds no commit and drops none; a local branch carrying commits origin does not +// have is left where it stands, because those commits exist nowhere else. +func alignToRemote(out io.Writer, wtPath, branch string) (string, error) { + remoteRef := "origin/" + branch + want, err := agent.GitRevParse(wtPath, remoteRef) + if err != nil { + return "", err + } + head, err := agent.GitRevParse(wtPath, "HEAD") + if err != nil { + return "", err + } + if head != want { + ahead, err := agent.GitAheadCount(wtPath, remoteRef, "HEAD") + if err != nil { + return "", err + } + if ahead > 0 { + return fmt.Sprintf("branch %s (local, %s not on %s, left at %s)", + branch, commitCount(ahead), remoteRef, shortSHA(head)), nil + } + if err := agent.GitMergeFFOnly(wtPath, remoteRef); err != nil { + return "", err + } + _, _ = fmt.Fprintf(out, "fast-forwarded stale %s to %s\n", branch, remoteRef) + } + if err := agent.GitSetUpstream(wtPath, branch, remoteRef); err != nil { + return "", err + } + return fmt.Sprintf("branch %s (tracking %s at %s)", branch, remoteRef, shortSHA(want)), nil +} + +func shortSHA(sha string) string { + if len(sha) > 7 { + return sha[:7] + } + return sha +} + // --- list ----------------------------------------------------------------- func newListCmd() *cobra.Command { diff --git a/cmd/agentws/new_test.go b/cmd/agentws/new_test.go new file mode 100644 index 0000000..440d078 --- /dev/null +++ b/cmd/agentws/new_test.go @@ -0,0 +1,192 @@ +package main + +import ( + "bytes" + "path/filepath" + "strings" + "testing" + + "git.unkin.net/unkin/agent-tools/internal/agent" +) + +// newFixtureOn builds the same origin/source/worktree-root layout as the prune +// fixture but with a chosen default branch, so `new` can be tested against a +// repo whose default is not "main". +func newFixtureOn(t *testing.T, defBranch string) *fixture { + t.Helper() + root := t.TempDir() + f := &fixture{ + root: root, + bare: filepath.Join(root, "origin.git"), + srcDir: filepath.Join(root, "src", "repo"), + wtRoot: filepath.Join(root, "worktrees"), + } + git(t, root, "init", "--bare", "-b", defBranch, f.bare) + + seed := filepath.Join(root, "seed") + git(t, root, "init", "-b", defBranch, seed) + identity(t, seed) + writeCommit(t, seed, "README.md", "hi\n", "init") + git(t, seed, "remote", "add", "origin", f.bare) + git(t, seed, "push", "-u", "origin", defBranch) + + git(t, root, "clone", f.bare, f.srcDir) + identity(t, f.srcDir) + + t.Setenv("AGENTWS_ROOT", f.wtRoot) + t.Setenv("AGENTWS_SRC_ROOT", filepath.Join(root, "src")) + t.Setenv("AGENTWS_OWNER", "unkin") + return f +} + +// runNew invokes `agentws new repo --branch ` and returns its output. +func runNew(t *testing.T, branch string, extra ...string) string { + t.Helper() + var out bytes.Buffer + cmd := newRootCmd() + cmd.SetArgs(append([]string{"new", "repo", "--branch", branch}, extra...)) + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("new %s: %v (output %q)", branch, err, out.String()) + } + return out.String() +} + +// pushBranch creates branch on origin carrying one commit and returns its SHA. +func pushBranch(t *testing.T, f *fixture, branch, file, content string) string { + t.Helper() + seed := filepath.Join(f.root, "seed") + git(t, seed, "checkout", "-b", branch) + writeCommit(t, seed, file, content, "work on "+branch) + git(t, seed, "push", "origin", branch) + return git(t, seed, "rev-parse", "HEAD") +} + +func wtPathFor(f *fixture, branch string) string { + return filepath.Join(f.wtRoot, agent.WorktreeDirName("repo", branch)) +} + +// The regression: a branch that already exists on origin must be checked out at +// origin's tip, not forked from the default branch. +func TestNewChecksOutExistingRemoteBranch(t *testing.T) { + f := newFixtureOn(t, "main") + want := pushBranch(t, f, "benvin/existing", "a.txt", "a\n") + + out := runNew(t, "benvin/existing") + + path := wtPathFor(f, "benvin/existing") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want origin/benvin/existing %s", got, want) + } + if upstream := git(t, path, "rev-parse", "--abbrev-ref", "HEAD@{upstream}"); upstream != "origin/benvin/existing" { + t.Errorf("upstream = %q, want origin/benvin/existing", upstream) + } + if !contains(out, "tracking origin/benvin/existing") { + t.Errorf("output %q does not say the remote branch was checked out", out) + } +} + +// A branch origin does not have is still forked from the default branch, and the +// output must say so rather than leaving the caller to guess. +func TestNewForksBranchMissingFromRemote(t *testing.T) { + f := newFixtureOn(t, "main") + want := git(t, f.srcDir, "rev-parse", "origin/main") + + out := runNew(t, "benvin/fresh") + + path := wtPathFor(f, "benvin/fresh") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want origin/main %s", got, want) + } + if !contains(out, "branch benvin/fresh (new, from origin/main)") { + t.Errorf("output %q does not report a new branch", out) + } +} + +// The base is the remote's own default branch, so a repo defaulting to master +// forks from master. +func TestNewForksFromMasterDefaultBranch(t *testing.T) { + f := newFixtureOn(t, "master") + want := git(t, f.srcDir, "rev-parse", "origin/master") + + out := runNew(t, "benvin/on-master") + + path := wtPathFor(f, "benvin/on-master") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want origin/master %s", got, want) + } + if !contains(out, "from origin/master") { + t.Errorf("output %q does not name origin/master as the base", out) + } +} + +// An existing remote branch beats --from: the flag is ignored and the caller is +// told, rather than the branch being silently re-forked. +func TestNewIgnoresFromWhenBranchIsOnRemote(t *testing.T) { + f := newFixtureOn(t, "main") + want := pushBranch(t, f, "benvin/with-from", "a.txt", "a\n") + + out := runNew(t, "benvin/with-from", "--from", "main") + + path := wtPathFor(f, "benvin/with-from") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want origin/benvin/with-from %s", got, want) + } + if !contains(out, "--from main ignored") { + t.Errorf("output %q does not report the ignored --from", out) + } +} + +// A local branch left behind by an earlier run must not pin the worktree to a +// commit origin has moved past. +func TestNewFastForwardsStaleLocalBranch(t *testing.T) { + f := newFixtureOn(t, "main") + stale := pushBranch(t, f, "benvin/stale", "a.txt", "a\n") + git(t, f.srcDir, "fetch", "origin") + git(t, f.srcDir, "branch", "benvin/stale", "origin/benvin/stale") + + seed := filepath.Join(f.root, "seed") + writeCommit(t, seed, "b.txt", "b\n", "more work") + git(t, seed, "push", "origin", "benvin/stale") + want := git(t, seed, "rev-parse", "HEAD") + if want == stale { + t.Fatal("fixture did not move origin/benvin/stale on") + } + + out := runNew(t, "benvin/stale") + + path := wtPathFor(f, "benvin/stale") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want origin/benvin/stale %s", got, want) + } + if !contains(out, "fast-forwarded stale benvin/stale") { + t.Errorf("output %q does not report the fast-forward", out) + } +} + +// A local branch carrying commits origin does not have keeps them: they exist +// nowhere else, so the worktree stays put and the output says so. +func TestNewKeepsLocalCommitsAheadOfRemote(t *testing.T) { + f := newFixtureOn(t, "main") + pushBranch(t, f, "benvin/ahead", "a.txt", "a\n") + git(t, f.srcDir, "fetch", "origin") + git(t, f.srcDir, "checkout", "-b", "benvin/ahead", "origin/benvin/ahead") + writeCommit(t, f.srcDir, "local.txt", "local\n", "local only") + want := git(t, f.srcDir, "rev-parse", "HEAD") + git(t, f.srcDir, "checkout", "main") + + out := runNew(t, "benvin/ahead") + + path := wtPathFor(f, "benvin/ahead") + if got := git(t, path, "rev-parse", "HEAD"); got != want { + t.Errorf("worktree HEAD = %s, want the local tip %s", got, want) + } + if !contains(out, "1 commit not on origin/benvin/ahead") { + t.Errorf("output %q does not report the unpushed commit", out) + } +} + +func contains(haystack, needle string) bool { + return strings.Contains(haystack, needle) +} diff --git a/internal/agent/git.go b/internal/agent/git.go index 54a1de6..675a4fa 100644 --- a/internal/agent/git.go +++ b/internal/agent/git.go @@ -109,6 +109,37 @@ func GitRemoteBranchExists(repoDir, remote, branch string) bool { return err == nil } +// GitRevParse resolves ref to a full object id in repoDir. +func GitRevParse(repoDir, ref string) (string, error) { + return runGit(repoDir, "rev-parse", ref) +} + +// GitAheadCount counts commits reachable from head that upstream does not hold. +func GitAheadCount(repoDir, upstream, head string) (int, error) { + out, err := runGit(repoDir, "rev-list", "--count", upstream+".."+head) + if err != nil { + return 0, err + } + n, err := strconv.Atoi(strings.TrimSpace(out)) + if err != nil { + return 0, fmt.Errorf("parse rev-list count %q: %w", out, err) + } + return n, nil +} + +// GitMergeFFOnly advances the branch checked out at dir to ref, failing rather +// than writing a merge commit when the move is not a fast-forward. +func GitMergeFFOnly(dir, ref string) error { + _, err := runGit(dir, "merge", "--ff-only", ref) + return err +} + +// GitSetUpstream points branch at the remote-tracking ref upstream. +func GitSetUpstream(repoDir, branch, upstream string) error { + _, err := runGit(repoDir, "branch", "--set-upstream-to="+upstream, branch) + return err +} + // GitIsDirty reports whether the checkout at dir has uncommitted or untracked // changes. func GitIsDirty(dir string) (bool, error) {