fix(merge): resolve multiple merge-related issues (ACS-194, ACS-179, ACS-174, ACS-163) (#885)
* fix(merge): resolve multiple merge-related issues (ACS-194, ACS-179, ACS-174, ACS-163)
- ACS-179: Fix TypeError in print_conflict_info - handle both string and dict conflict formats
- ACS-174: Handle git hook failures during merge - skip checkout if already on target branch
- ACS-163: Fix merge failure fallthrough - return False immediately when merge fails
- ACS-194: Improve AI merge success rate - add Haiku→Sonnet fallback with enhanced prompts
All fixes include regression tests.
* fix: improve merge robustness and fix test issues
This commit addresses multiple issues related to merge functionality
and test quality:
1. workspace.py:
- Fix case-insensitive natural language pattern matching to detect
AI explanations returned instead of code
2. worktree.py:
- Guard against None stderr when handling git hook failures
- Improve error messages for merge hook handling
3. workspace/display.py:
- Improve print_conflict_info() to properly categorize conflicts
(marker vs AI merge failures)
- Add severity indicators (🔴 for high, 🟡 for medium)
- Use shlex.quote() for proper git add command quoting
- Provide clearer guidance for different conflict types
4. tests/test_merge_ai_resolver.py:
- Fix test assertions to match actual AI_MERGE_SYSTEM_PROMPT content
(check for "intelligently" and "task's intent" instead of
non-existent "semantic understanding")
5. tests/test_workspace.py:
- Add side-effect verification for merge failure tests
- Remove unused fixtures
- Add assertions for severity emoji indicators
6. tests/test_worktree.py:
- Add subprocess return code checks for better test reliability
Related: ACS-194, ACS-179, ACS-174, ACS-163
* fix: improve display formatting and use proper imports
1. display.py:
- Fix trailing space when severity_icon is empty (low/unknown severity)
- Only add leading space to icon when icon is non-empty
2. workspace/__init__.py:
- Export AI_MERGE_SYSTEM_PROMPT and _build_merge_prompt
- Add to __all__ for public API access
3. test_merge_ai_resolver.py:
- Use standard imports from core.workspace package
- Remove fragile importlib.util dynamic loading
* fix: address all 6 review findings
1. workspace.py:
- Fix parameter naming: max_thinking -> max_thinking_tokens
(matches codebase convention used in 25+ locations)
- Extract hardcoded model constants: MERGE_FAST_MODEL,
MERGE_CAPABLE_MODEL, MERGE_FAST_THINKING, MERGE_COMPLEX_THINKING
- Improve natural language detection: check patterns at START of line
and require absence of code patterns to reduce false positives
2. display.py:
- Add critical severity icon handling (⛔ for critical severity)
3. worktree.py:
- Fix inconsistent stderr handling: use truthiness check for both
branches (empty strings show '<no stderr>')
4. test_workspace.py:
- Remove unused imports: patch, MagicMock from unittest.mock
Related: ACS-194
---------
Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
This commit is contained in:
+192
-90
@@ -277,6 +277,13 @@ def merge_existing_build(
|
||||
no_commit, stats, spec_name=spec_name, keep_worktree=True
|
||||
)
|
||||
return True
|
||||
else:
|
||||
# Standard git merge failed - report error and don't continue
|
||||
print()
|
||||
print_status(
|
||||
"Merge failed. Please check the errors above.", "error"
|
||||
)
|
||||
return False
|
||||
elif smart_result.get("git_conflicts"):
|
||||
# Had git conflicts that AI couldn't fully resolve
|
||||
resolved = smart_result.get("resolved", [])
|
||||
@@ -1425,18 +1432,41 @@ import os
|
||||
_merge_logger = logging.getLogger(__name__)
|
||||
|
||||
# System prompt for AI file merging
|
||||
AI_MERGE_SYSTEM_PROMPT = """You are an expert code merge assistant. Your task is to perform a 3-way merge of code files.
|
||||
AI_MERGE_SYSTEM_PROMPT = """You are an expert code merge assistant specializing in intelligent 3-way merges. Your task is to merge code changes from two branches while preserving all meaningful changes.
|
||||
|
||||
RULES:
|
||||
1. Preserve all functional changes from both versions (ours and theirs)
|
||||
2. Maintain code style consistency
|
||||
3. Resolve conflicts by understanding the semantic purpose of each change
|
||||
4. When changes are independent (different functions/sections), include both
|
||||
5. When changes overlap, combine them logically or prefer the more complete version
|
||||
6. Preserve all imports from both versions
|
||||
7. Output ONLY the merged code - no explanations, no markdown, no code fences
|
||||
CONTEXT:
|
||||
- "OURS" = current main branch (target for merge)
|
||||
- "THEIRS" = task worktree branch (changes being merged in)
|
||||
- "BASE" = common ancestor before changes
|
||||
|
||||
IMPORTANT: Output the raw merged file content only. Do not wrap in code blocks."""
|
||||
MERGE STRATEGY:
|
||||
1. **Preserve all functional changes** - Include all features, bug fixes, and improvements from both versions
|
||||
2. **Combine independent changes** - If changes are in different functions/sections, include both
|
||||
3. **Resolve overlapping changes intelligently**:
|
||||
- Prefer the more complete/updated implementation
|
||||
- Combine logic if both versions add value
|
||||
- When in doubt, favor the version that better addresses the task's intent
|
||||
4. **Maintain syntactic correctness** - Ensure the merged code is valid and compiles/runs
|
||||
5. **Preserve imports and dependencies** from both versions
|
||||
|
||||
HANDLING COMMON PATTERNS:
|
||||
- New functions/classes: Include all from both versions
|
||||
- Modified functions: Merge changes logically, prefer more complete version
|
||||
- Imports: Union of all imports from both versions
|
||||
- Comments/Documentation: Include relevant documentation from both
|
||||
- Configuration: Merge settings, with conflict resolution favoring task-specific values
|
||||
|
||||
CRITICAL RULES:
|
||||
- Output ONLY the merged code - no explanations, no prose, no markdown fences
|
||||
- If you cannot determine the correct merge, make a reasonable decision based on best practices
|
||||
- Never output error messages like "I need more context" - always provide a best-effort merge
|
||||
- Ensure the output is complete and syntactically valid code"""
|
||||
|
||||
# Model constants for AI merge two-tier strategy (ACS-194)
|
||||
MERGE_FAST_MODEL = "claude-haiku-4-5-20251001" # Fast model for simple merges
|
||||
MERGE_CAPABLE_MODEL = "claude-sonnet-4-5-20250929" # Capable model for complex merges
|
||||
MERGE_FAST_THINKING = 1024 # Lower thinking for fast/simple merges
|
||||
MERGE_COMPLEX_THINKING = 16000 # Higher thinking for complex merges
|
||||
|
||||
|
||||
def _infer_language_from_path(file_path: str) -> str:
|
||||
@@ -1525,7 +1555,7 @@ def _build_merge_prompt(
|
||||
if len(base_content) > 10000:
|
||||
base_content = base_content[:10000] + "\n... (truncated)"
|
||||
base_section = f"""
|
||||
BASE (common ancestor):
|
||||
BASE (common ancestor before changes):
|
||||
```{language}
|
||||
{base_content}
|
||||
```
|
||||
@@ -1537,20 +1567,22 @@ BASE (common ancestor):
|
||||
if len(worktree_content) > 15000:
|
||||
worktree_content = worktree_content[:15000] + "\n... (truncated)"
|
||||
|
||||
prompt = f"""Perform a 3-way merge for file: {file_path}
|
||||
Task being merged: {spec_name}
|
||||
prompt = f"""FILE: {file_path}
|
||||
TASK: {spec_name}
|
||||
|
||||
This is a 3-way code merge. You must combine changes from both versions.
|
||||
{base_section}
|
||||
OURS (current main branch):
|
||||
OURS (current main branch - target for merge):
|
||||
```{language}
|
||||
{main_content}
|
||||
```
|
||||
|
||||
THEIRS (changes from task worktree):
|
||||
THEIRS (task worktree branch - changes being merged):
|
||||
```{language}
|
||||
{worktree_content}
|
||||
```
|
||||
|
||||
Merge these versions, preserving all meaningful changes from both. Output only the merged file content, no explanations."""
|
||||
OUTPUT THE MERGED CODE ONLY. No explanations, no markdown fences."""
|
||||
|
||||
return prompt
|
||||
|
||||
@@ -1568,6 +1600,112 @@ def _strip_code_fences(content: str) -> str:
|
||||
return content
|
||||
|
||||
|
||||
async def _attempt_ai_merge(
|
||||
task: "ParallelMergeTask",
|
||||
prompt: str,
|
||||
model: str = MERGE_FAST_MODEL,
|
||||
max_thinking_tokens: int = MERGE_FAST_THINKING,
|
||||
) -> tuple[bool, str | None, str]:
|
||||
"""
|
||||
Attempt an AI merge with a specific model.
|
||||
|
||||
Args:
|
||||
task: The merge task with file contents
|
||||
prompt: The merge prompt
|
||||
model: Model to use for merge
|
||||
max_thinking_tokens: Max thinking tokens for the model
|
||||
|
||||
Returns:
|
||||
Tuple of (success, merged_content, error_message)
|
||||
"""
|
||||
try:
|
||||
from core.simple_client import create_simple_client
|
||||
except ImportError:
|
||||
return False, None, "core.simple_client not available"
|
||||
|
||||
client = create_simple_client(
|
||||
agent_type="merge_resolver",
|
||||
model=model,
|
||||
system_prompt=AI_MERGE_SYSTEM_PROMPT,
|
||||
max_thinking_tokens=max_thinking_tokens,
|
||||
)
|
||||
|
||||
response_text = ""
|
||||
async with client:
|
||||
await client.query(prompt)
|
||||
|
||||
async for msg in client.receive_response():
|
||||
msg_type = type(msg).__name__
|
||||
if msg_type == "AssistantMessage" and hasattr(msg, "content"):
|
||||
for block in msg.content:
|
||||
block_type = type(block).__name__
|
||||
if block_type == "TextBlock" and hasattr(block, "text"):
|
||||
response_text += block.text
|
||||
|
||||
if response_text:
|
||||
merged_content = _strip_code_fences(response_text.strip())
|
||||
|
||||
# Check if AI returned natural language instead of code (case-insensitive)
|
||||
# More robust detection: (1) Check if patterns are at START of line, (2) Check for
|
||||
# absence of code patterns like imports, function definitions, braces, etc.
|
||||
natural_language_patterns = [
|
||||
"i need to",
|
||||
"let me",
|
||||
"i cannot",
|
||||
"i'm unable",
|
||||
"the file appears",
|
||||
"i don't have",
|
||||
"unfortunately",
|
||||
"i apologize",
|
||||
]
|
||||
|
||||
first_line = merged_content.split("\n")[0] if merged_content else ""
|
||||
first_line_stripped = first_line.lstrip()
|
||||
first_line_lower = first_line_stripped.lower()
|
||||
|
||||
# Check if first line STARTS with natural language pattern (not just contains it)
|
||||
starts_with_prose = any(
|
||||
first_line_lower.startswith(pattern)
|
||||
for pattern in natural_language_patterns
|
||||
)
|
||||
|
||||
# Also check for absence of common code patterns to reduce false positives
|
||||
has_code_patterns = any(
|
||||
pattern in merged_content[:500] # Check first 500 chars for code patterns
|
||||
for pattern in [
|
||||
"import ", # Python/JS/TypeScript imports
|
||||
"from ", # Python imports
|
||||
"def ", # Python functions
|
||||
"function ", # JavaScript functions
|
||||
"const ", # JavaScript/TypeScript const
|
||||
"class ", # Class definitions
|
||||
"{", # Braces indicate code
|
||||
"}", # Braces indicate code
|
||||
"#!", # Shebang
|
||||
"<!--", # HTML comment
|
||||
]
|
||||
)
|
||||
|
||||
# Only reject if it starts with prose AND lacks code patterns
|
||||
if starts_with_prose and not has_code_patterns:
|
||||
return (
|
||||
False,
|
||||
None,
|
||||
f"AI returned explanation instead of code: {first_line[:80]}...",
|
||||
)
|
||||
|
||||
# Validate syntax
|
||||
is_valid, syntax_error = _validate_merged_syntax(
|
||||
task.file_path, merged_content, task.project_dir
|
||||
)
|
||||
if not is_valid:
|
||||
return False, None, f"Invalid syntax: {syntax_error}"
|
||||
|
||||
return True, merged_content, ""
|
||||
else:
|
||||
return False, None, "AI returned empty response"
|
||||
|
||||
|
||||
async def _merge_file_with_ai_async(
|
||||
task: ParallelMergeTask,
|
||||
semaphore: asyncio.Semaphore,
|
||||
@@ -1625,83 +1763,42 @@ async def _merge_file_with_ai_async(
|
||||
task.spec_name,
|
||||
)
|
||||
|
||||
# Call Claude Haiku for fast merge
|
||||
try:
|
||||
from core.simple_client import create_simple_client
|
||||
except ImportError:
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=None,
|
||||
success=False,
|
||||
error="core.simple_client not available",
|
||||
)
|
||||
|
||||
client = create_simple_client(
|
||||
agent_type="merge_resolver",
|
||||
model="claude-haiku-4-5-20251001",
|
||||
system_prompt=AI_MERGE_SYSTEM_PROMPT,
|
||||
max_thinking_tokens=1024, # Low thinking for speed
|
||||
# Call Claude Haiku for fast merge first, then fallback to Sonnet if it fails
|
||||
# This two-tier approach matches the chat agent's success rate
|
||||
# - Tier 1: Haiku (fast, handles simple merges)
|
||||
# - Tier 2: Sonnet (more capable, handles complex merges)
|
||||
debug(MODULE, f"Attempting AI merge for {task.file_path} with Haiku (fast)")
|
||||
success, merged_content, error = await _attempt_ai_merge(
|
||||
task,
|
||||
prompt,
|
||||
model=MERGE_FAST_MODEL,
|
||||
max_thinking_tokens=MERGE_FAST_THINKING,
|
||||
)
|
||||
|
||||
response_text = ""
|
||||
async with client:
|
||||
await client.query(prompt)
|
||||
|
||||
async for msg in client.receive_response():
|
||||
msg_type = type(msg).__name__
|
||||
if msg_type == "AssistantMessage" and hasattr(msg, "content"):
|
||||
for block in msg.content:
|
||||
# Must check block type - only TextBlock has .text attribute
|
||||
block_type = type(block).__name__
|
||||
if block_type == "TextBlock" and hasattr(block, "text"):
|
||||
response_text += block.text
|
||||
|
||||
if response_text:
|
||||
# Strip any code fences the model might have added
|
||||
merged_content = _strip_code_fences(response_text.strip())
|
||||
|
||||
# VALIDATION: Check if AI returned natural language instead of code
|
||||
# This catches cases where AI says "I need to see more..." instead of merging
|
||||
natural_language_patterns = [
|
||||
"I need to",
|
||||
"Let me",
|
||||
"I cannot",
|
||||
"I'm unable",
|
||||
"The file appears",
|
||||
"I don't have",
|
||||
"Unfortunately",
|
||||
"I apologize",
|
||||
]
|
||||
first_line = merged_content.split("\n")[0] if merged_content else ""
|
||||
if any(pattern in first_line for pattern in natural_language_patterns):
|
||||
debug_warning(
|
||||
MODULE,
|
||||
f"AI returned natural language instead of code for {task.file_path}: {first_line[:100]}",
|
||||
)
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=None,
|
||||
success=False,
|
||||
error=f"AI returned explanation instead of code: {first_line[:80]}...",
|
||||
)
|
||||
|
||||
# VALIDATION: Run syntax check on the merged content
|
||||
is_valid, syntax_error = _validate_merged_syntax(
|
||||
task.file_path, merged_content, task.project_dir
|
||||
if success and merged_content:
|
||||
debug(MODULE, f"Haiku merged {task.file_path} successfully")
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=merged_content,
|
||||
success=True,
|
||||
was_auto_merged=False,
|
||||
)
|
||||
if not is_valid:
|
||||
debug_warning(
|
||||
MODULE,
|
||||
f"AI merge produced invalid syntax for {task.file_path}: {syntax_error}",
|
||||
)
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=None,
|
||||
success=False,
|
||||
error=f"AI merge produced invalid syntax: {syntax_error}",
|
||||
)
|
||||
|
||||
debug(MODULE, f"AI merged {task.file_path} successfully")
|
||||
# Haiku failed, retry with Sonnet (more capable model)
|
||||
debug_warning(
|
||||
MODULE,
|
||||
f"Haiku merge failed for {task.file_path}: {error}, retrying with Sonnet...",
|
||||
)
|
||||
print(muted(f" Retrying {task.file_path} with more capable AI model..."))
|
||||
success, merged_content, error = await _attempt_ai_merge(
|
||||
task,
|
||||
prompt,
|
||||
model=MERGE_CAPABLE_MODEL,
|
||||
max_thinking_tokens=MERGE_COMPLEX_THINKING,
|
||||
)
|
||||
|
||||
if success and merged_content:
|
||||
debug(MODULE, f"Sonnet merged {task.file_path} successfully")
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=merged_content,
|
||||
@@ -1709,11 +1806,16 @@ async def _merge_file_with_ai_async(
|
||||
was_auto_merged=False,
|
||||
)
|
||||
else:
|
||||
# Both models failed
|
||||
debug_error(
|
||||
MODULE,
|
||||
f"Both AI models failed to merge {task.file_path}: {error}",
|
||||
)
|
||||
return ParallelMergeResult(
|
||||
file_path=task.file_path,
|
||||
merged_content=None,
|
||||
success=False,
|
||||
error="AI returned empty response",
|
||||
error=f"AI merge failed: {error}",
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
|
||||
@@ -28,6 +28,8 @@ _workspace_module = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(_workspace_module)
|
||||
merge_existing_build = _workspace_module.merge_existing_build
|
||||
_run_parallel_merges = _workspace_module._run_parallel_merges
|
||||
AI_MERGE_SYSTEM_PROMPT = _workspace_module.AI_MERGE_SYSTEM_PROMPT
|
||||
_build_merge_prompt = _workspace_module._build_merge_prompt
|
||||
|
||||
# Models and Enums
|
||||
# Display Functions
|
||||
@@ -107,6 +109,8 @@ __all__ = [
|
||||
# Merge Operations (from workspace.py)
|
||||
"merge_existing_build",
|
||||
"_run_parallel_merges", # Private but used internally
|
||||
"AI_MERGE_SYSTEM_PROMPT", # System prompt for AI merge (ACS-194)
|
||||
"_build_merge_prompt", # Internal prompt builder (ACS-194)
|
||||
# Models
|
||||
"WorkspaceMode",
|
||||
"WorkspaceChoice",
|
||||
|
||||
@@ -149,7 +149,14 @@ def print_merge_success(
|
||||
|
||||
|
||||
def print_conflict_info(result: dict) -> None:
|
||||
"""Print information about conflicts that occurred during merge."""
|
||||
"""Print information about conflicts that occurred during merge.
|
||||
|
||||
The conflicts can be either:
|
||||
- List of strings (file paths) - for git conflict markers
|
||||
- List of dicts with keys: file, reason, severity - for AI merge failures
|
||||
"""
|
||||
import shlex
|
||||
|
||||
from ui import highlight, muted, warning
|
||||
|
||||
conflicts = result.get("conflicts", [])
|
||||
@@ -162,12 +169,57 @@ def print_conflict_info(result: dict) -> None:
|
||||
f" {len(conflicts)} file{'s' if len(conflicts) != 1 else ''} had conflicts:"
|
||||
)
|
||||
)
|
||||
for conflict_file in conflicts:
|
||||
print(f" {highlight(conflict_file)}")
|
||||
|
||||
# Extract file paths from conflicts (handle both strings and dicts)
|
||||
file_paths: list[str] = []
|
||||
has_marker_conflicts = False
|
||||
has_ai_conflicts = False
|
||||
for conflict in conflicts:
|
||||
if isinstance(conflict, str):
|
||||
# Simple string - just the file path
|
||||
file_paths.append(conflict)
|
||||
print(f" {highlight(conflict)}")
|
||||
has_marker_conflicts = True
|
||||
elif isinstance(conflict, dict):
|
||||
# Dict with file, reason, severity keys
|
||||
file_path = conflict.get("file", "unknown")
|
||||
reason = conflict.get("reason", "")
|
||||
severity = conflict.get("severity", "medium")
|
||||
|
||||
# Add severity indicator
|
||||
severity_icon = ""
|
||||
if severity == "critical":
|
||||
severity_icon = "⛔"
|
||||
elif severity == "high":
|
||||
severity_icon = "🔴"
|
||||
elif severity == "medium":
|
||||
severity_icon = "🟡"
|
||||
|
||||
file_paths.append(file_path)
|
||||
# Only add space if icon is present (no trailing space when empty)
|
||||
icon_with_space = f" {severity_icon}" if severity_icon else ""
|
||||
print(f" {highlight(file_path)}{icon_with_space}")
|
||||
if reason:
|
||||
print(f" {muted(reason)}")
|
||||
has_ai_conflicts = True
|
||||
|
||||
print()
|
||||
print(muted(" These files have conflict markers (<<<<<<< ======= >>>>>>>)"))
|
||||
print(muted(" Review and resolve them, then run:"))
|
||||
print(f" git add {' '.join(conflicts)}")
|
||||
if has_marker_conflicts:
|
||||
print(
|
||||
muted(
|
||||
" Some files may contain conflict markers (<<<<<<< ======= >>>>>>>)."
|
||||
)
|
||||
)
|
||||
if has_ai_conflicts:
|
||||
print(
|
||||
muted(
|
||||
" Some files could not be auto-merged; review and resolve as needed."
|
||||
)
|
||||
)
|
||||
print(muted(" Then run:"))
|
||||
# Quote paths and dedupe while preserving order
|
||||
quoted = " ".join(shlex.quote(p) for p in dict.fromkeys(file_paths))
|
||||
print(f" git add {quoted}")
|
||||
print(" git commit")
|
||||
print()
|
||||
|
||||
|
||||
@@ -614,11 +614,27 @@ class WorktreeManager:
|
||||
else:
|
||||
print(f"Merging {info.branch} into {self.base_branch}...")
|
||||
|
||||
# Switch to base branch in main project
|
||||
result = self._run_git(["checkout", self.base_branch])
|
||||
if result.returncode != 0:
|
||||
print(f"Error: Could not checkout base branch: {result.stderr}")
|
||||
return False
|
||||
# Switch to base branch in main project, but skip if already on it
|
||||
# This avoids triggering git hooks unnecessarily
|
||||
current_branch = self._get_current_branch()
|
||||
if current_branch != self.base_branch:
|
||||
result = self._run_git(["checkout", self.base_branch])
|
||||
if result.returncode != 0:
|
||||
# Check if this is a hook failure vs actual checkout failure
|
||||
# Hook failures still change the branch but return non-zero
|
||||
new_branch = self._get_current_branch()
|
||||
if new_branch == self.base_branch:
|
||||
# Branch did change - likely a hook failure, continue with merge
|
||||
stderr_msg = result.stderr[:100] if result.stderr else "<no stderr>"
|
||||
debug_warning(
|
||||
"worktree",
|
||||
f"Checkout succeeded but hook returned non-zero: {stderr_msg}",
|
||||
)
|
||||
else:
|
||||
# Actual checkout failure
|
||||
stderr_msg = result.stderr[:100] if result.stderr else "<no stderr>"
|
||||
print(f"Error: Could not checkout base branch: {stderr_msg}")
|
||||
return False
|
||||
|
||||
# Merge the spec branch
|
||||
merge_args = ["merge", "--no-ff", info.branch]
|
||||
|
||||
@@ -207,3 +207,43 @@ class TestStatsTracking:
|
||||
|
||||
stats = mock_ai_resolver.stats
|
||||
assert stats["calls_made"] == 3
|
||||
|
||||
|
||||
class TestAIMergeRetryMechanism:
|
||||
"""Tests for AI merge retry mechanism with fallback (ACS-194)."""
|
||||
|
||||
def test_ai_merge_system_prompt_enhanced(self):
|
||||
"""AI merge system prompt is enhanced for better success rate (ACS-194)."""
|
||||
# Import from workspace package (standard import)
|
||||
from core.workspace import AI_MERGE_SYSTEM_PROMPT
|
||||
|
||||
# Verify the system prompt includes enhanced guidance
|
||||
assert "expert code merge assistant" in AI_MERGE_SYSTEM_PROMPT
|
||||
assert "3-way merges" in AI_MERGE_SYSTEM_PROMPT
|
||||
# Note: The prompt focuses on "intelligently" and "task's intent" not "semantic understanding"
|
||||
assert "intelligently" in AI_MERGE_SYSTEM_PROMPT.lower()
|
||||
assert "task's intent" in AI_MERGE_SYSTEM_PROMPT or "task intent" in AI_MERGE_SYSTEM_PROMPT
|
||||
assert "best-effort" in AI_MERGE_SYSTEM_PROMPT
|
||||
# Verify key merge strategies are documented
|
||||
assert "Preserve all functional changes" in AI_MERGE_SYSTEM_PROMPT
|
||||
assert "Combine independent changes" in AI_MERGE_SYSTEM_PROMPT
|
||||
assert "Resolve overlapping changes" in AI_MERGE_SYSTEM_PROMPT
|
||||
|
||||
def test_build_merge_prompt_includes_task_context(self):
|
||||
"""Merge prompt builder includes task context (ACS-194)."""
|
||||
# Import from workspace package (standard import)
|
||||
from core.workspace import _build_merge_prompt
|
||||
|
||||
# Test that prompt includes task name
|
||||
prompt = _build_merge_prompt(
|
||||
"test.py",
|
||||
"base content",
|
||||
"main content",
|
||||
"worktree content",
|
||||
"my-task-spec",
|
||||
)
|
||||
|
||||
assert "my-task-spec" in prompt
|
||||
assert "OURS" in prompt
|
||||
assert "THEIRS" in prompt
|
||||
assert "BASE" in prompt or "common ancestor" in prompt
|
||||
|
||||
+136
-1
@@ -13,7 +13,6 @@ Tests the workspace.py module functionality including:
|
||||
import subprocess
|
||||
import pytest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
from workspace import (
|
||||
WorkspaceMode,
|
||||
@@ -394,3 +393,139 @@ class TestPerSpecWorktreeName:
|
||||
# New path: .auto-claude/worktrees/tasks/{spec_name}
|
||||
assert "worktrees" in str(working_dir)
|
||||
assert working_dir.parent.name == "tasks"
|
||||
|
||||
|
||||
class TestConflictInfoDisplay:
|
||||
"""Tests for conflict info display function (ACS-179)."""
|
||||
|
||||
def test_print_conflict_info_with_string_list(self, capsys):
|
||||
"""print_conflict_info handles string list of file paths (ACS-179)."""
|
||||
from core.workspace.display import print_conflict_info
|
||||
|
||||
result = {
|
||||
"conflicts": ["file1.txt", "file2.py", "file3.js"]
|
||||
}
|
||||
|
||||
print_conflict_info(result)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert "3 file" in captured.out
|
||||
assert "file1.txt" in captured.out
|
||||
assert "file2.py" in captured.out
|
||||
assert "file3.js" in captured.out
|
||||
assert "git add" in captured.out
|
||||
|
||||
def test_print_conflict_info_with_dict_list(self, capsys):
|
||||
"""print_conflict_info handles dict list with file/reason/severity (ACS-179)."""
|
||||
from core.workspace.display import print_conflict_info
|
||||
|
||||
result = {
|
||||
"conflicts": [
|
||||
{"file": "file1.txt", "reason": "Syntax error", "severity": "high"},
|
||||
{"file": "file2.py", "reason": "Merge conflict", "severity": "medium"},
|
||||
{"file": "file3.js", "reason": "Unknown error", "severity": "low"},
|
||||
]
|
||||
}
|
||||
|
||||
print_conflict_info(result)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert "3 file" in captured.out
|
||||
assert "file1.txt" in captured.out
|
||||
assert "file2.py" in captured.out
|
||||
assert "file3.js" in captured.out
|
||||
assert "Syntax error" in captured.out
|
||||
assert "Merge conflict" in captured.out
|
||||
# Verify severity emoji indicators
|
||||
assert "🔴" in captured.out # High severity
|
||||
assert "🟡" in captured.out # Medium severity
|
||||
|
||||
def test_print_conflict_info_mixed_formats(self, capsys):
|
||||
"""print_conflict_info handles mixed string and dict conflicts (ACS-179)."""
|
||||
from core.workspace.display import print_conflict_info
|
||||
|
||||
result = {
|
||||
"conflicts": [
|
||||
"simple-file.txt",
|
||||
{"file": "complex-file.py", "reason": "AI merge failed", "severity": "high"},
|
||||
]
|
||||
}
|
||||
|
||||
print_conflict_info(result)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert "2 file" in captured.out
|
||||
assert "simple-file.txt" in captured.out
|
||||
assert "complex-file.py" in captured.out
|
||||
assert "AI merge failed" in captured.out
|
||||
|
||||
|
||||
class TestMergeErrorHandling:
|
||||
"""Tests for merge error handling (ACS-163)."""
|
||||
|
||||
def test_merge_failure_returns_false_immediately(self, temp_git_repo: Path):
|
||||
"""Failed merge returns False without falling through (ACS-163)."""
|
||||
manager = WorktreeManager(temp_git_repo)
|
||||
manager.setup()
|
||||
|
||||
# Create a worktree with changes
|
||||
worker_info = manager.create_worktree("worker-spec")
|
||||
(worker_info.path / "worker-file.txt").write_text("worker content")
|
||||
subprocess.run(["git", "add", "."], cwd=worker_info.path, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "Worker commit"],
|
||||
cwd=worker_info.path, capture_output=True
|
||||
)
|
||||
|
||||
# Create a conflicting change on main
|
||||
subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||
(temp_git_repo / "worker-file.txt").write_text("main content")
|
||||
subprocess.run(["git", "add", "."], cwd=temp_git_repo, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "Main commit"],
|
||||
cwd=temp_git_repo, capture_output=True
|
||||
)
|
||||
|
||||
# Merge should fail (conflict) and return False
|
||||
# This tests the fix for ACS-163 where failed merge would fall through
|
||||
result = manager.merge_worktree("worker-spec", delete_after=False)
|
||||
|
||||
# Should return False on merge conflict
|
||||
assert result is False
|
||||
|
||||
# Verify side effects: base branch content is unchanged
|
||||
subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||
base_content = (temp_git_repo / "worker-file.txt").read_text()
|
||||
assert base_content == "main content", "Base branch should be unchanged after failed merge"
|
||||
|
||||
# Verify worktree still exists (delete_after=False)
|
||||
assert worker_info.path.exists(), "Worktree should still exist after failed merge"
|
||||
|
||||
# Verify worktree content is unchanged
|
||||
worktree_content = (worker_info.path / "worker-file.txt").read_text()
|
||||
assert worktree_content == "worker content", "Worktree content should be unchanged"
|
||||
|
||||
def test_merge_success_returns_true(self, temp_git_repo: Path):
|
||||
"""Successful merge returns True (ACS-163 verification)."""
|
||||
manager = WorktreeManager(temp_git_repo)
|
||||
manager.setup()
|
||||
|
||||
# Create a worktree with non-conflicting changes
|
||||
worker_info = manager.create_worktree("worker-spec")
|
||||
(worker_info.path / "worker-file.txt").write_text("worker content")
|
||||
subprocess.run(["git", "add", "."], cwd=worker_info.path, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "Worker commit"],
|
||||
cwd=worker_info.path, capture_output=True
|
||||
)
|
||||
|
||||
# Merge should succeed
|
||||
result = manager.merge_worktree("worker-spec", delete_after=False)
|
||||
|
||||
assert result is True
|
||||
|
||||
# Verify the file was merged into base branch
|
||||
subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||
assert (temp_git_repo / "worker-file.txt").exists(), "Merged file should exist in base branch"
|
||||
merged_content = (temp_git_repo / "worker-file.txt").read_text()
|
||||
assert merged_content == "worker content", "Merged file should have worktree content"
|
||||
|
||||
@@ -171,6 +171,34 @@ class TestWorktreeCommitAndMerge:
|
||||
subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||
assert (temp_git_repo / "worker-file.txt").exists()
|
||||
|
||||
def test_merge_worktree_already_on_target_branch(self, temp_git_repo: Path):
|
||||
"""merge_worktree succeeds when already on target branch (ACS-174)."""
|
||||
manager = WorktreeManager(temp_git_repo)
|
||||
manager.setup()
|
||||
|
||||
# Ensure we're on the base branch
|
||||
result = subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||
assert result.returncode == 0, f"Checkout failed: {result.stderr}"
|
||||
|
||||
# Create a worktree with changes
|
||||
worker_info = manager.create_worktree("worker-spec")
|
||||
(worker_info.path / "worker-file.txt").write_text("worker content")
|
||||
result = subprocess.run(["git", "add", "."], cwd=worker_info.path, capture_output=True)
|
||||
assert result.returncode == 0, f"Git add failed: {result.stderr}"
|
||||
result = subprocess.run(
|
||||
["git", "commit", "-m", "Worker commit"],
|
||||
cwd=worker_info.path, capture_output=True
|
||||
)
|
||||
assert result.returncode == 0, f"Commit failed: {result.stderr}"
|
||||
|
||||
# Already on target branch, should skip checkout and still merge successfully
|
||||
result = manager.merge_worktree("worker-spec", delete_after=False)
|
||||
|
||||
assert result is True
|
||||
|
||||
# Verify file is in main branch
|
||||
assert (temp_git_repo / "worker-file.txt").exists()
|
||||
|
||||
|
||||
class TestChangeTracking:
|
||||
"""Tests for tracking changes in worktrees."""
|
||||
|
||||
Reference in New Issue
Block a user