feat: add configurable worktrees directory path#1322
feat: add configurable worktrees directory path#1322naaa760 wants to merge 1 commit intogeneralaction:mainfrom
Conversation
|
@naaa760 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 a global "Worktrees directory" setting that lets users choose where git worktrees are created, replacing the previously hard-coded However, three critical issues must be fixed before merging:
Confidence Score: 1/5
Sequence DiagramsequenceDiagram
participant User as User (Settings UI)
participant Renderer as RepositorySettingsCard
participant IPC as settingsIpc (Main)
participant Settings as settings.ts
participant Services as WorktreeService / WorktreePoolService / githubIpc
User->>Renderer: blur "Worktrees directory" input
Renderer->>IPC: rpc.appSettings.validateWorktreesPath(value)
IPC->>Settings: validateWorktreesPath(path)
Settings-->>IPC: null (ok) or error string
IPC-->>Renderer: null | error
alt validation passes
Renderer->>IPC: updateSettings({ repository: { worktreesDirectory: value } })
IPC->>Settings: updateAppSettings(partial)
Settings->>Settings: normalizeSettings() + validateWorktreesPath()
Settings-->>IPC: AppSettings (persisted to disk)
IPC-->>Renderer: updated settings
else validation fails
Renderer->>User: show inline error message
end
User->>Services: create worktree (task / PR / pool reserve)
Services->>Settings: getAppSettings()
Settings-->>Services: settings.repository.worktreesDirectory
Services->>Settings: getWorktreesBaseDir(projectPath, worktreesDirectory)
Settings-->>Services: resolved base directory path
Services->>Services: path.join(base, sluggedName-hash)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function validateWorktreesPath(dirPath: string): string | null { | ||
| const trimmed = typeof dirPath === 'string' ? dirPath.trim() : ''; | ||
| if (!trimmed) return null; | ||
| let expanded = trimmed; | ||
| if (expanded.startsWith('~')) { | ||
| expanded = join(homedir(), expanded.slice(1)); | ||
| } | ||
| const resolved = resolve(expanded); | ||
| if (existsSync(resolved)) { | ||
| try { | ||
| accessSync(resolved, constants.W_OK); | ||
| return null; | ||
| } catch { | ||
| return `Directory exists but is not writable: ${resolved}`; | ||
| } | ||
| } | ||
| const parent = dirname(resolved); | ||
| if (!existsSync(parent)) { | ||
| return `Parent directory does not exist: ${parent}`; | ||
| } | ||
| try { | ||
| accessSync(parent, constants.W_OK); | ||
| return null; | ||
| } catch { | ||
| return `Parent directory is not writable: ${parent}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
Validation and runtime path resolution use different bases for relative paths:
validateWorktreesPathresolves relative paths withresolve(expanded)→ anchored toprocess.cwd()getWorktreesBaseDirresolves relative paths withresolve(projectPath, expanded)→ anchored toprojectPath
When process.cwd() !== projectPath (typical in Electron), a relative path like ../shared-worktrees will pass validation (if it exists relative to cwd) but resolve to a different location at runtime (relative to projectPath).
Since this validator is exposed over IPC and the renderer can't meaningfully provide projectPath, reject relative paths entirely and accept only absolute paths or ~-prefixed home paths:
| export function validateWorktreesPath(dirPath: string): string | null { | |
| const trimmed = typeof dirPath === 'string' ? dirPath.trim() : ''; | |
| if (!trimmed) return null; | |
| let expanded = trimmed; | |
| if (expanded.startsWith('~')) { | |
| expanded = join(homedir(), expanded.slice(1)); | |
| } | |
| const resolved = resolve(expanded); | |
| if (existsSync(resolved)) { | |
| try { | |
| accessSync(resolved, constants.W_OK); | |
| return null; | |
| } catch { | |
| return `Directory exists but is not writable: ${resolved}`; | |
| } | |
| } | |
| const parent = dirname(resolved); | |
| if (!existsSync(parent)) { | |
| return `Parent directory does not exist: ${parent}`; | |
| } | |
| try { | |
| accessSync(parent, constants.W_OK); | |
| return null; | |
| } catch { | |
| return `Parent directory is not writable: ${parent}`; | |
| } | |
| } | |
| export function validateWorktreesPath(dirPath: string): string | null { | |
| const trimmed = typeof dirPath === 'string' ? dirPath.trim() : ''; | |
| if (!trimmed) return null; | |
| let expanded = trimmed; | |
| // Reject relative paths | |
| if (!expanded.startsWith('/') && !expanded.startsWith('~')) { | |
| return 'Path must be absolute or start with ~ (home directory)'; | |
| } | |
| if (expanded.startsWith('~')) { | |
| expanded = join(homedir(), expanded.slice(1)); | |
| } | |
| const resolved = resolve(expanded); | |
| // ... rest of validation |
| if (existsSync(resolved)) { | ||
| try { | ||
| accessSync(resolved, constants.W_OK); | ||
| return null; | ||
| } catch { | ||
| return `Directory exists but is not writable: ${resolved}`; | ||
| } |
There was a problem hiding this comment.
Validation accepts any writable path, including files. The accessSync(resolved, constants.W_OK) check passes for regular files as well as directories. If a user accidentally points to a file, validation silently succeeds but subsequent worktree creation will fail at the OS level.
Add a directory check:
| if (existsSync(resolved)) { | |
| try { | |
| accessSync(resolved, constants.W_OK); | |
| return null; | |
| } catch { | |
| return `Directory exists but is not writable: ${resolved}`; | |
| } | |
| if (existsSync(resolved)) { | |
| try { | |
| const stat = require('node:fs').lstatSync(resolved); | |
| if (!stat.isDirectory()) { | |
| return `Path exists but is not a directory: ${resolved}`; | |
| } | |
| accessSync(resolved, constants.W_OK); | |
| return null; | |
| } catch (e: any) { | |
| if (e?.message?.includes('is not a directory')) throw e; | |
| return `Directory exists but is not writable: ${resolved}`; | |
| } | |
| } |
1 participant
summary
Fixes
fix : #1313
Snapshot
Type of change
Mandatory Tasks
[ ] I have self-reviewed the code
[ ] A decent size PR without self-review might be rejected
Checklist
[ ] I have read the contributing guide
[ ] My code follows the style guidelines of this project (pnpm run format)
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have checked if my PR needs changes to the documentation
[ ] I have checked if my changes generate no new warnings (pnpm run lint)
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I haven't checked if new and existing unit tests pass locally with my changes
Ctrl+K to generate command