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
|
no_commit, stats, spec_name=spec_name, keep_worktree=True
|
||||||
)
|
)
|
||||||
return 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"):
|
elif smart_result.get("git_conflicts"):
|
||||||
# Had git conflicts that AI couldn't fully resolve
|
# Had git conflicts that AI couldn't fully resolve
|
||||||
resolved = smart_result.get("resolved", [])
|
resolved = smart_result.get("resolved", [])
|
||||||
@@ -1425,18 +1432,41 @@ import os
|
|||||||
_merge_logger = logging.getLogger(__name__)
|
_merge_logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
# System prompt for AI file merging
|
# 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:
|
CONTEXT:
|
||||||
1. Preserve all functional changes from both versions (ours and theirs)
|
- "OURS" = current main branch (target for merge)
|
||||||
2. Maintain code style consistency
|
- "THEIRS" = task worktree branch (changes being merged in)
|
||||||
3. Resolve conflicts by understanding the semantic purpose of each change
|
- "BASE" = common ancestor before changes
|
||||||
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
|
|
||||||
|
|
||||||
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:
|
def _infer_language_from_path(file_path: str) -> str:
|
||||||
@@ -1525,7 +1555,7 @@ def _build_merge_prompt(
|
|||||||
if len(base_content) > 10000:
|
if len(base_content) > 10000:
|
||||||
base_content = base_content[:10000] + "\n... (truncated)"
|
base_content = base_content[:10000] + "\n... (truncated)"
|
||||||
base_section = f"""
|
base_section = f"""
|
||||||
BASE (common ancestor):
|
BASE (common ancestor before changes):
|
||||||
```{language}
|
```{language}
|
||||||
{base_content}
|
{base_content}
|
||||||
```
|
```
|
||||||
@@ -1537,20 +1567,22 @@ BASE (common ancestor):
|
|||||||
if len(worktree_content) > 15000:
|
if len(worktree_content) > 15000:
|
||||||
worktree_content = worktree_content[:15000] + "\n... (truncated)"
|
worktree_content = worktree_content[:15000] + "\n... (truncated)"
|
||||||
|
|
||||||
prompt = f"""Perform a 3-way merge for file: {file_path}
|
prompt = f"""FILE: {file_path}
|
||||||
Task being merged: {spec_name}
|
TASK: {spec_name}
|
||||||
|
|
||||||
|
This is a 3-way code merge. You must combine changes from both versions.
|
||||||
{base_section}
|
{base_section}
|
||||||
OURS (current main branch):
|
OURS (current main branch - target for merge):
|
||||||
```{language}
|
```{language}
|
||||||
{main_content}
|
{main_content}
|
||||||
```
|
```
|
||||||
|
|
||||||
THEIRS (changes from task worktree):
|
THEIRS (task worktree branch - changes being merged):
|
||||||
```{language}
|
```{language}
|
||||||
{worktree_content}
|
{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
|
return prompt
|
||||||
|
|
||||||
@@ -1568,6 +1600,112 @@ def _strip_code_fences(content: str) -> str:
|
|||||||
return content
|
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(
|
async def _merge_file_with_ai_async(
|
||||||
task: ParallelMergeTask,
|
task: ParallelMergeTask,
|
||||||
semaphore: asyncio.Semaphore,
|
semaphore: asyncio.Semaphore,
|
||||||
@@ -1625,83 +1763,42 @@ async def _merge_file_with_ai_async(
|
|||||||
task.spec_name,
|
task.spec_name,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Call Claude Haiku for fast merge
|
# Call Claude Haiku for fast merge first, then fallback to Sonnet if it fails
|
||||||
try:
|
# This two-tier approach matches the chat agent's success rate
|
||||||
from core.simple_client import create_simple_client
|
# - Tier 1: Haiku (fast, handles simple merges)
|
||||||
except ImportError:
|
# - Tier 2: Sonnet (more capable, handles complex merges)
|
||||||
return ParallelMergeResult(
|
debug(MODULE, f"Attempting AI merge for {task.file_path} with Haiku (fast)")
|
||||||
file_path=task.file_path,
|
success, merged_content, error = await _attempt_ai_merge(
|
||||||
merged_content=None,
|
task,
|
||||||
success=False,
|
prompt,
|
||||||
error="core.simple_client not available",
|
model=MERGE_FAST_MODEL,
|
||||||
)
|
max_thinking_tokens=MERGE_FAST_THINKING,
|
||||||
|
|
||||||
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
|
|
||||||
)
|
)
|
||||||
|
|
||||||
response_text = ""
|
if success and merged_content:
|
||||||
async with client:
|
debug(MODULE, f"Haiku merged {task.file_path} successfully")
|
||||||
await client.query(prompt)
|
return ParallelMergeResult(
|
||||||
|
file_path=task.file_path,
|
||||||
async for msg in client.receive_response():
|
merged_content=merged_content,
|
||||||
msg_type = type(msg).__name__
|
success=True,
|
||||||
if msg_type == "AssistantMessage" and hasattr(msg, "content"):
|
was_auto_merged=False,
|
||||||
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 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(
|
return ParallelMergeResult(
|
||||||
file_path=task.file_path,
|
file_path=task.file_path,
|
||||||
merged_content=merged_content,
|
merged_content=merged_content,
|
||||||
@@ -1709,11 +1806,16 @@ async def _merge_file_with_ai_async(
|
|||||||
was_auto_merged=False,
|
was_auto_merged=False,
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
|
# Both models failed
|
||||||
|
debug_error(
|
||||||
|
MODULE,
|
||||||
|
f"Both AI models failed to merge {task.file_path}: {error}",
|
||||||
|
)
|
||||||
return ParallelMergeResult(
|
return ParallelMergeResult(
|
||||||
file_path=task.file_path,
|
file_path=task.file_path,
|
||||||
merged_content=None,
|
merged_content=None,
|
||||||
success=False,
|
success=False,
|
||||||
error="AI returned empty response",
|
error=f"AI merge failed: {error}",
|
||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -28,6 +28,8 @@ _workspace_module = importlib.util.module_from_spec(_spec)
|
|||||||
_spec.loader.exec_module(_workspace_module)
|
_spec.loader.exec_module(_workspace_module)
|
||||||
merge_existing_build = _workspace_module.merge_existing_build
|
merge_existing_build = _workspace_module.merge_existing_build
|
||||||
_run_parallel_merges = _workspace_module._run_parallel_merges
|
_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
|
# Models and Enums
|
||||||
# Display Functions
|
# Display Functions
|
||||||
@@ -107,6 +109,8 @@ __all__ = [
|
|||||||
# Merge Operations (from workspace.py)
|
# Merge Operations (from workspace.py)
|
||||||
"merge_existing_build",
|
"merge_existing_build",
|
||||||
"_run_parallel_merges", # Private but used internally
|
"_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
|
# Models
|
||||||
"WorkspaceMode",
|
"WorkspaceMode",
|
||||||
"WorkspaceChoice",
|
"WorkspaceChoice",
|
||||||
|
|||||||
@@ -149,7 +149,14 @@ def print_merge_success(
|
|||||||
|
|
||||||
|
|
||||||
def print_conflict_info(result: dict) -> None:
|
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
|
from ui import highlight, muted, warning
|
||||||
|
|
||||||
conflicts = result.get("conflicts", [])
|
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:"
|
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()
|
||||||
print(muted(" These files have conflict markers (<<<<<<< ======= >>>>>>>)"))
|
if has_marker_conflicts:
|
||||||
print(muted(" Review and resolve them, then run:"))
|
print(
|
||||||
print(f" git add {' '.join(conflicts)}")
|
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(" git commit")
|
||||||
print()
|
print()
|
||||||
|
|
||||||
|
|||||||
@@ -614,11 +614,27 @@ class WorktreeManager:
|
|||||||
else:
|
else:
|
||||||
print(f"Merging {info.branch} into {self.base_branch}...")
|
print(f"Merging {info.branch} into {self.base_branch}...")
|
||||||
|
|
||||||
# Switch to base branch in main project
|
# Switch to base branch in main project, but skip if already on it
|
||||||
result = self._run_git(["checkout", self.base_branch])
|
# This avoids triggering git hooks unnecessarily
|
||||||
if result.returncode != 0:
|
current_branch = self._get_current_branch()
|
||||||
print(f"Error: Could not checkout base branch: {result.stderr}")
|
if current_branch != self.base_branch:
|
||||||
return False
|
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 the spec branch
|
||||||
merge_args = ["merge", "--no-ff", info.branch]
|
merge_args = ["merge", "--no-ff", info.branch]
|
||||||
|
|||||||
@@ -207,3 +207,43 @@ class TestStatsTracking:
|
|||||||
|
|
||||||
stats = mock_ai_resolver.stats
|
stats = mock_ai_resolver.stats
|
||||||
assert stats["calls_made"] == 3
|
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 subprocess
|
||||||
import pytest
|
import pytest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch, MagicMock
|
|
||||||
|
|
||||||
from workspace import (
|
from workspace import (
|
||||||
WorkspaceMode,
|
WorkspaceMode,
|
||||||
@@ -394,3 +393,139 @@ class TestPerSpecWorktreeName:
|
|||||||
# New path: .auto-claude/worktrees/tasks/{spec_name}
|
# New path: .auto-claude/worktrees/tasks/{spec_name}
|
||||||
assert "worktrees" in str(working_dir)
|
assert "worktrees" in str(working_dir)
|
||||||
assert working_dir.parent.name == "tasks"
|
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)
|
subprocess.run(["git", "checkout", manager.base_branch], cwd=temp_git_repo, capture_output=True)
|
||||||
assert (temp_git_repo / "worker-file.txt").exists()
|
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:
|
class TestChangeTracking:
|
||||||
"""Tests for tracking changes in worktrees."""
|
"""Tests for tracking changes in worktrees."""
|
||||||
|
|||||||
Reference in New Issue
Block a user