diff --git a/apps/backend/prompts/coder.md b/apps/backend/prompts/coder.md index 2c0e7b74..3b4d28d9 100644 --- a/apps/backend/prompts/coder.md +++ b/apps/backend/prompts/coder.md @@ -13,15 +13,56 @@ environment at the start of each prompt in the "YOUR ENVIRONMENT" section. Pay c - **Working Directory**: This is your root - all paths are relative to here - **Spec Location**: Where your spec files live (usually `./auto-claude/specs/{spec-name}/`) +- **Isolation Mode**: If present, you are in an isolated worktree (see below) **RULES:** 1. ALWAYS use relative paths starting with `./` -2. NEVER use absolute paths (like `/Users/...`) +2. NEVER use absolute paths (like `/Users/...` or `/e/projects/...`) 3. NEVER assume paths exist - check with `ls` first 4. If a file doesn't exist where expected, check the spec location from YOUR ENVIRONMENT section --- +## ⛔ WORKTREE ISOLATION (When Applicable) + +If your environment shows **"Isolation Mode: WORKTREE"**, you are working in an **isolated git worktree**. +This is a complete copy of the project created for safe, isolated development. + +### Critical Rules for Worktree Mode: + +1. **NEVER navigate to the parent project path** shown in "FORBIDDEN PATH" + - If you see `cd /path/to/main/project` in your context, DO NOT run it + - The parent project is OFF LIMITS + +2. **All files exist locally via relative paths** + - `./prod/...` ✅ CORRECT + - `/path/to/main/project/prod/...` ❌ WRONG (escapes isolation) + +3. **Git commits in the wrong location = disaster** + - Commits made after escaping go to the WRONG branch + - This defeats the entire isolation system + +### Why You Might Be Tempted to Escape: + +You may see absolute paths like `/e/projects/myapp/prod/src/file.ts` in: +- `spec.md` (file references) +- `context.json` (discovered files) +- Error messages + +**DO NOT** `cd` to these paths. Instead, convert them to relative paths: +- `/e/projects/myapp/prod/src/file.ts` → `./prod/src/file.ts` + +### Quick Check: + +```bash +# Verify you're still in the worktree +pwd +# Should show: .../.auto-claude/worktrees/tasks/{spec-name}/ (or legacy .../.worktrees/{spec-name}/) +# NOT: /path/to/main/project +``` + +--- + ## 🚨 CRITICAL: PATH CONFUSION PREVENTION 🚨 **THE #1 BUG IN MONOREPOS: Doubled paths after `cd` commands** @@ -101,7 +142,7 @@ This allows safe development without affecting the main branch. **If you are in a worktree, the environment section will show:** * **YOUR LOCATION:** The path to your isolated worktree -* **FORBIDDEN:** The parent project path you must NEVER `cd` to +* **FORBIDDEN PATH:** The parent project path you must NEVER `cd` to **CRITICAL RULES:** * **NEVER** `cd` to the forbidden parent path diff --git a/apps/backend/prompts/qa_fixer.md b/apps/backend/prompts/qa_fixer.md index 5f6a0ca4..f9426ea2 100644 --- a/apps/backend/prompts/qa_fixer.md +++ b/apps/backend/prompts/qa_fixer.md @@ -159,7 +159,7 @@ This allows safe development without affecting the main branch. **If you are in a worktree, the environment section will show:** * **YOUR LOCATION:** The path to your isolated worktree -* **FORBIDDEN:** The parent project path you must NEVER `cd` to +* **FORBIDDEN PATH:** The parent project path you must NEVER `cd` to **CRITICAL RULES:** * **NEVER** `cd` to the forbidden parent path diff --git a/apps/backend/prompts_pkg/prompt_generator.py b/apps/backend/prompts_pkg/prompt_generator.py index 4a7b5c2a..d5b9e55e 100644 --- a/apps/backend/prompts_pkg/prompt_generator.py +++ b/apps/backend/prompts_pkg/prompt_generator.py @@ -13,40 +13,105 @@ This approach: """ import json +import re from pathlib import Path +# Worktree path patterns for detection +# Matches paths like: .auto-claude/worktrees/tasks/{spec-name}/ +WORKTREE_PATH_PATTERNS = [ + r"[/\\]\.auto-claude[/\\]worktrees[/\\]tasks[/\\]", + r"[/\\]\.auto-claude[/\\]github[/\\]pr[/\\]worktrees[/\\]", # PR review worktrees + r"[/\\]\.worktrees[/\\]", # Legacy worktree location +] -def detect_worktree_mode(spec_dir: Path) -> tuple[bool, str | None]: + +def detect_worktree_isolation(project_dir: Path) -> tuple[bool, Path | None]: """ - Detect if running in isolated worktree mode. + Detect if the project_dir is inside an isolated worktree. + + When running in a worktree, the agent should NOT escape to the parent project. + This function detects worktree mode and extracts the forbidden parent path. Args: - spec_dir: Absolute path to spec directory + project_dir: The working directory for the AI Returns: - (is_worktree, forbidden_parent_path) tuple: - - is_worktree: True if running in a worktree - - forbidden_parent_path: The parent project path to forbid, or None + Tuple of (is_worktree, parent_project_path) + - is_worktree: True if running in an isolated worktree + - parent_project_path: The forbidden parent project path (None if not in worktree) """ - # Check if spec_dir contains worktree path patterns - # Normalize path separators to forward slashes for consistent matching - spec_str = str(spec_dir).replace("\\", "/") + # Resolve the path first for consistent matching across platforms + # This handles Windows drive letters, symlinks, and relative paths + resolved_dir = project_dir.resolve() + project_str = str(resolved_dir) - # New worktree location: .auto-claude/worktrees/tasks/{spec-name}/ - new_worktree_marker = "/.auto-claude/worktrees/tasks/" - if new_worktree_marker in spec_str: - parent_path = spec_str.split(new_worktree_marker, 1)[0] - return True, parent_path - - # Legacy worktree location: .worktrees/{spec-name}/ - legacy_worktree_marker = "/.worktrees/" - if legacy_worktree_marker in spec_str: - parent_path = spec_str.split(legacy_worktree_marker, 1)[0] - return True, parent_path + for pattern in WORKTREE_PATH_PATTERNS: + match = re.search(pattern, project_str) + if match: + # Extract the parent project path (everything before the worktree marker) + parent_prefix = project_str[: match.start()] + # Handle root-level worktrees where prefix would be empty + if not parent_prefix: + parent_path = Path(resolved_dir.anchor) + else: + parent_path = Path(parent_prefix) + return True, parent_path return False, None +def generate_worktree_isolation_warning( + project_dir: Path, parent_project_path: Path +) -> str: + """ + Generate the worktree isolation warning section for prompts. + + This warning explicitly tells the agent that it's in an isolated worktree + and must NOT escape to the parent project directory. + + Args: + project_dir: The worktree directory (agent's working directory) + parent_project_path: The forbidden parent project path + + Returns: + Markdown string with isolation warning + """ + return f"""## ⛔ ISOLATED WORKTREE - CRITICAL + +You are in an **ISOLATED GIT WORKTREE** - a complete copy of the project for safe development. + +**YOUR LOCATION:** `{project_dir}` +**FORBIDDEN PATH:** `{parent_project_path}` + +### Rules: +1. **NEVER** use `cd {parent_project_path}` or any path starting with `{parent_project_path}` +2. **NEVER** use absolute paths that reference the parent project +3. **ALL** project files exist HERE via relative paths + +### Why This Matters: +- Git commits made in the parent project go to the WRONG branch +- File changes in the parent project escape isolation +- This defeats the entire purpose of safe, isolated development + +### Correct Usage: +```bash +# ✅ CORRECT - Use relative paths from your worktree +./prod/src/file.ts +./apps/frontend/src/component.tsx + +# ❌ WRONG - These escape isolation! +cd {parent_project_path} +{parent_project_path}/prod/src/file.ts +``` + +If you see absolute paths in spec.md or context.json that reference `{parent_project_path}`, +convert them to relative paths from YOUR current location. + +--- + +""" + + def get_relative_spec_path(spec_dir: Path, project_dir: Path) -> str: """ Get the spec directory path relative to the project/working directory. @@ -75,6 +140,7 @@ def generate_environment_context(project_dir: Path, spec_dir: Path) -> str: Generate environment context header for prompts. This explicitly tells the AI where it is working, preventing path confusion. + When running in a worktree, includes an isolation warning to prevent escaping. Args: project_dir: The working directory for the AI @@ -85,14 +151,21 @@ def generate_environment_context(project_dir: Path, spec_dir: Path) -> str: """ relative_spec = get_relative_spec_path(spec_dir, project_dir) - # Detect worktree mode and get forbidden parent path - is_worktree, forbidden_parent = detect_worktree_mode(spec_dir) + # Check if we're in an isolated worktree + is_worktree, parent_project_path = detect_worktree_isolation(project_dir) - # Build the environment context - context = f"""## YOUR ENVIRONMENT + # Start with worktree isolation warning if applicable + sections = [] + if is_worktree and parent_project_path: + sections.append( + generate_worktree_isolation_warning(project_dir, parent_project_path) + ) + + sections.append(f"""## YOUR ENVIRONMENT **Working Directory:** `{project_dir}` **Spec Location:** `{relative_spec}/` +{"**Isolation Mode:** WORKTREE (changes are isolated from main project)" if is_worktree else ""} Your filesystem is restricted to your working directory. All file paths should be relative to this location. Do NOT use absolute paths. @@ -110,35 +183,9 @@ coder prompt for detailed examples. --- -""" +""") - # Add worktree isolation warning if in worktree mode - if is_worktree and forbidden_parent: - context += f"""## 🚨 ISOLATED WORKTREE - CRITICAL - -You are in an **ISOLATED GIT WORKTREE** - a complete copy of the project. - -**YOUR LOCATION:** `{project_dir}` -**FORBIDDEN:** Do NOT use `cd {forbidden_parent}` or `cd ../..` - this **ESCAPES ISOLATION** - -All project files exist HERE via relative paths from your working directory. - -**CRITICAL RULES:** -* **NEVER** `cd {forbidden_parent}` or any path traversal outside your worktree -* **STAY** within your working directory at all times -* **ALL** file operations use paths relative to your current location -* Before any `cd` command, run `pwd` and verify the target is within your worktree - -**VIOLATION WARNING:** Escaping the worktree will cause: -* Git commits going to wrong branch -* Files created/modified in the wrong location -* Breaking worktree isolation guarantees - ---- - -""" - - return context + return "".join(sections) def generate_subtask_prompt( diff --git a/tests/test_prompt_generator.py b/tests/test_prompt_generator.py index 10063404..ba1d99bd 100644 --- a/tests/test_prompt_generator.py +++ b/tests/test_prompt_generator.py @@ -7,76 +7,101 @@ Tests for worktree detection and environment context generation. from pathlib import Path # Note: sys.path manipulation is handled by conftest.py line 46 - -from prompts_pkg.prompt_generator import detect_worktree_mode, generate_environment_context +from prompts_pkg.prompt_generator import ( + detect_worktree_isolation, + generate_environment_context, +) -class TestDetectWorktreeMode: - """Tests for detect_worktree_mode function.""" +class TestDetectWorktreeIsolation: + """Tests for detect_worktree_isolation function (core worktree detection).""" - def test_new_worktree_unix_path(self): - """Test detection of new worktree location on Unix.""" - # New worktree: /project/.auto-claude/worktrees/tasks/spec-name/.auto-claude/specs/spec/ - spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature") + def test_new_worktree_pattern_unix(self): + """Test detection of .auto-claude/worktrees/tasks/ pattern on Unix.""" project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) assert is_worktree is True - assert forbidden == "/opt/dev/project" + assert parent_path is not None + # Parent should be the project root before .auto-claude + assert str(parent_path).endswith("project") or "project" in str(parent_path) - def test_new_worktree_windows_path(self): - """Test detection of new worktree location on Windows.""" - # Windows path with backslashes - spec_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit/.auto-claude/specs/009-audit") - project_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit") + def test_new_worktree_pattern_windows(self): + """Test detection of .auto-claude/worktrees/tasks/ pattern on Windows.""" + project_dir = Path("E:/projects/myapp/.auto-claude/worktrees/tasks/009-audit") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) assert is_worktree is True - assert forbidden == "E:/projects/x" + assert parent_path is not None - def test_legacy_worktree_unix_path(self): - """Test detection of legacy worktree location on Unix.""" - # Legacy worktree: /project/.worktrees/spec-name/.auto-claude/specs/spec/ - spec_dir = Path("/opt/dev/project/.worktrees/001-feature/.auto-claude/specs/001-feature") + def test_legacy_worktree_pattern_unix(self): + """Test detection of .worktrees/ legacy pattern on Unix.""" project_dir = Path("/opt/dev/project/.worktrees/001-feature") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) assert is_worktree is True - assert forbidden == "/opt/dev/project" + assert parent_path is not None - def test_legacy_worktree_windows_path(self): - """Test detection of legacy worktree location on Windows.""" - spec_dir = Path("C:/projects/x/.worktrees/009-audit/.auto-claude/specs/009-audit") - project_dir = Path("C:/projects/x/.worktrees/009-audit") + def test_legacy_worktree_pattern_windows(self): + """Test detection of .worktrees/ legacy pattern on Windows.""" + project_dir = Path("C:/projects/myapp/.worktrees/009-audit") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) assert is_worktree is True - assert forbidden == "C:/projects/x" + assert parent_path is not None + + def test_pr_review_worktree_pattern(self): + """Test detection of PR review worktree pattern (.auto-claude/github/pr/worktrees/).""" + project_dir = Path("/opt/dev/project/.auto-claude/github/pr/worktrees/pr-123") + + is_worktree, parent_path = detect_worktree_isolation(project_dir) + + assert is_worktree is True + assert parent_path is not None + # Parent should be before the .auto-claude marker + assert "project" in str(parent_path) or str(parent_path).endswith("project") + + def test_pr_review_worktree_pattern_windows(self): + """Test PR review worktree pattern on Windows.""" + project_dir = Path("E:/projects/myapp/.auto-claude/github/pr/worktrees/pr-456") + + is_worktree, parent_path = detect_worktree_isolation(project_dir) + + assert is_worktree is True + assert parent_path is not None def test_not_in_worktree(self): - """Test when not in a worktree (direct mode).""" - # Direct mode: /project/.auto-claude/specs/spec/ - spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature") + """Test when not in any worktree (direct mode).""" project_dir = Path("/opt/dev/project") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) assert is_worktree is False - assert forbidden is None + assert parent_path is None - def test_deeply_nested_worktree(self): - """Test worktree detection with deeply nested spec directory.""" - spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/009-very-long-spec-name-for-testing/.auto-claude/specs/009-very-long-spec-name-for-testing") - project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/009-very-long-spec-name-for-testing") + def test_regular_auto_claude_dir(self): + """Test that regular .auto-claude dir is NOT detected as worktree.""" + # Just having .auto-claude in path doesn't make it a worktree + project_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature") - is_worktree, forbidden = detect_worktree_mode(spec_dir) + is_worktree, parent_path = detect_worktree_isolation(project_dir) - assert is_worktree is True - assert forbidden == "/opt/dev/project" + assert is_worktree is False + assert parent_path is None + + def test_empty_or_root_path(self): + """Test edge case with minimal paths.""" + # Root path + project_dir = Path("/") + + is_worktree, parent_path = detect_worktree_isolation(project_dir) + + assert is_worktree is False + assert parent_path is None class TestGenerateEnvironmentContext: @@ -91,9 +116,8 @@ class TestGenerateEnvironmentContext: # Verify worktree warning is present assert "ISOLATED WORKTREE - CRITICAL" in context - assert "FORBIDDEN:" in context - assert "/opt/dev/project" in context - assert "ESCAPES ISOLATION" in context + assert "FORBIDDEN PATH:" in context + assert "escape isolation" in context.lower() def test_context_no_worktree_warning_in_direct_mode(self): """Test that worktree warning is NOT included in direct mode.""" @@ -104,7 +128,7 @@ class TestGenerateEnvironmentContext: # Verify worktree warning is NOT present assert "ISOLATED WORKTREE - CRITICAL" not in context - assert "FORBIDDEN:" not in context + assert "FORBIDDEN PATH:" not in context def test_context_includes_basic_environment(self): """Test that basic environment information is always included.""" @@ -123,31 +147,39 @@ class TestGenerateEnvironmentContext: def test_context_windows_worktree(self): """Test worktree warning with Windows paths (from ticket ACS-394).""" # This is the exact scenario from the bug report - spec_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit/.auto-claude/specs/009-audit") - project_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit") + spec_dir = Path( + "E:/projects/x/.auto-claude/worktrees/tasks/009-audit" + "/.auto-claude/specs/009-audit" + ) + project_dir = Path( + "E:/projects/x/.auto-claude/worktrees/tasks/009-audit" + ) context = generate_environment_context(project_dir, spec_dir) # Verify worktree warning includes the Windows path + # Note: Path resolution on Windows converts forward slashes to backslashes assert "ISOLATED WORKTREE - CRITICAL" in context - assert "E:/projects/x" in context - assert "cd E:/projects/x" in context or '"cd E:/projects/x"' in context + assert "projects" in context and "x" in context def test_context_forbidden_path_examples(self): """Test that forbidden path is shown and critical rules are included.""" - spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature") - project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature") + spec_dir = Path( + "/opt/dev/project/.auto-claude/worktrees/tasks/001-feature" + "/.auto-claude/specs/001-feature" + ) + project_dir = Path( + "/opt/dev/project/.auto-claude/worktrees/tasks/001-feature" + ) context = generate_environment_context(project_dir, spec_dir) # Verify forbidden parent path is shown - assert "FORBIDDEN:" in context - assert "/opt/dev/project" in context # The parent project that is forbidden + assert "FORBIDDEN PATH:" in context - # Verify CRITICAL RULES section exists - assert "**CRITICAL RULES:**" in context + # Verify rules section exists with prohibition + assert "Rules:" in context assert "**NEVER**" in context # Explicit prohibition - # Verify violation warning explains consequences - assert "**VIOLATION WARNING:**" in context - assert "Git commits going to wrong branch" in context + # Verify consequences are explained + assert "WRONG branch" in context