diff --git a/cmd/opentree/cmd/config.go b/cmd/opentree/cmd/config.go index 12c1f72..d84b3be 100644 --- a/cmd/opentree/cmd/config.go +++ b/cmd/opentree/cmd/config.go @@ -60,7 +60,7 @@ var configListCmd = &cobra.Command{ fmt.Printf("worktree.base_dir = %s\n", cfg.Worktree.BaseDir) fmt.Printf("worktree.default_base = %s\n", cfg.Worktree.DefaultBase) fmt.Printf("tmux.session_prefix = %s\n", cfg.Tmux.SessionPrefix) - fmt.Printf("github.auto_push = %t\n", cfg.GitHub.AutoPush) + fmt.Printf("github.auto_push = %t\n", cfg.GitHub.AutoPush != nil && *cfg.GitHub.AutoPush) return nil } @@ -74,7 +74,7 @@ var configListCmd = &cobra.Command{ fmt.Printf("worktree.base_dir = %s (%s)\n", cfg.Worktree.BaseDir, sources.WorktreeBaseDir) fmt.Printf("worktree.default_base = %s (%s)\n", cfg.Worktree.DefaultBase, sources.WorktreeDefaultBase) fmt.Printf("tmux.session_prefix = %s (%s)\n", cfg.Tmux.SessionPrefix, sources.TmuxSessionPrefix) - fmt.Printf("github.auto_push = %t (%s)\n", cfg.GitHub.AutoPush, sources.GitHubAutoPush) + fmt.Printf("github.auto_push = %t (%s)\n", cfg.GitHub.AutoPush != nil && *cfg.GitHub.AutoPush, sources.GitHubAutoPush) return nil }, } @@ -171,7 +171,7 @@ func getConfigValue(cfg *config.Config, key string) (string, error) { case "tmux.session_prefix": return cfg.Tmux.SessionPrefix, nil case "github.auto_push": - return strconv.FormatBool(cfg.GitHub.AutoPush), nil + return strconv.FormatBool(cfg.GitHub.AutoPush != nil && *cfg.GitHub.AutoPush), nil default: return "", fmt.Errorf("unknown config key %q\nRun 'opentree config list' to see available keys", key) } @@ -198,7 +198,7 @@ func setConfigValue(cfg *config.Config, key, value string) error { if err != nil { return fmt.Errorf("invalid value for github.auto_push: must be true or false") } - cfg.GitHub.AutoPush = b + cfg.GitHub.AutoPush = &b default: return fmt.Errorf("unknown config key %q", key) } diff --git a/pkg/config/config.go b/pkg/config/config.go index fa0dea1..1e467d4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -56,7 +56,7 @@ type TmuxConfig struct { // GitHubConfig configures GitHub integration type GitHubConfig struct { - AutoPush bool `toml:"auto_push"` + AutoPush *bool `toml:"auto_push,omitempty"` } // ConfigSource tracks which config file provided each value. @@ -75,6 +75,9 @@ const ( SourceRepo = "repo" ) +// boolPtr returns a pointer to b. +func boolPtr(b bool) *bool { return &b } + // Default returns the default configuration func Default() *Config { return &Config{ @@ -90,7 +93,7 @@ func Default() *Config { SessionPrefix: "opentree", }, GitHub: GitHubConfig{ - AutoPush: false, + AutoPush: boolPtr(false), }, } } @@ -168,10 +171,7 @@ func mergeInto(dst, src *Config) { if src.Tmux.SessionPrefix != "" { dst.Tmux.SessionPrefix = src.Tmux.SessionPrefix } - // auto_push is a bool — we can't detect "was it set?" from the value alone. - // We use the presence of the file and the zero value. Since false == default, - // the only meaningful override is true. - if src.GitHub.AutoPush { + if src.GitHub.AutoPush != nil { dst.GitHub.AutoPush = src.GitHub.AutoPush } } @@ -223,10 +223,10 @@ func computeSources(resolved, global, repo *Config) ConfigSource { src.TmuxSessionPrefix = SourceRepo } - if global != nil && global.GitHub.AutoPush { + if global != nil && global.GitHub.AutoPush != nil { src.GitHubAutoPush = SourceGlobal } - if repo != nil && repo.GitHub.AutoPush { + if repo != nil && repo.GitHub.AutoPush != nil { src.GitHubAutoPush = SourceRepo } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 521ebd5..3c9cefb 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -26,7 +26,7 @@ func TestDefault(t *testing.T) { if cfg.Tmux.SessionPrefix != "opentree" { t.Errorf("Tmux.SessionPrefix = %q, want %q", cfg.Tmux.SessionPrefix, "opentree") } - if cfg.GitHub.AutoPush != false { + if cfg.GitHub.AutoPush == nil || *cfg.GitHub.AutoPush != false { t.Errorf("GitHub.AutoPush = %v, want false", cfg.GitHub.AutoPush) } } @@ -86,8 +86,8 @@ auto_push = true if cfg.Tmux.SessionPrefix != "myapp" { t.Errorf("Tmux.SessionPrefix = %q, want %q", cfg.Tmux.SessionPrefix, "myapp") } - if !cfg.GitHub.AutoPush { - t.Error("GitHub.AutoPush = false, want true") + if cfg.GitHub.AutoPush == nil || !*cfg.GitHub.AutoPush { + t.Error("GitHub.AutoPush = false/nil, want true") } } @@ -147,7 +147,7 @@ func TestSave_And_Load_RoundTrip(t *testing.T) { SessionPrefix: "proj", }, GitHub: GitHubConfig{ - AutoPush: true, + AutoPush: boolPtr(true), }, } @@ -175,8 +175,8 @@ func TestSave_And_Load_RoundTrip(t *testing.T) { if loaded.Tmux.SessionPrefix != original.Tmux.SessionPrefix { t.Errorf("Tmux.SessionPrefix = %q, want %q", loaded.Tmux.SessionPrefix, original.Tmux.SessionPrefix) } - if loaded.GitHub.AutoPush != original.GitHub.AutoPush { - t.Errorf("GitHub.AutoPush = %v, want %v", loaded.GitHub.AutoPush, original.GitHub.AutoPush) + if loaded.GitHub.AutoPush == nil || *loaded.GitHub.AutoPush != *original.GitHub.AutoPush { + t.Errorf("GitHub.AutoPush = %v, want %v", loaded.GitHub.AutoPush, *original.GitHub.AutoPush) } } @@ -272,7 +272,7 @@ func TestSaveGlobal_And_LoadGlobal_RoundTrip(t *testing.T) { Agent: AgentConfig{Command: "my-agent", Args: []string{"-v"}}, Worktree: WorktreeConfig{BaseDir: ".trees", DefaultBase: "develop"}, Tmux: TmuxConfig{SessionPrefix: "proj"}, - GitHub: GitHubConfig{AutoPush: true}, + GitHub: GitHubConfig{AutoPush: boolPtr(true)}, } if err := SaveGlobal(original); err != nil { @@ -371,6 +371,44 @@ command = "repo-agent" } } +func TestLoadWithSources_RepoFalseOverridesGlobalTrue(t *testing.T) { + xdgDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", xdgDir) + + globalToml := ` +[github] +auto_push = true +` + globalPath := filepath.Join(xdgDir, "opentree", "opentree.toml") + if err := os.MkdirAll(filepath.Dir(globalPath), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(globalPath, []byte(globalToml), 0644); err != nil { + t.Fatal(err) + } + + repoToml := ` +[github] +auto_push = false +` + repoPath := filepath.Join(t.TempDir(), "opentree.toml") + if err := os.WriteFile(repoPath, []byte(repoToml), 0644); err != nil { + t.Fatal(err) + } + + cfg, sources, err := LoadWithSources(repoPath) + if err != nil { + t.Fatalf("LoadWithSources() failed: %v", err) + } + + if cfg.GitHub.AutoPush == nil || *cfg.GitHub.AutoPush != false { + t.Errorf("GitHub.AutoPush = %v, want false (repo should override global)", cfg.GitHub.AutoPush) + } + if sources.GitHubAutoPush != SourceRepo { + t.Errorf("sources.GitHubAutoPush = %q, want %q", sources.GitHubAutoPush, SourceRepo) + } +} + func TestLoadWithSources_DefaultsWhenNeitherConfigExists(t *testing.T) { xdgDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", xdgDir) diff --git a/pkg/gitutil/gitutil.go b/pkg/gitutil/gitutil.go index f75c5b6..bc6d34b 100644 --- a/pkg/gitutil/gitutil.go +++ b/pkg/gitutil/gitutil.go @@ -46,6 +46,23 @@ func RepoRoot() (string, error) { return strings.TrimSpace(string(output)), nil } +// ValidateBranchName checks whether name is a valid git branch name +// by running `git check-ref-format --branch`. +func ValidateBranchName(name string) error { + if name == "" { + return fmt.Errorf("branch name cannot be empty") + } + cmd := exec.Command("git", "check-ref-format", "--branch", name) + if out, err := cmd.CombinedOutput(); err != nil { + msg := strings.TrimSpace(string(out)) + if msg != "" { + return fmt.Errorf("invalid branch name %q: %s", name, msg) + } + return fmt.Errorf("invalid branch name %q", name) + } + return nil +} + // SanitizeBranchName converts a branch name to a safe directory/window name. // Replaces "/" and ":" with "-". func SanitizeBranchName(name string) string { diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 91a2839..cbd2567 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -100,8 +100,8 @@ func (m Model) createWorkspaceFromRemoteCmd(branchName string) tea.Cmd { func (m Model) loadRemoteBranchesCmd() tea.Cmd { return func() tea.Msg { - branches, _ := gitutil.ListRemoteBranches(m.repoRoot, 10) - return remoteBranchesLoadedMsg{branches: branches} + branches, err := gitutil.ListRemoteBranches(m.repoRoot, 10) + return remoteBranchesLoadedMsg{branches: branches, err: err} } } @@ -221,7 +221,7 @@ func (m Model) checkBranchStatusCmd(wsName, branch, repoDir string, wasPushed bo return func() tea.Msg { status, err := m.prMgr.GetBranchAndPRStatus(branch, repoDir, wasPushed) if err != nil { - return nil + return errMsg{err} } return branchStatusCheckedMsg{wsName: wsName, status: status} } @@ -266,7 +266,7 @@ func (m Model) loadDiffCmd(ws WorkspaceItem) tea.Cmd { return errMsg{err} } - uncommitted, _ := m.worktreeMgr.DiffUncommitted(ws.Branch) + uncommitted, uncommittedErr := m.worktreeMgr.DiffUncommitted(ws.Branch) committedTrimmed := strings.TrimSpace(committed) uncommittedTrimmed := strings.TrimSpace(uncommitted) @@ -282,7 +282,9 @@ func (m Model) loadDiffCmd(ws WorkspaceItem) tea.Cmd { } } - if uncommittedTrimmed != "" { + if uncommittedErr != nil { + sections = append(sections, "══════ Uncommitted Changes ══════\n\n(error: "+uncommittedErr.Error()+")") + } else if uncommittedTrimmed != "" { header := "══════ Uncommitted Changes ══════" sections = append(sections, header+"\n\n"+uncommittedTrimmed) } diff --git a/pkg/tui/model.go b/pkg/tui/model.go index 839962d..9ac269c 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -130,6 +130,7 @@ type loadedWorkspacesMsg struct { type remoteBranchesLoadedMsg struct { branches []string + err error } type createdWorkspaceMsg struct { diff --git a/pkg/tui/tui_test.go b/pkg/tui/tui_test.go index bac77e5..177383b 100644 --- a/pkg/tui/tui_test.go +++ b/pkg/tui/tui_test.go @@ -650,14 +650,17 @@ func TestOpenPR_OKeyWithPRURLReturnsCmd(t *testing.T) { } } -func TestOpenPR_OKeyWithoutPRURLDoesNothing(t *testing.T) { +func TestOpenPR_OKeyWithoutPRURLShowsError(t *testing.T) { ws := testWS("no-pr") // no PRURL m := newTestModel(ws) - _, cmd := applyUpdate(m, keyMsg("o")) + updated, cmd := applyUpdate(m, keyMsg("o")) - if cmd != nil { - t.Error("expected nil cmd when pressing o on workspace without PR URL") + if cmd == nil { + t.Error("expected non-nil cmd (transient error) when pressing o on workspace without PR URL") + } + if updated.err == nil { + t.Error("expected transient error to be set") } } diff --git a/pkg/tui/update.go b/pkg/tui/update.go index 7dcfa6d..6c84295 100644 --- a/pkg/tui/update.go +++ b/pkg/tui/update.go @@ -10,6 +10,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/axelgar/opentree/pkg/config" + "github.com/axelgar/opentree/pkg/gitutil" ) func spinnerTickCmd() tea.Cmd { @@ -245,6 +246,13 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Batch(m.createWorkspaceFromIssueCmd(val), spinnerTickCmd()) } if m.createStep == 0 { + if err := gitutil.ValidateBranchName(val); err != nil { + m.err = err + m.appendErrLog(err.Error()) + return m, tea.Tick(3*time.Second, func(t time.Time) tea.Msg { + return clearErrorMsg{} + }) + } m.newBranchName = val m.createStep = 1 m.input.Placeholder = "Base branch" @@ -308,22 +316,23 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } return m, m.attachWorkspaceCmd(ws.Name) } case key.Matches(msg, m.keys.Diff): if len(visible) > 0 { - if m.isWorkspaceInFlight(visible[m.cursor].Name) { - return m, nil + ws := visible[m.cursor] + if m.isWorkspaceInFlight(ws.Name) { + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } - return m, m.loadDiffCmd(visible[m.cursor]) + return m, m.loadDiffCmd(ws) } case key.Matches(msg, m.keys.PR): if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } m.prGenerating = true m.prWsName = ws.Name @@ -335,27 +344,29 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } if ws.PRURL != "" { return m, openURLCmd(ws.PRURL) } + return m, m.transientErrCmd(fmt.Sprintf("no PR for %q — create one with 'p'", ws.Name)) } case key.Matches(msg, m.keys.Review): if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } if ws.PRURL != "" { return m, m.sendReviewsCmd(ws.Name) } + return m, m.transientErrCmd(fmt.Sprintf("no PR for %q — create one first with 'p'", ws.Name)) } case key.Matches(msg, m.keys.Select): if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } if m.selected[ws.Name] { delete(m.selected, ws.Name) @@ -375,7 +386,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } else if len(visible) > 0 { ws := visible[m.cursor] if m.isWorkspaceInFlight(ws.Name) { - return m, nil + return m, m.transientErrCmd(fmt.Sprintf("workspace %q has a pending operation", ws.Name)) } m.deleting = true m.deleteTarget = ws.Name @@ -412,6 +423,14 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case remoteBranchesLoadedMsg: + if msg.err != nil { + m.resetCreateMode() + m.err = fmt.Errorf("failed to load remote branches: %w", msg.err) + m.appendErrLog(m.err.Error()) + return m, tea.Tick(3*time.Second, func(t time.Time) tea.Msg { + return clearErrorMsg{} + }) + } m.remoteBranches = msg.branches m.filteredBranches = filterBranches(msg.branches, m.input.Value()) m.branchSuggestionCursor = 0 @@ -620,6 +639,14 @@ func (m *Model) resetCreateMode() { m.input.Placeholder = "New branch name" } +func (m *Model) transientErrCmd(msg string) tea.Cmd { + m.err = fmt.Errorf("%s", msg) + m.appendErrLog(msg) + return tea.Tick(3*time.Second, func(t time.Time) tea.Msg { + return clearErrorMsg{} + }) +} + func (m *Model) appendErrLog(msg string) { ts := time.Now().Format("15:04:05") entry := fmt.Sprintf("[%s] %s", ts, msg) diff --git a/pkg/tui/view.go b/pkg/tui/view.go index 3b56d8c..2c8cb11 100644 --- a/pkg/tui/view.go +++ b/pkg/tui/view.go @@ -170,6 +170,10 @@ func (m Model) View() string { sb.WriteString("\n") sb.WriteString(helpStyle.Render(" loading branches…")) sb.WriteString("\n") + } else { + sb.WriteString("\n") + sb.WriteString(helpStyle.Render(" no branches match")) + sb.WriteString("\n") } sb.WriteString("\n") sb.WriteString(helpStyle.Render("↑/↓ navigate • Tab select • Enter confirm • Esc cancel")) diff --git a/pkg/workspace/workspace.go b/pkg/workspace/workspace.go index c79af2c..1b53367 100644 --- a/pkg/workspace/workspace.go +++ b/pkg/workspace/workspace.go @@ -162,6 +162,7 @@ func (s *Service) CreateFromRemoteBranch(branchName string) (*state.Workspace, e agentCmd := s.cfg.Agent.Command if err := s.process.CreateWindow(branchName, worktreePath, agentCmd, s.cfg.Agent.Args...); err != nil { + _ = s.worktrees.Delete(branchName, true) // cleanup orphaned worktree return nil, fmt.Errorf("failed to create tmux window: %w", err) }