Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/backend/cli/batch_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def handle_batch_cleanup_command(project_dir: str, dry_run: bool = True) -> bool
True if successful
"""
specs_dir = Path(project_dir) / ".auto-claude" / "specs"
worktrees_dir = Path(project_dir) / ".worktrees"
worktrees_dir = Path(project_dir) / ".auto-claude" / "worktrees" / "tasks"

if not specs_dir.exists():
print_status("No specs directory found", "info")
Expand All @@ -209,7 +209,7 @@ def handle_batch_cleanup_command(project_dir: str, dry_run: bool = True) -> bool
print(f" - {spec_name}")
wt_path = worktrees_dir / spec_name
if wt_path.exists():
print(f" └─ .worktrees/{spec_name}/")
print(f" └─ .auto-claude/worktrees/tasks/{spec_name}/")
print()
print("Run with --no-dry-run to actually delete")

Expand Down
2 changes: 1 addition & 1 deletion apps/backend/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def find_spec(project_dir: Path, spec_identifier: str) -> Path | None:
return spec_folder

# Check worktree specs (for merge-preview, merge, review, discard operations)
worktree_base = project_dir / ".worktrees"
worktree_base = project_dir / ".auto-claude" / "worktrees" / "tasks"
if worktree_base.exists():
# Try exact match in worktree
worktree_spec = (
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/core/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
=============================================

Handles workspace isolation through Git worktrees, where each spec
gets its own isolated worktree in .worktrees/{spec-name}/.
gets its own isolated worktree in .auto-claude/worktrees/tasks/{spec-name}/.

This module has been refactored for better maintainability:
- Models and enums: workspace/models.py
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/core/workspace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
=============================

Handles workspace isolation through Git worktrees, where each spec
gets its own isolated worktree in .worktrees/{spec-name}/.
gets its own isolated worktree in .auto-claude/worktrees/tasks/{spec-name}/.

This package provides:
- Workspace setup and configuration
Expand Down
20 changes: 18 additions & 2 deletions apps/backend/core/workspace/finalization.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,15 @@ def handle_workspace_choice(
if staging_path:
print(highlight(f" cd {staging_path}"))
else:
print(highlight(f" cd {project_dir}/.worktrees/{spec_name}"))
worktree_path = get_existing_build_worktree(project_dir, spec_name)
if worktree_path:
print(highlight(f" cd {worktree_path}"))
else:
print(
highlight(
f" cd {project_dir}/.auto-claude/worktrees/tasks/{spec_name}"
)
)
Comment on lines +172 to +180
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block of code to determine and print the worktree path is duplicated later in this file (lines 243-251). To improve maintainability and avoid future inconsistencies, consider extracting this logic into a helper function.

Comment on lines +172 to +180
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the finalization.py file around the specified lines
cat -n apps/backend/core/workspace/finalization.py | head -250 | tail -100

Repository: AndyMik90/Auto-Claude

Length of output: 4344


🏁 Script executed:

# Find the get_existing_build_worktree function definition
rg -n "def get_existing_build_worktree" --type=py -A10

Repository: AndyMik90/Auto-Claude

Length of output: 936


🏁 Script executed:

# Check callers of the function containing this code (appears to be in a larger function)
rg -n "handle_workspace_choice\|print.*worktree" --type=py -B3 -A3

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Look for worktree creation logic
rg -n "\.auto-claude/worktrees" --type=py -B2 -A2

Repository: AndyMik90/Auto-Claude

Length of output: 2740


🏁 Script executed:

# Get full implementation of get_existing_build_worktree
rg -n "def get_existing_build_worktree" --type=py -A20

Repository: AndyMik90/Auto-Claude

Length of output: 1711


🏁 Script executed:

# Check what get_worktree_info returns and when it might be None
rg -n "def get_worktree_info" --type=py -A15

Repository: AndyMik90/Auto-Claude

Length of output: 1175


🏁 Script executed:

# Look for where worktrees are created
rg -n "def create_worktree\|def setup.*worktree" --type=py -A10

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Check the broader context around handle_workspace_choice - what calls it and when
rg -n "handle_workspace_choice" --type=py -B5 -A2

Repository: AndyMik90/Auto-Claude

Length of output: 3753


🏁 Script executed:

# Check context of when handle_workspace_choice is called - is worktree guaranteed to exist at this point?
rg -n "handle_workspace_choice" --type=py -B10 | head -50

Repository: AndyMik90/Auto-Claude

Length of output: 2995


🏁 Script executed:

# Check finalize_workspace to see if it creates the worktree
rg -n "def finalize_workspace" --type=py -A30

Repository: AndyMik90/Auto-Claude

Length of output: 2591


🏁 Script executed:

# Check if there are any tests or documentation about the edge case
rg -n "get_worktree_info\|get_existing_build_worktree" --type=py -B2 -A8 | grep -A8 "returns None\|returncode"

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Look at what could cause get_worktree_info to return None despite path existing
rg -n "rev-parse.*HEAD\|returncode != 0" --type=py -B5 -A3

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Check if the fallback path is ever actually relied upon or if it's purely defensive
rg -n "\.auto-claude/worktrees/tasks" --type=py | head -20

Repository: AndyMik90/Auto-Claude

Length of output: 887


🏁 Script executed:

# Let's check if there's any error handling after this code is printed
rg -n "cd {worktree" --type=py -A10 | head -40

Repository: AndyMik90/Auto-Claude

Length of output: 163


🏁 Script executed:

# Check if there are any comments or documentation about this fallback behavior
rg -n "fallback\|edge case" --type=py apps/backend/core/workspace/

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Let's check what happens after handle_workspace_choice is called
rg -n "handle_workspace_choice" --type=py -A5

Repository: AndyMik90/Auto-Claude

Length of output: 2719


🏁 Script executed:

# Check the actual scenario: when would staging_path be None but we still want to show the path?
cat -n apps/backend/core/workspace/finalization.py | sed -n '145,160p'

Repository: AndyMik90/Auto-Claude

Length of output: 678


🏁 Script executed:

# Let's look at the full context of the TEST and LATER branches to understand the intent
cat -n apps/backend/core/workspace/finalization.py | sed -n '156,200p'

Repository: AndyMik90/Auto-Claude

Length of output: 1932


🏁 Script executed:

# Let's understand the complete flow - check what happens in worktree.py get_worktree_info more carefully
cat -n apps/backend/core/worktree.py | sed -n '213,235p'

Repository: AndyMik90/Auto-Claude

Length of output: 1011


🏁 Script executed:

# Also check if there's any guarantee that the worktree must exist at this point
cat -n apps/backend/cli/build_commands.py | sed -n '280,300p'

Repository: AndyMik90/Auto-Claude

Length of output: 996


Add validation or warning for the fallback worktree path that may not exist.

When both worktree_info is None and get_existing_build_worktree() returns None, the code prints a default path at .auto-claude/worktrees/tasks/{spec_name} that may not actually exist on disk. Users who copy this path to test will encounter a "directory not found" error with no context about why.

Consider one of these approaches:

  • Add a warning message indicating the path should be created if not found
  • Check if the fallback path exists before printing it
  • Print messaging that indicates the path must be manually created

This affects both the TEST branch (lines 176-180) and LATER branch (lines 247-251).

🤖 Prompt for AI Agents
In apps/backend/core/workspace/finalization.py around lines 172-180 (and
similarly update the LATER branch at ~247-251), the code prints a fallback path
.auto-claude/worktrees/tasks/{spec_name} even when get_existing_build_worktree()
returned None which may not exist; change this to compute the fallback_path =
os.path.join(project_dir, ".auto-claude", "worktrees", "tasks", spec_name),
check os.path.exists(fallback_path) and if it exists print the highlighted cd
line, otherwise print a highlighted cd line plus a clear warning that the
directory does not exist and must be created (or offer to create it
automatically using os.makedirs with exist_ok=True if you prefer automatic
creation); apply the same existence check and warning behavior in the LATER
branch.


# Show likely test/run commands
if staging_path:
Expand Down Expand Up @@ -232,7 +240,15 @@ def handle_workspace_choice(
if staging_path:
print(highlight(f" cd {staging_path}"))
else:
print(highlight(f" cd {project_dir}/.worktrees/{spec_name}"))
worktree_path = get_existing_build_worktree(project_dir, spec_name)
if worktree_path:
print(highlight(f" cd {worktree_path}"))
else:
print(
highlight(
f" cd {project_dir}/.auto-claude/worktrees/tasks/{spec_name}"
)
)
print()
print("When you're ready to add it:")
print(highlight(f" python auto-claude/run.py --spec {spec_name} --merge"))
Expand Down
14 changes: 10 additions & 4 deletions apps/backend/core/workspace/git_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,16 @@ def get_existing_build_worktree(project_dir: Path, spec_name: str) -> Path | Non
Returns:
Path to the worktree if it exists for this spec, None otherwise
"""
# Per-spec worktree path: .worktrees/{spec-name}/
worktree_path = project_dir / ".worktrees" / spec_name
if worktree_path.exists():
return worktree_path
# New path first
new_path = project_dir / ".auto-claude" / "worktrees" / "tasks" / spec_name
if new_path.exists():
return new_path

# Legacy fallback
legacy_path = project_dir / ".worktrees" / spec_name
if legacy_path.exists():
return legacy_path

Comment on lines +225 to +234
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Good backward compatibility fix, but add migration logging.

The two-step lookup correctly restores backward compatibility with the legacy .worktrees/{spec_name} path. However, when the legacy path is found, no warning or log is emitted, so users remain unaware that their worktrees are in a deprecated location.

🔎 Proposed enhancement to log legacy path detection

Add logging when the legacy path is detected to inform users about the migration:

+import logging
+
+logger = logging.getLogger(__name__)
+
 def get_existing_build_worktree(project_dir: Path, spec_name: str) -> Path | None:
     """
     Check if there's an existing worktree for this specific spec.
 
     Args:
         project_dir: The main project directory
         spec_name: The spec folder name (e.g., "001-feature-name")
 
     Returns:
         Path to the worktree if it exists for this spec, None otherwise
     """
     # New path first
     new_path = project_dir / ".auto-claude" / "worktrees" / "tasks" / spec_name
     if new_path.exists():
         return new_path
 
     # Legacy fallback
     legacy_path = project_dir / ".worktrees" / spec_name
     if legacy_path.exists():
+        logger.warning(
+            f"Found worktree at deprecated location: {legacy_path}. "
+            f"Consider migrating to: {new_path}"
+        )
         return legacy_path
 
     return None
🤖 Prompt for AI Agents
In apps/backend/core/workspace/git_utils.py around lines 225 to 234, the
function falls back to the legacy path but does not emit any log or warning;
update the code to log a warning when legacy_path.exists() is used so users know
a deprecated location was detected and can migrate. Use the module's existing
logger (or the standard logging.getLogger(__name__)) to emit a clear warning
that includes both the legacy path and the preferred new_path; keep the log
message concise and localize it to the branch where legacy_path is returned.

return None


Expand Down
2 changes: 1 addition & 1 deletion apps/backend/core/workspace/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def get_next_spec_number(self) -> int:
max_number = max(max_number, self._scan_specs_dir(main_specs_dir))

# 2. Scan all worktree specs
worktrees_dir = self.project_dir / ".worktrees"
worktrees_dir = self.project_dir / ".auto-claude" / "worktrees" / "tasks"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, examine the file structure and the specific line mentioned
cd /tmp && find . -name "models.py" -path "*/workspace/*" 2>/dev/null | head -5

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Let me check the actual file from the context of the review
head -n 1 /dev/null && echo "Ready to investigate"

Repository: AndyMik90/Auto-Claude

Length of output: 85


🏁 Script executed:

# Get the git repository structure to find the file
find . -name "models.py" -path "*/core/workspace/*" 2>/dev/null

Repository: AndyMik90/Auto-Claude

Length of output: 104


🏁 Script executed:

# List Python files in core/workspace to understand structure
fd "\.py$" "apps/backend/core/workspace" 2>/dev/null | head -20

Repository: AndyMik90/Auto-Claude

Length of output: 303


🏁 Script executed:

# Check the actual line 252 and surrounding context
cat -n apps/backend/core/workspace/models.py | sed -n '240,265p'

Repository: AndyMik90/Auto-Claude

Length of output: 1214


🏁 Script executed:

# Search for worktree-related code and path handling
rg "worktrees_dir|\.worktrees" apps/backend/core/workspace/ -A 3 -B 3

Repository: AndyMik90/Auto-Claude

Length of output: 887


🏁 Script executed:

# Look for migration or backward compatibility code
rg "migration|backward|compat|old.*location" apps/backend/ -i

Repository: AndyMik90/Auto-Claude

Length of output: 48145


🏁 Script executed:

# Check for spec number calculation logic
rg "spec.*number|get_next_spec|worktree.*spec" apps/backend/ -i -A 5

Repository: AndyMik90/Auto-Claude

Length of output: 46364


🏁 Script executed:

# Look for references to the old .worktrees location
rg "\.worktrees" apps/backend/ --type py

Repository: AndyMik90/Auto-Claude

Length of output: 870


🏁 Script executed:

# Check CLAUDE.md for worktree-related guidance
cat CLAUDE.md | head -100

Repository: AndyMik90/Auto-Claude

Length of output: 2929


🏁 Script executed:

# Look for any migration scripts or documentation
find . -name "*migrat*" -o -name "*MIGRATION*" -o -name "*upgrade*" | grep -v node_modules | grep -v ".git"

Repository: AndyMik90/Auto-Claude

Length of output: 188


🏁 Script executed:

# Check git history for this specific line to understand if it changed
git log -p --all -S '.worktrees' -- apps/backend/core/workspace/ 2>/dev/null | head -100

Repository: AndyMik90/Auto-Claude

Length of output: 47


🏁 Script executed:

# Look for any references in README or documentation
find . -name "README*" -o -name "*.md" | xargs grep -l "worktree" 2>/dev/null | head -10

Repository: AndyMik90/Auto-Claude

Length of output: 365


🏁 Script executed:

# Check if there's any old .worktrees directory handling in the codebase
rg "worktrees_dir|old.*location|backward.*compat" apps/backend/core/workspace/ -B 2 -A 2

Repository: AndyMik90/Auto-Claude

Length of output: 3114


🏁 Script executed:

# Check CHANGELOG for context about the worktree path change
grep -A 5 -B 5 "worktree\|\.auto-claude\|\.worktrees" CHANGELOG.md 2>/dev/null | head -50

Repository: AndyMik90/Auto-Claude

Length of output: 1558


🏁 Script executed:

# Check if there are any warnings or migration notes in README
grep -A 10 -B 2 "migration\|worktree" README.md 2>/dev/null | head -80

Repository: AndyMik90/Auto-Claude

Length of output: 952


🏁 Script executed:

# Look for any test files that might show the expected behavior
find . -name "*test*" -type f -path "*/workspace/*" -o -path "*/worktree/*" | head -5

Repository: AndyMik90/Auto-Claude

Length of output: 47


Add fallback logic to scan old worktree location for spec number calculation or provide user migration guidance.

The SpecNumberLock.get_next_spec_number() method only scans specs in .auto-claude/worktrees/tasks/ but does not check for specs in any legacy .worktrees/ location. If existing worktrees with specs were created under the old path, they will be orphaned and their spec numbers won't be included in the global maximum, creating a collision risk.

Since the PR description mentions removing backward-compatibility code, either:

  1. Add fallback logic to also scan the old location (if it existed in prior versions), or
  2. Provide clear migration guidance in CLAUDE.md and release notes for users to manually migrate or clean up old worktrees

Currently, no migration code or user guidance exists for this breaking change.

if worktrees_dir.exists():
for worktree in worktrees_dir.iterdir():
if worktree.is_dir():
Expand Down
103 changes: 16 additions & 87 deletions apps/backend/core/worktree.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
=============================================

Each spec gets its own worktree:
- Worktree path: .worktrees/{spec-name}/
- Worktree path: .auto-claude/worktrees/tasks/{spec-name}/
- Branch name: auto-claude/{spec-name}

This allows:
Expand Down Expand Up @@ -48,14 +48,14 @@ class WorktreeManager:
"""
Manages per-spec Git worktrees.

Each spec gets its own worktree in .worktrees/{spec-name}/ with
Each spec gets its own worktree in .auto-claude/worktrees/tasks/{spec-name}/ with
a corresponding branch auto-claude/{spec-name}.
"""

def __init__(self, project_dir: Path, base_branch: str | None = None):
self.project_dir = project_dir
self.base_branch = base_branch or self._detect_base_branch()
self.worktrees_dir = project_dir / ".worktrees"
self.worktrees_dir = project_dir / ".auto-claude" / "worktrees" / "tasks"
self._merge_lock = asyncio.Lock()

def _detect_base_branch(self) -> str:
Expand Down Expand Up @@ -194,7 +194,7 @@ def _unstage_gitignored_files(self) -> None:

def setup(self) -> None:
"""Create worktrees directory if needed."""
self.worktrees_dir.mkdir(exist_ok=True)
self.worktrees_dir.mkdir(parents=True, exist_ok=True)

# ==================== Per-Spec Worktree Methods ====================

Expand Down Expand Up @@ -478,14 +478,12 @@ def list_all_worktrees(self) -> list[WorktreeInfo]:
"""List all spec worktrees."""
worktrees = []

if not self.worktrees_dir.exists():
return worktrees

for item in self.worktrees_dir.iterdir():
if item.is_dir():
info = self.get_worktree_info(item.name)
if info:
worktrees.append(info)
if self.worktrees_dir.exists():
for item in self.worktrees_dir.iterdir():
if item.is_dir():
info = self.get_worktree_info(item.name)
if info:
worktrees.append(info)

return worktrees

Expand Down Expand Up @@ -587,81 +585,12 @@ def get_test_commands(self, spec_name: str) -> list[str]:

return commands

# ==================== Backward Compatibility ====================
# These methods provide backward compatibility with the old single-worktree API

def get_staging_path(self) -> Path | None:
"""
Backward compatibility: Get path to any existing spec worktree.
Prefer using get_worktree_path(spec_name) instead.
"""
worktrees = self.list_all_worktrees()
if worktrees:
return worktrees[0].path
return None

def get_staging_info(self) -> WorktreeInfo | None:
"""
Backward compatibility: Get info about any existing spec worktree.
Prefer using get_worktree_info(spec_name) instead.
"""
worktrees = self.list_all_worktrees()
if worktrees:
return worktrees[0]
return None

def merge_staging(self, delete_after: bool = True) -> bool:
"""
Backward compatibility: Merge first found worktree.
Prefer using merge_worktree(spec_name) instead.
"""
worktrees = self.list_all_worktrees()
if worktrees:
return self.merge_worktree(worktrees[0].spec_name, delete_after)
return False

def remove_staging(self, delete_branch: bool = True) -> None:
"""
Backward compatibility: Remove first found worktree.
Prefer using remove_worktree(spec_name) instead.
"""
worktrees = self.list_all_worktrees()
if worktrees:
self.remove_worktree(worktrees[0].spec_name, delete_branch)

def get_or_create_staging(self, spec_name: str) -> WorktreeInfo:
"""
Backward compatibility: Alias for get_or_create_worktree.
"""
return self.get_or_create_worktree(spec_name)

def staging_exists(self) -> bool:
"""
Backward compatibility: Check if any spec worktree exists.
Prefer using worktree_exists(spec_name) instead.
"""
return len(self.list_all_worktrees()) > 0

def commit_in_staging(self, message: str) -> bool:
"""
Backward compatibility: Commit in first found worktree.
Prefer using commit_in_worktree(spec_name, message) instead.
"""
worktrees = self.list_all_worktrees()
if worktrees:
return self.commit_in_worktree(worktrees[0].spec_name, message)
return False

def has_uncommitted_changes(self, in_staging: bool = False) -> bool:
def has_uncommitted_changes(self, spec_name: str | None = None) -> bool:
"""Check if there are uncommitted changes."""
worktrees = self.list_all_worktrees()
if in_staging and worktrees:
cwd = worktrees[0].path
else:
cwd = None
cwd = None
if spec_name:
worktree_path = self.get_worktree_path(spec_name)
if worktree_path.exists():
cwd = worktree_path
result = self._run_git(["status", "--porcelain"], cwd=cwd)
return bool(result.stdout.strip())


# Keep STAGING_WORKTREE_NAME for backward compatibility in imports
STAGING_WORKTREE_NAME = "auto-claude"
20 changes: 1 addition & 19 deletions apps/backend/merge/git_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,11 @@ def find_worktree(project_dir: Path, task_id: str) -> Path | None:
Returns:
Path to the worktree, or None if not found
"""
# Check common locations
worktrees_dir = project_dir / ".worktrees"
worktrees_dir = project_dir / ".auto-claude" / "worktrees" / "tasks"
if worktrees_dir.exists():
# Look for worktree with task_id in name
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent legacy worktree path fallback across modules

The find_worktree function in merge/git_utils.py only checks the new path (.auto-claude/worktrees/tasks/), while get_existing_build_worktree in core/workspace/git_utils.py includes a legacy fallback to .worktrees/. This inconsistency means merge operations won't find worktrees that still exist in the old location, while other operations will. Users with existing worktrees in the legacy path may experience failures in merge-related workflows.

Additional Locations (1)

Fix in Cursor Fix in Web

for entry in worktrees_dir.iterdir():
if entry.is_dir() and task_id in entry.name:
return entry

# Try git worktree list
try:
result = subprocess.run(
["git", "worktree", "list", "--porcelain"],
cwd=project_dir,
capture_output=True,
text=True,
check=True,
)
for line in result.stdout.split("\n"):
if line.startswith("worktree ") and task_id in line:
return Path(line.split(" ", 1)[1])
except subprocess.CalledProcessError:
pass

return None


Expand Down
9 changes: 8 additions & 1 deletion apps/backend/merge/timeline_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,14 @@ def get_worktree_file_content(self, task_id: str, file_path: str) -> str:
task_id.replace("task-", "") if task_id.startswith("task-") else task_id
)

worktree_path = self.project_path / ".worktrees" / spec_name / file_path
worktree_path = (
self.project_path
/ ".auto-claude"
/ "worktrees"
/ "tasks"
/ spec_name
/ file_path
)
Comment on lines +192 to +199
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider single-line path construction for consistency.

The multi-line path construction here differs from the single-line style used consistently throughout the codebase (see apps/backend/core/workspace/models.py:252 and apps/backend/cli/batch_commands.py:187).

🔎 Proposed refactor for consistency
-        worktree_path = (
-            self.project_path
-            / ".auto-claude"
-            / "worktrees"
-            / "tasks"
-            / spec_name
-            / file_path
-        )
+        worktree_path = self.project_path / ".auto-claude" / "worktrees" / "tasks" / spec_name / file_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.

Suggested change
worktree_path = (
self.project_path
/ ".auto-claude"
/ "worktrees"
/ "tasks"
/ spec_name
/ file_path
)
worktree_path = self.project_path / ".auto-claude" / "worktrees" / "tasks" / spec_name / file_path
🤖 Prompt for AI Agents
In apps/backend/merge/timeline_git.py around lines 192 to 199, the worktree_path
is built across multiple lines; change it to a single-line Path join (e.g.,
self.project_path / ".auto-claude" / "worktrees" / "tasks" / spec_name /
file_path expressed on one line) to match the single-line style used elsewhere
for consistency, ensuring line length stays reasonable or wrap only by breaking
before operators if necessary.

if worktree_path.exists():
try:
return worktree_path.read_text(encoding="utf-8")
Expand Down
17 changes: 17 additions & 0 deletions apps/backend/prompts/github/pr_codebase_fit_agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,23 @@ You are a focused codebase fit review agent. You have been spawned by the orches

Ensure new code integrates well with the existing codebase. Check for consistency with project conventions, reuse of existing utilities, and architectural alignment. Focus ONLY on codebase fit - not security, logic correctness, or general quality.

## CRITICAL: PR Scope and Context

### What IS in scope (report these issues):
1. **Codebase fit issues in changed code** - New code not following project patterns
2. **Missed reuse opportunities** - "Existing `utils.ts` has a helper for this"
3. **Inconsistent with PR's own changes** - "You used `camelCase` here but `snake_case` elsewhere in the PR"
4. **Breaking conventions in touched areas** - "Your change deviates from the pattern in this file"

### What is NOT in scope (do NOT report):
1. **Pre-existing inconsistencies** - Old code that doesn't follow patterns
2. **Unrelated suggestions** - Don't suggest patterns for code the PR didn't touch

**Key distinction:**
- ✅ "Your new component doesn't follow the existing pattern in `components/`" - GOOD
- ✅ "Consider using existing `formatDate()` helper instead of new implementation" - GOOD
- ❌ "The old `legacy/` folder uses different naming conventions" - BAD (pre-existing)

## Codebase Fit Focus Areas

### 1. Naming Conventions
Expand Down
46 changes: 36 additions & 10 deletions apps/backend/prompts/github/pr_finding_validator.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,32 @@ You are a finding re-investigator. For each unresolved finding from a previous P

Your job is to prevent false positives from persisting indefinitely by actually reading the code and verifying the issue exists.

## CRITICAL: Check PR Scope First

**Before investigating any finding, verify it's within THIS PR's scope:**

1. **Check if the file is in the PR's changed files list** - If not, likely out-of-scope
2. **Check if the line number exists** - If finding cites line 710 but file has 600 lines, it's hallucinated
3. **Check for PR references in commit messages** - Commits like `fix: something (#584)` are from OTHER PRs

**Dismiss findings as `dismissed_false_positive` if:**
- The finding references a file NOT in the PR's changed files list AND is not about impact on that file
- The line number doesn't exist in the file (hallucinated)
- The finding is about code from a merged branch commit (not this PR's work)

**Keep findings valid if they're about:**
- Issues in code the PR actually changed
- Impact of PR changes on other code (e.g., "this change breaks callers in X")
- Missing updates to related code (e.g., "you updated A but forgot B")

## Your Mission

For each finding you receive:
1. **READ** the actual code at the file/line location using the Read tool
2. **ANALYZE** whether the described issue actually exists in the code
3. **PROVIDE** concrete code evidence for your conclusion
4. **RETURN** validation status with evidence
1. **VERIFY SCOPE** - Is this file/line actually part of this PR?
2. **READ** the actual code at the file/line location using the Read tool
3. **ANALYZE** whether the described issue actually exists in the code
4. **PROVIDE** concrete code evidence for your conclusion
5. **RETURN** validation status with evidence

## Investigation Process

Expand Down Expand Up @@ -122,12 +141,19 @@ Rate your confidence based on how certain you are:

Watch for these patterns that often indicate false positives:

1. **Sanitization elsewhere**: Input is validated/sanitized before reaching the flagged code
2. **Internal-only code**: Code only handles trusted internal data, not user input
3. **Framework protection**: Framework provides automatic protection (e.g., ORM parameterization)
4. **Dead code**: The flagged code is never executed in the current codebase
5. **Test code**: The issue is in test files where it's acceptable
6. **Misread syntax**: Original reviewer misunderstood the language syntax
1. **Non-existent line number**: The line number cited doesn't exist or is beyond EOF - hallucinated finding
2. **Merged branch code**: Finding is about code from a commit like `fix: something (#584)` - another PR
3. **Pre-existing issue, not impact**: Finding flags old bug in untouched code without showing how PR changes relate
4. **Sanitization elsewhere**: Input is validated/sanitized before reaching the flagged code
5. **Internal-only code**: Code only handles trusted internal data, not user input
6. **Framework protection**: Framework provides automatic protection (e.g., ORM parameterization)
7. **Dead code**: The flagged code is never executed in the current codebase
8. **Test code**: The issue is in test files where it's acceptable
9. **Misread syntax**: Original reviewer misunderstood the language syntax

**Note**: Findings about files outside the PR's changed list are NOT automatically false positives if they're about:
- Impact of PR changes on that file (e.g., "your change breaks X")
- Missing related updates (e.g., "you forgot to update Y")

## Common Valid Issue Patterns

Expand Down
Loading
Loading