From e2fcf857587150f3acb77a5be042acaa35976193 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 12 Mar 2026 08:11:28 +0100 Subject: [PATCH 1/2] fix bugs, silent failures, and simplify create-mode reset - config: remove dead AgentCommand source attribution code that was immediately overwritten, and unused `_ = resolved` / `defaults` var - workspace: clean up orphaned worktree if tmux window creation fails - tui: reset creating/filtering/prCreating flags on errMsg to prevent stuck dialogs - worktree: replace fmt.Sscanf (silent errors) with strconv.Atoi in parseNumstat - github: distinguish "no pull requests found" from real gh CLI errors in FetchPRReviews - tui/commands: surface DiffStats error as "diff unavailable" instead of silently showing stale "No changes"; remove redundant WindowID:"" - tui: extract resetCreateMode() helper used across all 5 create-mode exit paths - tui/view: extract renderCIBadge() helper, replace magic layout numbers with named constants Co-Authored-By: Claude Sonnet 4.6 --- pkg/config/config.go | 10 -------- pkg/github/github.go | 5 +++- pkg/tui/commands.go | 13 ++++++---- pkg/tui/update.go | 50 ++++++++++++++++---------------------- pkg/tui/view.go | 47 +++++++++++++++++++---------------- pkg/workspace/workspace.go | 1 + pkg/worktree/worktree.go | 9 +++++-- 7 files changed, 67 insertions(+), 68 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index c11106b..fa0dea1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -179,7 +179,6 @@ func mergeInto(dst, src *Config) { // computeSources compares a resolved config against global and repo raw configs // to determine which source provided each final value. func computeSources(resolved, global, repo *Config) ConfigSource { - defaults := Default() src := ConfigSource{ AgentCommand: SourceDefault, AgentArgs: SourceDefault, @@ -189,14 +188,6 @@ func computeSources(resolved, global, repo *Config) ConfigSource { GitHubAutoPush: SourceDefault, } - // Helper: check global then repo for each field - if global != nil && global.Agent.Command != "" && global.Agent.Command != defaults.Agent.Command { - src.AgentCommand = SourceGlobal - } - if repo != nil && repo.Agent.Command != "" && repo.Agent.Command != defaults.Agent.Command { - src.AgentCommand = SourceRepo - } - // If resolved value equals default but global/repo both set it, still annotate if global != nil && global.Agent.Command != "" { src.AgentCommand = SourceGlobal } @@ -239,7 +230,6 @@ func computeSources(resolved, global, repo *Config) ConfigSource { src.GitHubAutoPush = SourceRepo } - _ = resolved return src } diff --git a/pkg/github/github.go b/pkg/github/github.go index 9e1b8d3..40434a4 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -51,7 +51,10 @@ func (pm *PRManager) FetchPRReviews(branch string) ([]ReviewComment, error) { cmd := exec.Command("gh", "pr", "view", branch, "--json", "url,reviews") output, err := cmd.CombinedOutput() if err != nil { - return nil, nil // no PR for this branch + if strings.Contains(string(output), "no pull requests found") { + return nil, nil + } + return nil, fmt.Errorf("gh pr view failed: %w", err) } var prData struct { diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index b5bce97..d7738d0 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -35,11 +35,15 @@ func (m Model) loadWorkspacesCmd() tea.Msg { go func(i int, ws *state.Workspace) { defer wg.Done() - diff, fileChanges, _ := m.worktreeMgr.DiffStats(ws.Branch, ws.BaseBranch) + diff, fileChanges, err := m.worktreeMgr.DiffStats(ws.Branch, ws.BaseBranch) diffStat := "No changes" - lines := strings.Split(strings.TrimSpace(diff), "\n") - if len(lines) > 0 && lines[len(lines)-1] != "" { - diffStat = lines[len(lines)-1] + if err != nil { + diffStat = "diff unavailable" + } else { + lines := strings.Split(strings.TrimSpace(diff), "\n") + if len(lines) > 0 && lines[len(lines)-1] != "" { + diffStat = lines[len(lines)-1] + } } win, exists := windowMap[ws.Name] @@ -52,7 +56,6 @@ func (m Model) loadWorkspacesCmd() tea.Msg { Workspace: ws, DiffStat: diffStat, Active: exists && win.Active, - WindowID: "", FileChanges: fileChanges, } if exists { diff --git a/pkg/tui/update.go b/pkg/tui/update.go index 48aa361..450ec3e 100644 --- a/pkg/tui/update.go +++ b/pkg/tui/update.go @@ -206,24 +206,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if branchName == "" { return m, nil } - m.creating = false - m.remoteBranchMode = false - m.remoteBranches = nil - m.filteredBranches = nil - m.branchSuggestionCursor = 0 - m.input.SetValue("") - m.input.Placeholder = "New branch name" + m.resetCreateMode() m.workspaceCreating = true m.workspaceCreatingName = branchName return m, tea.Batch(m.createWorkspaceFromRemoteCmd(branchName), spinnerTickCmd()) case "esc": - m.creating = false - m.remoteBranchMode = false - m.remoteBranches = nil - m.filteredBranches = nil - m.branchSuggestionCursor = 0 - m.input.SetValue("") - m.input.Placeholder = "New branch name" + m.resetCreateMode() return m, nil default: m.input, cmd = m.input.Update(msg) @@ -240,10 +228,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if m.issueMode { - m.creating = false - m.issueMode = false - m.input.SetValue("") - m.input.Placeholder = "New branch name" + m.resetCreateMode() m.workspaceCreating = true m.workspaceCreatingName = "issue " + val return m, tea.Batch(m.createWorkspaceFromIssueCmd(val), spinnerTickCmd()) @@ -257,21 +242,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } branchName := m.newBranchName baseBranch := val - m.creating = false - m.createStep = 0 - m.newBranchName = "" - m.input.SetValue("") - m.input.Placeholder = "New branch name" + m.resetCreateMode() m.workspaceCreating = true m.workspaceCreatingName = branchName return m, tea.Batch(m.createWorkspaceCmd(branchName, baseBranch), spinnerTickCmd()) case "esc": - m.creating = false - m.issueMode = false - m.createStep = 0 - m.newBranchName = "" - m.input.SetValue("") - m.input.Placeholder = "New branch name" + m.resetCreateMode() return m, nil } m.input, cmd = m.input.Update(msg) @@ -592,6 +568,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.workspaceCreating = false m.workspaceDeleting = false m.workspaceDeletingNames = make(map[string]bool) + m.creating = false + m.filtering = false + m.prCreating = false m.err = msg.err m.appendErrLog(msg.err.Error()) return m, tea.Tick(3*time.Second, func(t time.Time) tea.Msg { @@ -617,6 +596,19 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, cmd } +func (m *Model) resetCreateMode() { + m.creating = false + m.remoteBranchMode = false + m.remoteBranches = nil + m.filteredBranches = nil + m.branchSuggestionCursor = 0 + m.issueMode = false + m.createStep = 0 + m.newBranchName = "" + m.input.SetValue("") + m.input.Placeholder = "New branch name" +} + 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 f6ff11a..c07dc49 100644 --- a/pkg/tui/view.go +++ b/pkg/tui/view.go @@ -10,6 +10,13 @@ import ( "github.com/axelgar/opentree/pkg/config" ) +const ( + headerFooterHeight = 8 + minDiffHeight = 5 + defaultPreviewWidth = 60 + minPreviewWidth = 20 +) + func (m Model) View() string { // Error log overlay if m.showErrLog { @@ -70,9 +77,9 @@ func (m Model) View() string { // Diff view overlay if m.diffViewing { lines := strings.Split(m.diffContent, "\n") - availHeight := m.height - 8 - if availHeight < 5 { - availHeight = 5 + availHeight := m.height - headerFooterHeight + if availHeight < minDiffHeight { + availHeight = minDiffHeight } // clamp scroll maxScroll := len(lines) - availHeight @@ -295,26 +302,12 @@ s.WriteString("\n\n") case ws.PRStatus == "open" && ws.MergeConflicts: title += " " + conflictsBadgeStyle.Render("PR open · conflicts") if ci, ok := m.ciStatus[ws.Name]; ok { - switch ci { - case "success": - title += " " + ciSuccessStyle.Render("✓ CI") - case "failure": - title += " " + ciFailureStyle.Render("✗ CI") - case "pending": - title += " " + ciPendingStyle.Render("⟳ CI") - } + title += renderCIBadge(ci) } case ws.PRStatus == "open": title += " " + prOpenBadgeStyle.Render("PR open") if ci, ok := m.ciStatus[ws.Name]; ok { - switch ci { - case "success": - title += " " + ciSuccessStyle.Render("✓ CI") - case "failure": - title += " " + ciFailureStyle.Render("✗ CI") - case "pending": - title += " " + ciPendingStyle.Render("⟳ CI") - } + title += renderCIBadge(ci) } case ws.BranchPushed: title += " " + pushedBadgeStyle.Render("pushed") @@ -385,8 +378,8 @@ s.WriteString("\n\n") if m.agentPreview != "" && m.cursor < len(visible) { wsName := visible[m.cursor].Name previewWidth := m.width - 8 - if previewWidth < 20 { - previewWidth = 60 + if previewWidth < minPreviewWidth { + previewWidth = defaultPreviewWidth } content := previewTitleStyle.Render("Agent Output: "+wsName) + "\n" + previewLineStyle.Render(m.agentPreview) @@ -503,3 +496,15 @@ func (m Model) sortedWorkspaces() []WorkspaceItem { } return ws } + +func renderCIBadge(ci string) string { + switch ci { + case "success": + return " " + ciSuccessStyle.Render("✓ CI") + case "failure": + return " " + ciFailureStyle.Render("✗ CI") + case "pending": + return " " + ciPendingStyle.Render("⟳ CI") + } + return "" +} diff --git a/pkg/workspace/workspace.go b/pkg/workspace/workspace.go index aad1128..919bed0 100644 --- a/pkg/workspace/workspace.go +++ b/pkg/workspace/workspace.go @@ -91,6 +91,7 @@ func (s *Service) Create(name, baseBranch string) (*state.Workspace, error) { agentCmd := s.cfg.Agent.Command if err := s.process.CreateWindow(name, worktreePath, agentCmd, s.cfg.Agent.Args...); err != nil { + _ = s.worktrees.Delete(name, true) // cleanup orphaned worktree return nil, fmt.Errorf("failed to create tmux window: %w", err) } diff --git a/pkg/worktree/worktree.go b/pkg/worktree/worktree.go index 51254cd..c927e79 100644 --- a/pkg/worktree/worktree.go +++ b/pkg/worktree/worktree.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "github.com/axelgar/opentree/pkg/gitutil" @@ -384,10 +385,14 @@ func parseNumstat(output string) []FileChange { added := 0 removed := 0 if parts[0] != "-" { - fmt.Sscanf(parts[0], "%d", &added) + if n, err := strconv.Atoi(parts[0]); err == nil { + added = n + } } if parts[1] != "-" { - fmt.Sscanf(parts[1], "%d", &removed) + if n, err := strconv.Atoi(parts[1]); err == nil { + removed = n + } } files = append(files, FileChange{ FileName: parts[2], From a01a7b398d9f163da7ad0ca1fa373f600bfe8085 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 12 Mar 2026 11:04:48 +0100 Subject: [PATCH 2/2] fix: check gh exit code 4 for missing PR in FetchPRReviews gh CLI exits with status 4 (resource not found) when a branch has no PR, not just status 1 with a text message. Check the exit code first, then fall back to string matching for older gh versions. Co-Authored-By: Claude Sonnet 4.6 --- pkg/github/github.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/github/github.go b/pkg/github/github.go index eb03370..a55e64e 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -53,6 +53,11 @@ func (pm *PRManager) FetchPRReviews(branch string) ([]ReviewComment, error) { cmd := exec.Command("gh", "pr", "view", branch, "--json", "url,reviews") output, err := cmd.CombinedOutput() if err != nil { + // gh exits 4 when the resource (PR) is not found; also guard on output text + // for older gh versions that may use exit code 1 with a message. + if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 4 { + return nil, nil + } if strings.Contains(string(output), "no pull requests found") { return nil, nil }