From 70b460ddd91074c82ae75c670be0993ce78538a8 Mon Sep 17 00:00:00 2001 From: Francisco Rodrigues Date: Sat, 18 Jul 2026 22:51:07 -0300 Subject: [PATCH] fix: label worktree rows by the branch they are for, not the folder name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main worktree vanished from `ch list worktree` and the TUI dashboard whenever its clone dir had a non-default branch checked out. Each row was labelled with whatever HEAD happened to be on, so a geomonitor clone sitting on docs/rbac-epic showed up as "docs/rbac-epic" — "main" never appeared, and nothing distinguished the main worktree from the branch it was holding. The CLI compounded it by dropping the head-state entirely. The fix separates addressing from display. Ref stays the worktree's identity — the folder-derived key every operation feeds back — while a new resolveDisplay decides the label from authoritative sources rather than reconstructing it from the folder name. git's live branch is the ground truth of what is checked out. The branch a worktree is *for* — its original — is the configured default branch for the main clone dir, or the branch a running session recorded in @codeherd_branch for any other worktree. A row is only shown as diverged, " (on )", when HEAD has actually left that original; the main clone on a feature branch therefore stays "main (on docs/rbac-epic)" and remains spottable. A worktree with no known original — a non-main worktree with no session — is never treated as diverged: it shows exactly what git reports. This is what keeps the folder name out of the label. Rendering the folder identity is what produced "chore-cron-rework (on chore/restore-cron-rework)" for a worktree simply sitting on its own branch, whose directory name happens not to match it. The session is a refinement, never a dependency: the common case is a worktree with no session, and it must resolve from git alone. Both surfaces now render through one FormatBranchLabel, so the CLI listing and the TUI agree on how a diverged or detached worktree reads. Display resolution moves after the session join in List, because a non-main worktree's original branch lives on its session, not in the git worktree entry. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/worktree.go | 6 +- cmd/worktree_integration_test.go | 129 ++++++++++++++++ internal/herd/workspace.go | 98 +++++++++++-- internal/herd/workspace_test.go | 244 ++++++++++++++++++++++++++++++- internal/tui/delegate.go | 16 +- internal/tui/delegate_test.go | 80 ++++++++++ 6 files changed, 541 insertions(+), 32 deletions(-) create mode 100644 cmd/worktree_integration_test.go diff --git a/cmd/worktree.go b/cmd/worktree.go index c9e17fd..ff5f202 100644 --- a/cmd/worktree.go +++ b/cmd/worktree.go @@ -41,11 +41,7 @@ func (c *ListWorktreeCmd) Run(cmd *cobra.Command, args []string) error { if ws.Agent != nil { sess = ws.Agent.Ref.CanonicalName() + " (running)" } - branch := ws.DisplayBranch - if branch == "" { - branch = "(detached)" - } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", ws.Ref.Project, branch, ws.Path, sess) + fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", ws.Ref.Project, ws.BranchLabel(), ws.Path, sess) } if err := w.Flush(); err != nil { return fmt.Errorf("flushing output: %w", err) diff --git a/cmd/worktree_integration_test.go b/cmd/worktree_integration_test.go new file mode 100644 index 0000000..175eb15 --- /dev/null +++ b/cmd/worktree_integration_test.go @@ -0,0 +1,129 @@ +//go:build integration + +package cmd_test + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// writeListWorktreeConfig writes a config with default_branch set so the main +// clone dir has a stable identity ("main") to fall back to when its HEAD is on +// another branch. +func writeListWorktreeConfig(t *testing.T, projectsDir string) string { + t.Helper() + cfgDir := t.TempDir() + cfgPath := filepath.Join(cfgDir, "config.toml") + content := `[defaults] +projects_dir = "` + projectsDir + `" + +[projects.myapp] +repo = "git@github.com:user/myapp.git" +default_branch = "main" +` + if err := os.WriteFile(cfgPath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return cfgPath +} + +func gitRun(t *testing.T, dir string, args ...string) { + t.Helper() + full := append([]string{"-C", dir}, args...) + if out, err := exec.Command("git", full...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +// TestListWorktree_noSession_showsLiveBranch is the end-to-end guard for the +// geomonitor bug over the primary path: worktrees with NO session. It drives +// the real `ch list worktree` over a real git repo, covering the whole flow the +// TUI shares (h.List -> workspaceFrom -> resolveDisplay -> FormatBranchLabel). +// +// The main clone dir, whose slot is the configured default branch, stays +// labelled "main" with the checkout as a hint. Every other worktree shows git's +// live branch verbatim — the folder name never surfaces, and no divergence hint +// is invented without a session to prove one. +func TestListWorktree_noSession_showsLiveBranch(t *testing.T) { + // list worktree reaches tmux via h.Sessions(); isolate so it neither reads + // nor leaks into the developer's server. + useIsolatedTmux(t) + + projectsDir := t.TempDir() + cloneDir := filepath.Join(projectsDir, "github.com", "user", "myapp") + initBareRepo(t, cloneDir) // main branch + one commit + + // The main clone dir has a non-default branch checked out. + gitRun(t, cloneDir, "checkout", "-b", "docs/rbac-epic") + + // A worktree whose directory name does not match its branch (geomonitor's + // chore-cron-rework), and a normal one. + mismatch := filepath.Join(cloneDir+"__worktrees", "chore-cron-rework") + gitRun(t, cloneDir, "worktree", "add", "-b", "chore/restore-cron-rework", mismatch, "main") + normal := filepath.Join(cloneDir+"__worktrees", "feat") + gitRun(t, cloneDir, "worktree", "add", "-b", "feat", normal, "main") + + cfgPath := writeListWorktreeConfig(t, projectsDir) + + out := captureStdout(t, func() { + if err := runCmd(t, "--config", cfgPath, "list", "worktree"); err != nil { + t.Fatalf("list worktree: %v", err) + } + }) + + // Main clone dir: slot is "main" (config), HEAD on docs/rbac-epic. + if !strings.Contains(out, "main (on docs/rbac-epic)") { + t.Errorf("main worktree should read %q, got:\n%s", "main (on docs/rbac-epic)", out) + } + // Mismatched worktree, no session: git's live branch, clean. + if !strings.Contains(out, "chore/restore-cron-rework") { + t.Errorf("mismatched worktree should show its live branch, got:\n%s", out) + } + // Normal worktree: its branch, clean. + if !strings.Contains(out, "feat") { + t.Errorf("normal worktree should show %q, got:\n%s", "feat", out) + } + // The folder name must never surface, and no hint may be invented. + if strings.Contains(out, "chore-cron-rework (") || strings.Contains(out, "(on chore/restore-cron-rework)") { + t.Errorf("non-session worktree must show the live branch without a hint, got:\n%s", out) + } + if strings.Contains(out, "docs/rbac-epic (on docs/rbac-epic)") { + t.Errorf("head hint restated the branch, got:\n%s", out) + } +} + +// TestListWorktree_sessionDivergence_showsRecordedBranch covers the refinement: +// when a running session records the branch the worktree is for, and HEAD has +// since moved off it, the row shows the recorded branch with the live checkout +// as a hint. A shell session persists in tmux and stamps @codeherd_branch. +func TestListWorktree_sessionDivergence_showsRecordedBranch(t *testing.T) { + useIsolatedTmux(t) + + projectsDir := t.TempDir() + cloneDir := filepath.Join(projectsDir, "github.com", "user", "myapp") + initBareRepo(t, cloneDir) + + cfgPath := writeListWorktreeConfig(t, projectsDir) + + // Create a worktree + shell session for "feat" (records @codeherd_branch). + if err := runCmd(t, "--config", cfgPath, "create", "session", "myapp", "feat", "--shell"); err != nil { + t.Fatalf("create shell session: %v", err) + } + + // Move HEAD off "feat" inside that worktree. + wtPath := filepath.Join(cloneDir+"__worktrees", "feat") + gitRun(t, wtPath, "checkout", "-b", "other") + + out := captureStdout(t, func() { + if err := runCmd(t, "--config", cfgPath, "list", "worktree"); err != nil { + t.Fatalf("list worktree: %v", err) + } + }) + + if !strings.Contains(out, "feat (on other)") { + t.Errorf("session-proven divergence should read %q, got:\n%s", "feat (on other)", out) + } +} diff --git a/internal/herd/workspace.go b/internal/herd/workspace.go index a589a1c..3107fae 100644 --- a/internal/herd/workspace.go +++ b/internal/herd/workspace.go @@ -20,12 +20,20 @@ type Workspace struct { Path string IsMain bool // true for the main clone dir - // DisplayBranch is what a front end should render: the branch HEAD is - // actually on. It is NOT identity and must never be fed back in — that - // round-trip is what orphaned an agent against a deleted worktree. + // DisplayBranch is the branch label a front end should render for this row, + // filled in by resolveDisplay. Normally it is git's live branch; only when + // HEAD has diverged from the branch the worktree is *for* (the default + // branch for the main clone dir; a running session's recorded branch + // otherwise) does it become that original branch, with HeadHint carrying the + // actual checkout. It is NOT a value to feed back in; use Ref for that — + // feeding DisplayBranch or Item.Branch back into an operation orphans an + // agent against a deleted worktree. Render it through FormatBranchLabel so + // the CLI and the TUI agree. DisplayBranch string - // HeadHint is "detached", "on ", or "" when HEAD agrees with Ref. + // HeadHint is "detached", "on ", or "" when HEAD is on the branch + // the worktree is for. Front ends render it alongside DisplayBranch via + // FormatBranchLabel. HeadHint string // Agent and Shell are nil when that session type is not running. @@ -33,6 +41,32 @@ type Workspace struct { Shell *Handle } +// BranchLabel renders this workspace's branch column via FormatBranchLabel. +func (w Workspace) BranchLabel() string { + return FormatBranchLabel(w.DisplayBranch, w.HeadHint) +} + +// FormatBranchLabel renders a worktree's branch column: the display branch, +// plus the head hint in parentheses when HEAD has diverged from the worktree's +// identity. It is the single formatter shared by `ch list worktree` and the TUI +// so both surfaces render a diverged or detached worktree identically. Because +// a diverged workspace already carries its identity in DisplayBranch and the +// live branch in headHint, the two never restate each other — there is no +// " (on )" to suppress. +func FormatBranchLabel(displayBranch, headHint string) string { + switch { + case displayBranch == "": + if headHint == "" { + return "" + } + return "(" + headHint + ")" + case headHint == "": + return displayBranch + default: + return displayBranch + " (" + headHint + ")" + } +} + // EnsureOpts configures workspace creation. The zero value creates the // worktree from the project's default branch and provisions nothing. type EnsureOpts struct { @@ -288,28 +322,68 @@ func (h *Herd) List(project string) ([]Workspace, error) { ws.Shell = &hd } } + // Display is resolved after the join: a non-main worktree's original + // branch lives on its session, so DisplayBranch/HeadHint cannot be + // decided until the sessions are attached. + resolveDisplay(&ws, defaultBranch, wt) out = append(out, ws) } } return out, nil } -// workspaceFrom derives identity and display from one git worktree entry. +// workspaceFrom derives a workspace's identity from one git worktree entry. +// Display (DisplayBranch/HeadHint) is left for resolveDisplay, which runs after +// sessions are joined — a non-main worktree's original branch is recorded on its +// session, not in the worktree entry. func (h *Herd) workspaceFrom(project, cloneDir, defaultBranch string, wt git.WorktreeInfo) Workspace { identity := semconv.WorktreeIdentityBranch(wt.Path, cloneDir, defaultBranch, wt.Branch) - ws := Workspace{ - Ref: h.Ref(project, identity), - Path: wt.Path, - IsMain: wt.Path == cloneDir, - DisplayBranch: wt.Branch, + return Workspace{ + Ref: h.Ref(project, identity), + Path: wt.Path, + IsMain: wt.Path == cloneDir, } +} + +// resolveDisplay fills DisplayBranch and HeadHint from the authoritative +// sources, once sessions are joined. +// +// git's wt.Branch is the ground truth of what is checked out. The "original" +// branch a worktree is *for* is the default branch for the main clone dir, or +// the branch a running session recorded (@codeherd_branch) for any other +// worktree. Divergence is HEAD leaving that original — which is why a worktree +// with no known original (a non-main worktree with no session) is never treated +// as diverged: it shows exactly what git reports. The folder-name identity in +// ws.Ref is only an addressing key and must never surface as a display value — +// rendering it is what showed "chore-cron-rework (on chore/restore-cron-rework)" +// for a worktree simply sitting on its own branch. +// +// The common case is a worktree with no session, so it must not depend on one; +// the session only refines a genuine divergence (created for X, now on Y) and +// recovers a branch name for a detached HEAD. +func resolveDisplay(ws *Workspace, defaultBranch string, wt git.WorktreeInfo) { + original := "" + switch { + case ws.IsMain: + original = defaultBranch + case ws.Agent != nil: + original = ws.Agent.Ref.Branch + case ws.Shell != nil: + original = ws.Shell.Ref.Branch + } + switch { case wt.Detached: + // No live branch to show; recover the original when we have one, + // otherwise the label is just "(detached)". + ws.DisplayBranch = original ws.HeadHint = "detached" - case wt.Branch != "" && semconv.FlattenBranch(wt.Branch) != semconv.FlattenBranch(identity): + case original != "" && semconv.FlattenBranch(wt.Branch) != semconv.FlattenBranch(original): + ws.DisplayBranch = original ws.HeadHint = "on " + wt.Branch + default: + ws.DisplayBranch = wt.Branch } - return ws } // Teardown stops a workspace's sessions and deletes its worktree. diff --git a/internal/herd/workspace_test.go b/internal/herd/workspace_test.go index 1bb204d..d2fd9e3 100644 --- a/internal/herd/workspace_test.go +++ b/internal/herd/workspace_test.go @@ -503,6 +503,11 @@ func TestList_cloneDirDetachedUsesDefaultBranch(t *testing.T) { if spaces[0].HeadHint != "detached" { t.Errorf("HeadHint = %q, want detached", spaces[0].HeadHint) } + // A detached clone dir still has an identity — the default branch — so the + // row stays labelled "main (detached)" rather than losing its branch. + if spaces[0].DisplayBranch != "main" { + t.Errorf("DisplayBranch = %q, want %q for a detached clone dir", spaces[0].DisplayBranch, "main") + } } func TestList_skipUncloned(t *testing.T) { @@ -851,9 +856,47 @@ func TestList_underProfile_findsRunningSession(t *testing.T) { } } -// A diverged HEAD changes what we render, never what we address. This is the -// other half of the shipped defect: Item.Branch held the display branch and -// round-tripped into wtSvc.Delete. +// The main worktree's row is labelled by its identity (the default branch), +// not by whatever HEAD happens to sit on. When the clone dir has a non-default +// branch checked out, DisplayBranch must stay "main" and HeadHint must carry +// the actual checkout — otherwise "main" vanishes from every listing and the +// main worktree becomes unspottable (the geomonitor bug). +func TestList_mainWorktree_divergedHead_displayBranchIsIdentity(t *testing.T) { + tmpDir := t.TempDir() + g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { + return []git.WorktreeInfo{{Path: cloneDirPath(tmpDir), Branch: "docs/rbac-epic"}}, nil + }} + h, _ := workspaceHerdIn(t, tmpDir, g, &fakeTmux{}) + if err := os.MkdirAll(cloneDirPath(tmpDir), 0o755); err != nil { + t.Fatal(err) + } + + spaces, err := h.List("") + if err != nil { + t.Fatalf("List: %v", err) + } + if len(spaces) != 1 { + t.Fatalf("expected 1 workspace, got %d", len(spaces)) + } + if !spaces[0].IsMain { + t.Errorf("IsMain = false, want true for the clone dir") + } + if spaces[0].Ref.Branch != "main" { + t.Errorf("Ref.Branch = %q, want %q — identity is the default branch", spaces[0].Ref.Branch, "main") + } + if spaces[0].DisplayBranch != "main" { + t.Errorf("DisplayBranch = %q, want %q — the main row is labelled by its identity", spaces[0].DisplayBranch, "main") + } + if spaces[0].HeadHint != "on docs/rbac-epic" { + t.Errorf("HeadHint = %q, want %q", spaces[0].HeadHint, "on docs/rbac-epic") + } +} + +// Addressing and display are separate. Ref stays the folder identity so +// operations always land on the right worktree; display, for a non-main +// worktree with no session, is git's live branch — the ground truth — with no +// divergence hint, because the folder name is only an addressing key and there +// is no recorded original branch to diverge from. (Row G′ of the settled table.) func TestList_divergedHead_refKeepsIdentityBranch(t *testing.T) { dir := t.TempDir() cloneDir := filepath.Join(dir, "github.com", "user", "myapp") @@ -863,7 +906,8 @@ func TestList_divergedHead_refKeepsIdentityBranch(t *testing.T) { } g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { - // The worktree was created for "feat" but HEAD now sits on "other". + // The folder is "feat" but HEAD sits on "other" — with no session, we + // cannot know "feat" was ever the intended branch, so trust git. return []git.WorktreeInfo{{Path: wtPath, Branch: "other"}}, nil }} cfg := &config.Config{ @@ -879,16 +923,204 @@ func TestList_divergedHead_refKeepsIdentityBranch(t *testing.T) { t.Fatalf("List: %v", err) } if spaces[0].Ref.Branch != "feat" { - t.Errorf("Ref.Branch = %q, want %q — identity must survive divergence", spaces[0].Ref.Branch, "feat") + t.Errorf("Ref.Branch = %q, want %q — identity must survive for addressing", spaces[0].Ref.Branch, "feat") } if spaces[0].DisplayBranch != "other" { - t.Errorf("DisplayBranch = %q, want %q", spaces[0].DisplayBranch, "other") + t.Errorf("DisplayBranch = %q, want %q — no session, so show git's live branch", spaces[0].DisplayBranch, "other") + } + if spaces[0].HeadHint != "" { + t.Errorf("HeadHint = %q, want empty — no recorded original to diverge from", spaces[0].HeadHint) + } +} + +// The geomonitor chore-cron-rework row: a worktree whose directory name does +// not match its branch, with no session. The folder name must never surface; +// we show git's live branch, clean, exactly as v0.2.0's CLI did. (Row F.) +func TestList_nonMainWorktree_dirNameMismatch_noSession_showsLiveBranch(t *testing.T) { + dir := t.TempDir() + cloneDir := filepath.Join(dir, "github.com", "user", "myapp") + wtPath := filepath.Join(dir, "github.com", "user", "myapp__worktrees", "chore-cron-rework") + if err := os.MkdirAll(cloneDir, 0o755); err != nil { + t.Fatal(err) + } + + g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { + return []git.WorktreeInfo{{Path: wtPath, Branch: "chore/restore-cron-rework"}}, nil + }} + cfg := &config.Config{ + Defaults: config.DefaultsConfig{ProjectsDir: dir}, + Projects: map[string]config.ProjectConfig{ + "myapp": {Repo: "git@github.com:user/myapp.git", DefaultBranch: "main"}, + }, + } + h := New(cfg, nil, Deps{Tmux: &fakeTmux{}, Git: g}) + + spaces, err := h.List("") + if err != nil { + t.Fatalf("List: %v", err) + } + if spaces[0].DisplayBranch != "chore/restore-cron-rework" { + t.Errorf("DisplayBranch = %q, want the live branch %q", spaces[0].DisplayBranch, "chore/restore-cron-rework") + } + if spaces[0].HeadHint != "" { + t.Errorf("HeadHint = %q, want empty — no session, no divergence", spaces[0].HeadHint) + } + if spaces[0].BranchLabel() != "chore/restore-cron-rework" { + t.Errorf("BranchLabel() = %q, want %q", spaces[0].BranchLabel(), "chore/restore-cron-rework") + } +} + +// A running session records the branch the worktree is for (@codeherd_branch). +// When HEAD has since moved off it, that is a genuine divergence: label the row +// by the recorded original and put the live branch in the hint. (Row G.) +func TestList_nonMainWorktree_sessionDivergence_showsRecordedBranch(t *testing.T) { + dir := t.TempDir() + cloneDir := filepath.Join(dir, "github.com", "user", "myapp") + wtPath := filepath.Join(dir, "github.com", "user", "myapp__worktrees", "feat") + if err := os.MkdirAll(cloneDir, 0o755); err != nil { + t.Fatal(err) + } + + g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { + return []git.WorktreeInfo{{Path: wtPath, Branch: "other"}}, nil + }} + f := &fakeTmux{Sessions: []sessionRow{ + {ID: "$1", Name: "myapp-feat", Canonical: "myapp-feat", + Type: "agent", Status: "running", Branch: "feat", Project: "myapp"}, + }} + cfg := &config.Config{ + Defaults: config.DefaultsConfig{ProjectsDir: dir}, + Projects: map[string]config.ProjectConfig{ + "myapp": {Repo: "git@github.com:user/myapp.git", DefaultBranch: "main"}, + }, + } + h := New(cfg, nil, Deps{Tmux: f, Git: g}) + + spaces, err := h.List("") + if err != nil { + t.Fatalf("List: %v", err) + } + if spaces[0].Agent == nil { + t.Fatal("expected the session to join its workspace") + } + if spaces[0].DisplayBranch != "feat" { + t.Errorf("DisplayBranch = %q, want the recorded branch %q", spaces[0].DisplayBranch, "feat") } if spaces[0].HeadHint != "on other" { t.Errorf("HeadHint = %q, want %q", spaces[0].HeadHint, "on other") } } +// When the recorded branch and git's HEAD agree, there is no divergence and we +// prefer git's live branch for display — the recorded value may be flattened, +// git's is the real, unflattened name. (Row E.) +func TestList_nonMainWorktree_sessionAgrees_showsLiveBranch(t *testing.T) { + dir := t.TempDir() + cloneDir := filepath.Join(dir, "github.com", "user", "myapp") + wtPath := filepath.Join(dir, "github.com", "user", "myapp__worktrees", "feat-x") + if err := os.MkdirAll(cloneDir, 0o755); err != nil { + t.Fatal(err) + } + + g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { + return []git.WorktreeInfo{{Path: wtPath, Branch: "feat/x"}}, nil + }} + f := &fakeTmux{Sessions: []sessionRow{ + // Recorded flattened (a session launched from a listed Ref); git's + // "feat/x" is the same branch, unflattened. + {ID: "$1", Name: "myapp-feat-x", Canonical: "myapp-feat-x", + Type: "agent", Status: "running", Branch: "feat-x", Project: "myapp"}, + }} + cfg := &config.Config{ + Defaults: config.DefaultsConfig{ProjectsDir: dir}, + Projects: map[string]config.ProjectConfig{ + "myapp": {Repo: "git@github.com:user/myapp.git", DefaultBranch: "main"}, + }, + } + h := New(cfg, nil, Deps{Tmux: f, Git: g}) + + spaces, err := h.List("") + if err != nil { + t.Fatalf("List: %v", err) + } + if spaces[0].DisplayBranch != "feat/x" { + t.Errorf("DisplayBranch = %q, want git's unflattened branch %q", spaces[0].DisplayBranch, "feat/x") + } + if spaces[0].HeadHint != "" { + t.Errorf("HeadHint = %q, want empty — recorded and live agree", spaces[0].HeadHint) + } +} + +// A detached non-main worktree with a running session recovers the branch from +// the session record, so the row stays identifiable rather than collapsing to a +// bare "(detached)". (Row H.) +func TestList_nonMainWorktree_detachedWithSession_recoversRecordedBranch(t *testing.T) { + dir := t.TempDir() + cloneDir := filepath.Join(dir, "github.com", "user", "myapp") + wtPath := filepath.Join(dir, "github.com", "user", "myapp__worktrees", "feat") + if err := os.MkdirAll(cloneDir, 0o755); err != nil { + t.Fatal(err) + } + + g := &fakeGit{ListFn: func(string) ([]git.WorktreeInfo, error) { + return []git.WorktreeInfo{{Path: wtPath, Branch: "", Detached: true}}, nil + }} + f := &fakeTmux{Sessions: []sessionRow{ + {ID: "$1", Name: "myapp-feat", Canonical: "myapp-feat", + Type: "agent", Status: "running", Branch: "feat", Project: "myapp"}, + }} + cfg := &config.Config{ + Defaults: config.DefaultsConfig{ProjectsDir: dir}, + Projects: map[string]config.ProjectConfig{ + "myapp": {Repo: "git@github.com:user/myapp.git", DefaultBranch: "main"}, + }, + } + h := New(cfg, nil, Deps{Tmux: f, Git: g}) + + spaces, err := h.List("") + if err != nil { + t.Fatalf("List: %v", err) + } + if spaces[0].DisplayBranch != "feat" { + t.Errorf("DisplayBranch = %q, want recovered %q", spaces[0].DisplayBranch, "feat") + } + if spaces[0].HeadHint != "detached" { + t.Errorf("HeadHint = %q, want detached", spaces[0].HeadHint) + } +} + +// FormatBranchLabel is the single formatter both the CLI and the TUI render +// through. A diverged workspace carries its identity in DisplayBranch and the +// live branch in HeadHint, so the two never restate each other. +func TestFormatBranchLabel(t *testing.T) { + tests := []struct { + name string + displayBranch string + headHint string + want string + }{ + {"plain branch", "chore/frontend-arch", "", "chore/frontend-arch"}, + {"main on default", "main", "", "main"}, + {"main diverged", "main", "on docs/rbac-epic", "main (on docs/rbac-epic)"}, + {"session divergence", "feat", "on other", "feat (on other)"}, + {"detached with identity", "main", "detached", "main (detached)"}, + {"detached without identity", "", "detached", "(detached)"}, + {"empty", "", "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := FormatBranchLabel(tt.displayBranch, tt.headHint); got != tt.want { + t.Errorf("FormatBranchLabel(%q, %q) = %q, want %q", tt.displayBranch, tt.headHint, got, tt.want) + } + // Workspace.BranchLabel is the same formatter over the struct. + ws := Workspace{DisplayBranch: tt.displayBranch, HeadHint: tt.headHint} + if got := ws.BranchLabel(); got != tt.want { + t.Errorf("Workspace.BranchLabel() = %q, want %q", got, tt.want) + } + }) + } +} + // workspaceHerdIn is workspaceHerd but reusing an existing tmpDir so a ListFn // closure can reference the clone path. func workspaceHerdIn(t *testing.T, tmpDir string, g *fakeGit, f *fakeTmux) (*Herd, string) { diff --git a/internal/tui/delegate.go b/internal/tui/delegate.go index 77aabcc..793682b 100644 --- a/internal/tui/delegate.go +++ b/internal/tui/delegate.go @@ -9,6 +9,7 @@ import ( tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" + "github.com/xico42/codeherd/internal/herd" "github.com/xico42/codeherd/internal/semconv" ) @@ -41,15 +42,12 @@ func (d itemDelegate) Render(w io.Writer, m list.Model, index int, listItem list isSelected := index == m.Index() - // Line 1: project / branch (+ head-state hint when HEAD diverged) - var line1 string - if item.Branch != "" { - line1 = item.Project + " / " + item.Branch - } else { - line1 = item.Project - } - if item.HeadHint != "" { - line1 += " (" + item.HeadHint + ")" + // Line 1: project / branch (+ head-state hint when HEAD diverged). The label + // is formatted by the same herd.FormatBranchLabel the CLI uses, so both + // surfaces render a diverged or detached worktree identically. + line1 := item.Project + if label := herd.FormatBranchLabel(item.Branch, item.HeadHint); label != "" { + line1 += " / " + label } cursor := " " diff --git a/internal/tui/delegate_test.go b/internal/tui/delegate_test.go index 8b13078..09211a0 100644 --- a/internal/tui/delegate_test.go +++ b/internal/tui/delegate_test.go @@ -7,6 +7,7 @@ import ( "charm.land/bubbles/v2/list" + "github.com/xico42/codeherd/internal/herd" "github.com/xico42/codeherd/internal/semconv" ) @@ -187,6 +188,85 @@ func TestDelegate_Render_noBranch(t *testing.T) { } } +// The main worktree, when its HEAD is on a non-default branch, must render as +// " / main (on )" — the identity branch labels the row and the +// checkout is a hint. The regression it guards: rendering the live branch as +// the label produced "geomonitor / docs/rbac-epic (on docs/rbac-epic)", with +// the live branch doubled and "main" gone. Starts from a herd.Workspace so it +// exercises the real buildItems -> delegate path, not a hand-built Item. +func TestDelegate_Render_mainWorktreeDivergedHead(t *testing.T) { + spaces := []herd.Workspace{{ + Ref: herd.Ref{Project: "geomonitor", Branch: "main"}, + DisplayBranch: "main", + HeadHint: "on docs/rbac-epic", + IsMain: true, + Path: "/p/geomonitor", + }} + items := buildItems(nil, spaces) + d := newDelegate() + m := list.New(items, d, 80, 10) + + var buf bytes.Buffer + d.Render(&buf, m, 0, items[0]) + out := buf.String() + + if !strings.Contains(out, "geomonitor / main (on docs/rbac-epic)") { + t.Errorf("render should label the main row by identity, got: %q", out) + } + if strings.Contains(out, "docs/rbac-epic (on docs/rbac-epic)") { + t.Errorf("render doubled the live branch instead of showing main, got: %q", out) + } +} + +// A non-main worktree with no session shows git's live branch, clean — the +// folder name never surfaces and there is no divergence hint. This is the +// geomonitor chore-cron-rework row. Starts from a herd.Workspace so it +// exercises the real buildItems -> delegate path. +func TestDelegate_Render_nonMainNoSessionShowsLiveBranch(t *testing.T) { + spaces := []herd.Workspace{{ + Ref: herd.Ref{Project: "geomonitor", Branch: "chore-cron-rework"}, + DisplayBranch: "chore/restore-cron-rework", + HeadHint: "", + Path: "/p/geomonitor/wt/chore-cron-rework", + }} + items := buildItems(nil, spaces) + d := newDelegate() + m := list.New(items, d, 80, 10) + + var buf bytes.Buffer + d.Render(&buf, m, 0, items[0]) + out := buf.String() + + if !strings.Contains(out, "geomonitor / chore/restore-cron-rework") { + t.Errorf("render should show the live branch, got: %q", out) + } + if strings.Contains(out, "(on ") || strings.Contains(out, "chore-cron-rework (") { + t.Errorf("render should not add a hint or show the folder name, got: %q", out) + } +} + +// With a session proving the divergence, the row shows the recorded branch and +// the live checkout as a hint. +func TestDelegate_Render_sessionDivergenceShowsRecordedBranch(t *testing.T) { + spaces := []herd.Workspace{{ + Ref: herd.Ref{Project: "geomonitor", Branch: "feat"}, + DisplayBranch: "feat", + HeadHint: "on other", + Path: "/p/geomonitor/wt/feat", + }} + items := buildItems(nil, spaces) + d := newDelegate() + m := list.New(items, d, 80, 10) + + var buf bytes.Buffer + d.Render(&buf, m, 0, items[0]) + out := buf.String() + + if !strings.Contains(out, "geomonitor / feat (on other)") { + t.Errorf("render should show recorded branch + hint, got: %q", out) + } +} + func TestDelegate_Render_headHint(t *testing.T) { d := newDelegate() m := list.New([]list.Item{