Fix new workspace to branch from default branch, not HEAD#170
Fix new workspace to branch from default branch, not HEAD#170andyrewlee merged 6 commits intomainfrom
Conversation
New workspaces previously defaulted to branching from HEAD, which resolves to whatever branch the primary checkout is on. Now resolves the repo's default branch via GetBaseBranch with a BranchExists guard, falling back to HEAD for repos with non-standard branch names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andyrewlee
left a comment
There was a problem hiding this comment.
Devin Review found 1 potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ pendingWorkspace still defaults empty base to "HEAD" instead of resolving the default branch like CreateWorkspace (internal/app/workspace_service_paths.go:60-61)
When a workspace is created from the dialog, Base: "" is passed through the message. handleCreateWorkspace at internal/app/app_input_messages_workspace.go:277 calls both pendingWorkspace(msg.Project, name, msg.Base) and createWorkspace(msg.Project, msg.Name, msg.Base) with the same empty msg.Base.
However, pendingWorkspace (internal/app/workspace_service_paths.go:60-61) still defaults empty base to "HEAD", while CreateWorkspace (internal/app/workspace_service.go:292-297) now resolves it via GetBaseBranch + BranchExists, yielding e.g. "main".
Root Cause and Impact
The pending workspace object (used to show an in-progress row in the dashboard during creation) is created with Base: "HEAD", while the actual workspace produced by CreateWorkspace gets Base: "main" (or whichever branch is resolved). This means the dashboard temporarily displays the wrong base ref while creation is in-flight.
This is not a severe functional issue because workspace identity (ID()) and creation-tracking (SetWorkspaceCreating) are both keyed on Repo/Root, not Base. But the pendingWorkspace function should mirror the new resolution logic introduced in CreateWorkspace for consistency.
View 4 additional findings in Devin Review.
GetBaseBranch now verifies the remote symbolic-ref branch exists locally before returning it, and returns an error instead of a silent "main" fallback. This removes the need for BranchExists and reduces git subprocess calls in the common case from 3 to 1. Also syncs pendingWorkspace default-base logic with CreateWorkspace. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andyrewlee
left a comment
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 GetFileDiff uses empty base string when GetBaseBranch fails, producing invalid git command (internal/git/diff.go:70-71)
The PR changes GetBaseBranch to return ("", error) instead of the previous fallback ("main", nil) when no default branch can be determined. However, an existing caller in GetFileDiff at internal/git/diff.go:70 discards the error with base, _ := GetBaseBranch(repoPath). When GetBaseBranch fails, base is now "", and the constructed diff argument becomes "...HEAD" (empty string + "...HEAD"), which is an invalid git argument.
Root Cause and Impact
Previously, GetBaseBranch always returned a non-empty string — it fell back to "main" at branch.go:34 (old code: return "main", nil). The caller at diff.go:70 relied on this guarantee:
base, _ := GetBaseBranch(repoPath)
args = []string{"diff", base + "...HEAD", ...}Now GetBaseBranch returns ("", error) on failure (branch.go:41), so base becomes "" and the git command becomes git diff ...HEAD which is invalid syntax. This will cause the git command to fail, and the diff will return an error result instead of showing the diff.
This affects repos where none of the candidate branches (main, master, develop, dev) exist locally and the remote default branch can't be resolved — exactly the "non-standard branch names" scenario this PR aims to handle gracefully. While the primary UI code path for DiffModeBranch was updated to use GetBranchFileDiff (which properly handles the error), GetFileDiff is a public API and the DiffModeBranch case is still reachable.
View 4 additional findings in Devin Review.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove always-true len(parts) > 0 guard in GetBaseBranch (strings.Split always returns at least one element) - Extract resolveBase helper to consolidate the trim/GetBaseBranch/HEAD fallback pattern used by pendingWorkspace and CreateWorkspace - Resolve base once in handleCreateWorkspace before passing to both pendingWorkspace and createWorkspace, eliminating theoretical inconsistency from duplicate GetBaseBranch calls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GetBaseBranch truncated branch names containing slashes (e.g. feature/foo → foo) because it split on "/" and took the last segment. Replace with strings.TrimPrefix to preserve the full branch name. Also adds remote tracking branch fallbacks for common candidates and symbolic-ref results, moves the blocking resolveBase call out of the synchronous handleCreateWorkspace handler, and makes pendingWorkspace use a simple TrimSpace+HEAD fallback instead of git calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The UI routes branch mode diffs through GetBranchFileDiff directly, making this case unreachable. It was also less correct — using base+"...HEAD" without computing merge-base first. Removing it so an accidental call falls through to the default (unstaged diff) rather than producing silently wrong branch diff behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New workspaces previously defaulted to branching from HEAD, which resolves to whatever branch the primary checkout is on. Now resolves the repo's default branch via GetBaseBranch with a BranchExists guard, falling back to HEAD for repos with non-standard branch names.
Summary
Describe the change and intended behavior.
Quality Checklist
make devchecklocally.make lint-strict-newlocally for changed code.make harness-presets.go test ./internal/tmux ./internal/e2e.