feat: fix bugs, add validation, and surface silent

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)
 	}

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
 	}
 

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)

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 {

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)
 		}

pkg/tui/model.go

@@ -130,6 +130,7 @@ type loadedWorkspacesMsg struct {
 
 type remoteBranchesLoadedMsg struct {
 	branches []string
+	err      error
 }
 
 type createdWorkspaceMsg struct {

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")
 	}
 }
 

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)