feat(worktree): add optional custom branch and worktree names#1380
feat(worktree): add optional custom branch and worktree names#1380drochag wants to merge 2 commits intogeneralaction:mainfrom
Conversation
Allow users to specify custom branch names and worktree directory names when creating tasks. When left empty, falls back to the existing auto-generation behavior. - Add validation helpers for branch/worktree names (nameValidation.ts) - Update WorktreeService and WorktreePoolService to accept custom names - Add input fields in TaskAdvancedSettings with real-time validation - Show dynamic placeholder suggesting derived worktree name from branch - Update IPC handlers and types throughout the stack
|
@drochag is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds optional custom branch and worktree directory names when creating tasks, with real-time frontend validation and backend sanitization. The overall structure is well-designed, but there are two logic bugs and one UX inconsistency worth addressing: Key changes:
Issues identified:
Confidence Score: 3/5
Sequence DiagramsequenceDiagram
actor User
participant TaskModal
participant nameValidation
participant taskCreationService
participant worktreeIpc (main)
participant WorktreePoolService
participant WorktreeService
User->>TaskModal: Type customBranchName / customWorktreeName
TaskModal->>nameValidation: validateBranchName(val)
nameValidation-->>TaskModal: null or error string
TaskModal->>nameValidation: validateWorktreeName(val)
nameValidation-->>TaskModal: null or error string
User->>TaskModal: Submit form
TaskModal->>taskCreationService: createTask({ customBranchName, customWorktreeName, ... })
alt Pool reserve available (local project)
taskCreationService->>worktreeIpc (main): worktreeClaimReserveAndSaveTask(args)
worktreeIpc (main)->>WorktreePoolService: claimReserve(..., customBranchName, customWorktreeName)
WorktreePoolService->>WorktreePoolService: transformReserve → sanitize names
WorktreePoolService-->>worktreeIpc (main): ClaimResult
worktreeIpc (main)-->>taskCreationService: { success, worktree }
else No pool reserve / fallback (local project)
taskCreationService->>worktreeIpc (main): worktreeCreate(args)
worktreeIpc (main)->>WorktreeService: createWorktree(..., customBranchName, customWorktreeName)
WorktreeService->>WorktreeService: sanitize names → git worktree add
WorktreeService-->>worktreeIpc (main): WorktreeInfo
worktreeIpc (main)-->>taskCreationService: { success, worktree }
else Remote project (bug: custom names dropped)
taskCreationService->>worktreeIpc (main): worktreeCreate(args)
worktreeIpc (main)->>worktreeIpc (main): isRemoteProject = true
worktreeIpc (main)->>worktreeIpc (main): remoteGitService.createWorktree(taskName, baseRef) ⚠️ custom names lost
worktreeIpc (main)-->>taskCreationService: { success, worktree }
end
taskCreationService-->>TaskModal: task created
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/main/services/WorktreeService.ts
Outdated
| /** Sanitize worktree directory name to ensure it's a valid path component */ | ||
| private sanitizeWorktreeName(name: string): string { | ||
| return name | ||
| .replace(/[/\\:*?"<>|]/g, '-') | ||
| .replace(/\s+/g, '-') | ||
| .replace(/-+/g, '-') | ||
| .replace(/^-|-$/g, '') | ||
| .slice(0, 100); | ||
| } |
There was a problem hiding this comment.
sanitizeWorktreeName can return an empty string when the input consists entirely of characters that get converted to dashes. For example, an input like --- or ??? passes frontend validation (those characters aren't caught by validateWorktreeName) but produces an empty string here, because:
- Characters like
?are replaced with- - Multiple dashes are collapsed to one via
/-+/g - Leading/trailing dashes are stripped via
/^-|-$/g - Result: empty string
This causes the worktree path to resolve to the worktrees/ directory itself rather than a subdirectory, leading to invalid git-worktree operations or filesystem errors.
A fallback is needed when sanitization results in an empty string:
private sanitizeWorktreeName(name: string): string {
const sanitized = name
.replace(/[/\\:*?"<>|]/g, '-')
.replace(/\s+/g, '-')
.replace(/-+/g, '-')
.replace(/^-|-$/g, '')
.slice(0, 100);
return sanitized || `worktree-${this.generateShortHash()}`;
}This issue also exists in WorktreePoolService.sanitizeWorktreeName (line 697).
| export function validateBranchName(name: string): string | null { | ||
| // Empty is valid (will auto-generate) | ||
| if (!name || !name.trim()) { | ||
| return null; | ||
| } | ||
|
|
||
| const trimmed = name.trim(); | ||
|
|
||
| if (trimmed.length > 250) { | ||
| return 'Branch name must be 250 characters or less'; | ||
| } | ||
|
|
||
| if (/\s/.test(trimmed)) { | ||
| return 'Branch name cannot contain whitespace'; | ||
| } | ||
|
|
||
| if (trimmed.includes('..')) { | ||
| return 'Branch name cannot contain ".."'; | ||
| } | ||
|
|
||
| if (trimmed.startsWith('-')) { | ||
| return 'Branch name cannot start with "-"'; | ||
| } | ||
|
|
||
| if (trimmed.endsWith('.lock')) { | ||
| return 'Branch name cannot end with ".lock"'; | ||
| } | ||
|
|
||
| if (trimmed.includes('@{')) { | ||
| return 'Branch name cannot contain "@{"'; | ||
| } | ||
|
|
||
| // Git also disallows some other characters, but we'll let the backend handle those | ||
| // via sanitization. These are the most common user errors. | ||
|
|
||
| return null; |
There was a problem hiding this comment.
validateBranchName misses several characters that git check-ref-format forbids: ~, ^, :, ?, *, [, and \. These characters are currently accepted by the frontend validator but silently transformed to - by the backend sanitizeBranchName, creating a confusing UX gap.
For example, a user typing feature~1 sees no validation error in the UI but ends up with a different branch name (feature-1) than what they entered.
Adding these characters to the validation would make the frontend and backend consistent:
if (/[\s~^:?*[\\\x00-\x1f\x7f]/.test(trimmed)) {
return 'Branch name contains invalid characters (spaces, ~, ^, :, ?, *, [, \\)';
}| /** Sanitize branch name to ensure it's a valid Git ref */ | ||
| private sanitizeBranchName(name: string): string { | ||
| let n = name | ||
| .replace(/\s+/g, '-') | ||
| .replace(/[^A-Za-z0-9._\/-]+/g, '-') | ||
| .replace(/-+/g, '-') | ||
| .replace(/\/+/g, '/'); | ||
| n = n.replace(/^[./-]+/, '').replace(/[./-]+$/, ''); | ||
| if (!n || n === 'HEAD') { | ||
| n = `emdash/${this.slugify('task')}-${this.generateShortHash()}`; | ||
| } | ||
| return n; | ||
| } | ||
|
|
||
| /** Sanitize worktree directory name to ensure it's a valid path component */ | ||
| private sanitizeWorktreeName(name: string): string { | ||
| return name | ||
| .replace(/[/\\:*?"<>|]/g, '-') | ||
| .replace(/\s+/g, '-') | ||
| .replace(/-+/g, '-') | ||
| .replace(/^-|-$/g, '') | ||
| .slice(0, 100); | ||
| } |
There was a problem hiding this comment.
sanitizeBranchName and sanitizeWorktreeName are duplicated verbatim in both WorktreeService (lines 401–422) and WorktreePoolService (lines 682–704). Keeping two copies means any future bug-fix or rule change must be applied in both places — and divergence is one commit away from happening.
Consider extracting these helpers into a shared module (e.g. src/main/lib/worktreeNameUtils.ts) and importing them in both services to prevent drift.
- Extract sanitization functions to shared module (worktreeNameUtils.ts) - Add fallback for empty sanitized worktree names - Add missing invalid characters to frontend branch name validation (~, ^, :, ?, *, [, \, and control characters)
|
Thanks for opening the PR and for your interest in contributing! Will have a look and let you know shortly:) |
|
Have been waiting for this for awhile, I had to rename branch name manually all the while |
3 participants
Summary
{prefix}/{slugged-name}-{hash})Changes
src/renderer/lib/nameValidation.ts- validation helpersWorktreeService,WorktreePoolService, andworktreeIpcto accept custom namesTaskAdvancedSettingswith validation and dynamic placeholderelectron-api.d.tswith new optional parametersTest plan
/) → shows validation error