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 */}