feat: add PR creation workflow for task worktrees (#677)
* feat: add PR creation workflow for task worktrees Adds the ability to push a worktree branch and create a GitHub Pull Request directly from the Auto-Claude UI, instead of manually merging changes locally. ## User Flow 1. User completes a task build in an isolated worktree 2. Instead of clicking "Merge", user can click "Create PR" button 3. A dialog shows source branch → target branch (default: develop) 4. User confirms, system pushes branch and creates GitHub PR via `gh` CLI 5. PR URL is displayed and can be opened in browser ## Changes ### Backend (Python) - Added `push_branch()` with timeout (120s) for git push - Added `create_pull_request()` with timeout (60s) for gh CLI - Added `push_and_create_pr()` orchestrator - Added `--create-pr` CLI argument with handler - Added BRANCH and LINK icons with unique ASCII fallbacks ### Frontend (TypeScript) - Added `WorktreeCreatePRResult` type - Added `TASK_WORKTREE_CREATE_PR` IPC channel - Added IPC handler with 2-min timeout and EAFP pattern - Added `createWorktreePR` preload API method - Created reusable `CreatePRDialog` component - Integrated PR button in `WorkspaceStatus` - Added i18n translations (EN + FR) ## Code Review Fixes (from PR #606) - All subprocess calls have timeouts (TimeoutExpired handled) - EAFP pattern for file existence checks (no TOCTOU) - IPC handler has timeout with process cleanup - Icon ASCII fallbacks are unique (`[BR]` for BRANCH, `[L]` for LINK) - All user-facing strings use i18n translation keys - Translations added to BOTH en/*.json AND fr/*.json - CreatePRDialog component is reusable - Proper typed objects (no type assertions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review comments and add PR status persistence Review comment fixes: - Fix NameError: use args.base_branch instead of undefined base_branch (main.py) - Add JSON output for frontend IPC consumption (main.py) - Narrow exception handling in _extract_spec_summary to (OSError, UnicodeDecodeError) - Narrow exception handling in _get_existing_pr_url to subprocess-specific exceptions - Add debug logging for exception cases in worktree.py - Add 'exit' event handler to IPC handler for robustness (worktree-handlers.ts) Additional improvements: - Persist PR status to both main and worktree locations - Add CreatePR button to Worktrees page with i18n support - Add CreatePRDialog tests (11 test cases) - Fix i18n compliance for all new strings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(a11y): use button instead of anchor for PR link action Addresses review comment: anchor elements should only be used for navigation, not for triggering actions. Using a button improves accessibility for screen readers and keyboard users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: address nitpick review comments Backend (worktree.py): - Add TypedDict types (PushBranchResult, PullRequestResult) for better type safety - Add retry logic with exponential backoff (3 attempts) for transient network failures - Retries on: connection errors, network issues, timeouts, reset connections Frontend: - Fix checkbox accessibility: add explicit id/htmlFor for draft PR checkbox - Normalize return type in TaskDetailModal.handleCreatePR to include all fields - Add message field to WorktreeCreatePRResult for consistency with other result types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: remove duplicate JSON output in create-pr command The JSON was being printed twice: 1. In workspace_commands.py handle_create_pr_command() 2. In main.py after calling handle_create_pr_command() This caused JSON.parse to fail with "Unexpected non-whitespace character after JSON" when the frontend tried to parse the output. Removed the duplicate print from main.py since workspace_commands.py already handles JSON output for frontend parsing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: make IPC handler debug logging conditional Debug output for MERGE and CREATE_PR handlers now only appears when: - process.env.DEBUG === 'true', OR - process.env.NODE_ENV === 'development' This matches the pattern used elsewhere in the codebase (project-initializer.ts, terminal-name-generator.ts, etc.) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address code review feedback on JSON parsing and status persistence - Use non-greedy regex pattern to extract last complete JSON object from stdout, avoiding issues with multiple JSON objects or garbage - Add validation that parsed JSON has expected shape before using (typeof checks for success, pr_url, already_exists, error fields) - Await persistPlanStatus calls instead of fire-and-forget to ensure status is persisted before resolving the IPC handler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: ensure parent directory exists before writing metadata Add mkdirSync with recursive:true before writeFileSync in updateTaskMetadataPrUrl to prevent write failures when the parent directory doesn't exist. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: add TypedDict for push_and_create_pr return type Add PushAndCreatePRResult TypedDict with all fields (success, pushed, remote, branch, pr_url, already_exists, error) for static type safety. Update push_and_create_pr method signature and return statements to use the TypedDict constructor. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(i18n): use feminine form for PR in French translation Change "PR créé" to "PR créée" to match French grammatical gender (PR is feminine: "la PR"). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(i18n): use translation key for Open PR button Replace hardcoded "Open PR" label with i18n key common:buttons.openPR in Worktrees.tsx. Add translation keys to en/common.json and fr/common.json. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(a11y): use semantic button for PR link in TaskMetadata Replace anchor element with semantic button for better accessibility. Screen readers now properly announce this as an interactive control. The visible URL text provides an accessible label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(a11y,i18n): use semantic button and i18n for PR status in TaskDetailModal - Replace anchor element with semantic button for PR link - Replace hardcoded "PR Created" with t('tasks:status.prCreated') - Apply fix to both the completion state link and the badge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(i18n): use translation keys for PR button in WorkspaceStatus Add useTranslation hook and replace hardcoded strings: - "Creating PR..." → t('taskReview:pr.actions.creating') - "Create PR" → t('common:buttons.createPR') 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: scope numeric assertions to stats container in CreatePRDialog Use within() to scope commit count and changes assertions to the stats container, avoiding accidental matches elsewhere in the dialog. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: handle success results without prUrl in CreatePRDialog Allow success state to render even without a URL (e.g., from the "no JSON in output, assuming success" fallback). The PR link button is now conditionally rendered only when prUrl is present. This prevents the dialog from showing an empty body when the backend returns { success: true, prUrl: undefined }. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review findings for PR creation feature Backend (worktree.py): - Validate PR URL extraction - set pr_url to None if no valid URL found - Add message field to TypedDicts for informative feedback - Handle missing URL gracefully for existing PRs with message Frontend (worktree-handlers.ts): - Add GIT_BRANCH_REGEX and PR_CREATION_TIMEOUT_MS as module-level constants - Add input validation for targetBranch parameter - Add branch name validation in getTaskBaseBranch - Fix inconsistent JSON regex pattern between success/error paths Tests (CreatePRDialog.test.tsx): - Add test for draft PR checkbox functionality - Add test for 'already exists' PR state - Add test for success without prUrl Constants (task.ts): - Add pr_created to TASK_STATUS_LABELS and TASK_STATUS_COLORS 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: extract helper functions from TASK_WORKTREE_CREATE_PR handler - Extract parsePRJsonOutput() for JSON parsing with snake_case/camelCase - Extract updateTaskStatusAfterPRCreation() for metadata updates - Extract buildCreatePRArgs() for argument construction with validation - Extract initializePythonEnvForPR() for Python environment setup - Add generic withRetry() helper with exponential backoff - Refactor inline updatePlanWithRetry() to use withRetry() helper Addresses HIGH priority review finding about handler complexity and MEDIUM priority finding about duplicated retry logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address additional PR review findings Backend (worktree.py): - Update PullRequestResult.pr_url and PushAndCreatePRResult.pr_url to allow None (str | None) for cases where PR was created but URL couldn't be extracted Frontend (CreatePRDialog): - Add data-testid="pr-stats-container" for stable test targeting - Update test to use getByTestId instead of brittle CSS class selector Frontend (TaskDetailModal): - Remove hardcoded English error strings from handleCreatePR - Propagate IPC errors directly, let CreatePRDialog use i18n fallbacks Frontend (TaskMetadata): - Add i18n support for "Pull Request" header label - Add translation keys to en/tasks.json and fr/tasks.json 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: handle success default and retry validation in PR handlers - Default success to false in parsePRJsonOutput to avoid masking failures when the field is missing from the JSON response - Add validation to withRetry to ensure at least one attempt is made by clamping maxRetries to a minimum of 1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(i18n): remove hardcoded error strings from Worktrees handleCreatePR Let CreatePRDialog handle i18n fallback for undefined error values instead of hardcoding 'Failed to create PR' and 'Unknown error'. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: reset isCreating flag when CreatePRDialog opens Prevents stale loading state when reopening the dialog after a previous PR creation attempt. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(test): use os.tmpdir() for cross-platform temp path matching Tests were hardcoded to expect /tmp/ but macOS uses /var/folders/.../T/ for temp files. Now dynamically uses os.tmpdir() for platform-independent path matching. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * style: apply pre-commit auto-fixes - Remove trailing whitespace from 20 files - Fix ruff lint errors in Python files - Apply ruff formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address CodeQL and code review findings - Extract escapeForRegex helper in claude-integration-handler.test.ts to deduplicate regex-escaping logic and avoid ReDoS false-positive - Anchor regex pattern in CreatePRDialog.test.tsx to prevent arbitrary host matching (CodeQL security alert) - Remove unused ExternalLink import from TaskCard.tsx - Add defensive window.electronAPI check in CreatePRDialog handleOpenPR to avoid runtime errors in test/misconfigured environments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address CodeQL and code review findings Frontend: - Fix CodeQL regex anchor issue in CreatePRDialog.test.tsx by using data-testid="pr-link-button" instead of URL regex pattern - Add data-testid to PR link button in CreatePRDialog.tsx - Add defensive window.electronAPI?.openExternal check in TaskCard.tsx Backend: - Add CreatePRResult TypedDict for type-safe return values - Wrap push_and_create_pr call in try/except for clean JSON output on exceptions instead of unhandled tracebacks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: add frontend validation for PR creation form - Add client-side validation for branch names and PR titles - Validate git branch name format (alphanumeric, hyphens, underscores, slashes) - Ensure PR title is not empty - Provide immediate user feedback before backend submission - Add localized error messages in English and French * refactor: improve error handling and import organization in PR creation - Clean up CreatePRResult error structure: separate user-friendly 'message' from technical 'error' field - Move get_existing_build_worktree import to module-level imports for consistency - Remove redundant local import inside handle_create_pr_command function - Improve API clarity by providing both user messages and technical error details * refactor: properly convert PushAndCreatePRResult to CreatePRResult in CLI handler - Convert raw PushAndCreatePRResult to expected CreatePRResult shape - Map fields appropriately: success, pr_url, already_exists, error, message - Maintain type safety by returning declared CreatePRResult instead of raw result - Preserve all essential information while conforming to API contract - Improve code maintainability and type correctness * feat: include push and branch details in CreatePRResult - Add pushed, remote, and branch fields to CreatePRResult type - Include push status, remote name, and branch name in CLI result - Provide complete operation details for frontend consumption - Enhance API with comprehensive PR creation status information - Maintain backward compatibility while adding useful metadata * fix: improve type safety and i18n consistency for task status - Add isValidDropColumn type guard in KanbanBoard.tsx to preserve literal types from TASK_STATUS_COLUMNS instead of using unsafe cast - Replace duplicate CheckCircle2 with GitPullRequest icon in TaskDetailModal PR button for visual consistency with TaskCard - Normalize pr_created i18n key to columns.pr_created namespace - Add pr_created translation keys to en/fr tasks.json columns section - Update all hardcoded status.prCreated references to use mapping 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: remove duplicate PR Created badges and unused import - Remove unused ExternalLink import from TaskDetailModal.tsx - Fix duplicate badge rendering for pr_created status in both TaskCard and TaskDetailModal - Consolidate to single badge showing 'PR Created' for completed PR tasks * refactor: extract status badge variant logic and use i18n for completion text - Extract complex badge variant ternary into getStatusBadgeVariant helper function in TaskDetailModal - Replace hardcoded 'Task completed' with i18n translation t('tasks:status.complete') - Update getStatusBadgeVariant in TaskCard to return 'success' for pr_created status - Use getStatusBadgeVariant consistently instead of hardcoded variant in pr_created conditional * fix: use optional chaining for electronAPI in PR URL button - Update TaskDetailModal PR URL button onClick to use window.electronAPI?.openExternal - Matches the pattern used in TaskCard.tsx handleViewPR function - Prevents runtime errors when electronAPI is undefined * fix: add URL validation for parsed PR URLs Add isValidGitHubUrl() helper to validate PR URLs are valid https://github.com or *.github.com URLs before using them. This improves robustness by filtering out invalid URLs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: extract WorktreeCreatePROptions into named exported type Extract the inline options object from createWorktreePR signature into a reusable named type. Updated all callers and related declarations to use the new type for consistency across components. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: use WorktreeCreatePROptions type and add defensive optional chaining - Update createWorktreePR implementation to use WorktreeCreatePROptions instead of inline type (matches interface declaration) - Add optional chaining for window.electronAPI?.openExternal in Worktrees - Remove unused ExternalLink import from Worktrees component 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review findings for code quality - Extract retry helper functions in worktree.py for DRY network error handling - Fix broad 'http' retry condition to exclude auth errors (401, 403) - Add Windows taskkill fallback for forceful process termination - Import CreatePRResult from worktree.py instead of duplicating TypedDict - Move import to top of worktree.py following Python conventions - Return result object from updateTaskStatusAfterPRCreation for better state tracking - Add PR title validation (printable chars, 256 char max) - Use WorktreeCreatePROptions type consistently in handler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review findings - dedupe retry logic and support GH Enterprise URLs - Refactor push_branch and create_pull_request to use _with_retry helper instead of duplicated retry loops (addresses code duplication issue) - Update isValidGitHubUrl to accept any HTTPS URL with /pull/\d+ path to support GitHub Enterprise instances with custom domains Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(ui): relax isValidGitHubUrl validation for GH Enterprise support - Remove /pull/\d+ path requirement that was too strict - Only require HTTPS protocol and non-empty hostname - Allows GitHub Enterprise URLs with custom domains to be parsed correctly Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address CodeRabbit feedback for PR creation - Fix undefined base_branch variable in CLI main.py with proper auto-detection - Improve event handling in worktree-handlers.ts with comprehensive exit event support - Fix dynamic retry count in error messages instead of hardcoded '3 attempts' - Use get_git_executable() and handle FileNotFoundError in push_branch method - Move debug_warning import to module level for better performance - Ensure all error messages reflect actual retry counts used * fix: address additional PR review feedback - main.py: Simplify PR creation by passing pr_target directly to handler, letting WorktreeManager._detect_base_branch handle detection internally - worktree.py: Fix _with_retry type signature to match actual tuple return, use get_git_executable() for proper git path resolution, move debug_warning import to top of file - worktree-handlers.ts: Extract duplicated close/exit callback logic into handleCreatePRProcessExit helper function - workspace_commands.py: Remove redundant json import (CodeQL fix) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(test): clear GIT_INDEX_FILE in temp_git_repo fixture Pre-commit sets GIT_INDEX_FILE to a relative path (.git/index.pre-commit) which causes git commands in temp repos to fail with "index file open failed: Not a directory" because the relative path resolves against the main repo instead of the temp repo. The fix saves and clears GIT_INDEX_FILE before creating the temp repo, then restores it in a finally block to ensure cleanup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: use proper base branch fallback for PR creation target The worktree status handlers were incorrectly determining baseBranch by checking the current HEAD branch in the main project directory. This caused the PR creation dialog to pre-populate the target branch with the user's current feature branch instead of main/develop. Added getEffectiveBaseBranch() helper that properly determines the base branch using this priority: 1. Task metadata baseBranch (from task_metadata.json) 2. Project settings mainBranch 3. Git detection (main/master branch existence) 4. Fallback to 'main' Fixed three handlers: - TASK_WORKTREE_STATUS - TASK_WORKTREE_DIFF - List worktrees helper Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
This commit is contained in:
ThrownLemon
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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/<number> 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(
|
||||
|
||||
@@ -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 = ("▣", "#")
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
|
||||
|
||||
// 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<string, unknown>;
|
||||
|
||||
|
||||
// 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<string, unknown>;
|
||||
|
||||
|
||||
// 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');
|
||||
|
||||
@@ -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
|
||||
*/
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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<string, number>);
|
||||
|
||||
|
||||
// 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));
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, unknown> = {};
|
||||
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<TaskStatusUpdateResult> {
|
||||
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<string | undefined> {
|
||||
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<T>(
|
||||
operation: () => Promise<T>,
|
||||
options: {
|
||||
maxRetries?: number;
|
||||
baseDelayMs?: number;
|
||||
onRetry?: (attempt: number, error: unknown) => void;
|
||||
shouldRetry?: (error: unknown) => boolean;
|
||||
} = {}
|
||||
): Promise<T> {
|
||||
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<IPCResult<WorktreeMergeResult>> => {
|
||||
// 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<boolean> => {
|
||||
// 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<IPCResult<WorktreeCreatePRResult>> => {
|
||||
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<void> => {
|
||||
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'
|
||||
};
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<string, string> {
|
||||
// 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<string, string> {
|
||||
: 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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -124,7 +124,7 @@ export async function saveProfilesFile(data: ProfilesFile): Promise<void> {
|
||||
// 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<boolean
|
||||
/**
|
||||
* Execute a function with exclusive file lock to prevent race conditions
|
||||
* This ensures atomic read-modify-write operations on the profiles file
|
||||
*
|
||||
*
|
||||
* @param fn Function to execute while holding the lock
|
||||
* @returns Result of the function execution
|
||||
*/
|
||||
export async function withProfilesLock<T>(fn: () => Promise<T>): Promise<T> {
|
||||
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<T>(fn: () => Promise<T>): Promise<T> {
|
||||
// EEXIST means another process won the race, proceed normally
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// Acquire lock with reasonable timeout
|
||||
let release: (() => Promise<void>) | undefined;
|
||||
try {
|
||||
@@ -205,7 +205,7 @@ export async function withProfilesLock<T>(fn: () => Promise<T>): Promise<T> {
|
||||
maxTimeout: 500
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
// Execute the function while holding the lock
|
||||
return await fn();
|
||||
} finally {
|
||||
@@ -219,7 +219,7 @@ export async function withProfilesLock<T>(fn: () => Promise<T>): Promise<T> {
|
||||
/**
|
||||
* 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
|
||||
|
||||
@@ -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<APIProfi
|
||||
const exists = file.profiles.some(
|
||||
(p) => 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';
|
||||
|
||||
@@ -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}'`);
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
|
||||
|
||||
@@ -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<IPCResult<{ ides: Array<{ id: string; name: string; path: string; installed: boolean }>; terminals: Array<{ id: string; name: string; path: string; installed: boolean }> }>>;
|
||||
archiveTasks: (projectId: string, taskIds: string[], version?: string) => Promise<IPCResult<boolean>>;
|
||||
unarchiveTasks: (projectId: string, taskIds: string[]) => Promise<IPCResult<boolean>>;
|
||||
createWorktreePR: (taskId: string, options?: WorktreeCreatePROptions) => Promise<IPCResult<WorktreeCreatePRResult>>;
|
||||
|
||||
// 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<IPCResult<boolean>> =>
|
||||
ipcRenderer.invoke(IPC_CHANNELS.TASK_UNARCHIVE, projectId, taskIds),
|
||||
|
||||
createWorktreePR: (taskId: string, options?: WorktreeCreatePROptions): Promise<IPCResult<WorktreeCreatePRResult>> =>
|
||||
ipcRenderer.invoke(IPC_CHANNELS.TASK_WORKTREE_CREATE_PR, taskId, options),
|
||||
|
||||
// Task Event Listeners
|
||||
onTaskProgress: (
|
||||
callback: (taskId: string, plan: ImplementationPlan, projectId?: string) => void
|
||||
|
||||
@@ -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<string>(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<TaskStatus, Task[]> = {
|
||||
// Note: pr_created tasks are shown in the 'done' column since they're essentially complete
|
||||
const grouped: Record<typeof TASK_STATUS_COLUMNS[number], Task[]> = {
|
||||
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) {
|
||||
|
||||
@@ -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]}
|
||||
</Badge>
|
||||
)}
|
||||
{/* Status badge - hide when execution phase badge is showing */}
|
||||
{!hasActiveExecution && (
|
||||
<Badge
|
||||
variant={isStuck ? 'warning' : isIncomplete ? 'warning' : getStatusBadgeVariant(task.status)}
|
||||
className="text-[10px] px-1.5 py-0.5"
|
||||
>
|
||||
{isStuck ? t('labels.needsRecovery') : isIncomplete ? t('labels.needsResume') : getStatusLabel(task.status)}
|
||||
</Badge>
|
||||
)}
|
||||
{/* Status badge - hide when execution phase badge is showing */}
|
||||
{!hasActiveExecution && (
|
||||
<>
|
||||
{task.status === 'pr_created' ? (
|
||||
<Badge
|
||||
variant={getStatusBadgeVariant(task.status)}
|
||||
className="text-[10px] px-1.5 py-0.5"
|
||||
>
|
||||
{getStatusLabel(task.status)}
|
||||
</Badge>
|
||||
) : (
|
||||
<Badge
|
||||
variant={isStuck ? 'warning' : isIncomplete ? 'warning' : getStatusBadgeVariant(task.status)}
|
||||
className="text-[10px] px-1.5 py-0.5"
|
||||
>
|
||||
{isStuck ? t('labels.needsRecovery') : isIncomplete ? t('labels.needsResume') : getStatusLabel(task.status)}
|
||||
</Badge>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
{/* Review reason badge - explains why task needs human review */}
|
||||
{reviewReasonInfo && !isStuck && !isIncomplete && (
|
||||
<Badge
|
||||
@@ -492,6 +515,31 @@ export const TaskCard = memo(function TaskCard({ task, onClick, onStatusChange }
|
||||
<Play className="mr-1.5 h-3 w-3" />
|
||||
{t('actions.resume')}
|
||||
</Button>
|
||||
) : task.status === 'pr_created' ? (
|
||||
<div className="flex gap-1">
|
||||
{task.metadata?.prUrl && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-7 px-2 cursor-pointer"
|
||||
onClick={handleViewPR}
|
||||
title={t('tooltips.viewPR')}
|
||||
>
|
||||
<GitPullRequest className="h-3 w-3" />
|
||||
</Button>
|
||||
)}
|
||||
{!task.metadata?.archivedAt && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-7 px-2 cursor-pointer"
|
||||
onClick={handleArchive}
|
||||
title={t('tooltips.archiveTask')}
|
||||
>
|
||||
<Archive className="h-3 w-3" />
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
) : task.status === 'done' && !task.metadata?.archivedAt ? (
|
||||
<Button
|
||||
variant="ghost"
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useEffect, useState, useCallback } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import {
|
||||
GitBranch,
|
||||
RefreshCw,
|
||||
@@ -8,6 +9,7 @@ import {
|
||||
FolderOpen,
|
||||
FolderGit,
|
||||
GitMerge,
|
||||
GitPullRequest,
|
||||
FileCode,
|
||||
Plus,
|
||||
Minus,
|
||||
@@ -40,13 +42,15 @@ import {
|
||||
} from './ui/alert-dialog';
|
||||
import { useProjectStore } from '../stores/project-store';
|
||||
import { useTaskStore } from '../stores/task-store';
|
||||
import type { WorktreeListItem, WorktreeMergeResult, TerminalWorktreeConfig } from '../../shared/types';
|
||||
import type { WorktreeListItem, WorktreeMergeResult, TerminalWorktreeConfig, WorktreeStatus, Task, WorktreeCreatePROptions, WorktreeCreatePRResult } from '../../shared/types';
|
||||
import { CreatePRDialog } from './task-detail/task-review/CreatePRDialog';
|
||||
|
||||
interface WorktreesProps {
|
||||
projectId: string;
|
||||
}
|
||||
|
||||
export function Worktrees({ projectId }: WorktreesProps) {
|
||||
const { t } = useTranslation(['common']);
|
||||
const projects = useProjectStore((state) => state.projects);
|
||||
const selectedProject = projects.find((p) => p.id === projectId);
|
||||
const tasks = useTaskStore((state) => state.tasks);
|
||||
@@ -71,6 +75,11 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
const [worktreeToDelete, setWorktreeToDelete] = useState<WorktreeListItem | null>(null);
|
||||
const [isDeleting, setIsDeleting] = useState(false);
|
||||
|
||||
// Create PR dialog state
|
||||
const [showCreatePRDialog, setShowCreatePRDialog] = useState(false);
|
||||
const [prWorktree, setPrWorktree] = useState<WorktreeListItem | null>(null);
|
||||
const [prTask, setPrTask] = useState<Task | null>(null);
|
||||
|
||||
// Load worktrees (both task and terminal worktrees)
|
||||
const loadWorktrees = useCallback(async () => {
|
||||
if (!projectId || !selectedProject) return;
|
||||
@@ -194,6 +203,49 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
setShowDeleteConfirm(true);
|
||||
};
|
||||
|
||||
// Convert WorktreeListItem to WorktreeStatus for the dialog
|
||||
const worktreeToStatus = (worktree: WorktreeListItem): WorktreeStatus => ({
|
||||
exists: true,
|
||||
worktreePath: worktree.path,
|
||||
branch: worktree.branch,
|
||||
baseBranch: worktree.baseBranch,
|
||||
commitCount: worktree.commitCount,
|
||||
filesChanged: worktree.filesChanged,
|
||||
additions: worktree.additions,
|
||||
deletions: worktree.deletions
|
||||
});
|
||||
|
||||
// Open Create PR dialog
|
||||
const openCreatePRDialog = (worktree: WorktreeListItem, task: Task) => {
|
||||
setPrWorktree(worktree);
|
||||
setPrTask(task);
|
||||
setShowCreatePRDialog(true);
|
||||
};
|
||||
|
||||
// Handle Create PR
|
||||
const handleCreatePR = async (options: WorktreeCreatePROptions): Promise<WorktreeCreatePRResult | null> => {
|
||||
if (!prTask) return null;
|
||||
|
||||
try {
|
||||
const result = await window.electronAPI.createWorktreePR(prTask.id, options);
|
||||
if (result.success && result.data) {
|
||||
if (result.data.success && result.data.prUrl && !result.data.alreadyExists) {
|
||||
// Update task in store
|
||||
useTaskStore.getState().updateTask(prTask.id, {
|
||||
status: 'pr_created',
|
||||
metadata: { ...prTask.metadata, prUrl: result.data.prUrl }
|
||||
});
|
||||
}
|
||||
return result.data;
|
||||
}
|
||||
// Propagate IPC error; let CreatePRDialog use its i18n fallback
|
||||
return { success: false, error: result.error, prUrl: undefined, alreadyExists: false };
|
||||
} catch (err) {
|
||||
// Propagate actual error message; let CreatePRDialog handle i18n fallback for undefined
|
||||
return { success: false, error: err instanceof Error ? err.message : undefined, prUrl: undefined, alreadyExists: false };
|
||||
}
|
||||
};
|
||||
|
||||
// Handle terminal worktree delete
|
||||
const handleDeleteTerminalWorktree = async () => {
|
||||
if (!terminalWorktreeToDelete || !selectedProject) return;
|
||||
@@ -357,6 +409,26 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
<GitMerge className="h-3.5 w-3.5 mr-1.5" />
|
||||
Merge to {worktree.baseBranch}
|
||||
</Button>
|
||||
{task && (
|
||||
<Button
|
||||
variant="info"
|
||||
size="sm"
|
||||
onClick={() => openCreatePRDialog(worktree, task)}
|
||||
>
|
||||
<GitPullRequest className="h-3.5 w-3.5 mr-1.5" />
|
||||
{t('common:buttons.createPR')}
|
||||
</Button>
|
||||
)}
|
||||
{task?.status === 'pr_created' && task.metadata?.prUrl && (
|
||||
<Button
|
||||
variant="info"
|
||||
size="sm"
|
||||
onClick={() => window.electronAPI?.openExternal(task.metadata?.prUrl ?? '')}
|
||||
>
|
||||
<GitPullRequest className="h-3.5 w-3.5 mr-1.5" />
|
||||
{t('common:buttons.openPR')}
|
||||
</Button>
|
||||
)}
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
@@ -646,6 +718,17 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
</AlertDialogFooter>
|
||||
</AlertDialogContent>
|
||||
</AlertDialog>
|
||||
|
||||
{/* Create PR Dialog */}
|
||||
{prTask && prWorktree && (
|
||||
<CreatePRDialog
|
||||
open={showCreatePRDialog}
|
||||
task={prTask}
|
||||
worktreeStatus={worktreeToStatus(prWorktree)}
|
||||
onOpenChange={setShowCreatePRDialog}
|
||||
onCreatePR={handleCreatePR}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -271,8 +271,8 @@ export function MemoriesTab({
|
||||
<Icon className="h-3.5 w-3.5" />
|
||||
<span>{config.label}</span>
|
||||
{count > 0 && (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className={cn(
|
||||
'ml-1 px-1.5 py-0 text-xs',
|
||||
isActive && 'bg-background/20'
|
||||
|
||||
@@ -92,11 +92,11 @@ function ListItem({ children, variant = 'default' }: { children: React.ReactNode
|
||||
// Check if memory content looks like a PR review
|
||||
function isPRReviewMemory(memory: MemoryEpisode): boolean {
|
||||
// Check by type first
|
||||
if (memory.type === 'pr_review' || memory.type === 'pr_finding' ||
|
||||
if (memory.type === 'pr_review' || memory.type === 'pr_finding' ||
|
||||
memory.type === 'pr_pattern' || memory.type === 'pr_gotcha') {
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
// Check by content structure (for session_insight type that's actually a PR review)
|
||||
try {
|
||||
const parsed = JSON.parse(memory.content);
|
||||
@@ -109,7 +109,7 @@ function isPRReviewMemory(memory: MemoryEpisode): boolean {
|
||||
export function MemoryCard({ memory }: MemoryCardProps) {
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const parsed = useMemo(() => parseMemoryContent(memory.content), [memory.content]);
|
||||
|
||||
|
||||
// Determine if there's meaningful content to show (must be called before early return)
|
||||
const hasContent = useMemo(() => {
|
||||
if (!parsed) return false;
|
||||
|
||||
@@ -91,7 +91,7 @@ function VerdictBadge({ verdict }: { verdict: string }) {
|
||||
|
||||
function SeverityBadge({ severity, count }: { severity: string; count: number }) {
|
||||
if (count === 0) return null;
|
||||
|
||||
|
||||
const colorMap: Record<string, string> = {
|
||||
critical: 'bg-red-600/20 text-red-400 border-red-600/30',
|
||||
high: 'bg-orange-500/20 text-orange-400 border-orange-500/30',
|
||||
@@ -129,7 +129,7 @@ export function PRReviewCard({ memory }: PRReviewCardProps) {
|
||||
}
|
||||
|
||||
const { finding_counts } = parsed.summary || { finding_counts: { critical: 0, high: 0, medium: 0, low: 0 } };
|
||||
const totalFindings = (finding_counts?.critical || 0) + (finding_counts?.high || 0) +
|
||||
const totalFindings = (finding_counts?.critical || 0) + (finding_counts?.high || 0) +
|
||||
(finding_counts?.medium || 0) + (finding_counts?.low || 0);
|
||||
const hasGotchas = parsed.gotchas && parsed.gotchas.length > 0;
|
||||
const hasPatterns = parsed.patterns && parsed.patterns.length > 0;
|
||||
@@ -229,7 +229,7 @@ export function PRReviewCard({ memory }: PRReviewCardProps) {
|
||||
{parsed.keyFindings.slice(0, 5).map((finding, idx) => (
|
||||
<div key={idx} className="text-sm">
|
||||
<div className="flex items-center gap-2">
|
||||
<Badge
|
||||
<Badge
|
||||
className={`text-xs ${
|
||||
finding.severity === 'critical' ? 'bg-red-600/20 text-red-400' :
|
||||
finding.severity === 'high' ? 'bg-orange-500/20 text-orange-400' :
|
||||
|
||||
@@ -151,7 +151,7 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions =
|
||||
if (prsNeedingPreload.length > 0) {
|
||||
const prNumbers = prsNeedingPreload.map(pr => pr.number);
|
||||
const batchReviews = await window.electronAPI.github.getPRReviewsBatch(projectId, prNumbers);
|
||||
|
||||
|
||||
// Update store with loaded results
|
||||
for (const reviewResult of Object.values(batchReviews)) {
|
||||
if (reviewResult) {
|
||||
@@ -246,7 +246,7 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions =
|
||||
// Update store with the loaded result
|
||||
// Preserve newCommitsCheck when loading existing review from disk
|
||||
usePRReviewStore.getState().setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true });
|
||||
|
||||
|
||||
// Always check for new commits when selecting a reviewed PR
|
||||
// This ensures fresh data even if we have a cached check from earlier in the session
|
||||
const reviewedCommitSha = result.reviewedCommitSha || (result as any).reviewed_commit_sha;
|
||||
|
||||
@@ -173,14 +173,14 @@ export function OllamaModelSelector({
|
||||
result.data.embedding_models.forEach((m: { name: string }) => {
|
||||
const name = m.name;
|
||||
installedFullNames.add(name);
|
||||
|
||||
|
||||
// Normalize :latest suffix
|
||||
if (name.endsWith(':latest')) {
|
||||
installedBaseNames.add(name.replace(':latest', ''));
|
||||
} else if (!name.includes(':')) {
|
||||
installedBaseNames.add(name);
|
||||
}
|
||||
|
||||
|
||||
// Handle quantization variants (e.g., qwen3-embedding:8b-q4_K_M)
|
||||
// Extract base:version without quantization suffix
|
||||
const quantMatch = name.match(/^([^:]+:[^-]+)/);
|
||||
@@ -301,7 +301,7 @@ export function OllamaModelSelector({
|
||||
*/
|
||||
const handleSelect = (model: OllamaModel) => {
|
||||
if (!model.installed || disabled) return;
|
||||
|
||||
|
||||
// Toggle behavior: if already selected, deselect by passing empty values
|
||||
if (selectedModel === model.name) {
|
||||
onModelSelect('', 0);
|
||||
|
||||
@@ -118,7 +118,7 @@ export function ModelSearchableSelect({
|
||||
} catch (err) {
|
||||
if (err instanceof Error && err.name !== 'AbortError') {
|
||||
// Check if it's specifically "not supported" or a general error
|
||||
if (err.message.includes('does not support model listing') ||
|
||||
if (err.message.includes('does not support model listing') ||
|
||||
err.message.includes('not_supported')) {
|
||||
setModelDiscoveryNotSupported(true);
|
||||
} else {
|
||||
@@ -142,13 +142,13 @@ export function ModelSearchableSelect({
|
||||
*/
|
||||
const handleOpen = () => {
|
||||
if (disabled) return;
|
||||
|
||||
|
||||
// If we already know model discovery isn't supported, don't open dropdown
|
||||
if (modelDiscoveryNotSupported) {
|
||||
setIsManualInput(true);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
setIsOpen(true);
|
||||
setSearchQuery('');
|
||||
|
||||
|
||||
@@ -27,11 +27,12 @@ import {
|
||||
Loader2,
|
||||
AlertTriangle,
|
||||
Pencil,
|
||||
X
|
||||
X,
|
||||
GitPullRequest
|
||||
} from 'lucide-react';
|
||||
import { cn } from '../../lib/utils';
|
||||
import { calculateProgress } from '../../lib/utils';
|
||||
import { startTask, stopTask, submitReview, recoverStuckTask, deleteTask } from '../../stores/task-store';
|
||||
import { startTask, stopTask, submitReview, recoverStuckTask, deleteTask, useTaskStore } from '../../stores/task-store';
|
||||
import { TASK_STATUS_LABELS } from '../../../shared/constants';
|
||||
import { TaskEditDialog } from '../TaskEditDialog';
|
||||
import { useTaskDetail } from './hooks/useTaskDetail';
|
||||
@@ -41,7 +42,7 @@ import { TaskSubtasks } from './TaskSubtasks';
|
||||
import { TaskLogs } from './TaskLogs';
|
||||
import { TaskFiles } from './TaskFiles';
|
||||
import { TaskReview } from './TaskReview';
|
||||
import type { Task } from '../../../shared/types';
|
||||
import type { Task, WorktreeCreatePROptions } from '../../../shared/types';
|
||||
|
||||
interface TaskDetailModalProps {
|
||||
open: boolean;
|
||||
@@ -163,6 +164,30 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals,
|
||||
state.setIsDiscarding(false);
|
||||
};
|
||||
|
||||
const handleCreatePR = async (options: WorktreeCreatePROptions) => {
|
||||
state.setIsCreatingPR(true);
|
||||
try {
|
||||
const result = await window.electronAPI.createWorktreePR(task.id, options);
|
||||
if (result.success && result.data) {
|
||||
// Update single task in store with new status and prUrl (more efficient than reloading all tasks)
|
||||
if (result.data.success && result.data.prUrl && !result.data.alreadyExists) {
|
||||
useTaskStore.getState().updateTask(task.id, {
|
||||
status: 'pr_created',
|
||||
metadata: { ...task.metadata, prUrl: result.data.prUrl }
|
||||
});
|
||||
}
|
||||
return result.data;
|
||||
}
|
||||
// Propagate IPC error; let CreatePRDialog use its i18n fallback
|
||||
return { success: false, error: result.error, prUrl: undefined, alreadyExists: false };
|
||||
} catch (error) {
|
||||
// Propagate actual error message; let CreatePRDialog handle i18n fallback for undefined
|
||||
return { success: false, error: error instanceof Error ? error.message : undefined, prUrl: undefined, alreadyExists: false };
|
||||
} finally {
|
||||
state.setIsCreatingPR(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleClose = () => {
|
||||
// Show toast notification if task is running
|
||||
if (state.isRunning && !state.isStuck) {
|
||||
@@ -175,6 +200,22 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals,
|
||||
onOpenChange(false);
|
||||
};
|
||||
|
||||
// Helper function to get status badge variant
|
||||
const getStatusBadgeVariant = (status: string, isStuck: boolean) => {
|
||||
if (isStuck) return 'warning';
|
||||
switch (status) {
|
||||
case 'done':
|
||||
case 'pr_created':
|
||||
return 'success';
|
||||
case 'human_review':
|
||||
return 'purple';
|
||||
case 'in_progress':
|
||||
return 'info';
|
||||
default:
|
||||
return 'secondary';
|
||||
}
|
||||
};
|
||||
|
||||
// Render primary action button based on state
|
||||
const renderPrimaryAction = () => {
|
||||
if (state.isStuck) {
|
||||
@@ -233,7 +274,28 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals,
|
||||
return (
|
||||
<div className="completion-state text-sm flex items-center gap-2 text-success">
|
||||
<CheckCircle2 className="h-5 w-5" />
|
||||
<span className="font-medium">Task completed</span>
|
||||
<span className="font-medium">{t('tasks:status.complete')}</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (task.status === 'pr_created') {
|
||||
return (
|
||||
<div className="flex items-center gap-4">
|
||||
<div className="completion-state text-sm flex items-center gap-2 text-success">
|
||||
<CheckCircle2 className="h-5 w-5" />
|
||||
<span className="font-medium">{t('tasks:status.complete')}</span>
|
||||
</div>
|
||||
{task.metadata?.prUrl && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => window.electronAPI?.openExternal(task.metadata!.prUrl!)}
|
||||
className="completion-state text-sm flex items-center gap-2 text-info cursor-pointer hover:underline bg-transparent border-none p-0"
|
||||
>
|
||||
<GitPullRequest className="h-5 w-5" />
|
||||
<span className="font-medium">{t(TASK_STATUS_LABELS[task.status])}</span>
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -295,12 +357,12 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals,
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Badge
|
||||
variant={task.status === 'done' ? 'success' : task.status === 'human_review' ? 'purple' : task.status === 'in_progress' ? 'info' : 'secondary'}
|
||||
className={cn('text-xs', (task.status === 'in_progress' && !state.isStuck) && 'status-running')}
|
||||
>
|
||||
{t(TASK_STATUS_LABELS[task.status])}
|
||||
</Badge>
|
||||
<Badge
|
||||
variant={getStatusBadgeVariant(task.status, state.isStuck)}
|
||||
className={cn('text-xs', (task.status === 'in_progress' && !state.isStuck) && 'status-running')}
|
||||
>
|
||||
{t(TASK_STATUS_LABELS[task.status])}
|
||||
</Badge>
|
||||
{task.status === 'human_review' && task.reviewReason && (
|
||||
<Badge
|
||||
variant={task.reviewReason === 'completed' ? 'success' : task.reviewReason === 'errors' ? 'destructive' : 'warning'}
|
||||
@@ -442,6 +504,10 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals,
|
||||
onClose={handleClose}
|
||||
onSwitchToTerminals={onSwitchToTerminals}
|
||||
onOpenInbuiltTerminal={onOpenInbuiltTerminal}
|
||||
showPRDialog={state.showPRDialog}
|
||||
isCreatingPR={state.isCreatingPR}
|
||||
onShowPRDialog={state.setShowPRDialog}
|
||||
onCreatePR={handleCreatePR}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
||||
@@ -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 && (
|
||||
<div className="bg-muted/30 rounded-lg px-4 py-3 border border-border/50 overflow-hidden max-w-full">
|
||||
<div
|
||||
<div
|
||||
className="prose prose-sm prose-invert max-w-none overflow-hidden prose-p:text-foreground/90 prose-p:leading-relaxed prose-headings:text-foreground prose-strong:text-foreground prose-li:text-foreground/90 prose-ul:my-2 prose-li:my-0.5 prose-a:break-all prose-pre:overflow-x-auto prose-img:max-w-full [&_img]:!max-w-full [&_img]:h-auto [&_code]:break-all [&_code]:whitespace-pre-wrap [&_*]:max-w-full"
|
||||
style={{ wordBreak: 'break-word', overflowWrap: 'anywhere' }}
|
||||
>
|
||||
@@ -201,6 +205,24 @@ export function TaskMetadata({ task }: TaskMetadataProps) {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Pull Request */}
|
||||
{task.metadata.prUrl && (
|
||||
<div>
|
||||
<h3 className="text-xs font-medium text-muted-foreground uppercase tracking-wide mb-1.5 flex items-center gap-1.5">
|
||||
<GitPullRequest className="h-3 w-3 text-info" />
|
||||
{t('tasks:metadata.pullRequest')}
|
||||
</h3>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => window.electronAPI.openExternal(task.metadata!.prUrl!)}
|
||||
className="text-sm text-info hover:underline flex items-center gap-1.5 bg-transparent border-none cursor-pointer p-0 text-left"
|
||||
>
|
||||
{task.metadata.prUrl}
|
||||
<ExternalLink className="h-3 w-3" />
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Acceptance Criteria */}
|
||||
{task.metadata.acceptanceCriteria && task.metadata.acceptanceCriteria.length > 0 && (
|
||||
<div>
|
||||
|
||||
@@ -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<WorktreeCreatePRResult | null>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -83,7 +89,11 @@ export function TaskReview({
|
||||
onLoadMergePreview,
|
||||
onClose,
|
||||
onSwitchToTerminals,
|
||||
onOpenInbuiltTerminal
|
||||
onOpenInbuiltTerminal,
|
||||
showPRDialog,
|
||||
isCreatingPR,
|
||||
onShowPRDialog,
|
||||
onCreatePR
|
||||
}: TaskReviewProps) {
|
||||
return (
|
||||
<div className="space-y-4">
|
||||
@@ -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 */}
|
||||
<CreatePRDialog
|
||||
open={showPRDialog}
|
||||
task={task}
|
||||
worktreeStatus={worktreeStatus}
|
||||
onOpenChange={onShowPRDialog}
|
||||
onCreatePR={onCreatePR}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
+409
@@ -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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
const branchInput = screen.getByLabelText(/target branch/i);
|
||||
expect(branchInput).toHaveValue('develop');
|
||||
});
|
||||
});
|
||||
|
||||
it('should display source branch info', async () => {
|
||||
render(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('auto-claude/implement-user-authentication')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('should display commit count and changes', async () => {
|
||||
render(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
// 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(
|
||||
<CreatePRDialog
|
||||
open={false}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
// Reopen dialog
|
||||
rerender(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
// 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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<CreatePRDialog
|
||||
open={true}
|
||||
task={mockTask}
|
||||
worktreeStatus={mockWorktreeStatus}
|
||||
onOpenChange={mockOnOpenChange}
|
||||
onCreatePR={mockOnCreatePR}
|
||||
/>
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<WorktreeCreatePRResult | null>;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<WorktreeCreatePRResult | null>(null);
|
||||
const [error, setError] = useState<string | null>(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 (
|
||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
||||
<DialogContent className="sm:max-w-[500px]">
|
||||
<DialogHeader>
|
||||
<DialogTitle className="flex items-center gap-2">
|
||||
<GitPullRequest className="h-5 w-5 text-primary" />
|
||||
{t('taskReview:pr.title')}
|
||||
</DialogTitle>
|
||||
<DialogDescription>
|
||||
{t('taskReview:pr.description', { taskTitle: task.title })}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
|
||||
{/* Success State */}
|
||||
{result?.success && (
|
||||
<div className="space-y-4">
|
||||
<div className="bg-success/10 border border-success/30 rounded-lg p-4">
|
||||
<p className="text-sm text-success font-medium mb-2">
|
||||
{result.alreadyExists
|
||||
? t('taskReview:pr.success.alreadyExists')
|
||||
: t('taskReview:pr.success.created')}
|
||||
</p>
|
||||
{result.prUrl && (
|
||||
<button
|
||||
type="button"
|
||||
data-testid="pr-link-button"
|
||||
onClick={handleOpenPR}
|
||||
className="text-sm text-primary hover:underline flex items-center gap-1 bg-transparent border-none cursor-pointer p-0"
|
||||
>
|
||||
{result.prUrl}
|
||||
<ExternalLink className="h-3 w-3" />
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
<DialogFooter>
|
||||
<Button onClick={handleClose}>
|
||||
{t('common:buttons.close')}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Error State */}
|
||||
{error && !result?.success && (
|
||||
<div className="space-y-4">
|
||||
<div className="bg-destructive/10 border border-destructive/30 rounded-lg p-4">
|
||||
<p className="text-sm text-destructive">{error}</p>
|
||||
</div>
|
||||
<DialogFooter>
|
||||
<Button variant="outline" onClick={handleClose}>
|
||||
{t('common:buttons.cancel')}
|
||||
</Button>
|
||||
<Button onClick={handleCreatePR} disabled={isCreating}>
|
||||
{t('taskReview:pr.actions.retry')}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Form State */}
|
||||
{!result?.success && !error && (
|
||||
<div className="space-y-4">
|
||||
{/* Branch Info */}
|
||||
<div className="bg-muted/50 rounded-lg p-3 text-sm" data-testid="pr-stats-container">
|
||||
<div className="flex justify-between mb-1">
|
||||
<span className="text-muted-foreground">{t('taskReview:pr.labels.sourceBranch')}:</span>
|
||||
<span className="font-mono">{worktreeStatus?.branch || t('taskReview:pr.labels.unknown')}</span>
|
||||
</div>
|
||||
{worktreeStatus?.exists && (
|
||||
<>
|
||||
<div className="flex justify-between mb-1">
|
||||
<span className="text-muted-foreground">{t('taskReview:pr.labels.commits')}:</span>
|
||||
<span>{worktreeStatus.commitCount || 0}</span>
|
||||
</div>
|
||||
<div className="flex justify-between">
|
||||
<span className="text-muted-foreground">{t('taskReview:pr.labels.changes')}:</span>
|
||||
<span>
|
||||
<span className="text-success">+{worktreeStatus.additions || 0}</span>
|
||||
{' / '}
|
||||
<span className="text-destructive">-{worktreeStatus.deletions || 0}</span>
|
||||
</span>
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Target Branch */}
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="targetBranch">{t('taskReview:pr.labels.targetBranch')}</Label>
|
||||
<Input
|
||||
id="targetBranch"
|
||||
value={targetBranch}
|
||||
onChange={(e) => setTargetBranch(e.target.value)}
|
||||
placeholder={worktreeStatus?.baseBranch || 'main'}
|
||||
/>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t('taskReview:pr.hints.targetBranch')}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{/* PR Title (optional) */}
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="prTitle">{t('taskReview:pr.labels.prTitle')}</Label>
|
||||
<Input
|
||||
id="prTitle"
|
||||
value={prTitle}
|
||||
onChange={(e) => setPrTitle(e.target.value)}
|
||||
placeholder={task.title}
|
||||
/>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t('taskReview:pr.hints.prTitle')}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{/* Draft PR Checkbox */}
|
||||
<div className="flex items-center gap-2">
|
||||
<Checkbox
|
||||
id="draft-pr-checkbox"
|
||||
checked={isDraft}
|
||||
onCheckedChange={(checked) => setIsDraft(checked === true)}
|
||||
/>
|
||||
<label htmlFor="draft-pr-checkbox" className="text-sm cursor-pointer">
|
||||
{t('taskReview:pr.labels.draftPR')}
|
||||
</label>
|
||||
</div>
|
||||
|
||||
<DialogFooter>
|
||||
<Button variant="outline" onClick={handleClose} disabled={isCreating}>
|
||||
{t('common:buttons.cancel')}
|
||||
</Button>
|
||||
<Button onClick={handleCreatePR} disabled={isCreating}>
|
||||
{isCreating ? (
|
||||
<>
|
||||
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
|
||||
{t('taskReview:pr.actions.creating')}
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<GitPullRequest className="mr-2 h-4 w-4" />
|
||||
{t('taskReview:pr.actions.create')}
|
||||
</>
|
||||
)}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</div>
|
||||
)}
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
);
|
||||
}
|
||||
@@ -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({
|
||||
</Button>
|
||||
)}
|
||||
|
||||
{/* Create PR Button */}
|
||||
{onShowPRDialog && (
|
||||
<Button
|
||||
variant="info"
|
||||
onClick={() => onShowPRDialog(true)}
|
||||
disabled={isMerging || isDiscarding || isCreatingPR}
|
||||
className="flex-1"
|
||||
>
|
||||
{isCreatingPR ? (
|
||||
<>
|
||||
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
|
||||
{t('taskReview:pr.actions.creating')}
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<GitPullRequest className="mr-2 h-4 w-4" />
|
||||
{t('common:buttons.createPR')}
|
||||
</>
|
||||
)}
|
||||
</Button>
|
||||
)}
|
||||
|
||||
<Button
|
||||
variant="outline"
|
||||
size="icon"
|
||||
onClick={() => onShowDiscardDialog(true)}
|
||||
disabled={isMerging || isDiscarding}
|
||||
disabled={isMerging || isDiscarding || isCreatingPR}
|
||||
className="text-muted-foreground hover:text-destructive hover:bg-destructive/10 hover:border-destructive/30"
|
||||
title="Discard build"
|
||||
>
|
||||
|
||||
@@ -12,5 +12,6 @@ export { QAFeedbackSection } from './QAFeedbackSection';
|
||||
export { DiscardDialog } from './DiscardDialog';
|
||||
export { DiffViewDialog } from './DiffViewDialog';
|
||||
export { ConflictDetailsDialog } from './ConflictDetailsDialog';
|
||||
export { CreatePRDialog } from './CreatePRDialog';
|
||||
export { LoadingMessage, NoWorkspaceMessage, StagedInProjectMessage } from './WorkspaceMessages';
|
||||
export { getSeverityIcon, getSeverityVariant } from './utils';
|
||||
|
||||
@@ -158,7 +158,7 @@ export function usePtyProcess({
|
||||
isCreatingRef.current = false;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
}, [terminalId, cwd, projectPath, cols, rows, skipCreation, recreationTrigger, getStore, onCreated, onError]);
|
||||
|
||||
// Function to prepare for recreation by preventing the effect from running
|
||||
|
||||
@@ -24,6 +24,8 @@ const buttonVariants = cva(
|
||||
'bg-[var(--success)] text-[var(--success-foreground)] hover:bg-[var(--success)]/90 active:scale-[0.98]',
|
||||
warning:
|
||||
'bg-warning text-warning-foreground hover:bg-warning/90 active:scale-[0.98]',
|
||||
info:
|
||||
'bg-info text-info-foreground hover:bg-info/90 active:scale-[0.98]',
|
||||
},
|
||||
size: {
|
||||
default: 'h-10 px-4 py-2 text-sm rounded-lg',
|
||||
|
||||
@@ -54,6 +54,14 @@ export const workspaceMock = {
|
||||
}
|
||||
}),
|
||||
|
||||
createWorktreePR: async () => ({
|
||||
success: true,
|
||||
data: {
|
||||
success: true,
|
||||
prUrl: 'https://github.com/example/repo/pull/123'
|
||||
}
|
||||
}),
|
||||
|
||||
discardWorktree: async () => ({
|
||||
success: true,
|
||||
data: {
|
||||
|
||||
@@ -35,6 +35,7 @@ export const IPC_CHANNELS = {
|
||||
TASK_WORKTREE_MERGE: 'task:worktreeMerge',
|
||||
TASK_WORKTREE_MERGE_PREVIEW: 'task:worktreeMergePreview', // Preview merge conflicts before merging
|
||||
TASK_WORKTREE_DISCARD: 'task:worktreeDiscard',
|
||||
TASK_WORKTREE_CREATE_PR: 'task:worktreeCreatePR',
|
||||
TASK_WORKTREE_OPEN_IN_IDE: 'task:worktreeOpenInIDE',
|
||||
TASK_WORKTREE_OPEN_IN_TERMINAL: 'task:worktreeOpenInTerminal',
|
||||
TASK_WORKTREE_DETECT_TOOLS: 'task:worktreeDetectTools', // Detect installed IDEs/terminals
|
||||
|
||||
@@ -17,21 +17,25 @@ export const TASK_STATUS_COLUMNS = [
|
||||
] as const;
|
||||
|
||||
// Status label translation keys (use with t() from react-i18next)
|
||||
// Note: pr_created maps to 'done' column in Kanban view (see KanbanBoard.tsx)
|
||||
export const TASK_STATUS_LABELS: Record<string, string> = {
|
||||
backlog: 'columns.backlog',
|
||||
in_progress: 'columns.in_progress',
|
||||
ai_review: 'columns.ai_review',
|
||||
human_review: 'columns.human_review',
|
||||
done: 'columns.done'
|
||||
done: 'columns.done',
|
||||
pr_created: 'columns.pr_created'
|
||||
};
|
||||
|
||||
// Status colors for UI
|
||||
// Note: pr_created maps to 'done' column in Kanban view (see KanbanBoard.tsx)
|
||||
export const TASK_STATUS_COLORS: Record<string, string> = {
|
||||
backlog: 'bg-muted text-muted-foreground',
|
||||
in_progress: 'bg-info/10 text-info',
|
||||
ai_review: 'bg-warning/10 text-warning',
|
||||
human_review: 'bg-purple-500/10 text-purple-400',
|
||||
done: 'bg-success/10 text-success'
|
||||
done: 'bg-success/10 text-success',
|
||||
pr_created: 'bg-info/10 text-info'
|
||||
};
|
||||
|
||||
// ============================================
|
||||
|
||||
@@ -66,6 +66,8 @@
|
||||
"confirm": "Confirm",
|
||||
"retry": "Retry",
|
||||
"create": "Create",
|
||||
"createPR": "Create PR",
|
||||
"openPR": "Open PR",
|
||||
"open": "Open",
|
||||
"start": "Start",
|
||||
"stop": "Stop",
|
||||
|
||||
@@ -3,5 +3,36 @@
|
||||
"openTerminal": "Open terminal",
|
||||
"openInbuilt": "Open in Inbuilt Terminal",
|
||||
"openExternal": "Open in External Terminal"
|
||||
},
|
||||
"pr": {
|
||||
"title": "Create Pull Request",
|
||||
"description": "Push branch and create a pull request for \"{{taskTitle}}\"",
|
||||
"errors": {
|
||||
"unknown": "An unknown error occurred while creating the pull request",
|
||||
"invalidBranchName": "Branch name contains invalid characters. Use only letters, numbers, hyphens (-), underscores (_), and forward slashes (/).",
|
||||
"emptyTitle": "Pull request title cannot be empty."
|
||||
},
|
||||
"success": {
|
||||
"created": "Pull request created successfully!",
|
||||
"alreadyExists": "A pull request already exists for this branch"
|
||||
},
|
||||
"actions": {
|
||||
"retry": "Retry",
|
||||
"creating": "Creating PR...",
|
||||
"create": "Create Pull Request"
|
||||
},
|
||||
"labels": {
|
||||
"sourceBranch": "Source branch",
|
||||
"targetBranch": "Target branch",
|
||||
"commits": "Commits",
|
||||
"changes": "Changes",
|
||||
"prTitle": "PR Title (optional)",
|
||||
"draftPR": "Create as draft PR",
|
||||
"unknown": "Unknown"
|
||||
},
|
||||
"hints": {
|
||||
"targetBranch": "Leave empty to use the default branch",
|
||||
"prTitle": "Leave empty to use the task title"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
"todo": "To Do",
|
||||
"in_progress": "In Progress",
|
||||
"review": "Review",
|
||||
"prCreated": "PR Created",
|
||||
"complete": "Complete",
|
||||
"archived": "Archived"
|
||||
},
|
||||
@@ -15,6 +16,7 @@
|
||||
"archive": "Archive",
|
||||
"delete": "Delete",
|
||||
"view": "View Details",
|
||||
"viewPR": "View PR",
|
||||
"moveTo": "Move to",
|
||||
"taskActions": "Task actions"
|
||||
},
|
||||
@@ -37,7 +39,8 @@
|
||||
},
|
||||
"tooltips": {
|
||||
"archiveTask": "Archive task",
|
||||
"archiveAllDone": "Archive all done tasks"
|
||||
"archiveAllDone": "Archive all done tasks",
|
||||
"viewPR": "Open pull request in browser"
|
||||
},
|
||||
"creation": {
|
||||
"title": "Create New Task",
|
||||
@@ -53,7 +56,8 @@
|
||||
"in_progress": "In Progress",
|
||||
"ai_review": "AI Review",
|
||||
"human_review": "Human Review",
|
||||
"done": "Done"
|
||||
"done": "Done",
|
||||
"pr_created": "PR Created"
|
||||
},
|
||||
"kanban": {
|
||||
"emptyBacklog": "No tasks planned",
|
||||
@@ -110,7 +114,8 @@
|
||||
"openInIDE": "Open in IDE"
|
||||
},
|
||||
"metadata": {
|
||||
"severity": "severity"
|
||||
"severity": "severity",
|
||||
"pullRequest": "Pull Request"
|
||||
},
|
||||
"images": {
|
||||
"removeImageAriaLabel": "Remove image {{filename}}",
|
||||
|
||||
@@ -66,6 +66,8 @@
|
||||
"confirm": "Confirmer",
|
||||
"retry": "Réessayer",
|
||||
"create": "Créer",
|
||||
"createPR": "Créer PR",
|
||||
"openPR": "Ouvrir la PR",
|
||||
"open": "Ouvrir",
|
||||
"start": "Démarrer",
|
||||
"stop": "Arrêter",
|
||||
|
||||
@@ -3,5 +3,36 @@
|
||||
"openTerminal": "Ouvrir le terminal",
|
||||
"openInbuilt": "Ouvrir dans le terminal intégré",
|
||||
"openExternal": "Ouvrir dans le terminal externe"
|
||||
},
|
||||
"pr": {
|
||||
"title": "Créer une Pull Request",
|
||||
"description": "Pousser la branche et créer une pull request pour \"{{taskTitle}}\"",
|
||||
"errors": {
|
||||
"unknown": "Une erreur inconnue s'est produite lors de la création de la pull request",
|
||||
"invalidBranchName": "Le nom de branche contient des caractères invalides. Utilisez uniquement des lettres, chiffres, tirets (-), underscores (_) et barres obliques (/).",
|
||||
"emptyTitle": "Le titre de la pull request ne peut pas être vide."
|
||||
},
|
||||
"success": {
|
||||
"created": "Pull request créée avec succès !",
|
||||
"alreadyExists": "Une pull request existe déjà pour cette branche"
|
||||
},
|
||||
"actions": {
|
||||
"retry": "Réessayer",
|
||||
"creating": "Création en cours...",
|
||||
"create": "Créer la Pull Request"
|
||||
},
|
||||
"labels": {
|
||||
"sourceBranch": "Branche source",
|
||||
"targetBranch": "Branche cible",
|
||||
"commits": "Commits",
|
||||
"changes": "Modifications",
|
||||
"prTitle": "Titre de la PR (optionnel)",
|
||||
"draftPR": "Créer comme brouillon",
|
||||
"unknown": "Inconnu"
|
||||
},
|
||||
"hints": {
|
||||
"targetBranch": "Laissez vide pour utiliser la branche par défaut",
|
||||
"prTitle": "Laissez vide pour utiliser le titre de la tâche"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
"todo": "À faire",
|
||||
"in_progress": "En cours",
|
||||
"review": "Révision",
|
||||
"prCreated": "PR créée",
|
||||
"complete": "Terminé",
|
||||
"archived": "Archivé"
|
||||
},
|
||||
@@ -15,6 +16,7 @@
|
||||
"archive": "Archiver",
|
||||
"delete": "Supprimer",
|
||||
"view": "Voir les détails",
|
||||
"viewPR": "Voir la PR",
|
||||
"moveTo": "Déplacer vers",
|
||||
"taskActions": "Actions de la tâche"
|
||||
},
|
||||
@@ -37,7 +39,8 @@
|
||||
},
|
||||
"tooltips": {
|
||||
"archiveTask": "Archiver la tâche",
|
||||
"archiveAllDone": "Archiver toutes les tâches terminées"
|
||||
"archiveAllDone": "Archiver toutes les tâches terminées",
|
||||
"viewPR": "Ouvrir la pull request dans le navigateur"
|
||||
},
|
||||
"creation": {
|
||||
"title": "Créer une nouvelle tâche",
|
||||
@@ -53,7 +56,8 @@
|
||||
"in_progress": "En cours",
|
||||
"ai_review": "Révision IA",
|
||||
"human_review": "Révision humaine",
|
||||
"done": "Terminé"
|
||||
"done": "Terminé",
|
||||
"pr_created": "PR créée"
|
||||
},
|
||||
"kanban": {
|
||||
"emptyBacklog": "Aucune tâche planifiée",
|
||||
@@ -110,7 +114,8 @@
|
||||
"openInIDE": "Ouvrir dans l'IDE"
|
||||
},
|
||||
"metadata": {
|
||||
"severity": "sévérité"
|
||||
"severity": "sévérité",
|
||||
"pullRequest": "Pull Request"
|
||||
},
|
||||
"images": {
|
||||
"removeImageAriaLabel": "Supprimer l'image {{filename}}",
|
||||
|
||||
@@ -36,6 +36,8 @@ import type {
|
||||
WorktreeMergeResult,
|
||||
WorktreeDiscardResult,
|
||||
WorktreeListResult,
|
||||
WorktreeCreatePROptions,
|
||||
WorktreeCreatePRResult,
|
||||
TaskRecoveryResult,
|
||||
TaskRecoveryOptions,
|
||||
TaskMetadata,
|
||||
@@ -167,6 +169,7 @@ export interface ElectronAPI {
|
||||
getWorktreeDiff: (taskId: string) => Promise<IPCResult<WorktreeDiff>>;
|
||||
mergeWorktree: (taskId: string, options?: { noCommit?: boolean }) => Promise<IPCResult<WorktreeMergeResult>>;
|
||||
mergeWorktreePreview: (taskId: string) => Promise<IPCResult<WorktreeMergeResult>>;
|
||||
createWorktreePR: (taskId: string, options?: WorktreeCreatePROptions) => Promise<IPCResult<WorktreeCreatePRResult>>;
|
||||
discardWorktree: (taskId: string) => Promise<IPCResult<WorktreeDiscardResult>>;
|
||||
listWorktrees: (projectId: string) => Promise<IPCResult<WorktreeListResult>>;
|
||||
worktreeOpenInIDE: (worktreePath: string, ide: SupportedIDE, customPath?: string) => Promise<IPCResult<{ opened: boolean }>>;
|
||||
|
||||
@@ -246,12 +246,12 @@ export interface GraphitiMemoryState {
|
||||
error_log: Array<{ timestamp: string; error: string }>;
|
||||
}
|
||||
|
||||
export type MemoryType =
|
||||
| 'session_insight'
|
||||
| 'codebase_discovery'
|
||||
| 'codebase_map'
|
||||
| 'pattern'
|
||||
| 'gotcha'
|
||||
export type MemoryType =
|
||||
| 'session_insight'
|
||||
| 'codebase_discovery'
|
||||
| 'codebase_map'
|
||||
| 'pattern'
|
||||
| 'gotcha'
|
||||
| 'task_outcome'
|
||||
| 'pr_review'
|
||||
| 'pr_finding'
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
import type { ThinkingLevel, PhaseModelConfig, PhaseThinkingConfig } from './settings';
|
||||
import type { ExecutionPhase as ExecutionPhaseType } from '../constants/phase-protocol';
|
||||
|
||||
export type TaskStatus = 'backlog' | 'in_progress' | 'ai_review' | 'human_review' | 'done';
|
||||
export type TaskStatus = 'backlog' | 'in_progress' | 'ai_review' | 'human_review' | 'pr_created' | 'done';
|
||||
|
||||
// Reason why a task is in human_review status
|
||||
// - 'completed': All subtasks done and QA passed, ready for final approval/merge
|
||||
@@ -228,6 +228,7 @@ export interface TaskMetadata {
|
||||
|
||||
// Git/Worktree configuration
|
||||
baseBranch?: string; // Override base branch for this task's worktree
|
||||
prUrl?: string; // GitHub PR URL if task has been submitted as a PR
|
||||
useWorktree?: boolean; // If false, use direct mode (no worktree isolation) - default is true for safety
|
||||
|
||||
// Archive status
|
||||
@@ -404,6 +405,26 @@ export interface WorktreeDiscardResult {
|
||||
message: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for creating a PR from a worktree
|
||||
*/
|
||||
export interface WorktreeCreatePROptions {
|
||||
targetBranch?: string;
|
||||
title?: string;
|
||||
draft?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of creating a PR from a worktree
|
||||
*/
|
||||
export interface WorktreeCreatePRResult {
|
||||
success: boolean;
|
||||
prUrl?: string;
|
||||
error?: string;
|
||||
message?: string; // Human-readable message for both success and error cases
|
||||
alreadyExists?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Information about a single spec worktree
|
||||
* Per-spec architecture: Each spec has its own worktree at .worktrees/{spec-name}/
|
||||
|
||||
@@ -31,79 +31,94 @@ PRWorktreeManager = pr_worktree_module.PRWorktreeManager
|
||||
|
||||
@pytest.fixture
|
||||
def temp_git_repo():
|
||||
"""Create a temporary git repository with remote origin for testing."""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
# Create a bare repo to act as "origin"
|
||||
origin_dir = Path(tmpdir) / "origin.git"
|
||||
origin_dir.mkdir()
|
||||
subprocess.run(
|
||||
["git", "init", "--bare"], cwd=origin_dir, check=True, capture_output=True
|
||||
)
|
||||
"""Create a temporary git repository with remote origin for testing.
|
||||
|
||||
# Create the working repo
|
||||
repo_dir = Path(tmpdir) / "test_repo"
|
||||
repo_dir.mkdir()
|
||||
Note: This fixture clears GIT_INDEX_FILE to avoid interference from pre-commit hooks.
|
||||
Pre-commit sets GIT_INDEX_FILE to a relative path (.git/index.pre-commit) which causes
|
||||
git commands in the temp repo to fail with "index file open failed: Not a directory"
|
||||
because the relative path resolves against the main repo, not the temp repo.
|
||||
"""
|
||||
# Clear pre-commit's GIT_INDEX_FILE to avoid interference with temp repo git commands
|
||||
# Pre-commit sets this to a relative path that breaks git operations in other repos
|
||||
saved_git_index_file = os.environ.pop('GIT_INDEX_FILE', None)
|
||||
|
||||
# Initialize git repo
|
||||
subprocess.run(["git", "init"], cwd=repo_dir, check=True, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "config", "user.email", "test@example.com"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.name", "Test User"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
try:
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
# Create a bare repo to act as "origin"
|
||||
origin_dir = Path(tmpdir) / "origin.git"
|
||||
origin_dir.mkdir()
|
||||
subprocess.run(
|
||||
["git", "init", "--bare"], cwd=origin_dir, check=True, capture_output=True
|
||||
)
|
||||
|
||||
# Add origin remote
|
||||
subprocess.run(
|
||||
["git", "remote", "add", "origin", str(origin_dir)],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
# Create the working repo
|
||||
repo_dir = Path(tmpdir) / "test_repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
# Create initial commit
|
||||
test_file = repo_dir / "test.txt"
|
||||
test_file.write_text("initial content")
|
||||
subprocess.run(["git", "add", "."], cwd=repo_dir, check=True, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "Initial commit"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
# Initialize git repo
|
||||
subprocess.run(["git", "init"], cwd=repo_dir, check=True, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "config", "user.email", "test@example.com"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.name", "Test User"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
# Push to origin so refs exist
|
||||
subprocess.run(
|
||||
["git", "push", "-u", "origin", "HEAD:main"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
# Add origin remote
|
||||
subprocess.run(
|
||||
["git", "remote", "add", "origin", str(origin_dir)],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
# Get the commit SHA
|
||||
result = subprocess.run(
|
||||
["git", "rev-parse", "HEAD"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
commit_sha = result.stdout.strip()
|
||||
# Create initial commit
|
||||
test_file = repo_dir / "test.txt"
|
||||
test_file.write_text("initial content")
|
||||
subprocess.run(["git", "add", "."], cwd=repo_dir, check=True, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "Initial commit"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
yield repo_dir, commit_sha
|
||||
# Push to origin so refs exist
|
||||
subprocess.run(
|
||||
["git", "push", "-u", "origin", "HEAD:main"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
# Cleanup worktrees before removing directory
|
||||
subprocess.run(
|
||||
["git", "worktree", "prune"],
|
||||
cwd=repo_dir,
|
||||
capture_output=True,
|
||||
)
|
||||
# Get the commit SHA
|
||||
result = subprocess.run(
|
||||
["git", "rev-parse", "HEAD"],
|
||||
cwd=repo_dir,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
commit_sha = result.stdout.strip()
|
||||
|
||||
yield repo_dir, commit_sha
|
||||
|
||||
# Cleanup worktrees before removing directory
|
||||
subprocess.run(
|
||||
["git", "worktree", "prune"],
|
||||
cwd=repo_dir,
|
||||
capture_output=True,
|
||||
)
|
||||
finally:
|
||||
# Restore GIT_INDEX_FILE if it was set
|
||||
if saved_git_index_file is not None:
|
||||
os.environ['GIT_INDEX_FILE'] = saved_git_index_file
|
||||
|
||||
|
||||
def test_create_and_remove_worktree(temp_git_repo):
|
||||
|
||||
@@ -99,18 +99,18 @@ def test_cache_invalidation_on_file_modification(mock_project_dir, mock_profile_
|
||||
|
||||
def test_cache_invalidation_on_file_deletion(mock_project_dir, mock_profile_path):
|
||||
reset_profile_cache()
|
||||
|
||||
|
||||
# 1. Create file
|
||||
current_hash = get_dir_hash(mock_project_dir)
|
||||
mock_profile_path.write_text(create_valid_profile_json(["unique_cmd_A"], current_hash))
|
||||
|
||||
|
||||
# 2. Load profile
|
||||
profile1 = get_security_profile(mock_project_dir)
|
||||
assert "unique_cmd_A" in profile1.get_all_allowed_commands()
|
||||
|
||||
|
||||
# 3. Delete file
|
||||
mock_profile_path.unlink()
|
||||
|
||||
|
||||
# 4. Call again - should handle deletion gracefully and fallback to fresh analysis
|
||||
profile2 = get_security_profile(mock_project_dir)
|
||||
assert "unique_cmd_A" not in profile2.get_all_allowed_commands()
|
||||
assert "unique_cmd_A" not in profile2.get_all_allowed_commands()
|
||||
|
||||
Reference in New Issue
Block a user