diff --git a/apps/backend/cli/main.py b/apps/backend/cli/main.py index 9b910b53..cfb6a6a4 100644 --- a/apps/backend/cli/main.py +++ b/apps/backend/cli/main.py @@ -38,6 +38,7 @@ from .utils import ( ) from .workspace_commands import ( handle_cleanup_worktrees_command, + handle_create_pr_command, handle_discard_command, handle_list_worktrees_command, handle_merge_command, @@ -153,6 +154,30 @@ Environment Variables: action="store_true", help="Discard an existing build (requires confirmation)", ) + build_group.add_argument( + "--create-pr", + action="store_true", + help="Push branch and create a GitHub Pull Request", + ) + + # PR options + parser.add_argument( + "--pr-target", + type=str, + metavar="BRANCH", + help="With --create-pr: target branch for PR (default: auto-detect)", + ) + parser.add_argument( + "--pr-title", + type=str, + metavar="TITLE", + help="With --create-pr: custom PR title (default: generated from spec name)", + ) + parser.add_argument( + "--pr-draft", + action="store_true", + help="With --create-pr: create as draft PR", + ) # Merge options parser.add_argument( @@ -365,6 +390,21 @@ def main() -> None: handle_discard_command(project_dir, spec_dir.name) return + if args.create_pr: + # Pass args.pr_target directly - WorktreeManager._detect_base_branch + # handles base branch detection internally when target_branch is None + result = handle_create_pr_command( + project_dir=project_dir, + spec_name=spec_dir.name, + target_branch=args.pr_target, + title=args.pr_title, + draft=args.pr_draft, + ) + # JSON output is already printed by handle_create_pr_command + if not result.get("success"): + sys.exit(1) + return + # Handle QA commands if args.qa_status: handle_qa_status_command(spec_dir) diff --git a/apps/backend/cli/workspace_commands.py b/apps/backend/cli/workspace_commands.py index e6f25091..85f9f732 100644 --- a/apps/backend/cli/workspace_commands.py +++ b/apps/backend/cli/workspace_commands.py @@ -5,6 +5,7 @@ Workspace Commands CLI commands for workspace management (merge, review, discard, list, cleanup) """ +import json import subprocess import sys from pathlib import Path @@ -22,6 +23,7 @@ from core.workspace.git_utils import ( get_merge_base, is_lock_file, ) +from core.worktree import PushAndCreatePRResult as CreatePRResult from core.worktree import WorktreeManager from debug import debug_warning from ui import ( @@ -31,6 +33,7 @@ from ui import ( from workspace import ( cleanup_all_worktrees, discard_existing_build, + get_existing_build_worktree, list_all_worktrees, merge_existing_build, review_existing_build, @@ -175,8 +178,6 @@ def _detect_worktree_base_branch( Returns: The detected base branch name, or None if unable to detect """ - import json - # Strategy 1: Check for worktree config file config_path = worktree_path / ".auto-claude" / "worktree-config.json" if config_path.exists(): @@ -1002,6 +1003,117 @@ def handle_merge_preview_command( } +def handle_create_pr_command( + project_dir: Path, + spec_name: str, + target_branch: str | None = None, + title: str | None = None, + draft: bool = False, +) -> CreatePRResult: + """ + Handle the --create-pr command: push branch and create a GitHub PR. + + Args: + project_dir: Path to the project directory + spec_name: Name of the spec (e.g., "001-feature-name") + target_branch: Target branch for PR (defaults to base branch) + title: Custom PR title (defaults to spec name) + draft: Whether to create as draft PR + + Returns: + CreatePRResult with success status, pr_url, and any errors + """ + from core.worktree import WorktreeManager + + print_banner() + print("\n" + "=" * 70) + print(" CREATE PULL REQUEST") + print("=" * 70) + + # Check if worktree exists + worktree_path = get_existing_build_worktree(project_dir, spec_name) + if not worktree_path: + print(f"\n{icon(Icons.ERROR)} No build found for spec: {spec_name}") + print("\nA completed build worktree is required to create a PR.") + print("Run your build first, then use --create-pr.") + error_result: CreatePRResult = { + "success": False, + "error": "No build found for this spec", + } + return error_result + + # Create worktree manager + manager = WorktreeManager(project_dir, base_branch=target_branch) + + print(f"\n{icon(Icons.BRANCH)} Pushing branch and creating PR...") + print(f" Spec: {spec_name}") + print(f" Target: {target_branch or manager.base_branch}") + if title: + print(f" Title: {title}") + if draft: + print(" Mode: Draft PR") + + # Push and create PR with exception handling for clean JSON output + try: + raw_result = manager.push_and_create_pr( + spec_name=spec_name, + target_branch=target_branch, + title=title, + draft=draft, + ) + except Exception as e: + debug_error(MODULE, f"Exception during PR creation: {e}") + error_result: CreatePRResult = { + "success": False, + "error": str(e), + "message": "Failed to create PR", + } + print(f"\n{icon(Icons.ERROR)} Failed to create PR: {e}") + print(json.dumps(error_result)) + return error_result + + # Convert PushAndCreatePRResult to CreatePRResult + result: CreatePRResult = { + "success": raw_result.get("success", False), + "pr_url": raw_result.get("pr_url"), + "already_exists": raw_result.get("already_exists", False), + "error": raw_result.get("error"), + "message": raw_result.get("message"), + "pushed": raw_result.get("pushed", False), + "remote": raw_result.get("remote", ""), + "branch": raw_result.get("branch", ""), + } + + if result.get("success"): + pr_url = result.get("pr_url") + already_exists = result.get("already_exists", False) + + if already_exists: + print(f"\n{icon(Icons.SUCCESS)} PR already exists!") + else: + print(f"\n{icon(Icons.SUCCESS)} PR created successfully!") + + if pr_url: + print(f"\n{icon(Icons.LINK)} {pr_url}") + else: + print(f"\n{icon(Icons.INFO)} Check GitHub for the PR URL") + + print("\nNext steps:") + print(" 1. Review the PR on GitHub") + print(" 2. Request reviews from your team") + print(" 3. Merge when approved") + + # Output JSON for frontend parsing + print(json.dumps(result)) + return result + else: + error = result.get("error", "Unknown error") + print(f"\n{icon(Icons.ERROR)} Failed to create PR: {error}") + # Output JSON for frontend parsing + print(json.dumps(result)) + return result + + def cleanup_old_worktrees_command( project_dir: Path, days: int = 30, dry_run: bool = False ) -> dict: diff --git a/apps/backend/core/worktree.py b/apps/backend/core/worktree.py index a8e19498..eb4870dd 100644 --- a/apps/backend/core/worktree.py +++ b/apps/backend/core/worktree.py @@ -19,11 +19,126 @@ import os import re import shutil import subprocess +import time +from collections.abc import Callable from dataclasses import dataclass from datetime import datetime from pathlib import Path +from typing import TypedDict, TypeVar -from core.git_executable import run_git +from core.git_executable import get_git_executable, run_git +from debug import debug_warning + +T = TypeVar("T") + + +def _is_retryable_network_error(stderr: str) -> bool: + """Check if an error is a retryable network/connection issue.""" + stderr_lower = stderr.lower() + return any( + term in stderr_lower + for term in ["connection", "network", "timeout", "reset", "refused"] + ) + + +def _is_retryable_http_error(stderr: str) -> bool: + """ + Check if an HTTP error is retryable (5xx errors, timeouts). + Excludes auth errors (401, 403) and client errors (404, 422). + """ + stderr_lower = stderr.lower() + # Check for HTTP 5xx errors (server errors are retryable) + if re.search(r"http[s]?\s*5\d{2}", stderr_lower): + return True + # Check for HTTP timeout patterns + if "http" in stderr_lower and "timeout" in stderr_lower: + return True + return False + + +def _with_retry( + operation: Callable[[], tuple[bool, T | None, str]], + max_retries: int = 3, + is_retryable: Callable[[str], bool] | None = None, + on_retry: Callable[[int, str], None] | None = None, +) -> tuple[T | None, str]: + """ + Execute an operation with retry logic. + + Args: + operation: Function that returns a tuple of (success: bool, result: T | None, error: str). + On success (success=True), result contains the value and error is empty. + On failure (success=False), result is None and error contains the message. + max_retries: Maximum number of retry attempts + is_retryable: Function to check if error is retryable based on error message + on_retry: Optional callback called before each retry with (attempt, error) + + Returns: + Tuple of (result, last_error) where result is T on success, None on failure + """ + last_error = "" + + for attempt in range(1, max_retries + 1): + try: + success, result, error = operation() + if success: + return result, "" + + last_error = error + + # Check if error is retryable + if is_retryable and attempt < max_retries and is_retryable(error): + if on_retry: + on_retry(attempt, error) + backoff = 2 ** (attempt - 1) + time.sleep(backoff) + continue + + break + + except subprocess.TimeoutExpired: + last_error = "Operation timed out" + if attempt < max_retries: + if on_retry: + on_retry(attempt, last_error) + backoff = 2 ** (attempt - 1) + time.sleep(backoff) + continue + break + + return None, last_error + + +class PushBranchResult(TypedDict, total=False): + """Result of pushing a branch to remote.""" + + success: bool + branch: str + remote: str + error: str + + +class PullRequestResult(TypedDict, total=False): + """Result of creating a pull request.""" + + success: bool + pr_url: str | None # None when PR was created but URL couldn't be extracted + already_exists: bool + error: str + message: str + + +class PushAndCreatePRResult(TypedDict, total=False): + """Result of push_and_create_pr operation.""" + + success: bool + pushed: bool + remote: str + branch: str + pr_url: str | None # None when PR was created but URL couldn't be extracted + already_exists: bool + error: str + message: str class WorktreeError(Exception): @@ -57,6 +172,11 @@ class WorktreeManager: a corresponding branch auto-claude/{spec-name}. """ + # Timeout constants for subprocess operations + GIT_PUSH_TIMEOUT = 120 # 2 minutes for git push (network operations) + GH_CLI_TIMEOUT = 60 # 1 minute for gh CLI commands + GH_QUERY_TIMEOUT = 30 # 30 seconds for gh CLI queries + def __init__(self, project_dir: Path, base_branch: str | None = None): self.project_dir = project_dir self.base_branch = base_branch or self._detect_base_branch() @@ -194,7 +314,7 @@ class WorktreeManager: def get_worktree_path(self, spec_name: str) -> Path: """Get the worktree path for a spec (checks new and legacy locations).""" - # New path first + # New path first (.auto-claude/worktrees/tasks/) new_path = self.worktrees_dir / spec_name if new_path.exists(): return new_path @@ -684,6 +804,363 @@ class WorktreeManager: result = self._run_git(["status", "--porcelain"], cwd=cwd) return bool(result.stdout.strip()) + # ==================== PR Creation Methods ==================== + + def push_branch(self, spec_name: str, force: bool = False) -> PushBranchResult: + """ + Push a spec's branch to the remote origin with retry logic. + + Args: + spec_name: The spec folder name + force: Whether to force push (use with caution) + + Returns: + PushBranchResult with keys: + - success: bool + - branch: str (branch name) + - remote: str (if successful) + - error: str (if failed) + """ + info = self.get_worktree_info(spec_name) + if not info: + return PushBranchResult( + success=False, + error=f"No worktree found for spec: {spec_name}", + ) + + # Push the branch to origin + push_args = ["push", "-u", "origin", info.branch] + if force: + push_args.insert(1, "--force") + + def do_push() -> tuple[bool, PushBranchResult | None, str]: + """Execute push operation for retry wrapper.""" + try: + git_executable = get_git_executable() + result = subprocess.run( + [git_executable] + push_args, + cwd=info.path, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=self.GIT_PUSH_TIMEOUT, + ) + + if result.returncode == 0: + return ( + True, + PushBranchResult( + success=True, + branch=info.branch, + remote="origin", + ), + "", + ) + return (False, None, result.stderr) + except FileNotFoundError: + return (False, None, "git executable not found") + + max_retries = 3 + result, last_error = _with_retry( + operation=do_push, + max_retries=max_retries, + is_retryable=_is_retryable_network_error, + ) + + if result: + return result + + # Handle timeout error message + if last_error == "Operation timed out": + return PushBranchResult( + success=False, + branch=info.branch, + error=f"Push timed out after {max_retries} attempts.", + ) + + return PushBranchResult( + success=False, + branch=info.branch, + error=f"Failed to push branch: {last_error}", + ) + + def create_pull_request( + self, + spec_name: str, + target_branch: str | None = None, + title: str | None = None, + draft: bool = False, + ) -> PullRequestResult: + """ + Create a GitHub pull request for a spec's branch using gh CLI with retry logic. + + Args: + spec_name: The spec folder name + target_branch: Target branch for PR (defaults to base_branch) + title: PR title (defaults to spec name) + draft: Whether to create as draft PR + + Returns: + PullRequestResult with keys: + - success: bool + - pr_url: str (if created) + - already_exists: bool (if PR already exists) + - error: str (if failed) + """ + info = self.get_worktree_info(spec_name) + if not info: + return PullRequestResult( + success=False, + error=f"No worktree found for spec: {spec_name}", + ) + + target = target_branch or self.base_branch + pr_title = title or f"auto-claude: {spec_name}" + + # Get PR body from spec.md if available + pr_body = self._extract_spec_summary(spec_name) + + # Build gh pr create command + gh_args = [ + "gh", + "pr", + "create", + "--base", + target, + "--head", + info.branch, + "--title", + pr_title, + "--body", + pr_body, + ] + if draft: + gh_args.append("--draft") + + def is_pr_retryable(stderr: str) -> bool: + """Check if PR creation error is retryable (network or HTTP 5xx).""" + return _is_retryable_network_error(stderr) or _is_retryable_http_error( + stderr + ) + + def do_create_pr() -> tuple[bool, PullRequestResult | None, str]: + """Execute PR creation for retry wrapper.""" + try: + result = subprocess.run( + gh_args, + cwd=info.path, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=self.GH_CLI_TIMEOUT, + ) + + # Check for "already exists" case (success, no retry needed) + if result.returncode != 0 and "already exists" in result.stderr.lower(): + existing_url = self._get_existing_pr_url(spec_name, target) + result_dict = PullRequestResult( + success=True, + pr_url=existing_url, + already_exists=True, + ) + if existing_url is None: + result_dict["message"] = ( + "PR already exists but URL could not be retrieved" + ) + return (True, result_dict, "") + + if result.returncode == 0: + # Extract PR URL from output + pr_url: str | None = result.stdout.strip() + if not pr_url.startswith("http"): + # Try to find URL in output + # Use general pattern to support GitHub Enterprise instances + # Matches any HTTPS URL with /pull/ path + match = re.search(r"https://[^\s]+/pull/\d+", result.stdout) + if match: + pr_url = match.group(0) + else: + # Invalid output - no valid URL found + pr_url = None + + return ( + True, + PullRequestResult( + success=True, + pr_url=pr_url, + already_exists=False, + ), + "", + ) + + return (False, None, result.stderr) + + except FileNotFoundError: + # gh CLI not installed - not retryable, raise to exit retry loop + raise + + max_retries = 3 + try: + result, last_error = _with_retry( + operation=do_create_pr, + max_retries=max_retries, + is_retryable=is_pr_retryable, + ) + + if result: + return result + + # Handle timeout error message + if last_error == "Operation timed out": + return PullRequestResult( + success=False, + error=f"PR creation timed out after {max_retries} attempts.", + ) + + return PullRequestResult( + success=False, + error=f"Failed to create PR: {last_error}", + ) + + except FileNotFoundError: + # gh CLI not installed + return PullRequestResult( + success=False, + error="gh CLI not found. Install from https://cli.github.com/", + ) + + def _extract_spec_summary(self, spec_name: str) -> str: + """Extract a summary from spec.md for PR body.""" + worktree_path = self.get_worktree_path(spec_name) + spec_path = worktree_path / ".auto-claude" / "specs" / spec_name / "spec.md" + + if not spec_path.exists(): + # Try project spec path + spec_path = ( + self.project_dir / ".auto-claude" / "specs" / spec_name / "spec.md" + ) + + if not spec_path.exists(): + return "Auto-generated PR from Auto-Claude build." + + try: + content = spec_path.read_text(encoding="utf-8") + # Extract first few paragraphs (skip title, get overview) + lines = content.split("\n") + summary_lines = [] + in_content = False + + for line in lines: + # Skip title headers + if line.startswith("# "): + continue + # Start capturing after first content line + if line.strip() and not line.startswith("#"): + in_content = True + if in_content: + if line.startswith("## ") and summary_lines: + break # Stop at next section + summary_lines.append(line) + if len(summary_lines) >= 10: # Limit to ~10 lines + break + + summary = "\n".join(summary_lines).strip() + if summary: + return summary + except (OSError, UnicodeDecodeError) as e: + # Silently fall back to default - file read errors shouldn't block PR creation + debug_warning( + "worktree", f"Could not extract spec summary for PR body: {e}" + ) + + return "Auto-generated PR from Auto-Claude build." + + def _get_existing_pr_url(self, spec_name: str, target_branch: str) -> str | None: + """Get the URL of an existing PR for this branch.""" + info = self.get_worktree_info(spec_name) + if not info: + return None + + try: + result = subprocess.run( + ["gh", "pr", "view", info.branch, "--json", "url", "--jq", ".url"], + cwd=info.path, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=self.GH_QUERY_TIMEOUT, + ) + if result.returncode == 0: + return result.stdout.strip() + except ( + subprocess.TimeoutExpired, + FileNotFoundError, + subprocess.SubprocessError, + ) as e: + # Silently ignore errors when fetching existing PR URL - this is a best-effort + # lookup that may fail due to network issues, missing gh CLI, or auth problems. + # Returning None allows the caller to handle missing URLs gracefully. + debug_warning("worktree", f"Could not get existing PR URL: {e}") + + return None + + def push_and_create_pr( + self, + spec_name: str, + target_branch: str | None = None, + title: str | None = None, + draft: bool = False, + force_push: bool = False, + ) -> PushAndCreatePRResult: + """ + Push branch and create a pull request in one operation. + + Args: + spec_name: The spec folder name + target_branch: Target branch for PR (defaults to base_branch) + title: PR title (defaults to spec name) + draft: Whether to create as draft PR + force_push: Whether to force push the branch + + Returns: + PushAndCreatePRResult with keys: + - success: bool + - pr_url: str (if created) + - pushed: bool (if push succeeded) + - already_exists: bool (if PR already exists) + - error: str (if failed) + """ + # Step 1: Push the branch + push_result = self.push_branch(spec_name, force=force_push) + if not push_result.get("success"): + return PushAndCreatePRResult( + success=False, + pushed=False, + error=push_result.get("error", "Push failed"), + ) + + # Step 2: Create the PR + pr_result = self.create_pull_request( + spec_name=spec_name, + target_branch=target_branch, + title=title, + draft=draft, + ) + + # Combine results + return PushAndCreatePRResult( + success=pr_result.get("success", False), + pushed=True, + remote=push_result.get("remote"), + branch=push_result.get("branch"), + pr_url=pr_result.get("pr_url"), + already_exists=pr_result.get("already_exists", False), + error=pr_result.get("error"), + ) + # ==================== Worktree Cleanup Methods ==================== def get_old_worktrees( diff --git a/apps/backend/ui/icons.py b/apps/backend/ui/icons.py index 2f274961..13675eb3 100644 --- a/apps/backend/ui/icons.py +++ b/apps/backend/ui/icons.py @@ -39,9 +39,10 @@ class Icons: FILE = ("📄", "[F]") GEAR = ("⚙", "[*]") SEARCH = ("🔍", "[?]") - BRANCH = ("", "[B]") + BRANCH = ("🌿", "[BR]") # [BR] to avoid collision with BLOCKED [B] COMMIT = ("◉", "(@)") LIGHTNING = ("⚡", "!") + LINK = ("🔗", "[L]") # For PR URLs # Progress SUBTASK = ("▣", "#") diff --git a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts index 1d9e0540..f3ca37d4 100644 --- a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts +++ b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts @@ -302,7 +302,7 @@ describe('Subprocess Spawn Integration', () => { await manager.startTaskExecution('task-2', TEST_PROJECT_PATH, 'spec-001'); expect(manager.getRunningTasks()).toHaveLength(2); - }); + }, 15000); it('should use configured Python path', async () => { const { spawn } = await import('child_process'); diff --git a/apps/frontend/src/main/agent/agent-process.test.ts b/apps/frontend/src/main/agent/agent-process.test.ts index db992bb5..c06b8f68 100644 --- a/apps/frontend/src/main/agent/agent-process.test.ts +++ b/apps/frontend/src/main/agent/agent-process.test.ts @@ -268,10 +268,10 @@ describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { await processManager.spawnProcess('task-1', '/fake/cwd', ['run.py'], {}, 'task-execution'); const envArg = spawnCalls[0].options.env as Record; - + // OAuth token should be present expect(envArg.CLAUDE_CODE_OAUTH_TOKEN).toBe('oauth-token-456'); - + // Stale ANTHROPIC_* vars should be cleared (empty string overrides process.env) expect(envArg.ANTHROPIC_AUTH_TOKEN).toBe(''); expect(envArg.ANTHROPIC_BASE_URL).toBe(''); @@ -292,7 +292,7 @@ describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { await processManager.spawnProcess('task-1', '/fake/cwd', ['run.py'], {}, 'task-execution'); const envArg = spawnCalls[0].options.env as Record; - + // Should clear the base URL (so Python uses default api.anthropic.com) expect(envArg.ANTHROPIC_BASE_URL).toBe(''); expect(envArg.CLAUDE_CODE_OAUTH_TOKEN).toBe('oauth-token-789'); @@ -314,7 +314,7 @@ describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { await processManager.spawnProcess('task-1', '/fake/cwd', ['run.py'], {}, 'task-execution'); const envArg = spawnCalls[0].options.env as Record; - + // Should use API profile vars, NOT clear them expect(envArg.ANTHROPIC_AUTH_TOKEN).toBe('sk-profile-active'); expect(envArg.ANTHROPIC_BASE_URL).toBe('https://active-profile.com'); diff --git a/apps/frontend/src/main/agent/env-utils.ts b/apps/frontend/src/main/agent/env-utils.ts index ba384dfa..3c479e60 100644 --- a/apps/frontend/src/main/agent/env-utils.ts +++ b/apps/frontend/src/main/agent/env-utils.ts @@ -4,18 +4,18 @@ /** * Get environment variables to clear ANTHROPIC_* vars when in OAuth mode - * - * When switching from API Profile mode to OAuth mode, residual ANTHROPIC_* + * + * When switching from API Profile mode to OAuth mode, residual ANTHROPIC_* * environment variables from process.env can cause authentication failures. * This function returns an object with empty strings for these vars when * no API profile is active, ensuring OAuth tokens are used correctly. - * + * * **Why empty strings?** Setting environment variables to empty strings (rather than * undefined) ensures they override any stale values from process.env. Python's SDK * treats empty strings as falsy in conditional checks like `if token:`, so empty * strings effectively disable these authentication parameters without leaving * undefined values that might be ignored during object spreading. - * + * * @param apiProfileEnv - Environment variables from getAPIProfileEnv() * @returns Object with empty ANTHROPIC_* vars if in OAuth mode, empty object otherwise */ diff --git a/apps/frontend/src/main/ipc-handlers/context/utils.ts b/apps/frontend/src/main/ipc-handlers/context/utils.ts index c8157517..6611e997 100644 --- a/apps/frontend/src/main/ipc-handlers/context/utils.ts +++ b/apps/frontend/src/main/ipc-handlers/context/utils.ts @@ -131,7 +131,7 @@ export interface EmbeddingValidationResult { /** * Validate embedding configuration based on the configured provider * Supports: openai, ollama, google, voyage, azure_openai - * + * * @returns validation result with provider info and reason if invalid */ export function validateEmbeddingConfiguration( diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index a8fea6d4..04fbd64a 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -181,10 +181,10 @@ export interface PRReviewMemory { /** * Save PR review insights to the Electron memory layer (LadybugDB) - * + * * Called after a PR review completes to persist learnings for cross-session context. * Extracts key findings, patterns, and gotchas from the review result. - * + * * @param result The completed PR review result * @param repo Repository name (owner/repo) * @param isFollowup Whether this is a follow-up review @@ -208,14 +208,14 @@ async function savePRReviewToMemory( // Build the memory content with comprehensive insights // We want to capture ALL meaningful findings so the AI can learn from patterns - + // Prioritize findings: critical > high > medium > low // Include all critical/high, top 5 medium, top 3 low const criticalFindings = result.findings.filter(f => f.severity === 'critical'); const highFindings = result.findings.filter(f => f.severity === 'high'); const mediumFindings = result.findings.filter(f => f.severity === 'medium').slice(0, 5); const lowFindings = result.findings.filter(f => f.severity === 'low').slice(0, 3); - + const keyFindingsToSave = [ ...criticalFindings, ...highFindings, @@ -233,8 +233,8 @@ async function savePRReviewToMemory( // Extract gotchas: security issues, critical bugs, and common mistakes const gotchaCategories = ['security', 'error_handling', 'data_validation', 'race_condition']; const gotchasToSave = result.findings - .filter(f => - f.severity === 'critical' || + .filter(f => + f.severity === 'critical' || f.severity === 'high' || gotchaCategories.includes(f.category?.toLowerCase() || '') ) @@ -246,7 +246,7 @@ async function savePRReviewToMemory( acc[cat] = (acc[cat] || 0) + 1; return acc; }, {} as Record); - + // Patterns are categories that appear multiple times (indicates a systematic issue) const patternsToSave = Object.entries(categoryGroups) .filter(([_, count]) => count >= 2) @@ -275,7 +275,7 @@ async function savePRReviewToMemory( // Add follow-up specific info if applicable if (isFollowup && result.resolvedFindings && result.unresolvedFindings) { - memoryContent.summary.verdict_reasoning = + memoryContent.summary.verdict_reasoning = `Resolved: ${result.resolvedFindings.length}, Unresolved: ${result.unresolvedFindings.length}`; } @@ -296,8 +296,8 @@ async function savePRReviewToMemory( } catch (error) { // Don't fail the review if memory save fails - debugLog('Error saving PR review to memory', { - error: error instanceof Error ? error.message : error + debugLog('Error saving PR review to memory', { + error: error instanceof Error ? error.message : error }); } } @@ -1728,7 +1728,7 @@ export function registerPRHandlers( const hasPending = checkRuns.check_runs.some( cr => cr.status !== 'completed' ); - + if (hasFailing) { ciStatus = 'failing'; } else if (hasPending) { @@ -2124,7 +2124,7 @@ export function registerPRHandlers( const reviewPath = path.join(memoryDir, `pr_${entry.pr_number}_review.json`); if (fs.existsSync(reviewPath)) { const reviewContent = fs.readFileSync(reviewPath, 'utf-8'); - + // Check if content matches query if (reviewContent.toLowerCase().includes(queryLower)) { const memory = JSON.parse(sanitizeNetworkData(reviewContent)); diff --git a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts b/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts index 49c504b0..8e22bc28 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts +++ b/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts @@ -49,12 +49,12 @@ describe('runPythonSubprocess', () => { // Arrange const pythonPath = '/path/with spaces/python'; const mockArgs = ['-c', 'print("hello")']; - - // Mock parsePythonCommand to return the path split logic if needed, - // or just rely on the mock above. + + // Mock parsePythonCommand to return the path split logic if needed, + // or just rely on the mock above. // Let's make sure our mock enables the scenario we want. vi.mocked(parsePythonCommand).mockReturnValue(['/path/with spaces/python', []]); - + // Act runPythonSubprocess({ pythonPath, diff --git a/apps/frontend/src/main/ipc-handlers/task/plan-file-utils.ts b/apps/frontend/src/main/ipc-handlers/task/plan-file-utils.ts index 933d0c5a..f90a3231 100644 --- a/apps/frontend/src/main/ipc-handlers/task/plan-file-utils.ts +++ b/apps/frontend/src/main/ipc-handlers/task/plan-file-utils.ts @@ -81,6 +81,8 @@ export function mapStatusToPlanStatus(status: TaskStatus): string { case 'ai_review': case 'human_review': return 'review'; + case 'pr_created': + return 'pr_created'; case 'done': return 'completed'; default: @@ -262,3 +264,41 @@ export async function createPlanIfNotExists( writeFileSync(planPath, JSON.stringify(plan, null, 2)); }); } + +/** + * Update task_metadata.json to add PR URL. + * This is a simple JSON file update (no locking needed as it's rarely updated concurrently). + * + * @param metadataPath - Path to the task_metadata.json file + * @param prUrl - The PR URL to add to metadata + * @returns true if metadata was updated, false if file doesn't exist or failed + */ +export function updateTaskMetadataPrUrl(metadataPath: string, prUrl: string): boolean { + try { + let metadata: Record = {}; + + // Try to read existing metadata + try { + const content = readFileSync(metadataPath, 'utf-8'); + metadata = JSON.parse(content); + } catch (err) { + if (!isFileNotFoundError(err)) { + throw err; + } + // File doesn't exist, will create new one + } + + // Update with prUrl + metadata.prUrl = prUrl; + + // Ensure parent directory exists before writing + mkdirSync(path.dirname(metadataPath), { recursive: true }); + + // Write back + writeFileSync(metadataPath, JSON.stringify(metadata, null, 2)); + return true; + } catch (err) { + console.warn(`[plan-file-utils] Could not update metadata at ${metadataPath}:`, err); + return false; + } +} diff --git a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts index 8c97ae76..30e8ca52 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -1,10 +1,12 @@ import { ipcMain, BrowserWindow, shell, app } from 'electron'; -import { IPC_CHANNELS, AUTO_BUILD_PATHS, DEFAULT_APP_SETTINGS, DEFAULT_FEATURE_MODELS, DEFAULT_FEATURE_THINKING, MODEL_ID_MAP, THINKING_BUDGET_MAP } from '../../../shared/constants'; -import type { IPCResult, WorktreeStatus, WorktreeDiff, WorktreeDiffFile, WorktreeMergeResult, WorktreeDiscardResult, WorktreeListResult, WorktreeListItem, SupportedIDE, SupportedTerminal, AppSettings } from '../../../shared/types'; +import { IPC_CHANNELS, AUTO_BUILD_PATHS, DEFAULT_APP_SETTINGS, DEFAULT_FEATURE_MODELS, DEFAULT_FEATURE_THINKING, MODEL_ID_MAP, THINKING_BUDGET_MAP, getSpecsDir } from '../../../shared/constants'; +import type { IPCResult, WorktreeStatus, WorktreeDiff, WorktreeDiffFile, WorktreeMergeResult, WorktreeDiscardResult, WorktreeListResult, WorktreeListItem, WorktreeCreatePROptions, WorktreeCreatePRResult, SupportedIDE, SupportedTerminal, AppSettings } from '../../../shared/types'; import path from 'path'; import { existsSync, readdirSync, statSync, readFileSync } from 'fs'; import { execSync, execFileSync, spawn, spawnSync, exec, execFile } from 'child_process'; -import { minimatch } from 'minimatch'; +import { createRequire } from 'module'; +const require = createRequire(import.meta.url); +const { minimatch } = require('minimatch'); import { projectStore } from '../../project-store'; import { getConfiguredPythonPath, PythonEnvManager, pythonEnvManager as pythonEnvManagerSingleton } from '../../python-env-manager'; import { getEffectiveSourcePath } from '../../updater/path-resolver'; @@ -17,6 +19,19 @@ import { getTaskWorktreeDir, findTaskWorktree, } from '../../worktree-paths'; +import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils'; + +// Regex pattern for validating git branch names +const GIT_BRANCH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/; + +// Maximum PR title length (GitHub's limit is 256 characters) +const MAX_PR_TITLE_LENGTH = 256; + +// Regex for validating PR title contains only printable characters +const PRINTABLE_CHARS_REGEX = /^[\x20-\x7E\u00A0-\uFFFF]*$/; + +// Timeout for PR creation operations (2 minutes for network operations) +const PR_CREATION_TIMEOUT_MS = 120000; /** * Read utility feature settings (for commit message, merge resolver) from settings file @@ -1285,7 +1300,10 @@ function getTaskBaseBranch(specDir: string): string | undefined { if (existsSync(metadataPath)) { const metadata = JSON.parse(readFileSync(metadataPath, 'utf-8')); // Return baseBranch if explicitly set (not the __project_default__ marker) - if (metadata.baseBranch && metadata.baseBranch !== '__project_default__') { + // Also validate it's a valid branch name to prevent malformed git commands + if (metadata.baseBranch && + metadata.baseBranch !== '__project_default__' && + GIT_BRANCH_REGEX.test(metadata.baseBranch)) { return metadata.baseBranch; } } @@ -1295,6 +1313,309 @@ function getTaskBaseBranch(specDir: string): string | undefined { return undefined; } +/** + * Get the effective base branch for a task with proper fallback chain. + * Priority: + * 1. Task metadata baseBranch (explicit task-level override from task_metadata.json) + * 2. Project settings mainBranch (project-level default) + * 3. Git default branch detection (main/master) + * 4. Fallback to 'main' + * + * This should be used instead of getting the current HEAD branch, + * as the user may be on a feature branch when viewing worktree status. + */ +function getEffectiveBaseBranch(projectPath: string, specId: string, projectMainBranch?: string): string { + // 1. Try task metadata baseBranch + const specDir = path.join(projectPath, '.auto-claude', 'specs', specId); + const taskBaseBranch = getTaskBaseBranch(specDir); + if (taskBaseBranch) { + return taskBaseBranch; + } + + // 2. Try project settings mainBranch + if (projectMainBranch && GIT_BRANCH_REGEX.test(projectMainBranch)) { + return projectMainBranch; + } + + // 3. Try to detect main/master branch + for (const branch of ['main', 'master']) { + try { + execFileSync(getToolPath('git'), ['rev-parse', '--verify', branch], { + cwd: projectPath, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return branch; + } catch { + // Branch doesn't exist, try next + } + } + + // 4. Fallback to 'main' + return 'main'; +} + +// ============================================ +// Helper functions for TASK_WORKTREE_CREATE_PR +// ============================================ + +/** + * Result of parsing JSON output from the create-pr Python script + */ +interface ParsedPRResult { + success: boolean; + prUrl?: string; + alreadyExists?: boolean; + error?: string; +} + +/** + * Validate that a URL is a valid GitHub PR URL. + * Supports both github.com and GitHub Enterprise instances (custom domains). + * Only requires HTTPS protocol and non-empty hostname to allow any GH Enterprise URL. + * @returns true if the URL is a valid HTTPS URL with a non-empty hostname + */ +function isValidGitHubUrl(url: string): boolean { + try { + const parsed = new URL(url); + // Only require HTTPS with non-empty hostname + // This supports GH Enterprise instances with custom domains + // The URL comes from gh CLI output which we trust to be valid + return parsed.protocol === 'https:' && parsed.hostname.length > 0; + } catch { + return false; + } +} + +/** + * Parse JSON output from the create-pr Python script + * Handles both snake_case and camelCase field names + * @returns ParsedPRResult if valid JSON found, null otherwise + */ +function parsePRJsonOutput(stdout: string): ParsedPRResult | null { + // Find the last complete JSON object in stdout (non-greedy, handles multiple objects) + const jsonMatches = stdout.match(/\{[^{}]*(?:\{[^{}]*\}[^{}]*)*\}/g); + const jsonMatch = jsonMatches && jsonMatches.length > 0 ? jsonMatches[jsonMatches.length - 1] : null; + + if (!jsonMatch) { + return null; + } + + try { + const parsed = JSON.parse(jsonMatch); + + // Validate parsed JSON has expected shape + if (typeof parsed !== 'object' || parsed === null) { + return null; + } + + // Extract and validate fields with proper type checking + // Handle both snake_case (from Python) and camelCase field names + // Default success to false to avoid masking failures when field is missing + const rawPrUrl = typeof parsed.pr_url === 'string' ? parsed.pr_url : + typeof parsed.prUrl === 'string' ? parsed.prUrl : undefined; + + // Validate PR URL is a valid GitHub URL for robustness + const validatedPrUrl = rawPrUrl && isValidGitHubUrl(rawPrUrl) ? rawPrUrl : undefined; + + return { + success: typeof parsed.success === 'boolean' ? parsed.success : false, + prUrl: validatedPrUrl, + alreadyExists: typeof parsed.already_exists === 'boolean' ? parsed.already_exists : + typeof parsed.alreadyExists === 'boolean' ? parsed.alreadyExists : undefined, + error: typeof parsed.error === 'string' ? parsed.error : undefined + }; + } catch { + return null; + } +} + +/** + * Result of updating task status after PR creation + */ +interface TaskStatusUpdateResult { + mainProjectStatus: boolean; + mainProjectMetadata: boolean; + worktreeStatus: boolean; + worktreeMetadata: boolean; +} + +/** + * Update task status and metadata after PR creation + * Updates both main project and worktree locations + * @returns Result object indicating which updates succeeded/failed + */ +async function updateTaskStatusAfterPRCreation( + specDir: string, + worktreePath: string | null, + prUrl: string, + autoBuildPath: string | undefined, + specId: string, + debug: (...args: unknown[]) => void +): Promise { + const result: TaskStatusUpdateResult = { + mainProjectStatus: false, + mainProjectMetadata: false, + worktreeStatus: false, + worktreeMetadata: false + }; + + const planPath = path.join(specDir, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN); + const metadataPath = path.join(specDir, 'task_metadata.json'); + + // Await status persistence to ensure completion before resolving + try { + const persisted = await persistPlanStatus(planPath, 'pr_created'); + result.mainProjectStatus = persisted; + debug('Main project status persisted to pr_created:', persisted); + } catch (err) { + debug('Failed to persist main project status:', err); + } + + // Update metadata with prUrl in main project + result.mainProjectMetadata = updateTaskMetadataPrUrl(metadataPath, prUrl); + debug('Main project metadata updated with prUrl:', result.mainProjectMetadata); + + // Also persist to WORKTREE location (worktree takes priority when loading tasks) + // This ensures the status persists after refresh since getTasks() prefers worktree version + if (worktreePath) { + const specsBaseDir = getSpecsDir(autoBuildPath); + const worktreePlanPath = path.join(worktreePath, specsBaseDir, specId, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN); + const worktreeMetadataPath = path.join(worktreePath, specsBaseDir, specId, 'task_metadata.json'); + + try { + const persisted = await persistPlanStatus(worktreePlanPath, 'pr_created'); + result.worktreeStatus = persisted; + debug('Worktree status persisted to pr_created:', persisted); + } catch (err) { + debug('Failed to persist worktree status:', err); + } + + result.worktreeMetadata = updateTaskMetadataPrUrl(worktreeMetadataPath, prUrl); + debug('Worktree metadata updated with prUrl:', result.worktreeMetadata); + } + + return result; +} + +/** + * Build arguments for the create-pr Python script + */ +function buildCreatePRArgs( + runScript: string, + specId: string, + projectPath: string, + options: WorktreeCreatePROptions | undefined, + taskBaseBranch: string | undefined +): { args: string[]; validationError?: string } { + const args = [ + runScript, + '--spec', specId, + '--project-dir', projectPath, + '--create-pr' + ]; + + // Add optional arguments with validation + if (options?.targetBranch) { + // Validate branch name to prevent malformed git commands + if (!GIT_BRANCH_REGEX.test(options.targetBranch)) { + return { args: [], validationError: 'Invalid target branch name' }; + } + args.push('--pr-target', options.targetBranch); + } + if (options?.title) { + // Validate title for printable characters and length limit + if (options.title.length > MAX_PR_TITLE_LENGTH) { + return { args: [], validationError: `PR title exceeds maximum length of ${MAX_PR_TITLE_LENGTH} characters` }; + } + if (!PRINTABLE_CHARS_REGEX.test(options.title)) { + return { args: [], validationError: 'PR title contains invalid characters' }; + } + args.push('--pr-title', options.title); + } + if (options?.draft) { + args.push('--pr-draft'); + } + + // Add --base-branch if task was created with a specific base branch + if (taskBaseBranch) { + args.push('--base-branch', taskBaseBranch); + } + + return { args }; +} + +/** + * Initialize Python environment for PR creation + * @returns Error message if initialization fails, undefined on success + */ +async function initializePythonEnvForPR( + pythonEnvManager: PythonEnvManager +): Promise { + if (pythonEnvManager.isEnvReady()) { + return undefined; + } + + const autoBuildSource = getEffectiveSourcePath(); + if (!autoBuildSource) { + return 'Python environment not ready and Auto Claude source not found'; + } + + const status = await pythonEnvManager.initialize(autoBuildSource); + if (!status.ready) { + return `Python environment not ready: ${status.error || 'Unknown error'}`; + } + + return undefined; +} + +/** + * Generic retry wrapper with exponential backoff + * @param operation - Async function to execute with retry + * @param options - Retry configuration options + * @returns Result of the operation or throws after all retries + */ +async function withRetry( + operation: () => Promise, + options: { + maxRetries?: number; + baseDelayMs?: number; + onRetry?: (attempt: number, error: unknown) => void; + shouldRetry?: (error: unknown) => boolean; + } = {} +): Promise { + const { maxRetries: rawMaxRetries = 3, baseDelayMs = 100, onRetry, shouldRetry } = options; + + // Ensure at least one attempt is made (clamp to minimum of 1) + const maxRetries = Math.max(1, rawMaxRetries); + + for (let attempt = 1; attempt <= maxRetries; attempt++) { + try { + return await operation(); + } catch (error) { + const isLastAttempt = attempt === maxRetries; + + // Check if we should retry this error + if (shouldRetry && !shouldRetry(error)) { + throw error; + } + + if (isLastAttempt) { + throw error; + } + + // Notify about retry + onRetry?.(attempt, error); + + // Wait before retry (exponential backoff) + await new Promise(r => setTimeout(r, baseDelayMs * Math.pow(2, attempt - 1))); + } + } + + // This should never be reached, but TypeScript needs it + throw new Error('Retry loop exited unexpectedly'); +} + /** * Register worktree management handlers */ @@ -1333,17 +1654,10 @@ export function registerWorktreeHandlers( encoding: 'utf-8' }).trim(); - // Get base branch - the current branch in the main project (where changes will be merged) - // This matches the Python merge logic which merges into the user's current branch - let baseBranch = 'main'; - try { - baseBranch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: project.path, - encoding: 'utf-8' - }).trim(); - } catch { - baseBranch = 'main'; - } + // 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, task.specId, project.settings?.mainBranch); // Get commit count (cross-platform - no shell syntax) let commitCount = 0; @@ -1432,16 +1746,10 @@ export function registerWorktreeHandlers( return { success: false, error: 'No worktree found for this task' }; } - // Get base branch - the current branch in the main project (where changes will be merged) - let baseBranch = 'main'; - try { - baseBranch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: project.path, - encoding: 'utf-8' - }).trim(); - } catch { - baseBranch = 'main'; - } + // 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, task.specId, project.settings?.mainBranch); // Get the diff with file stats const files: WorktreeDiffFile[] = []; @@ -1516,14 +1824,15 @@ export function registerWorktreeHandlers( ipcMain.handle( IPC_CHANNELS.TASK_WORKTREE_MERGE, async (_, taskId: string, options?: { noCommit?: boolean }): Promise> => { - // Always log merge operations for debugging + const isDebugMode = process.env.DEBUG === 'true' || process.env.NODE_ENV === 'development'; const debug = (...args: unknown[]) => { - console.warn('[MERGE DEBUG]', ...args); + if (isDebugMode) { + console.warn('[MERGE DEBUG]', ...args); + } }; try { - console.warn('[MERGE] Handler called with taskId:', taskId, 'options:', options); - debug('Starting merge for taskId:', taskId, 'options:', options); + debug('Handler called with taskId:', taskId, 'options:', options); // Ensure Python environment is ready if (!pythonEnvManager.isEnvReady()) { @@ -1628,10 +1937,10 @@ export function registerWorktreeHandlers( const taskBaseBranch = getTaskBaseBranch(specDir); const projectMainBranch = project.settings?.mainBranch; const effectiveBaseBranch = taskBaseBranch || projectMainBranch; - + if (effectiveBaseBranch) { args.push('--base-branch', effectiveBaseBranch); - debug('Using base branch:', effectiveBaseBranch, + debug('Using base branch:', effectiveBaseBranch, `(source: ${taskBaseBranch ? 'task metadata' : 'project settings'})`); } @@ -1940,50 +2249,54 @@ export function registerWorktreeHandlers( const { promises: fsPromises } = require('fs'); - // Fire and forget - don't block the response on file writes - // But add retry logic for transient failures and verification + // Update plan file with retry logic for transient failures // Uses EAFP pattern (try/catch) instead of LBYL (existsSync check) to avoid TOCTOU race conditions - const updatePlanWithRetry = async (planPath: string, isMain: boolean, maxRetries = 3) => { - for (let attempt = 1; attempt <= maxRetries; attempt++) { - try { - const planContent = await fsPromises.readFile(planPath, 'utf-8'); - const plan = JSON.parse(planContent); - plan.status = newStatus; - plan.planStatus = planStatus; - plan.updated_at = new Date().toISOString(); - if (staged) { - plan.stagedAt = new Date().toISOString(); - plan.stagedInMainProject = true; - } - await fsPromises.writeFile(planPath, JSON.stringify(plan, null, 2)); + const updatePlanWithRetry = async (planPath: string, isMain: boolean): Promise => { + // Helper to check if error is ENOENT (file not found) + const isFileNotFound = (err: unknown): boolean => + !!(err && typeof err === 'object' && 'code' in err && err.code === 'ENOENT'); - // Verify the write succeeded by reading back - const verifyContent = await fsPromises.readFile(planPath, 'utf-8'); - const verifyPlan = JSON.parse(verifyContent); - if (verifyPlan.status === newStatus && verifyPlan.planStatus === planStatus) { - return true; // Write verified - } - throw new Error('Write verification failed - status mismatch'); - } catch (persistError: unknown) { - // File doesn't exist - nothing to update (not an error) - if (persistError && typeof persistError === 'object' && 'code' in persistError && persistError.code === 'ENOENT') { - return true; - } - const isLastAttempt = attempt === maxRetries; - if (isLastAttempt) { - // Only log error if main plan fails; worktree plan might legitimately be missing or read-only - if (isMain) { - console.error('Failed to persist task status to main plan after retries:', persistError); - } else { - debug('Failed to persist task status to worktree plan (non-critical):', persistError); + try { + await withRetry( + async () => { + const planContent = await fsPromises.readFile(planPath, 'utf-8'); + const plan = JSON.parse(planContent); + plan.status = newStatus; + plan.planStatus = planStatus; + plan.updated_at = new Date().toISOString(); + if (staged) { + plan.stagedAt = new Date().toISOString(); + plan.stagedInMainProject = true; } - return false; + await fsPromises.writeFile(planPath, JSON.stringify(plan, null, 2)); + + // Verify the write succeeded by reading back + const verifyContent = await fsPromises.readFile(planPath, 'utf-8'); + const verifyPlan = JSON.parse(verifyContent); + if (verifyPlan.status !== newStatus || verifyPlan.planStatus !== planStatus) { + throw new Error('Write verification failed - status mismatch'); + } + }, + { + maxRetries: 3, + baseDelayMs: 100, + shouldRetry: (err) => !isFileNotFound(err) // Don't retry if file doesn't exist } - // Wait before retry (exponential backoff: 100ms, 200ms, 400ms) - await new Promise(r => setTimeout(r, 100 * Math.pow(2, attempt - 1))); + ); + return true; + } catch (err) { + // File doesn't exist - nothing to update (not an error) + if (isFileNotFound(err)) { + return true; } + // Only log error if main plan fails; worktree plan might legitimately be missing or read-only + if (isMain) { + console.error('Failed to persist task status to main plan after retries:', err); + } else { + debug('Failed to persist task status to worktree plan (non-critical):', err); + } + return false; } - return false; }; const updatePlans = async () => { @@ -2160,10 +2473,10 @@ export function registerWorktreeHandlers( const taskBaseBranch = getTaskBaseBranch(specDir); const projectMainBranch = project.settings?.mainBranch; const effectiveBaseBranch = taskBaseBranch || projectMainBranch; - + if (effectiveBaseBranch) { args.push('--base-branch', effectiveBaseBranch); - console.warn('[IPC] Using base branch for preview:', effectiveBaseBranch, + console.warn('[IPC] Using base branch for preview:', effectiveBaseBranch, `(source: ${taskBaseBranch ? 'task metadata' : 'project settings'})`); } @@ -2373,16 +2686,10 @@ export function registerWorktreeHandlers( encoding: 'utf-8' }).trim(); - // Get base branch - the current branch in the main project (where changes will be merged) - let baseBranch = 'main'; - try { - baseBranch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: project.path, - encoding: 'utf-8' - }).trim(); - } catch { - baseBranch = 'main'; - } + // 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); // Get commit count (cross-platform - no shell syntax) let commitCount = 0; @@ -2536,4 +2843,276 @@ export function registerWorktreeHandlers( } } ); + + /** + * Create a Pull Request from the worktree branch + * Pushes the branch to origin and creates a GitHub PR using gh CLI + */ + ipcMain.handle( + IPC_CHANNELS.TASK_WORKTREE_CREATE_PR, + async (_, taskId: string, options?: WorktreeCreatePROptions): Promise> => { + const isDebugMode = process.env.DEBUG === 'true' || process.env.NODE_ENV === 'development'; + const debug = (...args: unknown[]) => { + if (isDebugMode) { + console.warn('[CREATE_PR DEBUG]', ...args); + } + }; + + try { + debug('Handler called with taskId:', taskId, 'options:', options); + + // Ensure Python environment is ready + const pythonEnvError = await initializePythonEnvForPR(pythonEnvManager); + if (pythonEnvError) { + return { success: false, error: pythonEnvError }; + } + + const { task, project } = findTaskAndProject(taskId); + if (!task || !project) { + debug('Task or project not found'); + return { success: false, error: 'Task not found' }; + } + + debug('Found task:', task.specId, 'project:', project.path); + + // Use run.py --create-pr to handle the PR creation + const sourcePath = getEffectiveSourcePath(); + if (!sourcePath) { + return { success: false, error: 'Auto Claude source not found' }; + } + + const runScript = path.join(sourcePath, 'run.py'); + const specDir = path.join(project.path, project.autoBuildPath || '.auto-claude', 'specs', task.specId); + + // Use EAFP pattern - try to read specDir and catch ENOENT + try { + statSync(specDir); + } catch (err) { + if (err && typeof err === 'object' && 'code' in err && err.code === 'ENOENT') { + debug('Spec directory not found:', specDir); + return { success: false, error: 'Spec directory not found' }; + } + throw err; // Re-throw unexpected errors + } + + // Check worktree exists before creating PR + const worktreePath = findTaskWorktree(project.path, task.specId); + if (!worktreePath) { + debug('No worktree found for spec:', task.specId); + return { success: false, error: 'No worktree found for this task' }; + } + debug('Worktree path:', worktreePath); + + // Build arguments using helper function + const taskBaseBranch = getTaskBaseBranch(specDir); + const { args, validationError } = buildCreatePRArgs( + runScript, + task.specId, + project.path, + options, + taskBaseBranch + ); + if (validationError) { + return { success: false, error: validationError }; + } + if (taskBaseBranch) { + debug('Using stored base branch:', taskBaseBranch); + } + + // Use configured Python path + const pythonPath = getConfiguredPythonPath(); + debug('Running command:', pythonPath, args.join(' ')); + debug('Working directory:', sourcePath); + + // Get profile environment with OAuth token + const profileEnv = getProfileEnv(); + + return new Promise((resolve) => { + let timeoutId: NodeJS.Timeout | null = null; + let resolved = false; + + // Get Python environment for bundled packages + const pythonEnv = pythonEnvManagerSingleton.getPythonEnv(); + + // Parse Python command to handle space-separated commands like "py -3" + const [pythonCommand, pythonBaseArgs] = parsePythonCommand(pythonPath); + const createPRProcess = spawn(pythonCommand, [...pythonBaseArgs, ...args], { + cwd: sourcePath, + env: { + ...process.env, + ...pythonEnv, + ...profileEnv, + PYTHONUNBUFFERED: '1', + PYTHONUTF8: '1' + }, + stdio: ['ignore', 'pipe', 'pipe'] + }); + + let stdout = ''; + let stderr = ''; + + // Set up timeout to kill hung processes + timeoutId = setTimeout(() => { + if (!resolved) { + debug('TIMEOUT: Create PR process exceeded', PR_CREATION_TIMEOUT_MS, 'ms, killing...'); + resolved = true; + + // Platform-specific process termination with fallback + if (process.platform === 'win32') { + try { + createPRProcess.kill(); + // Fallback: forcefully kill with taskkill if process ignores initial kill + if (createPRProcess.pid) { + setTimeout(() => { + try { + spawn('taskkill', ['/pid', createPRProcess.pid!.toString(), '/f', '/t'], { + stdio: 'ignore', + detached: true + }).unref(); + } catch { + // Process may already be dead + } + }, 5000); + } + } catch { + // Process may already be dead + } + } else { + createPRProcess.kill('SIGTERM'); + setTimeout(() => { + try { + createPRProcess.kill('SIGKILL'); + } catch { + // Process may already be dead + } + }, 5000); + } + + resolve({ + success: false, + error: 'PR creation timed out. Check if the PR was created on GitHub.' + }); + } + }, PR_CREATION_TIMEOUT_MS); + + createPRProcess.stdout.on('data', (data: Buffer) => { + const chunk = data.toString(); + stdout += chunk; + debug('STDOUT:', chunk); + }); + + createPRProcess.stderr.on('data', (data: Buffer) => { + const chunk = data.toString(); + stderr += chunk; + debug('STDERR:', chunk); + }); + + /** + * Handle process exit - shared logic for both 'close' and 'exit' events. + * Parses JSON output, updates task status if PR was created, and resolves the promise. + * + * @param code - Process exit code (0 = success, non-zero = failure) + * @param eventSource - Which event triggered this ('close' or 'exit') for debug logging + */ + const handleCreatePRProcessExit = async (code: number | null, eventSource: 'close' | 'exit'): Promise => { + if (resolved) return; + resolved = true; + if (timeoutId) clearTimeout(timeoutId); + + debug(`Process exited via ${eventSource} event with code:`, code); + debug('Full stdout:', stdout); + debug('Full stderr:', stderr); + + if (code === 0) { + // Parse JSON output using helper function + const result = parsePRJsonOutput(stdout); + if (result) { + debug('Parsed result:', result); + + // Only update task status if a NEW PR was created (not if it already exists) + if (result.success !== false && result.prUrl && !result.alreadyExists) { + await updateTaskStatusAfterPRCreation( + specDir, + worktreePath, + result.prUrl, + project.autoBuildPath, + task.specId, + debug + ); + } else if (result.alreadyExists) { + debug('PR already exists, not updating task status'); + } + + resolve({ + success: true, + data: { + success: result.success, + prUrl: result.prUrl, + error: result.error, + alreadyExists: result.alreadyExists + } + }); + } else { + // No JSON found, but process succeeded + debug('No JSON in output, assuming success'); + resolve({ + success: true, + data: { + success: true, + prUrl: undefined + } + }); + } + } else { + debug('Process failed with code:', code); + + // Try to parse JSON from stdout even on failure + const result = parsePRJsonOutput(stdout); + if (result) { + debug('Parsed error result:', result); + resolve({ + success: false, + error: result.error || 'Failed to create PR' + }); + } else { + // Fallback to raw output if JSON parsing fails + // Prefer stdout over stderr since stderr often contains debug messages + resolve({ + success: false, + error: stdout || stderr || 'Failed to create PR' + }); + } + } + }; + + createPRProcess.on('close', (code: number | null) => { + handleCreatePRProcessExit(code, 'close'); + }); + + // Also listen to 'exit' event in case 'close' doesn't fire + createPRProcess.on('exit', (code: number | null) => { + // Give close event a chance to fire first with complete output + setTimeout(() => handleCreatePRProcessExit(code, 'exit'), 100); + }); + + createPRProcess.on('error', (err: Error) => { + if (resolved) return; + resolved = true; + if (timeoutId) clearTimeout(timeoutId); + debug('Process spawn error:', err); + resolve({ + success: false, + error: `Failed to run create-pr: ${err.message}` + }); + }); + }); + } catch (error) { + console.error('[CREATE_PR] Exception in handler:', error); + return { + success: false, + error: error instanceof Error ? error.message : 'Failed to create PR' + }; + } + } + ); } diff --git a/apps/frontend/src/main/memory-service.ts b/apps/frontend/src/main/memory-service.ts index c7dd4ccd..9e419cc8 100644 --- a/apps/frontend/src/main/memory-service.ts +++ b/apps/frontend/src/main/memory-service.ts @@ -139,7 +139,7 @@ function getBackendPythonPath(): string { const venvPython = process.platform === 'win32' ? path.join(backendPath, '.venv', 'Scripts', 'python.exe') : path.join(backendPath, '.venv', 'bin', 'python'); - + if (fs.existsSync(venvPython)) { console.log(`[MemoryService] Using backend venv Python: ${venvPython}`); return venvPython; @@ -159,7 +159,7 @@ function getBackendPythonPath(): string { function getMemoryPythonEnv(): Record { // Start with the standard Python environment from the manager const baseEnv = pythonEnvManager.getPythonEnv(); - + // For packaged apps, ensure PYTHONPATH includes bundled site-packages // even if the manager hasn't been fully initialized if (app.isPackaged) { @@ -172,7 +172,7 @@ function getMemoryPythonEnv(): Record { : bundledSitePackages; } } - + return baseEnv; } @@ -627,10 +627,10 @@ export class MemoryService { /** * Add an episode to the memory database - * + * * This allows the Electron app to save memories (like PR review insights) * directly to LadybugDB without going through the full Graphiti system. - * + * * @param name Episode name/title * @param content Episode content (will be JSON stringified if object) * @param episodeType Type of episode (session_insight, pattern, gotcha, task_outcome, pr_review) diff --git a/apps/frontend/src/main/project-store.ts b/apps/frontend/src/main/project-store.ts index dedc374f..a3153211 100644 --- a/apps/frontend/src/main/project-store.ts +++ b/apps/frontend/src/main/project-store.ts @@ -564,6 +564,7 @@ export class ProjectStore { 'done': 'done', 'human_review': 'human_review', 'ai_review': 'ai_review', + 'pr_created': 'pr_created', // PR has been created for this task 'backlog': 'backlog' }; const storedStatus = statusMap[plan.status]; @@ -573,6 +574,11 @@ export class ProjectStore { return { status: 'done' }; } + // If task has a PR created, always respect that status + if (storedStatus === 'pr_created') { + return { status: 'pr_created' }; + } + // For other stored statuses, validate against calculated status if (storedStatus) { // Planning/coding status from the backend should be respected even if subtasks aren't in progress yet diff --git a/apps/frontend/src/main/services/profile/profile-manager.ts b/apps/frontend/src/main/services/profile/profile-manager.ts index 83029f4b..6cb1bf1f 100644 --- a/apps/frontend/src/main/services/profile/profile-manager.ts +++ b/apps/frontend/src/main/services/profile/profile-manager.ts @@ -124,7 +124,7 @@ export async function saveProfilesFile(data: ProfilesFile): Promise { // Write file with formatted JSON const content = JSON.stringify(data, null, 2); await fs.writeFile(filePath, content, 'utf-8'); - + // Set secure file permissions (user read/write only - 0600) const permissionsValid = await validateFilePermissions(filePath); if (!permissionsValid) { @@ -167,17 +167,17 @@ export async function validateFilePermissions(filePath: string): Promise(fn: () => Promise): Promise { const filePath = getProfilesFilePath(); const dir = path.dirname(filePath); - + // Ensure directory and file exist before trying to lock await fs.mkdir(dir, { recursive: true }); - + // Create file if it doesn't exist (needed for lockfile to work) try { await fs.access(filePath); @@ -194,7 +194,7 @@ export async function withProfilesLock(fn: () => Promise): Promise { // EEXIST means another process won the race, proceed normally } } - + // Acquire lock with reasonable timeout let release: (() => Promise) | undefined; try { @@ -205,7 +205,7 @@ export async function withProfilesLock(fn: () => Promise): Promise { maxTimeout: 500 } }); - + // Execute the function while holding the lock return await fn(); } finally { @@ -219,7 +219,7 @@ export async function withProfilesLock(fn: () => Promise): Promise { /** * Atomically modify the profiles file * Loads, modifies, and saves the file within an exclusive lock - * + * * @param modifier Function that modifies the ProfilesFile * @returns The modified ProfilesFile */ @@ -229,25 +229,25 @@ export async function atomicModifyProfiles( return await withProfilesLock(async () => { // Load current state const file = await loadProfilesFile(); - + // Apply modification const modifiedFile = await modifier(file); - + // Save atomically (write to temp file and rename) const filePath = getProfilesFilePath(); const tempPath = `${filePath}.tmp`; - + try { // Write to temp file const content = JSON.stringify(modifiedFile, null, 2); await fs.writeFile(tempPath, content, 'utf-8'); - + // Set permissions on temp file await fs.chmod(tempPath, 0o600); - + // Atomically replace original file await fs.rename(tempPath, filePath); - + return modifiedFile; } catch (error) { // Clean up temp file on error diff --git a/apps/frontend/src/main/services/profile/profile-service.ts b/apps/frontend/src/main/services/profile/profile-service.ts index f3902049..c57f60ce 100644 --- a/apps/frontend/src/main/services/profile/profile-service.ts +++ b/apps/frontend/src/main/services/profile/profile-service.ts @@ -71,7 +71,7 @@ export function validateApiKey(apiKey: string): boolean { /** * Validate that profile name is unique (case-insensitive, trimmed) - * + * * WARNING: This is for UX feedback only. Do NOT rely on this for correctness. * The actual uniqueness check happens atomically inside create/update operations * to prevent TOCTOU race conditions. @@ -144,7 +144,7 @@ export async function createProfile(input: CreateProfileInput): Promise p.name.trim().toLowerCase() === trimmed ); - + if (exists) { throw new Error('A profile with this name already exists'); } @@ -420,10 +420,10 @@ export async function testConnection( const messagesErrorName = messagesError instanceof Error ? messagesError.name : ''; // 400/422 errors mean the endpoint is valid, just our test request was invalid // This is expected - we're just testing connectivity - if (messagesErrorName === 'BadRequestError' || + if (messagesErrorName === 'BadRequestError' || messagesErrorName === 'InvalidRequestError' || - (messagesError instanceof Error && 'status' in messagesError && - ((messagesError as { status?: number }).status === 400 || + (messagesError instanceof Error && 'status' in messagesError && + ((messagesError as { status?: number }).status === 400 || (messagesError as { status?: number }).status === 422))) { // Endpoint is valid, connection successful return { @@ -452,7 +452,7 @@ export async function testConnection( // Map SDK errors to TestConnectionResult error types // Use error.name for instanceof-like checks (works with mocks that set this.name) const errorName = error instanceof Error ? error.name : ''; - + if (errorName === 'AuthenticationError' || error instanceof AuthenticationError) { return { success: false, @@ -580,7 +580,7 @@ export async function discoverModels( // Map SDK errors to thrown errors with errorType property // Use error.name for instanceof-like checks (works with mocks that set this.name) const errorName = error instanceof Error ? error.name : ''; - + if (errorName === 'AuthenticationError' || error instanceof AuthenticationError) { const authError: Error & { errorType?: string } = new Error('Authentication failed. Please check your API key.'); authError.errorType = 'auth'; diff --git a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts index 739b58bd..92090e0b 100644 --- a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts +++ b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts @@ -1,8 +1,12 @@ import { writeFileSync } from 'fs'; +import { tmpdir } from 'os'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type * as pty from '@lydell/node-pty'; import type { TerminalProcess } from '../types'; +/** Escape special regex characters in a string for safe use in RegExp constructor */ +const escapeForRegex = (str: string): string => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const mockGetClaudeCliInvocation = vi.fn(); const mockGetClaudeProfileManager = vi.fn(); const mockPersistSession = vi.fn(); @@ -229,7 +233,8 @@ describe('claude-integration-handler', () => { const tokenPath = vi.mocked(writeFileSync).mock.calls[0]?.[0] as string; const tokenContents = vi.mocked(writeFileSync).mock.calls[0]?.[1] as string; - expect(tokenPath).toMatch(/^\/tmp\/\.claude-token-1234-[0-9a-f]{16}$/); + const tmpDir = escapeForRegex(tmpdir()); + expect(tokenPath).toMatch(new RegExp(`^${tmpDir}/\\.claude-token-1234-[0-9a-f]{16}$`)); expect(tokenContents).toBe("export CLAUDE_CODE_OAUTH_TOKEN='token-value'\n"); const written = vi.mocked(terminal.pty.write).mock.calls[0][0] as string; expect(written).toContain("HISTFILE= HISTCONTROL=ignorespace "); @@ -271,7 +276,8 @@ describe('claude-integration-handler', () => { const tokenPath = vi.mocked(writeFileSync).mock.calls[0]?.[0] as string; const tokenContents = vi.mocked(writeFileSync).mock.calls[0]?.[1] as string; - expect(tokenPath).toMatch(/^\/tmp\/\.claude-token-5678-[0-9a-f]{16}$/); + const tmpDir = escapeForRegex(tmpdir()); + expect(tokenPath).toMatch(new RegExp(`^${tmpDir}/\\.claude-token-5678-[0-9a-f]{16}$`)); expect(tokenContents).toBe("export CLAUDE_CODE_OAUTH_TOKEN='token-value'\n"); const written = vi.mocked(terminal.pty.write).mock.calls[0][0] as string; expect(written).toContain(`source '${tokenPath}'`); diff --git a/apps/frontend/src/preload/api/profile-api.ts b/apps/frontend/src/preload/api/profile-api.ts index e285c6f1..69b27890 100644 --- a/apps/frontend/src/preload/api/profile-api.ts +++ b/apps/frontend/src/preload/api/profile-api.ts @@ -84,7 +84,7 @@ export const createProfileAPI = (): ProfileAPI => ({ if (signal && signal.aborted) { return Promise.reject(new DOMException('The operation was aborted.', 'AbortError')); } - + // Setup abort listener AFTER checking aborted status to avoid race condition if (signal && typeof signal.addEventListener === 'function') { try { @@ -97,7 +97,7 @@ export const createProfileAPI = (): ProfileAPI => ({ } else if (signal) { console.warn('[preload/profile-api] signal provided but addEventListener not available - signal may have been serialized'); } - + return ipcRenderer.invoke(IPC_CHANNELS.PROFILES_TEST_CONNECTION, baseUrl, apiKey, requestId); }, diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index c2f3a678..27e55dd1 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -11,7 +11,9 @@ import type { TaskLogs, TaskLogStreamChunk, SupportedIDE, - SupportedTerminal + SupportedTerminal, + WorktreeCreatePROptions, + WorktreeCreatePRResult } from '../../shared/types'; export interface TaskAPI { @@ -57,6 +59,7 @@ export interface TaskAPI { worktreeDetectTools: () => Promise; terminals: Array<{ id: string; name: string; path: string; installed: boolean }> }>>; archiveTasks: (projectId: string, taskIds: string[], version?: string) => Promise>; unarchiveTasks: (projectId: string, taskIds: string[]) => Promise>; + createWorktreePR: (taskId: string, options?: WorktreeCreatePROptions) => Promise>; // Task Event Listeners // Note: projectId is optional for backward compatibility - events without projectId will still work @@ -160,6 +163,9 @@ export const createTaskAPI = (): TaskAPI => ({ unarchiveTasks: (projectId: string, taskIds: string[]): Promise> => ipcRenderer.invoke(IPC_CHANNELS.TASK_UNARCHIVE, projectId, taskIds), + createWorktreePR: (taskId: string, options?: WorktreeCreatePROptions): Promise> => + ipcRenderer.invoke(IPC_CHANNELS.TASK_WORKTREE_CREATE_PR, taskId, options), + // Task Event Listeners onTaskProgress: ( callback: (taskId: string, plan: ImplementationPlan, projectId?: string) => void diff --git a/apps/frontend/src/renderer/components/KanbanBoard.tsx b/apps/frontend/src/renderer/components/KanbanBoard.tsx index 30fc3c10..69a4612c 100644 --- a/apps/frontend/src/renderer/components/KanbanBoard.tsx +++ b/apps/frontend/src/renderer/components/KanbanBoard.tsx @@ -30,6 +30,12 @@ import { cn } from '../lib/utils'; import { persistTaskStatus, archiveTasks } from '../stores/task-store'; import type { Task, TaskStatus } from '../../shared/types'; +// Type guard for valid drop column targets - preserves literal type from TASK_STATUS_COLUMNS +const VALID_DROP_COLUMNS = new Set(TASK_STATUS_COLUMNS); +function isValidDropColumn(id: string): id is typeof TASK_STATUS_COLUMNS[number] { + return VALID_DROP_COLUMNS.has(id); +} + interface KanbanBoardProps { tasks: Task[]; onTaskClick: (task: Task) => void; @@ -356,7 +362,8 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR ); const tasksByStatus = useMemo(() => { - const grouped: Record = { + // Note: pr_created tasks are shown in the 'done' column since they're essentially complete + const grouped: Record = { backlog: [], in_progress: [], ai_review: [], @@ -365,14 +372,16 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR }; filteredTasks.forEach((task) => { - if (grouped[task.status]) { - grouped[task.status].push(task); + // Map pr_created tasks to the done column + const targetColumn = task.status === 'pr_created' ? 'done' : task.status; + if (grouped[targetColumn]) { + grouped[targetColumn].push(task); } }); // Sort tasks within each column by createdAt (newest first) Object.keys(grouped).forEach((status) => { - grouped[status as TaskStatus].sort((a, b) => { + grouped[status as typeof TASK_STATUS_COLUMNS[number]].sort((a, b) => { const dateA = new Date(a.createdAt).getTime(); const dateB = new Date(b.createdAt).getTime(); return dateB - dateA; // Descending order (newest first) @@ -418,7 +427,7 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR const overId = over.id as string; // Check if over a column - if (TASK_STATUS_COLUMNS.includes(overId as TaskStatus)) { + if (isValidDropColumn(overId)) { setOverColumnId(overId); return; } @@ -441,8 +450,8 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR const overId = over.id as string; // Check if dropped on a column - if (TASK_STATUS_COLUMNS.includes(overId as TaskStatus)) { - const newStatus = overId as TaskStatus; + if (isValidDropColumn(overId)) { + const newStatus = overId; const task = tasks.find((t) => t.id === activeTaskId); if (task && task.status !== newStatus) { diff --git a/apps/frontend/src/renderer/components/TaskCard.tsx b/apps/frontend/src/renderer/components/TaskCard.tsx index f07db15b..9e941a7d 100644 --- a/apps/frontend/src/renderer/components/TaskCard.tsx +++ b/apps/frontend/src/renderer/components/TaskCard.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useRef, useCallback, memo, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; -import { Play, Square, Clock, Zap, Target, Shield, Gauge, Palette, FileCode, Bug, Wrench, Loader2, AlertTriangle, RotateCcw, Archive, MoreVertical } from 'lucide-react'; +import { Play, Square, Clock, Zap, Target, Shield, Gauge, Palette, FileCode, Bug, Wrench, Loader2, AlertTriangle, RotateCcw, Archive, GitPullRequest, MoreVertical } from 'lucide-react'; import { Card, CardContent } from './ui/card'; import { Badge } from './ui/badge'; import { Button } from './ui/button'; @@ -74,6 +74,7 @@ function taskCardPropsAreEqual(prevProps: TaskCardProps, nextProps: TaskCardProp prevTask.metadata?.category === nextTask.metadata?.category && prevTask.metadata?.complexity === nextTask.metadata?.complexity && prevTask.metadata?.archivedAt === nextTask.metadata?.archivedAt && + prevTask.metadata?.prUrl === nextTask.metadata?.prUrl && // Check if any subtask statuses changed (compare all subtasks) prevTask.subtasks.every((s, i) => s.status === nextTask.subtasks[i]?.status) ); @@ -247,6 +248,13 @@ export const TaskCard = memo(function TaskCard({ task, onClick, onStatusChange } } }; + const handleViewPR = (e: React.MouseEvent) => { + e.stopPropagation(); + if (task.metadata?.prUrl && window.electronAPI?.openExternal) { + window.electronAPI.openExternal(task.metadata.prUrl); + } + }; + const getStatusBadgeVariant = (status: string) => { switch (status) { case 'in_progress': @@ -255,6 +263,8 @@ export const TaskCard = memo(function TaskCard({ task, onClick, onStatusChange } return 'warning'; case 'human_review': return 'purple'; + case 'pr_created': + return 'success'; case 'done': return 'success'; default: @@ -270,6 +280,8 @@ export const TaskCard = memo(function TaskCard({ task, onClick, onStatusChange } return t('labels.aiReview'); case 'human_review': return t('labels.needsReview'); + case 'pr_created': + return t('columns.pr_created'); case 'done': return t('status.complete'); default: @@ -369,15 +381,26 @@ export const TaskCard = memo(function TaskCard({ task, onClick, onStatusChange } {EXECUTION_PHASE_LABELS[executionPhase]} )} - {/* Status badge - hide when execution phase badge is showing */} - {!hasActiveExecution && ( - - {isStuck ? t('labels.needsRecovery') : isIncomplete ? t('labels.needsResume') : getStatusLabel(task.status)} - - )} + {/* Status badge - hide when execution phase badge is showing */} + {!hasActiveExecution && ( + <> + {task.status === 'pr_created' ? ( + + {getStatusLabel(task.status)} + + ) : ( + + {isStuck ? t('labels.needsRecovery') : isIncomplete ? t('labels.needsResume') : getStatusLabel(task.status)} + + )} + + )} {/* Review reason badge - explains why task needs human review */} {reviewReasonInfo && !isStuck && !isIncomplete && ( {t('actions.resume')} + ) : task.status === 'pr_created' ? ( +
+ {task.metadata?.prUrl && ( + + )} + {!task.metadata?.archivedAt && ( + + )} +
) : task.status === 'done' && !task.metadata?.archivedAt ? ( + {task && ( + + )} + {task?.status === 'pr_created' && task.metadata?.prUrl && ( + + )} + )} ); } @@ -295,12 +357,12 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, ) : ( <> - - {t(TASK_STATUS_LABELS[task.status])} - + + {t(TASK_STATUS_LABELS[task.status])} + {task.status === 'human_review' && task.reviewReason && ( )} diff --git a/apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx b/apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx index 34e5180d..cfd0c1d0 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskMetadata.tsx @@ -1,3 +1,4 @@ +import { useTranslation } from 'react-i18next'; import { Target, Bug, @@ -9,8 +10,10 @@ import { Lightbulb, Users, GitBranch, + GitPullRequest, ListChecks, - Clock + Clock, + ExternalLink } from 'lucide-react'; import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; @@ -48,6 +51,7 @@ interface TaskMetadataProps { } export function TaskMetadata({ task }: TaskMetadataProps) { + const { t } = useTranslation(['tasks']); const hasClassification = task.metadata && ( task.metadata.category || task.metadata.priority || @@ -139,7 +143,7 @@ export function TaskMetadata({ task }: TaskMetadataProps) { {/* Description - Primary Content */} {task.description && (
-
@@ -201,6 +205,24 @@ export function TaskMetadata({ task }: TaskMetadataProps) {
)} + {/* Pull Request */} + {task.metadata.prUrl && ( +
+

+ + {t('tasks:metadata.pullRequest')} +

+ +
+ )} + {/* Acceptance Criteria */} {task.metadata.acceptanceCriteria && task.metadata.acceptanceCriteria.length > 0 && (
diff --git a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx index 7dd2d1ca..de60d8d0 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx @@ -1,4 +1,4 @@ -import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo } from '../../../shared/types'; +import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, WorktreeCreatePRResult } from '../../../shared/types'; import { StagedSuccessMessage, WorkspaceStatus, @@ -8,7 +8,8 @@ import { ConflictDetailsDialog, LoadingMessage, NoWorkspaceMessage, - StagedInProjectMessage + StagedInProjectMessage, + CreatePRDialog } from './task-review'; interface TaskReviewProps { @@ -42,6 +43,11 @@ interface TaskReviewProps { onClose?: () => void; onSwitchToTerminals?: () => void; onOpenInbuiltTerminal?: (id: string, cwd: string) => void; + // PR creation + showPRDialog: boolean; + isCreatingPR: boolean; + onShowPRDialog: (show: boolean) => void; + onCreatePR: (options: { targetBranch?: string; title?: string; draft?: boolean }) => Promise; } /** @@ -83,7 +89,11 @@ export function TaskReview({ onLoadMergePreview, onClose, onSwitchToTerminals, - onOpenInbuiltTerminal + onOpenInbuiltTerminal, + showPRDialog, + isCreatingPR, + onShowPRDialog, + onCreatePR }: TaskReviewProps) { return (
@@ -110,12 +120,14 @@ export function TaskReview({ isLoadingPreview={isLoadingPreview} isMerging={isMerging} isDiscarding={isDiscarding} + isCreatingPR={isCreatingPR} onShowDiffDialog={onShowDiffDialog} onShowDiscardDialog={onShowDiscardDialog} onShowConflictDialog={onShowConflictDialog} onLoadMergePreview={onLoadMergePreview} onStageOnlyChange={onStageOnlyChange} onMerge={onMerge} + onShowPRDialog={onShowPRDialog} onClose={onClose} onSwitchToTerminals={onSwitchToTerminals} onOpenInbuiltTerminal={onOpenInbuiltTerminal} @@ -164,6 +176,15 @@ export function TaskReview({ onOpenChange={onShowConflictDialog} onMerge={onMerge} /> + + {/* Create PR Dialog */} +
); } 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 5ff22412..b4f3b5df 100644 --- a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts +++ b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts @@ -46,6 +46,8 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { } | null>(null); const [isLoadingPreview, setIsLoadingPreview] = useState(false); const [showConflictDialog, setShowConflictDialog] = useState(false); + const [showPRDialog, setShowPRDialog] = useState(false); + const [isCreatingPR, setIsCreatingPR] = useState(false); const selectedProject = useProjectStore((state) => state.getSelectedProject()); const isRunning = task.status === 'in_progress'; @@ -282,6 +284,8 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { mergePreview, isLoadingPreview, showConflictDialog, + showPRDialog, + isCreatingPR, // Setters setFeedback, @@ -313,6 +317,8 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { setMergePreview, setIsLoadingPreview, setShowConflictDialog, + setShowPRDialog, + setIsCreatingPR, // Handlers handleLogsScroll, diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.test.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.test.tsx new file mode 100644 index 00000000..17add18f --- /dev/null +++ b/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.test.tsx @@ -0,0 +1,409 @@ +/** + * @vitest-environment jsdom + */ +/** + * CreatePRDialog Tests + * + * Tests the Create PR dialog component functionality. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import '../../../../shared/i18n'; +import { CreatePRDialog } from './CreatePRDialog'; +import type { Task, WorktreeStatus } from '../../../../shared/types'; + +// Mock electronAPI +vi.mock('../../../../preload/api', () => ({})); + +// Mock window.electronAPI +const mockOpenExternal = vi.fn(); +Object.defineProperty(window, 'electronAPI', { + value: { + openExternal: mockOpenExternal + }, + writable: true +}); + +describe('CreatePRDialog', () => { + const mockOnOpenChange = vi.fn(); + const mockOnCreatePR = vi.fn(); + + const mockTask: Task = { + id: 'task-123', + specId: 'spec-123', + projectId: 'project-123', + title: 'Implement user authentication', + description: 'Add login and registration functionality', + status: 'human_review', + subtasks: [], + logs: [], + createdAt: new Date(), + updatedAt: new Date() + }; + + const mockWorktreeStatus: WorktreeStatus = { + exists: true, + worktreePath: '/path/to/worktree', + branch: 'auto-claude/implement-user-authentication', + baseBranch: 'develop', + commitCount: 5, + filesChanged: 10, + additions: 200, + deletions: 50 + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockOnCreatePR.mockResolvedValue({ success: true, prUrl: 'https://github.com/test/pr/1' }); + }); + + it('should render dialog when open', async () => { + render( + + ); + + await waitFor(() => { + // Check for the dialog title (h2 element) + expect(screen.getByRole('heading', { name: /create pull request/i })).toBeInTheDocument(); + }); + }); + + it('should default PR title to task title', async () => { + render( + + ); + + await waitFor(() => { + const titleInput = screen.getByLabelText(/pr title/i); + expect(titleInput).toHaveValue('Implement user authentication'); + }); + }); + + it('should default target branch to worktree base branch', async () => { + render( + + ); + + await waitFor(() => { + const branchInput = screen.getByLabelText(/target branch/i); + expect(branchInput).toHaveValue('develop'); + }); + }); + + it('should display source branch info', async () => { + render( + + ); + + await waitFor(() => { + expect(screen.getByText('auto-claude/implement-user-authentication')).toBeInTheDocument(); + }); + }); + + it('should display commit count and changes', async () => { + render( + + ); + + await waitFor(() => { + // Use data-testid for stable test targeting + const statsContainer = screen.getByTestId('pr-stats-container'); + expect(statsContainer).toBeInTheDocument(); + + // Scope assertions to the stats container to avoid accidental matches elsewhere + const stats = within(statsContainer); + expect(stats.getByText('5')).toBeInTheDocument(); // commit count + expect(stats.getByText('+200')).toBeInTheDocument(); // additions + expect(stats.getByText('-50')).toBeInTheDocument(); // deletions + }); + }); + + it('should call onCreatePR with form values when Create PR is clicked', async () => { + render( + + ); + + await waitFor(() => { + expect(screen.getByLabelText(/pr title/i)).toHaveValue('Implement user authentication'); + }); + + // Find the submit button (not the heading) - it's the one with "Create Pull Request" text inside a button + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(mockOnCreatePR).toHaveBeenCalledWith({ + targetBranch: 'develop', + title: 'Implement user authentication', + draft: false + }); + }); + }); + + it('should allow modifying PR title before creating', async () => { + render( + + ); + + await waitFor(() => { + expect(screen.getByLabelText(/pr title/i)).toHaveValue('Implement user authentication'); + }); + + const titleInput = screen.getByLabelText(/pr title/i); + fireEvent.change(titleInput, { target: { value: 'Custom PR Title' } }); + + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(mockOnCreatePR).toHaveBeenCalledWith({ + targetBranch: 'develop', + title: 'Custom PR Title', + draft: false + }); + }); + }); + + it('should close dialog when Cancel is clicked', async () => { + render( + + ); + + const cancelButton = screen.getByRole('button', { name: /cancel/i }); + fireEvent.click(cancelButton); + + await waitFor(() => { + expect(mockOnOpenChange).toHaveBeenCalledWith(false); + }); + }); + + it('should show success state after PR is created', async () => { + mockOnCreatePR.mockResolvedValue({ + success: true, + prUrl: 'https://github.com/test/repo/pull/123' + }); + + render( + + ); + + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(screen.getByText('https://github.com/test/repo/pull/123')).toBeInTheDocument(); + }); + }); + + it('should show error state when PR creation fails', async () => { + mockOnCreatePR.mockResolvedValue({ + success: false, + error: 'Failed to push branch to remote' + }); + + render( + + ); + + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(screen.getByText('Failed to push branch to remote')).toBeInTheDocument(); + }); + }); + + it('should reset form when dialog is reopened', async () => { + const { rerender } = render( + + ); + + // Modify the title + await waitFor(() => { + expect(screen.getByLabelText(/pr title/i)).toHaveValue('Implement user authentication'); + }); + + const titleInput = screen.getByLabelText(/pr title/i); + fireEvent.change(titleInput, { target: { value: 'Modified Title' } }); + expect(titleInput).toHaveValue('Modified Title'); + + // Close dialog + rerender( + + ); + + // Reopen dialog + rerender( + + ); + + // Should reset to task title + await waitFor(() => { + expect(screen.getByLabelText(/pr title/i)).toHaveValue('Implement user authentication'); + }); + }); + + it('should call onCreatePR with draft: true when draft checkbox is checked', async () => { + render( + + ); + + await waitFor(() => { + expect(screen.getByLabelText(/pr title/i)).toHaveValue('Implement user authentication'); + }); + + // Find and click the draft checkbox + const draftCheckbox = screen.getByRole('checkbox'); + fireEvent.click(draftCheckbox); + + // Create the PR + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(mockOnCreatePR).toHaveBeenCalledWith({ + targetBranch: 'develop', + title: 'Implement user authentication', + draft: true + }); + }); + }); + + it('should show already exists message when PR already exists', async () => { + mockOnCreatePR.mockResolvedValue({ + success: true, + prUrl: 'https://github.com/test/repo/pull/456', + alreadyExists: true + }); + + render( + + ); + + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + expect(screen.getByText(/already exists/i)).toBeInTheDocument(); + expect(screen.getByText('https://github.com/test/repo/pull/456')).toBeInTheDocument(); + }); + }); + + it('should show success state without link when prUrl is undefined', async () => { + mockOnCreatePR.mockResolvedValue({ + success: true, + prUrl: undefined + }); + + render( + + ); + + const createButton = screen.getByRole('button', { name: /create pull request/i }); + fireEvent.click(createButton); + + await waitFor(() => { + // Should show success message but no link + expect(screen.getByText(/created/i)).toBeInTheDocument(); + // Should not have any PR link button (no prUrl to display) + expect(screen.queryByTestId('pr-link-button')).not.toBeInTheDocument(); + }); + }); +}); diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.tsx new file mode 100644 index 00000000..da524957 --- /dev/null +++ b/apps/frontend/src/renderer/components/task-detail/task-review/CreatePRDialog.tsx @@ -0,0 +1,274 @@ +import { useState, useEffect } from 'react'; +import { GitPullRequest, Loader2, ExternalLink } from 'lucide-react'; +import { useTranslation } from 'react-i18next'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '../../ui/dialog'; +import { Button } from '../../ui/button'; +import { Input } from '../../ui/input'; +import { Label } from '../../ui/label'; +import { Checkbox } from '../../ui/checkbox'; +import type { Task, WorktreeStatus, WorktreeCreatePRResult } from '../../../../shared/types'; + +interface CreatePRDialogProps { + open: boolean; + task: Task; + worktreeStatus: WorktreeStatus | null; + onOpenChange: (open: boolean) => void; + onCreatePR: (options: { targetBranch?: string; title?: string; draft?: boolean }) => Promise; +} + +/** + * Dialog for creating a Pull Request from a worktree branch + * Allows user to specify target branch, PR title, and draft status + */ +export function CreatePRDialog({ + open, + task, + worktreeStatus, + onOpenChange, + onCreatePR +}: CreatePRDialogProps) { + const { t } = useTranslation(['taskReview', 'common']); + const [targetBranch, setTargetBranch] = useState(''); + const [prTitle, setPrTitle] = useState(''); + const [isDraft, setIsDraft] = useState(false); + const [isCreating, setIsCreating] = useState(false); + const [result, setResult] = useState(null); + const [error, setError] = useState(null); + + // Reset state when dialog opens + useEffect(() => { + if (open) { + setTargetBranch(worktreeStatus?.baseBranch || ''); + setPrTitle(task.title); + setIsDraft(false); + setIsCreating(false); + setResult(null); + setError(null); + } + }, [open, worktreeStatus?.baseBranch, task.title]); + + // Frontend validation functions + const validateBranchName = (branch: string): string | null => { + if (!branch.trim()) return null; // Empty is OK, will use default + // Basic git branch name rules: no spaces, .., @{, \, etc. + if (!/^[a-zA-Z0-9/_-]+$/.test(branch)) { + return t('taskReview:pr.errors.invalidBranchName'); + } + return null; + }; + + const validatePRTitle = (title: string): string | null => { + if (!title.trim()) { + return t('taskReview:pr.errors.emptyTitle'); + } + return null; + }; + + const handleCreatePR = async () => { + // Frontend validation before submitting + const branchError = validateBranchName(targetBranch); + if (branchError) { + setError(branchError); + return; + } + + const titleError = validatePRTitle(prTitle); + if (titleError) { + setError(titleError); + return; + } + + setIsCreating(true); + setError(null); + setResult(null); + + try { + const prResult = await onCreatePR({ + targetBranch: targetBranch || undefined, + title: prTitle || undefined, + draft: isDraft + }); + + if (prResult) { + if (prResult.success) { + setResult(prResult); + } else { + setError(prResult.error || t('taskReview:pr.errors.unknown')); + } + } else { + setError(t('taskReview:pr.errors.unknown')); + } + } catch (err) { + setError(err instanceof Error ? err.message : t('taskReview:pr.errors.unknown')); + } finally { + setIsCreating(false); + } + }; + + const handleClose = () => { + onOpenChange(false); + }; + + const handleOpenPR = () => { + if (result?.prUrl && window.electronAPI?.openExternal) { + window.electronAPI.openExternal(result.prUrl); + } + }; + + return ( + + + + + + {t('taskReview:pr.title')} + + + {t('taskReview:pr.description', { taskTitle: task.title })} + + + + {/* Success State */} + {result?.success && ( +
+
+

+ {result.alreadyExists + ? t('taskReview:pr.success.alreadyExists') + : t('taskReview:pr.success.created')} +

+ {result.prUrl && ( + + )} +
+ + + +
+ )} + + {/* Error State */} + {error && !result?.success && ( +
+
+

{error}

+
+ + + + +
+ )} + + {/* Form State */} + {!result?.success && !error && ( +
+ {/* Branch Info */} +
+
+ {t('taskReview:pr.labels.sourceBranch')}: + {worktreeStatus?.branch || t('taskReview:pr.labels.unknown')} +
+ {worktreeStatus?.exists && ( + <> +
+ {t('taskReview:pr.labels.commits')}: + {worktreeStatus.commitCount || 0} +
+
+ {t('taskReview:pr.labels.changes')}: + + +{worktreeStatus.additions || 0} + {' / '} + -{worktreeStatus.deletions || 0} + +
+ + )} +
+ + {/* Target Branch */} +
+ + setTargetBranch(e.target.value)} + placeholder={worktreeStatus?.baseBranch || 'main'} + /> +

+ {t('taskReview:pr.hints.targetBranch')} +

+
+ + {/* PR Title (optional) */} +
+ + setPrTitle(e.target.value)} + placeholder={task.title} + /> +

+ {t('taskReview:pr.hints.prTitle')} +

+
+ + {/* Draft PR Checkbox */} +
+ setIsDraft(checked === true)} + /> + +
+ + + + + +
+ )} + + + ); +} 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 b474c914..a68de379 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 @@ -5,6 +5,7 @@ import { Minus, Eye, GitMerge, + GitPullRequest, FolderX, Loader2, RotateCcw, @@ -14,6 +15,7 @@ import { Code, Terminal } from 'lucide-react'; +import { useTranslation } from 'react-i18next'; import { Button } from '../../ui/button'; import { Checkbox } from '../../ui/checkbox'; import { cn } from '../../../lib/utils'; @@ -28,12 +30,14 @@ interface WorkspaceStatusProps { isLoadingPreview: boolean; isMerging: boolean; isDiscarding: boolean; + isCreatingPR?: boolean; onShowDiffDialog: (show: boolean) => void; onShowDiscardDialog: (show: boolean) => void; onShowConflictDialog: (show: boolean) => void; onLoadMergePreview: () => void; onStageOnlyChange: (value: boolean) => void; onMerge: () => void; + onShowPRDialog?: (show: boolean) => void; onClose?: () => void; onSwitchToTerminals?: () => void; onOpenInbuiltTerminal?: (id: string, cwd: string) => void; @@ -84,16 +88,19 @@ export function WorkspaceStatus({ isLoadingPreview, isMerging, isDiscarding, + isCreatingPR, onShowDiffDialog, onShowDiscardDialog, onShowConflictDialog, onLoadMergePreview, onStageOnlyChange, onMerge, + onShowPRDialog, onClose, onSwitchToTerminals, onOpenInbuiltTerminal }: WorkspaceStatusProps) { + const { t } = useTranslation(['taskReview', 'common']); const { settings } = useSettingsStore(); const preferredIDE = settings.preferredIDE || 'vscode'; const preferredTerminal = settings.preferredTerminal || 'system'; @@ -433,11 +440,33 @@ export function WorkspaceStatus({ )} + {/* Create PR Button */} + {onShowPRDialog && ( + + )} +