v0.21.15.0 fix(workspace): sanitize default branch name so spaces don't break worktree create#67
Merged
Merged
Conversation
…t worktree add A workspace name with a space (e.g. "testing codex") fataled creation: `git worktree add -b 'testing codex'` rejects the ref. canopy sanitized the on-disk path but passed the raw name to git as the branch. Create now derives the default branch via git.Sanitize(name) on the fallback path only. An explicit --branch is still passed verbatim (feature/oauth is a valid ref Sanitize would mangle). This realigns branch == path basename == tmux session for every input. Tests cover both conditional branches plus Sanitize edge cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Performance / correctness fix. Typing a workspace name with a space in the TUI new-workspace box (e.g.
testing codex) fataled creation:canopy sanitized the name correctly for the on-disk path (
testing codex→testing-codex) but passed the raw, space-bearing name straight togit worktree add -bas the branch. Git accepts slashes in refs (feature/oauth) but rejects spaces, colons,..,~— so any git-invalid name broke creation.The fix sanitizes the branch on the fallback path only (
internal/workspace/lifecycle.goCreate):branch := git.Sanitize(name)when no explicit--branchwas supplied. An explicitcanopy new x --branch feature/yis still passed to git verbatim — the caller knows what they want, andfeature/yis a valid refSanitizewould have mangled tofeature-y. This also realigns a latent inconsistency: the on-disk path basename was alreadygit.Sanitize(name), so for any space-bearing name the branch and path had silently diverged. Nowbranch == path basename == tmux sessionholds for every input (the v0 "four names match" invariant).internal/git/worktree.go'sSanitizedoc comment was rewritten to drop the now-false "the git branch keeps its original name" claim.Test Coverage
All new code paths covered (effectively 100%):
TestCreate_SanitizesBranchNameWhenNoExplicitBranch—testing codex→ branchtesting-codex, creates cleanly, verifies the real git branch viarev-parse.TestCreate_PreservesExplicitBranchName—--branch feature/ysurvives intact (not sanitized).TestCreate_AlreadyValidNameUnchanged— guards against over-sanitizing valid names.TestSanitize— +6 table cases (interior space, collapsing spaces, padded interior, multiple spaces, tilde, idempotent).Both branches of the
if opts.Branch != ""conditional are exercised.Tests:
internal/workspace+3 funcs;internal/gitTestSanitize16 → 22 cases.Pre-Landing Review
No issues. Diff is minimal with no SQL/LLM/security surface. Adversarial note:
git.Sanitizecould collapse a pathological all-symbol name (e.g."///") to"", but that's pre-existing (the path basename already usedSanitize), not introduced here, and namegen handles the empty-name case. The dot-sequence gap (.., trailing.,.locksurviveSanitize) is a rarer instance of the same class — captured as a P3 follow-up in TODOS.md rather than expanded here.Verification
go build ./...— OKgo vet ./...— cleango test ./...— all packages passgo test -tags=e2e ./internal/git/ ./internal/workspace/— pass (real git + tmux)Test plan
go test ./...)🤖 Generated with Claude Code