From 26beefe39d44d13d24cc87282709d63f489e67d7 Mon Sep 17 00:00:00 2001 From: Andy <119136210+AndyMik90@users.noreply.github.com> Date: Fri, 26 Dec 2025 13:42:45 +0100 Subject: [PATCH] feat(merge,oauth): add path-aware AI merge resolution and device code streaming (#296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * improve/merge-confclit-layer * improve AI resolution * fix caching on merge conflicts * imrpove merge layer with rebase * fix(github): add OAuth authentication to follow-up PR review The follow-up PR review AI analysis was failing with "Could not resolve authentication method" because AsyncAnthropic() was instantiated without credentials. The codebase uses OAuth tokens (not ANTHROPIC_API_KEY), so the client needs the auth_token parameter. Uses get_auth_token() from core.auth to retrieve the OAuth token from environment variables or macOS Keychain, matching how initial reviews authenticate via create_client(). * fix(merge): add validation to prevent AI writing natural language to files When AI merge receives truncated file contents (due to character limits), it sometimes responds with explanations like "I need to see the complete file contents..." instead of actual merged code. This garbage was being written directly to source files. Adds two validation layers after AI merge: 1. Natural language detection - catches patterns like "I need to", "Let me" 2. Syntax validation - uses esbuild to verify TypeScript/JavaScript syntax If either validation fails, the merge returns an error instead of writing invalid content to the file. Also adds project_dir field to ParallelMergeTask to enable syntax validation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix(merge): skip git merge when AI already resolved path-mapped files When AI successfully merges path-mapped files (due to file renames between branches), the check only looked at `conflicts_resolved` which was 0 for path-mapped cases. This caused the code to fall through to `git merge` which then failed with conflicts. Now also checks `files_merged` and `ai_assisted` stats to determine if AI has already handled the merge. When files are AI-merged, they're already written and staged - no need for git merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix device code issue with github * fix(frontend): remember GitHub auth method (OAuth vs PAT) in settings Previously, after authenticating via GitHub OAuth, the settings page would show "Personal Access Token" input even though OAuth was used. This was confusing for users who expected to see their OAuth status. Added githubAuthMethod field to track how authentication was performed. Settings UI now shows "Authenticated via GitHub OAuth" when OAuth was used, with option to switch to manual token if needed. The auth method persists across settings reopening. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * refactor(backend): centralize OAuth client creation in core/client.py - Add create_message_client() for simple message API calls - Refactor followup_reviewer.py to use centralized client factory - Remove direct anthropic.AsyncAnthropic import from followup_reviewer - Add proper ValueError handling for missing OAuth token - Update docstrings to document both client factories This ensures all AI interactions use the centralized OAuth authentication in core/, avoiding direct ANTHROPIC_API_KEY usage per CLAUDE.md guidelines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix(frontend): remove unused statusColor variable in WorkspaceStatus Dead code cleanup - the statusColor variable was computed but never used in the component. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix(tests): add BrowserWindow mock to oauth-handlers tests The sendDeviceCodeToRenderer function uses BrowserWindow.getAllWindows() which wasn't mocked, causing unhandled rejection errors in tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix(backend): use ClaudeSDKClient instead of raw anthropic SDK Remove direct anthropic SDK import from core/client.py and update followup_reviewer.py to use ClaudeSDKClient directly as per project conventions. All AI interactions should use claude-agent-sdk. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * fix(backend): add debug logging for AI response in followup_reviewer Add logging to diagnose why AI review returns no JSON - helps identify if response is in thinking blocks vs text blocks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- apps/backend/cli/workspace_commands.py | 69 ++++- apps/backend/core/client.py | 4 + apps/backend/core/workspace.py | 264 ++++++++++++++++-- apps/backend/core/workspace/git_utils.py | 105 +++++++ apps/backend/core/workspace/models.py | 1 + .../github/services/followup_reviewer.py | 52 +++- .../github/__tests__/oauth-handlers.spec.ts | 12 +- .../ipc-handlers/github/oauth-handlers.ts | 22 +- .../src/preload/api/modules/github-api.ts | 11 + apps/frontend/src/renderer/App.tsx | 4 +- .../renderer/components/GitHubSetupModal.tsx | 5 +- .../GitHubIntegrationSection.tsx | 36 ++- .../project-settings/GitHubOAuthFlow.tsx | 32 +++ .../integrations/GitHubIntegration.tsx | 4 +- .../task-detail/hooks/useTaskDetail.ts | 19 +- .../task-review/WorkspaceStatus.tsx | 61 +++- .../frontend/src/renderer/lib/browser-mock.ts | 1 + .../renderer/lib/mocks/integration-mock.ts | 5 +- apps/frontend/src/shared/constants/ipc.ts | 3 + apps/frontend/src/shared/types/ipc.ts | 5 + apps/frontend/src/shared/types/project.ts | 1 + apps/frontend/src/shared/types/task.ts | 13 + tests/test_merge_parallel.py | 17 +- 23 files changed, 661 insertions(+), 85 deletions(-) diff --git a/apps/backend/cli/workspace_commands.py b/apps/backend/cli/workspace_commands.py index eb0cbe72..5e3d68a5 100644 --- a/apps/backend/cli/workspace_commands.py +++ b/apps/backend/cli/workspace_commands.py @@ -14,7 +14,14 @@ _PARENT_DIR = Path(__file__).parent.parent if str(_PARENT_DIR) not in sys.path: sys.path.insert(0, str(_PARENT_DIR)) -from core.workspace.git_utils import _is_auto_claude_file, is_lock_file +from core.workspace.git_utils import ( + _is_auto_claude_file, + apply_path_mapping, + detect_file_renames, + get_file_content_from_ref, + get_merge_base, + is_lock_file, +) from debug import debug_warning from ui import ( Icons, @@ -680,6 +687,58 @@ def handle_merge_preview_command( # but we want to show the user all files that will be merged total_files_from_git = len(all_changed_files) + # Detect files that need AI merge due to path mappings (file renames) + # This happens when the target branch has renamed/moved files that the + # worktree modified at their old locations + path_mapped_ai_merges: list[dict] = [] + path_mappings: dict[str, str] = {} + + if git_conflicts["needs_rebase"] and git_conflicts["commits_behind"] > 0: + # Get the merge-base between the branches + spec_branch = git_conflicts["spec_branch"] + base_branch = git_conflicts["base_branch"] + merge_base = get_merge_base(project_dir, spec_branch, base_branch) + + if merge_base: + # Detect file renames between merge-base and current base branch + path_mappings = detect_file_renames( + project_dir, merge_base, base_branch + ) + + if path_mappings: + debug( + MODULE, + f"Detected {len(path_mappings)} file rename(s) between merge-base and target", + sample_mappings={ + k: v for k, v in list(path_mappings.items())[:3] + }, + ) + + # Check which changed files have path mappings and need AI merge + for file_path in all_changed_files: + mapped_path = apply_path_mapping(file_path, path_mappings) + if mapped_path != file_path: + # File was renamed - check if both versions exist + worktree_content = get_file_content_from_ref( + project_dir, spec_branch, file_path + ) + target_content = get_file_content_from_ref( + project_dir, base_branch, mapped_path + ) + + if worktree_content and target_content: + path_mapped_ai_merges.append( + { + "oldPath": file_path, + "newPath": mapped_path, + "reason": "File was renamed/moved and modified in both branches", + } + ) + debug( + MODULE, + f"Path-mapped file needs AI merge: {file_path} -> {mapped_path}", + ) + result = { "success": True, # Use git diff files as the authoritative list of files to merge @@ -693,6 +752,9 @@ def handle_merge_preview_command( "commitsBehind": git_conflicts["commits_behind"], "baseBranch": git_conflicts["base_branch"], "specBranch": git_conflicts["spec_branch"], + # Path-mapped files that need AI merge due to renames + "pathMappedAIMerges": path_mapped_ai_merges, + "totalRenames": len(path_mappings), }, "summary": { # Use git diff count, not semantic tracker count @@ -702,6 +764,8 @@ def handle_merge_preview_command( "autoMergeable": summary.get("auto_mergeable", 0), "hasGitConflicts": git_conflicts["has_conflicts"] and len(non_lock_conflicting_files) > 0, + # Include path-mapped AI merge count for UI display + "pathMappedAIMergeCount": len(path_mapped_ai_merges), }, # Include lock files info so UI can optionally show them "lockFilesExcluded": lock_files_excluded, @@ -716,6 +780,8 @@ def handle_merge_preview_command( total_conflicts=result["summary"]["totalConflicts"], has_git_conflicts=git_conflicts["has_conflicts"], auto_mergeable=result["summary"]["autoMergeable"], + path_mapped_ai_merges=len(path_mapped_ai_merges), + total_renames=len(path_mappings), ) return result @@ -736,5 +802,6 @@ def handle_merge_preview_command( "conflictFiles": 0, "totalConflicts": 0, "autoMergeable": 0, + "pathMappedAIMergeCount": 0, }, } diff --git a/apps/backend/core/client.py b/apps/backend/core/client.py index 48de8d87..310842cd 100644 --- a/apps/backend/core/client.py +++ b/apps/backend/core/client.py @@ -3,6 +3,10 @@ Claude SDK Client Configuration =============================== Functions for creating and configuring the Claude Agent SDK client. + +All AI interactions should use `create_client()` to ensure consistent OAuth authentication +and proper tool/MCP configuration. For simple message calls without full agent sessions, +use `ClaudeSDKClient` directly with `allowed_tools=[]` and `max_turns=1`. """ import json diff --git a/apps/backend/core/workspace.py b/apps/backend/core/workspace.py index a6fa65cd..45d5476a 100644 --- a/apps/backend/core/workspace.py +++ b/apps/backend/core/workspace.py @@ -84,6 +84,12 @@ from core.workspace.git_utils import ( _is_auto_claude_file, get_existing_build_worktree, ) +from core.workspace.git_utils import ( + apply_path_mapping as _apply_path_mapping, +) +from core.workspace.git_utils import ( + detect_file_renames as _detect_file_renames, +) from core.workspace.git_utils import ( get_changed_files_from_branch as _get_changed_files_from_branch, ) @@ -93,6 +99,9 @@ from core.workspace.git_utils import ( from core.workspace.git_utils import ( is_lock_file as _is_lock_file, ) +from core.workspace.git_utils import ( + validate_merged_syntax as _validate_merged_syntax, +) # Import from refactored modules in core/workspace/ from core.workspace.models import ( @@ -230,12 +239,15 @@ def merge_existing_build( if smart_result is not None: # Smart merge handled it (success or identified conflicts) if smart_result.get("success"): - # Check if smart merge resolved git conflicts directly + # Check if smart merge resolved git conflicts or path-mapped files stats = smart_result.get("stats", {}) had_conflicts = stats.get("conflicts_resolved", 0) > 0 + files_merged = stats.get("files_merged", 0) > 0 + ai_assisted = stats.get("ai_assisted", 0) > 0 - if had_conflicts: - # Git conflicts were resolved (via AI or lock file exclusion) - changes are already staged + if had_conflicts or files_merged or ai_assisted: + # Git conflicts were resolved OR path-mapped files were AI merged + # Changes are already written and staged - no need for git merge _print_merge_success( no_commit, stats, spec_name=spec_name, keep_worktree=True ) @@ -246,7 +258,7 @@ def merge_existing_build( return True else: - # No git conflicts, do standard git merge + # No conflicts and no files merged - do standard git merge success_result = manager.merge_worktree( spec_name, delete_after=False, no_commit=no_commit ) @@ -731,6 +743,23 @@ def _resolve_git_conflicts_with_ai( merge_base=merge_base[:12] if merge_base else None, ) + # Detect file renames between merge-base and target branch + # This handles cases where files were moved/renamed (e.g., directory restructures) + path_mappings: dict[str, str] = {} + if merge_base: + path_mappings = _detect_file_renames(project_dir, merge_base, base_branch) + if path_mappings: + debug( + MODULE, + f"Detected {len(path_mappings)} file renames between merge-base and target", + sample_mappings=dict(list(path_mappings.items())[:5]), + ) + print( + muted( + f" Detected {len(path_mappings)} file rename(s) since branch creation" + ) + ) + # FIX: Copy NEW files FIRST before resolving conflicts # This ensures dependencies exist before files that import them are written changed_files = _get_changed_files_from_branch( @@ -748,14 +777,24 @@ def _resolve_git_conflicts_with_ai( project_dir, spec_branch, file_path ) if content is not None: - target_path = project_dir / file_path + # Apply path mapping - write to new location if file was renamed + target_file_path = _apply_path_mapping(file_path, path_mappings) + target_path = project_dir / target_file_path target_path.parent.mkdir(parents=True, exist_ok=True) target_path.write_text(content, encoding="utf-8") subprocess.run( - ["git", "add", file_path], cwd=project_dir, capture_output=True + ["git", "add", target_file_path], + cwd=project_dir, + capture_output=True, ) - resolved_files.append(file_path) - debug(MODULE, f"Copied new file: {file_path}") + resolved_files.append(target_file_path) + if target_file_path != file_path: + debug( + MODULE, + f"Copied new file with path mapping: {file_path} -> {target_file_path}", + ) + else: + debug(MODULE, f"Copied new file: {file_path}") except Exception as e: debug_warning(MODULE, f"Could not copy new file {file_path}: {e}") @@ -769,20 +808,26 @@ def _resolve_git_conflicts_with_ai( debug(MODULE, "Categorizing conflicting files for parallel processing") for file_path in conflicting_files: - debug(MODULE, f"Categorizing conflicting file: {file_path}") + # Apply path mapping to get the target path in the current branch + target_file_path = _apply_path_mapping(file_path, path_mappings) + debug( + MODULE, + f"Categorizing conflicting file: {file_path}" + + (f" -> {target_file_path}" if target_file_path != file_path else ""), + ) try: - # Get content from main branch + # Get content from main branch using MAPPED path (file may have been renamed) main_content = _get_file_content_from_ref( - project_dir, base_branch, file_path + project_dir, base_branch, target_file_path ) - # Get content from worktree branch + # Get content from worktree branch using ORIGINAL path worktree_content = _get_file_content_from_ref( project_dir, spec_branch, file_path ) - # Get content from merge-base (common ancestor) + # Get content from merge-base (common ancestor) using ORIGINAL path base_content = None if merge_base: base_content = _get_file_content_from_ref( @@ -795,38 +840,49 @@ def _resolve_git_conflicts_with_ai( if main_content is None: # File only exists in worktree - it's a new file (no AI needed) - simple_merges.append((file_path, worktree_content)) + # Write to target path (mapped if applicable) + simple_merges.append((target_file_path, worktree_content)) debug(MODULE, f" {file_path}: new file (no AI needed)") elif worktree_content is None: # File only exists in main - was deleted in worktree (no AI needed) - simple_merges.append((file_path, None)) # None = delete + simple_merges.append((target_file_path, None)) # None = delete debug(MODULE, f" {file_path}: deleted (no AI needed)") else: # File exists in both - check if it's a lock file - if _is_lock_file(file_path): + if _is_lock_file(target_file_path): # Lock files should be excluded from merge entirely # They must be regenerated after merge by running the package manager # (e.g., npm install, pnpm install, uv sync, cargo update) # # Strategy: Take main branch version and let user regenerate - lock_files_excluded.append(file_path) - simple_merges.append((file_path, main_content)) + lock_files_excluded.append(target_file_path) + simple_merges.append((target_file_path, main_content)) debug( MODULE, - f" {file_path}: lock file (excluded - will use main version)", + f" {target_file_path}: lock file (excluded - will use main version)", ) else: # Regular file - needs AI merge + # Store the TARGET path for writing, but track original for content retrieval files_needing_ai_merge.append( ParallelMergeTask( - file_path=file_path, + file_path=target_file_path, # Use target path for writing main_content=main_content, worktree_content=worktree_content, base_content=base_content, spec_name=spec_name, + project_dir=project_dir, ) ) - debug(MODULE, f" {file_path}: needs AI merge") + debug( + MODULE, + f" {file_path}: needs AI merge" + + ( + f" (will write to {target_file_path})" + if target_file_path != file_path + else "" + ), + ) except Exception as e: print(error(f" ✗ Failed to categorize {file_path}: {e}")) @@ -946,29 +1002,140 @@ def _resolve_git_conflicts_with_ai( if f not in conflicting_files and s != "A" # Skip new files, already copied ] + # Separate files that need AI merge (path-mapped) from simple copies + path_mapped_files: list[ParallelMergeTask] = [] + simple_copy_files: list[ + tuple[str, str, str] + ] = [] # (file_path, target_path, status) + for file_path, status in non_conflicting: + # Apply path mapping for renamed/moved files + target_file_path = _apply_path_mapping(file_path, path_mappings) + + if target_file_path != file_path and status != "D": + # File was renamed/moved - needs AI merge to incorporate changes + # Get content from worktree (old path) and target branch (new path) + worktree_content = _get_file_content_from_ref( + project_dir, spec_branch, file_path + ) + target_content = _get_file_content_from_ref( + project_dir, base_branch, target_file_path + ) + base_content = None + if merge_base: + base_content = _get_file_content_from_ref( + project_dir, merge_base, file_path + ) + + if worktree_content and target_content: + # Both exist - need AI merge + path_mapped_files.append( + ParallelMergeTask( + file_path=target_file_path, + main_content=target_content, + worktree_content=worktree_content, + base_content=base_content, + spec_name=spec_name, + project_dir=project_dir, + ) + ) + debug( + MODULE, + f"Path-mapped file needs AI merge: {file_path} -> {target_file_path}", + ) + elif worktree_content: + # Only exists in worktree - simple copy to new path + simple_copy_files.append((file_path, target_file_path, status)) + else: + # No path mapping or deletion - simple operation + simple_copy_files.append((file_path, target_file_path, status)) + + # Process path-mapped files with AI merge + if path_mapped_files: + print() + print_status( + f"Merging {len(path_mapped_files)} path-mapped file(s) with AI...", + "progress", + ) + + import time + + start_time = time.time() + + # Run parallel merges for path-mapped files + path_mapped_results = asyncio.run( + _run_parallel_merges( + tasks=path_mapped_files, + project_dir=project_dir, + max_concurrent=MAX_PARALLEL_AI_MERGES, + ) + ) + + elapsed = time.time() - start_time + + for result in path_mapped_results: + if result.success: + target_path = project_dir / result.file_path + target_path.parent.mkdir(parents=True, exist_ok=True) + target_path.write_text(result.merged_content, encoding="utf-8") + subprocess.run( + ["git", "add", result.file_path], + cwd=project_dir, + capture_output=True, + ) + resolved_files.append(result.file_path) + + if result.was_auto_merged: + auto_merged_count += 1 + print(success(f" ✓ {result.file_path} (auto-merged)")) + else: + ai_merged_count += 1 + print(success(f" ✓ {result.file_path} (AI merged)")) + else: + print(error(f" ✗ {result.file_path}: {result.error}")) + remaining_conflicts.append( + { + "file": result.file_path, + "reason": result.error or "AI could not merge path-mapped file", + "severity": "high", + } + ) + + print(muted(f" Path-mapped merge completed in {elapsed:.1f}s")) + + # Process simple copy/delete files + for file_path, target_file_path, status in simple_copy_files: try: if status == "D": - # Deleted in worktree - target_path = project_dir / file_path + # Deleted in worktree - delete from target path + target_path = project_dir / target_file_path if target_path.exists(): target_path.unlink() subprocess.run( - ["git", "add", file_path], cwd=project_dir, capture_output=True + ["git", "add", target_file_path], + cwd=project_dir, + capture_output=True, ) else: - # Added or modified - copy from worktree + # Modified without path change - simple copy content = _get_file_content_from_ref( project_dir, spec_branch, file_path ) if content is not None: - target_path = project_dir / file_path + target_path = project_dir / target_file_path target_path.parent.mkdir(parents=True, exist_ok=True) target_path.write_text(content, encoding="utf-8") subprocess.run( - ["git", "add", file_path], cwd=project_dir, capture_output=True + ["git", "add", target_file_path], + cwd=project_dir, + capture_output=True, ) - resolved_files.append(file_path) + resolved_files.append(target_file_path) + if target_file_path != file_path: + debug( + MODULE, + f"Merged with path mapping: {file_path} -> {target_file_path}", + ) except Exception as e: print(muted(f" Warning: Could not process {file_path}: {e}")) @@ -1274,6 +1441,47 @@ async def _merge_file_with_ai_async( # Strip any code fences the model might have added merged_content = _strip_code_fences(response_text.strip()) + # VALIDATION: Check if AI returned natural language instead of code + # This catches cases where AI says "I need to see more..." instead of merging + natural_language_patterns = [ + "I need to", + "Let me", + "I cannot", + "I'm unable", + "The file appears", + "I don't have", + "Unfortunately", + "I apologize", + ] + first_line = merged_content.split("\n")[0] if merged_content else "" + if any(pattern in first_line for pattern in natural_language_patterns): + debug_warning( + MODULE, + f"AI returned natural language instead of code for {task.file_path}: {first_line[:100]}", + ) + return ParallelMergeResult( + file_path=task.file_path, + merged_content=None, + success=False, + error=f"AI returned explanation instead of code: {first_line[:80]}...", + ) + + # VALIDATION: Run syntax check on the merged content + is_valid, syntax_error = _validate_merged_syntax( + task.file_path, merged_content, task.project_dir + ) + if not is_valid: + debug_warning( + MODULE, + f"AI merge produced invalid syntax for {task.file_path}: {syntax_error}", + ) + return ParallelMergeResult( + file_path=task.file_path, + merged_content=None, + success=False, + error=f"AI merge produced invalid syntax: {syntax_error}", + ) + debug(MODULE, f"AI merged {task.file_path} successfully") return ParallelMergeResult( file_path=task.file_path, diff --git a/apps/backend/core/workspace/git_utils.py b/apps/backend/core/workspace/git_utils.py index c29b2d19..d460dc97 100644 --- a/apps/backend/core/workspace/git_utils.py +++ b/apps/backend/core/workspace/git_utils.py @@ -83,6 +83,111 @@ MERGE_LOCK_TIMEOUT = 300 # 5 minutes MAX_SYNTAX_FIX_RETRIES = 2 +def detect_file_renames( + project_dir: Path, + from_ref: str, + to_ref: str, +) -> dict[str, str]: + """ + Detect file renames between two git refs using git's rename detection. + + This analyzes the commit history between two refs to find all file + renames/moves. Critical for merging changes from older branches that + used a different directory structure. + + Uses git's -M flag for rename detection with high similarity threshold. + + Args: + project_dir: Project directory + from_ref: Starting ref (e.g., merge-base commit or old branch) + to_ref: Target ref (e.g., current branch HEAD) + + Returns: + Dict mapping old_path -> new_path for all renamed files + """ + renames: dict[str, str] = {} + + try: + # Use git log with rename detection to find all renames between refs + # -M flag enables rename detection + # --diff-filter=R shows only renames + # --name-status shows status and file names + result = subprocess.run( + [ + "git", + "log", + "--name-status", + "-M", + "--diff-filter=R", + "--format=", # No commit info, just file changes + f"{from_ref}..{to_ref}", + ], + cwd=project_dir, + capture_output=True, + text=True, + ) + + if result.returncode == 0: + for line in result.stdout.strip().split("\n"): + if line.startswith("R"): + # Format: R100\told_path\tnew_path (tab-separated) + parts = line.split("\t") + if len(parts) >= 3: + old_path = parts[1] + new_path = parts[2] + renames[old_path] = new_path + + except Exception: + pass # Return empty dict on error + + return renames + + +def apply_path_mapping(file_path: str, mappings: dict[str, str]) -> str: + """ + Apply file path mappings to get the new path for a file. + + Args: + file_path: Original file path (from older branch) + mappings: Dict of old_path -> new_path from detect_file_renames + + Returns: + Mapped new path if found, otherwise original path + """ + # Direct match + if file_path in mappings: + return mappings[file_path] + + # No mapping found + return file_path + + +def get_merge_base(project_dir: Path, ref1: str, ref2: str) -> str | None: + """ + Get the merge-base commit between two refs. + + Args: + project_dir: Project directory + ref1: First ref (branch/commit) + ref2: Second ref (branch/commit) + + Returns: + Merge-base commit hash, or None if not found + """ + try: + result = subprocess.run( + ["git", "merge-base", ref1, ref2], + cwd=project_dir, + capture_output=True, + text=True, + ) + if result.returncode == 0: + return result.stdout.strip() + except Exception: + pass + return None + + def has_uncommitted_changes(project_dir: Path) -> bool: """Check if user has unsaved work.""" result = subprocess.run( diff --git a/apps/backend/core/workspace/models.py b/apps/backend/core/workspace/models.py index 039bf786..cc94413e 100644 --- a/apps/backend/core/workspace/models.py +++ b/apps/backend/core/workspace/models.py @@ -36,6 +36,7 @@ class ParallelMergeTask: worktree_content: str base_content: str | None spec_name: str + project_dir: Path @dataclass diff --git a/apps/backend/runners/github/services/followup_reviewer.py b/apps/backend/runners/github/services/followup_reviewer.py index 61a6ad79..240cd068 100644 --- a/apps/backend/runners/github/services/followup_reviewer.py +++ b/apps/backend/runners/github/services/followup_reviewer.py @@ -543,10 +543,6 @@ class FollowupReviewer: Returns parsed AI response with finding resolutions and new findings, or None if AI review fails. """ - # Use raw Anthropic client for simple message API calls - # (ClaudeSDKClient is for agent sessions, not direct message calls) - import anthropic - self._report_progress( "analyzing", 65, "Running AI-powered review...", context.pr_number ) @@ -623,21 +619,51 @@ Please analyze this follow-up review context and provide your response in the JS """ try: - # Create Anthropic client for simple message API call - # Note: For agent sessions with tools, use ClaudeSDKClient instead - client = anthropic.AsyncAnthropic() + # Use ClaudeSDKClient directly for simple message calls + # (no agent tools needed, just a single query/response) + from claude_agent_sdk import ClaudeAgentOptions, ClaudeSDKClient + model = self.config.model or "claude-sonnet-4-5-20250929" - response = await client.messages.create( - model=model, - max_tokens=4096, - messages=[{"role": "user", "content": user_message}], + client = ClaudeSDKClient( + options=ClaudeAgentOptions( + model=model, + system_prompt="You are a code review assistant. Analyze the provided context and respond with valid JSON.", + allowed_tools=[], + max_turns=1, + max_thinking_tokens=2048, + ) ) - # Parse the response - response_text = response.content[0].text + response_text = "" + async with client: + await client.query(user_message) + + async for msg in client.receive_response(): + msg_type = type(msg).__name__ + logger.debug(f"AI response message type: {msg_type}") + if msg_type == "AssistantMessage" and hasattr(msg, "content"): + for block in msg.content: + block_type = type(block).__name__ + logger.debug(f" Content block type: {block_type}") + if hasattr(block, "text"): + response_text += block.text + elif hasattr(block, "thinking"): + # Skip thinking blocks - we only want the final text + logger.debug(" (skipping thinking block)") + + if not response_text: + logger.warning("AI returned empty response (no text blocks found)") + return None + + logger.debug(f"AI response text (first 500 chars): {response_text[:500]}") return self._parse_ai_response(response_text) + except ValueError as e: + # OAuth token not found + logger.warning(f"No OAuth token available for AI review: {e}") + print("AI review failed: No OAuth token found", flush=True) + return None except Exception as e: logger.error(f"AI review failed: {e}") return None diff --git a/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts b/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts index 6ff4db97..6136fdf3 100644 --- a/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts +++ b/apps/frontend/src/main/ipc-handlers/github/__tests__/oauth-handlers.spec.ts @@ -44,11 +44,21 @@ vi.mock('electron', () => { } })(); + // Mock BrowserWindow for sendDeviceCodeToRenderer + const mockBrowserWindow = { + getAllWindows: () => [{ + webContents: { + send: vi.fn() + } + }] + }; + return { ipcMain: mockIpcMain, shell: { openExternal: (...args: unknown[]) => mockOpenExternal(...args) - } + }, + BrowserWindow: mockBrowserWindow }; }); diff --git a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts index 23616588..5f34cf34 100644 --- a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts @@ -3,12 +3,28 @@ * Provides a simpler OAuth flow than manual PAT creation */ -import { ipcMain, shell } from 'electron'; +import { ipcMain, shell, BrowserWindow } from 'electron'; import { execSync, execFileSync, spawn } from 'child_process'; import { IPC_CHANNELS } from '../../../shared/constants'; import type { IPCResult } from '../../../shared/types'; import { getAugmentedEnv, findExecutable } from '../../env-utils'; +/** + * Send device code info to all renderer windows immediately when extracted + * This allows the UI to display the code while the auth process is still running + */ +function sendDeviceCodeToRenderer(deviceCode: string, authUrl: string, browserOpened: boolean): void { + debugLog('Sending device code to renderer windows'); + const windows = BrowserWindow.getAllWindows(); + for (const win of windows) { + win.webContents.send(IPC_CHANNELS.GITHUB_AUTH_DEVICE_CODE, { + deviceCode, + authUrl, + browserOpened + }); + } +} + // Debug logging helper const DEBUG = process.env.DEBUG === 'true' || process.env.NODE_ENV === 'development'; @@ -265,6 +281,10 @@ export function registerStartGhAuth(): void { // Don't fail here - we'll return the device code so user can manually navigate } + // IMMEDIATELY send device code to renderer so user can see it while auth is in progress + // This is critical - the frontend needs to display the code while the gh process is still running + sendDeviceCodeToRenderer(extractedDeviceCode, extractedAuthUrl, browserOpenedSuccessfully); + // Extraction complete - mutex flag stays true to prevent re-extraction // The deviceCodeExtracted flag will prevent future attempts extractionInProgress = false; diff --git a/apps/frontend/src/preload/api/modules/github-api.ts b/apps/frontend/src/preload/api/modules/github-api.ts index 0b18b4a3..4fb5ff1e 100644 --- a/apps/frontend/src/preload/api/modules/github-api.ts +++ b/apps/frontend/src/preload/api/modules/github-api.ts @@ -155,6 +155,11 @@ export interface GitHubAPI { getGitHubUser: () => Promise>; listGitHubUserRepos: () => Promise }>>; + // OAuth event listener - receives device code immediately when extracted + onGitHubAuthDeviceCode: ( + callback: (data: { deviceCode: string; authUrl: string; browserOpened: boolean }) => void + ) => IpcListenerCleanup; + // Repository detection and management detectGitHubRepo: (projectPath: string) => Promise>; getGitHubBranches: (repo: string, token: string) => Promise>; @@ -398,6 +403,12 @@ export const createGitHubAPI = (): GitHubAPI => ({ listGitHubUserRepos: (): Promise }>> => invokeIpc(IPC_CHANNELS.GITHUB_LIST_USER_REPOS), + // OAuth event listener - receives device code immediately when extracted (during auth process) + onGitHubAuthDeviceCode: ( + callback: (data: { deviceCode: string; authUrl: string; browserOpened: boolean }) => void + ): IpcListenerCleanup => + createIpcListener(IPC_CHANNELS.GITHUB_AUTH_DEVICE_CODE, callback), + // Repository detection and management detectGitHubRepo: (projectPath: string): Promise> => invokeIpc(IPC_CHANNELS.GITHUB_DETECT_REPO, projectPath), diff --git a/apps/frontend/src/renderer/App.tsx b/apps/frontend/src/renderer/App.tsx index bdde8872..24c3282d 100644 --- a/apps/frontend/src/renderer/App.tsx +++ b/apps/frontend/src/renderer/App.tsx @@ -504,6 +504,7 @@ export function App() { githubToken: string; githubRepo: string; mainBranch: string; + githubAuthMethod?: 'oauth' | 'pat'; }) => { if (!gitHubSetupProject) return; @@ -518,7 +519,8 @@ export function App() { await window.electronAPI.updateProjectEnv(gitHubSetupProject.id, { githubEnabled: true, githubToken: settings.githubToken, // GitHub token for repo access - githubRepo: settings.githubRepo + githubRepo: settings.githubRepo, + githubAuthMethod: settings.githubAuthMethod // Track how user authenticated }); // Update project settings with mainBranch diff --git a/apps/frontend/src/renderer/components/GitHubSetupModal.tsx b/apps/frontend/src/renderer/components/GitHubSetupModal.tsx index 788e0b47..ef7d5426 100644 --- a/apps/frontend/src/renderer/components/GitHubSetupModal.tsx +++ b/apps/frontend/src/renderer/components/GitHubSetupModal.tsx @@ -42,7 +42,7 @@ interface GitHubSetupModalProps { open: boolean; onOpenChange: (open: boolean) => void; project: Project; - onComplete: (settings: { githubToken: string; githubRepo: string; mainBranch: string }) => void; + onComplete: (settings: { githubToken: string; githubRepo: string; mainBranch: string; githubAuthMethod?: 'oauth' | 'pat' }) => void; onSkip?: () => void; } @@ -367,7 +367,8 @@ export function GitHubSetupModal({ onComplete({ githubToken, githubRepo, - mainBranch: selectedBranch + mainBranch: selectedBranch, + githubAuthMethod: 'oauth' // Setup modal always uses OAuth flow }); } }; diff --git a/apps/frontend/src/renderer/components/project-settings/GitHubIntegrationSection.tsx b/apps/frontend/src/renderer/components/project-settings/GitHubIntegrationSection.tsx index 65a3394e..47c79c7d 100644 --- a/apps/frontend/src/renderer/components/project-settings/GitHubIntegrationSection.tsx +++ b/apps/frontend/src/renderer/components/project-settings/GitHubIntegrationSection.tsx @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { Github, RefreshCw, KeyRound, Info } from 'lucide-react'; +import { Github, RefreshCw, KeyRound, Info, CheckCircle2 } from 'lucide-react'; import { CollapsibleSection } from './CollapsibleSection'; import { StatusBadge } from './StatusBadge'; import { PasswordInput } from './PasswordInput'; @@ -31,17 +31,24 @@ export function GitHubIntegrationSection({ isCheckingGitHub, projectName, }: GitHubIntegrationSectionProps) { - const [showOAuthFlow, setShowOAuthFlow] = useState(false); + // Show OAuth flow if user previously used OAuth, or if there's no token yet + const [showOAuthFlow, setShowOAuthFlow] = useState( + envConfig.githubAuthMethod === 'oauth' || (!envConfig.githubToken && !envConfig.githubAuthMethod) + ); const badge = envConfig.githubEnabled ? ( ) : null; const handleOAuthSuccess = (token: string, _username?: string) => { - onUpdateConfig({ githubToken: token }); + onUpdateConfig({ githubToken: token, githubAuthMethod: 'oauth' }); setShowOAuthFlow(false); }; + const handleManualTokenChange = (value: string) => { + onUpdateConfig({ githubToken: value, githubAuthMethod: 'pat' }); + }; + return ( - {showOAuthFlow ? ( + {/* Show OAuth connected state when authenticated via OAuth */} + {envConfig.githubAuthMethod === 'oauth' && envConfig.githubToken && !showOAuthFlow ? ( +
+
+ + +
+
+ + Authenticated via GitHub OAuth (gh CLI) +
+
+ ) : showOAuthFlow ? (
@@ -125,7 +151,7 @@ export function GitHubIntegrationSection({

onUpdateConfig({ githubToken: value })} + onChange={handleManualTokenChange} placeholder="ghp_xxxxxxxx or github_pat_xxxxxxxx" />
diff --git a/apps/frontend/src/renderer/components/project-settings/GitHubOAuthFlow.tsx b/apps/frontend/src/renderer/components/project-settings/GitHubOAuthFlow.tsx index 16764b24..229cc01c 100644 --- a/apps/frontend/src/renderer/components/project-settings/GitHubOAuthFlow.tsx +++ b/apps/frontend/src/renderer/components/project-settings/GitHubOAuthFlow.tsx @@ -111,6 +111,38 @@ export function GitHubOAuthFlow({ onSuccess, onCancel }: GitHubOAuthFlowProps) { // eslint-disable-next-line react-hooks/exhaustive-deps -- Only run once on mount, checkGitHubStatus is intentionally excluded }, [clearAuthTimeout]); + // Listen for device code events from the main process + // This allows us to display the code IMMEDIATELY when extracted, not after the auth completes + useEffect(() => { + if (status !== 'authenticating') { + return; + } + + debugLog('Setting up device code event listener'); + + // Listen for device code from main process (sent immediately when extracted) + const cleanup = window.electronAPI.onGitHubAuthDeviceCode((data) => { + debugLog('Received device code from main process:', { + hasCode: !!data.deviceCode, + authUrl: data.authUrl, + browserOpened: data.browserOpened + }); + + if (data.deviceCode) { + setDeviceCode(data.deviceCode); + } + if (data.authUrl) { + setAuthUrl(data.authUrl); + } + setBrowserOpened(data.browserOpened); + }); + + return () => { + debugLog('Cleaning up device code event listener'); + cleanup(); + }; + }, [status]); + const checkGitHubStatus = async () => { debugLog('checkGitHubStatus() called'); setStatus('checking'); diff --git a/apps/frontend/src/renderer/components/settings/integrations/GitHubIntegration.tsx b/apps/frontend/src/renderer/components/settings/integrations/GitHubIntegration.tsx index ad21e613..6fa39781 100644 --- a/apps/frontend/src/renderer/components/settings/integrations/GitHubIntegration.tsx +++ b/apps/frontend/src/renderer/components/settings/integrations/GitHubIntegration.tsx @@ -158,8 +158,8 @@ export function GitHubIntegration({ debugLog('handleOAuthSuccess called with token length:', token.length); debugLog('OAuth username:', username); - // Update the token - updateEnvConfig({ githubToken: token }); + // Update the token and auth method + updateEnvConfig({ githubToken: token, githubAuthMethod: 'oauth' }); // Show success state with username setOauthUsername(username || null); diff --git a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts index 9d5aab0c..676a099a 100644 --- a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts +++ b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts @@ -193,21 +193,14 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { }); }, []); - // Restore merge preview from sessionStorage on mount (survives HMR reloads) + // Clear merge preview cache when task changes to ensure fresh data is fetched + // This invalidates any stale cached data (e.g., old uncommitted changes status) useEffect(() => { const storageKey = `mergePreview-${task.id}`; - const stored = sessionStorage.getItem(storageKey); - if (stored) { - try { - const previewData = JSON.parse(stored); - console.warn('%c[useTaskDetail] Restored merge preview from sessionStorage:', 'color: magenta;', previewData); - setMergePreview(previewData); - // Don't auto-popup - restored data stays silent - } catch { - console.warn('[useTaskDetail] Failed to parse stored merge preview'); - sessionStorage.removeItem(storageKey); - } - } + // Clear any existing cached preview - we want fresh data when opening a task + sessionStorage.removeItem(storageKey); + setMergePreview(null); + console.warn('[useTaskDetail] Cleared merge preview cache for task:', task.id); }, [task.id]); // Load merge preview (conflict detection) diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx index d21ec622..8e48f2e6 100644 --- a/apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx +++ b/apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx @@ -62,14 +62,21 @@ export function WorkspaceStatus({ const uncommittedCount = mergePreview?.uncommittedChanges?.count || 0; const hasAIConflicts = mergePreview && mergePreview.conflicts.length > 0; - // Determine overall status - const statusColor = hasGitConflicts - ? 'warning' - : hasUncommittedChanges - ? 'warning' - : mergePreview && !hasAIConflicts - ? 'success' - : 'info'; + // 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; + const commitsBehind = mergePreview?.gitConflicts?.commitsBehind || 0; + + // Path-mapped files that need AI merge due to file renames + const pathMappedAIMergeCount = mergePreview?.summary?.pathMappedAIMergeCount || 0; + const totalRenames = mergePreview?.gitConflicts?.totalRenames || 0; + + // Branch is behind if needsRebase is true and there are commits to catch up on + // This triggers AI merge for path-mapped files even without explicit conflicts + const isBranchBehind = needsRebase && commitsBehind > 0; + + // Has path-mapped files that need AI merge + const hasPathMappedMerges = pathMappedAIMergeCount > 0; return (
@@ -201,7 +208,7 @@ export function WorkspaceStatus({ {mergePreview && (
AI will resolve
+ ) : isBranchBehind || hasPathMappedMerges ? ( + <> + +
+ + {hasPathMappedMerges ? 'Files Renamed' : 'Branch Behind'} + + + AI will resolve ({hasPathMappedMerges ? `${pathMappedAIMergeCount} files` : `${commitsBehind} commits`}) + +
+ ) : !hasAIConflicts ? ( <> @@ -234,7 +253,7 @@ export function WorkspaceStatus({ )}
- {(hasGitConflicts || hasAIConflicts) && ( + {(hasGitConflicts || isBranchBehind || hasPathMappedMerges || hasAIConflicts) && (
{/* Actions Footer */} @@ -293,7 +328,7 @@ export function WorkspaceStatus({ {/* Primary Actions */}