feat: add UI configuration for custom worktree location #456
Conversation
Allows users to configure worktree location per project through the project settings UI. The custom path is stored in the project's .env file as WORKTREE_BASE_PATH. Changes: - Add worktreePath field to ProjectEnvConfig type - Create WorktreeSettings UI component with path input and browser - Integrate WorktreeSettings into GeneralSettings page - Update env-handlers to read/write WORKTREE_BASE_PATH - Replace all hardcoded .worktrees references with env var Backend files updated: - cli/utils.py - Use WORKTREE_BASE_PATH for spec finding - cli/batch_commands.py - Use env var for cleanup operations - core/workspace/git_utils.py - Use env var for worktree lookups - core/workspace/models.py - Use env var for spec number scanning Frontend files updated: - project-store.ts - Use env var for task scanning - WorktreeSettings.tsx - New component for UI configuration - GeneralSettings.tsx - Integrate worktree settings section - env-handlers.ts - Handle WORKTREE_BASE_PATH in .env file Features: - Supports both relative and absolute paths - Shows resolved path preview - Converts paths intelligently (relative if inside project) - Provides examples and help text - Defaults to .worktrees when not configured
Replace window.require('path') with custom browser-compatible path
utilities to avoid 'window.require is not a function' error in the
Electron renderer process.
- Add isAbsolutePath() to detect absolute paths (Unix/Windows/UNC)
- Add joinPath() for simple path joining
- Add getRelativePath() to convert absolute to relative paths
- All functions work in browser context without Node.js modules
…ager Critical fixes to make worktree path configuration actually work: 1. WorktreeManager now reads WORKTREE_BASE_PATH environment variable - Previously hardcoded to .worktrees - Now respects custom path from project settings 2. Load project's .env file after determining project directory - Backend was only loading apps/backend/.env - Now also loads <project>/.auto-claude/.env with override=True - This ensures WORKTREE_BASE_PATH is available to all code Without these changes, the UI setting was being saved but never used. Files modified: - apps/backend/core/worktree.py - Use WORKTREE_BASE_PATH env var - apps/backend/cli/main.py - Load project .env file
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds configurable worktree base path support end-to-end: introduces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as GeneralSettings / WorktreeSettings
participant IPC as Env IPC Handler
participant FS as File System (.env)
participant Backend as Backend Worktree Logic
User->>UI: Open Project Settings
UI->>IPC: ENV_GET
IPC->>FS: Read project .env (includes WORKTREE_BASE_PATH)
FS-->>IPC: .env contents
IPC-->>UI: ProjectEnvConfig { worktreePath }
User->>UI: Edit worktree base path
UI->>IPC: ENV_UPDATE { worktreePath }
IPC->>FS: Write updated .env
FS-->>IPC: Write success
IPC-->>UI: Ack / updated config
Note right of Backend: On startup or when enumerating worktrees
Backend->>Backend: get_worktree_base_path(project_dir)
Backend->>FS: Resolve worktrees dir and enumerate specs
FS-->>Backend: Spec list / paths
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
Summary of ChangesHello @sbeardsley, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant quality-of-life improvement by enabling users to customize the storage location for Git worktrees. This feature, accessible through the project settings UI, enhances flexibility for developers who might prefer to store worktrees outside the default Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a great new feature allowing users to configure a custom worktree location. The changes are well-structured across the frontend and backend. My review focuses on improving maintainability by reducing code duplication for configuration values in both Python and TypeScript, and suggests a more robust approach for the path utility functions in the new frontend component. Overall, a solid implementation.
apps/backend/cli/batch_commands.py
Outdated
| worktree_base_path = os.getenv("WORKTREE_BASE_PATH", ".worktrees") | ||
| worktrees_dir = Path(project_dir) / worktree_base_path |
There was a problem hiding this comment.
The logic for retrieving the worktree base path is duplicated across multiple Python files (e.g., cli/batch_commands.py, cli/utils.py, core/worktree.py). This use of magic strings for the environment variable name and default path makes the code harder to maintain.
To improve this, I recommend centralizing this configuration. You could create a new core/config.py module to hold these constants and a helper function.
core/config.py:
import os
WORKTREE_BASE_PATH_VAR = "WORKTREE_BASE_PATH"
DEFAULT_WORKTREE_PATH = ".worktrees"
def get_worktree_base_path() -> str:
"""Gets the worktree base path from environment variables, with a default."""
return os.getenv(WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH)Then, you could import and use this function across the backend, ensuring a single source of truth. For example, in this file:
from core.config import get_worktree_base_path
# ...
worktree_base_path = get_worktree_base_path()
worktrees_dir = Path(project_dir) / worktree_base_pathThis change would significantly improve the maintainability of your configuration.
| const worktreeBasePath = process.env.WORKTREE_BASE_PATH || '.worktrees'; | ||
| const worktreesDir = path.join(project.path, worktreeBasePath); |
There was a problem hiding this comment.
The environment variable name WORKTREE_BASE_PATH and the default path '.worktrees' are hardcoded here. These values are also used in other frontend files (WorktreeSettings.tsx, env-handlers.ts), which can lead to inconsistencies if they need to be changed.
To avoid this, I recommend defining these as shared constants, for example in a new file src/shared/constants/worktree.ts and exporting them through src/shared/constants/index.ts.
src/shared/constants/worktree.ts:
export const WORKTREE_BASE_PATH_VAR = 'WORKTREE_BASE_PATH';
export const DEFAULT_WORKTREE_PATH = '.worktrees';Then you can import and use these constants consistently. For this file, it would look like:
import { WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH } from '../../shared/constants';
// ...
const worktreeBasePath = process.env[WORKTREE_BASE_PATH_VAR] || DEFAULT_WORKTREE_PATH;
const worktreesDir = path.join(project.path, worktreeBasePath);This will make your configuration more robust and easier to manage.
| const isAbsolutePath = (p: string): boolean => { | ||
| // Unix absolute path starts with / | ||
| // Windows absolute path starts with drive letter (C:\) or UNC path (\\) | ||
| return p.startsWith('/') || /^[a-zA-Z]:[/\\]/.test(p) || p.startsWith('\\\\'); | ||
| }; | ||
|
|
||
| const joinPath = (...parts: string[]): string => { | ||
| // Simple path join that works in browser | ||
| return parts.join('/').replace(/\/+/g, '/'); | ||
| }; | ||
|
|
||
| const getRelativePath = (from: string, to: string): string => { | ||
| // Normalize paths | ||
| const fromParts = from.split(/[/\\]/).filter(Boolean); | ||
| const toParts = to.split(/[/\\]/).filter(Boolean); | ||
|
|
||
| // Find common base | ||
| let commonLength = 0; | ||
| const minLength = Math.min(fromParts.length, toParts.length); | ||
| for (let i = 0; i < minLength; i++) { | ||
| if (fromParts[i] === toParts[i]) { | ||
| commonLength = i + 1; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If no common base, paths are on different roots | ||
| if (commonLength === 0) { | ||
| return to; // Return absolute path | ||
| } | ||
|
|
||
| // Build relative path | ||
| const upCount = fromParts.length - commonLength; | ||
| const downParts = toParts.slice(commonLength); | ||
|
|
||
| if (upCount === 0 && downParts.length === 0) { | ||
| return '.'; | ||
| } | ||
|
|
||
| const ups = Array(upCount).fill('..'); | ||
| return [...ups, ...downParts].join('/'); | ||
| }; |
There was a problem hiding this comment.
The custom path utility functions (isAbsolutePath, joinPath, getRelativePath) are a good effort to avoid Node.js dependencies in the renderer. However, re-implementing path logic, especially getRelativePath, can be complex and prone to edge cases (e.g., handling . or .. segments within paths, case-sensitivity on different filesystems).
While this implementation seems to work for the common cases of absolute paths returned by the dialog, it could become a source of bugs if used in more complex scenarios.
For improved robustness and to avoid maintaining custom path logic, consider using a well-tested, browser-compatible library like path-browserify. It's a polyfill for Node's path module and would handle all edge cases correctly.
If you prefer to keep the custom implementation to minimize dependencies, I'd recommend adding more comprehensive unit tests for these utility functions to cover various edge cases.
| ? (isAbsolutePath(worktreePath) | ||
| ? worktreePath | ||
| : joinPath(projectPath, worktreePath)) | ||
| : joinPath(projectPath, '.worktrees'); |
There was a problem hiding this comment.
The default worktree path '.worktrees' is hardcoded here. This value is also used in other places like project-store.ts and env-handlers.ts.
To improve maintainability and ensure consistency, this should be a shared constant as suggested in my other comment.
Using a constant would look like this:
import { DEFAULT_WORKTREE_PATH } from '../../../shared/constants';
// ...
: joinPath(projectPath, DEFAULT_WORKTREE_PATH);Resolved merge conflict in package-lock.json by accepting upstream version and regenerating.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (12)
apps/backend/cli/batch_commands.pyapps/backend/cli/main.pyapps/backend/cli/utils.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/workspace/models.pyapps/backend/core/worktree.pyapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.tsapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/settings/sections/SectionRouter.tsxapps/frontend/src/shared/types/project.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/settings/sections/SectionRouter.tsxapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/settings/sections/SectionRouter.tsxapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/settings/sections/SectionRouter.tsxapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.ts
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/core/workspace/models.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.pyapps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/cli/main.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/core/workspace/models.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.pyapps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/cli/main.py
apps/backend/core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement dynamic command allowlisting based on detected project stack using
core/security.pyfor bash command validation
Files:
apps/backend/core/workspace/models.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.py
🧠 Learnings (2)
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Use git worktrees for isolated agent builds with branches following the naming pattern `auto-claude/{spec-name}`
Applied to files:
apps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.ts
📚 Learning: 2025-12-19T15:00:48.233Z
Learnt from: AndyMik90
Repo: AndyMik90/Auto-Claude PR: 41
File: auto-claude/qa/loop.py:126-136
Timestamp: 2025-12-19T15:00:48.233Z
Learning: In auto-claude/qa/loop.py, when creating clients for QA fixer sessions (including human feedback processing), use get_phase_model(spec_dir, "qa", model) instead of hardcoding "sonnet" as the fallback to support dynamic model selection based on profiles.
Applied to files:
apps/backend/cli/main.py
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (1)
apps/frontend/src/shared/types/project.ts (1)
ProjectEnvConfig(274-322)
apps/frontend/src/renderer/components/settings/sections/SectionRouter.tsx (1)
apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (2)
envConfig(23-23)updateEnvConfig(29-29)
apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx (3)
apps/frontend/src/shared/types/project.ts (1)
ProjectEnvConfig(274-322)apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (1)
WorktreeSettings(59-141)apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (2)
envConfig(23-23)updateEnvConfig(29-29)
🔇 Additional comments (18)
apps/backend/cli/main.py (1)
281-286: LGTM! Project-specific environment loading is correctly implemented.The project-specific
.envfile is loaded withoverride=Trueafter the project directory is determined, which correctly allows project settings to override backend defaults. The existence check prevents errors when the file doesn't exist.apps/backend/cli/utils.py (1)
85-86: Verify path traversal safety with custom worktree paths.The
WORKTREE_BASE_PATHenvironment variable is user-configurable and could contain:
- Absolute paths (e.g.,
/tmp/worktrees) - Python'sPathoperator will ignoreproject_dirwhen joining with an absolute path- Relative paths with
..(e.g.,../../worktrees) - Could escape the project directoryWhile this may be intended behavior for flexibility, consider:
- Documenting the security implications in the
.envtemplate- Adding validation if paths must remain within the project directory
- Or explicitly allowing absolute paths if that's the intended design
Based on the PR objectives mentioning "supports relative and absolute paths", this appears to be intentional. Please confirm this is the desired behavior and ensure it's properly documented for users.
apps/frontend/src/main/project-store.ts (1)
267-268: LGTM! Consistent with backend implementation.The worktree base path configuration is correctly implemented using
process.env.WORKTREE_BASE_PATHwith the same default as the backend. This ensures consistent behavior across frontend and backend.Note: Same path traversal considerations as the backend apply here - the environment variable can contain absolute or relative paths that may escape the project directory, which appears intentional per the PR objectives.
apps/frontend/src/main/ipc-handlers/env-handlers.ts (3)
101-103: LGTM! Worktree path configuration is correctly integrated.The worktreePath field is properly mapped to the
WORKTREE_BASE_PATHenvironment variable, consistent with other configuration fields in this handler.
188-191: Excellent documentation in .env template.The comments clearly explain the worktree path configuration, including support for both relative and absolute paths, and the default behavior. This will help users understand and configure this feature correctly.
346-348: LGTM! Environment variable reading is correctly implemented.The
WORKTREE_BASE_PATHenvironment variable is correctly read from the.envfile and mapped toconfig.worktreePath, completing the read-modify-write cycle for this configuration field.apps/frontend/src/shared/types/project.ts (1)
307-307: LGTM! Type definition is correctly added.The
worktreePathfield is properly defined as an optional string in theProjectEnvConfiginterface, with clear documentation and logical placement in the Git/Worktree Settings section.apps/frontend/src/renderer/components/settings/sections/SectionRouter.tsx (1)
103-104: LGTM! Props are correctly passed to GeneralSettings.The
envConfigandupdateEnvConfigprops are properly passed to theGeneralSettingscomponent, consistent with how other settings sections receive environment configuration. This enables the worktree configuration UI in the General settings.apps/backend/core/workspace/models.py (2)
9-9: LGTM! Import is necessary for environment variable access.The
osimport is correctly added to support reading theWORKTREE_BASE_PATHenvironment variable.
253-254: LGTM! Worktree path is correctly configurable.The
WORKTREE_BASE_PATHenvironment variable is properly used for spec number coordination. Since spec numbering must scan all locations (main + worktrees) to prevent collisions, it's critical that this path matches what other components use.The implementation correctly reads from the environment on each call, ensuring consistency with other components like
WorktreeManager.apps/backend/core/worktree.py (1)
59-62: LGTM! Core worktree path configuration is correctly implemented.The
WorktreeManagernow correctly reads the worktree base path from theWORKTREE_BASE_PATHenvironment variable during initialization. The path is stored as an instance variable and used consistently throughout all worktree operations.Note: The same path traversal considerations mentioned for other files apply here - the environment variable can contain absolute or relative paths that may reference locations outside the project directory. This appears intentional based on the PR objectives supporting "relative and absolute paths."
The
setup()method (line 201) creates the directory if needed, which correctly handles custom paths.apps/backend/core/workspace/git_utils.py (1)
10-10: LGTM!The
osimport is appropriately added to support environment variable reading for the configurable worktree base path.apps/backend/cli/batch_commands.py (2)
9-9: LGTM!The
osimport is correctly added to support reading theWORKTREE_BASE_PATHenvironment variable.
214-214: Good UX improvement!Displaying the actual configured
worktree_base_pathinstead of the hardcoded.worktreesprovides better user feedback during dry-run operations.apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (1)
59-91: Component logic looks sound.The path resolution logic correctly handles both absolute and relative paths, and the browse handler intelligently converts selected paths to relative notation when inside the project directory.
apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx (3)
24-27: LGTM!The imports are correctly updated to include
ProjectEnvConfigtype and the newWorktreeSettingscomponent.
37-39: LGTM!The props are correctly typed with
ProjectEnvConfig | nullforenvConfigand a properly typed update function.
48-50: LGTM!The props are correctly destructured from the component parameters and match the interface definition.
| {/* Worktree Location */} | ||
| <section className="space-y-4"> | ||
| <h3 className="text-sm font-semibold text-foreground">Worktree Location</h3> | ||
| <WorktreeSettings | ||
| envConfig={envConfig} | ||
| updateEnvConfig={updateEnvConfig} | ||
| projectPath={project.path} | ||
| /> | ||
| </section> | ||
|
|
There was a problem hiding this comment.
Fix: Use i18n translation key for section heading.
The section heading "Worktree Location" (line 161) is hardcoded and violates the coding guideline requiring i18n translation keys for all user-facing text in the frontend.
The WorktreeSettings component integration is otherwise correctly implemented with proper props passing.
🔎 Proposed fix
{/* Worktree Location */}
<section className="space-y-4">
- <h3 className="text-sm font-semibold text-foreground">Worktree Location</h3>
+ <h3 className="text-sm font-semibold text-foreground">{t('projectSections.worktree.title')}</h3>
<WorktreeSettings
envConfig={envConfig}
updateEnvConfig={updateEnvConfig}As per coding guidelines, use the useTranslation() hook with namespace prefixes.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
around lines 159-168, the heading "Worktree Location" is hardcoded; replace it
with an i18n key by using the useTranslation hook with the component namespace
(e.g., const { t } = useTranslation('project-settings')) and change the h3
content to t('worktreeLocation') (or t('general.worktreeLocation') if you use a
sub-key); ensure the translation key is added to the appropriate locale files
and remove the hardcoded string.
| const isAbsolutePath = (p: string): boolean => { | ||
| // Unix absolute path starts with / | ||
| // Windows absolute path starts with drive letter (C:\) or UNC path (\\) | ||
| return p.startsWith('/') || /^[a-zA-Z]:[/\\]/.test(p) || p.startsWith('\\\\'); | ||
| }; | ||
|
|
||
| const joinPath = (...parts: string[]): string => { | ||
| // Simple path join that works in browser | ||
| return parts.join('/').replace(/\/+/g, '/'); | ||
| }; | ||
|
|
||
| const getRelativePath = (from: string, to: string): string => { | ||
| // Normalize paths | ||
| const fromParts = from.split(/[/\\]/).filter(Boolean); | ||
| const toParts = to.split(/[/\\]/).filter(Boolean); | ||
|
|
||
| // Find common base | ||
| let commonLength = 0; | ||
| const minLength = Math.min(fromParts.length, toParts.length); | ||
| for (let i = 0; i < minLength; i++) { | ||
| if (fromParts[i] === toParts[i]) { | ||
| commonLength = i + 1; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If no common base, paths are on different roots | ||
| if (commonLength === 0) { | ||
| return to; // Return absolute path | ||
| } | ||
|
|
||
| // Build relative path | ||
| const upCount = fromParts.length - commonLength; | ||
| const downParts = toParts.slice(commonLength); | ||
|
|
||
| if (upCount === 0 && downParts.length === 0) { | ||
| return '.'; | ||
| } | ||
|
|
||
| const ups = Array(upCount).fill('..'); | ||
| return [...ups, ...downParts].join('/'); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxRepository: AndyMik90/Auto-Claude
Length of output: 5907
🏁 Script executed:
cd apps/frontend && cat package.json | head -50Repository: AndyMik90/Auto-Claude
Length of output: 2321
🏁 Script executed:
rg -n "isAbsolutePath|joinPath|getRelativePath" apps/frontend/src/Repository: AndyMik90/Auto-Claude
Length of output: 1198
🏁 Script executed:
rg -n "from ['\"]pathe['\"]|import.*pathe" apps/frontend/Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
fd -e json package.json apps/frontend/ --exec grep -l "pathe"Repository: AndyMik90/Auto-Claude
Length of output: 47
Wrap hardcoded user-facing text with i18n translation keys.
The useTranslation(['settings']) hook is imported and initialized (line 64) but unused. All UI text should use the t function with namespace prefixes per coding guidelines. Apply translations to:
- Line 78: "Select Worktree Location"
- Lines 98-99: Worktrees description and default path info
- Line 104: "Worktree Base Path"
- Line 108: Input placeholder
- Lines 121-123: Path format information and examples
- Lines 127, 132-137: "Resolved Path", "Common Use Cases" labels and descriptions
The custom path utilities (lines 9-51) handle the required use cases correctly, including Windows cross-drive detection.
🤖 Prompt for AI Agents
In apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx
around lines 9 to 51 and UI lines 64–137, the file defines path helper utilities
(which are fine and should remain) but contains hardcoded user-facing strings
instead of using the initialized useTranslation(['settings']) hook; replace each
literal with t('settings:<key>') keys using the settings namespace: change
"Select Worktree Location" (line ~78) to t('settings:selectWorktreeLocation'),
the worktrees description and default path info (lines ~98-99) to
t('settings:worktreesDescription') and t('settings:defaultPathInfo'), "Worktree
Base Path" (line ~104) to t('settings:worktreeBasePath'), the input placeholder
(line ~108) to t('settings:worktreeBasePathPlaceholder'), the path format info
and examples (lines ~121-123) to appropriate keys like
t('settings:pathFormatInfo') and t('settings:pathExamples'), and the "Resolved
Path", "Common Use Cases" labels and descriptions (lines ~127, 132-137) to
t('settings:resolvedPath') and t('settings:commonUseCases')/related keys; ensure
the component actually calls const { t } = useTranslation(['settings']) and
update any JSX to use these t(...) calls so there are no unused imports or
hardcoded strings.
| return ( | ||
| <section className="space-y-4"> | ||
| <div className="flex items-start gap-2 rounded-lg border border-border bg-muted/50 p-3"> | ||
| <Info className="h-4 w-4 text-muted-foreground mt-0.5 shrink-0" /> | ||
| <div className="text-xs text-muted-foreground space-y-1"> | ||
| <p>Worktrees are isolated Git branches where Auto Claude builds features safely.</p> | ||
| <p className="font-medium">Default: <code>.worktrees/</code> in your project root</p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <Label htmlFor="worktreePath">Worktree Base Path</Label> | ||
| <div className="flex gap-2"> | ||
| <Input | ||
| id="worktreePath" | ||
| placeholder=".worktrees (default)" | ||
| value={worktreePath} | ||
| onChange={(e) => updateEnvConfig({ worktreePath: e.target.value })} | ||
| /> | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| size="icon" | ||
| onClick={handleBrowse} | ||
| > | ||
| <FolderOpen className="h-4 w-4" /> | ||
| </Button> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground"> | ||
| Supports relative paths (e.g., <code>worktrees</code>) or absolute paths (e.g., <code>/tmp/worktrees</code>) | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="rounded-lg border border-border bg-muted/50 p-3"> | ||
| <p className="text-xs font-medium text-foreground">Resolved Path:</p> | ||
| <code className="text-xs text-muted-foreground break-all">{resolvedPath}</code> | ||
| </div> | ||
|
|
||
| <div className="text-xs text-muted-foreground space-y-1"> | ||
| <p className="font-medium">Common Use Cases:</p> | ||
| <ul className="list-disc list-inside space-y-0.5 ml-2"> | ||
| <li>External drive: <code>/Volumes/FastSSD/worktrees</code></li> | ||
| <li>Temp directory: <code>/tmp/my-project-worktrees</code></li> | ||
| <li>Shared builds: <code>../shared-worktrees</code></li> | ||
| </ul> | ||
| </div> | ||
| </section> | ||
| ); |
There was a problem hiding this comment.
Critical: Replace all hardcoded strings with i18n translation keys.
All user-facing text in this component violates the coding guideline: "Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings." The useTranslation hook is imported but not utilized for any of the text content.
Hardcoded strings that need translation keys:
- "Worktrees are isolated Git branches..." (line 98)
- "Default: .worktrees/ in your project root" (line 99)
- "Worktree Base Path" (line 104)
- ".worktrees (default)" (line 108)
- "Supports relative paths..." (lines 121-122)
- "Resolved Path:" (line 127)
- "Common Use Cases:" (line 132)
- Example paths (lines 134-136)
🔎 Example fix for a few strings
- <Label htmlFor="worktreePath">Worktree Base Path</Label>
+ <Label htmlFor="worktreePath">{t('settings:projectSections.worktree.basePathLabel')}</Label>
<Input
id="worktreePath"
- placeholder=".worktrees (default)"
+ placeholder={t('settings:projectSections.worktree.basePathPlaceholder')}
value={worktreePath}
onChange={(e) => updateEnvConfig({ worktreePath: e.target.value })}
/>
- <p className="text-xs text-muted-foreground">
- Supports relative paths (e.g., <code>worktrees</code>) or absolute paths (e.g., <code>/tmp/worktrees</code>)
- </p>
+ <p className="text-xs text-muted-foreground">
+ {t('settings:projectSections.worktree.pathTypeDescription')}
+ </p>All strings must be moved to translation files following the project's i18n namespace conventions.
As per coding guidelines, use useTranslation() hook with namespace prefixes.
Committable suggestion skipped: line range outside the PR's diff.
|
I have read the CLA Document and I hereby sign the CLA |
- Add centralized backend config with path validation - Create apps/backend/core/config.py with get_worktree_base_path() - Validates paths to prevent .auto-claude/ and .git/ locations - Supports both relative and absolute paths (intentional for external drives) - Add frontend shared constants to reduce duplication - Create apps/frontend/src/shared/constants/worktree.ts - Export WORKTREE_BASE_PATH_VAR and DEFAULT_WORKTREE_PATH - Fix i18n compliance - replace all hardcoded strings with translation keys - Add worktree translations to en/settings.json and fr/settings.json - Update WorktreeSettings.tsx to use Trans component for formatted text - Update GeneralSettings.tsx heading to use translation key - Update all backend files to use centralized config - apps/backend/core/worktree.py - apps/backend/cli/batch_commands.py - apps/backend/cli/utils.py - apps/backend/core/workspace/git_utils.py - apps/backend/core/workspace/models.py - Update frontend to use shared constants - apps/frontend/src/main/project-store.ts Addresses CodeRabbit and Gemini Code Assist review comments on PR AndyMik90#456
MikeeBuilds
added
feature
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
6 issue(s) must be addressed (3 required, 3 recommended), 1 suggestions
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Medium | Based on lines changed |
| Security Impact | Low | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 3 issue(s)
- Medium: 3 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (7 selected of 7 total)
🟠 [HIGH] All user-facing strings in new component are hardcoded without i18n
📁 apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx:1
The new WorktreeSettings.tsx component (141 lines) contains 15+ hardcoded user-facing strings that violate the project's critical i18n requirement from CLAUDE.md: 'CRITICAL: Always use i18n translation keys for all user-facing text in the frontend.' Hardcoded strings include: 'Worktree Base Path', 'Worktrees are isolated Git branches...', 'Default:', 'Supports relative paths...', 'Resolved Path:', 'Common Use Cases:', 'External drive:', 'Temp directory:', 'Shared builds:', 'Select Worktree Location', and button/placeholder text. This affects localization support for French and any future languages.
Suggested fix:
1. Add translation keys to apps/frontend/src/shared/i18n/locales/en/settings.json under a new 'worktree' section. 2. Add corresponding French translations to fr/settings.json. 3. Import useTranslation and replace all hardcoded strings with t('settings:worktree.keyName') calls.
🟠 [HIGH] Hardcoded 'Worktree Location' section heading violates i18n
📁 apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx:161
The section heading 'Worktree Location' on line 161 is hardcoded in JSX. Per project guidelines, all section headings must use translation keys. Other headings in this file (Auto-Build Integration, Agent Configuration, Notifications) are also hardcoded, indicating a broader pattern of i18n non-compliance in project-settings components.
Suggested fix:
Replace with {t('projectSections.general.worktreeLocation')} and add the key to both en/settings.json and fr/settings.json
🟡 [MEDIUM] WORKTREE_BASE_PATH used without path traversal validation
📁 apps/backend/core/workspace/git_utils.py:225
The environment variable WORKTREE_BASE_PATH is read via os.getenv() and used directly in path construction without validation. A malicious or misconfigured value like '../../../etc' could allow worktree operations outside the project directory. While environment variables are typically trusted, defense-in-depth requires validating that the resolved path remains within expected boundaries.
Suggested fix:
Add path validation: 1. Resolve the full path using os.path.realpath(). 2. Verify the resolved path is within the project directory or an allowed external location. 3. Reject paths containing '..' after resolution. Example: `resolved = Path(worktree_base_path).resolve(); if not str(resolved).startswith(str(project_dir.resolve())): raise ValueError('Worktree path must be within project')`
🟡 [MEDIUM] WORKTREE_BASE_PATH used without path traversal validation
📁 apps/backend/cli/batch_commands.py:188
Same pattern as git_utils.py - the environment variable is read and used directly for path construction without validation. This file handles batch cleanup operations, so an invalid path could affect unintended directories.
Suggested fix:
Create a centralized utility function for getting the validated worktree path: def get_worktree_base(project_dir: Path) -> Path that validates the path stays within safe boundaries. Use this function in all 6 files that read WORKTREE_BASE_PATH.
🟠 [HIGH] WORKTREE_BASE_PATH retrieval duplicated across 6+ files
📁 apps/backend/core/worktree.py:61
The pattern os.getenv('WORKTREE_BASE_PATH', '.worktrees') is duplicated in: 1) worktree.py:61, 2) git_utils.py:225, 3) models.py:253, 4) utils.py:85, 5) batch_commands.py:188, 6) project-store.ts:267. This violates DRY principle and makes it difficult to change the default value or add validation consistently. The quality-reviewer identified this as a high severity issue.
Suggested fix:
Create centralized utilities: 1. Backend (Python): Add get_worktree_base_path(project_dir) to core/config.py. 2. Frontend (TypeScript): Add getWorktreeBasePath(projectPath) to shared/utils/paths.ts. Update all 6+ locations to use these centralized functions.
🟡 [MEDIUM] getRelativePath function has edge cases with trailing slashes
📁 apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx:21
The custom getRelativePath function (lines 21-48) may produce incorrect results when: 1) Paths have trailing slashes - the split/filter approach normalizes this but the output may not match user expectations. 2) Paths on different drive roots on Windows - returns absolute path but this behavior isn't documented. 3) When 'from' and 'to' are identical - returns '.' which may not work correctly in all path.join() contexts.
Suggested fix:
Add input normalization: 1. Trim trailing slashes from inputs. 2. Handle the identical path edge case explicitly. 3. Add unit tests for edge cases. Consider using a well-tested path library if available in the browser context.
🔵 [LOW] Custom path utilities could be extracted to shared module
📁 apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx:8
The component implements three browser-compatible path utilities (isAbsolutePath, joinPath, getRelativePath) inline. While necessary for renderer context (no Node.js), these could be reused elsewhere. The joinPath function is particularly simple and may not handle all edge cases (double slashes with empty parts).
Suggested fix:
Extract to apps/frontend/src/shared/utils/browser-paths.ts for reuse and easier testing. Add JSDoc comments explaining browser compatibility constraints.
This review was generated by Auto Claude.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/project-store.ts (1)
648-648: Use the shared constant for consistency.The
findAllSpecPathsmethod still uses the hardcoded'.worktrees'string instead of the centralized configuration. This creates inconsistency and could lead to bugs if users configure a different worktree location.🔎 Proposed fix
// 2. Check worktrees - const worktreesDir = path.join(projectPath, '.worktrees'); + const worktreeBasePath = process.env[WORKTREE_BASE_PATH_VAR] || DEFAULT_WORKTREE_PATH; + const worktreesDir = path.join(projectPath, worktreeBasePath); if (existsSync(worktreesDir)) {
♻️ Duplicate comments (2)
apps/backend/core/workspace/git_utils.py (1)
227-229: Path validation now handled by centralized config.The previous review concern about path traversal has been addressed by the centralized
get_worktree_base_path()function incore/config.py, which validates paths and prevents:
- Paths inside
.auto-claude/or.git/- Malicious relative paths escaping to system directories
The function returns
DEFAULT_WORKTREE_PATHif validation fails.apps/backend/cli/batch_commands.py (1)
189-191: Duplication and path validation concerns resolved.The previous review concerns about code duplication (using magic strings) and path traversal have been properly addressed by using the centralized
get_worktree_base_path()function fromcore/config.py.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (15)
apps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/core/config.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/workspace/models.pyapps/backend/core/worktree.pyapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.tsapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/shared/constants/index.tsapps/frontend/src/shared/constants/worktree.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.jsonapps/frontend/src/shared/types/project.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.tsapps/frontend/src/shared/constants/worktree.tsapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.tsapps/frontend/src/shared/constants/worktree.tsapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/shared/constants/index.tsapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/main/project-store.tsapps/frontend/src/shared/constants/worktree.tsapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/core/config.pyapps/backend/core/workspace/models.pyapps/backend/cli/batch_commands.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.pyapps/backend/cli/utils.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/core/config.pyapps/backend/core/workspace/models.pyapps/backend/cli/batch_commands.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.pyapps/backend/cli/utils.py
apps/backend/core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement dynamic command allowlisting based on detected project stack using
core/security.pyfor bash command validation
Files:
apps/backend/core/config.pyapps/backend/core/workspace/models.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/worktree.py
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
🧠 Learnings (5)
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsxapps/frontend/src/shared/i18n/locales/fr/settings.json
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsxapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/i18n/locales/fr/settings.json
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Use git worktrees for isolated agent builds with branches following the naming pattern `auto-claude/{spec-name}`
Applied to files:
apps/frontend/src/main/ipc-handlers/env-handlers.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/main/project-store.tsapps/backend/cli/batch_commands.pyapps/frontend/src/shared/i18n/locales/fr/settings.json
🧬 Code graph analysis (8)
apps/backend/core/config.py (1)
apps/frontend/src/shared/constants/worktree.ts (2)
WORKTREE_BASE_PATH_VAR(9-9)DEFAULT_WORKTREE_PATH(12-12)
apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (2)
apps/frontend/src/shared/types/project.ts (1)
ProjectEnvConfig(274-342)apps/frontend/src/shared/constants/worktree.ts (1)
DEFAULT_WORKTREE_PATH(12-12)
apps/backend/core/workspace/models.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(17-61)
apps/frontend/src/main/project-store.ts (1)
apps/frontend/src/shared/constants/worktree.ts (2)
WORKTREE_BASE_PATH_VAR(9-9)DEFAULT_WORKTREE_PATH(12-12)
apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx (3)
apps/frontend/src/shared/types/project.ts (1)
ProjectEnvConfig(274-342)apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (1)
WorktreeSettings(60-160)apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (2)
envConfig(23-23)updateEnvConfig(29-29)
apps/backend/cli/batch_commands.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(17-61)
apps/backend/core/workspace/git_utils.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(17-61)
apps/backend/core/worktree.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(17-61)
🪛 Biome (2.1.2)
apps/frontend/src/main/project-store.ts
[error] 267-267: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
🔇 Additional comments (13)
apps/frontend/src/shared/i18n/locales/fr/settings.json (1)
217-231: LGTM! French translations complete and accurate.The French translations for the worktree settings UI are well-structured and accurately mirror the English locale. The technical terms are appropriately preserved (e.g., "worktrees"), and the HTML tag markers (
<1>,<2>) for i18n interpolation are correctly maintained.Based on learnings, this follows the coding guideline to add translation keys to all language files (minimum: en/.json and fr/.json).
apps/frontend/src/shared/constants/index.ts (1)
35-37: LGTM! Consistent barrel export pattern.The worktree constants export follows the established pattern used throughout this file and properly extends the central constants barrel.
apps/frontend/src/shared/types/project.ts (1)
307-307: LGTM! Well-documented optional field.The
worktreePathfield is appropriately optional with a clear comment explaining its purpose. The type signature correctly allows both relative and absolute path strings, aligning with the backend validation logic inget_worktree_base_path.apps/backend/core/workspace/models.py (1)
255-256: LGTM! Correct usage of centralized worktree base path.The integration with
get_worktree_base_path()is correct and properly passesself.project_dirfor validation. This ensures spec numbering scans all worktrees regardless of the configured base path.apps/frontend/src/shared/constants/worktree.ts (1)
1-12: LGTM! Well-structured constants aligned with backend.The constants are properly defined and documented. The naming (
WORKTREE_BASE_PATH_VAR,DEFAULT_WORKTREE_PATH) aligns with the backend configuration inapps/backend/core/config.py, ensuring consistency across the stack.apps/backend/cli/batch_commands.py (1)
216-216: LGTM! Correct usage of dynamic base path in output.The cleanup command now correctly displays the configured worktree base path instead of the hardcoded ".worktrees", providing accurate feedback to users.
apps/frontend/src/main/ipc-handlers/env-handlers.ts (3)
101-103: LGTM! Correct handling of worktreePath in env generation.The code properly reads
config.worktreePathand maps it to theWORKTREE_BASE_PATHenvironment variable, maintaining consistency with the backend configuration.
234-237: Consider i18n for .env file comments.The comments in the generated
.envfile are hardcoded in English. While.envfiles typically use English comments by convention, the coding guidelines specify that "all user-facing text in the frontend" should use i18n translation keys.If these comments are considered user-facing (since they appear in the generated file that users may read), consider whether they should be internationalized. However, this may be an acceptable exception given that:
.envfiles are technical configuration files- Environment variable names are inherently English
- Many other comments in this file (lines 194-308) are also hardcoded
Should
.envfile comments be treated as user-facing text requiring i18n, or are they an exception to the i18n guideline due to the technical nature of configuration files? Please clarify the scope of the i18n requirement.
422-424: LGTM! Correct parsing of WORKTREE_BASE_PATH.The env handler properly reads the
WORKTREE_BASE_PATHenvironment variable and maps it toconfig.worktreePath, maintaining bidirectional sync between the UI configuration and the.envfile.apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx (1)
60-160: LGTM! i18n properly implemented.The component correctly uses the
useTranslationhook with thesettingsnamespace and all user-facing text is properly wrapped with translation keys using eithert()or<Trans>components. This fully addresses the critical issue raised in the previous review.The custom path utilities (lines 9-52) are simple but handle the required use cases correctly, including cross-platform absolute path detection and relative path computation.
apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx (1)
159-167: LGTM! Hardcoded string replaced with i18n key.The section heading now correctly uses
t('projectSections.worktree.title')instead of a hardcoded string, fully addressing the critical issue from the previous review. The WorktreeSettings component is properly integrated with correct prop passing.apps/frontend/src/shared/i18n/locales/en/settings.json (1)
218-231: French translations for the worktree section are complete with all required keys present.apps/backend/core/config.py (1)
17-61: No changes required — design pattern is correct.All five callers use the function identically: they receive the returned string and immediately compose it with
project_dirusing the/operator (e.g.,project_dir / worktree_base_path). The function correctly validates the final resolved path while returning the original string, which allows flexible composition. There is no inconsistency or TOCTOU-like issue, as the validation checks structural safety (preventing paths from entering.auto-claude/or.git/) rather than file state. The docstring is clear that the return value is a string, and all callers work with this expectation as intended.
| import { WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH } from '../shared/constants'; | ||
| const worktreeBasePath = process.env[WORKTREE_BASE_PATH_VAR] || DEFAULT_WORKTREE_PATH; | ||
| const worktreesDir = path.join(project.path, worktreeBasePath); |
There was a problem hiding this comment.
Critical: Move import to top-level.
The inline import violates JavaScript/TypeScript module system conventions and is flagged by static analysis. Import statements must be at the top level of the file. This can cause runtime errors with certain bundlers and breaks tree-shaking optimization.
🔎 Proposed fix
Move the import to the top of the file with other imports (around line 6):
import { app } from 'electron';
import { readFileSync, writeFileSync, existsSync, mkdirSync, readdirSync, Dirent } from 'fs';
import path from 'path';
import { v4 as uuidv4 } from 'uuid';
import type { Project, ProjectSettings, Task, TaskStatus, TaskMetadata, ImplementationPlan, ReviewReason, PlanSubtask } from '../shared/types';
-import { DEFAULT_PROJECT_SETTINGS, AUTO_BUILD_PATHS, getSpecsDir } from '../shared/constants';
+import { DEFAULT_PROJECT_SETTINGS, AUTO_BUILD_PATHS, getSpecsDir, WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH } from '../shared/constants';
import { getAutoBuildPath, isInitialized } from './project-initializer';Then remove the inline import at line 267:
// 2. Scan worktree specs directories
// NOTE FOR MAINTAINERS: Worktree tasks are only included if the spec also exists in main.
// This prevents deleted tasks from "coming back" when the worktree isn't cleaned up.
// Alternative behavior: include all worktree tasks (remove the mainSpecIds check below).
- import { WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH } from '../shared/constants';
const worktreeBasePath = process.env[WORKTREE_BASE_PATH_VAR] || DEFAULT_WORKTREE_PATH;
const worktreesDir = path.join(project.path, worktreeBasePath);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 267-267: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
🤖 Prompt for AI Agents
apps/frontend/src/main/project-store.ts around lines 267 to 269: there is an
inline import for constants which must be moved to the top-level; remove the
inline import from line 267 and add the import statement with
WORKTREE_BASE_PATH_VAR and DEFAULT_WORKTREE_PATH to the file's existing import
block near line 6 so all imports are top-level, then keep the existing logic
that computes worktreeBasePath and worktreesDir unchanged.
- Add comprehensive module-level docstring with examples - Add inline docstrings for WORKTREE_BASE_PATH_VAR constant - Add inline docstrings for DEFAULT_WORKTREE_PATH constant - Enhance get_worktree_base_path() docstring with: - Detailed description of functionality - Supported path types section - Security validations section - Comprehensive Args and Returns sections - Multiple usage examples - Design decision note Addresses docstring coverage check (72.73% -> improved)
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @sbeardsley. * #456 (comment) The following files were modified: * `apps/backend/cli/batch_commands.py` * `apps/backend/cli/main.py` * `apps/backend/core/config.py` * `apps/backend/core/workspace/models.py` * `apps/backend/core/worktree.py` * `apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx` * `apps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx` * `apps/frontend/src/renderer/components/settings/sections/SectionRouter.tsx`
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/backend/core/config.py (1)
97-99: The past review comment regarding substring validation remains unaddressed.The substring check for ".auto-claude" and ".git" can produce false positives (e.g., paths containing "worktrees/.git-backup" or "my-auto-claude-config" would be incorrectly rejected). The previous reviewer's suggestion to check path components rather than substrings is still valid.
🔎 Improved validation checking path components
# If no project_dir provided, return as-is (basic validation only) if not project_dir: # Check for obviously dangerous patterns - normalized = Path(worktree_base_path).as_posix() - if ".auto-claude" in normalized or ".git" in normalized: + parts = set(Path(worktree_base_path).parts) + if ".auto-claude" in parts or ".git" in parts: return DEFAULT_WORKTREE_PATH return worktree_base_pathThis validates path segments rather than substrings, preventing false positives while maintaining security.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/backend/core/config.py
🧰 Additional context used
📓 Path-based instructions (2)
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/core/config.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/core/config.py
apps/backend/core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement dynamic command allowlisting based on detected project stack using
core/security.pyfor bash command validation
Files:
apps/backend/core/config.py
🧬 Code graph analysis (1)
apps/backend/core/config.py (1)
apps/frontend/src/shared/constants/worktree.ts (2)
WORKTREE_BASE_PATH_VAR(9-9)DEFAULT_WORKTREE_PATH(12-12)
🔇 Additional comments (2)
apps/backend/core/config.py (2)
1-39: LGTM! Well-structured module with clear documentation.The module docstring, constants, and their documentation are comprehensive and follow Python best practices. The constants correctly match the frontend definitions, ensuring consistency across the codebase.
42-91: Excellent documentation quality.The docstring is comprehensive and well-structured, clearly explaining supported path types, security validations, parameters, return values, and usage examples. The explicit note about allowing absolute paths outside the project directory (lines 87-90) is particularly valuable for understanding the design intent.
| worktree_base_path = os.getenv(WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH) | ||
|
|
||
| # If no project_dir provided, return as-is (basic validation only) | ||
| if not project_dir: | ||
| # Check for obviously dangerous patterns | ||
| normalized = Path(worktree_base_path).as_posix() | ||
| if ".auto-claude" in normalized or ".git" in normalized: | ||
| return DEFAULT_WORKTREE_PATH | ||
| return worktree_base_path | ||
|
|
||
| # Resolve the absolute path | ||
| if Path(worktree_base_path).is_absolute(): | ||
| resolved = Path(worktree_base_path).resolve() | ||
| else: | ||
| resolved = (project_dir / worktree_base_path).resolve() | ||
|
|
||
| # Prevent paths inside .auto-claude/ or .git/ | ||
| auto_claude_dir = (project_dir / ".auto-claude").resolve() | ||
| git_dir = (project_dir / ".git").resolve() | ||
|
|
||
| resolved_str = str(resolved) | ||
| if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( | ||
| str(git_dir) | ||
| ): | ||
| return DEFAULT_WORKTREE_PATH | ||
|
|
||
| return worktree_base_path |
There was a problem hiding this comment.
Add error handling for Path operations to prevent crashes.
Multiple Path operations (lines 97, 103, 104, 106, 109-110) can raise exceptions (ValueError, OSError, RuntimeError) when processing malformed or invalid paths from the environment. Without error handling, a malformed WORKTREE_BASE_PATH in a project's .env will crash the backend during project loading.
The docstring promises that the function "falls back to DEFAULT_WORKTREE_PATH if validation fails" (line 57), but this only occurs for security checks, not for malformed paths.
🔎 Wrap Path operations in try-except for graceful fallback
def get_worktree_base_path(project_dir: Path | None = None) -> str:
"""..."""
worktree_base_path = os.getenv(WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH)
# If no project_dir provided, return as-is (basic validation only)
if not project_dir:
- # Check for obviously dangerous patterns
- normalized = Path(worktree_base_path).as_posix()
- if ".auto-claude" in normalized or ".git" in normalized:
- return DEFAULT_WORKTREE_PATH
- return worktree_base_path
+ try:
+ # Check for obviously dangerous patterns
+ normalized = Path(worktree_base_path).as_posix()
+ if ".auto-claude" in normalized or ".git" in normalized:
+ return DEFAULT_WORKTREE_PATH
+ return worktree_base_path
+ except (ValueError, OSError):
+ return DEFAULT_WORKTREE_PATH
- # Resolve the absolute path
- if Path(worktree_base_path).is_absolute():
- resolved = Path(worktree_base_path).resolve()
- else:
- resolved = (project_dir / worktree_base_path).resolve()
-
- # Prevent paths inside .auto-claude/ or .git/
- auto_claude_dir = (project_dir / ".auto-claude").resolve()
- git_dir = (project_dir / ".git").resolve()
-
- resolved_str = str(resolved)
- if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith(
- str(git_dir)
- ):
- return DEFAULT_WORKTREE_PATH
-
- return worktree_base_path
+ try:
+ # Resolve the absolute path
+ if Path(worktree_base_path).is_absolute():
+ resolved = Path(worktree_base_path).resolve()
+ else:
+ resolved = (project_dir / worktree_base_path).resolve()
+
+ # Prevent paths inside .auto-claude/ or .git/
+ auto_claude_dir = (project_dir / ".auto-claude").resolve()
+ git_dir = (project_dir / ".git").resolve()
+
+ resolved_str = str(resolved)
+ if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith(
+ str(git_dir)
+ ):
+ return DEFAULT_WORKTREE_PATH
+
+ return worktree_base_path
+ except (ValueError, OSError, RuntimeError):
+ # Return default for any path resolution/validation errors
+ return DEFAULT_WORKTREE_PATH📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| worktree_base_path = os.getenv(WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH) | |
| # If no project_dir provided, return as-is (basic validation only) | |
| if not project_dir: | |
| # Check for obviously dangerous patterns | |
| normalized = Path(worktree_base_path).as_posix() | |
| if ".auto-claude" in normalized or ".git" in normalized: | |
| return DEFAULT_WORKTREE_PATH | |
| return worktree_base_path | |
| # Resolve the absolute path | |
| if Path(worktree_base_path).is_absolute(): | |
| resolved = Path(worktree_base_path).resolve() | |
| else: | |
| resolved = (project_dir / worktree_base_path).resolve() | |
| # Prevent paths inside .auto-claude/ or .git/ | |
| auto_claude_dir = (project_dir / ".auto-claude").resolve() | |
| git_dir = (project_dir / ".git").resolve() | |
| resolved_str = str(resolved) | |
| if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( | |
| str(git_dir) | |
| ): | |
| return DEFAULT_WORKTREE_PATH | |
| return worktree_base_path | |
| worktree_base_path = os.getenv(WORKTREE_BASE_PATH_VAR, DEFAULT_WORKTREE_PATH) | |
| # If no project_dir provided, return as-is (basic validation only) | |
| if not project_dir: | |
| try: | |
| # Check for obviously dangerous patterns | |
| normalized = Path(worktree_base_path).as_posix() | |
| if ".auto-claude" in normalized or ".git" in normalized: | |
| return DEFAULT_WORKTREE_PATH | |
| return worktree_base_path | |
| except (ValueError, OSError): | |
| return DEFAULT_WORKTREE_PATH | |
| try: | |
| # Resolve the absolute path | |
| if Path(worktree_base_path).is_absolute(): | |
| resolved = Path(worktree_base_path).resolve() | |
| else: | |
| resolved = (project_dir / worktree_base_path).resolve() | |
| # Prevent paths inside .auto-claude/ or .git/ | |
| auto_claude_dir = (project_dir / ".auto-claude").resolve() | |
| git_dir = (project_dir / ".git").resolve() | |
| resolved_str = str(resolved) | |
| if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( | |
| str(git_dir) | |
| ): | |
| return DEFAULT_WORKTREE_PATH | |
| return worktree_base_path | |
| except (ValueError, OSError, RuntimeError): | |
| # Return default for any path resolution/validation errors | |
| return DEFAULT_WORKTREE_PATH |
🤖 Prompt for AI Agents
In apps/backend/core/config.py around lines 92 to 118, several Path operations
(resolve(), is_absolute(), as_posix(), and joining with project_dir) can raise
ValueError/OSError/RuntimeError for malformed environment values; wrap all
Path-related operations in a try/except that catches these exceptions and any
other OSError/ValueError, and on exception return DEFAULT_WORKTREE_PATH (and
optionally log the exception) so the function truly falls back instead of
crashing. Ensure the try block covers the normalization, absolute check, resolve
calls, and the .auto-claude/.git resolution checks, and keep the same security
checks but with safe error handling that returns DEFAULT_WORKTREE_PATH on
failure.
- Fix path validation in config.py to check path segments instead of substrings - Prevents false positives like 'worktrees/.git-backup' - Uses Path.parts to check for .auto-claude/.git as components - Move all get_worktree_base_path imports to module level for consistency - batch_commands.py: Move import from function to top - utils.py: Move import from function to top - git_utils.py: Move import from function to top - models.py: Move import from function to top - worktree.py: Move import from __init__ to top Addresses PR review feedback from CodeRabbit and Gemini Code Assist
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/backend/core/config.py (2)
92-100: Error handling for Path operations is still missing (per previous review).The path validation improvement (checking
partsinstead of substring) successfully addresses the first review comment. However, the second review comment's concern remains unaddressed: Line 97'sPath(worktree_base_path).partscan raiseValueErrororOSErrorfor malformed paths (e.g., paths with null bytes, invalid characters, or OS-specific issues).Without a try-except wrapper, a malformed
WORKTREE_BASE_PATHin a project's.envwill crash the backend during project loading, contradicting the docstring's promise to "fall back to DEFAULT_WORKTREE_PATH if validation fails" (line 57).🔎 Wrap Path operations in try-except
# If no project_dir provided, return as-is (basic validation only) if not project_dir: + try: - # Check for .auto-claude or .git as path components, not substrings - parts = set(Path(worktree_base_path).parts) - if ".auto-claude" in parts or ".git" in parts: + # Check for .auto-claude or .git as path components, not substrings + parts = set(Path(worktree_base_path).parts) + if ".auto-claude" in parts or ".git" in parts: + return DEFAULT_WORKTREE_PATH + return worktree_base_path + except (ValueError, OSError): return DEFAULT_WORKTREE_PATH - return worktree_base_path
102-116: Error handling for Path operations is still missing (per previous review).Lines 103-106 (resolve operations) and lines 109-110 (security directory resolution) can raise
ValueError,OSError, orRuntimeErrorfor malformed or problematic paths. The second review comment's concern remains unaddressed.🔎 Wrap all Path operations in try-except
+ try: - # Resolve the absolute path - if Path(worktree_base_path).is_absolute(): - resolved = Path(worktree_base_path).resolve() - else: - resolved = (project_dir / worktree_base_path).resolve() - - # Prevent paths inside .auto-claude/ or .git/ - auto_claude_dir = (project_dir / ".auto-claude").resolve() - git_dir = (project_dir / ".git").resolve() - - resolved_str = str(resolved) - if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( - str(git_dir) - ): - return DEFAULT_WORKTREE_PATH - - return worktree_base_path + # Resolve the absolute path + if Path(worktree_base_path).is_absolute(): + resolved = Path(worktree_base_path).resolve() + else: + resolved = (project_dir / worktree_base_path).resolve() + + # Prevent paths inside .auto-claude/ or .git/ + auto_claude_dir = (project_dir / ".auto-claude").resolve() + git_dir = (project_dir / ".git").resolve() + + resolved_str = str(resolved) + if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( + str(git_dir) + ): + return DEFAULT_WORKTREE_PATH + + return worktree_base_path + except (ValueError, OSError, RuntimeError): + # Return default for any path resolution/validation errors + return DEFAULT_WORKTREE_PATH
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/core/config.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/workspace/models.pyapps/backend/core/worktree.py
🧰 Additional context used
📓 Path-based instructions (2)
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/core/worktree.pyapps/backend/core/config.pyapps/backend/core/workspace/git_utils.pyapps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/core/workspace/models.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/core/worktree.pyapps/backend/core/config.pyapps/backend/core/workspace/git_utils.pyapps/backend/cli/batch_commands.pyapps/backend/cli/utils.pyapps/backend/core/workspace/models.py
apps/backend/core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement dynamic command allowlisting based on detected project stack using
core/security.pyfor bash command validation
Files:
apps/backend/core/worktree.pyapps/backend/core/config.pyapps/backend/core/workspace/git_utils.pyapps/backend/core/workspace/models.py
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Use git worktrees for isolated agent builds with branches following the naming pattern `auto-claude/{spec-name}`
Applied to files:
apps/backend/cli/batch_commands.pyapps/backend/cli/utils.py
🧬 Code graph analysis (6)
apps/backend/core/worktree.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(42-118)
apps/backend/core/config.py (1)
apps/frontend/src/shared/constants/worktree.ts (2)
WORKTREE_BASE_PATH_VAR(9-9)DEFAULT_WORKTREE_PATH(12-12)
apps/backend/core/workspace/git_utils.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(42-118)
apps/backend/cli/batch_commands.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(42-118)
apps/backend/cli/utils.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(42-118)
apps/backend/core/workspace/models.py (1)
apps/backend/core/config.py (1)
get_worktree_base_path(42-118)
🔇 Additional comments (12)
apps/backend/core/worktree.py (2)
25-26: LGTM: Import moved to module level.The import has been correctly moved to module level as suggested in the previous review, eliminating unnecessary overhead on every
WorktreeManagerinstantiation.
60-64: LGTM: Correct usage of configurable worktree base path.The implementation correctly:
- Calls
get_worktree_base_path(project_dir)for full security validation- Constructs the worktree directory path using Path division
- Includes a helpful comment documenting the configurability
apps/backend/cli/utils.py (2)
18-18: LGTM: Import correctly placed at module level.The import follows project conventions and is properly ordered with other
coremodule imports.
85-87: LGTM: Correct usage for worktree spec lookup.The implementation correctly uses the configurable base path for locating specs in worktrees, with security validation delegated to
get_worktree_base_path.apps/backend/cli/batch_commands.py (2)
9-12: LGTM: Imports correctly placed at module level.Both the
osimport and theget_worktree_base_pathimport are properly placed at module level, following Python best practices.
189-190: LGTM: Correct usage with appropriate type conversion.The implementation correctly:
- Converts the string
project_dirparameter toPathfor validation- Uses the returned base path to construct the worktrees directory
- Updates the dry-run output (line 215) to reflect the configured path
apps/backend/core/workspace/git_utils.py (2)
10-15: LGTM: Imports correctly organized at module level.The imports are properly placed at module level with appropriate spacing, following Python best practices.
227-229: LGTM: Correct per-spec worktree path construction.The implementation correctly:
- Uses
get_worktree_base_path(project_dir)for security validation- Constructs the full worktree path by combining base path and spec name
- Includes an updated comment documenting the configurability
apps/backend/core/workspace/models.py (2)
9-15: LGTM: Imports correctly organized at module level.The imports are properly placed at module level with appropriate spacing, addressing the previous review feedback.
255-256: LGTM: Correct usage for spec numbering coordination.The implementation correctly uses the configurable base path to scan all worktrees for spec numbers, ensuring proper coordination across both main project and worktree specs.
apps/backend/core/config.py (2)
1-40: LGTM! Well-structured configuration module.The module docstring, imports, and constants are well-documented and provide clear guidance. The constants align with the frontend (see
apps/frontend/src/shared/constants/worktree.ts).
42-91: Excellent documentation with clear security considerations.The docstring thoroughly explains the function's behavior, supported path types, security validations, and design decisions. The examples are particularly helpful.
| resolved_str = str(resolved) | ||
| if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( | ||
| str(git_dir) | ||
| ): | ||
| return DEFAULT_WORKTREE_PATH |
There was a problem hiding this comment.
Fix security check to avoid false positives for similar directory names.
The startswith check on string paths creates false positives. For example:
- A path resolving to
/project/.auto-claude-backupwould incorrectly match/project/.auto-claude - A path resolving to
/project/.git-hookswould incorrectly match/project/.git
Use Path.is_relative_to() (available since Python 3.9, compatible with the Python 3.12+ requirement) to check true parent-child relationships.
🔎 Use Path.is_relative_to() for accurate directory containment checks
# Prevent paths inside .auto-claude/ or .git/
auto_claude_dir = (project_dir / ".auto-claude").resolve()
git_dir = (project_dir / ".git").resolve()
- resolved_str = str(resolved)
- if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith(
- str(git_dir)
- ):
+ if resolved.is_relative_to(auto_claude_dir) or resolved.is_relative_to(git_dir):
return DEFAULT_WORKTREE_PATH
return worktree_base_path📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resolved_str = str(resolved) | |
| if resolved_str.startswith(str(auto_claude_dir)) or resolved_str.startswith( | |
| str(git_dir) | |
| ): | |
| return DEFAULT_WORKTREE_PATH | |
| resolved_str = str(resolved) | |
| if resolved.is_relative_to(auto_claude_dir) or resolved.is_relative_to(git_dir): | |
| return DEFAULT_WORKTREE_PATH | |
| return worktree_base_path |
🤖 Prompt for AI Agents
In apps/backend/core/config.py around lines 112 to 116, the security check uses
string startswith() which produces false positives for similarly named
directories; replace the string comparisons with proper Path containment checks:
convert resolved, auto_claude_dir, and git_dir to pathlib.Path and use
Path.is_relative_to() (or check equality or is_relative_to) to determine true
parent/child relationships, and return DEFAULT_WORKTREE_PATH only when resolved
is equal to or is_relative_to those directories.
4 participants
Allows users to configure worktree location per project through the project settings UI. The custom path is stored in each project's .env file as WORKTREE_BASE_PATH.
Changes
Frontend:
worktreePathfield toProjectEnvConfigtypeWorktreeSettings.tsxcomponent with path input and browse buttonWORKTREE_BASE_PATHin .env filesBackend:
WorktreeManagerto readWORKTREE_BASE_PATHenvironment variable.worktreesreferences with env varFiles Modified:
apps/frontend/src/shared/types/project.ts- Add worktreePath typeapps/frontend/src/renderer/components/project-settings/WorktreeSettings.tsx- NEW componentapps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx- Integrate componentapps/frontend/src/main/ipc-handlers/env-handlers.ts- Handle WORKTREE_BASE_PATHapps/backend/core/worktree.py- Use environment variableapps/backend/cli/main.py- Load project .env fileapps/backend/cli/utils.py- Use env var for spec findingapps/backend/cli/batch_commands.py- Use env var for cleanupapps/backend/core/workspace/git_utils.py- Use env var for worktree lookupsapps/backend/core/workspace/models.py- Use env var for scanningapps/frontend/src/main/project-store.ts- Use env var for task scanningFeatures
.worktreeswhen not configuredTesting
worktrees,../project-name-worktrees)/tmp/worktrees)Screenshots
The new "Worktree Location" section appears in Project Settings → General, after the Agent Configuration section.
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.