electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(merge): use git merge for diverged branches with progress tracking (#1605)

Browse Source 
* fix(merge): use git merge instead of file copy for diverged branches

Replace direct file copy with proper git merge for worktree branches that
have diverged from develop but have no actual conflicts. This preserves
changes from both branches instead of overwriting develop-side changes.

Key changes:
- Use `git merge --no-commit --no-ff` when branches diverged but no conflicts
- Add real-time merge progress tracking with UI overlay
- Emit structured JSON progress events from Python to Electron via stdout
- Add stall detection (30s) and progress visualization in frontend

The previous approach used direct file copy as a fallback when rebase failed
due to worktree lock, which would overwrite any develop-side changes. Now we
properly detect "diverged but no conflicts" scenarios and let git handle the
merge correctly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(merge): add conflict scenario detection for better UX messaging

Add intelligent detection of merge conflict scenarios to provide clearer
guidance when staging task changes:

- already_merged: Task changes identical to target branch - show "Mark as Done"
- superseded: Target has newer version - show "View Comparison" / "Discard"
- diverged: Both branches modified - standard AI merge flow

Backend: Add _detect_conflict_scenario() that compares file contents between
spec branch, base branch, and merge-base to classify the scenario.

Frontend: Add scenario-specific banners and action buttons that guide users
to the appropriate action instead of showing confusing "Branch Diverged" errors.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-1 - Add BILLING_FAILURE_PATTERNS regex array with patterns for credit/billing errors

Added comprehensive regex patterns to detect billing and credit failures:
- Credit balance patterns (insufficient, low, empty, exhausted)
- Billing error patterns (payment failed, subscription expired)
- Usage quota patterns (monthly limits, plan limits)
- API error patterns (billing_error, insufficient_credits, 402)
- Balance/funds patterns and add credits messages

* auto-claude: subtask-1-2 - Add BillingFailureDetectionResult interface parallel to AuthFailureDetectionResult

Add new TypeScript interface for billing failure detection that mirrors
the AuthFailureDetectionResult pattern with:
- isBillingFailure: boolean flag
- profileId: optional profile identifier
- failureType: specific billing failure types (insufficient_credits,
  payment_required, subscription_inactive, unknown)
- message: user-friendly error message
- originalError: raw error from process output

* auto-claude: subtask-1-3 - Add classifyBillingFailureType() and getBillingFailureMessage() helpers

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-4 - Add detectBillingFailure() function to detect billing errors in output

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Add BillingFailureInfo interface to terminal.ts

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-1 - Add onBillingFailure callback to SubprocessOptions

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-2 - Add checkBillingFailure helper function in runPythonSubprocess

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-3-3 - Integrate checkBillingFailure into stdout/stderr handlers and close handler

- Added checkBillingFailure(line) call after checkAuthFailure(line) in stdout handler
- Added checkBillingFailure(line) call after checkAuthFailure(line) in stderr handler
- Added killedDueToBillingFailure check in close handler with proper error message
- Follows the exact same pattern as auth failure detection integration

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: billing detection patterns and add unit tests (qa-requested)

Fixes:
- Fix regex for "credit balance is too low" by adding optional (too\s+)? group
- Add extra_usage pattern to BILLING_FAILURE_PATTERNS for Claude API errors
- Fix 402 false positive by requiring HTTP/status/code/error context prefix
- Add 31 unit tests for detectBillingFailure, isBillingFailureError, and
  classifyBillingFailureType covering spec appendix messages, negative cases,
  false positive checks, and cross-detection tests

Verified:
- All 84 rate-limit-detector tests pass (53 existing + 31 new)
- TypeScript compilation succeeds with zero errors
- Full test suite passes (2599/2599 + 6 skipped)

QA Fix Session: 1

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: PR review feedback - async worktree listing and merge abort check

Issue 1 (HIGH): Convert synchronous git operations to async in
TASK_LIST_WORKTREES handler to prevent UI freezing:
- Use execFileAsync instead of execFileSync for git commands
- Use fsPromises.readdir/stat instead of readdirSync/statSync
- Process worktrees in parallel with Promise.all()

Issue 2 (MEDIUM): Add error check for git merge --abort:
- Check abort_result.returncode after merge --abort
- Log error and return None on failure to avoid inconsistent state
- Matches existing pattern from rebase --abort at line 870

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Revert popover removal and remove only the Claude.ai/code link

Partially reverts 8d18cc81a which removed the entire ClaudeCodeStatusBadge
popover. The intent was only to remove the "Learn more about Claude Code"
link that pointed to claude.ai/code.

Changes:
- Restore full popover functionality (version selector, installation
  selector, update/rollback dialogs)
- Remove only the "Learn more about Claude Code" button that linked to
  https://claude.ai/code
- Keep the Changelog link to GitHub

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: remove temporary debug file logging from PR review agents

Remove the _PRDebugLogger class and all file-based debug logging that was
writing to .auto-claude/github/pr/debug_logs/. This was temporary instrumentation
for measuring agent communication patterns.

Also adds circuit breaker protection (MAX_MESSAGE_COUNT=500) to prevent
runaway retry loops, and retry logic for the FindingValidator agent.

Changes:
- Remove _PRDebugLogger class (~220 lines)
- Remove all _dbg.* calls from process_sdk_stream
- Keep system_prompt/agent_definitions params for backwards compat (unused)
- Add circuit breaker to abort processing if msg_count > limit
- Add retry logic with MAX_VALIDATION_RETRIES=2 for FindingValidator

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(pr-review): remove programmatic file scanning, fix SDK tool concurrency

Architecture Changes:
- Remove legacy programmatic file scanning from PRContextGatherer
- LLM agents now discover relevant files via their tools (Glob/Grep/Read)
- This removes the 2000 file scan limit and lets agents use judgment

SDK Tool Concurrency Fix:
- Add retry logic with MAX_RETRIES=3 for tool use 400 errors
- Add _is_tool_concurrency_error() detection in sdk_utils.py
- Add prompt guidance for sequential tool execution
- Upgrade claude-agent-sdk to >=0.1.25

Bug Fixes:
- Add in-progress review tracking to BotDetector (30min timeout)
- Fix missing dict keys in workspace_commands.py error path
- Fix stuck loading state in ClaudeCodeStatusBadge.tsx

i18n:
- Replace 11 hardcoded strings in WorkspaceStatus.tsx with translation keys
- Add 13 new translation keys to en/fr taskReview.json

Tests:
- Update TestReverseDepDetection to reflect new LLM-driven architecture

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-01-30 11:37:32 +01:00
committed by GitHub
parent 7bf12e8566
commit bde2ca4b2f
30 changed files with 2937 additions and 1176 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/cli/workspace_commands.py
+197
View File
@@ -536,6 +536,175 @@ def handle_cleanup_worktrees_command(project_dir: Path) -> None:
cleanup_all_worktrees(project_dir, confirm=True)
def _detect_conflict_scenario(
project_dir: Path,
conflicting_files: list[str],
spec_branch: str,
base_branch: str,
) -> dict:
"""
Analyze conflicting files to determine the conflict scenario.
This helps distinguish between:
- 'already_merged': Task changes already identical in target branch
- 'superseded': Target has newer version of same feature
- 'diverged': Standard diverged branches (AI can resolve)
- 'normal_conflict': Actual conflicting changes
Returns dict with:
- scenario: 'already_merged' | 'superseded' | 'diverged' | 'normal_conflict'
- already_merged_files: files identical in task and target
- details: additional context
"""
if not conflicting_files:
return {
"scenario": "normal_conflict",
"already_merged_files": [],
"details": "No conflicting files to analyze",
}
already_merged_files = []
superseded_files = []
diverged_files = []
try:
# Get the merge-base commit
merge_base_result = subprocess.run(
["git", "merge-base", base_branch, spec_branch],
cwd=project_dir,
capture_output=True,
text=True,
)
if merge_base_result.returncode != 0:
debug_warning(
MODULE, "Could not find merge base for conflict scenario detection"
)
return {
"scenario": "normal_conflict",
"already_merged_files": [],
"details": "Could not determine merge base",
}
merge_base = merge_base_result.stdout.strip()
for file_path in conflicting_files:
try:
# Get content from spec branch (task's changes)
spec_content_result = subprocess.run(
["git", "show", f"{spec_branch}:{file_path}"],
cwd=project_dir,
capture_output=True,
text=True,
)
# Get content from base branch (target)
base_content_result = subprocess.run(
["git", "show", f"{base_branch}:{file_path}"],
cwd=project_dir,
capture_output=True,
text=True,
)
# Get content from merge-base (original state)
merge_base_content_result = subprocess.run(
["git", "show", f"{merge_base}:{file_path}"],
cwd=project_dir,
capture_output=True,
text=True,
)
# Check file existence in each ref
spec_exists = spec_content_result.returncode == 0
base_exists = base_content_result.returncode == 0
merge_base_exists = merge_base_content_result.returncode == 0
if spec_exists and base_exists:
spec_content = spec_content_result.stdout
base_content = base_content_result.stdout
# If contents are identical, the changes are already merged
if spec_content == base_content:
already_merged_files.append(file_path)
debug(
MODULE,
f"File {file_path}: already merged (identical content)",
)
elif merge_base_exists:
merge_base_content = merge_base_content_result.stdout
# If base has changed from merge_base but spec matches merge_base,
# the task's changes are superseded by newer changes
if spec_content == merge_base_content:
superseded_files.append(file_path)
debug(
MODULE,
f"File {file_path}: superseded (base has newer changes)",
)
else:
diverged_files.append(file_path)
debug(
MODULE,
f"File {file_path}: diverged (both branches modified)",
)
else:
diverged_files.append(file_path)
else:
diverged_files.append(file_path)
except Exception as e:
debug_warning(
MODULE, f"Error analyzing file {file_path} for scenario: {e}"
)
diverged_files.append(file_path)
# Determine overall scenario based on dominant pattern
total_files = len(conflicting_files)
if len(already_merged_files) == total_files:
scenario = "already_merged"
details = "All conflicting files have identical content in both branches"
elif len(already_merged_files) > total_files / 2:
scenario = "already_merged"
details = f"{len(already_merged_files)} of {total_files} files already have the same content"
elif len(superseded_files) == total_files:
scenario = "superseded"
details = "All task changes have been superseded by newer changes in the target branch"
elif len(superseded_files) > total_files / 2:
scenario = "superseded"
details = (
f"{len(superseded_files)} of {total_files} files have been superseded"
)
elif diverged_files:
scenario = "diverged"
details = f"{len(diverged_files)} files have diverged and need AI merge"
else:
scenario = "normal_conflict"
details = "Standard merge conflicts detected"
debug(
MODULE,
f"Conflict scenario: {scenario}",
already_merged=len(already_merged_files),
superseded=len(superseded_files),
diverged=len(diverged_files),
)
return {
"scenario": scenario,
"already_merged_files": already_merged_files,
"superseded_files": superseded_files,
"diverged_files": diverged_files,
"details": details,
}
except Exception as e:
debug_error(MODULE, f"Error detecting conflict scenario: {e}")
return {
"scenario": "normal_conflict",
"already_merged_files": [],
"superseded_files": [],
"diverged_files": [],
"details": f"Error during analysis: {e}",
}
def _check_git_merge_conflicts(
project_dir: Path, spec_name: str, base_branch: str | None = None
) -> dict:
@@ -879,6 +1048,24 @@ def handle_merge_preview_command(
f for f in git_conflicts.get("conflicting_files", []) if not is_lock_file(f)
]
# Detect conflict scenario (already_merged, superseded, diverged, normal_conflict)
# This helps the UI show appropriate messaging and actions
conflict_scenario = None
if non_lock_conflicting_files:
conflict_scenario = _detect_conflict_scenario(
project_dir,
non_lock_conflicting_files,
git_conflicts["spec_branch"],
git_conflicts["base_branch"],
)
debug(
MODULE,
f"Conflict scenario detected: {conflict_scenario.get('scenario')}",
already_merged_files=len(
conflict_scenario.get("already_merged_files", [])
),
)
# Use git diff file count as the authoritative totalFiles count
# The semantic tracker may not track all files (e.g., test files, config files)
# but we want to show the user all files that will be merged
@@ -952,6 +1139,16 @@ def handle_merge_preview_command(
# Path-mapped files that need AI merge due to renames
"pathMappedAIMerges": path_mapped_ai_merges,
"totalRenames": len(path_mappings),
# Conflict scenario detection for better UX messaging
"scenario": conflict_scenario.get("scenario")
if conflict_scenario
else None,
"alreadyMergedFiles": conflict_scenario.get("already_merged_files", [])
if conflict_scenario
else [],
"scenarioMessage": conflict_scenario.get("details")
if conflict_scenario
else None,
},
"summary": {
# Use git diff count, not semantic tracker count
apps/backend/core/workspace.py
+192 -175
View File
@@ -121,6 +121,7 @@ from merge import (
FileTimelineTracker,
MergeOrchestrator,
)
from merge.progress import MergeProgressCallback, MergeProgressStage, emit_progress
MODULE = "workspace"
@@ -145,6 +146,26 @@ MODULE = "workspace"
# - _heuristic_merge
def _create_merge_progress_callback() -> MergeProgressCallback | None:
"""
Create a progress callback for merge operations when running as a subprocess.
Returns emit_progress (writing JSON to stdout) only when stdout is piped
(i.e., running as a subprocess from the Electron frontend). Returns None
when running interactively in a terminal to avoid polluting CLI output.
This function must be called at runtime (not at import time) to ensure
sys.stdout state is accurate.
"""
import sys
# Only emit progress JSON when stdout is piped (subprocess mode).
# In interactive CLI mode (TTY), progress JSON would clutter the output.
if not sys.stdout.isatty():
return emit_progress
return None
def merge_existing_build(
project_dir: Path,
spec_name: str,
@@ -252,10 +273,11 @@ def merge_existing_build(
had_conflicts = stats.get("conflicts_resolved", 0) > 0
ai_assisted = stats.get("ai_assisted", 0) > 0
direct_copy = stats.get("direct_copy", False)
git_merge_used = stats.get("git_merge", False)
if had_conflicts or ai_assisted or direct_copy:
# AI resolved conflicts, assisted with merges, or direct copy was used
# Changes are already written and staged - no need for git merge
if had_conflicts or ai_assisted or direct_copy or git_merge_used:
# AI resolved conflicts, assisted with merges, git merge was used, or direct copy was used
# Changes are already written and staged - no need for additional git merge
_print_merge_success(
no_commit, stats, spec_name=spec_name, keep_worktree=True
)
@@ -402,9 +424,20 @@ def _try_smart_merge_inner(
no_commit=no_commit,
)
# Create progress callback for subprocess mode (Electron frontend).
# Only emits JSON to stdout when piped, not in interactive CLI.
progress_callback = _create_merge_progress_callback()
try:
print(muted(" Analyzing changes with intent-aware merge..."))
if progress_callback is not None:
progress_callback(
MergeProgressStage.ANALYZING,
0,
"Starting merge analysis",
)
# Capture worktree state in FileTimelineTracker before merge
try:
timeline_tracker = FileTimelineTracker(project_dir)
@@ -440,6 +473,13 @@ def _try_smart_merge_inner(
)
# Check for git-level conflicts first (branch divergence)
if progress_callback is not None:
progress_callback(
MergeProgressStage.DETECTING_CONFLICTS,
25,
"Checking for git-level conflicts",
)
debug(MODULE, "Checking for git-level conflicts")
git_conflicts = _check_git_conflicts(project_dir, spec_name)
@@ -492,12 +532,11 @@ def _try_smart_merge_inner(
# If rebase succeeded and now there are no conflicts,
# the diverged_but_no_conflicts path will handle the merge
else:
# Rebase failed - continue with conflict resolution as before
# The AI resolver will handle the conflicts
print(
warning(
" Rebase encountered issues, using AI conflict resolution..."
)
# Rebase failed (likely due to worktree lock) - continue with merge
# Git merge or AI resolver will handle it depending on conflict state
debug(
MODULE,
"Rebase skipped or failed, continuing with merge flow",
)
if git_conflicts.get("has_conflicts"):
@@ -518,6 +557,18 @@ def _try_smart_merge_inner(
num_conflicts=len(git_conflicts.get("conflicting_files", [])),
)
if progress_callback is not None:
progress_callback(
MergeProgressStage.RESOLVING,
50,
f"Resolving {len(git_conflicts.get('conflicting_files', []))} conflicting files with AI",
{
"conflicts_found": len(
git_conflicts.get("conflicting_files", [])
)
},
)
# Try to resolve git conflicts with AI
resolution_result = _resolve_git_conflicts_with_ai(
project_dir,
@@ -535,6 +586,19 @@ def _try_smart_merge_inner(
resolved_files=resolution_result.get("resolved_files", []),
stats=resolution_result.get("stats", {}),
)
if progress_callback is not None:
stats = resolution_result.get("stats", {})
progress_callback(
MergeProgressStage.COMPLETE,
100,
"Merge complete",
{
"conflicts_found": stats.get("conflicts_resolved", 0),
"conflicts_resolved": stats.get("conflicts_resolved", 0),
},
)
return resolution_result
else:
# AI couldn't resolve all conflicts
@@ -547,6 +611,19 @@ def _try_smart_merge_inner(
resolved_files=resolution_result.get("resolved_files", []),
error=resolution_result.get("error"),
)
if progress_callback is not None:
progress_callback(
MergeProgressStage.ERROR,
0,
"Some conflicts could not be resolved",
{
"conflicts_found": len(
resolution_result.get("remaining_conflicts", [])
),
},
)
return {
"success": False,
"conflicts": resolution_result.get("remaining_conflicts", []),
@@ -555,148 +632,81 @@ def _try_smart_merge_inner(
"error": resolution_result.get("error"),
}
# Check if branches diverged but no actual conflicts (can do direct copy)
# Check if branches diverged but no actual conflicts (use git merge)
if git_conflicts.get("diverged_but_no_conflicts"):
debug(MODULE, "Branches diverged but no conflicts - doing direct file copy")
debug(MODULE, "Branches diverged but no conflicts - using git merge")
print(muted(" Branches diverged but no conflicts detected"))
print(muted(" Copying changed files directly from worktree..."))
print(muted(" Using git merge to combine changes..."))
# Get changed files from spec branch
spec_branch = f"auto-claude/{spec_name}"
base_branch = git_conflicts.get("base_branch", "main")
# Get merge-base for diff
merge_base_result = run_git(
["merge-base", base_branch, spec_branch],
# Use git merge --no-commit to combine changes from both branches
# Since merge-tree confirmed no conflicts, this should succeed cleanly
merge_result = run_git(
["merge", "--no-commit", "--no-ff", spec_branch],
cwd=project_dir,
)
merge_base = (
merge_base_result.stdout.strip()
if merge_base_result.returncode == 0
else None
)
if merge_base:
# Get list of changed files in spec branch
changed_files = _get_changed_files_from_branch(
project_dir, merge_base, spec_branch
if merge_result.returncode == 0:
# Merge succeeded - get list of files that were merged
# Use git diff --cached to see what's staged
diff_result = run_git(
["diff", "--cached", "--name-only"],
cwd=project_dir,
)
merged_files = [
f.strip()
for f in diff_result.stdout.splitlines()
if f.strip() and not _is_auto_claude_file(f.strip())
]
debug_success(
MODULE,
"Git merge succeeded",
merged_files_count=len(merged_files),
)
resolved_files = []
skipped_files = [] # Track files that failed to copy
files_to_stage = []
for file_path, status in changed_files:
if _is_auto_claude_file(file_path):
continue
for file_path in merged_files:
print(success(f" ✓ {file_path}"))
try:
target_path = project_dir / file_path
if status == "D":
# Deleted in worktree
if target_path.exists():
target_path.unlink()
files_to_stage.append(file_path)
resolved_files.append(file_path)
print(success(f" ✓ {file_path} (deleted)"))
else:
# New or modified - copy from spec branch
target_path.parent.mkdir(parents=True, exist_ok=True)
if _is_binary_file(file_path):
binary_content = _get_binary_file_content_from_ref(
project_dir, spec_branch, file_path
)
if binary_content is not None:
target_path.write_bytes(binary_content)
files_to_stage.append(file_path)
resolved_files.append(file_path)
status_label = (
"new file" if status == "A" else "updated"
)
print(
success(f" ✓ {file_path} ({status_label})")
)
else:
skipped_files.append(file_path)
debug_warning(
MODULE,
f"Could not retrieve binary content for {file_path}",
)
else:
content = _get_file_content_from_ref(
project_dir, spec_branch, file_path
)
if content is not None:
target_path.write_text(content, encoding="utf-8")
files_to_stage.append(file_path)
resolved_files.append(file_path)
status_label = (
"new file" if status == "A" else "updated"
)
print(
success(f" ✓ {file_path} ({status_label})")
)
else:
skipped_files.append(file_path)
debug_warning(
MODULE,
f"Could not retrieve content for {file_path}",
)
except Exception as e:
skipped_files.append(file_path)
debug_warning(MODULE, f"Could not copy {file_path}: {e}")
# Stage all files in a single git add call for efficiency
if files_to_stage:
add_result = run_git(
["add"] + files_to_stage,
cwd=project_dir,
if progress_callback is not None:
progress_callback(
MergeProgressStage.COMPLETE,
100,
f"Git merge complete ({len(merged_files)} files)",
)
if add_result.returncode != 0:
debug_warning(
MODULE,
f"Failed to stage files for direct copy: {add_result.stderr}",
)
# Return failure - files were written but not staged
return {
"success": False,
"error": f"Failed to stage files: {add_result.stderr}",
"resolved_files": [],
}
# Build result - check for skipped files to detect partial merges
result = {
"success": len(skipped_files) == 0,
"resolved_files": resolved_files,
return {
"success": True,
"resolved_files": merged_files,
"stats": {
"files_merged": len(resolved_files),
"files_merged": len(merged_files),
"conflicts_resolved": 0,
"ai_assisted": 0,
"auto_merged": len(resolved_files),
"direct_copy": True, # Flag indicating direct copy was used
"skipped_count": len(skipped_files),
"auto_merged": len(merged_files),
"git_merge": True, # Flag indicating git merge was used
},
}
if skipped_files:
result["skipped_files"] = skipped_files
result["partial_success"] = len(resolved_files) > 0
print()
print(
warning(
f" ⚠ {len(skipped_files)} file(s) could not be retrieved:"
)
)
for skipped_file in skipped_files:
print(muted(f" - {skipped_file}"))
print(muted(" These files may need manual review."))
return result
else:
# merge-base failed - branches may not share history
# Merge failed unexpectedly - abort and fall back to semantic analysis
debug_warning(
MODULE,
"Could not find merge-base between branches - falling back to semantic analysis",
"Git merge failed unexpectedly despite no conflicts detected",
stderr=merge_result.stderr[:500] if merge_result.stderr else "",
)
# Abort the merge to restore clean state
abort_result = run_git(["merge", "--abort"], cwd=project_dir)
if abort_result.returncode != 0:
debug_error(
MODULE,
"Failed to abort merge - repo may be in inconsistent state",
stderr=abort_result.stderr,
)
return None # Trigger fallback to avoid operating on inconsistent state
print(
warning(
" Git merge failed unexpectedly, falling back to semantic analysis..."
)
)
# No git conflicts - proceed with semantic analysis
@@ -725,6 +735,14 @@ def _try_smart_merge_inner(
# All conflicts can be auto-merged or no conflicts
print(muted(" All changes compatible, proceeding with merge..."))
if progress_callback is not None:
progress_callback(
MergeProgressStage.COMPLETE,
100,
f"Analysis complete ({files_to_merge} files compatible)",
)
return {
"success": True,
"stats": {
@@ -737,6 +755,13 @@ def _try_smart_merge_inner(
# If smart merge fails, fall back to git
import traceback
if progress_callback is not None:
progress_callback(
MergeProgressStage.ERROR,
0,
f"Smart merge error: {e}",
)
print(muted(f" Smart merge error: {e}"))
traceback.print_exc()
return None
@@ -748,14 +773,11 @@ def _rebase_spec_branch(
base_branch: str,
) -> bool:
"""
Rebase the spec branch onto the latest base branch.
Attempt to rebase the spec branch onto the latest base branch.
This performs an automatic rebase of the spec branch onto the current
base branch (main/develop) to bring it up to date before merging.
If conflicts occur during rebase, the function aborts and returns False
so that the caller can fall back to AI conflict resolution.
The function preserves the current HEAD by restoring it after completion.
NOTE: This will fail if the spec branch is checked out in a worktree,
which is the normal case. The caller should handle failure gracefully
by falling back to git merge or AI conflict resolution.
Args:
project_dir: The project directory
@@ -764,22 +786,36 @@ def _rebase_spec_branch(
Returns:
True if rebase succeeded cleanly or branch was already up-to-date,
False if rebase failed due to conflicts or other errors (aborted, no ref movement)
False if rebase failed (worktree lock, conflicts, or other errors)
"""
spec_branch = f"auto-claude/{spec_name}"
debug(
MODULE,
"Rebasing spec branch",
"Attempting to rebase spec branch",
spec_branch=spec_branch,
base_branch=base_branch,
)
# Save original branch to restore after rebase (HIGH: prevents leaving repo on spec branch)
# Check if spec branch is used by a worktree (common case)
# In this case, we can't checkout/rebase from the main repo
worktree_list_result = run_git(["worktree", "list", "--porcelain"], cwd=project_dir)
if worktree_list_result.returncode == 0:
# Check if spec_branch is in use by a worktree
output = worktree_list_result.stdout
if f"branch refs/heads/{spec_branch}" in output:
debug(
MODULE,
"Spec branch is checked out in a worktree - skipping rebase",
spec_branch=spec_branch,
)
# This is expected - return False to let caller use git merge instead
return False
# Save original branch to restore after rebase
original_branch_result = run_git(
["rev-parse", "--abbrev-ref", "HEAD"], cwd=project_dir
)
# Check returncode and validate stdout before using original_branch
if original_branch_result.returncode != 0:
debug_error(
MODULE,
@@ -795,7 +831,6 @@ def _rebase_spec_branch(
)
return False
# Save current state for recovery
# Get the current commit of spec_branch before rebase
before_commit_result = run_git(["rev-parse", spec_branch], cwd=project_dir)
if before_commit_result.returncode != 0:
@@ -804,8 +839,6 @@ def _rebase_spec_branch(
"Could not get spec branch commit before rebase",
stderr=before_commit_result.stderr,
)
# Restore original branch before returning
run_git(["checkout", original_branch], cwd=project_dir)
return False
before_commit = before_commit_result.stdout.strip()
@@ -813,22 +846,18 @@ def _rebase_spec_branch(
print(muted(f" Rebasing {spec_branch} onto {base_branch}..."))
try:
# Perform the rebase using safe/standard invocation:
# 1. Checkout the spec branch first
# 2. Run standard rebase (no strategy options - let conflicts stop the rebase)
# If conflicts occur, we'll abort and let AI handle them during merge
# Try to checkout the spec branch
checkout_result = run_git(["checkout", spec_branch], cwd=project_dir)
if checkout_result.returncode != 0:
debug_error(
# Checkout failed - likely due to worktree lock
debug(
MODULE,
"Could not checkout spec branch for rebase",
stderr=checkout_result.stderr,
"Could not checkout spec branch for rebase (likely worktree lock)",
stderr=checkout_result.stderr[:200] if checkout_result.stderr else "",
)
return False
# Run standard rebase - will stop on conflicts so we can detect them
# Git syntax: git rebase [options] <upstream>
# where <upstream> is the branch to rebase onto
# Run standard rebase
rebase_result = run_git(
["rebase", base_branch],
cwd=project_dir,
@@ -838,9 +867,6 @@ def _rebase_spec_branch(
# Rebase failed - check if it was due to conflicts
status_result = run_git(["status", "--porcelain"], cwd=project_dir)
# MEDIUM: Properly parse git status output for conflict markers
# Git status --porcelain uses two-character status codes:
# UU = both modified, AA = both added, DD = both deleted, etc.
has_unmerged = any(
line[:2] in ("UU", "AA", "DD", "AU", "UA", "DU", "UD")
for line in status_result.stdout.splitlines()
@@ -848,7 +874,6 @@ def _rebase_spec_branch(
)
# Abort the rebase to return to clean state
# NEW-002: If abort fails, immediately return False (repo in bad state)
abort_result = run_git(["rebase", "--abort"], cwd=project_dir)
if abort_result.returncode != 0:
debug_error(
@@ -856,19 +881,16 @@ def _rebase_spec_branch(
"Failed to abort rebase - repo may be in inconsistent state",
stderr=abort_result.stderr,
)
return False # Abort failed - cannot safely continue
if has_unmerged:
# Rebase failed due to conflicts - we aborted, so no ref movement happened
debug_warning(
MODULE,
"Rebase encountered conflicts - aborted, will use AI conflict resolution",
stderr=rebase_result.stderr[:200] if rebase_result.stderr else "",
)
# Return False since we aborted - no rebase occurred, caller should use AI
return False
# Other error (not conflict-related)
if has_unmerged:
debug_warning(
MODULE,
"Rebase encountered conflicts - aborted, will use alternative merge",
stderr=rebase_result.stderr[:200] if rebase_result.stderr else "",
)
return False
debug_error(
MODULE,
"Rebase failed with unexpected error",
@@ -882,9 +904,7 @@ def _rebase_spec_branch(
if after_commit_result.returncode == 0:
after_commit_hash = after_commit_result.stdout.strip()
# Verify the branch actually moved (commit changed)
if before_commit == after_commit_hash:
# MEDIUM: Branch already up-to-date is a success condition, not failure
debug(
MODULE,
"Branch already up-to-date, no rebase needed",
@@ -904,8 +924,7 @@ def _rebase_spec_branch(
debug_error(MODULE, "Could not verify spec branch commit after rebase")
return False
finally:
# HIGH: Always restore original branch, even on error/exception
# NEW-001: Log restoration failure (cannot modify return from finally block)
# Always restore original branch
if original_branch:
restore_result = run_git(["checkout", original_branch], cwd=project_dir)
if restore_result.returncode != 0:
@@ -914,8 +933,6 @@ def _rebase_spec_branch(
f"Failed to restore original branch '{original_branch}'",
stderr=restore_result.stderr,
)
# Note: Cannot modify return value from finally block,
# but restoration failure is rare and non-critical (user can manually switch back)
def _check_git_conflicts(project_dir: Path, spec_name: str) -> dict:
apps/backend/merge/conflict_resolver.py
+19 -1
View File
@@ -17,6 +17,7 @@ import logging
from .ai_resolver import AIResolver
from .auto_merger import AutoMerger, MergeContext
from .file_merger import apply_ai_merge, extract_location_content
from .progress import MergeProgressCallback, MergeProgressStage
from .types import (
ConflictRegion,
ConflictSeverity,
@@ -60,6 +61,7 @@ class ConflictResolver:
baseline_content: str,
task_snapshots: list[TaskSnapshot],
conflicts: list[ConflictRegion],
progress_callback: MergeProgressCallback | None = None,
) -> MergeResult:
"""
Resolve conflicts using AutoMerger and AIResolver.
@@ -69,6 +71,8 @@ class ConflictResolver:
baseline_content: Original file content
task_snapshots: Snapshots from all tasks modifying this file
conflicts: List of detected conflicts
progress_callback: Optional callback for emitting per-conflict
resolution progress with details about current file and conflict count
Returns:
MergeResult with resolution details
@@ -78,8 +82,22 @@ class ConflictResolver:
remaining: list[ConflictRegion] = []
ai_calls = 0
tokens_used = 0
total_conflicts = len(conflicts)
for conflict in conflicts:
for idx, conflict in enumerate(conflicts):
if progress_callback:
# Emit per-conflict progress within the resolving stage (50-75%)
conflict_percent = 50 + int((idx / max(total_conflicts, 1)) * 25)
progress_callback(
stage=MergeProgressStage.RESOLVING,
percent=conflict_percent,
message=f"Resolving conflict {idx + 1}/{total_conflicts} in {file_path}",
details={
"current_file": file_path,
"conflicts_found": total_conflicts,
"conflicts_resolved": len(resolved),
},
)
# Try auto-merge first
if conflict.can_auto_merge and conflict.merge_strategy:
context = MergeContext(
apps/backend/merge/merge_pipeline.py
+13
View File
@@ -18,6 +18,7 @@ import logging
from .conflict_detector import ConflictDetector
from .conflict_resolver import ConflictResolver
from .file_merger import apply_single_task_changes, combine_non_conflicting_changes
from .progress import MergeProgressCallback, MergeProgressStage
from .types import (
ChangeType,
FileAnalysis,
@@ -57,6 +58,7 @@ class MergePipeline:
file_path: str,
baseline_content: str,
task_snapshots: list[TaskSnapshot],
progress_callback: MergeProgressCallback | None = None,
) -> MergeResult:
"""
Merge changes from multiple tasks for a single file.
@@ -65,6 +67,8 @@ class MergePipeline:
file_path: Path to the file
baseline_content: Original baseline content
task_snapshots: Snapshots from tasks that modified this file
progress_callback: Optional callback for emitting per-file progress
within the 'resolving' stage (50-75% range)
Returns:
MergeResult with merged content or conflict info
@@ -72,6 +76,14 @@ class MergePipeline:
task_ids = [s.task_id for s in task_snapshots]
logger.info(f"Merging {file_path} with {len(task_snapshots)} task(s)")
if progress_callback:
progress_callback(
stage=MergeProgressStage.RESOLVING,
percent=50,
message=f"Merging file: {file_path}",
details={"current_file": file_path},
)
# If only one task modified the file, no conflict possible
if len(task_snapshots) == 1:
snapshot = task_snapshots[0]
@@ -119,6 +131,7 @@ class MergePipeline:
baseline_content=baseline_content,
task_snapshots=task_snapshots,
conflicts=conflicts,
progress_callback=progress_callback,
)
def _build_task_analyses(
apps/backend/merge/orchestrator.py
+167 -4
View File
@@ -33,6 +33,7 @@ from .merge_pipeline import MergePipeline
# Re-export models for backwards compatibility
from .models import MergeReport, MergeStats, TaskMergeRequest
from .progress import MergeProgressCallback, MergeProgressStage
from .semantic_analyzer import SemanticAnalyzer
from .types import (
ConflictRegion,
@@ -260,6 +261,7 @@ class MergeOrchestrator:
task_id: str,
worktree_path: Path | None = None,
target_branch: str = "main",
progress_callback: MergeProgressCallback | None = None,
) -> MergeReport:
"""
Merge a single task's changes into the target branch.
@@ -268,6 +270,8 @@ class MergeOrchestrator:
task_id: The task identifier
worktree_path: Path to the task's worktree (auto-detected if not provided)
target_branch: Branch to merge into
progress_callback: Optional callback for progress updates.
Called with (stage, percent, message, details) at key pipeline stages.
Returns:
MergeReport with results
@@ -284,7 +288,20 @@ class MergeOrchestrator:
report = MergeReport(started_at=datetime.now(), tasks_merged=[task_id])
start_time = datetime.now()
def _emit(
stage: MergeProgressStage,
percent: int,
message: str,
details: dict[str, Any] | None = None,
) -> None:
"""Emit progress if a callback is provided."""
if progress_callback is not None:
progress_callback(stage, percent, message, details)
try:
# --- ANALYZING stage (0-25%) ---
_emit(MergeProgressStage.ANALYZING, 0, "Starting merge analysis")
# Find worktree if not provided
if worktree_path is None:
debug_detailed(MODULE, "Auto-detecting worktree path...")
@@ -293,16 +310,23 @@ class MergeOrchestrator:
debug_error(MODULE, f"Could not find worktree for task {task_id}")
report.success = False
report.error = f"Could not find worktree for task {task_id}"
_emit(
MergeProgressStage.ERROR,
0,
f"Could not find worktree for task {task_id}",
)
return report
debug_detailed(MODULE, f"Found worktree: {worktree_path}")
# Ensure evolution data is up to date
_emit(MergeProgressStage.ANALYZING, 5, "Loading file evolution data")
debug(MODULE, "Refreshing evolution data from git...")
self.evolution_tracker.refresh_from_git(
task_id, worktree_path, target_branch=target_branch
)
# Get files modified by this task
_emit(MergeProgressStage.ANALYZING, 15, "Running semantic analysis")
modifications = self.evolution_tracker.get_task_modifications(task_id)
debug(
MODULE,
@@ -312,11 +336,38 @@ class MergeOrchestrator:
if not modifications:
debug_warning(MODULE, f"No modifications found for task {task_id}")
logger.info(f"No modifications found for task {task_id}")
_emit(
MergeProgressStage.COMPLETE,
100,
"No modifications found",
)
report.completed_at = datetime.now()
return report
# Process each modified file
for file_path, snapshot in modifications:
_emit(
MergeProgressStage.ANALYZING,
25,
f"Found {len(modifications)} modified files",
)
# --- DETECTING_CONFLICTS stage (25-50%) ---
_emit(
MergeProgressStage.DETECTING_CONFLICTS,
25,
"Detecting conflicts",
)
# --- RESOLVING stage (50-75%) ---
total_files = len(modifications)
for idx, (file_path, snapshot) in enumerate(modifications):
file_percent = 50 + int((idx / max(total_files, 1)) * 25)
_emit(
MergeProgressStage.RESOLVING,
file_percent,
f"Merging file {idx + 1}/{total_files}",
{"current_file": file_path},
)
debug_detailed(
MODULE,
f"Processing file: {file_path}",
@@ -349,13 +400,31 @@ class MergeOrchestrator:
file=file_path,
)
# --- VALIDATING stage (75-100%) ---
_emit(
MergeProgressStage.VALIDATING,
75,
"Validating merge results",
{
"conflicts_found": report.stats.conflicts_detected,
"conflicts_resolved": report.stats.conflicts_auto_resolved,
},
)
report.success = report.stats.files_failed == 0
_emit(
MergeProgressStage.VALIDATING,
90,
"Validation complete",
)
except Exception as e:
debug_error(MODULE, f"Merge failed for task {task_id}", error=str(e))
logger.exception(f"Merge failed for task {task_id}")
report.success = False
report.error = str(e)
_emit(MergeProgressStage.ERROR, 0, f"Merge failed: {e}")
report.completed_at = datetime.now()
report.stats.duration_seconds = (
@@ -366,6 +435,18 @@ class MergeOrchestrator:
if not self.dry_run:
self._save_report(report, task_id)
# --- COMPLETE stage (100%) ---
if report.success:
_emit(
MergeProgressStage.COMPLETE,
100,
f"Merge complete for {task_id}",
{
"conflicts_found": report.stats.conflicts_detected,
"conflicts_resolved": report.stats.conflicts_auto_resolved,
},
)
debug_success(
MODULE,
f"Merge complete for {task_id}",
@@ -382,6 +463,7 @@ class MergeOrchestrator:
self,
requests: list[TaskMergeRequest],
target_branch: str = "main",
progress_callback: MergeProgressCallback | None = None,
) -> MergeReport:
"""
Merge multiple tasks' changes.
@@ -392,6 +474,8 @@ class MergeOrchestrator:
Args:
requests: List of merge requests (one per task)
target_branch: Branch to merge into
progress_callback: Optional callback for progress updates.
Called with (stage, percent, message, details) at key pipeline stages.
Returns:
MergeReport with combined results
@@ -402,11 +486,33 @@ class MergeOrchestrator:
)
start_time = datetime.now()
def _emit(
stage: MergeProgressStage,
percent: int,
message: str,
details: dict[str, Any] | None = None,
) -> None:
"""Emit progress if a callback is provided."""
if progress_callback is not None:
progress_callback(stage, percent, message, details)
try:
# --- ANALYZING stage (0-25%) ---
_emit(
MergeProgressStage.ANALYZING,
0,
f"Starting merge analysis for {len(requests)} tasks",
)
# Sort by priority (higher first)
requests = sorted(requests, key=lambda r: -r.priority)
# Refresh evolution data for all tasks
_emit(
MergeProgressStage.ANALYZING,
5,
"Loading file evolution data",
)
for request in requests:
if request.worktree_path and request.worktree_path.exists():
self.evolution_tracker.refresh_from_git(
@@ -416,11 +522,38 @@ class MergeOrchestrator:
)
# Find all files modified by any task
_emit(
MergeProgressStage.ANALYZING,
15,
"Running semantic analysis",
)
task_ids = [r.task_id for r in requests]
file_tasks = self.evolution_tracker.get_files_modified_by_tasks(task_ids)
# Process each file
for file_path, modifying_tasks in file_tasks.items():
_emit(
MergeProgressStage.ANALYZING,
25,
f"Found {len(file_tasks)} files to merge",
)
# --- DETECTING_CONFLICTS stage (25-50%) ---
_emit(
MergeProgressStage.DETECTING_CONFLICTS,
25,
"Detecting conflicts across tasks",
)
# --- RESOLVING stage (50-75%) ---
total_files = len(file_tasks)
for idx, (file_path, modifying_tasks) in enumerate(file_tasks.items()):
file_percent = 50 + int((idx / max(total_files, 1)) * 25)
_emit(
MergeProgressStage.RESOLVING,
file_percent,
f"Merging file {idx + 1}/{total_files}",
{"current_file": file_path},
)
# Get snapshots from all tasks that modified this file
evolution = self.evolution_tracker.get_file_evolution(file_path)
if not evolution:
@@ -466,8 +599,25 @@ class MergeOrchestrator:
report.file_results[file_path] = result
self._update_stats(report.stats, result)
# --- VALIDATING stage (75-100%) ---
_emit(
MergeProgressStage.VALIDATING,
75,
"Validating merge results",
{
"conflicts_found": report.stats.conflicts_detected,
"conflicts_resolved": report.stats.conflicts_auto_resolved,
},
)
report.success = report.stats.files_failed == 0
_emit(
MergeProgressStage.VALIDATING,
90,
"Validation complete",
)
except Exception as e:
debug_error(
MODULE,
@@ -478,6 +628,7 @@ class MergeOrchestrator:
logger.exception("Merge failed")
report.success = False
report.error = str(e)
_emit(MergeProgressStage.ERROR, 0, f"Merge failed: {e}")
report.completed_at = datetime.now()
report.stats.duration_seconds = (
@@ -489,6 +640,18 @@ class MergeOrchestrator:
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
self._save_report(report, f"multi_{timestamp}")
# --- COMPLETE stage (100%) ---
if report.success:
_emit(
MergeProgressStage.COMPLETE,
100,
f"Merge complete for {len(requests)} tasks",
{
"conflicts_found": report.stats.conflicts_detected,
"conflicts_resolved": report.stats.conflicts_auto_resolved,
},
)
return report
def _merge_file(
apps/backend/merge/progress.py
+105
View File
@@ -0,0 +1,105 @@
"""
Merge Progress Emission
=======================
Structured progress event emission for the merge pipeline.
This module provides the progress reporting infrastructure used by the
merge orchestrator to communicate real-time status updates to the
Electron frontend via stdout JSON lines.
Progress events are emitted as JSON lines to stdout with type='progress',
allowing the frontend to parse them separately from the final merge result.
Components:
- MergeProgressStage: Enum of pipeline stages
- MergeProgressCallback: Protocol for type-safe callback threading
- emit_progress: Function to emit structured progress events to stdout
"""
from __future__ import annotations
import json
from enum import Enum
from typing import Any, Protocol
class MergeProgressStage(Enum):
"""
Stages of the merge pipeline.
Each stage corresponds to a phase of the merge process and maps
to a percentage range for progress reporting:
- ANALYZING: 0-25% — Loading file evolution, running semantic analysis
- DETECTING_CONFLICTS: 25-50% — Conflict detection and compatibility checks
- RESOLVING: 50-75% — Auto-merge and AI resolution of conflicts
- VALIDATING: 75-100% — Final validation of merged results
- COMPLETE: 100% — Merge finished successfully
- ERROR: N/A — Merge failed with an error
"""
ANALYZING = "analyzing"
DETECTING_CONFLICTS = "detecting_conflicts"
RESOLVING = "resolving"
VALIDATING = "validating"
COMPLETE = "complete"
ERROR = "error"
class MergeProgressCallback(Protocol):
"""
Protocol for type-safe progress callback threading.
Implementations receive structured progress updates from the merge
pipeline stages and can forward them to any output channel.
Args:
stage: Current pipeline stage
percent: Progress percentage (0-100)
message: Human-readable status message
details: Optional additional context (conflicts_found, conflicts_resolved, current_file)
"""
def __call__(
self,
stage: MergeProgressStage,
percent: int,
message: str,
details: dict[str, Any] | None = None,
) -> None: ...
def emit_progress(
stage: MergeProgressStage,
percent: int,
message: str,
details: dict[str, Any] | None = None,
) -> None:
"""
Emit a progress event as a JSON line to stdout.
The Electron main process parses these JSON lines from the merge
subprocess stdout and forwards them to the renderer via IPC.
Args:
stage: Current pipeline stage
percent: Progress percentage (0-100), clamped to valid range
message: Human-readable status message
details: Optional dict with additional context. Supported keys:
- conflicts_found (int): Number of conflicts detected
- conflicts_resolved (int): Number of conflicts resolved so far
- current_file (str): File currently being processed
"""
percent = max(0, min(100, percent))
event: dict[str, Any] = {
"type": "progress",
"stage": stage.value,
"percent": percent,
"message": message,
}
if details:
event["details"] = details
print(json.dumps(event), flush=True)
apps/backend/prompts/github/pr_parallel_orchestrator.md
+11
View File
@@ -2,6 +2,17 @@
You are an expert PR reviewer orchestrating a comprehensive, parallel code review. Your role is to analyze the PR, delegate to specialized review agents, and synthesize their findings into a final verdict.
## CRITICAL: Tool Execution Strategy
**IMPORTANT: Execute tool calls ONE AT A TIME, waiting for each result before making the next call.**
When you need to use multiple tools (Read, Grep, Glob, Task):
- ✅ Make ONE tool call, wait for the result
- ✅ Process the result, then make the NEXT tool call
- ❌ Do NOT make multiple tool calls in a single response
**Why this matters:** Parallel tool execution can cause API errors when some tools fail while others succeed. Sequential execution ensures reliable operation and proper error handling.
## Core Principle
**YOU decide which agents to invoke based on YOUR analysis of the PR.** There are no programmatic rules - you evaluate the PR's content, complexity, and risk areas, then delegate to the appropriate specialists.
apps/backend/requirements.txt
+3 -3
View File
@@ -1,7 +1,7 @@
# Auto-Build Framework Dependencies
# SDK 0.1.22+ required for custom subagent support (bundles CLI 2.1.19+)
# Earlier versions bundled CLI 2.1.9 which didn't properly register --agents flag
claude-agent-sdk>=0.1.22
# SDK 0.1.25+ required for improved tool use concurrency handling
# Earlier versions had 400 errors when tool_use blocks had partial failures
claude-agent-sdk>=0.1.25
python-dotenv>=1.0.0
# TOML parsing fallback for Python < 3.11
apps/backend/runners/github/bot_detection.py
+176 -8
View File
@@ -10,6 +10,8 @@ Key Features:
- Skips re-reviewing bot commits
- Implements "cooling off" period to prevent rapid re-reviews
- Tracks reviewed commits to avoid duplicate reviews
- In-progress tracking to prevent concurrent reviews
- Stale review detection with automatic cleanup
Usage:
detector = BotDetector(bot_token="ghp_...")
@@ -20,8 +22,16 @@ Usage:
print(f"Skipping PR: {reason}")
return
# Mark review as started (prevents concurrent reviews)
detector.mark_review_started(pr_number)
# Perform review...
# After successful review, mark as reviewed
detector.mark_reviewed(pr_number, head_sha)
# Or if review failed:
detector.mark_review_finished(pr_number, success=False)
"""
from __future__ import annotations
@@ -55,11 +65,15 @@ class BotDetectionState:
# PR number -> last review timestamp (ISO format)
last_review_times: dict[int, str] = field(default_factory=dict)
# PR number -> in-progress review start time (ISO format)
in_progress_reviews: dict[int, str] = field(default_factory=dict)
def to_dict(self) -> dict:
"""Convert to dictionary for JSON serialization."""
return {
"reviewed_commits": self.reviewed_commits,
"last_review_times": self.last_review_times,
"in_progress_reviews": self.in_progress_reviews,
}
@classmethod
@@ -68,6 +82,7 @@ class BotDetectionState:
return cls(
reviewed_commits=data.get("reviewed_commits", {}),
last_review_times=data.get("last_review_times", {}),
in_progress_reviews=data.get("in_progress_reviews", {}),
)
def save(self, state_dir: Path) -> None:
@@ -104,11 +119,16 @@ class BotDetector:
- 1-minute cooling off period between reviews of same PR (for testing)
- Tracks reviewed commit SHAs to avoid duplicate reviews
- Identifies bot user from token to skip bot-authored content
- In-progress tracking to prevent concurrent reviews
- Stale review detection (30-minute timeout)
"""
# Cooling off period in minutes (reduced to 1 for testing large PRs)
COOLING_OFF_MINUTES = 1
# Timeout for in-progress reviews in minutes (after this, review is considered stale/crashed)
IN_PROGRESS_TIMEOUT_MINUTES = 30
def __init__(
self,
state_dir: Path,
@@ -301,6 +321,104 @@ class BotDetector:
reviewed = self.state.reviewed_commits.get(str(pr_number), [])
return commit_sha in reviewed
def is_review_in_progress(self, pr_number: int) -> tuple[bool, str]:
"""
Check if a review is currently in progress for this PR.
Also detects stale reviews (started > IN_PROGRESS_TIMEOUT_MINUTES ago).
Args:
pr_number: The PR number
Returns:
Tuple of (is_in_progress, reason_message)
"""
pr_key = str(pr_number)
start_time_str = self.state.in_progress_reviews.get(pr_key)
if not start_time_str:
return False, ""
try:
start_time = datetime.fromisoformat(start_time_str)
time_elapsed = datetime.now() - start_time
# Check if review is stale (timeout exceeded)
if time_elapsed > timedelta(minutes=self.IN_PROGRESS_TIMEOUT_MINUTES):
# Mark as stale and clear the in-progress state
print(
f"[BotDetector] Review for PR #{pr_number} is stale "
f"(started {int(time_elapsed.total_seconds() / 60)}m ago, "
f"timeout: {self.IN_PROGRESS_TIMEOUT_MINUTES}m) - clearing in-progress state",
file=sys.stderr,
)
self.mark_review_finished(pr_number, success=False)
return False, ""
# Review is actively in progress
minutes_elapsed = int(time_elapsed.total_seconds() / 60)
reason = f"Review already in progress (started {minutes_elapsed}m ago)"
print(f"[BotDetector] PR #{pr_number}: {reason}", file=sys.stderr)
return True, reason
except (ValueError, TypeError) as e:
print(
f"[BotDetector] Error parsing in-progress start time: {e}",
file=sys.stderr,
)
# Clear invalid state
self.mark_review_finished(pr_number, success=False)
return False, ""
def mark_review_started(self, pr_number: int) -> None:
"""
Mark a review as started for this PR.
This should be called when beginning a review to prevent concurrent reviews.
Args:
pr_number: The PR number
"""
pr_key = str(pr_number)
# Record start time
self.state.in_progress_reviews[pr_key] = datetime.now().isoformat()
# Save state
self.state.save(self.state_dir)
logger.info(f"[BotDetector] Marked PR #{pr_number} review as started")
print(f"[BotDetector] Started review for PR #{pr_number}", file=sys.stderr)
def mark_review_finished(self, pr_number: int, success: bool = True) -> None:
"""
Mark a review as finished for this PR.
This clears the in-progress state. Should be called when review completes
(successfully or with error) or when detected as stale.
Args:
pr_number: The PR number
success: Whether the review completed successfully
"""
pr_key = str(pr_number)
# Clear in-progress state
if pr_key in self.state.in_progress_reviews:
del self.state.in_progress_reviews[pr_key]
# Save state
self.state.save(self.state_dir)
status = "successfully" if success else "with error/timeout"
logger.info(
f"[BotDetector] Marked PR #{pr_number} review as finished ({status})"
)
print(
f"[BotDetector] Finished review for PR #{pr_number} ({status})",
file=sys.stderr,
)
def should_skip_pr_review(
self,
pr_number: int,
@@ -335,13 +453,19 @@ class BotDetector:
print(f"[BotDetector] SKIP PR #{pr_number}: {reason}")
return True, reason
# Check 3: Are we in the cooling off period?
# Check 3: Is a review already in progress?
is_in_progress, reason = self.is_review_in_progress(pr_number)
if is_in_progress:
print(f"[BotDetector] SKIP PR #{pr_number}: {reason}")
return True, reason
# Check 4: Are we in the cooling off period?
is_cooling, reason = self.is_within_cooling_off(pr_number)
if is_cooling:
print(f"[BotDetector] SKIP PR #{pr_number}: {reason}")
return True, reason
# Check 4: Have we already reviewed this exact commit?
# Check 5: Have we already reviewed this exact commit?
head_sha = self.get_last_commit_sha(commits) if commits else None
if head_sha and self.has_reviewed_commit(pr_number, head_sha):
reason = f"Already reviewed commit {head_sha[:8]}"
@@ -357,6 +481,7 @@ class BotDetector:
Mark a PR as reviewed at a specific commit.
This should be called after successfully posting a review.
Also clears the in-progress state.
Args:
pr_number: The PR number
@@ -374,6 +499,10 @@ class BotDetector:
# Update last review time
self.state.last_review_times[pr_key] = datetime.now().isoformat()
# Clear in-progress state
if pr_key in self.state.in_progress_reviews:
del self.state.in_progress_reviews[pr_key]
# Save state
self.state.save(self.state_dir)
@@ -397,6 +526,9 @@ class BotDetector:
if pr_key in self.state.last_review_times:
del self.state.last_review_times[pr_key]
if pr_key in self.state.in_progress_reviews:
del self.state.in_progress_reviews[pr_key]
self.state.save(self.state_dir)
print(f"[BotDetector] Cleared state for PR #{pr_number}")
@@ -412,13 +544,16 @@ class BotDetector:
total_reviews = sum(
len(commits) for commits in self.state.reviewed_commits.values()
)
in_progress_count = len(self.state.in_progress_reviews)
return {
"bot_username": self.bot_username,
"review_own_prs": self.review_own_prs,
"total_prs_tracked": total_prs,
"total_reviews_performed": total_reviews,
"in_progress_reviews": in_progress_count,
"cooling_off_minutes": self.COOLING_OFF_MINUTES,
"in_progress_timeout_minutes": self.IN_PROGRESS_TIMEOUT_MINUTES,
}
def cleanup_stale_prs(self, max_age_days: int = 30) -> int:
@@ -428,6 +563,9 @@ class BotDetector:
This prevents unbounded growth of the state file by cleaning up
entries for PRs that are likely closed/merged.
Also cleans up stale in-progress reviews (reviews that have been
in progress for longer than IN_PROGRESS_TIMEOUT_MINUTES).
Args:
max_age_days: Remove PRs not reviewed in this many days (default: 30)
@@ -435,8 +573,13 @@ class BotDetector:
Number of PRs cleaned up
"""
cutoff = datetime.now() - timedelta(days=max_age_days)
in_progress_cutoff = datetime.now() - timedelta(
minutes=self.IN_PROGRESS_TIMEOUT_MINUTES
)
prs_to_remove: list[str] = []
stale_in_progress: list[str] = []
# Find stale reviewed PRs
for pr_key, last_review_str in self.state.last_review_times.items():
try:
last_review = datetime.fromisoformat(last_review_str)
@@ -446,18 +589,43 @@ class BotDetector:
# Invalid timestamp - mark for removal
prs_to_remove.append(pr_key)
# Find stale in-progress reviews
for pr_key, start_time_str in self.state.in_progress_reviews.items():
try:
start_time = datetime.fromisoformat(start_time_str)
if start_time < in_progress_cutoff:
stale_in_progress.append(pr_key)
except (ValueError, TypeError):
# Invalid timestamp - mark for removal
stale_in_progress.append(pr_key)
# Remove stale PRs
for pr_key in prs_to_remove:
if pr_key in self.state.reviewed_commits:
del self.state.reviewed_commits[pr_key]
if pr_key in self.state.last_review_times:
del self.state.last_review_times[pr_key]
if pr_key in self.state.in_progress_reviews:
del self.state.in_progress_reviews[pr_key]
if prs_to_remove:
# Remove stale in-progress reviews
for pr_key in stale_in_progress:
if pr_key in self.state.in_progress_reviews:
del self.state.in_progress_reviews[pr_key]
total_cleaned = len(prs_to_remove) + len(stale_in_progress)
if total_cleaned > 0:
self.state.save(self.state_dir)
print(
f"[BotDetector] Cleaned up {len(prs_to_remove)} stale PRs "
f"(older than {max_age_days} days)"
)
if prs_to_remove:
print(
f"[BotDetector] Cleaned up {len(prs_to_remove)} stale PRs "
f"(older than {max_age_days} days)"
)
if stale_in_progress:
print(
f"[BotDetector] Cleaned up {len(stale_in_progress)} stale in-progress reviews "
f"(older than {self.IN_PROGRESS_TIMEOUT_MINUTES} minutes)"
)
return len(prs_to_remove)
return total_cleaned
apps/backend/runners/github/context_gatherer.py
+19 -226
View File
@@ -19,7 +19,6 @@ from __future__ import annotations
import ast
import asyncio
import json
import os
import re
from dataclasses import dataclass, field
from pathlib import Path
@@ -825,41 +824,11 @@ class PRContextGatherer:
"""
Find files related to the changes.
This includes:
- Test files for changed source files
- Imported modules and dependencies
- Configuration files in the same directory
- Related type definition files
- Reverse dependencies (files that import changed files)
DEPRECATED: LLM agents now discover related files themselves using Read, Grep, and Glob tools.
This method returns an empty list - agents have domain expertise to find what's relevant.
"""
related = set()
for changed_file in changed_files:
path = Path(changed_file.path)
# Find test files
related.update(self._find_test_files(path))
# Find imported files (for supported languages)
if path.suffix in [".ts", ".tsx", ".js", ".jsx", ".py"]:
related.update(self._find_imports(changed_file.content, path))
# Find config files in same directory
related.update(self._find_config_files(path.parent))
# Find type definition files
if path.suffix in [".ts", ".tsx"]:
related.update(self._find_type_definitions(path))
# Find reverse dependencies (files that import this file)
related.update(self._find_dependents(changed_file.path))
# Remove files that are already in changed_files
changed_paths = {cf.path for cf in changed_files}
related = {r for r in related if r not in changed_paths}
# Use smart prioritization with increased limit (50 instead of 20)
return self._prioritize_related_files(related, limit=50)
# Return empty list - LLM agents will discover files via their tools
return []
def _find_test_files(self, source_path: Path) -> set[str]:
"""Find test files related to a source file."""
@@ -1071,170 +1040,35 @@ class PRContextGatherer:
"""
Find files that import the given file (reverse dependencies).
Uses pure Python to search for import statements referencing this file.
Cross-platform compatible (Windows, macOS, Linux).
Limited to prevent performance issues on large codebases.
DEPRECATED: LLM agents now discover reverse dependencies themselves using Grep and Read tools.
Returns empty set - agents can search the codebase with their domain expertise.
Args:
file_path: Path of the file to find dependents for
max_results: Maximum number of dependents to return
Returns:
Set of file paths that import this file.
Empty set - LLM agents will discover dependents via Grep tool.
"""
dependents: set[str] = set()
path_obj = Path(file_path)
stem = path_obj.stem # e.g., 'helpers' from 'utils/helpers.ts'
# NOTE: We no longer skip generic filenames like "utils", "types", "index".
# The LLM-driven exploration system decides what's relevant based on the PR context.
# Widely-used utilities are often the MOST important files to track dependents for.
# Build regex patterns and file extensions based on file type
pattern = None
file_extensions = []
if path_obj.suffix in [".ts", ".tsx", ".js", ".jsx"]:
# Match various import styles for JS/TS
# from './helpers', from '../utils/helpers', from '@/utils/helpers'
# Escape stem for regex safety
escaped_stem = re.escape(stem)
pattern = re.compile(rf"['\"].*{escaped_stem}['\"]")
file_extensions = [".ts", ".tsx", ".js", ".jsx"]
elif path_obj.suffix == ".py":
# Match Python imports: from .helpers import, import helpers
escaped_stem = re.escape(stem)
pattern = re.compile(rf"(from.*{escaped_stem}|import.*{escaped_stem})")
file_extensions = [".py"]
else:
return dependents
# Directories to exclude
exclude_dirs = {
"node_modules",
".git",
"dist",
"build",
"__pycache__",
".venv",
"venv",
}
# Walk the project directory
project_path = Path(self.project_dir)
files_checked = 0
max_files_to_check = 2000 # Prevent infinite scanning on large codebases
try:
for root, dirs, files in os.walk(project_path):
# Modify dirs in-place to exclude certain directories
dirs[:] = [d for d in dirs if d not in exclude_dirs]
for filename in files:
# Check if we've hit the file limit
if files_checked >= max_files_to_check:
safe_print(
f"[Context] File scan limit ({max_files_to_check}) reached for {file_path}. "
f"Found {len(dependents)} dependents. "
f"LLM agents can explore additional callers if needed via Read/Grep tools."
)
return dependents
# Check if file has the right extension
if not any(filename.endswith(ext) for ext in file_extensions):
continue
file_full_path = Path(root) / filename
files_checked += 1
# Get relative path from project root
try:
relative_path = file_full_path.relative_to(project_path)
relative_path_str = str(relative_path).replace("\\", "/")
# Don't include the file itself
if relative_path_str == file_path:
continue
# Search for the pattern in the file
try:
with open(
file_full_path, encoding="utf-8", errors="ignore"
) as f:
content = f.read()
if pattern.search(content):
dependents.add(relative_path_str)
if len(dependents) >= max_results:
return dependents
except (OSError, UnicodeDecodeError):
# Skip files that can't be read
continue
except ValueError:
# File is not relative to project_path, skip it
continue
except Exception as e:
safe_print(f"[Context] Error finding dependents: {e}")
return dependents
# Return empty set - LLM agents will use Grep to find importers when needed
return set()
def _prioritize_related_files(self, files: set[str], limit: int = 50) -> list[str]:
"""
Prioritize related files by relevance.
Priority order:
1. Test files (most important for review context)
2. Type definition files (.d.ts)
3. Configuration files
4. Direct imports/dependents
5. Other files
DEPRECATED: LLM agents now prioritize exploration based on their domain expertise.
Returns empty list since _find_related_files no longer populates files.
Args:
files: Set of file paths to prioritize
limit: Maximum number of files to return
Returns:
List of files sorted by priority, limited to `limit`.
Empty list - LLM agents handle prioritization via their tools.
"""
test_files = []
type_files = []
config_files = []
other_files = []
for f in files:
path = Path(f)
name_lower = path.name.lower()
# Test files
if (
".test." in name_lower
or ".spec." in name_lower
or name_lower.startswith("test_")
or name_lower.endswith("_test.py")
or "__tests__" in f
):
test_files.append(f)
# Type definition files
elif name_lower.endswith(".d.ts") or "types" in name_lower:
type_files.append(f)
# Config files
elif name_lower in [
n.lower() for n in CONFIG_FILE_NAMES
] or name_lower.endswith((".config.js", ".config.ts", "rc", "rc.json")):
config_files.append(f)
else:
other_files.append(f)
# Sort within each category alphabetically for consistency, then combine
prioritized = (
sorted(test_files)
+ sorted(type_files)
+ sorted(config_files)
+ sorted(other_files)
)
return prioritized[:limit]
# Return empty list - LLM agents will prioritize exploration themselves
return []
def _load_json_safe(self, filename: str) -> dict | None:
"""
@@ -1462,59 +1296,18 @@ class PRContextGatherer:
"""
Find files related to the changes using a specific project root.
This static method allows finding related files AFTER a worktree
has been created, ensuring files exist in the worktree filesystem.
DEPRECATED: LLM agents now discover related files themselves using Read, Grep, and Glob tools.
This method returns an empty list - agents have domain expertise to find what's relevant.
Args:
changed_files: List of changed files from the PR
project_root: Path to search for related files (e.g., worktree path)
Returns:
List of related file paths (relative to project root)
Empty list - LLM agents will discover files via their tools.
"""
related: set[str] = set()
for changed_file in changed_files:
path = Path(changed_file.path)
# Find test files
test_patterns = [
# Jest/Vitest patterns
path.parent / f"{path.stem}.test{path.suffix}",
path.parent / f"{path.stem}.spec{path.suffix}",
path.parent / "__tests__" / f"{path.name}",
# Python patterns
path.parent / f"test_{path.stem}.py",
path.parent / f"{path.stem}_test.py",
# Go patterns
path.parent / f"{path.stem}_test.go",
]
for test_path in test_patterns:
full_path = project_root / test_path
if full_path.exists() and full_path.is_file():
related.add(str(test_path))
# Find config files in same directory
for name in CONFIG_FILE_NAMES:
config_path = path.parent / name
full_path = project_root / config_path
if full_path.exists() and full_path.is_file():
related.add(str(config_path))
# Find type definition files
if path.suffix in [".ts", ".tsx"]:
type_def = path.parent / f"{path.stem}.d.ts"
full_path = project_root / type_def
if full_path.exists() and full_path.is_file():
related.add(str(type_def))
# Remove files that are already in changed_files
changed_paths = {cf.path for cf in changed_files}
related = {r for r in related if r not in changed_paths}
# Limit to 50 most relevant files (increased from 20)
return sorted(related)[:50]
# Return empty list - LLM agents will discover files via their tools
return []
class FollowupContextGatherer:
apps/backend/runners/github/orchestrator.py
+28 -1
View File
@@ -396,9 +396,15 @@ class GitHubOrchestrator:
# No existing review found, create skip result
return await self._create_skip_result(pr_number, skip_reason)
else:
# For other skip reasons (bot-authored, cooling off), create a skip result
# For other skip reasons (bot-authored, cooling off, in-progress), create a skip result
return await self._create_skip_result(pr_number, skip_reason)
# Mark review as started (prevents concurrent reviews)
self.bot_detector.mark_review_started(pr_number)
safe_print(
f"[BOT DETECTION] Marked PR #{pr_number} review as started", flush=True
)
self._report_progress(
"analyzing", 30, "Running multi-pass review...", pr_number=pr_number
)
@@ -572,6 +578,13 @@ class GitHubOrchestrator:
except Exception as e:
import traceback
# Mark review as finished with error
self.bot_detector.mark_review_finished(pr_number, success=False)
safe_print(
f"[BOT DETECTION] Marked PR #{pr_number} review as finished (error)",
flush=True,
)
# Log full exception details for debugging
error_details = f"{type(e).__name__}: {e}"
full_traceback = traceback.format_exc()
@@ -634,6 +647,13 @@ class GitHubOrchestrator:
pr_number=pr_number,
)
# Mark review as started (prevents concurrent reviews)
self.bot_detector.mark_review_started(pr_number)
safe_print(
f"[BOT DETECTION] Marked PR #{pr_number} follow-up review as started",
flush=True,
)
try:
# Import here to avoid circular imports at module level
try:
@@ -956,6 +976,13 @@ class GitHubOrchestrator:
return result
except Exception as e:
# Mark review as finished with error
self.bot_detector.mark_review_finished(pr_number, success=False)
safe_print(
f"[BOT DETECTION] Marked PR #{pr_number} follow-up review as finished (error)",
flush=True,
)
result = PRReviewResult(
pr_number=pr_number,
repo=self.config.repo,
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py
+207 -165
View File
@@ -29,7 +29,7 @@ from claude_agent_sdk import AgentDefinition
try:
from ...core.client import create_client
from ...phase_config import get_thinking_budget, resolve_model_id
from ..context_gatherer import PRContext, PRContextGatherer, _validate_git_ref
from ..context_gatherer import PRContext, _validate_git_ref
from ..gh_client import GHClient
from ..models import (
BRANCH_BEHIND_BLOCKER_MSG,
@@ -51,7 +51,7 @@ try:
)
from .sdk_utils import process_sdk_stream
except (ImportError, ValueError, SystemError):
from context_gatherer import PRContext, PRContextGatherer, _validate_git_ref
from context_gatherer import PRContext, _validate_git_ref
from core.client import create_client
from gh_client import GHClient
from models import (
@@ -255,9 +255,8 @@ class ParallelOrchestratorReviewer:
"Security specialist. Use for OWASP Top 10, authentication, "
"injection, cryptographic issues, and sensitive data exposure. "
"Invoke when PR touches auth, API endpoints, user input, database queries, "
"or file operations. IMPORTANT: Also check related files listed in the "
"PR context - callers may be affected by security changes, and tests "
"should verify security behavior."
"or file operations. Use Read, Grep, and Glob tools to explore related files, "
"callers, and tests as needed."
),
prompt=with_working_dir(
security_prompt, "You are a security expert. Find vulnerabilities."
@@ -269,9 +268,8 @@ class ParallelOrchestratorReviewer:
description=(
"Code quality expert. Use for complexity, duplication, error handling, "
"maintainability, and pattern adherence. Invoke when PR has complex logic, "
"large functions, or significant business logic changes. IMPORTANT: Check "
"related files for pattern consistency - if a pattern is changed, similar "
"code elsewhere should be updated too."
"large functions, or significant business logic changes. Use Grep to search "
"for similar patterns across the codebase for consistency checks."
),
prompt=with_working_dir(
quality_prompt,
@@ -285,8 +283,7 @@ class ParallelOrchestratorReviewer:
"Logic and correctness specialist. Use for algorithm verification, "
"edge cases, state management, and race conditions. Invoke when PR has "
"algorithmic changes, data transformations, concurrent operations, or bug fixes. "
"IMPORTANT: Check callers and dependents in related files - logic changes "
"may break assumptions made by code that uses this file."
"Use Grep to find callers and dependents that may be affected by logic changes."
),
prompt=with_working_dir(
logic_prompt, "You are a logic expert. Find correctness issues."
@@ -299,8 +296,7 @@ class ParallelOrchestratorReviewer:
"Codebase consistency expert. Use for naming conventions, ecosystem fit, "
"architectural alignment, and avoiding reinvention. Invoke when PR introduces "
"new patterns, large additions, or code that might duplicate existing functionality. "
"IMPORTANT: Use related files to understand existing patterns - new code "
"should match established conventions in the codebase."
"Use Grep and Glob to explore existing patterns and conventions in the codebase."
),
prompt=with_working_dir(
codebase_fit_prompt,
@@ -329,7 +325,7 @@ class ParallelOrchestratorReviewer:
"Reads the ACTUAL CODE at the finding location with fresh eyes. "
"CRITICAL: Invoke for ALL findings after specialist agents complete. "
"Can confirm findings as valid OR dismiss them as false positives. "
"Check related files for mitigations the original agent missed."
"Use Read, Grep, and Glob to check for mitigations the original agent missed."
),
prompt=with_working_dir(
validator_prompt, "You validate whether findings are real issues."
@@ -397,80 +393,10 @@ Found {len(context.ai_bot_comments)} comments from AI tools.
{chr(10).join(commits_list)}
"""
# Build related files section (CONTEXT-02)
# Removed: Related files and import graph sections
# LLM agents now discover relevant files themselves via Read, Grep, Glob tools
related_files_section = ""
if context.related_files:
# Categorize by type
tests = [
f
for f in context.related_files
if ".test." in f
or "_test." in f
or f.startswith("test")
or "/tests/" in f
or "\\tests\\" in f
]
deps = [f for f in context.related_files if f not in tests]
# Limit to avoid context overflow
tests = tests[:15]
deps = deps[:15]
tests_str = ", ".join(f"`{t}`" for t in tests) if tests else "None found"
deps_str = ", ".join(f"`{d}`" for d in deps) if deps else "None found"
related_files_section = f"""
### Related Files to Investigate
These files are related to the changes (imports, tests, dependents). **Pass relevant files to specialists when delegating.**
**Tests** ({len(tests)} files): {tests_str}
**Dependencies/Callers** ({len(deps)} files): {deps_str}
**When delegating to specialists, include relevant files:**
- **security-reviewer**: Mention files that handle the same data flow
- **logic-reviewer**: Mention callers that depend on changed function signatures
- **quality-reviewer**: Mention files with similar patterns for consistency check
- **codebase-fit-reviewer**: Mention existing implementations of similar features
Example delegation: "Review the auth changes in login.ts. Also check auth_middleware.ts and auth.test.ts which use this module."
"""
# Build import graph summary (CONTEXT-03)
import_graph_section = ""
import_entries = []
changed_paths = {f.path for f in context.changed_files}
for file in context.changed_files[:10]: # Limit to 10 files
# Find what this file imports (look for related files it references)
imports_this = [
r
for r in context.related_files
if r in (file.content or "") and r not in changed_paths
][:5]
# Find what imports this file (reverse deps in related_files)
# Match by filename stem to catch imports without extension
file_stem = file.path.split("/")[-1].split(".")[0]
imported_by = [
r
for r in context.related_files
if file_stem in r and r not in changed_paths
][:5]
if imports_this or imported_by:
entry = f"**{file.path}**"
if imports_this:
entry += f"\n - Imports: {', '.join(imports_this)}"
if imported_by:
entry += f"\n - Imported by: {', '.join(imported_by)}"
import_entries.append(entry)
if import_entries:
import_graph_section = f"""
### Import Relationships
How the changed files connect to the codebase:
{chr(10).join(import_entries[:20])}
"""
pr_context = f"""
---
@@ -790,25 +716,9 @@ The SDK will run invoked agents in parallel automatically.
else self.project_dir
)
# Rescan for related files using the worktree/project root
# This fixes the issue where related files were 0 because context gathering
# happened BEFORE the worktree was created (PR files didn't exist locally)
if context.changed_files:
new_related_files = PRContextGatherer.find_related_files_for_root(
context.changed_files,
project_root,
)
# Always log rescan result (not gated by DEBUG_MODE)
if new_related_files:
context.related_files = new_related_files
safe_print(
f"[PRReview] Rescanned in worktree: found {len(new_related_files)} related files"
)
else:
safe_print(
f"[PRReview] Rescanned in worktree: found 0 related files "
f"(initial scan found {len(context.related_files)})"
)
# Removed: Related files rescanning
# LLM agents now discover relevant files themselves via Read, Grep, Glob tools
# No need to pre-scan the codebase programmatically
# Build orchestrator prompt AFTER worktree creation and related files rescan
prompt = self._build_orchestrator_prompt(context)
@@ -839,36 +749,110 @@ The SDK will run invoked agents in parallel automatically.
)
# Run orchestrator session using shared SDK stream processor
async with client:
await client.query(prompt)
# Retry logic for tool use concurrency errors
MAX_RETRIES = 3
RETRY_DELAY = 2.0 # seconds between retries
safe_print(
f"[ParallelOrchestrator] Running orchestrator ({model})...",
flush=True,
)
result_text = ""
structured_output = None
agents_invoked = []
msg_count = 0
last_error = None
# Process SDK stream with shared utility
stream_result = await process_sdk_stream(
client=client,
context_name="ParallelOrchestrator",
model=model,
system_prompt=prompt,
agent_definitions=agent_defs,
)
# Check for stream processing errors
if stream_result.get("error"):
logger.error(
f"[ParallelOrchestrator] SDK stream failed: {stream_result['error']}"
for attempt in range(MAX_RETRIES):
if attempt > 0:
logger.info(
f"[ParallelOrchestrator] Retry attempt {attempt}/{MAX_RETRIES - 1} "
f"after tool concurrency error"
)
raise RuntimeError(
f"SDK stream processing failed: {stream_result['error']}"
safe_print(
f"[ParallelOrchestrator] Retry {attempt}/{MAX_RETRIES - 1} "
f"(tool concurrency error detected)"
)
# Small delay before retry
import asyncio
await asyncio.sleep(RETRY_DELAY)
# Recreate client for retry (fresh session)
client = self._create_sdk_client(
project_root, model, thinking_budget
)
result_text = stream_result["result_text"]
structured_output = stream_result["structured_output"]
agents_invoked = stream_result["agents_invoked"]
msg_count = stream_result["msg_count"]
try:
async with client:
await client.query(prompt)
safe_print(
f"[ParallelOrchestrator] Running orchestrator ({model})...",
flush=True,
)
# Process SDK stream with shared utility
stream_result = await process_sdk_stream(
client=client,
context_name="ParallelOrchestrator",
model=model,
system_prompt=prompt,
agent_definitions=agent_defs,
)
error = stream_result.get("error")
# Check for tool concurrency error specifically
if (
error == "tool_use_concurrency_error"
and attempt < MAX_RETRIES - 1
):
logger.warning(
f"[ParallelOrchestrator] Tool concurrency error on attempt {attempt + 1}, "
f"will retry..."
)
last_error = error
continue # Retry
# Check for other stream processing errors
if error:
logger.error(
f"[ParallelOrchestrator] SDK stream failed: {error}"
)
raise RuntimeError(f"SDK stream processing failed: {error}")
# Success - extract results and break retry loop
result_text = stream_result["result_text"]
structured_output = stream_result["structured_output"]
agents_invoked = stream_result["agents_invoked"]
msg_count = stream_result["msg_count"]
break # Success, exit retry loop
except Exception as e:
# Check if this is a retryable error
error_str = str(e).lower()
is_retryable = (
"400" in error_str
or "concurrency" in error_str
or "tool_use" in error_str
)
if is_retryable and attempt < MAX_RETRIES - 1:
logger.warning(
f"[ParallelOrchestrator] Retryable error on attempt {attempt + 1}: {e}"
)
last_error = str(e)
continue # Retry
# Not retryable or out of retries - re-raise
raise
else:
# All retries exhausted
logger.error(
f"[ParallelOrchestrator] Failed after {MAX_RETRIES} attempts. "
f"Last error: {last_error}"
)
raise RuntimeError(
f"Orchestrator failed after {MAX_RETRIES} retry attempts. "
f"Last error: {last_error}"
)
self._report_progress(
"finalizing",
@@ -1427,6 +1411,10 @@ The SDK will run invoked agents in parallel automatically.
if not findings:
return []
# Retry configuration for API errors
MAX_VALIDATION_RETRIES = 2
VALIDATOR_MAX_MESSAGES = 200 # Lower limit for validator (simpler task)
# Build validation prompt with all findings
findings_json = []
for f in findings:
@@ -1466,47 +1454,101 @@ For EACH finding above:
model_shorthand = self.config.model or "sonnet"
model = resolve_model_id(model_shorthand)
# Create validator client (inherits worktree filesystem access)
try:
validator_client = create_client(
project_dir=worktree_path,
spec_dir=self.github_dir,
model=model,
agent_type="pr_finding_validator",
max_thinking_tokens=get_thinking_budget("medium"),
output_format={
"type": "json_schema",
"schema": FindingValidationResponse.model_json_schema(),
},
)
except Exception as e:
logger.error(f"[PRReview] Failed to create validator client: {e}")
# Fail-safe: return original findings
return findings
# Run validation
try:
async with validator_client:
await validator_client.query(prompt)
stream_result = await process_sdk_stream(
client=validator_client,
context_name="FindingValidator",
model=model,
system_prompt=prompt,
# Retry loop for transient API errors
last_error = None
for attempt in range(MAX_VALIDATION_RETRIES + 1):
if attempt > 0:
logger.info(
f"[PRReview] Validation retry {attempt}/{MAX_VALIDATION_RETRIES}"
)
safe_print(
f"[FindingValidator] Retry attempt {attempt}/{MAX_VALIDATION_RETRIES}"
)
if stream_result.get("error"):
logger.error(
f"[PRReview] Validation failed: {stream_result['error']}"
# Create validator client (inherits worktree filesystem access)
try:
validator_client = create_client(
project_dir=worktree_path,
spec_dir=self.github_dir,
model=model,
agent_type="pr_finding_validator",
max_thinking_tokens=get_thinking_budget("medium"),
output_format={
"type": "json_schema",
"schema": FindingValidationResponse.model_json_schema(),
},
)
except Exception as e:
logger.error(f"[PRReview] Failed to create validator client: {e}")
last_error = e
continue # Try again
# Run validation
try:
async with validator_client:
await validator_client.query(prompt)
stream_result = await process_sdk_stream(
client=validator_client,
context_name="FindingValidator",
model=model,
system_prompt=prompt,
max_messages=VALIDATOR_MAX_MESSAGES,
)
# Fail-safe: return original findings
return findings
structured_output = stream_result.get("structured_output")
error = stream_result.get("error")
if error:
# Check for specific error types that warrant retry
error_str = str(error).lower()
is_retryable = (
"400" in error_str
or "concurrency" in error_str
or "circuit breaker" in error_str
or "tool_use" in error_str
)
except Exception as e:
logger.error(f"[PRReview] Validation stream error: {e}")
if is_retryable and attempt < MAX_VALIDATION_RETRIES:
logger.warning(
f"[PRReview] Retryable validation error: {error}"
)
last_error = Exception(error)
continue # Retry
logger.error(f"[PRReview] Validation failed: {error}")
# Fail-safe: return original findings
return findings
structured_output = stream_result.get("structured_output")
# Success - break out of retry loop
if structured_output:
break
except Exception as e:
error_str = str(e).lower()
is_retryable = (
"400" in error_str
or "concurrency" in error_str
or "rate" in error_str
)
if is_retryable and attempt < MAX_VALIDATION_RETRIES:
logger.warning(f"[PRReview] Retryable stream error: {e}")
last_error = e
continue # Retry
logger.error(f"[PRReview] Validation stream error: {e}")
# Fail-safe: return original findings
return findings
else:
# All retries exhausted
logger.error(
f"[PRReview] Validation failed after {MAX_VALIDATION_RETRIES} retries. "
f"Last error: {last_error}"
)
safe_print(
f"[FindingValidator] ERROR: Validation failed after {MAX_VALIDATION_RETRIES} retries"
)
# Fail-safe: return original findings
return findings
apps/backend/runners/github/services/pr_review_engine.py
+2 -11
View File
@@ -139,18 +139,9 @@ class PRReviewEngine:
files_list.append(f"- ... and {len(context.changed_files) - 20} more files")
files_str = "\n".join(files_list)
# NEW: Format related files (imports, tests, etc.)
# Removed: Related files section
# LLM agents now discover relevant files themselves via Read, Grep, Glob tools
related_files_str = ""
if context.related_files:
related_files_list = [f"- `{f}`" for f in context.related_files[:10]]
if len(context.related_files) > 10:
related_files_list.append(
f"- ... and {len(context.related_files) - 10} more"
)
related_files_str = f"""
### Related Files (imports, tests, configs)
{chr(10).join(related_files_list)}
"""
# NEW: Format commits for context
commits_str = ""
apps/backend/runners/github/services/sdk_utils.py
+80 -256
View File
@@ -25,243 +25,6 @@ logger = logging.getLogger(__name__)
# Check if debug mode is enabled
DEBUG_MODE = os.environ.get("DEBUG", "").lower() in ("true", "1", "yes")
# ── TEMPORARY: Per-PR full agent communication logger (v2) ────────────
# Writes every message to .auto-claude/github/pr/debug_logs/<context>_<ts>.log
# Remove after measurement phase is complete.
import datetime as _dt
import json as _json
from pathlib import Path as _Path
# Derive project root dynamically from this file's location
# sdk_utils.py is at: apps/backend/runners/github/services/sdk_utils.py
# So project root is 5 levels up
_PROJECT_ROOT = _Path(__file__).resolve().parent.parent.parent.parent.parent
_PR_LOG_DIR = _PROJECT_ROOT / ".auto-claude" / "github" / "pr" / "debug_logs"
class _PRDebugLogger:
"""Writes full agent communication to a log file for review.
Improvements (v2):
- System prompt and agent definitions logged at session start
- No truncation on thinking, text, tool input, or tool results
- No duplicate logging (single structured dump per message)
- Empty/whitespace content shown via repr()
- Agent attribution via subagent_tool_ids mapping
"""
def __init__(self, context_name: str, model: str | None = None):
self._f = None
self._subagent_tool_ids: dict[str, str] = {} # tool_id -> agent_name
try:
_PR_LOG_DIR.mkdir(parents=True, exist_ok=True)
ts = _dt.datetime.now().strftime("%Y%m%d_%H%M%S")
self.path = _PR_LOG_DIR / f"{context_name}_{ts}.log"
self._f = open(self.path, "w", encoding="utf-8")
self._write(
f"=== {context_name} Session Started at {_dt.datetime.now().isoformat()} ==="
)
if model:
self._write(f"Model: {model}")
self._write("")
except OSError as e:
# Failed to create directory or open file - logging disabled
logger.warning(f"PR debug logger disabled: {e}")
self.path = None
def _write(self, text: str):
# Skip logging if file handle was not created successfully
if self._f is None:
return
try:
self._f.write(text + "\n")
self._f.flush()
except (OSError, ValueError) as e:
# File write failed (file closed, disk full, etc.) - disable logging
logger.warning(f"PR debug logger write failed: {e}")
self._f = None
# ── Session preamble loggers ──────────────────────────────────────
def log_system_prompt(self, prompt: str):
"""Log the full system prompt (no truncation)."""
self._write(f"\n{'#' * 80}")
self._write("# SYSTEM PROMPT (full orchestrator instructions + PR context)")
self._write(f"# Length: {len(prompt)} chars")
self._write(f"{'#' * 80}")
self._write(prompt)
self._write(f"{'#' * 80}\n")
def log_agent_definitions(self, agents: dict):
"""Log all specialist agent definitions (prompts, tools, descriptions)."""
self._write(f"\n{'#' * 80}")
self._write(f"# AGENT DEFINITIONS ({len(agents)} specialists)")
self._write(f"{'#' * 80}")
for name, defn in agents.items():
self._write(f"\n--- Agent: {name} ---")
self._write(f" description: {getattr(defn, 'description', 'N/A')}")
self._write(f" model: {getattr(defn, 'model', 'N/A')}")
self._write(f" tools: {getattr(defn, 'tools', 'N/A')}")
prompt = getattr(defn, "prompt", "")
self._write(f" prompt ({len(prompt)} chars):")
self._write(prompt)
self._write(f"{'#' * 80}\n")
# ── Agent attribution ─────────────────────────────────────────────
def set_subagent_mapping(self, mapping: dict[str, str]):
"""Update the tool_id -> agent_name mapping for attribution."""
self._subagent_tool_ids = mapping
def _get_agent_label(self, tool_id: str) -> str:
"""Return agent label if this tool_id belongs to a known subagent."""
agent = self._subagent_tool_ids.get(tool_id)
return f" [Agent:{agent}]" if agent else ""
# ── Per-message logger (single structured dump) ───────────────────
def log_message(self, msg_count: int, msg_type: str, msg: object):
self._write(f"\n{'=' * 80}")
self._write(f"--- Message #{msg_count} [{msg_type}] ---")
self._write(f"{'=' * 80}")
self._dump_raw(msg)
def _dump_raw(self, msg: object, indent: int = 0):
"""Dump full raw message content recursively — NO truncation."""
prefix = " " * indent
# Content blocks
if hasattr(msg, "content"):
content = msg.content
if isinstance(content, list):
self._write(f"{prefix}[content] ({len(content)} blocks):")
for i, block in enumerate(content):
block_type = type(block).__name__
self._write(f"{prefix} [{i}] {block_type}:")
self._dump_block(block, indent + 2)
elif isinstance(content, str):
if not content or content.isspace():
self._write(
f"{prefix}[content] (string, {len(content)} chars): {repr(content)}"
)
else:
self._write(f"{prefix}[content] (string, {len(content)} chars):")
self._write(content)
else:
self._write(f"{prefix}[content] ({type(content).__name__}):")
self._write(f"{prefix} {str(content)}")
# Role / type
if hasattr(msg, "role"):
self._write(f"{prefix}[role] {msg.role}")
if hasattr(msg, "type") and not hasattr(msg, "content"):
self._write(f"{prefix}[type] {msg.type}")
# Structured output
if hasattr(msg, "structured_output") and msg.structured_output:
self._write(f"{prefix}[structured_output]:")
try:
self._write(_json.dumps(msg.structured_output, indent=2, default=str))
except Exception:
self._write(f"{prefix} {str(msg.structured_output)}")
# Result message fields
if hasattr(msg, "subtype"):
self._write(f"{prefix}[subtype] {msg.subtype}")
if hasattr(msg, "is_error"):
self._write(f"{prefix}[is_error] {msg.is_error}")
if hasattr(msg, "duration_ms"):
self._write(f"{prefix}[duration_ms] {msg.duration_ms}")
if hasattr(msg, "session_id"):
self._write(f"{prefix}[session_id] {msg.session_id}")
# Catch-all for messages without content blocks
for attr in ("text", "thinking", "name", "id", "input", "tool_use_id"):
if hasattr(msg, attr) and not hasattr(msg, "content"):
val = getattr(msg, attr)
if val is not None:
self._write(f"{prefix}[{attr}] {str(val)}")
def _dump_block(self, block: object, indent: int = 0):
"""Dump a single content block — NO truncation."""
prefix = " " * indent
block_type = getattr(block, "type", type(block).__name__)
if block_type in ("text", "TextBlock") and hasattr(block, "text"):
text = block.text
if not text or text.isspace():
self._write(f"{prefix}[text] ({len(text)} chars): {repr(text)}")
else:
self._write(f"{prefix}[text] ({len(text)} chars):")
self._write(text)
elif block_type in ("thinking", "ThinkingBlock") and hasattr(block, "thinking"):
text = block.thinking or getattr(block, "text", "")
self._write(f"{prefix}[thinking] ({len(text)} chars):")
self._write(text)
elif block_type in ("tool_use", "ToolUseBlock"):
tool_name = getattr(block, "name", "unknown")
tool_id = getattr(block, "id", "unknown")
tool_input = getattr(block, "input", {})
agent_label = self._get_agent_label(tool_id)
self._write(f"{prefix}[tool_use] {tool_name} (id={tool_id}){agent_label}")
try:
self._write(_json.dumps(tool_input, indent=2, default=str))
except Exception:
self._write(str(tool_input))
elif block_type in ("tool_result", "ToolResultBlock"):
tool_id = getattr(block, "tool_use_id", "unknown")
is_error = getattr(block, "is_error", False)
result = getattr(block, "content", "")
if isinstance(result, list):
result = " ".join(str(getattr(c, "text", c)) for c in result)
status = "ERROR" if is_error else "OK"
agent_label = self._get_agent_label(tool_id)
self._write(
f"{prefix}[tool_result] (tool_id={tool_id}) {status}{agent_label}"
)
self._write(str(result))
else:
# Unknown block type — dump everything we can
self._write(f"{prefix}[{block_type}] (raw dump):")
for attr in dir(block):
if not attr.startswith("_"):
try:
val = getattr(block, attr)
if not callable(val):
self._write(f"{prefix} {attr}: {str(val)}")
except Exception:
pass
# ── Structured output (standalone, for final result) ──────────────
def log_structured_output(self, output: dict):
self._write("[STRUCTURED_OUTPUT]")
try:
self._write(_json.dumps(output, indent=2, default=str))
except Exception:
self._write(str(output))
# ── Session close ─────────────────────────────────────────────────
def close(self, summary: dict):
self._write("\n=== Session Ended ===")
self._write(f"Messages: {summary.get('msg_count', '?')}")
self._write(f"Agents invoked: {summary.get('agents_invoked', [])}")
self._write(f"Error: {summary.get('error')}")
self._write(f"Log file: {self.path}")
if self._f is not None:
try:
self._f.close()
except OSError as e:
logger.warning(f"PR debug logger close failed: {e}")
# ── END TEMPORARY ──────────────────────────────────────────────────────
def _short_model_name(model: str | None) -> str:
"""Convert full model name to a short display name for logs.
@@ -361,6 +124,49 @@ def _get_tool_detail(tool_name: str, tool_input: dict[str, Any]) -> str:
return f"Using tool: {tool_name}"
# Circuit breaker threshold - abort if message count exceeds this
# Prevents runaway retry loops from consuming unbounded resources
MAX_MESSAGE_COUNT = 500
def _is_tool_concurrency_error(text: str) -> bool:
"""
Detect the specific tool use concurrency error pattern.
This error occurs when Claude makes multiple parallel tool_use blocks
and some fail, corrupting the tool_use/tool_result message pairing.
Args:
text: Text to check for error pattern
Returns:
True if this is the tool concurrency error, False otherwise
"""
text_lower = text.lower()
# Check for the specific error message pattern
# Pattern 1: Explicit concurrency or tool_use errors with 400
has_400 = "400" in text_lower
has_tool = "tool" in text_lower
if has_400 and has_tool:
# Look for specific keywords indicating tool concurrency issues
error_keywords = [
"concurrency",
"tool_use",
"tool use",
"tool_result",
"tool result",
]
if any(keyword in text_lower for keyword in error_keywords):
return True
# Pattern 2: API error with 400 and tool mention
if "api error" in text_lower and has_400 and has_tool:
return True
return False
async def process_sdk_stream(
client: Any,
on_thinking: Callable[[str], None] | None = None,
@@ -370,8 +176,10 @@ async def process_sdk_stream(
on_structured_output: Callable[[dict[str, Any]], None] | None = None,
context_name: str = "SDK",
model: str | None = None,
system_prompt: str | None = None,
agent_definitions: dict | None = None,
max_messages: int | None = None,
# Deprecated parameters (kept for backwards compatibility, no longer used)
system_prompt: str | None = None, # noqa: ARG001
agent_definitions: dict | None = None, # noqa: ARG001
) -> dict[str, Any]:
"""
Process SDK response stream with customizable callbacks.
@@ -392,8 +200,7 @@ async def process_sdk_stream(
on_structured_output: Callback for structured output - receives dict
context_name: Name for logging (e.g., "ParallelOrchestrator", "ParallelFollowup")
model: Model name for logging (e.g., "claude-sonnet-4-5-20250929")
system_prompt: Full system prompt sent to the agent (logged at session start)
agent_definitions: Dict of agent name -> AgentDefinition (logged at session start)
max_messages: Optional override for max message count circuit breaker (default: MAX_MESSAGE_COUNT)
Returns:
Dictionary with:
@@ -412,15 +219,11 @@ async def process_sdk_stream(
# Track subagent tool IDs to log their results
subagent_tool_ids: dict[str, str] = {} # tool_id -> agent_name
completed_agent_tool_ids: set[str] = set() # tool_ids of completed agents
# Track tool concurrency errors for retry logic
detected_concurrency_error = False
# TEMPORARY: per-PR debug file logger
_dbg = _PRDebugLogger(context_name, model=model)
# Log session preamble: system prompt and agent definitions
if system_prompt:
_dbg.log_system_prompt(system_prompt)
if agent_definitions:
_dbg.log_agent_definitions(agent_definitions)
# Circuit breaker: max messages before aborting
message_limit = max_messages if max_messages is not None else MAX_MESSAGE_COUNT
safe_print(f"[{context_name}] Processing SDK stream...")
if DEBUG_MODE:
@@ -435,7 +238,17 @@ async def process_sdk_stream(
try:
msg_type = type(msg).__name__
msg_count += 1
_dbg.log_message(msg_count, msg_type, msg)
# CIRCUIT BREAKER: Abort if message count exceeds threshold
# This prevents runaway retry loops (e.g., 400 errors causing infinite retries)
if msg_count > message_limit:
stream_error = (
f"Circuit breaker triggered: message count ({msg_count}) "
f"exceeded limit ({message_limit}). Possible retry loop detected."
)
logger.error(f"[{context_name}] {stream_error}")
safe_print(f"[{context_name}] ERROR: {stream_error}")
break
# Log progress periodically so user knows AI is working
if msg_count - last_progress_log >= PROGRESS_LOG_INTERVAL:
@@ -504,7 +317,6 @@ async def process_sdk_stream(
agents_invoked.append(agent_name)
# Track this tool ID to log its result later
subagent_tool_ids[tool_id] = agent_name
_dbg.set_subagent_mapping(subagent_tool_ids)
# Log with model info if available
model_info = f" [{_short_model_name(model)}]" if model else ""
safe_print(
@@ -611,6 +423,15 @@ async def process_sdk_stream(
block_type = type(block).__name__
if block_type == "TextBlock" and hasattr(block, "text"):
result_text += block.text
# Check for tool concurrency error pattern in text output
if _is_tool_concurrency_error(block.text):
detected_concurrency_error = True
logger.warning(
f"[{context_name}] Detected tool use concurrency error in response"
)
safe_print(
f"[{context_name}] WARNING: Tool concurrency error detected"
)
# Always print text content preview (not just in DEBUG_MODE)
text_preview = block.text[:500].replace("\n", " ").strip()
if text_preview:
@@ -655,7 +476,6 @@ async def process_sdk_stream(
# Only capture if we don't already have it (avoid duplicates)
if structured_output is None:
structured_output = msg.structured_output
_dbg.log_structured_output(msg.structured_output)
safe_print(f"[{context_name}] Received structured output")
if on_structured_output:
on_structured_output(msg.structured_output)
@@ -728,7 +548,14 @@ async def process_sdk_stream(
safe_print(f"[{context_name}] Session ended. Total messages: {msg_count}")
result = {
# Set error flag if tool concurrency error was detected
if detected_concurrency_error and not stream_error:
stream_error = "tool_use_concurrency_error"
logger.warning(
f"[{context_name}] Tool use concurrency error detected - caller should retry"
)
return {
"result_text": result_text,
"structured_output": structured_output,
"agents_invoked": agents_invoked,
@@ -736,6 +563,3 @@ async def process_sdk_stream(
"subagent_tool_ids": subagent_tool_ids,
"error": stream_error,
}
_dbg.close(result)
safe_print(f"[{context_name}] Full debug log: {_dbg.path}")
return result
apps/backend/runners/github/test_bot_detection.py
+171
View File
@@ -532,5 +532,176 @@ class TestGhExecutableDetection:
mock_run.assert_not_called()
class TestInProgressTracking:
"""Test in-progress review tracking."""
def test_mark_review_started(self, mock_bot_detector, temp_state_dir):
"""Test marking review as started."""
mock_bot_detector.mark_review_started(123)
# Check state
assert "123" in mock_bot_detector.state.in_progress_reviews
start_time_str = mock_bot_detector.state.in_progress_reviews["123"]
start_time = datetime.fromisoformat(start_time_str)
# Should be very recent (within last 5 seconds)
time_diff = datetime.now() - start_time
assert time_diff.total_seconds() < 5
# Check persistence
loaded = BotDetectionState.load(temp_state_dir)
assert "123" in loaded.in_progress_reviews
def test_mark_review_finished_success(self, mock_bot_detector, temp_state_dir):
"""Test marking review as finished successfully."""
mock_bot_detector.mark_review_started(123)
assert "123" in mock_bot_detector.state.in_progress_reviews
mock_bot_detector.mark_review_finished(123, success=True)
# In-progress state should be cleared
assert "123" not in mock_bot_detector.state.in_progress_reviews
# Check persistence
loaded = BotDetectionState.load(temp_state_dir)
assert "123" not in loaded.in_progress_reviews
def test_mark_review_finished_error(self, mock_bot_detector):
"""Test marking review as finished with error."""
mock_bot_detector.mark_review_started(123)
mock_bot_detector.mark_review_finished(123, success=False)
# In-progress state should be cleared
assert "123" not in mock_bot_detector.state.in_progress_reviews
def test_is_review_in_progress_active(self, mock_bot_detector):
"""Test detecting active in-progress review."""
mock_bot_detector.mark_review_started(123)
is_in_progress, reason = mock_bot_detector.is_review_in_progress(123)
assert is_in_progress is True
assert "already in progress" in reason.lower()
def test_is_review_in_progress_not_started(self, mock_bot_detector):
"""Test checking in-progress when review not started."""
is_in_progress, reason = mock_bot_detector.is_review_in_progress(999)
assert is_in_progress is False
assert reason == ""
def test_is_review_in_progress_stale(self, mock_bot_detector):
"""Test detecting stale in-progress review."""
# Set review start time to 31 minutes ago (past timeout)
stale_time = datetime.now() - timedelta(minutes=31)
mock_bot_detector.state.in_progress_reviews["123"] = stale_time.isoformat()
is_in_progress, reason = mock_bot_detector.is_review_in_progress(123)
# Should detect as stale and clear it
assert is_in_progress is False
assert reason == ""
# Should be removed from state
assert "123" not in mock_bot_detector.state.in_progress_reviews
def test_is_review_in_progress_invalid_timestamp(self, mock_bot_detector):
"""Test handling invalid timestamp in in-progress state."""
mock_bot_detector.state.in_progress_reviews["123"] = "invalid-timestamp"
is_in_progress, reason = mock_bot_detector.is_review_in_progress(123)
# Should clear invalid state
assert is_in_progress is False
assert reason == ""
assert "123" not in mock_bot_detector.state.in_progress_reviews
def test_should_skip_review_in_progress(self, mock_bot_detector):
"""Test skipping PR when review is in progress."""
mock_bot_detector.mark_review_started(123)
pr_data = {"author": {"login": "alice"}}
commits = [{"author": {"login": "alice"}, "oid": "abc123"}]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is True
assert "already in progress" in reason.lower()
def test_mark_reviewed_clears_in_progress(self, mock_bot_detector):
"""Test that mark_reviewed also clears in-progress state."""
mock_bot_detector.mark_review_started(123)
assert "123" in mock_bot_detector.state.in_progress_reviews
mock_bot_detector.mark_reviewed(123, "abc123")
# In-progress should be cleared
assert "123" not in mock_bot_detector.state.in_progress_reviews
# Reviewed state should be set
assert "123" in mock_bot_detector.state.reviewed_commits
assert "abc123" in mock_bot_detector.state.reviewed_commits["123"]
def test_clear_pr_state_clears_in_progress(self, mock_bot_detector):
"""Test that clear_pr_state also clears in-progress state."""
mock_bot_detector.mark_review_started(123)
mock_bot_detector.mark_reviewed(123, "abc123")
assert (
"123" in mock_bot_detector.state.in_progress_reviews or True
) # May be cleared by mark_reviewed
assert "123" in mock_bot_detector.state.reviewed_commits
# Start another review
mock_bot_detector.mark_review_started(123)
assert "123" in mock_bot_detector.state.in_progress_reviews
mock_bot_detector.clear_pr_state(123)
# Everything should be cleared
assert "123" not in mock_bot_detector.state.in_progress_reviews
assert "123" not in mock_bot_detector.state.reviewed_commits
assert "123" not in mock_bot_detector.state.last_review_times
def test_get_stats_includes_in_progress(self, mock_bot_detector):
"""Test that get_stats includes in-progress count."""
mock_bot_detector.mark_review_started(123)
mock_bot_detector.mark_review_started(456)
mock_bot_detector.mark_reviewed(789, "abc123")
stats = mock_bot_detector.get_stats()
assert stats["in_progress_reviews"] == 2
assert stats["total_prs_tracked"] == 1 # Only 789 is tracked as reviewed
assert stats["in_progress_timeout_minutes"] == 30
def test_cleanup_stale_prs_removes_stale_in_progress(self, mock_bot_detector):
"""Test that cleanup_stale_prs removes stale in-progress reviews."""
# Add a stale in-progress review (32 minutes ago)
stale_time = datetime.now() - timedelta(minutes=32)
mock_bot_detector.state.in_progress_reviews["123"] = stale_time.isoformat()
# Add an active in-progress review (5 minutes ago)
active_time = datetime.now() - timedelta(minutes=5)
mock_bot_detector.state.in_progress_reviews["456"] = active_time.isoformat()
# Add a stale reviewed PR (40 days ago)
stale_review_time = datetime.now() - timedelta(days=40)
mock_bot_detector.state.reviewed_commits["789"] = ["abc123"]
mock_bot_detector.state.last_review_times["789"] = stale_review_time.isoformat()
cleaned = mock_bot_detector.cleanup_stale_prs(max_age_days=30)
# Should remove stale in-progress and stale reviewed PR
assert cleaned == 2 # 1 stale in-progress + 1 stale reviewed
assert "123" not in mock_bot_detector.state.in_progress_reviews
assert (
"456" in mock_bot_detector.state.in_progress_reviews
) # Active one remains
assert "789" not in mock_bot_detector.state.reviewed_commits
if __name__ == "__main__":
pytest.main([__file__, "-v"])
apps/backend/security/hooks.py
+28 -10
View File
@@ -38,7 +38,7 @@ async def bash_security_hook(
context: Optional context
Returns:
Empty dict to allow, or {"decision": "block", "reason": "..."} to block
Empty dict to allow, or hookSpecificOutput with permissionDecision "deny" to block
"""
if input_data.get("tool_name") != "Bash":
return {}
@@ -49,15 +49,21 @@ async def bash_security_hook(
# Check if tool_input is None (malformed tool call)
if tool_input is None:
return {
"decision": "block",
"reason": "Bash tool_input is None - malformed tool call from SDK",
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": "Bash tool_input is None - malformed tool call from SDK",
}
}
# Check if tool_input is a dict
if not isinstance(tool_input, dict):
return {
"decision": "block",
"reason": f"Bash tool_input must be dict, got {type(tool_input).__name__}",
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": f"Bash tool_input must be dict, got {type(tool_input).__name__}",
}
}
# Now safe to access command
@@ -97,8 +103,11 @@ async def bash_security_hook(
if not commands:
# Could not parse - fail safe by blocking
return {
"decision": "block",
"reason": f"Could not parse command for security validation: {command}",
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": f"Could not parse command for security validation: {command}",
}
}
# Split into segments for per-command validation
@@ -114,8 +123,11 @@ async def bash_security_hook(
if not is_allowed:
return {
"decision": "block",
"reason": reason,
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": reason,
}
}
# Additional validation for sensitive commands
@@ -127,7 +139,13 @@ async def bash_security_hook(
validator = VALIDATORS[cmd]
allowed, reason = validator(cmd_segment)
if not allowed:
return {"decision": "block", "reason": reason}
return {
"hookSpecificOutput": {
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": reason,
}
}
return {}
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts
+151 -117
View File
@@ -3,7 +3,7 @@ import { IPC_CHANNELS, AUTO_BUILD_PATHS, DEFAULT_APP_SETTINGS, DEFAULT_FEATURE_M
import type { IPCResult, WorktreeStatus, WorktreeDiff, WorktreeDiffFile, WorktreeMergeResult, WorktreeDiscardResult, WorktreeListResult, WorktreeListItem, WorktreeCreatePROptions, WorktreeCreatePRResult, SupportedIDE, SupportedTerminal, AppSettings } from '../../../shared/types';
import path from 'path';
import { minimatch } from 'minimatch';
import { existsSync, readdirSync, statSync, readFileSync } from 'fs';
import { existsSync, readdirSync, statSync, readFileSync, promises as fsPromises } from 'fs';
import { execSync, execFileSync, spawn, spawnSync, exec, execFile } from 'child_process';
import { homedir } from 'os';
import { projectStore } from '../../project-store';
@@ -2009,6 +2009,18 @@ export function registerWorktreeHandlers(
debug('TIMEOUT: Merge process exceeded', MERGE_TIMEOUT_MS, 'ms, killing...');
resolved = true;
// Send timeout error progress event to the renderer
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.TASK_MERGE_PROGRESS, taskId, {
type: 'progress',
stage: 'error',
percent: 0,
message: 'Merge process timed out after 10 minutes',
details: {}
});
}
// Platform-specific process termination with fallback
killProcessGracefully(mergeProcess, {
debugPrefix: '[MERGE]',
@@ -2042,10 +2054,40 @@ export function registerWorktreeHandlers(
}
}, MERGE_TIMEOUT_MS);
let lineBuffer = ''; // Buffer for partial JSON lines spanning data chunks
mergeProcess.stdout.on('data', (data: Buffer) => {
const chunk = data.toString('utf-8');
stdout += chunk;
debug('STDOUT:', chunk);
// Prepend any buffered partial line from previous chunk
const combined = lineBuffer + chunk;
const lines = combined.split('\n');
// Last element may be a partial line - buffer it for next chunk
lineBuffer = lines.pop() || '';
for (const line of lines) {
const trimmed = line.trim();
if (!trimmed) continue;
try {
const parsed = JSON.parse(trimmed);
if (parsed && parsed.type === 'progress') {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.TASK_MERGE_PROGRESS, taskId, parsed);
}
// Don't accumulate progress lines in stdout - they are not part of the final result
continue;
}
} catch {
// Not valid JSON - treat as regular output
}
// Accumulate non-progress lines for final result parsing
stdout += line + '\n';
}
});
mergeProcess.stderr.on('data', (data: Buffer) => {
@@ -2060,6 +2102,24 @@ export function registerWorktreeHandlers(
resolved = true;
if (timeoutId) clearTimeout(timeoutId);
// Flush any remaining buffered line
if (lineBuffer.trim()) {
try {
const parsed = JSON.parse(lineBuffer.trim());
if (parsed && parsed.type === 'progress') {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.TASK_MERGE_PROGRESS, taskId, parsed);
}
} else {
stdout += lineBuffer;
}
} catch {
stdout += lineBuffer;
}
lineBuffer = '';
}
debug('Process exited with code:', code, 'signal:', signal);
debug('Full stdout:', stdout);
debug('Full stderr:', stderr);
@@ -2353,6 +2413,19 @@ export function registerWorktreeHandlers(
resolved = true;
if (timeoutId) clearTimeout(timeoutId);
console.error('[MERGE] Process spawn error:', err);
// Send error progress event to the renderer
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.TASK_MERGE_PROGRESS, taskId, {
type: 'progress',
stage: 'error',
percent: 0,
message: `Merge process crashed: ${err.message}`,
details: {}
});
}
resolve({
success: false,
error: `Failed to run merge: ${err.message}`
@@ -2657,154 +2730,115 @@ export function registerWorktreeHandlers(
}
);
// Promisified execFile for async git operations
const execFileAsync = promisify(execFile);
/**
* List all spec worktrees for a project
* Per-spec architecture: Each spec has its own worktree at .auto-claude/worktrees/tasks/{spec-name}/
*
* Options:
* - includeStats: When true, fetches commit count, files changed, additions, deletions per worktree.
* When false (default), only fetches branch and base branch info for faster listing.
*/
ipcMain.handle(
IPC_CHANNELS.TASK_LIST_WORKTREES,
async (_, projectId: string, options?: { includeStats?: boolean }): Promise<IPCResult<WorktreeListResult>> => {
async (_, projectId: string): Promise<IPCResult<WorktreeListResult>> => {
try {
const project = projectStore.getProject(projectId);
if (!project) {
return { success: false, error: 'Project not found' };
}
const includeStats = options?.includeStats ?? false;
const worktreesDir = getTaskWorktreeDir(project.path);
const gitPath = getToolPath('git');
// Detect the project's default branch once (main/master) instead of per-worktree
let projectDefaultBranch: string | undefined;
if (!project.settings?.mainBranch || !GIT_BRANCH_REGEX.test(project.settings.mainBranch)) {
for (const branch of ['main', 'master']) {
try {
await execFileAsync(gitPath, ['rev-parse', '--verify', branch], {
cwd: project.path,
encoding: 'utf-8',
});
projectDefaultBranch = branch;
break;
} catch {
// Branch doesn't exist, try next
}
}
if (!projectDefaultBranch) {
projectDefaultBranch = 'main'; // fallback
}
}
// Helper to get effective base branch using cached project default
const getBaseBranchForEntry = (entry: string): string => {
// 1. Try task metadata baseBranch
const specDir = path.join(project.path, '.auto-claude', 'specs', entry);
const taskBaseBranch = getTaskBaseBranch(specDir);
if (taskBaseBranch) return taskBaseBranch;
// 2. Try project settings mainBranch
if (project.settings?.mainBranch && GIT_BRANCH_REGEX.test(project.settings.mainBranch)) {
return project.settings.mainBranch;
}
// 3. Use pre-detected project default branch
return projectDefaultBranch || 'main';
};
// Async helper to process a single worktree entry
const processWorktreeEntryAsync = async (entry: string, entryPath: string): Promise<WorktreeListItem | null> => {
// Helper to process a single worktree entry (async)
const processWorktreeEntry = async (entry: string, entryPath: string): Promise<WorktreeListItem | null> => {
try {
// Get branch info (always needed)
const { stdout: branchOutput } = await execFileAsync(gitPath, ['rev-parse', '--abbrev-ref', 'HEAD'], {
// Get branch info (async)
const branchResult = await execFileAsync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], {
cwd: entryPath,
encoding: 'utf-8'
});
const branch = branchOutput.trim();
const branch = (branchResult.stdout as string).trim();
const baseBranch = getBaseBranchForEntry(entry);
// Get base branch using proper fallback chain:
// 1. Task metadata baseBranch, 2. Project settings mainBranch, 3. main/master detection
// Note: We do NOT use current HEAD as that may be a feature branch
const baseBranch = getEffectiveBaseBranch(project.path, entry, project.settings?.mainBranch);
const item: WorktreeListItem = {
// Get commit count (async, cross-platform - no shell syntax)
let commitCount = 0;
try {
const countResult = await execFileAsync(getToolPath('git'), ['rev-list', '--count', `${baseBranch}..HEAD`], {
cwd: entryPath,
encoding: 'utf-8'
});
commitCount = parseInt((countResult.stdout as string).trim(), 10) || 0;
} catch {
commitCount = 0;
}
// Get diff stats (async, cross-platform - no shell syntax)
let filesChanged = 0;
let additions = 0;
let deletions = 0;
try {
const diffResult = await execFileAsync(getToolPath('git'), ['diff', '--shortstat', `${baseBranch}...HEAD`], {
cwd: entryPath,
encoding: 'utf-8'
});
const diffStat = (diffResult.stdout as string).trim();
const filesMatch = diffStat.match(/(\d+) files? changed/);
const addMatch = diffStat.match(/(\d+) insertions?/);
const delMatch = diffStat.match(/(\d+) deletions?/);
if (filesMatch) filesChanged = parseInt(filesMatch[1], 10) || 0;
if (addMatch) additions = parseInt(addMatch[1], 10) || 0;
if (delMatch) deletions = parseInt(delMatch[1], 10) || 0;
} catch {
// Ignore diff errors
}
return {
specName: entry,
path: entryPath,
branch,
baseBranch,
commitCount,
filesChanged,
additions,
deletions
};
// Only fetch stats when requested
if (includeStats) {
// Run commit count and diff stats in parallel
const [countResult, diffResult] = await Promise.allSettled([
execFileAsync(gitPath, ['rev-list', '--count', `${baseBranch}..HEAD`], {
cwd: entryPath,
encoding: 'utf-8',
}),
execFileAsync(gitPath, ['diff', '--shortstat', `${baseBranch}...HEAD`], {
cwd: entryPath,
encoding: 'utf-8',
}),
]);
if (countResult.status === 'fulfilled') {
item.commitCount = parseInt(countResult.value.stdout.trim(), 10) || 0;
} else {
item.commitCount = 0;
}
if (diffResult.status === 'fulfilled') {
const diffStat = diffResult.value.stdout.trim();
const filesMatch = diffStat.match(/(\d+) files? changed/);
const addMatch = diffStat.match(/(\d+) insertions?/);
const delMatch = diffStat.match(/(\d+) deletions?/);
item.filesChanged = filesMatch ? parseInt(filesMatch[1], 10) || 0 : 0;
item.additions = addMatch ? parseInt(addMatch[1], 10) || 0 : 0;
item.deletions = delMatch ? parseInt(delMatch[1], 10) || 0 : 0;
} else {
item.filesChanged = 0;
item.additions = 0;
item.deletions = 0;
}
}
return item;
} catch (gitError) {
console.error(`Error getting info for worktree ${entry}:`, gitError);
// Skip this worktree if we can't get git info
return null;
}
};
// Scan worktrees directory and process all entries in parallel
let worktrees: WorktreeListItem[] = [];
if (existsSync(worktreesDir)) {
const entries = readdirSync(worktreesDir);
const dirEntries: Array<{ entry: string; entryPath: string }> = [];
for (const entry of entries) {
const entryPath = path.join(worktreesDir, entry);
try {
const stat = statSync(entryPath);
if (stat.isDirectory()) {
dirEntries.push({ entry, entryPath });
}
} catch {
// Skip entries that can't be stat'd
}
}
// Process all worktrees in parallel
const results = await Promise.allSettled(
dirEntries.map(({ entry, entryPath }) => processWorktreeEntryAsync(entry, entryPath))
);
worktrees = results
.filter((r): r is PromiseFulfilledResult<WorktreeListItem | null> => r.status === 'fulfilled')
.map(r => r.value)
.filter((item): item is WorktreeListItem => item !== null);
// Scan worktrees directory (async)
if (!existsSync(worktreesDir)) {
return { success: true, data: { worktrees: [] } };
}
const entries = await fsPromises.readdir(worktreesDir);
// Process all worktrees in parallel for better performance
const worktreePromises = entries.map(async (entry) => {
const entryPath = path.join(worktreesDir, entry);
try {
const stat = await fsPromises.stat(entryPath);
if (stat.isDirectory()) {
return processWorktreeEntry(entry, entryPath);
}
} catch {
// Skip entries that can't be stat'd
}
return null;
});
const results = await Promise.all(worktreePromises);
const worktrees = results.filter((w): w is WorktreeListItem => w !== null);
return { success: true, data: { worktrees } };
} catch (error) {
console.error('Failed to list worktrees:', error);
apps/frontend/src/preload/api/task-api.ts
+21
View File
@@ -10,6 +10,7 @@ import type {
TaskMetadata,
TaskLogs,
TaskLogStreamChunk,
MergeProgress,
SupportedIDE,
SupportedTerminal,
WorktreeCreatePROptions,
@@ -84,6 +85,9 @@ export interface TaskAPI {
unwatchTaskLogs: (specId: string) => Promise<IPCResult>;
onTaskLogsChanged: (callback: (specId: string, logs: TaskLogs) => void) => () => void;
onTaskLogsStream: (callback: (specId: string, chunk: TaskLogStreamChunk) => void) => () => void;
// Merge Progress Events
onMergeProgress: (callback: (taskId: string, progress: MergeProgress) => void) => () => void;
}
export const createTaskAPI = (): TaskAPI => ({
@@ -308,5 +312,22 @@ export const createTaskAPI = (): TaskAPI => ({
return () => {
ipcRenderer.removeListener(IPC_CHANNELS.TASK_LOGS_STREAM, handler);
};
},
// Merge Progress Events
onMergeProgress: (
callback: (taskId: string, progress: MergeProgress) => void
): (() => void) => {
const handler = (
_event: Electron.IpcRendererEvent,
taskId: string,
progress: MergeProgress
): void => {
callback(taskId, progress);
};
ipcRenderer.on(IPC_CHANNELS.TASK_MERGE_PROGRESS, handler);
return () => {
ipcRenderer.removeListener(IPC_CHANNELS.TASK_MERGE_PROGRESS, handler);
};
}
});
apps/frontend/src/renderer/components/ClaudeCodeStatusBadge.tsx
+604 -59
View File
@@ -6,10 +6,33 @@ import {
AlertTriangle,
X,
Loader2,
Download,
RefreshCw,
ExternalLink,
FolderOpen,
} from "lucide-react";
import { Button } from "./ui/button";
import { Popover, PopoverContent, PopoverTrigger } from "./ui/popover";
import { Tooltip, TooltipContent, TooltipTrigger } from "./ui/tooltip";
import {
AlertDialog,
AlertDialogAction,
AlertDialogCancel,
AlertDialogContent,
AlertDialogDescription,
AlertDialogFooter,
AlertDialogHeader,
AlertDialogTitle,
} from "./ui/alert-dialog";
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from "./ui/select";
import { cn } from "../lib/utils";
import type { ClaudeCodeVersionInfo } from "../../shared/types/cli";
import type { ClaudeCodeVersionInfo, ClaudeInstallationInfo } from "../../shared/types/cli";
interface ClaudeCodeStatusBadgeProps {
className?: string;
@@ -19,15 +42,36 @@ type StatusType = "loading" | "installed" | "outdated" | "not-found" | "error";
// Check every 24 hours
const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000;
// Delay before re-checking version after install/update
const VERSION_RECHECK_DELAY_MS = 5000;
/**
* Claude Code CLI status badge for the sidebar.
* Shows installation status with a tooltip on hover.
* Shows installation status and provides quick access to install/update.
*/
export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) {
const { t } = useTranslation(["common", "navigation"]);
const [status, setStatus] = useState<StatusType>("loading");
const [versionInfo, setVersionInfo] = useState<ClaudeCodeVersionInfo | null>(null);
const [isInstalling, setIsInstalling] = useState(false);
const [lastChecked, setLastChecked] = useState<Date | null>(null);
const [isOpen, setIsOpen] = useState(false);
const [showUpdateWarning, setShowUpdateWarning] = useState(false);
// Version rollback state
const [availableVersions, setAvailableVersions] = useState<string[]>([]);
const [isLoadingVersions, setIsLoadingVersions] = useState(false);
const [versionsError, setVersionsError] = useState<string | null>(null);
const [selectedVersion, setSelectedVersion] = useState<string | null>(null);
const [showRollbackWarning, setShowRollbackWarning] = useState(false);
const [installError, setInstallError] = useState<string | null>(null);
// CLI path selection state
const [installations, setInstallations] = useState<ClaudeInstallationInfo[]>([]);
const [isLoadingInstallations, setIsLoadingInstallations] = useState(false);
const [installationsError, setInstallationsError] = useState<string | null>(null);
const [selectedInstallation, setSelectedInstallation] = useState<string | null>(null);
const [showPathChangeWarning, setShowPathChangeWarning] = useState(false);
// Check Claude Code version
const checkVersion = useCallback(async () => {
@@ -41,6 +85,7 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
if (result.success && result.data) {
setVersionInfo(result.data);
setLastChecked(new Date());
if (!result.data.installed) {
setStatus("not-found");
@@ -52,11 +97,60 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
} else {
setStatus("error");
}
} catch {
} catch (err) {
console.error("Failed to check Claude Code version:", err);
setStatus("error");
}
}, []);
// Fetch available versions
const fetchVersions = useCallback(async () => {
if (!window.electronAPI?.getClaudeCodeVersions) {
return;
}
setIsLoadingVersions(true);
setVersionsError(null);
try {
const result = await window.electronAPI.getClaudeCodeVersions();
if (result.success && result.data) {
setAvailableVersions(result.data.versions);
} else {
setVersionsError(result.error || "Failed to load versions");
}
} catch (err) {
console.error("Failed to fetch versions:", err);
setVersionsError("Failed to load versions");
} finally {
setIsLoadingVersions(false);
}
}, []);
// Fetch CLI installations
const fetchInstallations = useCallback(async () => {
if (!window.electronAPI?.getClaudeCodeInstallations) {
return;
}
setIsLoadingInstallations(true);
setInstallationsError(null);
try {
const result = await window.electronAPI.getClaudeCodeInstallations();
if (result.success && result.data) {
setInstallations(result.data.installations);
} else {
setInstallationsError(result.error || "Failed to load installations");
}
} catch (err) {
console.error("Failed to fetch installations:", err);
setInstallationsError("Failed to load installations");
} finally {
setIsLoadingInstallations(false);
}
}, []);
// Initial check and periodic re-check
useEffect(() => {
checkVersion();
@@ -69,6 +163,156 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
return () => clearInterval(interval);
}, [checkVersion]);
// Fetch versions when popover opens and Claude is installed
useEffect(() => {
if (isOpen && versionInfo?.installed && availableVersions.length === 0) {
fetchVersions();
}
}, [isOpen, versionInfo?.installed, availableVersions.length, fetchVersions]);
// Fetch installations when popover opens
useEffect(() => {
if (isOpen && installations.length === 0) {
fetchInstallations();
}
}, [isOpen, installations.length, fetchInstallations]);
// Perform the actual install/update
const performInstall = async () => {
setIsInstalling(true);
setShowUpdateWarning(false);
setInstallError(null);
try {
if (!window.electronAPI?.installClaudeCode) {
setInstallError("Installation not available");
setIsInstalling(false);
return;
}
const result = await window.electronAPI.installClaudeCode();
if (result.success) {
// Re-check after a delay
setTimeout(() => {
checkVersion();
}, VERSION_RECHECK_DELAY_MS);
} else {
setInstallError(result.error || "Installation failed");
}
} catch (err) {
console.error("Failed to install Claude Code:", err);
setInstallError(err instanceof Error ? err.message : "Installation failed");
} finally {
setIsInstalling(false);
}
};
// Perform version rollback/switch
const performVersionSwitch = async () => {
if (!selectedVersion) return;
setIsInstalling(true);
setShowRollbackWarning(false);
setInstallError(null);
try {
if (!window.electronAPI?.installClaudeCodeVersion) {
setInstallError("Version switching not available");
return;
}
const result = await window.electronAPI.installClaudeCodeVersion(selectedVersion);
if (result.success) {
// Re-check after a delay
setTimeout(() => {
checkVersion();
}, VERSION_RECHECK_DELAY_MS);
} else {
setInstallError(result.error || "Failed to switch version");
}
} catch (err) {
console.error("Failed to switch Claude Code version:", err);
setInstallError(err instanceof Error ? err.message : "Failed to switch version");
} finally {
setIsInstalling(false);
setSelectedVersion(null);
}
};
// Perform CLI path switch
const performPathSwitch = async () => {
if (!selectedInstallation) return;
setIsInstalling(true);
setShowPathChangeWarning(false);
setInstallError(null);
try {
if (!window.electronAPI?.setClaudeCodeActivePath) {
setInstallError("Path switching not available");
return;
}
const result = await window.electronAPI.setClaudeCodeActivePath(selectedInstallation);
if (result.success) {
// Re-check version and refresh installations
setTimeout(() => {
checkVersion();
fetchInstallations();
}, VERSION_RECHECK_DELAY_MS);
} else {
setInstallError(result.error || "Failed to switch CLI path");
}
} catch (err) {
console.error("Failed to switch Claude CLI path:", err);
setInstallError(err instanceof Error ? err.message : "Failed to switch CLI path");
} finally {
setIsInstalling(false);
setSelectedInstallation(null);
}
};
// Handle install/update button click
const handleInstall = () => {
if (status === "outdated") {
// Show warning for updates since it will close running Claude sessions
setShowUpdateWarning(true);
} else {
// Fresh install - no warning needed
performInstall();
}
};
// Handle installation selection
const handleInstallationSelect = (cliPath: string) => {
// Don't do anything if it's the currently active installation
const installation = installations.find(i => i.path === cliPath);
if (installation?.isActive) {
return;
}
setInstallError(null);
setSelectedInstallation(cliPath);
setShowPathChangeWarning(true);
};
// Normalize version string by removing 'v' prefix for comparison
const normalizeVersion = (v: string) => v.replace(/^v/, '');
// Handle version selection
const handleVersionSelect = (version: string) => {
// Don't do anything if it's the currently installed version (normalize both for comparison)
const normalizedSelected = normalizeVersion(version);
const normalizedInstalled = versionInfo?.installed ? normalizeVersion(versionInfo.installed) : '';
if (normalizedSelected === normalizedInstalled) {
return;
}
setInstallError(null);
setSelectedVersion(version);
setShowRollbackWarning(true);
};
// Get status indicator color
const getStatusColor = () => {
switch (status) {
@@ -84,27 +328,7 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
}
};
// Get tooltip text
const getTooltipText = () => {
switch (status) {
case "loading":
return t("navigation:claudeCode.checking", "Checking Claude Code...");
case "installed":
return versionInfo?.installed
? t("navigation:claudeCode.upToDateWithVersion", "Claude Code {{version}} installed", {
version: versionInfo.installed,
})
: t("navigation:claudeCode.upToDate", "Claude Code is up to date");
case "outdated":
return t("navigation:claudeCode.updateAvailable", "Claude Code update available");
case "not-found":
return t("navigation:claudeCode.notInstalled", "Claude Code not installed");
case "error":
return t("navigation:claudeCode.error", "Error checking Claude Code");
}
};
// Get status icon for the badge
// Get status icon
const getStatusIcon = () => {
switch (status) {
case "loading":
@@ -114,52 +338,373 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
case "outdated":
return <AlertTriangle className="h-3 w-3" />;
case "not-found":
case "error":
return <X className="h-3 w-3" />;
case "error":
return <AlertTriangle className="h-3 w-3" />;
}
};
// Get tooltip text
const getTooltipText = () => {
switch (status) {
case "loading":
return t("navigation:claudeCode.checking", "Checking Claude Code...");
case "installed":
return t("navigation:claudeCode.upToDate", "Claude Code is up to date");
case "outdated":
return t("navigation:claudeCode.updateAvailable", "Claude Code update available");
case "not-found":
return t("navigation:claudeCode.notInstalled", "Claude Code not installed");
case "error":
return t("navigation:claudeCode.error", "Error checking Claude Code");
}
};
return (
<Tooltip>
<TooltipTrigger asChild>
<div
className={cn(
"flex items-center gap-2 px-2 py-1.5 text-xs rounded-md",
status === "not-found" || status === "error" ? "text-destructive" : "",
status === "outdated" ? "text-yellow-600 dark:text-yellow-500" : "",
className
)}
>
<div className="relative">
<Terminal className="h-4 w-4" />
<span
<Popover open={isOpen} onOpenChange={setIsOpen}>
<Tooltip>
<TooltipTrigger asChild>
<PopoverTrigger asChild>
<Button
variant="ghost"
size="sm"
className={cn(
"absolute -bottom-0.5 -right-0.5 h-2 w-2 rounded-full",
getStatusColor()
"w-full justify-start gap-2 text-xs",
status === "not-found" || status === "error" ? "text-destructive" : "",
status === "outdated" ? "text-yellow-600 dark:text-yellow-500" : "",
className
)}
/>
>
<div className="relative">
<Terminal className="h-4 w-4" />
<span
className={cn(
"absolute -bottom-0.5 -right-0.5 h-2 w-2 rounded-full",
getStatusColor()
)}
/>
</div>
<span className="truncate">Claude Code</span>
{status === "outdated" && (
<span className="ml-auto text-[10px] bg-yellow-500/20 text-yellow-600 dark:text-yellow-400 px-1.5 py-0.5 rounded">
{t("common:update", "Update")}
</span>
)}
{status === "not-found" && (
<span className="ml-auto text-[10px] bg-destructive/20 text-destructive px-1.5 py-0.5 rounded">
{t("common:install", "Install")}
</span>
)}
</Button>
</PopoverTrigger>
</TooltipTrigger>
<TooltipContent side="right">{getTooltipText()}</TooltipContent>
</Tooltip>
<PopoverContent side="right" align="end" className="w-72">
<div className="space-y-3">
{/* Header */}
<div className="flex items-center gap-2">
<div className="flex h-8 w-8 items-center justify-center rounded-lg bg-primary/10">
<Terminal className="h-4 w-4 text-primary" />
</div>
<div>
<h4 className="text-sm font-medium">Claude Code CLI</h4>
<p className="text-xs text-muted-foreground flex items-center gap-1">
{getStatusIcon()}
{status === "installed" && t("navigation:claudeCode.installed", "Installed")}
{status === "outdated" && t("navigation:claudeCode.outdated", "Update available")}
{status === "not-found" && t("navigation:claudeCode.missing", "Not installed")}
{status === "loading" && t("navigation:claudeCode.checking", "Checking...")}
{status === "error" && t("navigation:claudeCode.error", "Error")}
</p>
</div>
</div>
<span className="truncate">Claude Code</span>
{status === "outdated" && (
<span className="ml-auto text-[10px] bg-yellow-500/20 text-yellow-600 dark:text-yellow-400 px-1.5 py-0.5 rounded flex items-center gap-1">
{getStatusIcon()}
{t("common:update", "Update")}
</span>
{/* Version info */}
{versionInfo && status !== "loading" && (
<div className="text-xs space-y-1 p-2 bg-muted rounded-md">
{versionInfo.installed && (
<div className="flex justify-between">
<span className="text-muted-foreground">
{t("navigation:claudeCode.current", "Current")}:
</span>
<span className="font-mono">{versionInfo.installed}</span>
</div>
)}
{versionInfo.latest && versionInfo.latest !== "unknown" && (
<div className="flex justify-between">
<span className="text-muted-foreground">
{t("navigation:claudeCode.latest", "Latest")}:
</span>
<span className="font-mono">{versionInfo.latest}</span>
</div>
)}
{versionInfo.path && (
<div className="flex justify-between items-center gap-2">
<span className="text-muted-foreground flex items-center gap-1">
<FolderOpen className="h-3 w-3" />
{t("navigation:claudeCode.path", "Path")}:
</span>
<span
className="font-mono text-[10px] truncate max-w-[140px]"
title={versionInfo.path}
>
{versionInfo.path}
</span>
</div>
)}
{lastChecked && (
<div className="flex justify-between text-muted-foreground">
<span>{t("navigation:claudeCode.lastChecked", "Last checked")}:</span>
<span>{lastChecked.toLocaleTimeString()}</span>
</div>
)}
</div>
)}
{status === "not-found" && (
<span className="ml-auto text-[10px] bg-destructive/20 text-destructive px-1.5 py-0.5 rounded flex items-center gap-1">
{getStatusIcon()}
{t("common:install", "Install")}
</span>
{/* Actions */}
<div className="flex gap-2">
{(status === "not-found" || status === "outdated") && (
<Button
size="sm"
className="flex-1 gap-1"
onClick={handleInstall}
disabled={isInstalling}
>
{isInstalling ? (
<Loader2 className="h-3 w-3 animate-spin" />
) : (
<Download className="h-3 w-3" />
)}
{status === "outdated"
? t("common:update", "Update")
: t("common:install", "Install")}
</Button>
)}
<Button
variant="outline"
size="sm"
className="gap-1"
onClick={() => checkVersion()}
disabled={status === "loading"}
>
<RefreshCw className={cn("h-3 w-3", status === "loading" && "animate-spin")} />
{t("common:refresh", "Refresh")}
</Button>
</div>
{/* Install/Update error display */}
{installError && (
<div className="text-xs p-2 bg-destructive/10 text-destructive rounded-md flex items-center gap-2">
<AlertTriangle className="h-3 w-3 shrink-0" />
<span>{installError}</span>
</div>
)}
{status === "installed" && (
<span className="ml-auto text-[10px] text-muted-foreground flex items-center gap-1">
{getStatusIcon()}
</span>
{/* Version selector - only show when Claude is installed */}
{versionInfo?.installed && (
<div className="space-y-1.5">
<label className="text-xs text-muted-foreground">
{t("navigation:claudeCode.switchVersion", "Switch Version")}
</label>
<Select
value={selectedVersion || ""}
onValueChange={handleVersionSelect}
disabled={isLoadingVersions || isInstalling}
>
<SelectTrigger className="h-8 text-xs">
<SelectValue
placeholder={
isLoadingVersions
? t("navigation:claudeCode.loadingVersions", "Loading versions...")
: versionsError
? t("navigation:claudeCode.failedToLoadVersions", "Failed to load versions")
: t("navigation:claudeCode.selectVersion", "Select version")
}
/>
</SelectTrigger>
<SelectContent>
{availableVersions.map((version) => {
const isCurrentVersion = normalizeVersion(version) === normalizeVersion(versionInfo.installed || '');
return (
<SelectItem
key={version}
value={version}
className="text-xs"
disabled={isCurrentVersion}
>
<span className="font-mono">{version}</span>
{isCurrentVersion && (
<span className="ml-2 text-muted-foreground">
({t("navigation:claudeCode.currentVersion", "Current")})
</span>
)}
</SelectItem>
);
})}
</SelectContent>
</Select>
</div>
)}
{/* CLI Installation selector - show when multiple installations are found */}
{installations.length > 1 && (
<div className="space-y-1.5">
<label className="text-xs text-muted-foreground">
{t("navigation:claudeCode.switchInstallation", "Switch Installation")}
</label>
<Select
value={selectedInstallation || ""}
onValueChange={handleInstallationSelect}
disabled={isLoadingInstallations || isInstalling}
>
<SelectTrigger className="h-8 text-xs">
<SelectValue
placeholder={
isLoadingInstallations
? t("navigation:claudeCode.loadingInstallations", "Loading installations...")
: installationsError
? t("navigation:claudeCode.failedToLoadInstallations", "Failed to load installations")
: t("navigation:claudeCode.selectInstallation", "Select installation")
}
/>
</SelectTrigger>
<SelectContent>
{installations.map((installation) => (
<SelectItem
key={installation.path}
value={installation.path}
className="text-xs"
disabled={installation.isActive}
>
<div className="flex flex-col">
<span className="font-mono text-[10px] truncate max-w-[180px]" title={installation.path}>
{/* Split on both path separators for cross-platform compatibility */}
{installation.path.split(/[/\\]/).slice(-2).join('/') || installation.path}
</span>
<span className="text-muted-foreground text-[9px]">
{installation.version ? `v${installation.version}` : t("navigation:claudeCode.versionUnknown", "version unknown")} ({installation.source})
{installation.isActive && ` - ${t("navigation:claudeCode.activeInstallation", "Active")}`}
</span>
</div>
</SelectItem>
))}
</SelectContent>
</Select>
</div>
)}
{/* Changelog link */}
<Button
variant="link"
size="sm"
className="w-full text-xs text-muted-foreground gap-1"
onClick={() =>
window.electronAPI?.openExternal?.(
"https://github.com/anthropics/claude-code/blob/main/CHANGELOG.md"
)
}
aria-label={t(
"navigation:claudeCode.viewChangelogAriaLabel",
"View Claude Code Changelog (opens in new window)"
)}
>
{t("navigation:claudeCode.viewChangelog", "View Claude Code Changelog")}
<ExternalLink className="h-3 w-3" aria-hidden="true" />
</Button>
</div>
</TooltipTrigger>
<TooltipContent side="right">{getTooltipText()}</TooltipContent>
</Tooltip>
</PopoverContent>
{/* Update warning dialog */}
<AlertDialog open={showUpdateWarning} onOpenChange={setShowUpdateWarning}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>
{t("navigation:claudeCode.updateWarningTitle", "Update Claude Code?")}
</AlertDialogTitle>
<AlertDialogDescription>
{t(
"navigation:claudeCode.updateWarningDescription",
"Updating will close all running Claude Code sessions. Any unsaved work in those sessions may be lost. Make sure to save your work before proceeding."
)}
<span className="block mt-2 font-semibold text-foreground">
{t(
"navigation:claudeCode.updateWarningTerminalNote",
"A terminal window will open to run the installation command. Please wait for the installation to complete before continuing."
)}
</span>
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>{t("common:cancel", "Cancel")}</AlertDialogCancel>
<AlertDialogAction onClick={performInstall}>
{t("navigation:claudeCode.updateAnyway", "Open Terminal & Update")}
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
{/* Version rollback warning dialog */}
<AlertDialog open={showRollbackWarning} onOpenChange={setShowRollbackWarning}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>
{t("navigation:claudeCode.rollbackWarningTitle", "Switch to version {{version}}?", {
version: selectedVersion,
})}
</AlertDialogTitle>
<AlertDialogDescription>
{t(
"navigation:claudeCode.rollbackWarningDescription",
"Switching versions will close all running Claude Code sessions. Any unsaved work in those sessions may be lost. Make sure to save your work before proceeding."
)}
<span className="block mt-2 font-semibold text-foreground">
{t(
"navigation:claudeCode.rollbackWarningTerminalNote",
"A terminal window will open to run the installation command. Please wait for the installation to complete before continuing."
)}
</span>
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel onClick={() => setSelectedVersion(null)}>
{t("common:cancel", "Cancel")}
</AlertDialogCancel>
<AlertDialogAction onClick={performVersionSwitch}>
{t("navigation:claudeCode.switchAnyway", "Open Terminal & Switch")}
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
{/* Path change warning dialog */}
<AlertDialog open={showPathChangeWarning} onOpenChange={setShowPathChangeWarning}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>
{t("navigation:claudeCode.pathChangeWarningTitle", "Switch CLI installation?")}
</AlertDialogTitle>
<AlertDialogDescription>
{t(
"navigation:claudeCode.pathChangeWarningDescription",
"Switching CLI installations will use a different Claude Code binary. Any running sessions will continue using the previous installation until restarted."
)}
<span className="block mt-2 font-mono text-xs break-all">
{selectedInstallation}
</span>
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel onClick={() => setSelectedInstallation(null)}>
{t("common:cancel", "Cancel")}
</AlertDialogCancel>
<AlertDialogAction onClick={performPathSwitch}>
{t("navigation:claudeCode.switchInstallationConfirm", "Switch")}
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
</Popover>
);
}
apps/frontend/src/renderer/components/task-detail/task-review/MergeProgressOverlay.tsx
+226
View File
@@ -0,0 +1,226 @@
import { useState, useRef, useEffect, useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { ChevronDown, ChevronRight, FileCode, AlertTriangle, Loader2, CheckCircle2, XCircle, Clock } from 'lucide-react';
import { Progress } from '../../ui/progress';
import { cn } from '../../../lib/utils';
import type { MergeProgress, MergeLogEntry, MergeLogEntryType } from '../../../../shared/types';
interface MergeProgressOverlayProps {
mergeProgress: MergeProgress | null;
logEntries: MergeLogEntry[];
}
/** Time in ms without a progress update before showing stalled indicator */
const STALL_THRESHOLD_MS = 30000;
const STAGE_TO_I18N_KEY: Record<string, string> = {
analyzing: 'stages.analyzing',
detecting_conflicts: 'stages.detectingConflicts',
resolving: 'stages.resolving',
validating: 'stages.validating',
complete: 'stages.complete',
error: 'stages.error',
stalled: 'stages.stalled',
};
const LOG_TYPE_COLORS: Record<MergeLogEntryType, string> = {
info: 'text-info',
success: 'text-success',
warning: 'text-warning',
error: 'text-destructive',
};
/**
* Overlay component displaying real-time merge progress with a progress bar,
* stage label, conflict counter, current file indicator, and expandable log viewer.
*
* Detects stalled merges when no progress update is received for 30+ seconds.
*/
export function MergeProgressOverlay({ mergeProgress, logEntries }: MergeProgressOverlayProps) {
const { t } = useTranslation(['taskReview']);
const [logsExpanded, setLogsExpanded] = useState(false);
const [isStalled, setIsStalled] = useState(false);
const logContainerRef = useRef<HTMLDivElement>(null);
const stallTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
// Reset stall timer whenever we receive a new progress update
const resetStallTimer = useCallback(() => {
setIsStalled(false);
if (stallTimerRef.current) {
clearTimeout(stallTimerRef.current);
}
stallTimerRef.current = setTimeout(() => {
setIsStalled(true);
}, STALL_THRESHOLD_MS);
}, []);
// Start/reset stall detection when progress updates arrive
useEffect(() => {
if (mergeProgress && mergeProgress.stage !== 'complete' && mergeProgress.stage !== 'error') {
resetStallTimer();
} else {
// Clear timer on terminal states
setIsStalled(false);
if (stallTimerRef.current) {
clearTimeout(stallTimerRef.current);
stallTimerRef.current = null;
}
}
}, [mergeProgress, resetStallTimer]);
// Cleanup stall timer on unmount
useEffect(() => {
return () => {
if (stallTimerRef.current) {
clearTimeout(stallTimerRef.current);
}
};
}, []);
// Auto-scroll log viewer to bottom when new entries arrive
useEffect(() => {
if (logsExpanded && logContainerRef.current) {
logContainerRef.current.scrollTop = logContainerRef.current.scrollHeight;
}
}, [logEntries, logsExpanded]);
if (!mergeProgress) {
return null;
}
const { stage, percent, message, details } = mergeProgress;
const isError = stage === 'error';
const isComplete = stage === 'complete';
// Use stalled stage label when stalled, otherwise use the current stage
const effectiveStage = isStalled && !isError && !isComplete ? 'stalled' : stage;
const stageLabel = STAGE_TO_I18N_KEY[effectiveStage]
? t(`taskReview:mergeProgress.${STAGE_TO_I18N_KEY[effectiveStage]}`)
: message;
const conflictsFound = details?.conflicts_found ?? 0;
const conflictsResolved = details?.conflicts_resolved ?? 0;
const currentFile = details?.current_file;
return (
<div
className={cn(
'rounded-xl border p-4 space-y-3',
isError && 'border-destructive/50 bg-destructive/5',
isComplete && 'border-success/50 bg-success/5',
isStalled && !isError && !isComplete && 'border-warning/50 bg-warning/5',
!isError && !isComplete && !isStalled && 'border-info/50 bg-info/5'
)}
>
{/* Stage label and percentage */}
<div className="flex items-center justify-between">
<div className="flex items-center gap-2">
{isError ? (
<XCircle className="h-4 w-4 text-destructive shrink-0" />
) : isComplete ? (
<CheckCircle2 className="h-4 w-4 text-success shrink-0" />
) : isStalled ? (
<Clock className="h-4 w-4 text-warning shrink-0" />
) : (
<Loader2 className="h-4 w-4 animate-spin text-info shrink-0" />
)}
<span
className={cn(
'text-sm font-medium',
isError && 'text-destructive',
isComplete && 'text-success',
isStalled && !isError && !isComplete && 'text-warning'
)}
>
{stageLabel}
</span>
</div>
<span className="text-sm font-semibold tabular-nums">{percent}%</span>
</div>
{/* Progress bar */}
<Progress
value={percent}
className={cn(
'h-2',
isError && '[&>div]:bg-destructive',
isComplete && '[&>div]:bg-success',
isStalled && !isError && !isComplete && '[&>div]:bg-warning',
!isError && !isComplete && !isStalled && '[&>div]:bg-info'
)}
animated={!isError && !isComplete}
/>
{/* Conflict counter */}
{conflictsFound > 0 && (
<div className="flex items-center gap-2 text-xs text-muted-foreground">
<AlertTriangle className="h-3.5 w-3.5 text-warning shrink-0" />
<span>
{t('taskReview:mergeProgress.conflictCounter', {
found: conflictsFound,
resolved: conflictsResolved,
})}
</span>
</div>
)}
{/* Current file indicator */}
{currentFile && !isComplete && (
<div className="flex items-center gap-2 text-xs text-muted-foreground">
<FileCode className="h-3.5 w-3.5 shrink-0" />
<span className="truncate font-mono" title={currentFile}>
{t('taskReview:mergeProgress.currentFile')}: {currentFile}
</span>
</div>
)}
{/* Completion / error messages */}
{isComplete && (
<p className="text-xs text-success">{t('taskReview:mergeProgress.completionMessage')}</p>
)}
{isError && (
<p className="text-xs text-destructive">{t('taskReview:mergeProgress.errorMessage')}</p>
)}
{/* Expandable log viewer */}
{logEntries.length > 0 && (
<div>
<button
type="button"
onClick={() => setLogsExpanded(!logsExpanded)}
className="flex items-center gap-1 text-xs text-muted-foreground hover:text-foreground transition-colors"
>
{logsExpanded ? (
<ChevronDown className="h-3.5 w-3.5" />
) : (
<ChevronRight className="h-3.5 w-3.5" />
)}
{logsExpanded
? t('taskReview:mergeProgress.hideLogs')
: t('taskReview:mergeProgress.viewLogs')}
</button>
{logsExpanded && (
<div
ref={logContainerRef}
className="mt-2 max-h-48 overflow-y-auto rounded-lg border bg-background/50 p-2 scrollbar-thin scrollbar-thumb-border scrollbar-track-transparent"
>
<div className="space-y-1">
{logEntries.map((entry, idx) => (
<div key={idx} className="flex gap-2 text-xs font-mono">
<span className="text-muted-foreground shrink-0">
{new Date(entry.timestamp).toLocaleTimeString()}
</span>
<span className={cn(LOG_TYPE_COLORS[entry.type])}>
{entry.message}
</span>
</div>
))}
</div>
</div>
)}
</div>
)}
</div>
);
}
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx
+255 -33
View File
@@ -1,3 +1,4 @@
import { useState, useEffect, useRef } from 'react';
import {
GitBranch,
FileCode,
@@ -13,14 +14,17 @@ import {
CheckCircle,
GitCommit,
Code,
Terminal
Terminal,
Info,
CheckCheck
} from 'lucide-react';
import { useTranslation } from 'react-i18next';
import { Button } from '../../ui/button';
import { Checkbox } from '../../ui/checkbox';
import { Tooltip, TooltipContent, TooltipTrigger } from '../../ui/tooltip';
import { cn } from '../../../lib/utils';
import type { WorktreeStatus, MergeConflict, MergeStats, GitConflictInfo, SupportedIDE, SupportedTerminal } from '../../../../shared/types';
import { MergeProgressOverlay } from './MergeProgressOverlay';
import type { WorktreeStatus, MergeConflict, MergeStats, GitConflictInfo, SupportedIDE, SupportedTerminal, MergeProgress, MergeLogEntry, MergeLogEntryType } from '../../../../shared/types';
import { useSettingsStore } from '../../../stores/settings-store';
interface WorkspaceStatusProps {
@@ -106,6 +110,97 @@ export function WorkspaceStatus({
const preferredIDE = settings.preferredIDE || 'vscode';
const preferredTerminal = settings.preferredTerminal || 'system';
// Merge progress state
const [mergeProgress, setMergeProgress] = useState<MergeProgress | null>(null);
const [logEntries, setLogEntries] = useState<MergeLogEntry[]>([]);
const [showOverlay, setShowOverlay] = useState(false);
const prevIsMergingRef = useRef(isMerging);
const mergeStartTimeRef = useRef<number | null>(null);
const minDisplayTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const ipcCleanupRef = useRef<(() => void) | null>(null);
// Reset state when isMerging transitions from false → true
useEffect(() => {
if (isMerging && !prevIsMergingRef.current) {
setMergeProgress(null);
setLogEntries([]);
setShowOverlay(true);
mergeStartTimeRef.current = Date.now();
}
prevIsMergingRef.current = isMerging;
}, [isMerging]);
// Minimum display time: keep overlay visible for at least 500ms after merge ends
useEffect(() => {
if (!isMerging && showOverlay && mergeStartTimeRef.current !== null) {
const elapsed = Date.now() - mergeStartTimeRef.current;
const MIN_DISPLAY_MS = 500;
const remaining = Math.max(0, MIN_DISPLAY_MS - elapsed);
if (remaining > 0) {
minDisplayTimerRef.current = setTimeout(() => {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}, remaining);
} else {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}
}
return () => {
if (minDisplayTimerRef.current) {
clearTimeout(minDisplayTimerRef.current);
minDisplayTimerRef.current = null;
}
};
}, [isMerging, showOverlay]);
// Subscribe to merge progress IPC events
useEffect(() => {
if (!isMerging) return;
const stageToLogType = (stage: string): MergeLogEntryType => {
switch (stage) {
case 'complete': return 'success';
case 'error': return 'error';
case 'resolving': return 'warning';
default: return 'info';
}
};
const cleanup = window.electronAPI.onMergeProgress((_taskId: string, progress: MergeProgress) => {
setMergeProgress(progress);
setLogEntries(prev => [
...prev,
{
timestamp: new Date().toISOString(),
type: stageToLogType(progress.stage),
message: progress.message,
details: progress.details?.current_file,
}
]);
});
// Store cleanup ref so we can call it on unmount even if isMerging changes
ipcCleanupRef.current = cleanup;
return cleanup;
}, [isMerging]);
// Ensure IPC listener cleanup on unmount during active merge
useEffect(() => {
return () => {
if (ipcCleanupRef.current) {
ipcCleanupRef.current();
ipcCleanupRef.current = null;
}
if (minDisplayTimerRef.current) {
clearTimeout(minDisplayTimerRef.current);
}
};
}, []);
const handleOpenInIDE = async () => {
if (!worktreeStatus.worktreePath) return;
try {
@@ -137,6 +232,12 @@ export function WorkspaceStatus({
const uncommittedCount = mergePreview?.uncommittedChanges?.count || 0;
const hasAIConflicts = mergePreview && mergePreview.conflicts.length > 0;
// Conflict scenario detection for better UX messaging
const conflictScenario = mergePreview?.gitConflicts?.scenario;
const alreadyMergedFiles = mergePreview?.gitConflicts?.alreadyMergedFiles || [];
const isAlreadyMerged = conflictScenario === 'already_merged';
const isSuperseded = conflictScenario === 'superseded';
// Check if branch needs rebase (main has advanced since spec was created)
// This requires AI merge even if no explicit file conflicts are detected
const needsRebase = mergePreview?.gitConflicts?.needsRebase;
@@ -177,7 +278,7 @@ export function WorkspaceStatus({
<div className="flex items-center gap-4 text-xs">
<span className="flex items-center gap-1.5 text-muted-foreground">
<FileCode className="h-3.5 w-3.5" />
<span className="font-medium text-foreground">{worktreeStatus.filesChanged || 0}</span> files
<span className="font-medium text-foreground">{worktreeStatus.filesChanged || 0}</span> {t('taskReview:merge.status.files')}
</span>
<span className="flex items-center gap-1.5 text-muted-foreground">
<GitCommit className="h-3.5 w-3.5" />
@@ -267,8 +368,48 @@ export function WorkspaceStatus({
</div>
)}
{/* Already Merged Scenario - Show friendly message when task changes exist in target */}
{mergePreview && isAlreadyMerged && (
<div className="flex items-start gap-2 p-2.5 rounded-lg bg-success/10 border border-success/20">
<CheckCheck className="h-4 w-4 text-success mt-0.5 flex-shrink-0" />
<div className="flex-1 min-w-0">
<p className="text-sm font-medium text-success">
{t('taskReview:merge.alreadyMergedTitle')}
</p>
<p className="text-xs text-muted-foreground mt-0.5">
{t('taskReview:merge.alreadyMergedDescription')}
</p>
{alreadyMergedFiles.length > 0 && alreadyMergedFiles.length <= 5 && (
<div className="mt-2 text-xs text-muted-foreground">
<span className="font-medium">{t('taskReview:merge.matchingFiles')}:</span>
<ul className="mt-1 list-disc list-inside">
{alreadyMergedFiles.map(file => (
<li key={file} className="truncate">{file}</li>
))}
</ul>
</div>
)}
</div>
</div>
)}
{/* Superseded Scenario - Target has newer version of changes */}
{mergePreview && isSuperseded && (
<div className="flex items-start gap-2 p-2.5 rounded-lg bg-info/10 border border-info/20">
<Info className="h-4 w-4 text-info mt-0.5 flex-shrink-0" />
<div className="flex-1 min-w-0">
<p className="text-sm font-medium text-info">
{t('taskReview:merge.supersededTitle')}
</p>
<p className="text-xs text-muted-foreground mt-0.5">
{t('taskReview:merge.supersededDescription')}
</p>
</div>
</div>
)}
{/* Merge Status */}
{mergePreview && (
{mergePreview && !isAlreadyMerged && !isSuperseded && (
<div className={cn(
"flex items-center justify-between p-2.5 rounded-lg border",
hasGitConflicts || isBranchBehind || hasPathMappedMerges
@@ -282,8 +423,8 @@ export function WorkspaceStatus({
<>
<AlertTriangle className="h-4 w-4 text-warning" />
<div>
<span className="text-sm font-medium text-warning">Branch Diverged</span>
<span className="text-xs text-muted-foreground ml-2">AI will resolve</span>
<span className="text-sm font-medium text-warning">{t('taskReview:merge.status.branchDiverged')}</span>
<span className="text-xs text-muted-foreground ml-2">{t('taskReview:merge.status.aiWillResolve')}</span>
</div>
</>
) : isBranchBehind || hasPathMappedMerges ? (
@@ -291,26 +432,26 @@ export function WorkspaceStatus({
<AlertTriangle className="h-4 w-4 text-warning" />
<div>
<span className="text-sm font-medium text-warning">
{hasPathMappedMerges ? 'Files Renamed' : 'Branch Behind'}
{hasPathMappedMerges ? t('taskReview:merge.status.filesRenamed') : t('taskReview:merge.status.branchBehind')}
</span>
<span className="text-xs text-muted-foreground ml-2">
AI will resolve ({hasPathMappedMerges ? `${pathMappedAIMergeCount} files` : `${commitsBehind} commits`})
{t('taskReview:merge.status.aiWillResolve')} ({hasPathMappedMerges ? `${pathMappedAIMergeCount} ${t('taskReview:merge.status.files')}` : `${commitsBehind} commits`})
</span>
</div>
</>
) : !hasAIConflicts ? (
<>
<CheckCircle className="h-4 w-4 text-success" />
<span className="text-sm font-medium text-success">Ready to merge</span>
<span className="text-sm font-medium text-success">{t('taskReview:merge.status.readyToMerge')}</span>
<span className="text-xs text-muted-foreground ml-1">
{mergePreview.summary.totalFiles} files
{mergePreview.summary.totalFiles} {t('taskReview:merge.status.files')}
</span>
</>
) : (
<>
<AlertTriangle className="h-4 w-4 text-warning" />
<span className="text-sm font-medium text-warning">
{mergePreview.conflicts.length} conflict{mergePreview.conflicts.length !== 1 ? 's' : ''}
{mergePreview.conflicts.length} {mergePreview.conflicts.length !== 1 ? t('taskReview:merge.status.conflicts') : t('taskReview:merge.status.conflict')}
</span>
</>
)}
@@ -323,7 +464,7 @@ export function WorkspaceStatus({
onClick={() => onShowConflictDialog(true)}
className="h-7 text-xs"
>
Details
{t('taskReview:merge.status.details')}
</Button>
)}
<Button
@@ -332,7 +473,7 @@ export function WorkspaceStatus({
onClick={onLoadMergePreview}
disabled={isLoadingPreview}
className="h-7 px-2"
title="Refresh"
title={t('taskReview:merge.status.refresh')}
>
{isLoadingPreview ? (
<Loader2 className="h-3.5 w-3.5 animate-spin" />
@@ -344,8 +485,8 @@ export function WorkspaceStatus({
</div>
)}
{/* Git Conflicts Details */}
{hasGitConflicts && mergePreview?.gitConflicts && (
{/* Git Conflicts Details - hide for already_merged/superseded scenarios */}
{hasGitConflicts && mergePreview?.gitConflicts && !isAlreadyMerged && !isSuperseded && (
<div className="text-xs text-muted-foreground pl-6">
{t('taskReview:merge.branchHasNewCommits', { branch: mergePreview.gitConflicts.baseBranch, count: mergePreview.gitConflicts.commitsBehind })}
{mergePreview.gitConflicts.conflictingFiles.length > 0 && (
@@ -357,7 +498,7 @@ export function WorkspaceStatus({
)}
{/* Branch Behind Details (no explicit conflicts but needs AI merge due to path mappings) */}
{!hasGitConflicts && isBranchBehind && mergePreview?.gitConflicts && (
{!hasGitConflicts && isBranchBehind && mergePreview?.gitConflicts && !isAlreadyMerged && !isSuperseded && (
<div className="text-xs text-muted-foreground pl-6">
{t('taskReview:merge.branchHasNewCommitsSinceBuild', { branch: mergePreview.gitConflicts.baseBranch, count: commitsBehind })}
{hasPathMappedMerges ? (
@@ -373,10 +514,15 @@ export function WorkspaceStatus({
)}
</div>
{/* Merge Progress Overlay — shown during merge and for minimum display time after */}
{(isMerging || showOverlay) && (
<MergeProgressOverlay mergeProgress={mergeProgress} logEntries={logEntries} />
)}
{/* Actions Footer */}
<div className="px-4 py-3 bg-muted/20 border-t border-border space-y-3">
{/* Stage Only Option - only show after conflicts have been checked */}
{mergePreview && (
{/* Stage Only Option - only show after conflicts have been checked (not for already_merged/superseded) */}
{mergePreview && !isAlreadyMerged && !isSuperseded && (
<label className="inline-flex items-center gap-2.5 text-sm cursor-pointer select-none px-3 py-2 rounded-lg border border-border bg-background/50 hover:bg-background/80 transition-colors">
<Checkbox
checked={stageOnly}
@@ -386,7 +532,7 @@ export function WorkspaceStatus({
<span className={cn(
"transition-colors",
stageOnly ? "text-foreground" : "text-muted-foreground"
)}>Stage only (review in IDE before committing)</span>
)}>{t('taskReview:merge.status.stageOnly')}</span>
</label>
)}
@@ -417,8 +563,81 @@ export function WorkspaceStatus({
</Button>
)}
{/* State 3: Merge preview loaded - show appropriate merge/stage button */}
{mergePreview && !isLoadingPreview && (
{/* State 3a: Already Merged - show "Mark as Done" as primary action */}
{mergePreview && !isLoadingPreview && isAlreadyMerged && (
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="success"
onClick={onMerge}
disabled={isMerging || isDiscarding}
className="flex-1"
>
{isMerging ? (
<>
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
{t('taskReview:merge.buttons.completing')}
</>
) : (
<>
<CheckCheck className="mr-2 h-4 w-4" />
{t('taskReview:merge.actions.markAsDone')}
</>
)}
</Button>
</TooltipTrigger>
<TooltipContent>
<p className="max-w-xs">
{t('taskReview:merge.alreadyMergedTooltip')}
</p>
</TooltipContent>
</Tooltip>
)}
{/* State 3b: Superseded - show both "View Comparison" and "Discard" */}
{mergePreview && !isLoadingPreview && isSuperseded && (
<>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="outline"
onClick={() => onShowConflictDialog(true)}
disabled={isMerging || isDiscarding}
className="flex-1"
>
<Eye className="mr-2 h-4 w-4" />
{t('taskReview:merge.actions.viewComparison')}
</Button>
</TooltipTrigger>
<TooltipContent>
<p className="max-w-xs">
{t('taskReview:merge.supersededCompareTooltip')}
</p>
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="destructive"
onClick={() => onShowDiscardDialog(true)}
disabled={isMerging || isDiscarding}
className="flex-1"
>
<FolderX className="mr-2 h-4 w-4" />
{t('taskReview:merge.actions.discardTask')}
</Button>
</TooltipTrigger>
<TooltipContent>
<p className="max-w-xs">
{t('taskReview:merge.supersededDiscardTooltip')}
</p>
</TooltipContent>
</Tooltip>
</>
)}
{/* State 3c: Normal merge - show appropriate merge/stage button */}
{mergePreview && !isLoadingPreview && !isAlreadyMerged && !isSuperseded && (
<Tooltip>
<TooltipTrigger asChild>
<Button
@@ -456,8 +675,8 @@ export function WorkspaceStatus({
</Tooltip>
)}
{/* Create PR Button */}
{onShowPRDialog && (
{/* Create PR Button - hide for already_merged/superseded scenarios */}
{onShowPRDialog && !isAlreadyMerged && !isSuperseded && (
<Button
variant="info"
onClick={() => onShowPRDialog(true)}
@@ -478,16 +697,19 @@ export function WorkspaceStatus({
</Button>
)}
<Button
variant="outline"
size="icon"
onClick={() => onShowDiscardDialog(true)}
disabled={isMerging || isDiscarding || isCreatingPR}
className="text-muted-foreground hover:text-destructive hover:bg-destructive/10 hover:border-destructive/30"
title="Discard build"
>
<FolderX className="h-4 w-4" />
</Button>
{/* Discard button - hide for superseded (shown as primary action there) */}
{!isSuperseded && (
<Button
variant="outline"
size="icon"
onClick={() => onShowDiscardDialog(true)}
disabled={isMerging || isDiscarding || isCreatingPR}
className="text-muted-foreground hover:text-destructive hover:bg-destructive/10 hover:border-destructive/30"
title={t('taskReview:merge.status.discardBuild')}
>
<FolderX className="h-4 w-4" />
</Button>
)}
</div>
</div>
</div>
apps/frontend/src/renderer/lib/mocks/task-mock.ts
+2 -1
View File
@@ -97,5 +97,6 @@ export const taskMock = {
onTaskStatusChange: () => () => {},
onTaskExecutionProgress: () => () => {},
onTaskLogsChanged: () => () => {},
onTaskLogsStream: () => () => {}
onTaskLogsStream: () => () => {},
onMergeProgress: () => () => {}
};
apps/frontend/src/shared/constants/ipc.ts
+1
View File
@@ -62,6 +62,7 @@ export const IPC_CHANNELS = {
TASK_LOGS_UNWATCH: 'task:logsUnwatch', // Stop watching for log changes
TASK_LOGS_CHANGED: 'task:logsChanged', // Event: logs changed (main -> renderer)
TASK_LOGS_STREAM: 'task:logsStream', // Event: streaming log chunk (main -> renderer)
TASK_MERGE_PROGRESS: 'task:mergeProgress', // Event: merge progress update (main -> renderer)
// Terminal operations
TERMINAL_CREATE: 'terminal:create',
apps/frontend/src/shared/i18n/locales/en/navigation.json
-1
View File
@@ -44,7 +44,6 @@
"claudeCode": {
"checking": "Checking Claude Code...",
"upToDate": "Claude Code is up to date",
"upToDateWithVersion": "Claude Code {{version}} installed",
"updateAvailable": "Claude Code update available",
"notInstalled": "Claude Code not installed",
"error": "Error checking Claude Code",
apps/frontend/src/shared/i18n/locales/en/taskReview.json
+54 -1
View File
@@ -22,6 +22,29 @@
"fileRenamesDetected": "{{count}} file rename detected - AI will handle the merge.",
"fileRenamesDetected_other": "{{count}} file renames detected - AI will handle the merge.",
"filesRenamedOrMoved": "Files may have been renamed or moved - AI will handle the merge.",
"alreadyMergedTitle": "Changes already in your branch",
"alreadyMergedDescription": "These changes appear to already exist in your current branch. You can safely mark this task as done.",
"alreadyMergedTooltip": "The task's changes are already present in your branch. Marking as done will clean up the worktree without merging.",
"matchingFiles": "Matching files",
"supersededTitle": "Changes superseded",
"supersededDescription": "Your current branch has a newer version of these changes. Consider discarding this task or viewing the comparison.",
"supersededCompareTooltip": "View a detailed comparison to see how the current branch differs from this task's changes.",
"supersededDiscardTooltip": "Remove this task's worktree since the changes are no longer needed.",
"status": {
"branchDiverged": "Branch Diverged",
"aiWillResolve": "AI will resolve",
"filesRenamed": "Files Renamed",
"branchBehind": "Branch Behind",
"readyToMerge": "Ready to merge",
"files": "files",
"file": "file",
"conflict": "conflict",
"conflicts": "conflicts",
"details": "Details",
"refresh": "Refresh",
"stageOnly": "Stage only (review in IDE before committing)",
"discardBuild": "Discard build"
},
"buttons": {
"stageWithAIMerge": "Stage with AI Merge",
"mergeWithAI": "Merge with AI",
@@ -29,7 +52,13 @@
"mergeTo": "Merge to {{branch}}",
"resolving": "Resolving...",
"staging": "Staging...",
"merging": "Merging..."
"merging": "Merging...",
"completing": "Completing..."
},
"actions": {
"markAsDone": "Mark as Done",
"discardTask": "Discard Task",
"viewComparison": "View Comparison"
}
},
"pr": {
@@ -63,6 +92,30 @@
"prTitle": "Leave empty to use the task title"
}
},
"mergeProgress": {
"stages": {
"analyzing": "Analyzing changes",
"detectingConflicts": "Detecting conflicts",
"resolving": "Resolving conflicts",
"validating": "Validating merge",
"complete": "Merge complete",
"error": "Merge failed",
"stalled": "Merge stalled"
},
"conflictCounter": "{{found}} found, {{resolved}} resolved",
"currentFile": "Current file",
"viewLogs": "View logs",
"hideLogs": "Hide logs",
"logTypes": {
"info": "Info",
"warning": "Warning",
"error": "Error",
"conflict": "Conflict",
"resolution": "Resolution"
},
"completionMessage": "All changes have been merged successfully.",
"errorMessage": "An error occurred during the merge process."
},
"bulkPR": {
"title": "Create Pull Requests",
"description": "Create pull requests for {{count}} selected tasks",
apps/frontend/src/shared/i18n/locales/fr/navigation.json
-1
View File
@@ -44,7 +44,6 @@
"claudeCode": {
"checking": "Vérification de Claude Code...",
"upToDate": "Claude Code est à jour",
"upToDateWithVersion": "Claude Code {{version}} installé",
"updateAvailable": "Mise à jour Claude Code disponible",
"notInstalled": "Claude Code non installé",
"error": "Erreur de vérification de Claude Code",
apps/frontend/src/shared/i18n/locales/fr/taskReview.json
+54 -1
View File
@@ -22,6 +22,29 @@
"fileRenamesDetected": "{{count}} renommage de fichier détecté - l'IA gérera la fusion.",
"fileRenamesDetected_other": "{{count}} renommages de fichiers détectés - l'IA gérera la fusion.",
"filesRenamedOrMoved": "Des fichiers ont peut-être été renommés ou déplacés - l'IA gérera la fusion.",
"alreadyMergedTitle": "Modifications déjà dans votre branche",
"alreadyMergedDescription": "Ces modifications semblent déjà exister dans votre branche actuelle. Vous pouvez marquer cette tâche comme terminée en toute sécurité.",
"alreadyMergedTooltip": "Les modifications de la tâche sont déjà présentes dans votre branche. Marquer comme terminé nettoiera le worktree sans fusionner.",
"matchingFiles": "Fichiers correspondants",
"supersededTitle": "Modifications remplacées",
"supersededDescription": "Votre branche actuelle a une version plus récente de ces modifications. Envisagez de supprimer cette tâche ou de voir la comparaison.",
"supersededCompareTooltip": "Voir une comparaison détaillée pour voir comment la branche actuelle diffère des modifications de cette tâche.",
"supersededDiscardTooltip": "Supprimer le worktree de cette tâche puisque les modifications ne sont plus nécessaires.",
"status": {
"branchDiverged": "Branche divergée",
"aiWillResolve": "L'IA résoudra",
"filesRenamed": "Fichiers renommés",
"branchBehind": "Branche en retard",
"readyToMerge": "Prêt à fusionner",
"files": "fichiers",
"file": "fichier",
"conflict": "conflit",
"conflicts": "conflits",
"details": "Détails",
"refresh": "Actualiser",
"stageOnly": "Préparer seulement (réviser dans l'IDE avant de commiter)",
"discardBuild": "Supprimer le build"
},
"buttons": {
"stageWithAIMerge": "Préparer avec fusion IA",
"mergeWithAI": "Fusionner avec IA",
@@ -29,7 +52,13 @@
"mergeTo": "Fusionner vers {{branch}}",
"resolving": "Résolution...",
"staging": "Préparation...",
"merging": "Fusion..."
"merging": "Fusion...",
"completing": "Finalisation..."
},
"actions": {
"markAsDone": "Marquer comme terminé",
"discardTask": "Supprimer la tâche",
"viewComparison": "Voir la comparaison"
}
},
"pr": {
@@ -63,6 +92,30 @@
"prTitle": "Laissez vide pour utiliser le titre de la tâche"
}
},
"mergeProgress": {
"stages": {
"analyzing": "Analyse des modifications",
"detectingConflicts": "Détection des conflits",
"resolving": "Résolution des conflits",
"validating": "Validation de la fusion",
"complete": "Fusion terminée",
"error": "Échec de la fusion",
"stalled": "Fusion bloquée"
},
"conflictCounter": "{{found}} trouvés, {{resolved}} résolus",
"currentFile": "Fichier actuel",
"viewLogs": "Voir les logs",
"hideLogs": "Masquer les logs",
"logTypes": {
"info": "Info",
"warning": "Avertissement",
"error": "Erreur",
"conflict": "Conflit",
"resolution": "Résolution"
},
"completionMessage": "Toutes les modifications ont été fusionnées avec succès.",
"errorMessage": "Une erreur s'est produite pendant le processus de fusion."
},
"bulkPR": {
"title": "Créer des Pull Requests",
"description": "Créer des pull requests pour {{count}} tâches sélectionnées",
apps/frontend/src/shared/types/ipc.ts
+7 -3
View File
@@ -3,6 +3,7 @@
*/
import type { IPCResult } from './common';
import type { KanbanPreferences } from './kanban';
import type { SupportedIDE, SupportedTerminal } from './settings';
import type {
Project,
@@ -44,7 +45,8 @@ import type {
TaskMetadata,
TaskLogs,
TaskLogStreamChunk,
ImageAttachment
ImageAttachment,
MergeProgress
} from './task';
import type {
TerminalCreateOptions,
@@ -136,7 +138,6 @@ import type {
GitLabNewCommitsCheck
} from './integrations';
import type { APIProfile, ProfilesFile, TestConnectionResult, DiscoverModelsResult } from './profile';
import type { KanbanPreferences } from './kanban';
// Electron API exposed via contextBridge
// Tab state interface (persisted in main process)
@@ -159,7 +160,7 @@ export interface ElectronAPI {
getTabState: () => Promise<IPCResult<TabState>>;
saveTabState: (tabState: TabState) => Promise<IPCResult>;
// Kanban Preferences (persisted in main process per project)
// Kanban preferences (per-project column collapse state)
getKanbanPreferences: (projectId: string) => Promise<IPCResult<KanbanPreferences | null>>;
saveKanbanPreferences: (projectId: string, preferences: KanbanPreferences) => Promise<IPCResult>;
@@ -771,6 +772,9 @@ export interface ElectronAPI {
onTaskLogsStream: (
callback: (specId: string, chunk: TaskLogStreamChunk) => void
) => () => void;
onMergeProgress: (
callback: (taskId: string, progress: MergeProgress) => void
) => () => void;
// File explorer operations
listDirectory: (dirPath: string) => Promise<IPCResult<FileNode[]>>;
apps/frontend/src/shared/types/task.ts
+41 -4
View File
@@ -352,6 +352,13 @@ export interface PathMappedAIMerge {
reason: string;
}
// Conflict scenario types for better UX messaging
// - 'already_merged': Task changes already identical in target branch
// - 'superseded': Target has newer version of same feature
// - 'diverged': Standard diverged branches (AI can resolve)
// - 'normal_conflict': Actual conflicting changes
export type ConflictScenario = 'already_merged' | 'superseded' | 'diverged' | 'normal_conflict';
// Git-level conflict information (branch divergence)
export interface GitConflictInfo {
hasConflicts: boolean;
@@ -364,6 +371,12 @@ export interface GitConflictInfo {
pathMappedAIMerges?: PathMappedAIMerge[];
// Total number of file renames detected
totalRenames?: number;
// Conflict scenario for better UX messaging
scenario?: ConflictScenario;
// Files that are already merged (identical in both branches)
alreadyMergedFiles?: string[];
// Human-readable message about the scenario
scenarioMessage?: string;
}
// Summary statistics from merge preview/execution
@@ -379,6 +392,30 @@ export interface MergeStats {
pathMappedAIMergeCount?: number;
}
// Merge progress tracking (for progress bar during merge operations)
export type MergeStage = 'analyzing' | 'detecting_conflicts' | 'resolving' | 'validating' | 'complete' | 'error';
export interface MergeProgress {
stage: MergeStage;
percent: number;
message: string;
details?: {
conflicts_found?: number;
conflicts_resolved?: number;
current_file?: string;
};
}
// Merge log entry (for conflict resolution logging)
export type MergeLogEntryType = 'info' | 'success' | 'warning' | 'error';
export interface MergeLogEntry {
timestamp: string;
type: MergeLogEntryType;
message: string;
details?: string;
}
export interface WorktreeMergeResult {
success: boolean;
message: string;
@@ -442,10 +479,10 @@ export interface WorktreeListItem {
path: string;
branch: string;
baseBranch: string;
commitCount?: number;
filesChanged?: number;
additions?: number;
deletions?: number;
commitCount: number;
filesChanged: number;
additions: number;
deletions: number;
}
/**
tests/test_integration_phase4.py
+103 -95
View File
@@ -27,20 +27,18 @@ import importlib.util
# Load file_lock first (models.py depends on it)
file_lock_spec = importlib.util.spec_from_file_location(
"file_lock",
backend_path / "runners" / "github" / "file_lock.py"
"file_lock", backend_path / "runners" / "github" / "file_lock.py"
)
file_lock_module = importlib.util.module_from_spec(file_lock_spec)
sys.modules['file_lock'] = file_lock_module
sys.modules["file_lock"] = file_lock_module
file_lock_spec.loader.exec_module(file_lock_module)
# Load models next
models_spec = importlib.util.spec_from_file_location(
"models",
backend_path / "runners" / "github" / "models.py"
"models", backend_path / "runners" / "github" / "models.py"
)
models_module = importlib.util.module_from_spec(models_spec)
sys.modules['models'] = models_module
sys.modules["models"] = models_module
models_spec.loader.exec_module(models_module)
PRReviewFinding = models_module.PRReviewFinding
PRReviewResult = models_module.PRReviewResult
@@ -50,56 +48,58 @@ ReviewCategory = models_module.ReviewCategory
# Load services module dependencies for parallel_orchestrator_reviewer
category_utils_spec = importlib.util.spec_from_file_location(
"category_utils",
backend_path / "runners" / "github" / "services" / "category_utils.py"
backend_path / "runners" / "github" / "services" / "category_utils.py",
)
category_utils_module = importlib.util.module_from_spec(category_utils_spec)
sys.modules['services.category_utils'] = category_utils_module
sys.modules["services.category_utils"] = category_utils_module
category_utils_spec.loader.exec_module(category_utils_module)
# Load io_utils
io_utils_spec = importlib.util.spec_from_file_location(
"io_utils",
backend_path / "runners" / "github" / "services" / "io_utils.py"
"io_utils", backend_path / "runners" / "github" / "services" / "io_utils.py"
)
io_utils_module = importlib.util.module_from_spec(io_utils_spec)
sys.modules['services.io_utils'] = io_utils_module
sys.modules["services.io_utils"] = io_utils_module
io_utils_spec.loader.exec_module(io_utils_module)
# Load pydantic_models
pydantic_models_spec = importlib.util.spec_from_file_location(
"pydantic_models",
backend_path / "runners" / "github" / "services" / "pydantic_models.py"
backend_path / "runners" / "github" / "services" / "pydantic_models.py",
)
pydantic_models_module = importlib.util.module_from_spec(pydantic_models_spec)
sys.modules['services.pydantic_models'] = pydantic_models_module
sys.modules["services.pydantic_models"] = pydantic_models_module
pydantic_models_spec.loader.exec_module(pydantic_models_module)
AgentAgreement = pydantic_models_module.AgentAgreement
# Load agent_utils (shared utility for working directory injection)
agent_utils_spec = importlib.util.spec_from_file_location(
"agent_utils",
backend_path / "runners" / "github" / "services" / "agent_utils.py"
"agent_utils", backend_path / "runners" / "github" / "services" / "agent_utils.py"
)
agent_utils_module = importlib.util.module_from_spec(agent_utils_spec)
sys.modules['services.agent_utils'] = agent_utils_module
sys.modules["services.agent_utils"] = agent_utils_module
agent_utils_spec.loader.exec_module(agent_utils_module)
# Load parallel_orchestrator_reviewer (contains _is_finding_in_scope and _cross_validate_findings)
orchestrator_spec = importlib.util.spec_from_file_location(
"parallel_orchestrator_reviewer",
backend_path / "runners" / "github" / "services" / "parallel_orchestrator_reviewer.py"
backend_path
/ "runners"
/ "github"
/ "services"
/ "parallel_orchestrator_reviewer.py",
)
orchestrator_module = importlib.util.module_from_spec(orchestrator_spec)
# Mock dependencies that aren't needed for unit testing
# IMPORTANT: Save and restore ALL mocked modules to avoid polluting sys.modules for other tests
_modules_to_mock = [
'context_gatherer',
'core.client',
'gh_client',
'phase_config',
'services.pr_worktree_manager',
'services.sdk_utils',
'claude_agent_sdk',
"context_gatherer",
"core.client",
"gh_client",
"phase_config",
"services.pr_worktree_manager",
"services.sdk_utils",
"claude_agent_sdk",
]
_original_modules = {name: sys.modules.get(name) for name in _modules_to_mock}
for name in _modules_to_mock:
@@ -120,6 +120,7 @@ _is_finding_in_scope = orchestrator_module._is_finding_in_scope
# Phase 5+ Tests: Scope Filtering (Updated)
# =============================================================================
class TestScopeFiltering:
"""Test scope filtering logic (updated for Phase 5 - uses is_impact_finding schema field)."""
@@ -132,11 +133,12 @@ class TestScopeFiltering:
ParallelOrchestratorFinding Pydantic model. The actual code uses
getattr(finding, 'is_impact_finding', False) to access it.
"""
def _make_finding(
file: str = "src/test.py",
line: int = 10,
is_impact_finding: bool = False,
**kwargs
**kwargs,
):
defaults = {
"id": "TEST001",
@@ -152,6 +154,7 @@ class TestScopeFiltering:
# Set is_impact_finding as attribute (accessed via getattr in _is_finding_in_scope)
finding.is_impact_finding = is_impact_finding
return finding
return _make_finding
def test_finding_in_changed_files_passes(self, make_finding):
@@ -166,9 +169,7 @@ class TestScopeFiltering:
"""Finding for a file NOT in changed_files should be filtered."""
changed_files = ["src/auth.py", "src/utils.py"]
finding = make_finding(
file="src/database.py",
line=10,
description="This code has a bug"
file="src/database.py", line=10, description="This code has a bug"
)
is_valid, reason = _is_finding_in_scope(finding, changed_files)
@@ -199,7 +200,7 @@ class TestScopeFiltering:
file="src/utils.py",
line=10,
is_impact_finding=True, # Schema field replaces keyword detection
description="This change breaks the helper function in utils.py"
description="This change breaks the helper function in utils.py",
)
is_valid, _ = _is_finding_in_scope(finding, changed_files)
assert is_valid
@@ -213,7 +214,7 @@ class TestScopeFiltering:
file="src/database.py",
line=20,
is_impact_finding=False,
description="database.py depends on modified auth module"
description="database.py depends on modified auth module",
)
is_valid, reason = _is_finding_in_scope(finding, changed_files)
assert not is_valid
@@ -248,15 +249,14 @@ github_dir = backend_path / "runners" / "github"
# Load context_gatherer module directly using spec loader
# This avoids the complex package import chain
_cg_spec = importlib.util.spec_from_file_location(
"context_gatherer_isolated",
github_dir / "context_gatherer.py"
"context_gatherer_isolated", github_dir / "context_gatherer.py"
)
_cg_module = importlib.util.module_from_spec(_cg_spec)
# Set up minimal module environment
sys.modules['context_gatherer_isolated'] = _cg_module
sys.modules["context_gatherer_isolated"] = _cg_module
# Mock only the gh_client dependency
_mock_gh = MagicMock()
sys.modules['gh_client'] = _mock_gh
sys.modules["gh_client"] = _mock_gh
_cg_spec.loader.exec_module(_cg_module)
PRContextGathererIsolated = _cg_module.PRContextGatherer
@@ -278,7 +278,9 @@ class TestImportDetection:
(src_dir / "config.ts").write_text("export const config = { debug: true };")
# Create index.ts that re-exports
(src_dir / "index.ts").write_text("export * from './utils';\nexport { config } from './config';")
(src_dir / "index.ts").write_text(
"export * from './utils';\nexport { config } from './config';"
)
# Create shared directory
shared_dir = src_dir / "shared"
@@ -286,7 +288,9 @@ class TestImportDetection:
(shared_dir / "types.ts").write_text("export type User = { id: string };")
# Create Python module
(src_dir / "python_module.py").write_text("from .helpers import util_func\nimport os")
(src_dir / "python_module.py").write_text(
"from .helpers import util_func\nimport os"
)
(src_dir / "helpers.py").write_text("def util_func(): pass")
(src_dir / "__init__.py").write_text("")
@@ -295,19 +299,19 @@ class TestImportDetection:
def test_path_alias_detection(self, temp_project):
"""Path alias imports (@/utils) should be detected and resolved."""
import json
# Create tsconfig.json with path aliases
tsconfig = {
"compilerOptions": {
"paths": {
"@/*": ["src/*"],
"@shared/*": ["src/shared/*"]
}
"paths": {"@/*": ["src/*"], "@shared/*": ["src/shared/*"]}
}
}
(temp_project / "tsconfig.json").write_text(json.dumps(tsconfig))
# Create the target file that the alias points to
(temp_project / "src" / "utils.ts").write_text("export const helper = () => {};")
(temp_project / "src" / "utils.ts").write_text(
"export const helper = () => {};"
)
# Test file with alias import
test_content = "import { helper } from '@/utils';"
@@ -322,7 +326,9 @@ class TestImportDetection:
assert isinstance(imports, set)
# Normalize paths for cross-platform comparison (Windows uses backslashes)
normalized_imports = {p.replace("\\", "/") for p in imports}
assert "src/utils.ts" in normalized_imports, f"Expected 'src/utils.ts' in imports, got: {imports}"
assert "src/utils.ts" in normalized_imports, (
f"Expected 'src/utils.ts' in imports, got: {imports}"
)
def test_commonjs_require_detection(self, temp_project):
"""CommonJS require('./utils') should be detected."""
@@ -384,7 +390,18 @@ class TestImportDetection:
class TestReverseDepDetection:
"""Test reverse dependency detection (Phase 2)."""
"""Test reverse dependency detection (Phase 2).
ARCHITECTURE NOTE (2025-01): These tests document that programmatic file scanning
has been intentionally removed. The _find_dependents() method now returns an empty
set because LLM agents handle file discovery via their tools (Glob, Grep, Read).
This design change:
- Removes the legacy 2000 file scan limit
- Lets LLM agents use their judgment to find relevant files
- Avoids pre-loading context that may not be needed
- Scales better for large codebases
"""
@pytest.fixture
def temp_project_with_deps(self, tmp_path):
@@ -392,7 +409,7 @@ class TestReverseDepDetection:
src_dir = tmp_path / "src"
src_dir.mkdir()
# Create a utility file with non-generic name (helpers is in skip list)
# Create a utility file with non-generic name
(src_dir / "formatter.ts").write_text(
"export function format(s: string) { return s; }"
)
@@ -405,63 +422,49 @@ class TestReverseDepDetection:
"import { format } from './formatter';\nexport const fetch = () => {};"
)
# Create a file that does NOT import formatter
(src_dir / "standalone.ts").write_text(
"export const standalone = () => {};"
)
return tmp_path
def test_finds_files_importing_changed_file(self, temp_project_with_deps):
"""Verify grep-based detection finds files that import a given file."""
def test_find_dependents_returns_empty_set(self, temp_project_with_deps):
"""_find_dependents() returns empty - LLM agents discover files via tools.
This is intentional: programmatic file scanning was removed in favor of
letting LLM agents use Glob/Grep/Read tools to discover relevant files
based on the PR context they receive.
"""
gatherer = PRContextGathererIsolated(temp_project_with_deps, pr_number=1)
# Use non-generic name (helpers is in the skip list)
dependents = gatherer._find_dependents("src/formatter.ts", max_results=10)
# Should find auth.ts and api.ts as dependents
assert any("auth.ts" in d for d in dependents)
assert any("api.ts" in d for d in dependents)
# Should NOT include standalone.ts
assert not any("standalone.ts" in d for d in dependents)
# Method now intentionally returns empty set
assert dependents == set()
def test_generic_names_not_skipped(self, tmp_path):
"""Generic names (index, main, utils) are no longer skipped - LLM decides relevance."""
def test_find_dependents_empty_for_any_file(self, tmp_path):
"""Verify _find_dependents() returns empty for any input.
The LLM-driven architecture means agents decide what's relevant,
not programmatic scanning.
"""
src_dir = tmp_path / "src"
src_dir.mkdir()
# Create files with generic names
(src_dir / "index.ts").write_text("export * from './utils';")
(src_dir / "main.ts").write_text("import { x } from './index';")
gatherer = PRContextGathererIsolated(tmp_path, pr_number=1)
dependents = gatherer._find_dependents("src/index.ts")
# Generic names should NOT be skipped anymore (behavior changed in Phase 4)
# The LLM-driven system decides what's relevant based on PR context
dependents_index = gatherer._find_dependents("src/index.ts")
# Returns empty - LLM agents handle file discovery
assert dependents == set()
# main.ts imports index, so it should be found as a dependent
assert "src/main.ts" in dependents_index
def test_respects_file_limit(self, tmp_path):
"""Large repo search should stop after reaching file limit."""
def test_find_dependents_returns_set_type(self, tmp_path):
"""Verify _find_dependents() returns correct type (set)."""
src_dir = tmp_path / "src"
src_dir.mkdir()
(src_dir / "unique_name.ts").write_text("export const x = 1;")
(src_dir / "file.ts").write_text("export const x = 1;")
gatherer = PRContextGathererIsolated(tmp_path, pr_number=1)
dependents = gatherer._find_dependents("src/file.ts")
# Mock os.walk to generate more files than the limit (2000)
# This simulates a large codebase without creating actual files
def mock_walk(path):
# Yield a directory with 3000 TypeScript files
yield (str(path), [], [f"file{i}.ts" for i in range(3000)])
with patch.object(_cg_module.os, "walk", mock_walk):
# The function should stop after max_files_to_check (2000) files
# and return gracefully without hanging
dependents = gatherer._find_dependents("src/unique_name.ts")
# Should return a set (may be empty since mock files don't contain imports)
# Should return a set (empty, but correct type)
assert isinstance(dependents, set)
@@ -479,6 +482,7 @@ class TestCrossValidation:
@pytest.fixture
def make_finding(self):
"""Factory fixture to create PRReviewFinding instances."""
def _make_finding(
id: str = "TEST001",
file: str = "src/test.py",
@@ -487,7 +491,7 @@ class TestCrossValidation:
severity: ReviewSeverity = ReviewSeverity.HIGH,
confidence: float = 0.7,
source_agents: list = None,
**kwargs
**kwargs,
):
return PRReviewFinding(
id=id,
@@ -499,8 +503,11 @@ class TestCrossValidation:
line=line,
confidence=confidence,
source_agents=source_agents or [],
**{k: v for k, v in kwargs.items() if k not in ["title", "description"]}
**{
k: v for k, v in kwargs.items() if k not in ["title", "description"]
},
)
return _make_finding
@pytest.fixture
@@ -508,18 +515,13 @@ class TestCrossValidation:
"""Create a mock ParallelOrchestratorReviewer instance."""
from models import GitHubRunnerConfig
config = GitHubRunnerConfig(
token="test-token",
repo="test/repo"
)
config = GitHubRunnerConfig(token="test-token", repo="test/repo")
# Create minimal directory structure
github_dir = tmp_path / ".auto-claude" / "github"
github_dir.mkdir(parents=True)
reviewer = ParallelOrchestratorReviewer(
project_dir=tmp_path,
github_dir=github_dir,
config=config
project_dir=tmp_path, github_dir=github_dir, config=config
)
return reviewer
@@ -533,7 +535,7 @@ class TestCrossValidation:
category=ReviewCategory.SECURITY,
confidence=0.7,
source_agents=["security-reviewer"],
description="SQL injection risk"
description="SQL injection risk",
)
finding2 = make_finding(
id="F2",
@@ -542,10 +544,12 @@ class TestCrossValidation:
category=ReviewCategory.SECURITY,
confidence=0.6,
source_agents=["quality-reviewer"],
description="Input not sanitized"
description="Input not sanitized",
)
validated, agreement = mock_reviewer._cross_validate_findings([finding1, finding2])
validated, agreement = mock_reviewer._cross_validate_findings(
[finding1, finding2]
)
# Should merge into one finding
assert len(validated) == 1
@@ -582,8 +586,12 @@ class TestCrossValidation:
def test_merged_finding_has_cross_validated_true(self, make_finding, mock_reviewer):
"""Merged multi-agent findings should have cross_validated=True."""
finding1 = make_finding(id="F1", file="src/test.py", line=5, source_agents=["agent1"])
finding2 = make_finding(id="F2", file="src/test.py", line=5, source_agents=["agent2"])
finding1 = make_finding(
id="F1", file="src/test.py", line=5, source_agents=["agent1"]
)
finding2 = make_finding(
id="F2", file="src/test.py", line=5, source_agents=["agent2"]
)
validated, _ = mock_reviewer._cross_validate_findings([finding1, finding2])