diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4758d6396a..59f1be1485 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -522,144 +522,6 @@ jobs: sha256sum ./* > checksums.sha256 cat checksums.sha256 - - name: Scan with VirusTotal - id: virustotal - continue-on-error: true - if: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && inputs.dry_run != true) }} - env: - VT_API_KEY: ${{ secrets.VIRUSTOTAL_API_KEY }} - run: | - if [ -z "$VT_API_KEY" ]; then - echo "::warning::VIRUSTOTAL_API_KEY not configured, skipping scan" - echo "vt_results=" >> $GITHUB_OUTPUT - exit 0 - fi - - echo "## VirusTotal Scan Results" > vt_results.md - echo "" >> vt_results.md - - for file in release-assets/*.{exe,dmg,AppImage,deb,flatpak}; do - [ -f "$file" ] || continue - filename=$(basename "$file") - filesize=$(stat -c%s "$file" 2>/dev/null || stat -f%z "$file") - echo "Scanning $filename (${filesize} bytes)..." - - # For files > 32MB, get a special upload URL first - if [ "$filesize" -gt 33554432 ]; then - echo " Large file detected, requesting upload URL..." - upload_url_response=$(curl -s --request GET \ - --url "https://www.virustotal.com/api/v3/files/upload_url" \ - --header "x-apikey: $VT_API_KEY") - - upload_url=$(echo "$upload_url_response" | jq -r '.data // empty') - if [ -z "$upload_url" ]; then - echo "::warning::Failed to get upload URL for large file $filename" - echo "Response: $upload_url_response" - echo "- $filename - ⚠️ Upload failed (large file)" >> vt_results.md - continue - fi - api_url="$upload_url" - else - api_url="https://www.virustotal.com/api/v3/files" - fi - - # Upload file to VirusTotal - response=$(curl -s --request POST \ - --url "$api_url" \ - --header "x-apikey: $VT_API_KEY" \ - --form "file=@$file") - - # Check if response is valid JSON before parsing - if ! echo "$response" | jq -e . >/dev/null 2>&1; then - echo "::warning::VirusTotal returned invalid JSON for $filename" - echo "Response (first 500 chars): ${response:0:500}" - echo "- $filename - ⚠️ Scan failed (invalid response)" >> vt_results.md - continue - fi - - # Check for API error response - error_code=$(echo "$response" | jq -r '.error.code // empty') - if [ -n "$error_code" ]; then - error_msg=$(echo "$response" | jq -r '.error.message // "Unknown error"') - echo "::warning::VirusTotal API error for $filename: $error_code - $error_msg" - echo "- $filename - ⚠️ Scan failed ($error_code)" >> vt_results.md - continue - fi - - # Extract analysis ID - analysis_id=$(echo "$response" | jq -r '.data.id // empty') - - if [ -z "$analysis_id" ]; then - echo "::warning::Failed to upload $filename to VirusTotal" - echo "Response: $response" - echo "- $filename - ⚠️ Upload failed" >> vt_results.md - continue - fi - - echo "Uploaded $filename, analysis ID: $analysis_id" - - # Wait for analysis to complete (max 5 minutes per file) - analysis="" - for i in {1..30}; do - sleep 10 - analysis=$(curl -s --request GET \ - --url "https://www.virustotal.com/api/v3/analyses/$analysis_id" \ - --header "x-apikey: $VT_API_KEY") - - # Validate JSON response - if ! echo "$analysis" | jq -e . >/dev/null 2>&1; then - echo " Warning: Invalid JSON response on attempt $i, retrying..." - continue - fi - - status=$(echo "$analysis" | jq -r '.data.attributes.status // "unknown"') - echo " Status: $status (attempt $i/30)" - - if [ "$status" = "completed" ]; then - break - fi - done - - # Final validation that we have valid analysis data - if ! echo "$analysis" | jq -e '.data.attributes.stats' >/dev/null 2>&1; then - echo "::warning::Could not get complete analysis for $filename, using local hash" - file_hash=$(sha256sum "$file" | cut -d' ' -f1) - echo "- [$filename](https://www.virustotal.com/gui/file/$file_hash) - ⚠️ Analysis incomplete" >> vt_results.md - continue - fi - - # Get file hash for permanent URL - file_hash=$(echo "$analysis" | jq -r '.meta.file_info.sha256 // empty') - - if [ -z "$file_hash" ]; then - # Fallback: calculate hash locally - file_hash=$(sha256sum "$file" | cut -d' ' -f1) - fi - - # Get detection stats - malicious=$(echo "$analysis" | jq -r '.data.attributes.stats.malicious // 0') - suspicious=$(echo "$analysis" | jq -r '.data.attributes.stats.suspicious // 0') - undetected=$(echo "$analysis" | jq -r '.data.attributes.stats.undetected // 0') - - vt_url="https://www.virustotal.com/gui/file/$file_hash" - - if [ "$malicious" -gt 0 ] || [ "$suspicious" -gt 0 ]; then - echo "::warning::$filename has $malicious malicious and $suspicious suspicious detections (likely false positives)" - echo "- [$filename]($vt_url) - ⚠️ **$malicious malicious, $suspicious suspicious** detections (review recommended)" >> vt_results.md - else - echo "$filename is clean ($undetected engines, 0 detections)" - echo "- [$filename]($vt_url) - ✅ Clean ($undetected engines, 0 detections)" >> vt_results.md - fi - done - - echo "" >> vt_results.md - - # Save results for release notes - cat vt_results.md - echo "vt_results<> $GITHUB_OUTPUT - cat vt_results.md >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT - - name: Dry run summary if: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run == true }} run: | @@ -741,9 +603,9 @@ jobs: --- - ${{ steps.virustotal.outputs.vt_results }} - **Full Changelog**: https://github.com/${{ github.repository }}/blob/main/CHANGELOG.md + + _VirusTotal scan results will be added automatically after release._ files: release-assets/* draft: false prerelease: ${{ contains(github.ref, 'beta') || contains(github.ref, 'alpha') }} diff --git a/.github/workflows/test-azure-auth.yml b/.github/workflows/test-azure-auth.yml new file mode 100644 index 0000000000..f3fb73c276 --- /dev/null +++ b/.github/workflows/test-azure-auth.yml @@ -0,0 +1,21 @@ +name: Test Azure Auth + +on: + workflow_dispatch: + +jobs: + test-auth: + runs-on: windows-latest + permissions: + id-token: write + contents: read + steps: + - name: Azure Login (OIDC) + uses: azure/login@v2 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + + - name: Success + run: echo "Azure authentication successful!" diff --git a/.github/workflows/virustotal-scan.yml b/.github/workflows/virustotal-scan.yml new file mode 100644 index 0000000000..6223337058 --- /dev/null +++ b/.github/workflows/virustotal-scan.yml @@ -0,0 +1,343 @@ +name: VirusTotal Scan + +# Runs AFTER release is published to avoid blocking release creation +# VirusTotal scans can take 5+ minutes per file, which delays releases + +on: + release: + types: [published] + workflow_dispatch: + inputs: + tag: + description: 'Release tag to scan (e.g., v2.8.0)' + required: true + type: string + +# Prevent TOCTOU race condition when updating release notes +# If two runs target the same tag, queue them instead of running in parallel +concurrency: + group: virustotal-${{ github.event.inputs.tag || github.event.release.tag_name }} + cancel-in-progress: false + +jobs: + scan: + name: Scan release assets + runs-on: ubuntu-latest + permissions: + contents: write # Required to update release notes + steps: + - name: Determine release tag + id: tag + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + echo "tag=${{ github.event.inputs.tag }}" >> $GITHUB_OUTPUT + else + echo "tag=${{ github.event.release.tag_name }}" >> $GITHUB_OUTPUT + fi + + - name: Check for API key + id: check-key + env: + VT_KEY: ${{ secrets.VIRUSTOTAL_API_KEY }} + run: | + if [ -z "$VT_KEY" ]; then + echo "::warning::VIRUSTOTAL_API_KEY not configured, skipping scan" + echo "has_key=false" >> $GITHUB_OUTPUT + else + echo "has_key=true" >> $GITHUB_OUTPUT + fi + + - name: Download release assets + if: steps.check-key.outputs.has_key == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + TAG="${{ steps.tag.outputs.tag }}" + echo "Downloading assets for release $TAG..." + + mkdir -p release-assets + + # First verify the release exists + if ! gh release view "$TAG" --repo "${{ github.repository }}" >/dev/null 2>&1; then + echo "::error::Release $TAG not found" + exit 1 + fi + + # Download assets, distinguishing between "no matching assets" and real errors + set +e + gh release download "$TAG" \ + --repo "${{ github.repository }}" \ + --pattern "*.exe" \ + --pattern "*.dmg" \ + --pattern "*.AppImage" \ + --pattern "*.deb" \ + --pattern "*.flatpak" \ + --dir release-assets 2>&1 + exit_code=$? + set -e + + if [ $exit_code -ne 0 ]; then + # Check if it's just "no assets matched" vs a real error + asset_count=$(gh release view "$TAG" --repo "${{ github.repository }}" --json assets -q '.assets | length') + if [ "$asset_count" -eq 0 ]; then + echo "Release has no assets yet (this is OK for new releases)" + else + # Check if any scannable assets exist that should have been downloaded + scannable_assets=$(gh release view "$TAG" --repo "${{ github.repository }}" --json assets \ + -q '.assets[].name | select(test("\\.(exe|dmg|AppImage|deb|flatpak)$"))' | wc -l) + if [ "$scannable_assets" -gt 0 ]; then + echo "::error::Download failed - $scannable_assets scannable asset(s) exist but download failed" + exit 1 + fi + echo "No assets matched the patterns (exe, dmg, AppImage, deb, flatpak)" + fi + fi + + echo "Downloaded assets:" + ls -la release-assets/ || echo "No assets found" + + - name: Scan with VirusTotal + if: steps.check-key.outputs.has_key == 'true' + id: virustotal + env: + VT_API_KEY: ${{ secrets.VIRUSTOTAL_API_KEY }} + run: | + echo "## VirusTotal Scan Results" > vt_results.md + echo "" >> vt_results.md + + # Check if there are any files to scan + shopt -s nullglob + files=(release-assets/*.{exe,dmg,AppImage,deb,flatpak}) + if [ ${#files[@]} -eq 0 ]; then + echo "No scannable files found in release assets" + echo "- No executable files found in release" >> vt_results.md + echo "vt_results<> $GITHUB_OUTPUT + cat vt_results.md >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + exit 0 + fi + + for file in "${files[@]}"; do + [ -f "$file" ] || continue + filename=$(basename "$file") + filesize=$(stat -c%s "$file" 2>/dev/null || stat -f%z "$file") + echo "Scanning $filename (${filesize} bytes)..." + + # VirusTotal requires special upload URL for files > 32MB + LARGE_FILE_THRESHOLD=33554432 # 32 MB in bytes + if [ "$filesize" -gt "$LARGE_FILE_THRESHOLD" ]; then + echo " Large file detected, requesting upload URL..." + upload_http_response=$(curl -s -w '\n%{http_code}' --request GET \ + --url "https://www.virustotal.com/api/v3/files/upload_url" \ + --header "x-apikey: $VT_API_KEY") + upload_http_code=$(echo "$upload_http_response" | tail -1) + upload_url_response=$(echo "$upload_http_response" | sed '$d') + + if [ "$upload_http_code" != "200" ]; then + echo "::warning::Failed to get upload URL for large file $filename (HTTP $upload_http_code)" + echo "- $filename - ⚠️ Upload failed (large file, HTTP $upload_http_code)" >> vt_results.md + continue + fi + + upload_url=$(echo "$upload_url_response" | jq -r '.data // empty') + if [ -z "$upload_url" ]; then + echo "::warning::Failed to get upload URL for large file $filename" + echo "Response: $upload_url_response" + echo "- $filename - ⚠️ Upload failed (large file)" >> vt_results.md + continue + fi + api_url="$upload_url" + else + api_url="https://www.virustotal.com/api/v3/files" + fi + + # Upload file to VirusTotal (capture HTTP status code) + http_response=$(curl -s -w '\n%{http_code}' --request POST \ + --url "$api_url" \ + --header "x-apikey: $VT_API_KEY" \ + --form "file=@$file") + http_code=$(echo "$http_response" | tail -1) + response=$(echo "$http_response" | sed '$d') + + # Check HTTP status code first + if [ "$http_code" != "200" ]; then + echo "::warning::VirusTotal returned HTTP $http_code for $filename" + if [ "$http_code" = "429" ]; then + echo "- $filename - ⚠️ Scan failed (rate limited)" >> vt_results.md + elif [ "$http_code" = "403" ]; then + echo "- $filename - ⚠️ Scan failed (forbidden - check API key)" >> vt_results.md + else + echo "- $filename - ⚠️ Scan failed (HTTP $http_code)" >> vt_results.md + fi + continue + fi + + # Check if response is valid JSON before parsing + if ! echo "$response" | jq -e . >/dev/null 2>&1; then + echo "::warning::VirusTotal returned invalid JSON for $filename" + echo "Response (first 500 chars): ${response:0:500}" + echo "- $filename - ⚠️ Scan failed (invalid response)" >> vt_results.md + continue + fi + + # Check for API error response + error_code=$(echo "$response" | jq -r '.error.code // empty') + if [ -n "$error_code" ]; then + error_msg=$(echo "$response" | jq -r '.error.message // "Unknown error"') + echo "::warning::VirusTotal API error for $filename: $error_code - $error_msg" + echo "- $filename - ⚠️ Scan failed ($error_code)" >> vt_results.md + continue + fi + + # Extract analysis ID + analysis_id=$(echo "$response" | jq -r '.data.id // empty') + + if [ -z "$analysis_id" ]; then + echo "::warning::Failed to upload $filename to VirusTotal" + echo "Response: $response" + echo "- $filename - ⚠️ Upload failed" >> vt_results.md + continue + fi + + echo "Uploaded $filename, analysis ID: $analysis_id" + + # Wait for analysis to complete (max 5 minutes per file) + analysis="" + for i in {1..30}; do + sleep 10 + analysis_http_response=$(curl -s -w '\n%{http_code}' --request GET \ + --url "https://www.virustotal.com/api/v3/analyses/$analysis_id" \ + --header "x-apikey: $VT_API_KEY") + analysis_http_code=$(echo "$analysis_http_response" | tail -1) + analysis=$(echo "$analysis_http_response" | sed '$d') + + # Check HTTP status code + if [ "$analysis_http_code" != "200" ]; then + echo " Warning: HTTP $analysis_http_code on attempt $i, retrying..." + if [ "$analysis_http_code" = "429" ]; then + echo " Rate limited, waiting longer..." + sleep 30 + fi + continue + fi + + # Validate JSON response + if ! echo "$analysis" | jq -e . >/dev/null 2>&1; then + echo " Warning: Invalid JSON response on attempt $i, retrying..." + continue + fi + + status=$(echo "$analysis" | jq -r '.data.attributes.status // "unknown"') + echo " Status: $status (attempt $i/30)" + + if [ "$status" = "completed" ]; then + break + fi + done + + # Handle analysis timeout - if loop completed without status=completed + if [ "$status" != "completed" ]; then + echo "::warning::Analysis timed out for $filename (status: $status after 5 minutes)" + file_hash=$(sha256sum "$file" | cut -d' ' -f1) + echo "- [$filename](https://www.virustotal.com/gui/file/$file_hash) - ⚠️ Analysis timed out" >> vt_results.md + continue + fi + + # Final validation that we have valid analysis data + if ! echo "$analysis" | jq -e '.data.attributes.stats' >/dev/null 2>&1; then + echo "::warning::Could not get complete analysis for $filename, using local hash" + file_hash=$(sha256sum "$file" | cut -d' ' -f1) + echo "- [$filename](https://www.virustotal.com/gui/file/$file_hash) - ⚠️ Analysis incomplete" >> vt_results.md + continue + fi + + # Get file hash for permanent URL + file_hash=$(echo "$analysis" | jq -r '.meta.file_info.sha256 // empty') + + if [ -z "$file_hash" ]; then + # Fallback: calculate hash locally + file_hash=$(sha256sum "$file" | cut -d' ' -f1) + fi + + # Get detection stats + malicious=$(echo "$analysis" | jq -r '.data.attributes.stats.malicious // 0') + suspicious=$(echo "$analysis" | jq -r '.data.attributes.stats.suspicious // 0') + undetected=$(echo "$analysis" | jq -r '.data.attributes.stats.undetected // 0') + + vt_url="https://www.virustotal.com/gui/file/$file_hash" + + if [ "$malicious" -gt 0 ] || [ "$suspicious" -gt 0 ]; then + echo "::warning::$filename has $malicious malicious and $suspicious suspicious detections (likely false positives)" + echo "- [$filename]($vt_url) - ⚠️ **$malicious malicious, $suspicious suspicious** detections (review recommended)" >> vt_results.md + else + echo "$filename is clean ($undetected engines, 0 detections)" + echo "- [$filename]($vt_url) - ✅ Clean ($undetected engines, 0 detections)" >> vt_results.md + fi + done + + echo "" >> vt_results.md + + # Save results for next step + cat vt_results.md + echo "vt_results<> $GITHUB_OUTPUT + cat vt_results.md >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + - name: Update release notes with scan results + if: steps.check-key.outputs.has_key == 'true' && steps.virustotal.outputs.vt_results != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + TAG="${{ steps.tag.outputs.tag }}" + + # Get current release body with error checking + if ! current_body=$(gh release view "$TAG" --repo "${{ github.repository }}" --json body -q '.body'); then + echo "::error::Failed to fetch current release body for $TAG" + exit 1 + fi + + # Additional safeguard for empty body + if [ -z "$current_body" ]; then + echo "::warning::Release body is empty, this may indicate a problem" + fi + + # Check if VirusTotal results already exist in the body + if echo "$current_body" | grep -q "## VirusTotal Scan Results"; then + echo "VirusTotal results already in release notes, skipping update" + exit 0 + fi + + # Use file-based approach to avoid shell expansion issues + # First, write current body to file + echo "$current_body" > release-body.md + + # Remove placeholder text if present (portable sed approach) + sed '/_VirusTotal scan results will be added automatically after release\./d' release-body.md > release-body.tmp && mv release-body.tmp release-body.md + + # Append separator and VT results + echo "" >> release-body.md + echo "---" >> release-body.md + echo "" >> release-body.md + cat vt_results.md >> release-body.md + + # Update release using --notes-file to avoid shell quoting issues + gh release edit "$TAG" \ + --repo "${{ github.repository }}" \ + --notes-file release-body.md + + echo "✅ Updated release notes with VirusTotal scan results" + + - name: Summary + if: always() + run: | + echo "## VirusTotal Scan Summary" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Release:** ${{ steps.tag.outputs.tag }}" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + if [ "${{ steps.check-key.outputs.has_key }}" = "false" ]; then + echo "⚠️ Scan skipped: VIRUSTOTAL_API_KEY not configured" >> $GITHUB_STEP_SUMMARY + elif [ -f vt_results.md ]; then + cat vt_results.md >> $GITHUB_STEP_SUMMARY + else + echo "No scan results available" >> $GITHUB_STEP_SUMMARY + fi diff --git a/.gitignore b/.gitignore index 6d2e458532..e0acc0dc5b 100644 --- a/.gitignore +++ b/.gitignore @@ -168,3 +168,4 @@ OPUS_ANALYSIS_AND_IDEAS.md # Auto Claude generated files .security-key +/shared_docs \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 26892a833d..55704e2bd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,94 @@ +## 2.7.4 - Terminal & Workflow Enhancements + +### ✨ New Features + +- Added task worktrees section in terminal with ability to invoke Claude with YOLO mode (--dangerously-skip-permissions) + +- Added searchable branch combobox to worktree creation dialog for easier branch selection + +- Added Claude Code version rollback feature to switch between installed versions + +- Embedded Sentry DSN at build time for better error tracking in packaged apps + +### 🛠️ Improvements + +- Made worktree isolation prominent in UI to help users understand workspace isolation + +- Enhanced terminal recreation logic with retry mechanism for more reliable terminal recovery + +- Improved worktree name input UX for better user experience + +- Improved Claude CLI detection with installation selector when multiple versions found + +- Enhanced terminal drag and drop reordering with collision detection + +- Synced worktree config to renderer on terminal restoration for consistency + +### 🐛 Bug Fixes + +- Fixed Windows claude.cmd validation in GUI to work reliably across different setups + +- Fixed profile manager initialization timing issue before auth checks + +- Fixed terminal recreation and label reset when user closes Claude + +- Fixed duplicate Kanban task creation that occurred on rapid button clicks + +- Fixed GitHub PR preloading to prevent loading PRs currently under review + +- Fixed UI to display actual base branch name instead of hardcoded "main" + +- Fixed Claude CLI detection to properly identify available installations + +- Fixed broken pipe errors in backend with Sentry integration + +- Fixed app update state persistence for Install button visibility + +- Fixed merge logic to include files with content changes even when semantic analysis is empty + +- Fixed security profile inheritance in worktrees and shell -c command validation + +- Fixed auth auto-switch on 401 errors and improved OAuth-only profile handling + +- Fixed "already up to date" case handling in worktree operations + +- Resolved circular import issues in GitHub context gatherer and services + +--- + +## What's Changed + +- fix: validate Windows claude.cmd reliably in GUI by @Umaru in 1ae3359b +- fix: await profile manager initialization before auth check by @StillKnotKnown in c8374bc1 +- feat: add file/screenshot upload to QA feedback interface by @Andy in 88277f84 +- feat(terminal): add task worktrees section and remove terminal limit by @Andy in 17118b07 +- fix(terminal): enhance terminal recreation logic with retry mechanism by @Andy in df1b8a3f +- fix(terminal): improve worktree name input UX by @Andy in 54e9f228 +- feat(ui): make worktree isolation prominent in UI by @Andy in 4dbb7ee4 +- feat(terminal): add YOLO mode to invoke Claude with --dangerously-skip-permissions by @Andy in d48e5f68 +- fix(ui): prevent duplicate Kanban task creation on rapid button clicks by @Andy in 2d1d3ef1 +- feat(sentry): embed Sentry DSN at build time for packaged apps by @Andy in aed28c5f +- fix(github): resolve circular import issues in context_gatherer and services by @Andy in 0307a4a9 +- fix(github-prs): prevent preloading of PRs currently under review by @Andy in 1babcc86 +- fix(ui): display actual base branch name instead of hardcoded main by @Andy in 5d07d5f1 +- ci(release): move VirusTotal scan to separate post-release workflow by @Andy in 553d1e8d +- fix: improve Claude CLI detection and add installation selector by @Andy in e07a0dbd +- fix(backend): add Sentry integration and fix broken pipe errors by @Andy in aa9fbe9d +- fix(app-update): persist downloaded update state for Install button visibility by @Andy in 6f059bb5 +- fix(terminal): detect Claude exit and reset label when user closes Claude by @Andy in 14982e66 +- fix(merge): include files with content changes even when semantic analysis is empty by @Andy in 4736b6b6 +- fix(frontend): sync worktree config to renderer on terminal restoration by @Andy in 68fe0860 +- feat(frontend): add searchable branch combobox to worktree creation dialog by @Andy in 2a2dc3b8 +- fix(security): inherit security profiles in worktrees and validate shell -c commands by @Andy in 750ea8d1 +- feat(frontend): add Claude Code version rollback feature by @Andy in 8d21978f +- fix(ACS-181): enable auto-switch on 401 auth errors & OAuth-only profiles by @Michael Ludlow in e7427321 +- fix(terminal): add collision detection for terminal drag and drop reordering by @Andy in 1701160b +- fix(worktree): handle "already up to date" case correctly by @StillKnotKnown in 74ed4320 + +## Thanks to all contributors + +@Umaru, @StillKnotKnown, @Andy, @Michael Ludlow, @AndyMik90 + ## 2.7.3 - Reliability & Stability Focus ### ✨ New Features diff --git a/README.md b/README.md index 6592bac309..e4004eec19 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ ### Stable Release -[![Stable](https://img.shields.io/badge/stable-2.7.3-blue?style=flat-square)](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.3) +[![Stable](https://img.shields.io/badge/stable-2.7.4-blue?style=flat-square)](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.4) diff --git a/apps/backend/__init__.py b/apps/backend/__init__.py index aeb22c7403..6c55946b8a 100644 --- a/apps/backend/__init__.py +++ b/apps/backend/__init__.py @@ -19,5 +19,5 @@ See README.md for full documentation. """ -__version__ = "2.7.3" +__version__ = "2.7.4" __author__ = "Auto Claude Team" diff --git a/apps/backend/cli/main.py b/apps/backend/cli/main.py index cfb6a6a414..0b4db55cec 100644 --- a/apps/backend/cli/main.py +++ b/apps/backend/cli/main.py @@ -288,6 +288,28 @@ def main() -> None: # Set up environment first setup_environment() + # Initialize Sentry early to capture any startup errors + from core.sentry import capture_exception, init_sentry + + init_sentry(component="cli") + + try: + _run_cli() + except KeyboardInterrupt: + # Clean exit on Ctrl+C + sys.exit(130) + except Exception as e: + # Capture unexpected errors to Sentry + capture_exception(e) + print(f"\nUnexpected error: {e}") + sys.exit(1) + + +def _run_cli() -> None: + """Run the CLI logic (extracted for error handling).""" + # Import here to avoid import errors during startup + from core.sentry import set_context + # Parse arguments args = parse_args() @@ -358,6 +380,15 @@ def main() -> None: debug_success("run.py", "Spec found", spec_dir=str(spec_dir)) + # Set Sentry context for error tracking + set_context( + "spec", + { + "name": spec_dir.name, + "project": str(project_dir), + }, + ) + # Handle build management commands if args.merge_preview: from cli.workspace_commands import handle_merge_preview_command diff --git a/apps/backend/core/client.py b/apps/backend/core/client.py index 69c9c0e239..91f384cf80 100644 --- a/apps/backend/core/client.py +++ b/apps/backend/core/client.py @@ -17,6 +17,8 @@ import logging import os import platform +import shutil +import subprocess import threading import time from pathlib import Path @@ -121,6 +123,290 @@ def invalidate_project_cache(project_dir: Path | None = None) -> None: logger.debug(f"Invalidated project index cache for {project_dir}") +# ============================================================================= +# Claude CLI Path Detection +# ============================================================================= +# Cross-platform detection of Claude Code CLI binary. +# This mirrors the frontend's cli-tool-manager.ts logic to ensure consistency. + +_CLAUDE_CLI_CACHE: dict[str, str | None] = {} +_CLI_CACHE_LOCK = threading.Lock() + + +def _get_claude_detection_paths() -> dict[str, list[str]]: + """ + Get all candidate paths for Claude CLI detection. + + Returns platform-specific paths where Claude CLI might be installed. + + IMPORTANT: This function mirrors the frontend's getClaudeDetectionPaths() + in apps/frontend/src/main/cli-tool-manager.ts. Both implementations MUST + be kept in sync to ensure consistent detection behavior across the + Python backend and Electron frontend. + + When adding new detection paths, update BOTH: + 1. This function (_get_claude_detection_paths in client.py) + 2. getClaudeDetectionPaths() in cli-tool-manager.ts + + Returns: + Dict with 'homebrew', 'platform', and 'nvm' path lists + """ + home_dir = Path.home() + is_windows = platform.system() == "Windows" + + homebrew_paths = [ + "/opt/homebrew/bin/claude", # Apple Silicon + "/usr/local/bin/claude", # Intel Mac + ] + + if is_windows: + platform_paths = [ + str(home_dir / "AppData" / "Local" / "Programs" / "claude" / "claude.exe"), + str(home_dir / "AppData" / "Roaming" / "npm" / "claude.cmd"), + str(home_dir / ".local" / "bin" / "claude.exe"), + "C:\\Program Files\\Claude\\claude.exe", + "C:\\Program Files (x86)\\Claude\\claude.exe", + ] + else: + platform_paths = [ + str(home_dir / ".local" / "bin" / "claude"), + str(home_dir / "bin" / "claude"), + ] + + nvm_versions_dir = str(home_dir / ".nvm" / "versions" / "node") + + return { + "homebrew": homebrew_paths, + "platform": platform_paths, + "nvm_versions_dir": nvm_versions_dir, + } + + +def _is_secure_path(path_str: str) -> bool: + """ + Validate that a path doesn't contain dangerous characters. + + Prevents command injection attacks by rejecting paths with shell metacharacters, + directory traversal patterns, or environment variable expansion. + + Args: + path_str: Path to validate + + Returns: + True if the path is safe, False otherwise + """ + import re + + dangerous_patterns = [ + r'[;&|`${}[\]<>!"^]', # Shell metacharacters + r"%[^%]+%", # Windows environment variable expansion + r"\.\./", # Unix directory traversal + r"\.\.\\", # Windows directory traversal + r"[\r\n]", # Newlines (command injection) + ] + + for pattern in dangerous_patterns: + if re.search(pattern, path_str): + return False + + return True + + +def _validate_claude_cli(cli_path: str) -> tuple[bool, str | None]: + """ + Validate that a Claude CLI path is executable and returns a version. + + Includes security validation to prevent command injection attacks. + + Args: + cli_path: Path to the Claude CLI executable + + Returns: + Tuple of (is_valid, version_string or None) + + Note: + Cross-references with frontend's validateClaudeCliAsync() in + apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts + Both should be kept in sync for consistent behavior. + """ + import re + + # Security validation: reject paths with shell metacharacters or directory traversal + if not _is_secure_path(cli_path): + logger.warning(f"Rejecting insecure Claude CLI path: {cli_path}") + return False, None + + try: + is_windows = platform.system() == "Windows" + + # Augment PATH with the CLI directory for proper resolution + env = os.environ.copy() + cli_dir = os.path.dirname(cli_path) + if cli_dir: + env["PATH"] = cli_dir + os.pathsep + env.get("PATH", "") + + # For Windows .cmd/.bat files, use cmd.exe with proper quoting + # /d = disable AutoRun registry commands + # /s = strip first and last quotes, preserving inner quotes + # /c = run command then terminate + if is_windows and cli_path.lower().endswith((".cmd", ".bat")): + # Get cmd.exe path from environment or use default + cmd_exe = os.environ.get("ComSpec") or os.path.join( + os.environ.get("SystemRoot", "C:\\Windows"), "System32", "cmd.exe" + ) + # Use double-quoted command line for paths with spaces + cmd_line = f'""{cli_path}" --version"' + result = subprocess.run( + [cmd_exe, "/d", "/s", "/c", cmd_line], + capture_output=True, + text=True, + timeout=5, + env=env, + creationflags=subprocess.CREATE_NO_WINDOW, + ) + else: + result = subprocess.run( + [cli_path, "--version"], + capture_output=True, + text=True, + timeout=5, + env=env, + creationflags=subprocess.CREATE_NO_WINDOW if is_windows else 0, + ) + + if result.returncode == 0: + # Extract version from output (e.g., "claude-code version 1.0.0") + output = result.stdout.strip() + match = re.search(r"(\d+\.\d+\.\d+)", output) + version = match.group(1) if match else output.split("\n")[0] + return True, version + + return False, None + except (subprocess.TimeoutExpired, FileNotFoundError, OSError) as e: + logger.debug(f"Claude CLI validation failed for {cli_path}: {e}") + return False, None + + +def find_claude_cli() -> str | None: + """ + Find the Claude Code CLI binary path. + + Uses cross-platform detection with the following priority: + 1. CLAUDE_CLI_PATH environment variable (user override) + 2. shutil.which() - system PATH lookup + 3. Homebrew paths (macOS) + 4. NVM paths (Unix - checks Node.js version manager) + 5. Platform-specific standard locations + + Returns: + Path to Claude CLI if found and valid, None otherwise + """ + # Check cache first + cache_key = "claude_cli" + with _CLI_CACHE_LOCK: + if cache_key in _CLAUDE_CLI_CACHE: + cached = _CLAUDE_CLI_CACHE[cache_key] + logger.debug(f"Using cached Claude CLI path: {cached}") + return cached + + is_windows = platform.system() == "Windows" + paths = _get_claude_detection_paths() + + # 1. Check environment variable override + env_path = os.environ.get("CLAUDE_CLI_PATH") + if env_path: + if Path(env_path).exists(): + valid, version = _validate_claude_cli(env_path) + if valid: + logger.info(f"Using CLAUDE_CLI_PATH: {env_path} (v{version})") + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = env_path + return env_path + logger.warning(f"CLAUDE_CLI_PATH is set but invalid: {env_path}") + + # 2. Try shutil.which() - most reliable cross-platform PATH lookup + which_path = shutil.which("claude") + if which_path: + valid, version = _validate_claude_cli(which_path) + if valid: + logger.info(f"Found Claude CLI in PATH: {which_path} (v{version})") + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = which_path + return which_path + + # 3. Homebrew paths (macOS) + if platform.system() == "Darwin": + for hb_path in paths["homebrew"]: + if Path(hb_path).exists(): + valid, version = _validate_claude_cli(hb_path) + if valid: + logger.info(f"Found Claude CLI (Homebrew): {hb_path} (v{version})") + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = hb_path + return hb_path + + # 4. NVM paths (Unix only) - check Node.js version manager installations + if not is_windows: + nvm_dir = Path(paths["nvm_versions_dir"]) + if nvm_dir.exists(): + try: + # Get all version directories and sort by version (newest first) + version_dirs = [] + for entry in nvm_dir.iterdir(): + if entry.is_dir() and entry.name.startswith("v"): + # Parse version: v20.0.0 -> (20, 0, 0) + try: + parts = entry.name[1:].split(".") + if len(parts) == 3: + version_dirs.append( + (tuple(int(p) for p in parts), entry.name) + ) + except ValueError: + continue + + # Sort by version descending (newest first) + version_dirs.sort(reverse=True) + + for _, version_name in version_dirs: + nvm_claude = nvm_dir / version_name / "bin" / "claude" + if nvm_claude.exists(): + valid, version = _validate_claude_cli(str(nvm_claude)) + if valid: + logger.info( + f"Found Claude CLI (NVM): {nvm_claude} (v{version})" + ) + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = str(nvm_claude) + return str(nvm_claude) + except OSError as e: + logger.debug(f"Error scanning NVM directory: {e}") + + # 5. Platform-specific standard locations + for plat_path in paths["platform"]: + if Path(plat_path).exists(): + valid, version = _validate_claude_cli(plat_path) + if valid: + logger.info(f"Found Claude CLI: {plat_path} (v{version})") + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = plat_path + return plat_path + + # Not found + logger.warning( + "Claude CLI not found. Install with: npm install -g @anthropic-ai/claude-code" + ) + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE[cache_key] = None + return None + + +def clear_claude_cli_cache() -> None: + """Clear the Claude CLI path cache, forcing re-detection on next call.""" + with _CLI_CACHE_LOCK: + _CLAUDE_CLI_CACHE.clear() + logger.debug("Claude CLI cache cleared") + + from agents.tools_pkg import ( CONTEXT7_TOOLS, ELECTRON_TOOLS, @@ -780,8 +1066,16 @@ def create_client( print(" - CLAUDE.md: disabled by project settings") print() + # Find Claude CLI path for SDK + # This ensures the SDK can find the Claude Code binary even if it's not in PATH + cli_path = find_claude_cli() + if cli_path: + print(f" - Claude CLI: {cli_path}") + else: + print(" - Claude CLI: using SDK default detection") + # Build options dict, conditionally including output_format - options_kwargs = { + options_kwargs: dict[str, Any] = { "model": model, "system_prompt": base_prompt, "allowed_tools": allowed_tools_list, @@ -804,6 +1098,10 @@ def create_client( "enable_file_checkpointing": True, } + # Add CLI path if found (helps SDK find Claude Code in non-standard locations) + if cli_path: + options_kwargs["cli_path"] = cli_path + # Add structured output format if specified # See: https://platform.claude.com/docs/en/agent-sdk/structured-outputs if output_format: diff --git a/apps/backend/core/io_utils.py b/apps/backend/core/io_utils.py new file mode 100644 index 0000000000..c5a8a15549 --- /dev/null +++ b/apps/backend/core/io_utils.py @@ -0,0 +1,94 @@ +""" +I/O Utilities for Safe Console Output +===================================== + +Safe I/O operations for processes running as subprocesses. + +When the backend runs as a subprocess of the Electron app, the parent +process may close the pipe at any time (e.g., user closes the app, +process killed, etc.). This module provides utilities to handle these +cases gracefully. +""" + +from __future__ import annotations + +import logging +import sys + +logger = logging.getLogger(__name__) + +# Track if pipe is broken to avoid repeated failed writes +_pipe_broken = False + + +def safe_print(message: str, flush: bool = True) -> None: + """ + Print to stdout with BrokenPipeError handling. + + When running as a subprocess (e.g., from Electron), the parent process + may close the pipe at any time. This function gracefully handles that + case instead of raising an exception. + + Args: + message: The message to print + flush: Whether to flush stdout after printing (default True) + """ + global _pipe_broken + + # Skip if we already know the pipe is broken + if _pipe_broken: + return + + try: + print(message, flush=flush) + except BrokenPipeError: + # Pipe closed by parent process - this is expected during shutdown + _pipe_broken = True + # Quietly close stdout to prevent further errors + try: + sys.stdout.close() + except Exception: + pass + logger.debug("Output pipe closed by parent process") + except ValueError as e: + # Handle writes to closed file (can happen after stdout.close()) + if "closed file" in str(e).lower(): + _pipe_broken = True + logger.debug("Output stream closed") + else: + # Re-raise unexpected ValueErrors + raise + except OSError as e: + # Handle other pipe-related errors (EPIPE, etc.) + if e.errno == 32: # EPIPE - Broken pipe + _pipe_broken = True + try: + sys.stdout.close() + except Exception: + pass + logger.debug("Output pipe closed (EPIPE)") + else: + # Re-raise unexpected OS errors + raise + + +def is_pipe_broken() -> bool: + """Check if the output pipe has been closed.""" + return _pipe_broken + + +def reset_pipe_state() -> None: + """ + Reset pipe broken state. + + Useful for testing or when starting a new subprocess context where + stdout has been reopened. Should only be called when stdout is known + to be functional (e.g., in a fresh subprocess with a new stdout). + + Warning: + Calling this after stdout has been closed will result in safe_print() + attempting to write to the closed stream. The ValueError will be + caught and the pipe will be marked as broken again. + """ + global _pipe_broken + _pipe_broken = False diff --git a/apps/backend/core/sentry.py b/apps/backend/core/sentry.py new file mode 100644 index 0000000000..0634f4ad99 --- /dev/null +++ b/apps/backend/core/sentry.py @@ -0,0 +1,406 @@ +""" +Sentry Error Tracking for Python Backend +========================================= + +Initializes Sentry for the Python backend with: +- Privacy-preserving path masking (usernames removed) +- Release tracking matching the Electron frontend +- Environment variable configuration (same as frontend) + +Configuration: +- SENTRY_DSN: Required to enable Sentry (same as frontend) +- SENTRY_TRACES_SAMPLE_RATE: Performance monitoring sample rate (0-1, default: 0.1) +- SENTRY_ENVIRONMENT: Override environment (default: auto-detected) + +Privacy Note: +- Usernames are masked from all file paths +- Project paths remain visible for debugging (this is expected) +- No user identifiers are collected +""" + +from __future__ import annotations + +import logging +import os +import re +import sys +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +# Track initialization state +_sentry_initialized = False +_sentry_enabled = False + +# Production trace sample rate (10%) +PRODUCTION_TRACE_SAMPLE_RATE = 0.1 + + +def _get_version() -> str: + """ + Get the application version. + + Tries to read from package.json in the frontend directory, + falling back to a default version. + """ + try: + # Try to find package.json relative to this file + backend_dir = Path(__file__).parent.parent + frontend_dir = backend_dir.parent / "frontend" + package_json = frontend_dir / "package.json" + + if package_json.exists(): + import json + + with open(package_json) as f: + data = json.load(f) + return data.get("version", "0.0.0") + except Exception as e: + logger.debug(f"Version detection failed: {e}") + + return "0.0.0" + + +def _mask_user_paths(text: str) -> str: + """ + Mask user-specific paths for privacy. + + Replaces usernames in common OS path patterns: + - macOS: /Users/username/... becomes /Users/***/... + - Windows: C:\\Users\\username\\... becomes C:\\Users\\***\\... + - Linux: /home/username/... becomes /home/***/... + - WSL: /mnt/c/Users/username/... becomes /mnt/c/Users/***/... + + Note: Project paths remain visible for debugging purposes. + """ + if not text: + return text + + # macOS: /Users/username/... + text = re.sub(r"/Users/[^/]+(?=/|$)", "/Users/***", text) + + # Windows: C:\Users\username\... + text = re.sub( + r"[A-Za-z]:\\Users\\[^\\]+(?=\\|$)", + lambda m: f"{m.group(0)[0]}:\\Users\\***", + text, + ) + + # Linux: /home/username/... + text = re.sub(r"/home/[^/]+(?=/|$)", "/home/***", text) + + # WSL: /mnt/c/Users/username/... (accessing Windows filesystem from WSL) + text = re.sub( + r"/mnt/[a-z]/Users/[^/]+(?=/|$)", + lambda m: f"{m.group(0)[:6]}/Users/***", + text, + ) + + return text + + +def _mask_object_paths(obj: Any, _depth: int = 0) -> Any: + """ + Recursively mask paths in an object. + + Args: + obj: The object to mask paths in + _depth: Current recursion depth (internal use) + + Returns: + Object with paths masked + """ + # Prevent stack overflow on deeply nested or circular structures + if _depth > 50: + return obj + + if obj is None: + return obj + + if isinstance(obj, str): + return _mask_user_paths(obj) + + if isinstance(obj, list): + return [_mask_object_paths(item, _depth + 1) for item in obj] + + if isinstance(obj, dict): + return { + key: _mask_object_paths(value, _depth + 1) for key, value in obj.items() + } + + return obj + + +def _before_send(event: dict, hint: dict) -> dict | None: + """ + Process event before sending to Sentry. + + Applies privacy masking to all paths in the event. + """ + if not _sentry_enabled: + return None + + # Mask paths in exception stack traces + if "exception" in event and "values" in event["exception"]: + for exception in event["exception"]["values"]: + if "stacktrace" in exception and "frames" in exception["stacktrace"]: + for frame in exception["stacktrace"]["frames"]: + if "filename" in frame: + frame["filename"] = _mask_user_paths(frame["filename"]) + if "abs_path" in frame: + frame["abs_path"] = _mask_user_paths(frame["abs_path"]) + if "value" in exception: + exception["value"] = _mask_user_paths(exception["value"]) + + # Mask paths in breadcrumbs + if "breadcrumbs" in event: + for breadcrumb in event.get("breadcrumbs", {}).get("values", []): + if "message" in breadcrumb: + breadcrumb["message"] = _mask_user_paths(breadcrumb["message"]) + if "data" in breadcrumb: + breadcrumb["data"] = _mask_object_paths(breadcrumb["data"]) + + # Mask paths in message + if "message" in event: + event["message"] = _mask_user_paths(event["message"]) + + # Mask paths in tags + if "tags" in event: + event["tags"] = _mask_object_paths(event["tags"]) + + # Mask paths in contexts + if "contexts" in event: + event["contexts"] = _mask_object_paths(event["contexts"]) + + # Mask paths in extra data + if "extra" in event: + event["extra"] = _mask_object_paths(event["extra"]) + + # Clear user info for privacy + if "user" in event: + event["user"] = {} + + return event + + +def init_sentry( + component: str = "backend", + force_enable: bool = False, +) -> bool: + """ + Initialize Sentry for the Python backend. + + Args: + component: Component name for tagging (e.g., "backend", "github-runner") + force_enable: Force enable even without packaged app detection + + Returns: + True if Sentry was initialized, False otherwise + """ + global _sentry_initialized, _sentry_enabled + + if _sentry_initialized: + return _sentry_enabled + + _sentry_initialized = True + + # Get DSN from environment variable + dsn = os.environ.get("SENTRY_DSN", "") + + if not dsn: + logger.debug("[Sentry] No SENTRY_DSN configured - error reporting disabled") + return False + + # Check if we should enable Sentry + # Enable if: + # - Running from packaged app (detected by __compiled__ or frozen) + # - SENTRY_DEV=true is set + # - force_enable is True + is_packaged = getattr(sys, "frozen", False) or hasattr(sys, "__compiled__") + sentry_dev = os.environ.get("SENTRY_DEV", "").lower() in ("true", "1", "yes") + should_enable = is_packaged or sentry_dev or force_enable + + if not should_enable: + logger.debug( + "[Sentry] Development mode - error reporting disabled (set SENTRY_DEV=true to enable)" + ) + return False + + try: + import sentry_sdk + from sentry_sdk.integrations.logging import LoggingIntegration + except ImportError: + logger.warning("[Sentry] sentry-sdk not installed - error reporting disabled") + return False + + # Get configuration from environment variables + version = _get_version() + environment = os.environ.get( + "SENTRY_ENVIRONMENT", "production" if is_packaged else "development" + ) + + # Get sample rates + traces_sample_rate = PRODUCTION_TRACE_SAMPLE_RATE + try: + env_rate = os.environ.get("SENTRY_TRACES_SAMPLE_RATE") + if env_rate: + parsed = float(env_rate) + if 0 <= parsed <= 1: + traces_sample_rate = parsed + except (ValueError, TypeError): + pass + + # Configure logging integration to capture errors and warnings + logging_integration = LoggingIntegration( + level=logging.INFO, # Capture INFO and above as breadcrumbs + event_level=logging.ERROR, # Send ERROR and above as events + ) + + # Initialize Sentry + sentry_sdk.init( + dsn=dsn, + environment=environment, + release=f"auto-claude@{version}", + traces_sample_rate=traces_sample_rate, + before_send=_before_send, + integrations=[logging_integration], + # Don't send PII + send_default_pii=False, + ) + + # Set component tag + sentry_sdk.set_tag("component", component) + + _sentry_enabled = True + logger.info( + f"[Sentry] Backend initialized (component: {component}, release: auto-claude@{version}, traces: {traces_sample_rate})" + ) + + return True + + +def capture_exception(error: Exception, **kwargs) -> None: + """ + Capture an exception and send to Sentry. + + Safe to call even if Sentry is not initialized. + + Args: + error: The exception to capture + **kwargs: Additional context to attach to the event + """ + if not _sentry_enabled: + logger.error(f"[Sentry] Not enabled, exception not captured: {error}") + return + + try: + import sentry_sdk + + with sentry_sdk.push_scope() as scope: + for key, value in kwargs.items(): + # Apply defensive path masking for extra data + masked_value = ( + _mask_object_paths(value) + if isinstance(value, (str, dict, list)) + else value + ) + scope.set_extra(key, masked_value) + sentry_sdk.capture_exception(error) + except ImportError: + logger.error(f"[Sentry] SDK not installed, exception not captured: {error}") + except Exception as e: + logger.error(f"[Sentry] Failed to capture exception: {e}") + + +def capture_message(message: str, level: str = "info", **kwargs) -> None: + """ + Capture a message and send to Sentry. + + Safe to call even if Sentry is not initialized. + + Args: + message: The message to capture + level: Log level (debug, info, warning, error, fatal) + **kwargs: Additional context to attach to the event + """ + if not _sentry_enabled: + return + + try: + import sentry_sdk + + with sentry_sdk.push_scope() as scope: + for key, value in kwargs.items(): + # Apply defensive path masking for extra data (same as capture_exception) + masked_value = ( + _mask_object_paths(value) + if isinstance(value, (str, dict, list)) + else value + ) + scope.set_extra(key, masked_value) + sentry_sdk.capture_message(message, level=level) + except ImportError: + logger.debug("[Sentry] SDK not installed") + except Exception as e: + logger.error(f"[Sentry] Failed to capture message: {e}") + + +def set_context(name: str, data: dict) -> None: + """ + Set context data for subsequent events. + + Safe to call even if Sentry is not initialized. + + Args: + name: Context name (e.g., "pr_review", "spec") + data: Context data dictionary + """ + if not _sentry_enabled: + return + + try: + import sentry_sdk + + # Apply path masking to context data before sending to Sentry + masked_data = _mask_object_paths(data) + sentry_sdk.set_context(name, masked_data) + except ImportError: + logger.debug("[Sentry] SDK not installed") + except Exception as e: + logger.debug(f"Failed to set context '{name}': {e}") + + +def set_tag(key: str, value: str) -> None: + """ + Set a tag for subsequent events. + + Safe to call even if Sentry is not initialized. + + Args: + key: Tag key + value: Tag value + """ + if not _sentry_enabled: + return + + try: + import sentry_sdk + + # Apply path masking to tag value + masked_value = _mask_user_paths(value) if isinstance(value, str) else value + sentry_sdk.set_tag(key, masked_value) + except ImportError: + logger.debug("[Sentry] SDK not installed") + except Exception as e: + logger.debug(f"Failed to set tag '{key}': {e}") + + +def is_enabled() -> bool: + """Check if Sentry is enabled.""" + return _sentry_enabled + + +def is_initialized() -> bool: + """Check if Sentry initialization has been attempted.""" + return _sentry_initialized diff --git a/apps/backend/core/simple_client.py b/apps/backend/core/simple_client.py index 9d910aadbd..4245ba1a2d 100644 --- a/apps/backend/core/simple_client.py +++ b/apps/backend/core/simple_client.py @@ -26,6 +26,7 @@ from agents.tools_pkg import get_agent_config, get_default_thinking_level from claude_agent_sdk import ClaudeAgentOptions, ClaudeSDKClient from core.auth import get_sdk_env_vars, require_auth_token +from core.client import find_claude_cli from phase_config import get_thinking_budget @@ -84,14 +85,22 @@ def create_simple_client( thinking_level = get_default_thinking_level(agent_type) max_thinking_tokens = get_thinking_budget(thinking_level) - return ClaudeSDKClient( - options=ClaudeAgentOptions( - model=model, - system_prompt=system_prompt, - allowed_tools=allowed_tools, - max_turns=max_turns, - cwd=str(cwd.resolve()) if cwd else None, - env=sdk_env, - max_thinking_tokens=max_thinking_tokens, - ) - ) + # Find Claude CLI path (handles non-standard installations) + cli_path = find_claude_cli() + + # Build options dict + options_kwargs = { + "model": model, + "system_prompt": system_prompt, + "allowed_tools": allowed_tools, + "max_turns": max_turns, + "cwd": str(cwd.resolve()) if cwd else None, + "env": sdk_env, + "max_thinking_tokens": max_thinking_tokens, + } + + # Add CLI path if found + if cli_path: + options_kwargs["cli_path"] = cli_path + + return ClaudeSDKClient(options=ClaudeAgentOptions(**options_kwargs)) diff --git a/apps/backend/core/workspace/setup.py b/apps/backend/core/workspace/setup.py index 06269e7c1e..adf31d3155 100644 --- a/apps/backend/core/workspace/setup.py +++ b/apps/backend/core/workspace/setup.py @@ -296,6 +296,23 @@ def setup_workspace( f"Security config copied: {', '.join(security_files_copied)}", "success" ) + # Mark the security profile as inherited from parent project + # This prevents hash-based re-analysis which would produce a broken profile + # (worktrees lack node_modules and other build artifacts needed for detection) + if PROFILE_FILENAME in security_files_copied: + profile_path = worktree_info.path / PROFILE_FILENAME + try: + with open(profile_path) as f: + profile_data = json.load(f) + profile_data["inherited_from"] = str(project_dir.resolve()) + with open(profile_path, "w") as f: + json.dump(profile_data, f, indent=2) + debug( + MODULE, f"Marked security profile as inherited from {project_dir}" + ) + except (OSError, json.JSONDecodeError) as e: + debug_warning(MODULE, f"Failed to mark profile as inherited: {e}") + # Ensure .auto-claude/ is in the worktree's .gitignore # This is critical because the worktree inherits .gitignore from the base branch, # which may not have .auto-claude/ if that change wasn't committed/pushed. diff --git a/apps/backend/core/worktree.py b/apps/backend/core/worktree.py index a5fdc6d57e..a94964ef7f 100644 --- a/apps/backend/core/worktree.py +++ b/apps/backend/core/worktree.py @@ -647,7 +647,29 @@ def merge_worktree( result = self._run_git(merge_args) if result.returncode != 0: - print("Merge conflict! Aborting merge...") + # Check if it's "already up to date" - not an error + output = (result.stdout + result.stderr).lower() + if "already up to date" in output or "already up-to-date" in output: + print(f"Branch {info.branch} is already up to date.") + if no_commit: + print("No changes to stage.") + if delete_after: + self.remove_worktree(spec_name, delete_branch=True) + return True + # Check for actual conflicts + if "conflict" in output: + print("Merge conflict! Aborting merge...") + self._run_git(["merge", "--abort"]) + return False + # Other error - show details + stderr_msg = ( + result.stderr[:200] + if result.stderr + else result.stdout[:200] + if result.stdout + else "" + ) + print(f"Merge failed: {stderr_msg}") self._run_git(["merge", "--abort"]) return False diff --git a/apps/backend/merge/file_evolution/evolution_queries.py b/apps/backend/merge/file_evolution/evolution_queries.py index 22c509e51b..b8f23be59c 100644 --- a/apps/backend/merge/file_evolution/evolution_queries.py +++ b/apps/backend/merge/file_evolution/evolution_queries.py @@ -102,7 +102,7 @@ def get_task_modifications( modifications = [] for file_path, evolution in evolutions.items(): snapshot = evolution.get_task_snapshot(task_id) - if snapshot and snapshot.semantic_changes: + if snapshot and snapshot.has_modifications: modifications.append((file_path, snapshot)) return modifications @@ -125,7 +125,7 @@ def get_files_modified_by_tasks( for file_path, evolution in evolutions.items(): for snapshot in evolution.task_snapshots: - if snapshot.task_id in task_ids and snapshot.semantic_changes: + if snapshot.task_id in task_ids and snapshot.has_modifications: if file_path not in file_tasks: file_tasks[file_path] = [] file_tasks[file_path].append(snapshot.task_id) diff --git a/apps/backend/merge/merge_pipeline.py b/apps/backend/merge/merge_pipeline.py index 3f64b55f28..8a2cacaa7c 100644 --- a/apps/backend/merge/merge_pipeline.py +++ b/apps/backend/merge/merge_pipeline.py @@ -75,6 +75,18 @@ def merge_file( # If only one task modified the file, no conflict possible if len(task_snapshots) == 1: snapshot = task_snapshots[0] + + # Check if file has modifications but semantic analysis returned empty + # This happens for: function body changes, unsupported file types (Rust, Go, etc.) + # In this case, signal that the caller should use the worktree version directly + if snapshot.has_modifications and not snapshot.semantic_changes: + return MergeResult( + decision=MergeDecision.DIRECT_COPY, + file_path=file_path, + merged_content=None, # Caller must read from worktree + explanation=f"File modified by {snapshot.task_id} but no semantic changes detected - use worktree version", + ) + merged = apply_single_task_changes(baseline_content, snapshot, file_path) return MergeResult( decision=MergeDecision.AUTO_MERGED, diff --git a/apps/backend/merge/orchestrator.py b/apps/backend/merge/orchestrator.py index d02fee22c1..bc48131330 100644 --- a/apps/backend/merge/orchestrator.py +++ b/apps/backend/merge/orchestrator.py @@ -203,6 +203,58 @@ def merge_pipeline(self) -> MergePipeline: ) return self._merge_pipeline + def _read_worktree_file_for_direct_copy( + self, + file_path: str, + worktree_path: Path | None, + ) -> tuple[str | None, bool]: + """ + Read file content from worktree for DIRECT_COPY merge. + + Args: + file_path: Relative path to the file + worktree_path: Path to the worktree directory + + Returns: + Tuple of (content, success). If success is False, content is None + and the caller should mark the merge as FAILED. + """ + if not worktree_path: + logger.warning( + f"DIRECT_COPY: No worktree path provided for file: {file_path}" + ) + debug_warning( + MODULE, + "DIRECT_COPY: No worktree path provided", + file=file_path, + ) + return None, False + + worktree_file = worktree_path / file_path + if not worktree_file.exists(): + logger.warning(f"DIRECT_COPY: Worktree file not found: {worktree_file}") + debug_warning( + MODULE, + "DIRECT_COPY: Worktree file not found", + file=str(worktree_file), + ) + return None, False + + try: + content = worktree_file.read_text(encoding="utf-8") + debug_detailed( + MODULE, + f"Read file from worktree for direct copy: {file_path}", + ) + return content, True + except UnicodeDecodeError: + content = worktree_file.read_text(encoding="utf-8", errors="replace") + debug_detailed( + MODULE, + f"Read file from worktree with encoding fallback: {file_path}", + ) + return content, True + def merge_task( self, task_id: str, @@ -275,6 +327,20 @@ def merge_task( task_snapshots=[snapshot], target_branch=target_branch, ) + + # Handle DIRECT_COPY: read file directly from worktree + # This happens when file has modifications but semantic analysis + # couldn't parse the changes (body modifications, unsupported languages) + if result.decision == MergeDecision.DIRECT_COPY: + content, success = self._read_worktree_file_for_direct_copy( + file_path, worktree_path + ) + if success: + result.merged_content = content + else: + result.decision = MergeDecision.FAILED + result.error = "Worktree file not found for DIRECT_COPY" + report.file_results[file_path] = result self._update_stats(report.stats, result) debug_verbose( @@ -374,12 +440,41 @@ def merge_tasks( task_snapshots=snapshots, target_branch=target_branch, ) + + # Handle DIRECT_COPY: read file directly from worktree + # For multi-task merges, use the first task's worktree that modified this file + if result.decision == MergeDecision.DIRECT_COPY: + # Find the worktree path from the first task that modified this file + worktree_path = None + for tid in modifying_tasks: + for req in requests: + if req.task_id == tid and req.worktree_path: + worktree_path = req.worktree_path + break + if worktree_path: + break + + content, success = self._read_worktree_file_for_direct_copy( + file_path, worktree_path + ) + if success: + result.merged_content = content + else: + result.decision = MergeDecision.FAILED + result.error = "Worktree file not found for DIRECT_COPY" + report.file_results[file_path] = result self._update_stats(report.stats, result) report.success = report.stats.files_failed == 0 except Exception as e: + debug_error( + MODULE, + "Merge failed for tasks", + task_ids=[r.task_id for r in requests], + error=str(e), + ) logger.exception("Merge failed") report.success = False report.error = str(e) @@ -589,7 +684,7 @@ def write_merged_files( written = [] for file_path, result in report.file_results.items(): - if result.merged_content: + if result.merged_content is not None: out_path = output_dir / file_path out_path.parent.mkdir(parents=True, exist_ok=True) out_path.write_text(result.merged_content, encoding="utf-8") @@ -640,7 +735,7 @@ def _update_stats(self, stats: MergeStats, result) -> None: ) stats.conflicts_auto_resolved += len(result.conflicts_resolved) - if result.decision == MergeDecision.AUTO_MERGED: + if result.decision in (MergeDecision.AUTO_MERGED, MergeDecision.DIRECT_COPY): stats.files_auto_merged += 1 elif result.decision == MergeDecision.AI_MERGED: stats.files_ai_merged += 1 diff --git a/apps/backend/merge/types.py b/apps/backend/merge/types.py index 9e11832f94..d1ceafb745 100644 --- a/apps/backend/merge/types.py +++ b/apps/backend/merge/types.py @@ -133,6 +133,7 @@ class MergeDecision(Enum): AI_MERGED = "ai_merged" # AI resolved the conflict NEEDS_HUMAN_REVIEW = "needs_human_review" # Flagged for human FAILED = "failed" # Could not merge + DIRECT_COPY = "direct_copy" # Use worktree version directly (no semantic merge) @dataclass @@ -414,6 +415,34 @@ def from_dict(cls, data: dict[str, Any]) -> TaskSnapshot: raw_diff=data.get("raw_diff"), ) + @property + def has_modifications(self) -> bool: + """ + Check if this snapshot represents actual file modifications. + + Returns True if the file was modified, using content hash comparison + as the source of truth. This handles cases where the semantic analyzer + couldn't detect changes (e.g., function body modifications, unsupported + file types like Rust) but the file was actually changed. + + Also returns True for newly created files (where content_hash_before + is empty but content_hash_after is set). + """ + # If we have semantic changes, the file was definitely modified + if self.semantic_changes: + return True + + # Handle new files: if before is empty but after has content, it's a new file + if not self.content_hash_before and self.content_hash_after: + return True + + # Fall back to content hash comparison for files where semantic + # analysis returned empty (body modifications, unsupported languages) + if self.content_hash_before and self.content_hash_after: + return self.content_hash_before != self.content_hash_after + + return False + @dataclass class FileEvolution: @@ -534,7 +563,11 @@ def to_dict(self) -> dict[str, Any]: @property def success(self) -> bool: """Check if merge was successful.""" - return self.decision in {MergeDecision.AUTO_MERGED, MergeDecision.AI_MERGED} + return self.decision in { + MergeDecision.AUTO_MERGED, + MergeDecision.AI_MERGED, + MergeDecision.DIRECT_COPY, + } @property def needs_human_review(self) -> bool: diff --git a/apps/backend/project/analyzer.py b/apps/backend/project/analyzer.py index 67570add28..6054f8f500 100644 --- a/apps/backend/project/analyzer.py +++ b/apps/backend/project/analyzer.py @@ -188,10 +188,38 @@ def compute_project_hash(self) -> str: return hasher.hexdigest() def should_reanalyze(self, profile: SecurityProfile) -> bool: - """Check if project has changed since last analysis.""" + """Check if project has changed since last analysis. + + Never re-analyzes inherited profiles (from worktrees) since they + came from a validated parent project with full context (e.g., node_modules). + """ + # Never re-analyze inherited profiles - they came from a validated parent + # But validate that inherited_from points to a legitimate parent + if profile.inherited_from: + parent = Path(profile.inherited_from) + # Validate the inherited_from path: + # 1. Must exist and be a directory + # 2. Current project must be a descendant of the parent + # 3. Parent must contain a valid security profile + if ( + parent.exists() + and parent.is_dir() + and self._is_descendant_of(self.project_dir, parent) + and (parent / self.PROFILE_FILENAME).exists() + ): + return False + # If validation fails, treat as non-inherited and check hash current_hash = self.compute_project_hash() return current_hash != profile.project_hash + def _is_descendant_of(self, child: Path, parent: Path) -> bool: + """Check if child path is a descendant of parent path.""" + try: + child.resolve().relative_to(parent.resolve()) + return True + except ValueError: + return False + def analyze(self, force: bool = False) -> SecurityProfile: """ Perform full project analysis. @@ -205,7 +233,12 @@ def analyze(self, force: bool = False) -> SecurityProfile: # Check for existing profile existing = self.load_profile() if existing and not force and not self.should_reanalyze(existing): - print(f"Using cached security profile (hash: {existing.project_hash[:8]})") + if existing.inherited_from: + print("Using inherited security profile from parent project") + else: + print( + f"Using cached security profile (hash: {existing.project_hash[:8]})" + ) return existing print("Analyzing project structure for security profile...") diff --git a/apps/backend/project/command_registry/base.py b/apps/backend/project/command_registry/base.py index 9c070632ac..04c5f7637b 100644 --- a/apps/backend/project/command_registry/base.py +++ b/apps/backend/project/command_registry/base.py @@ -158,6 +158,10 @@ "pkill": "validate_pkill", "kill": "validate_kill", "killall": "validate_killall", + # Shell interpreters - validate commands inside -c + "bash": "validate_shell_c", + "sh": "validate_shell_c", + "zsh": "validate_shell_c", } diff --git a/apps/backend/project/models.py b/apps/backend/project/models.py index 9f5514f314..b36279b4a8 100644 --- a/apps/backend/project/models.py +++ b/apps/backend/project/models.py @@ -52,6 +52,9 @@ class SecurityProfile: project_dir: str = "" created_at: str = "" project_hash: str = "" + inherited_from: str = ( + "" # Source project path if inherited from parent (e.g., worktree) + ) def get_all_allowed_commands(self) -> set[str]: """Get the complete set of allowed commands.""" @@ -64,7 +67,7 @@ def get_all_allowed_commands(self) -> set[str]: def to_dict(self) -> dict: """Convert to JSON-serializable dict.""" - return { + result = { "base_commands": sorted(self.base_commands), "stack_commands": sorted(self.stack_commands), "script_commands": sorted(self.script_commands), @@ -75,6 +78,10 @@ def to_dict(self) -> dict: "created_at": self.created_at, "project_hash": self.project_hash, } + # Only include inherited_from if set (to keep backward compatibility) + if self.inherited_from: + result["inherited_from"] = self.inherited_from + return result @classmethod def from_dict(cls, data: dict) -> "SecurityProfile": @@ -87,6 +94,7 @@ def from_dict(cls, data: dict) -> "SecurityProfile": project_dir=data.get("project_dir", ""), created_at=data.get("created_at", ""), project_hash=data.get("project_hash", ""), + inherited_from=data.get("inherited_from", ""), ) if "detected_stack" in data: diff --git a/apps/backend/requirements.txt b/apps/backend/requirements.txt index 95c8a1eacb..13a9e1fb6c 100644 --- a/apps/backend/requirements.txt +++ b/apps/backend/requirements.txt @@ -19,3 +19,6 @@ google-generativeai>=0.8.0 # Pydantic for structured output schemas pydantic>=2.0.0 + +# Error tracking (optional - requires SENTRY_DSN environment variable) +sentry-sdk>=2.0.0 diff --git a/apps/backend/runners/github/context_gatherer.py b/apps/backend/runners/github/context_gatherer.py index 9a3c551261..374f11c68a 100644 --- a/apps/backend/runners/github/context_gatherer.py +++ b/apps/backend/runners/github/context_gatherer.py @@ -25,7 +25,11 @@ try: from .gh_client import GHClient, PRTooLargeError + from .services.io_utils import safe_print except (ImportError, ValueError, SystemError): + # Import from core.io_utils directly to avoid circular import with services package + # (services/__init__.py imports pr_review_engine which imports context_gatherer) + from core.io_utils import safe_print from gh_client import GHClient, PRTooLargeError # Validation patterns for git refs and paths (defense-in-depth) @@ -232,11 +236,11 @@ async def gather(self) -> PRContext: Returns: PRContext with all necessary information for review """ - print(f"[Context] Gathering context for PR #{self.pr_number}...", flush=True) + safe_print(f"[Context] Gathering context for PR #{self.pr_number}...") # Fetch basic PR metadata pr_data = await self._fetch_pr_metadata() - print( + safe_print( f"[Context] PR metadata: {pr_data['title']} by {pr_data['author']['login']}", flush=True, ) @@ -248,7 +252,7 @@ async def gather(self) -> PRContext: if head_sha and base_sha: refs_available = await self._ensure_pr_refs_available(head_sha, base_sha) if not refs_available: - print( + safe_print( "[Context] Warning: Could not fetch PR refs locally. " "Will use GitHub API patches as fallback.", flush=True, @@ -256,27 +260,27 @@ async def gather(self) -> PRContext: # Fetch changed files with content changed_files = await self._fetch_changed_files(pr_data) - print(f"[Context] Fetched {len(changed_files)} changed files", flush=True) + safe_print(f"[Context] Fetched {len(changed_files)} changed files") # Fetch full diff diff = await self._fetch_pr_diff() - print(f"[Context] Fetched diff: {len(diff)} chars", flush=True) + safe_print(f"[Context] Fetched diff: {len(diff)} chars") # Detect repo structure repo_structure = self._detect_repo_structure() - print("[Context] Detected repo structure", flush=True) + safe_print("[Context] Detected repo structure") # Find related files related_files = self._find_related_files(changed_files) - print(f"[Context] Found {len(related_files)} related files", flush=True) + safe_print(f"[Context] Found {len(related_files)} related files") # Fetch commits commits = await self._fetch_commits() - print(f"[Context] Fetched {len(commits)} commits", flush=True) + safe_print(f"[Context] Fetched {len(commits)} commits") # Fetch AI bot comments for triage ai_bot_comments = await self._fetch_ai_bot_comments() - print(f"[Context] Fetched {len(ai_bot_comments)} AI bot comments", flush=True) + safe_print(f"[Context] Fetched {len(ai_bot_comments)} AI bot comments") # Check if diff was truncated (empty diff but files were changed) diff_truncated = len(diff) == 0 and len(changed_files) > 0 @@ -287,7 +291,7 @@ async def gather(self) -> PRContext: has_merge_conflicts = mergeable == "CONFLICTING" if has_merge_conflicts: - print( + safe_print( f"[Context] ⚠️ PR has merge conflicts (mergeStateStatus: {merge_state_status})", flush=True, ) @@ -356,12 +360,12 @@ async def _ensure_pr_refs_available(self, head_sha: str, base_sha: str) -> bool: """ # Validate SHAs before using in git commands if not _validate_git_ref(head_sha): - print( + safe_print( f"[Context] Invalid head SHA rejected: {head_sha[:50]}...", flush=True ) return False if not _validate_git_ref(base_sha): - print( + safe_print( f"[Context] Invalid base SHA rejected: {base_sha[:50]}...", flush=True ) return False @@ -381,14 +385,14 @@ async def _ensure_pr_refs_available(self, head_sha: str, base_sha: str) -> bool: stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=30.0) if proc.returncode == 0: - print( + safe_print( f"[Context] Fetched PR refs: base={base_sha[:8]} → head={head_sha[:8]}", flush=True, ) return True else: # If direct SHA fetch fails, try fetching the PR ref - print("[Context] Direct SHA fetch failed, trying PR ref...", flush=True) + safe_print("[Context] Direct SHA fetch failed, trying PR ref...") proc2 = await asyncio.create_subprocess_exec( "git", "fetch", @@ -400,21 +404,21 @@ async def _ensure_pr_refs_available(self, head_sha: str, base_sha: str) -> bool: ) await asyncio.wait_for(proc2.communicate(), timeout=30.0) if proc2.returncode == 0: - print( + safe_print( f"[Context] Fetched PR ref: refs/pr/{self.pr_number}", flush=True, ) return True - print( + safe_print( f"[Context] Failed to fetch PR refs: {stderr.decode('utf-8')}", flush=True, ) return False except asyncio.TimeoutError: - print("[Context] Timeout fetching PR refs", flush=True) + safe_print("[Context] Timeout fetching PR refs") return False except Exception as e: - print(f"[Context] Error fetching PR refs: {e}", flush=True) + safe_print(f"[Context] Error fetching PR refs: {e}") return False async def _fetch_changed_files(self, pr_data: dict) -> list[ChangedFile]: @@ -435,7 +439,7 @@ async def _fetch_changed_files(self, pr_data: dict) -> list[ChangedFile]: additions = file_info.get("additions", 0) deletions = file_info.get("deletions", 0) - print(f"[Context] Processing {path} ({status})...", flush=True) + safe_print(f"[Context] Processing {path} ({status})...") # Use commit SHAs if available (works for fork PRs), fallback to branch names head_ref = pr_data.get("headRefOid") or pr_data["headRefName"] @@ -491,10 +495,10 @@ async def _read_file_content(self, path: str, ref: str) -> str: """ # Validate inputs to prevent command injection if not _validate_file_path(path): - print(f"[Context] Invalid file path rejected: {path[:50]}...", flush=True) + safe_print(f"[Context] Invalid file path rejected: {path[:50]}...") return "" if not _validate_git_ref(ref): - print(f"[Context] Invalid git ref rejected: {ref[:50]}...", flush=True) + safe_print(f"[Context] Invalid git ref rejected: {ref[:50]}...") return "" try: @@ -515,10 +519,10 @@ async def _read_file_content(self, path: str, ref: str) -> str: return stdout.decode("utf-8") except asyncio.TimeoutError: - print(f"[Context] Timeout reading {path} from {ref}", flush=True) + safe_print(f"[Context] Timeout reading {path} from {ref}") return "" except Exception as e: - print(f"[Context] Error reading {path} from {ref}: {e}", flush=True) + safe_print(f"[Context] Error reading {path} from {ref}: {e}") return "" async def _get_file_patch(self, path: str, base_ref: str, head_ref: str) -> str: @@ -535,15 +539,15 @@ async def _get_file_patch(self, path: str, base_ref: str, head_ref: str) -> str: """ # Validate inputs to prevent command injection if not _validate_file_path(path): - print(f"[Context] Invalid file path rejected: {path[:50]}...", flush=True) + safe_print(f"[Context] Invalid file path rejected: {path[:50]}...") return "" if not _validate_git_ref(base_ref): - print( + safe_print( f"[Context] Invalid base ref rejected: {base_ref[:50]}...", flush=True ) return "" if not _validate_git_ref(head_ref): - print( + safe_print( f"[Context] Invalid head ref rejected: {head_ref[:50]}...", flush=True ) return "" @@ -563,7 +567,7 @@ async def _get_file_patch(self, path: str, base_ref: str, head_ref: str) -> str: stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=10.0) if proc.returncode != 0: - print( + safe_print( f"[Context] Failed to get patch for {path}: {stderr.decode('utf-8')}", flush=True, ) @@ -571,10 +575,10 @@ async def _get_file_patch(self, path: str, base_ref: str, head_ref: str) -> str: return stdout.decode("utf-8") except asyncio.TimeoutError: - print(f"[Context] Timeout getting patch for {path}", flush=True) + safe_print(f"[Context] Timeout getting patch for {path}") return "" except Exception as e: - print(f"[Context] Error getting patch for {path}: {e}", flush=True) + safe_print(f"[Context] Error getting patch for {path}: {e}") return "" async def _fetch_pr_diff(self) -> str: @@ -587,8 +591,8 @@ async def _fetch_pr_diff(self) -> str: try: return await self.gh_client.pr_diff(self.pr_number) except PRTooLargeError as e: - print(f"[Context] Warning: {str(e)}", flush=True) - print( + safe_print(f"[Context] Warning: {str(e)}") + safe_print( "[Context] Skipping full diff - will use individual file patches", flush=True, ) @@ -630,7 +634,7 @@ async def _fetch_ai_bot_comments(self) -> list[AIBotComment]: ai_comments.append(ai_comment) except Exception as e: - print(f"[Context] Error fetching AI bot comments: {e}", flush=True) + safe_print(f"[Context] Error fetching AI bot comments: {e}") return ai_comments @@ -698,7 +702,7 @@ async def _fetch_pr_review_comments(self) -> list[dict]: return json.loads(result.stdout) return [] except Exception as e: - print(f"[Context] Error fetching review comments: {e}", flush=True) + safe_print(f"[Context] Error fetching review comments: {e}") return [] async def _fetch_pr_issue_comments(self) -> list[dict]: @@ -717,7 +721,7 @@ async def _fetch_pr_issue_comments(self) -> list[dict]: return json.loads(result.stdout) return [] except Exception as e: - print(f"[Context] Error fetching issue comments: {e}", flush=True) + safe_print(f"[Context] Error fetching issue comments: {e}") return [] def _detect_repo_structure(self) -> str: @@ -1015,7 +1019,7 @@ async def gather(self) -> FollowupReviewContext: previous_sha = self.previous_review.reviewed_commit_sha if not previous_sha: - print( + safe_print( "[Followup] No reviewed_commit_sha in previous review, cannot gather incremental context", flush=True, ) @@ -1026,7 +1030,7 @@ async def gather(self) -> FollowupReviewContext: current_commit_sha="", ) - print( + safe_print( f"[Followup] Gathering context since commit {previous_sha[:8]}...", flush=True, ) @@ -1035,7 +1039,7 @@ async def gather(self) -> FollowupReviewContext: current_sha = await self.gh_client.get_pr_head_sha(self.pr_number) if not current_sha: - print("[Followup] Could not fetch current HEAD SHA", flush=True) + safe_print("[Followup] Could not fetch current HEAD SHA") return FollowupReviewContext( pr_number=self.pr_number, previous_review=self.previous_review, @@ -1044,7 +1048,7 @@ async def gather(self) -> FollowupReviewContext: ) if previous_sha == current_sha: - print("[Followup] No new commits since last review", flush=True) + safe_print("[Followup] No new commits since last review") return FollowupReviewContext( pr_number=self.pr_number, previous_review=self.previous_review, @@ -1052,7 +1056,7 @@ async def gather(self) -> FollowupReviewContext: current_commit_sha=current_sha, ) - print( + safe_print( f"[Followup] Comparing {previous_sha[:8]}...{current_sha[:8]}", flush=True ) @@ -1065,29 +1069,29 @@ async def gather(self) -> FollowupReviewContext: pr_files, new_commits = await self.gh_client.get_pr_files_changed_since( self.pr_number, previous_sha, reviewed_file_blobs=reviewed_file_blobs ) - print( + safe_print( f"[Followup] PR has {len(pr_files)} files, " f"{len(new_commits)} commits since last review" + (" (blob comparison used)" if reviewed_file_blobs else ""), flush=True, ) except Exception as e: - print(f"[Followup] Error getting PR files/commits: {e}", flush=True) + safe_print(f"[Followup] Error getting PR files/commits: {e}") # Fallback to compare_commits if PR endpoints fail - print("[Followup] Falling back to commit comparison...", flush=True) + safe_print("[Followup] Falling back to commit comparison...") try: comparison = await self.gh_client.compare_commits( previous_sha, current_sha ) new_commits = comparison.get("commits", []) pr_files = comparison.get("files", []) - print( + safe_print( f"[Followup] Fallback: Found {len(new_commits)} commits, " f"{len(pr_files)} files (may include merge-introduced changes)", flush=True, ) except Exception as e2: - print(f"[Followup] Fallback also failed: {e2}", flush=True) + safe_print(f"[Followup] Fallback also failed: {e2}") return FollowupReviewContext( pr_number=self.pr_number, previous_review=self.previous_review, @@ -1099,7 +1103,7 @@ async def gather(self) -> FollowupReviewContext: # Use PR files as the canonical list (excludes files from merged branches) commits = new_commits files = pr_files - print( + safe_print( f"[Followup] Found {len(commits)} new commits, {len(files)} changed files", flush=True, ) @@ -1123,7 +1127,7 @@ async def gather(self) -> FollowupReviewContext: self.pr_number, self.previous_review.reviewed_at ) except Exception as e: - print(f"[Followup] Error fetching comments: {e}", flush=True) + safe_print(f"[Followup] Error fetching comments: {e}") comments = {"review_comments": [], "issue_comments": []} # Get formal PR reviews since last review (from Cursor, CodeRabbit, etc.) @@ -1132,7 +1136,7 @@ async def gather(self) -> FollowupReviewContext: self.pr_number, self.previous_review.reviewed_at ) except Exception as e: - print(f"[Followup] Error fetching PR reviews: {e}", flush=True) + safe_print(f"[Followup] Error fetching PR reviews: {e}") pr_reviews = [] # Separate AI bot comments from contributor comments @@ -1179,7 +1183,7 @@ async def gather(self) -> FollowupReviewContext: contributor_reviews ) - print( + safe_print( f"[Followup] Found {total_contributor_feedback} contributor feedback " f"({len(contributor_comments)} comments, {len(contributor_reviews)} reviews), " f"{total_ai_feedback} AI feedback " @@ -1200,12 +1204,12 @@ async def gather(self) -> FollowupReviewContext: has_merge_conflicts = mergeable == "CONFLICTING" if has_merge_conflicts: - print( + safe_print( f"[Followup] ⚠️ PR has merge conflicts (mergeStateStatus: {merge_state_status})", flush=True, ) except Exception as e: - print(f"[Followup] Could not fetch merge status: {e}", flush=True) + safe_print(f"[Followup] Could not fetch merge status: {e}") return FollowupReviewContext( pr_number=self.pr_number, diff --git a/apps/backend/runners/github/orchestrator.py b/apps/backend/runners/github/orchestrator.py index 22d3e144f6..60ed964883 100644 --- a/apps/backend/runners/github/orchestrator.py +++ b/apps/backend/runners/github/orchestrator.py @@ -46,6 +46,7 @@ PRReviewEngine, TriageEngine, ) + from .services.io_utils import safe_print except (ImportError, ValueError, SystemError): # When imported directly (runner.py adds github dir to path) from bot_detection import BotDetector @@ -74,6 +75,7 @@ PRReviewEngine, TriageEngine, ) + from services.io_utils import safe_print @dataclass @@ -267,12 +269,12 @@ async def _post_ai_triage_replies( comment_id=triage.comment_id, body=triage.response_comment, ) - print( + safe_print( f"[AI TRIAGE] Posted reply to {triage.tool_name} comment {triage.comment_id}", flush=True, ) except Exception as e: - print( + safe_print( f"[AI TRIAGE] Failed to post reply to comment {triage.comment_id}: {e}", flush=True, ) @@ -295,7 +297,7 @@ async def review_pr( Returns: PRReviewResult with findings and overall assessment """ - print( + safe_print( f"[DEBUG orchestrator] review_pr() called for PR #{pr_number}", flush=True ) @@ -308,14 +310,14 @@ async def review_pr( try: # Gather PR context - print("[DEBUG orchestrator] Creating context gatherer...", flush=True) + safe_print("[DEBUG orchestrator] Creating context gatherer...") gatherer = PRContextGatherer( self.project_dir, pr_number, repo=self.config.repo ) - print("[DEBUG orchestrator] Gathering PR context...", flush=True) + safe_print("[DEBUG orchestrator] Gathering PR context...") pr_context = await gatherer.gather() - print( + safe_print( f"[DEBUG orchestrator] Context gathered: {pr_context.title} " f"({len(pr_context.changed_files)} files, {len(pr_context.related_files)} related)", flush=True, @@ -331,14 +333,14 @@ async def review_pr( # Allow forcing a review to bypass "already reviewed" check if should_skip and force_review and "Already reviewed" in skip_reason: - print( + safe_print( f"[BOT DETECTION] Force review requested - bypassing: {skip_reason}", flush=True, ) should_skip = False if should_skip: - print( + safe_print( f"[BOT DETECTION] Skipping PR #{pr_number}: {skip_reason}", flush=True, ) @@ -348,7 +350,7 @@ async def review_pr( if "Already reviewed" in skip_reason: existing_review = PRReviewResult.load(self.github_dir, pr_number) if existing_review: - print( + safe_print( "[BOT DETECTION] Returning existing review (no new commits)", flush=True, ) @@ -373,14 +375,14 @@ async def review_pr( ) # Delegate to PR Review Engine - print("[DEBUG orchestrator] Running multi-pass review...", flush=True) + safe_print("[DEBUG orchestrator] Running multi-pass review...") ( findings, structural_issues, ai_triages, quick_scan, ) = await self.pr_review_engine.run_multi_pass_review(pr_context) - print( + safe_print( f"[DEBUG orchestrator] Multi-pass review complete: " f"{len(findings)} findings, {len(structural_issues)} structural, {len(ai_triages)} AI triages", flush=True, @@ -407,12 +409,12 @@ async def review_pr( ci_log_parts.append(f"{pending_without_awaiting} pending") if awaiting > 0: ci_log_parts.append(f"{awaiting} awaiting approval") - print( + safe_print( f"[orchestrator] CI status: {', '.join(ci_log_parts)}", flush=True, ) if awaiting > 0: - print( + safe_print( f"[orchestrator] ⚠️ {awaiting} workflow(s) from fork need maintainer approval to run", flush=True, ) @@ -426,7 +428,7 @@ async def review_pr( has_merge_conflicts=pr_context.has_merge_conflicts, merge_state_status=pr_context.merge_state_status, ) - print( + safe_print( f"[DEBUG orchestrator] Verdict: {verdict.value} - {verdict_reasoning}", flush=True, ) @@ -471,12 +473,12 @@ async def review_pr( blob_sha = file.get("sha", "") if filename and blob_sha: file_blobs[filename] = blob_sha - print( + safe_print( f"[Review] Captured {len(file_blobs)} file blob SHAs for follow-up tracking", flush=True, ) except Exception as e: - print( + safe_print( f"[Review] Warning: Could not capture file blobs: {e}", flush=True ) @@ -544,11 +546,11 @@ async def review_pr( # Log full exception details for debugging error_details = f"{type(e).__name__}: {e}" full_traceback = traceback.format_exc() - print( + safe_print( f"[ERROR orchestrator] PR review failed for #{pr_number}: {error_details}", flush=True, ) - print(f"[ERROR orchestrator] Full traceback:\n{full_traceback}", flush=True) + safe_print(f"[ERROR orchestrator] Full traceback:\n{full_traceback}") result = PRReviewResult( pr_number=pr_number, @@ -577,7 +579,7 @@ async def followup_review_pr(self, pr_number: int) -> PRReviewResult: Raises: ValueError: If no previous review exists for this PR """ - print( + safe_print( f"[DEBUG orchestrator] followup_review_pr() called for PR #{pr_number}", flush=True, ) @@ -622,7 +624,7 @@ async def followup_review_pr(self, pr_number: int) -> PRReviewResult: # Check if context gathering failed if followup_context.error: - print( + safe_print( f"[Followup] Context gathering failed: {followup_context.error}", flush=True, ) @@ -653,7 +655,7 @@ async def followup_review_pr(self, pr_number: int) -> PRReviewResult: if not has_commits and not has_file_changes: base_sha = previous_review.reviewed_commit_sha[:8] - print( + safe_print( f"[Followup] No changes since last review at {base_sha}", flush=True, ) @@ -700,7 +702,7 @@ async def followup_review_pr(self, pr_number: int) -> PRReviewResult: # Use parallel orchestrator for follow-up if enabled if self.config.use_parallel_orchestrator: - print( + safe_print( "[AI] Using parallel orchestrator for follow-up review (SDK subagents)...", flush=True, ) @@ -746,7 +748,7 @@ async def followup_review_pr(self, pr_number: int) -> PRReviewResult: # (CI status was already passed to AI via followup_context.ci_status) failed_checks = followup_context.ci_status.get("failed_checks", []) if failed_checks: - print( + safe_print( f"[Followup] CI checks failing: {failed_checks}", flush=True, ) @@ -1259,7 +1261,7 @@ async def triage_issues( issue["number"], result.labels_to_remove ) except Exception as e: - print(f"Failed to apply labels to #{issue['number']}: {e}") + safe_print(f"Failed to apply labels to #{issue['number']}: {e}") # Save result await result.save(self.github_dir) diff --git a/apps/backend/runners/github/runner.py b/apps/backend/runners/github/runner.py index b3934cdc93..103ffc7281 100644 --- a/apps/backend/runners/github/runner.py +++ b/apps/backend/runners/github/runner.py @@ -65,6 +65,11 @@ if env_file.exists(): load_dotenv(env_file) +# Initialize Sentry early to capture any startup errors +from core.sentry import capture_exception, init_sentry, set_context + +init_sentry(component="github-runner") + from debug import debug_error # Add github runner directory to path for direct imports @@ -73,6 +78,7 @@ # Now import models and orchestrator directly (they use relative imports internally) from models import GitHubRunnerConfig from orchestrator import GitHubOrchestrator, ProgressCallback +from services.io_utils import safe_print def print_progress(callback: ProgressCallback) -> None: @@ -83,7 +89,7 @@ def print_progress(callback: ProgressCallback) -> None: elif callback.issue_number: prefix = f"[Issue #{callback.issue_number}] " - print(f"{prefix}[{callback.progress:3d}%] {callback.message}", flush=True) + safe_print(f"{prefix}[{callback.progress:3d}%] {callback.message}") def get_config(args) -> GitHubRunnerConfig: @@ -110,8 +116,8 @@ def get_config(args) -> GitHubRunnerConfig: break if os.environ.get("DEBUG"): - print(f"[DEBUG] gh CLI path: {gh_path}", flush=True) - print( + safe_print(f"[DEBUG] gh CLI path: {gh_path}") + safe_print( f"[DEBUG] PATH env: {os.environ.get('PATH', 'NOT SET')[:200]}...", flush=True, ) @@ -149,16 +155,20 @@ def get_config(args) -> GitHubRunnerConfig: if result.returncode == 0: repo = result.stdout.strip() elif os.environ.get("DEBUG"): - print(f"[DEBUG] gh repo view failed: {result.stderr}", flush=True) + safe_print(f"[DEBUG] gh repo view failed: {result.stderr}") except FileNotFoundError: pass # gh not installed or not in PATH if not token: - print("Error: No GitHub token found. Set GITHUB_TOKEN or run 'gh auth login'") + safe_print( + "Error: No GitHub token found. Set GITHUB_TOKEN or run 'gh auth login'" + ) sys.exit(1) if not repo: - print("Error: No GitHub repo found. Set GITHUB_REPO or run from a git repo.") + safe_print( + "Error: No GitHub repo found. Set GITHUB_REPO or run from a git repo." + ) sys.exit(1) return GitHubRunnerConfig( @@ -185,18 +195,18 @@ async def cmd_review_pr(args) -> int: debug = os.environ.get("DEBUG") if debug: - print(f"[DEBUG] Starting PR review for PR #{args.pr_number}", flush=True) - print(f"[DEBUG] Project directory: {args.project}", flush=True) - print("[DEBUG] Building config...", flush=True) + safe_print(f"[DEBUG] Starting PR review for PR #{args.pr_number}") + safe_print(f"[DEBUG] Project directory: {args.project}") + safe_print("[DEBUG] Building config...") config = get_config(args) if debug: - print( + safe_print( f"[DEBUG] Config built: repo={config.repo}, model={config.model}", flush=True, ) - print("[DEBUG] Creating orchestrator...", flush=True) + safe_print("[DEBUG] Creating orchestrator...") orchestrator = GitHubOrchestrator( project_dir=args.project, @@ -205,8 +215,8 @@ async def cmd_review_pr(args) -> int: ) if debug: - print("[DEBUG] Orchestrator created", flush=True) - print( + safe_print("[DEBUG] Orchestrator created") + safe_print( f"[DEBUG] Calling orchestrator.review_pr({args.pr_number})...", flush=True ) @@ -215,27 +225,27 @@ async def cmd_review_pr(args) -> int: result = await orchestrator.review_pr(args.pr_number, force_review=force_review) if debug: - print(f"[DEBUG] review_pr returned, success={result.success}", flush=True) + safe_print(f"[DEBUG] review_pr returned, success={result.success}") if result.success: - print(f"\n{'=' * 60}") - print(f"PR #{result.pr_number} Review Complete") - print(f"{'=' * 60}") - print(f"Status: {result.overall_status}") - print(f"Summary: {result.summary}") - print(f"Findings: {len(result.findings)}") + safe_print(f"\n{'=' * 60}") + safe_print(f"PR #{result.pr_number} Review Complete") + safe_print(f"{'=' * 60}") + safe_print(f"Status: {result.overall_status}") + safe_print(f"Summary: {result.summary}") + safe_print(f"Findings: {len(result.findings)}") if result.findings: - print("\nFindings by severity:") + safe_print("\nFindings by severity:") for f in result.findings: emoji = {"critical": "!", "high": "*", "medium": "-", "low": "."} - print( + safe_print( f" {emoji.get(f.severity.value, '?')} [{f.severity.value.upper()}] {f.title}" ) - print(f" File: {f.file}:{f.line}") + safe_print(f" File: {f.file}:{f.line}") return 0 else: - print(f"\nReview failed: {result.error}") + safe_print(f"\nReview failed: {result.error}") return 1 @@ -251,18 +261,18 @@ async def cmd_followup_review_pr(args) -> int: debug = os.environ.get("DEBUG") if debug: - print(f"[DEBUG] Starting follow-up review for PR #{args.pr_number}", flush=True) - print(f"[DEBUG] Project directory: {args.project}", flush=True) - print("[DEBUG] Building config...", flush=True) + safe_print(f"[DEBUG] Starting follow-up review for PR #{args.pr_number}") + safe_print(f"[DEBUG] Project directory: {args.project}") + safe_print("[DEBUG] Building config...") config = get_config(args) if debug: - print( + safe_print( f"[DEBUG] Config built: repo={config.repo}, model={config.model}", flush=True, ) - print("[DEBUG] Creating orchestrator...", flush=True) + safe_print("[DEBUG] Creating orchestrator...") orchestrator = GitHubOrchestrator( project_dir=args.project, @@ -271,8 +281,8 @@ async def cmd_followup_review_pr(args) -> int: ) if debug: - print("[DEBUG] Orchestrator created", flush=True) - print( + safe_print("[DEBUG] Orchestrator created") + safe_print( f"[DEBUG] Calling orchestrator.followup_review_pr({args.pr_number})...", flush=True, ) @@ -280,43 +290,43 @@ async def cmd_followup_review_pr(args) -> int: try: result = await orchestrator.followup_review_pr(args.pr_number) except ValueError as e: - print(f"\nFollow-up review failed: {e}") + safe_print(f"\nFollow-up review failed: {e}") return 1 if debug: - print( + safe_print( f"[DEBUG] followup_review_pr returned, success={result.success}", flush=True ) if result.success: - print(f"\n{'=' * 60}") - print(f"PR #{result.pr_number} Follow-up Review Complete") - print(f"{'=' * 60}") - print(f"Status: {result.overall_status}") - print(f"Is Follow-up: {result.is_followup_review}") + safe_print(f"\n{'=' * 60}") + safe_print(f"PR #{result.pr_number} Follow-up Review Complete") + safe_print(f"{'=' * 60}") + safe_print(f"Status: {result.overall_status}") + safe_print(f"Is Follow-up: {result.is_followup_review}") if result.resolved_findings: - print(f"Resolved: {len(result.resolved_findings)} finding(s)") + safe_print(f"Resolved: {len(result.resolved_findings)} finding(s)") if result.unresolved_findings: - print(f"Still Open: {len(result.unresolved_findings)} finding(s)") + safe_print(f"Still Open: {len(result.unresolved_findings)} finding(s)") if result.new_findings_since_last_review: - print( + safe_print( f"New Issues: {len(result.new_findings_since_last_review)} finding(s)" ) - print(f"\nSummary:\n{result.summary}") + safe_print(f"\nSummary:\n{result.summary}") if result.findings: - print("\nRemaining Findings:") + safe_print("\nRemaining Findings:") for f in result.findings: emoji = {"critical": "!", "high": "*", "medium": "-", "low": "."} - print( + safe_print( f" {emoji.get(f.severity.value, '?')} [{f.severity.value.upper()}] {f.title}" ) - print(f" File: {f.file}:{f.line}") + safe_print(f" File: {f.file}:{f.line}") return 0 else: - print(f"\nFollow-up review failed: {result.error}") + safe_print(f"\nFollow-up review failed: {result.error}") return 1 @@ -335,9 +345,9 @@ async def cmd_triage(args) -> int: apply_labels=args.apply_labels, ) - print(f"\n{'=' * 60}") - print(f"Triaged {len(results)} issues") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print(f"Triaged {len(results)} issues") + safe_print(f"{'=' * 60}") for r in results: flags = [] @@ -349,12 +359,12 @@ async def cmd_triage(args) -> int: flags.append("CREEP") flag_str = f" [{', '.join(flags)}]" if flags else "" - print( + safe_print( f" #{r.issue_number}: {r.category.value} (confidence: {r.confidence:.0%}){flag_str}" ) if r.labels_to_add: - print(f" + Labels: {', '.join(r.labels_to_add)}") + safe_print(f" + Labels: {', '.join(r.labels_to_add)}") return 0 @@ -371,16 +381,16 @@ async def cmd_auto_fix(args) -> int: state = await orchestrator.auto_fix_issue(args.issue_number) - print(f"\n{'=' * 60}") - print(f"Auto-Fix State for Issue #{state.issue_number}") - print(f"{'=' * 60}") - print(f"Status: {state.status.value}") + safe_print(f"\n{'=' * 60}") + safe_print(f"Auto-Fix State for Issue #{state.issue_number}") + safe_print(f"{'=' * 60}") + safe_print(f"Status: {state.status.value}") if state.spec_id: - print(f"Spec ID: {state.spec_id}") + safe_print(f"Spec ID: {state.spec_id}") if state.pr_number: - print(f"PR: #{state.pr_number}") + safe_print(f"PR: #{state.pr_number}") if state.error: - print(f"Error: {state.error}") + safe_print(f"Error: {state.error}") return 0 @@ -398,11 +408,11 @@ async def cmd_check_labels(args) -> int: issues = await orchestrator.check_auto_fix_labels() if issues: - print(f"Found {len(issues)} issues with auto-fix labels:") + safe_print(f"Found {len(issues)} issues with auto-fix labels:") for num in issues: - print(f" #{num}") + safe_print(f" #{num}") else: - print("No issues with auto-fix labels found.") + safe_print("No issues with auto-fix labels found.") return 0 @@ -419,8 +429,8 @@ async def cmd_check_new(args) -> int: issues = await orchestrator.check_new_issues() - print("JSON Output") - print(json.dumps(issues)) + safe_print("JSON Output") + safe_print(json.dumps(issues)) return 0 @@ -435,12 +445,12 @@ async def cmd_queue(args) -> int: queue = await orchestrator.get_auto_fix_queue() - print(f"\n{'=' * 60}") - print(f"Auto-Fix Queue ({len(queue)} items)") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print(f"Auto-Fix Queue ({len(queue)} items)") + safe_print(f"{'=' * 60}") if not queue: - print("Queue is empty.") + safe_print("Queue is empty.") return 0 for state in queue: @@ -455,11 +465,11 @@ async def cmd_queue(args) -> int: "failed": "ERR", } emoji = status_emoji.get(state.status.value, "???") - print(f" [{emoji}] #{state.issue_number}: {state.status.value}") + safe_print(f" [{emoji}] #{state.issue_number}: {state.status.value}") if state.pr_number: - print(f" PR: #{state.pr_number}") + safe_print(f" PR: #{state.pr_number}") if state.error: - print(f" Error: {state.error[:50]}...") + safe_print(f" Error: {state.error[:50]}...") return 0 @@ -477,22 +487,24 @@ async def cmd_batch_issues(args) -> int: issue_numbers = args.issues if args.issues else None batches = await orchestrator.batch_and_fix_issues(issue_numbers) - print(f"\n{'=' * 60}") - print(f"Created {len(batches)} batches from similar issues") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print(f"Created {len(batches)} batches from similar issues") + safe_print(f"{'=' * 60}") if not batches: - print("No batches created. Either no issues found or all issues are unique.") + safe_print( + "No batches created. Either no issues found or all issues are unique." + ) return 0 for batch in batches: issue_nums = ", ".join(f"#{i.issue_number}" for i in batch.issues) - print(f"\n Batch: {batch.batch_id}") - print(f" Issues: {issue_nums}") - print(f" Theme: {batch.theme}") - print(f" Status: {batch.status.value}") + safe_print(f"\n Batch: {batch.batch_id}") + safe_print(f" Issues: {issue_nums}") + safe_print(f" Theme: {batch.theme}") + safe_print(f" Status: {batch.status.value}") if batch.spec_id: - print(f" Spec: {batch.spec_id}") + safe_print(f" Spec: {batch.spec_id}") return 0 @@ -507,14 +519,14 @@ async def cmd_batch_status(args) -> int: status = await orchestrator.get_batch_status() - print(f"\n{'=' * 60}") - print("Batch Status") - print(f"{'=' * 60}") - print(f"Total batches: {status.get('total_batches', 0)}") - print(f"Pending: {status.get('pending', 0)}") - print(f"Processing: {status.get('processing', 0)}") - print(f"Completed: {status.get('completed', 0)}") - print(f"Failed: {status.get('failed', 0)}") + safe_print(f"\n{'=' * 60}") + safe_print("Batch Status") + safe_print(f"{'=' * 60}") + safe_print(f"Total batches: {status.get('total_batches', 0)}") + safe_print(f"Pending: {status.get('pending', 0)}") + safe_print(f"Processing: {status.get('processing', 0)}") + safe_print(f"Completed: {status.get('completed', 0)}") + safe_print(f"Failed: {status.get('failed', 0)}") return 0 @@ -543,47 +555,47 @@ async def cmd_analyze_preview(args) -> int: ) if not result.get("success"): - print(f"Error: {result.get('error', 'Unknown error')}") + safe_print(f"Error: {result.get('error', 'Unknown error')}") return 1 - print(f"\n{'=' * 60}") - print("Issue Analysis Preview") - print(f"{'=' * 60}") - print(f"Total issues: {result.get('total_issues', 0)}") - print(f"Analyzed: {result.get('analyzed_issues', 0)}") - print(f"Already batched: {result.get('already_batched', 0)}") - print(f"Proposed batches: {len(result.get('proposed_batches', []))}") - print(f"Single issues: {len(result.get('single_issues', []))}") + safe_print(f"\n{'=' * 60}") + safe_print("Issue Analysis Preview") + safe_print(f"{'=' * 60}") + safe_print(f"Total issues: {result.get('total_issues', 0)}") + safe_print(f"Analyzed: {result.get('analyzed_issues', 0)}") + safe_print(f"Already batched: {result.get('already_batched', 0)}") + safe_print(f"Proposed batches: {len(result.get('proposed_batches', []))}") + safe_print(f"Single issues: {len(result.get('single_issues', []))}") proposed_batches = result.get("proposed_batches", []) if proposed_batches: - print(f"\n{'=' * 60}") - print("Proposed Batches (for human review)") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print("Proposed Batches (for human review)") + safe_print(f"{'=' * 60}") for i, batch in enumerate(proposed_batches, 1): confidence = batch.get("confidence", 0) validated = "" if batch.get("validated") else "[NEEDS REVIEW] " - print( + safe_print( f"\n Batch {i}: {validated}{batch.get('theme', 'No theme')} ({confidence:.0%} confidence)" ) - print(f" Primary issue: #{batch.get('primary_issue')}") - print(f" Issue count: {batch.get('issue_count', 0)}") - print(f" Reasoning: {batch.get('reasoning', 'N/A')}") - print(" Issues:") + safe_print(f" Primary issue: #{batch.get('primary_issue')}") + safe_print(f" Issue count: {batch.get('issue_count', 0)}") + safe_print(f" Reasoning: {batch.get('reasoning', 'N/A')}") + safe_print(" Issues:") for item in batch.get("issues", []): similarity = item.get("similarity_to_primary", 0) - print( + safe_print( f" - #{item['issue_number']}: {item.get('title', '?')} ({similarity:.0%})" ) # Output JSON for programmatic use if getattr(args, "json", False): - print(f"\n{'=' * 60}") - print("JSON Output") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print("JSON Output") + safe_print(f"{'=' * 60}") # Print JSON on single line to avoid corruption from line-by-line stdout prefixes - print(json.dumps(result)) + safe_print(json.dumps(result)) return 0 @@ -608,24 +620,24 @@ async def cmd_approve_batches(args) -> int: with open(args.batch_file) as f: approved_batches = json.load(f) except (json.JSONDecodeError, FileNotFoundError) as e: - print(f"Error loading batch file: {e}") + safe_print(f"Error loading batch file: {e}") return 1 if not approved_batches: - print("No batches in file to approve.") + safe_print("No batches in file to approve.") return 0 - print(f"Approving and executing {len(approved_batches)} batches...") + safe_print(f"Approving and executing {len(approved_batches)} batches...") created_batches = await orchestrator.approve_and_execute_batches(approved_batches) - print(f"\n{'=' * 60}") - print(f"Created {len(created_batches)} batches") - print(f"{'=' * 60}") + safe_print(f"\n{'=' * 60}") + safe_print(f"Created {len(created_batches)} batches") + safe_print(f"{'=' * 60}") for batch in created_batches: issue_nums = ", ".join(f"#{i.issue_number}" for i in batch.issues) - print(f" {batch.batch_id}: {issue_nums}") + safe_print(f" {batch.batch_id}: {issue_nums}") return 0 @@ -800,20 +812,33 @@ def main(): handler = commands.get(args.command) if not handler: - print(f"Unknown command: {args.command}") + safe_print(f"Unknown command: {args.command}") sys.exit(1) try: + # Set context for Sentry + set_context( + "command", + { + "name": args.command, + "project": str(args.project), + "repo": args.repo or "auto-detect", + }, + ) + exit_code = asyncio.run(handler(args)) sys.exit(exit_code) except KeyboardInterrupt: - print("\nInterrupted.") + safe_print("\nInterrupted.") sys.exit(1) except Exception as e: import traceback + # Capture exception with Sentry + capture_exception(e, command=args.command) + debug_error("github_runner", "Command failed", error=str(e)) - print(f"Error: {e}") + safe_print(f"Error: {e}") traceback.print_exc() sys.exit(1) diff --git a/apps/backend/runners/github/services/__init__.py b/apps/backend/runners/github/services/__init__.py index f36e0b512c..18228804a9 100644 --- a/apps/backend/runners/github/services/__init__.py +++ b/apps/backend/runners/github/services/__init__.py @@ -3,14 +3,23 @@ ============================ Service layer for GitHub automation workflows. + +NOTE: Uses lazy imports to avoid circular dependency with context_gatherer.py. +The circular import chain was: orchestrator → context_gatherer → services.io_utils +→ services/__init__ → pr_review_engine → context_gatherer (circular!) """ -from .autofix_processor import AutoFixProcessor -from .batch_processor import BatchProcessor -from .pr_review_engine import PRReviewEngine -from .prompt_manager import PromptManager -from .response_parsers import ResponseParser -from .triage_engine import TriageEngine +from __future__ import annotations + +# Lazy import mapping - classes are loaded on first access +_LAZY_IMPORTS: dict[str, tuple[str, str]] = { + "AutoFixProcessor": (".autofix_processor", "AutoFixProcessor"), + "BatchProcessor": (".batch_processor", "BatchProcessor"), + "PRReviewEngine": (".pr_review_engine", "PRReviewEngine"), + "PromptManager": (".prompt_manager", "PromptManager"), + "ResponseParser": (".response_parsers", "ResponseParser"), + "TriageEngine": (".triage_engine", "TriageEngine"), +} __all__ = [ "PromptManager", @@ -20,3 +29,19 @@ "AutoFixProcessor", "BatchProcessor", ] + +# Cache for lazily loaded modules +_loaded: dict[str, object] = {} + + +def __getattr__(name: str) -> object: + """Lazy import handler - loads classes on first access.""" + if name in _LAZY_IMPORTS: + if name not in _loaded: + module_name, attr_name = _LAZY_IMPORTS[name] + import importlib + + module = importlib.import_module(module_name, __name__) + _loaded[name] = getattr(module, attr_name) + return _loaded[name] + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/apps/backend/runners/github/services/batch_processor.py b/apps/backend/runners/github/services/batch_processor.py index 396a5461f5..5e013c4b02 100644 --- a/apps/backend/runners/github/services/batch_processor.py +++ b/apps/backend/runners/github/services/batch_processor.py @@ -12,8 +12,10 @@ try: from ..models import AutoFixState, AutoFixStatus, GitHubRunnerConfig + from .io_utils import safe_print except (ImportError, ValueError, SystemError): from models import AutoFixState, AutoFixStatus, GitHubRunnerConfig + from services.io_utils import safe_print class BatchProcessor: @@ -76,10 +78,10 @@ async def batch_and_fix_issues( try: if not issues: - print("[BATCH] No issues to batch", flush=True) + safe_print("[BATCH] No issues to batch") return [] - print( + safe_print( f"[BATCH] Analyzing {len(issues)} issues for similarity...", flush=True ) @@ -123,7 +125,7 @@ async def batch_and_fix_issues( # Create batches (includes AI validation) batches = await batcher.create_batches(issues, exclude_issues) - print(f"[BATCH] Created {len(batches)} validated batches", flush=True) + safe_print(f"[BATCH] Created {len(batches)} validated batches") self._report_progress("batching", 60, f"Created {len(batches)} batches") @@ -137,7 +139,7 @@ async def batch_and_fix_issues( f"Processing batch {i + 1}/{len(batches)} ({len(issue_nums)} issues)...", ) - print( + safe_print( f"[BATCH] Batch {batch.batch_id}: {len(issue_nums)} issues - {issue_nums}", flush=True, ) @@ -164,7 +166,7 @@ async def batch_and_fix_issues( return batches except Exception as e: - print(f"[BATCH] Error batching issues: {e}", flush=True) + safe_print(f"[BATCH] Error batching issues: {e}") import traceback traceback.print_exc() @@ -204,7 +206,7 @@ async def analyze_issues_preview( issues = issues[:max_issues] - print( + safe_print( f"[PREVIEW] Analyzing {len(issues)} issues for grouping...", flush=True ) self._report_progress("analyzing", 20, f"Analyzing {len(issues)} issues...") @@ -343,7 +345,7 @@ async def analyze_issues_preview( reasoning = result.reasoning refined_theme = result.common_theme or refined_theme except Exception as e: - print(f"[PREVIEW] Validation error: {e}", flush=True) + safe_print(f"[PREVIEW] Validation error: {e}") validated = True confidence = 0.5 reasoning = "Validation skipped due to error" @@ -380,7 +382,7 @@ async def analyze_issues_preview( except Exception as e: import traceback - print(f"[PREVIEW] Error: {e}", flush=True) + safe_print(f"[PREVIEW] Error: {e}") traceback.print_exc() return { "success": False, diff --git a/apps/backend/runners/github/services/followup_reviewer.py b/apps/backend/runners/github/services/followup_reviewer.py index 5c1c8bbca0..a0474fe919 100644 --- a/apps/backend/runners/github/services/followup_reviewer.py +++ b/apps/backend/runners/github/services/followup_reviewer.py @@ -35,6 +35,7 @@ ReviewSeverity, ) from .category_utils import map_category + from .io_utils import safe_print from .prompt_manager import PromptManager from .pydantic_models import FollowupReviewResponse except (ImportError, ValueError, SystemError): @@ -47,6 +48,7 @@ ReviewSeverity, ) from services.category_utils import map_category + from services.io_utils import safe_print from services.prompt_manager import PromptManager from services.pydantic_models import FollowupReviewResponse @@ -102,7 +104,7 @@ def _report_progress( "pr_number": pr_number, } ) - print(f"[Followup] [{phase}] {message}", flush=True) + safe_print(f"[Followup] [{phase}] {message}") async def review_followup( self, @@ -691,7 +693,7 @@ async def _run_ai_review( logger.debug( f"[Followup] Using output_format schema: {list(schema.get('properties', {}).keys())}" ) - print(f"[Followup] SDK query with output_format, model={model}", flush=True) + safe_print(f"[Followup] SDK query with output_format, model={model}") # Iterate through messages from the query # Note: max_turns=2 because structured output uses a tool call + response @@ -726,7 +728,7 @@ async def _run_ai_review( logger.info( "[Followup] Found StructuredOutput tool use" ) - print( + safe_print( "[Followup] Using SDK structured output", flush=True, ) @@ -744,7 +746,7 @@ async def _run_ai_review( logger.info( "[Followup] Found structured_output attribute on message" ) - print( + safe_print( "[Followup] Using SDK structured output (direct attribute)", flush=True, ) @@ -768,7 +770,7 @@ async def _run_ai_review( except ValueError as e: # OAuth token not found logger.warning(f"No OAuth token available for AI review: {e}") - print("AI review failed: No OAuth token found", flush=True) + safe_print("AI review failed: No OAuth token found") return None except Exception as e: logger.error(f"AI review with structured output failed: {e}") diff --git a/apps/backend/runners/github/services/io_utils.py b/apps/backend/runners/github/services/io_utils.py new file mode 100644 index 0000000000..d9fb42053b --- /dev/null +++ b/apps/backend/runners/github/services/io_utils.py @@ -0,0 +1,14 @@ +""" +I/O Utilities for GitHub Services +================================= + +This module re-exports safe I/O utilities from core.io_utils for +backwards compatibility. New code should import directly from core.io_utils. +""" + +from __future__ import annotations + +# Re-export from core for backwards compatibility +from core.io_utils import is_pipe_broken, reset_pipe_state, safe_print + +__all__ = ["safe_print", "is_pipe_broken", "reset_pipe_state"] diff --git a/apps/backend/runners/github/services/parallel_followup_reviewer.py b/apps/backend/runners/github/services/parallel_followup_reviewer.py index bbc23a1c8c..84e4c8f77f 100644 --- a/apps/backend/runners/github/services/parallel_followup_reviewer.py +++ b/apps/backend/runners/github/services/parallel_followup_reviewer.py @@ -44,6 +44,7 @@ ReviewSeverity, ) from .category_utils import map_category + from .io_utils import safe_print from .pr_worktree_manager import PRWorktreeManager from .pydantic_models import ParallelFollowupResponse from .sdk_utils import process_sdk_stream @@ -62,6 +63,7 @@ ) from phase_config import get_thinking_budget from services.category_utils import map_category + from services.io_utils import safe_print from services.pr_worktree_manager import PRWorktreeManager from services.pydantic_models import ParallelFollowupResponse from services.sdk_utils import process_sdk_stream @@ -456,7 +458,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: if head_sha and _validate_git_ref(head_sha): try: if DEBUG_MODE: - print( + safe_print( f"[Followup] DEBUG: Creating worktree for head_sha={head_sha}", flush=True, ) @@ -464,13 +466,13 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: head_sha, context.pr_number ) project_root = worktree_path - print( + safe_print( f"[Followup] Using worktree at {worktree_path.name} for PR review", flush=True, ) except Exception as e: if DEBUG_MODE: - print( + safe_print( f"[Followup] DEBUG: Worktree creation FAILED: {e}", flush=True, ) @@ -520,7 +522,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: async with client: await client.query(prompt) - print( + safe_print( f"[ParallelFollowup] Running orchestrator ({model})...", flush=True, ) @@ -572,7 +574,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: logger.info( f"[ParallelFollowup] Session complete. Agents invoked: {final_agents}" ) - print( + safe_print( f"[ParallelFollowup] Complete. Agents invoked: {final_agents}", flush=True, ) @@ -601,7 +603,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: "Blocked: PR has merge conflicts with base branch. " "Resolve conflicts before merge." ) - print( + safe_print( "[ParallelFollowup] ⚠️ PR has merge conflicts - blocking merge", flush=True, ) @@ -616,7 +618,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: ): verdict = MergeVerdict.NEEDS_REVISION verdict_reasoning = BRANCH_BEHIND_REASONING - print( + safe_print( "[ParallelFollowup] ⚠️ PR branch is behind base - needs update", flush=True, ) @@ -711,7 +713,7 @@ async def review(self, context: FollowupReviewContext) -> PRReviewResult: except Exception as e: logger.error(f"[ParallelFollowup] Review failed: {e}", exc_info=True) - print(f"[ParallelFollowup] Error: {e}", flush=True) + safe_print(f"[ParallelFollowup] Error: {e}") return PRReviewResult( pr_number=context.pr_number, @@ -742,12 +744,12 @@ def _parse_structured_output( # Log agents from structured output agents_from_output = response.agents_invoked or [] if agents_from_output: - print( + safe_print( f"[ParallelFollowup] Specialist agents invoked: {', '.join(agents_from_output)}", flush=True, ) for agent in agents_from_output: - print(f"[Agent:{agent}] Analysis complete", flush=True) + safe_print(f"[Agent:{agent}] Analysis complete") findings = [] resolved_ids = [] @@ -762,7 +764,7 @@ def _parse_structured_output( validation_map[fv.finding_id] = fv if fv.validation_status == "dismissed_false_positive": dismissed_ids.append(fv.finding_id) - print( + safe_print( f"[ParallelFollowup] Finding {fv.finding_id} DISMISSED as false positive: {fv.explanation[:100]}", flush=True, ) @@ -774,7 +776,7 @@ def _parse_structured_output( # Check if finding was validated and dismissed as false positive if rv.finding_id in dismissed_ids: # Finding-validator determined this was a false positive - skip it - print( + safe_print( f"[ParallelFollowup] Skipping {rv.finding_id} - dismissed as false positive by finding-validator", flush=True, ) @@ -887,27 +889,27 @@ def _parse_structured_output( ) # Log findings summary for verification - print( + safe_print( f"[ParallelFollowup] Parsed {len(findings)} findings, " f"{len(resolved_ids)} resolved, {len(unresolved_ids)} unresolved, " f"{len(new_finding_ids)} new", flush=True, ) if dismissed_ids: - print( + safe_print( f"[ParallelFollowup] Validation: {len(dismissed_ids)} findings dismissed as false positives, " f"{confirmed_valid_count} confirmed valid, {needs_human_count} need human review", flush=True, ) if findings: - print("[ParallelFollowup] Findings summary:", flush=True) + safe_print("[ParallelFollowup] Findings summary:") for i, f in enumerate(findings, 1): validation_note = "" if f.validation_status == "confirmed_valid": validation_note = " [VALIDATED]" elif f.validation_status == "needs_human_review": validation_note = " [NEEDS HUMAN REVIEW]" - print( + safe_print( f" [{f.severity.value.upper()}] {i}. {f.title} ({f.file}:{f.line}){validation_note}", flush=True, ) diff --git a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py index 254f5087fd..1d88a1ff80 100644 --- a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py +++ b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py @@ -40,6 +40,7 @@ ReviewSeverity, ) from .category_utils import map_category + from .io_utils import safe_print from .pr_worktree_manager import PRWorktreeManager from .pydantic_models import ParallelOrchestratorResponse from .sdk_utils import process_sdk_stream @@ -58,6 +59,7 @@ ) from phase_config import get_thinking_budget from services.category_utils import map_category + from services.io_utils import safe_print from services.pr_worktree_manager import PRWorktreeManager from services.pydantic_models import ParallelOrchestratorResponse from services.sdk_utils import process_sdk_stream @@ -380,12 +382,12 @@ def _log_agents_invoked(self, agents: list[str]) -> None: agents: List of agent names that were invoked """ if agents: - print( + safe_print( f"[ParallelOrchestrator] Specialist agents invoked: {', '.join(agents)}", flush=True, ) for agent in agents: - print(f"[Agent:{agent}] Analysis complete", flush=True) + safe_print(f"[Agent:{agent}] Analysis complete") def _log_findings_summary(self, findings: list[PRReviewFinding]) -> None: """Log findings summary for verification. @@ -394,13 +396,13 @@ def _log_findings_summary(self, findings: list[PRReviewFinding]) -> None: findings: List of findings to summarize """ if findings: - print( + safe_print( f"[ParallelOrchestrator] Parsed {len(findings)} findings from structured output", flush=True, ) - print("[ParallelOrchestrator] Findings summary:", flush=True) + safe_print("[ParallelOrchestrator] Findings summary:") for i, f in enumerate(findings, 1): - print( + safe_print( f" [{f.severity.value.upper()}] {i}. {f.title} ({f.file}:{f.line})", flush=True, ) @@ -474,15 +476,15 @@ async def review(self, context: PRContext) -> PRReviewResult: head_sha = context.head_sha or context.head_branch if DEBUG_MODE: - print( + safe_print( f"[PRReview] DEBUG: context.head_sha='{context.head_sha}'", flush=True, ) - print( + safe_print( f"[PRReview] DEBUG: context.head_branch='{context.head_branch}'", flush=True, ) - print(f"[PRReview] DEBUG: resolved head_sha='{head_sha}'", flush=True) + safe_print(f"[PRReview] DEBUG: resolved head_sha='{head_sha}'") # SECURITY: Validate the resolved head_sha (whether SHA or branch name) # This catches invalid refs early before subprocess calls @@ -495,7 +497,7 @@ async def review(self, context: PRContext) -> PRReviewResult: if not head_sha: if DEBUG_MODE: - print("[PRReview] DEBUG: No head_sha - using fallback", flush=True) + safe_print("[PRReview] DEBUG: No head_sha - using fallback") logger.warning( "[ParallelOrchestrator] No head_sha available, using current checkout" ) @@ -507,7 +509,7 @@ async def review(self, context: PRContext) -> PRReviewResult: ) else: if DEBUG_MODE: - print( + safe_print( f"[PRReview] DEBUG: Creating worktree for head_sha={head_sha}", flush=True, ) @@ -517,14 +519,14 @@ async def review(self, context: PRContext) -> PRReviewResult: ) project_root = worktree_path if DEBUG_MODE: - print( + safe_print( f"[PRReview] DEBUG: Using worktree as " f"project_root={project_root}", flush=True, ) except (RuntimeError, ValueError) as e: if DEBUG_MODE: - print( + safe_print( f"[PRReview] DEBUG: Worktree creation FAILED: {e}", flush=True, ) @@ -564,7 +566,7 @@ async def review(self, context: PRContext) -> PRReviewResult: async with client: await client.query(prompt) - print( + safe_print( f"[ParallelOrchestrator] Running orchestrator ({model})...", flush=True, ) @@ -608,7 +610,7 @@ async def review(self, context: PRContext) -> PRReviewResult: logger.info( f"[ParallelOrchestrator] Session complete. Agents invoked: {final_agents}" ) - print( + safe_print( f"[ParallelOrchestrator] Complete. Agents invoked: {final_agents}", flush=True, ) diff --git a/apps/backend/runners/github/services/pr_review_engine.py b/apps/backend/runners/github/services/pr_review_engine.py index d8832539e7..f68a8ec3ad 100644 --- a/apps/backend/runners/github/services/pr_review_engine.py +++ b/apps/backend/runners/github/services/pr_review_engine.py @@ -21,6 +21,7 @@ ReviewPass, StructuralIssue, ) + from .io_utils import safe_print from .prompt_manager import PromptManager from .response_parsers import ResponseParser except (ImportError, ValueError, SystemError): @@ -32,6 +33,7 @@ ReviewPass, StructuralIssue, ) + from services.io_utils import safe_print from services.prompt_manager import PromptManager from services.response_parsers import ResponseParser @@ -80,19 +82,19 @@ def needs_deep_analysis(self, scan_result: dict, context: PRContext) -> bool: total_changes = context.total_additions + context.total_deletions if total_changes > 200: - print( + safe_print( f"[AI] Deep analysis needed: {total_changes} lines changed", flush=True ) return True complexity = scan_result.get("complexity", "low") if complexity in ["high", "medium"]: - print(f"[AI] Deep analysis needed: {complexity} complexity", flush=True) + safe_print(f"[AI] Deep analysis needed: {complexity} complexity") return True risk_areas = scan_result.get("risk_areas", []) if risk_areas: - print( + safe_print( f"[AI] Deep analysis needed: {len(risk_areas)} risk areas", flush=True ) return True @@ -111,7 +113,7 @@ def deduplicate_findings( seen.add(key) unique.append(f) else: - print( + safe_print( f"[AI] Skipping duplicate finding: {f.file}:{f.line} - {f.title}", flush=True, ) @@ -171,7 +173,7 @@ async def run_review_pass( # If diff is empty/truncated, build composite from individual file patches if context.diff_truncated or not context.diff: - print( + safe_print( f"[AI] Building composite diff from {len(context.changed_files)} file patches...", flush=True, ) @@ -260,7 +262,7 @@ async def run_review_pass( error_msg = f"Review pass {review_pass.value} failed: {e}" logger.error(error_msg) logger.error(f"Traceback: {traceback.format_exc()}") - print(f"[AI] ERROR: {error_msg}", flush=True) + safe_print(f"[AI] ERROR: {error_msg}") # Re-raise to allow caller to handle or track partial failures raise RuntimeError(error_msg) from e @@ -281,7 +283,7 @@ async def run_multi_pass_review( """ # Use parallel orchestrator with SDK subagents if enabled if self.config.use_parallel_orchestrator: - print( + safe_print( "[AI] Using parallel orchestrator PR review (SDK subagents)...", flush=True, ) @@ -303,7 +305,7 @@ async def run_multi_pass_review( result = await orchestrator.review(context) - print( + safe_print( f"[PR Review Engine] Parallel orchestrator returned {len(result.findings)} findings", flush=True, ) @@ -322,7 +324,7 @@ async def run_multi_pass_review( ai_triages = [] # Pass 1: Quick Scan (must run first - determines if deep analysis needed) - print("[AI] Pass 1/6: Quick Scan - Understanding scope...", flush=True) + safe_print("[AI] Pass 1/6: Quick Scan - Understanding scope...") self._report_progress( "analyzing", 35, @@ -339,7 +341,7 @@ async def run_multi_pass_review( parallel_tasks = [] task_names = [] - print("[AI] Running passes 2-6 in parallel...", flush=True) + safe_print("[AI] Running passes 2-6 in parallel...") self._report_progress( "analyzing", 50, @@ -348,50 +350,50 @@ async def run_multi_pass_review( ) async def run_security_pass(): - print( + safe_print( "[AI] Pass 2/6: Security Review - Analyzing vulnerabilities...", flush=True, ) findings = await self.run_review_pass(ReviewPass.SECURITY, context) - print(f"[AI] Security pass complete: {len(findings)} findings", flush=True) + safe_print(f"[AI] Security pass complete: {len(findings)} findings") return ("security", findings) async def run_quality_pass(): - print( + safe_print( "[AI] Pass 3/6: Quality Review - Checking code quality...", flush=True ) findings = await self.run_review_pass(ReviewPass.QUALITY, context) - print(f"[AI] Quality pass complete: {len(findings)} findings", flush=True) + safe_print(f"[AI] Quality pass complete: {len(findings)} findings") return ("quality", findings) async def run_structural_pass(): - print( + safe_print( "[AI] Pass 4/6: Structural Review - Checking for feature creep...", flush=True, ) result_text = await self._run_structural_pass(context) issues = self.parser.parse_structural_issues(result_text) - print(f"[AI] Structural pass complete: {len(issues)} issues", flush=True) + safe_print(f"[AI] Structural pass complete: {len(issues)} issues") return ("structural", issues) async def run_ai_triage_pass(): - print( + safe_print( "[AI] Pass 5/6: AI Comment Triage - Verifying other AI comments...", flush=True, ) result_text = await self._run_ai_triage_pass(context) triages = self.parser.parse_ai_comment_triages(result_text) - print( + safe_print( f"[AI] AI triage complete: {len(triages)} comments triaged", flush=True ) return ("ai_triage", triages) async def run_deep_pass(): - print( + safe_print( "[AI] Pass 6/6: Deep Analysis - Reviewing business logic...", flush=True ) findings = await self.run_review_pass(ReviewPass.DEEP_ANALYSIS, context) - print(f"[AI] Deep analysis complete: {len(findings)} findings", flush=True) + safe_print(f"[AI] Deep analysis complete: {len(findings)} findings") return ("deep", findings) # Always run security, quality, structural @@ -408,22 +410,22 @@ async def run_deep_pass(): if has_ai_comments: parallel_tasks.append(run_ai_triage_pass()) task_names.append("AI Triage") - print( + safe_print( f"[AI] Found {len(context.ai_bot_comments)} AI comments to triage", flush=True, ) else: - print("[AI] Pass 5/6: Skipped (no AI comments to triage)", flush=True) + safe_print("[AI] Pass 5/6: Skipped (no AI comments to triage)") # Only run deep analysis if needed if needs_deep: parallel_tasks.append(run_deep_pass()) task_names.append("Deep Analysis") else: - print("[AI] Pass 6/6: Skipped (changes not complex enough)", flush=True) + safe_print("[AI] Pass 6/6: Skipped (changes not complex enough)") # Run all passes in parallel - print( + safe_print( f"[AI] Executing {len(parallel_tasks)} passes in parallel: {', '.join(task_names)}", flush=True, ) @@ -432,7 +434,7 @@ async def run_deep_pass(): # Collect results from all parallel passes for i, result in enumerate(results): if isinstance(result, Exception): - print(f"[AI] Pass '{task_names[i]}' failed: {result}", flush=True) + safe_print(f"[AI] Pass '{task_names[i]}' failed: {result}") elif isinstance(result, tuple): pass_type, data = result if pass_type in ("security", "quality", "deep"): @@ -450,12 +452,12 @@ async def run_deep_pass(): ) # Deduplicate findings - print( + safe_print( f"[AI] Deduplicating {len(all_findings)} findings from all passes...", flush=True, ) unique_findings = self.deduplicate_findings(all_findings) - print( + safe_print( f"[AI] Multi-pass review complete: {len(unique_findings)} findings, " f"{len(structural_issues)} structural issues, {len(ai_triages)} AI triages", flush=True, @@ -509,7 +511,7 @@ async def _run_structural_pass(self, context: PRContext) -> str: if block_type == "TextBlock" and hasattr(block, "text"): result_text += block.text except Exception as e: - print(f"[AI] Structural pass error: {e}", flush=True) + safe_print(f"[AI] Structural pass error: {e}") return result_text @@ -567,7 +569,7 @@ async def _run_ai_triage_pass(self, context: PRContext) -> str: if block_type == "TextBlock" and hasattr(block, "text"): result_text += block.text except Exception as e: - print(f"[AI] AI triage pass error: {e}", flush=True) + safe_print(f"[AI] AI triage pass error: {e}") return result_text diff --git a/apps/backend/runners/github/services/response_parsers.py b/apps/backend/runners/github/services/response_parsers.py index 2df83ea06b..c0b31e87c4 100644 --- a/apps/backend/runners/github/services/response_parsers.py +++ b/apps/backend/runners/github/services/response_parsers.py @@ -21,6 +21,7 @@ TriageCategory, TriageResult, ) + from .io_utils import safe_print except (ImportError, ValueError, SystemError): from models import ( AICommentTriage, @@ -32,6 +33,7 @@ TriageCategory, TriageResult, ) + from services.io_utils import safe_print # Evidence-based validation replaces confidence scoring # Findings without evidence are filtered out instead of using confidence thresholds @@ -57,10 +59,10 @@ def parse_scan_result(response_text: str) -> dict: ) if json_match: result = json.loads(json_match.group(1)) - print(f"[AI] Quick scan result: {result}", flush=True) + safe_print(f"[AI] Quick scan result: {result}") return result except (json.JSONDecodeError, ValueError) as e: - print(f"[AI] Failed to parse scan result: {e}", flush=True) + safe_print(f"[AI] Failed to parse scan result: {e}") return default_result @@ -87,7 +89,7 @@ def parse_review_findings( # Apply evidence-based validation if require_evidence and len(evidence.strip()) < MIN_EVIDENCE_LENGTH: - print( + safe_print( f"[AI] Dropped finding '{f.get('title', 'unknown')}': " f"insufficient evidence ({len(evidence.strip())} chars < {MIN_EVIDENCE_LENGTH})", flush=True, @@ -117,7 +119,7 @@ def parse_review_findings( ) ) except (json.JSONDecodeError, KeyError, ValueError) as e: - print(f"Failed to parse findings: {e}") + safe_print(f"Failed to parse findings: {e}") return findings @@ -147,7 +149,7 @@ def parse_structural_issues(response_text: str) -> list[StructuralIssue]: ) ) except (json.JSONDecodeError, KeyError, ValueError) as e: - print(f"Failed to parse structural issues: {e}") + safe_print(f"Failed to parse structural issues: {e}") return issues @@ -180,7 +182,7 @@ def parse_ai_comment_triages(response_text: str) -> list[AICommentTriage]: ) ) except (json.JSONDecodeError, KeyError, ValueError) as e: - print(f"Failed to parse AI comment triages: {e}") + safe_print(f"Failed to parse AI comment triages: {e}") return triages @@ -218,6 +220,6 @@ def parse_triage_result(issue: dict, response_text: str, repo: str) -> TriageRes result.comment = data.get("comment") except (json.JSONDecodeError, KeyError, ValueError) as e: - print(f"Failed to parse triage result: {e}") + safe_print(f"Failed to parse triage result: {e}") return result diff --git a/apps/backend/runners/github/services/sdk_utils.py b/apps/backend/runners/github/services/sdk_utils.py index 7471f16360..c6ac7d8970 100644 --- a/apps/backend/runners/github/services/sdk_utils.py +++ b/apps/backend/runners/github/services/sdk_utils.py @@ -15,6 +15,11 @@ from collections.abc import Callable from typing import Any +try: + from .io_utils import safe_print +except (ImportError, ValueError, SystemError): + from core.io_utils import safe_print + logger = logging.getLogger(__name__) # Check if debug mode is enabled @@ -66,9 +71,9 @@ async def process_sdk_stream( # Track subagent tool IDs to log their results subagent_tool_ids: dict[str, str] = {} # tool_id -> agent_name - print(f"[{context_name}] Processing SDK stream...", flush=True) + safe_print(f"[{context_name}] Processing SDK stream...") if DEBUG_MODE: - print(f"[DEBUG {context_name}] Awaiting response stream...", flush=True) + safe_print(f"[DEBUG {context_name}] Awaiting response stream...") try: async for msg in client.receive_response(): @@ -81,9 +86,8 @@ async def process_sdk_stream( msg_details = "" if hasattr(msg, "type"): msg_details = f" (type={msg.type})" - print( - f"[DEBUG {context_name}] Message #{msg_count}: {msg_type}{msg_details}", - flush=True, + safe_print( + f"[DEBUG {context_name}] Message #{msg_count}: {msg_type}{msg_details}" ) # Track thinking blocks @@ -94,16 +98,14 @@ async def process_sdk_stream( msg, "text", "" ) if thinking_text: - print( - f"[{context_name}] AI thinking: {len(thinking_text)} chars", - flush=True, + safe_print( + f"[{context_name}] AI thinking: {len(thinking_text)} chars" ) if DEBUG_MODE: # Show first 200 chars of thinking preview = thinking_text[:200].replace("\n", " ") - print( - f"[DEBUG {context_name}] Thinking preview: {preview}...", - flush=True, + safe_print( + f"[DEBUG {context_name}] Thinking preview: {preview}..." ) # Invoke callback if on_thinking: @@ -118,9 +120,8 @@ async def process_sdk_stream( tool_input = getattr(msg, "input", {}) if DEBUG_MODE: - print( - f"[DEBUG {context_name}] Tool call: {tool_name} (id={tool_id})", - flush=True, + safe_print( + f"[DEBUG {context_name}] Tool call: {tool_name} (id={tool_id})" ) if tool_name == "Task": @@ -129,9 +130,7 @@ async def process_sdk_stream( agents_invoked.append(agent_name) # Track this tool ID to log its result later subagent_tool_ids[tool_id] = agent_name - print( - f"[{context_name}] Invoked agent: {agent_name}", flush=True - ) + safe_print(f"[{context_name}] Invoked agent: {agent_name}") elif tool_name == "StructuredOutput": if tool_input: # Warn if overwriting existing structured output @@ -141,19 +140,13 @@ async def process_sdk_stream( f"overwriting previous output" ) structured_output = tool_input - print( - f"[{context_name}] Received structured output", - flush=True, - ) + safe_print(f"[{context_name}] Received structured output") # Invoke callback if on_structured_output: on_structured_output(tool_input) elif DEBUG_MODE: # Log other tool calls in debug mode - print( - f"[DEBUG {context_name}] Other tool: {tool_name}", - flush=True, - ) + safe_print(f"[DEBUG {context_name}] Other tool: {tool_name}") # Invoke callback for all tool uses if on_tool_use: @@ -180,15 +173,13 @@ async def process_sdk_stream( result_preview = ( str(result_content)[:600].replace("\n", " ").strip() ) - print( - f"[Agent:{agent_name}] {status}: {result_preview}{'...' if len(str(result_content)) > 600 else ''}", - flush=True, + safe_print( + f"[Agent:{agent_name}] {status}: {result_preview}{'...' if len(str(result_content)) > 600 else ''}" ) elif DEBUG_MODE: status = "ERROR" if is_error else "OK" - print( - f"[DEBUG {context_name}] Tool result: {tool_id} [{status}]", - flush=True, + safe_print( + f"[DEBUG {context_name}] Tool result: {tool_id} [{status}]" ) # Invoke callback @@ -214,9 +205,8 @@ async def process_sdk_stream( if agent_name not in agents_invoked: agents_invoked.append(agent_name) subagent_tool_ids[tool_id] = agent_name - print( - f"[{context_name}] Invoking agent: {agent_name}", - flush=True, + safe_print( + f"[{context_name}] Invoking agent: {agent_name}" ) elif tool_name == "StructuredOutput": if tool_input: @@ -242,9 +232,8 @@ async def process_sdk_stream( # Always print text content preview (not just in DEBUG_MODE) text_preview = block.text[:500].replace("\n", " ").strip() if text_preview: - print( - f"[{context_name}] AI response: {text_preview}{'...' if len(block.text) > 500 else ''}", - flush=True, + safe_print( + f"[{context_name}] AI response: {text_preview}{'...' if len(block.text) > 500 else ''}" ) # Invoke callback if on_text: @@ -304,9 +293,8 @@ async def process_sdk_stream( result_preview = ( str(result_content)[:600].replace("\n", " ").strip() ) - print( - f"[Agent:{agent_name}] {status}: {result_preview}{'...' if len(str(result_content)) > 600 else ''}", - flush=True, + safe_print( + f"[Agent:{agent_name}] {status}: {result_preview}{'...' if len(str(result_content)) > 600 else ''}" ) # Invoke callback @@ -319,25 +307,25 @@ async def process_sdk_stream( f"[{context_name}] Error processing message #{msg_count}: {msg_error}" ) if DEBUG_MODE: - print( - f"[DEBUG {context_name}] Message processing error: {msg_error}", - flush=True, + safe_print( + f"[DEBUG {context_name}] Message processing error: {msg_error}" ) # Continue processing subsequent messages + except BrokenPipeError: + # Pipe closed by parent process - expected during shutdown + stream_error = "Output pipe closed" + logger.debug(f"[{context_name}] Output pipe closed by parent process") except Exception as e: # Log stream-level errors stream_error = str(e) logger.error(f"[{context_name}] SDK stream processing failed: {e}") - print(f"[{context_name}] ERROR: Stream processing failed: {e}", flush=True) + safe_print(f"[{context_name}] ERROR: Stream processing failed: {e}") if DEBUG_MODE: - print( - f"[DEBUG {context_name}] Session ended. Total messages: {msg_count}", - flush=True, - ) + safe_print(f"[DEBUG {context_name}] Session ended. Total messages: {msg_count}") - print(f"[{context_name}] Session ended. Total messages: {msg_count}", flush=True) + safe_print(f"[{context_name}] Session ended. Total messages: {msg_count}") return { "result_text": result_text, diff --git a/apps/backend/runners/gitlab/orchestrator.py b/apps/backend/runners/gitlab/orchestrator.py index bc33aa20a4..088ecca8ca 100644 --- a/apps/backend/runners/gitlab/orchestrator.py +++ b/apps/backend/runners/gitlab/orchestrator.py @@ -36,6 +36,17 @@ ) from services import MRReviewEngine +# Import safe_print for BrokenPipeError handling +try: + from core.io_utils import safe_print +except ImportError: + # Fallback for direct script execution + import sys + from pathlib import Path + + sys.path.insert(0, str(Path(__file__).parent.parent.parent)) + from core.io_utils import safe_print + @dataclass class ProgressCallback: @@ -121,7 +132,7 @@ def _forward_progress(self, callback) -> None: async def _gather_mr_context(self, mr_iid: int) -> MRContext: """Gather context for an MR.""" - print(f"[GitLab] Fetching MR !{mr_iid} data...", flush=True) + safe_print(f"[GitLab] Fetching MR !{mr_iid} data...") # Get MR details mr_data = self.client.get_mr(mr_iid) @@ -187,7 +198,7 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: Returns: MRReviewResult with findings and overall assessment """ - print(f"[GitLab] Starting review for MR !{mr_iid}", flush=True) + safe_print(f"[GitLab] Starting review for MR !{mr_iid}") self._report_progress( "gathering_context", @@ -199,10 +210,9 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: try: # Gather MR context context = await self._gather_mr_context(mr_iid) - print( + safe_print( f"[GitLab] Context gathered: {context.title} " - f"({len(context.changed_files)} files, {context.total_additions}+/{context.total_deletions}-)", - flush=True, + f"({len(context.changed_files)} files, {context.total_additions}+/{context.total_deletions}-)" ) self._report_progress( @@ -213,7 +223,7 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: findings, verdict, summary, blockers = await self.review_engine.run_review( context ) - print(f"[GitLab] Review complete: {len(findings)} findings", flush=True) + safe_print(f"[GitLab] Review complete: {len(findings)} findings") # Map verdict to overall_status if verdict == MergeVerdict.BLOCKED: @@ -264,7 +274,7 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: error_msg = f"MR !{mr_iid} not found in GitLab." elif e.code == 429: error_msg = "GitLab rate limit exceeded. Please try again later." - print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}", flush=True) + safe_print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}") result = MRReviewResult( mr_iid=mr_iid, project=self.config.project, @@ -276,7 +286,7 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: except json.JSONDecodeError as e: error_msg = f"Invalid JSON response from GitLab: {e}" - print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}", flush=True) + safe_print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}") result = MRReviewResult( mr_iid=mr_iid, project=self.config.project, @@ -288,7 +298,7 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: except OSError as e: error_msg = f"File system error: {e}" - print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}", flush=True) + safe_print(f"[GitLab] Review failed for !{mr_iid}: {error_msg}") result = MRReviewResult( mr_iid=mr_iid, project=self.config.project, @@ -302,8 +312,8 @@ async def review_mr(self, mr_iid: int) -> MRReviewResult: # Catch-all for unexpected errors, with full traceback for debugging error_details = f"{type(e).__name__}: {e}" full_traceback = traceback.format_exc() - print(f"[GitLab] Review failed for !{mr_iid}: {error_details}", flush=True) - print(f"[GitLab] Traceback:\n{full_traceback}", flush=True) + safe_print(f"[GitLab] Review failed for !{mr_iid}: {error_details}") + safe_print(f"[GitLab] Traceback:\n{full_traceback}") result = MRReviewResult( mr_iid=mr_iid, @@ -326,7 +336,7 @@ async def followup_review_mr(self, mr_iid: int) -> MRReviewResult: Returns: MRReviewResult with follow-up analysis """ - print(f"[GitLab] Starting follow-up review for MR !{mr_iid}", flush=True) + safe_print(f"[GitLab] Starting follow-up review for MR !{mr_iid}") # Load previous review previous_review = MRReviewResult.load(self.gitlab_dir, mr_iid) diff --git a/apps/backend/runners/gitlab/runner.py b/apps/backend/runners/gitlab/runner.py index d4f61827bb..a24b284c0f 100644 --- a/apps/backend/runners/gitlab/runner.py +++ b/apps/backend/runners/gitlab/runner.py @@ -38,6 +38,7 @@ # Add gitlab runner directory to path for direct imports sys.path.insert(0, str(Path(__file__).parent)) +from core.io_utils import safe_print from models import GitLabRunnerConfig from orchestrator import GitLabOrchestrator, ProgressCallback @@ -48,7 +49,7 @@ def print_progress(callback: ProgressCallback) -> None: if callback.mr_iid: prefix = f"[MR !{callback.mr_iid}] " - print(f"{prefix}[{callback.progress:3d}%] {callback.message}", flush=True) + safe_print(f"{prefix}[{callback.progress:3d}%] {callback.message}") def get_config(args) -> GitLabRunnerConfig: @@ -122,27 +123,24 @@ async def cmd_review_mr(args) -> int: sys.stdout.reconfigure(line_buffering=True) sys.stderr.reconfigure(line_buffering=True) - print(f"[DEBUG] Starting MR review for MR !{args.mr_iid}", flush=True) - print(f"[DEBUG] Project directory: {args.project_dir}", flush=True) + safe_print(f"[DEBUG] Starting MR review for MR !{args.mr_iid}") + safe_print(f"[DEBUG] Project directory: {args.project_dir}") - print("[DEBUG] Building config...", flush=True) + safe_print("[DEBUG] Building config...") config = get_config(args) - print( - f"[DEBUG] Config built: project={config.project}, model={config.model}", - flush=True, - ) + safe_print(f"[DEBUG] Config built: project={config.project}, model={config.model}") - print("[DEBUG] Creating orchestrator...", flush=True) + safe_print("[DEBUG] Creating orchestrator...") orchestrator = GitLabOrchestrator( project_dir=args.project_dir, config=config, progress_callback=print_progress, ) - print("[DEBUG] Orchestrator created", flush=True) + safe_print("[DEBUG] Orchestrator created") - print(f"[DEBUG] Calling orchestrator.review_mr({args.mr_iid})...", flush=True) + safe_print(f"[DEBUG] Calling orchestrator.review_mr({args.mr_iid})...") result = await orchestrator.review_mr(args.mr_iid) - print(f"[DEBUG] review_mr returned, success={result.success}", flush=True) + safe_print(f"[DEBUG] review_mr returned, success={result.success}") if result.success: print(f"\n{'=' * 60}") @@ -174,27 +172,22 @@ async def cmd_followup_review_mr(args) -> int: sys.stdout.reconfigure(line_buffering=True) sys.stderr.reconfigure(line_buffering=True) - print(f"[DEBUG] Starting follow-up review for MR !{args.mr_iid}", flush=True) - print(f"[DEBUG] Project directory: {args.project_dir}", flush=True) + safe_print(f"[DEBUG] Starting follow-up review for MR !{args.mr_iid}") + safe_print(f"[DEBUG] Project directory: {args.project_dir}") - print("[DEBUG] Building config...", flush=True) + safe_print("[DEBUG] Building config...") config = get_config(args) - print( - f"[DEBUG] Config built: project={config.project}, model={config.model}", - flush=True, - ) + safe_print(f"[DEBUG] Config built: project={config.project}, model={config.model}") - print("[DEBUG] Creating orchestrator...", flush=True) + safe_print("[DEBUG] Creating orchestrator...") orchestrator = GitLabOrchestrator( project_dir=args.project_dir, config=config, progress_callback=print_progress, ) - print("[DEBUG] Orchestrator created", flush=True) + safe_print("[DEBUG] Orchestrator created") - print( - f"[DEBUG] Calling orchestrator.followup_review_mr({args.mr_iid})...", flush=True - ) + safe_print(f"[DEBUG] Calling orchestrator.followup_review_mr({args.mr_iid})...") try: result = await orchestrator.followup_review_mr(args.mr_iid) @@ -202,7 +195,7 @@ async def cmd_followup_review_mr(args) -> int: print(f"\nFollow-up review failed: {e}") return 1 - print(f"[DEBUG] followup_review_mr returned, success={result.success}", flush=True) + safe_print(f"[DEBUG] followup_review_mr returned, success={result.success}") if result.success: print(f"\n{'=' * 60}") diff --git a/apps/backend/runners/gitlab/services/mr_review_engine.py b/apps/backend/runners/gitlab/services/mr_review_engine.py index ef8ef9aaf0..0a8d518a4b 100644 --- a/apps/backend/runners/gitlab/services/mr_review_engine.py +++ b/apps/backend/runners/gitlab/services/mr_review_engine.py @@ -34,6 +34,17 @@ ReviewSeverity, ) +# Import safe_print for BrokenPipeError handling +try: + from core.io_utils import safe_print +except ImportError: + # Fallback for direct script execution + import sys + from pathlib import Path as PathLib + + sys.path.insert(0, str(PathLib(__file__).parent.parent.parent.parent)) + from core.io_utils import safe_print + @dataclass class ProgressCallback: @@ -246,7 +257,7 @@ async def run_review( return self._parse_review_result(result_text) except Exception as e: - print(f"[AI] Review error: {e}", flush=True) + safe_print(f"[AI] Review error: {e}") raise RuntimeError(f"Review failed: {e}") from e def _parse_review_result( @@ -297,14 +308,11 @@ def _parse_review_result( f"{finding.title} ({finding.file}:{finding.line})" ) except (ValueError, KeyError) as e: - print(f"[AI] Skipping invalid finding: {e}", flush=True) + safe_print(f"[AI] Skipping invalid finding: {e}") except json.JSONDecodeError as e: - print(f"[AI] Failed to parse JSON: {e}", flush=True) - print( - f"[AI] Raw response (first 500 chars): {result_text[:500]}", - flush=True, - ) + safe_print(f"[AI] Failed to parse JSON: {e}") + safe_print(f"[AI] Raw response (first 500 chars): {result_text[:500]}") summary = "Review completed but failed to parse structured output. Please re-run the review." # Return with empty findings but keep verdict as READY_TO_MERGE # since we couldn't determine if there are actual issues diff --git a/apps/backend/runners/spec_runner.py b/apps/backend/runners/spec_runner.py index 30adbf3fa6..39868daf50 100644 --- a/apps/backend/runners/spec_runner.py +++ b/apps/backend/runners/spec_runner.py @@ -93,6 +93,11 @@ elif dev_env_file.exists(): load_dotenv(dev_env_file) +# Initialize Sentry early to capture any startup errors +from core.sentry import capture_exception, init_sentry + +init_sentry(component="spec-runner") + from debug import debug, debug_error, debug_section, debug_success from phase_config import resolve_model_id from review import ReviewState @@ -370,6 +375,14 @@ def main(): f"To continue: python auto-claude/spec_runner.py --continue {orchestrator.spec_dir.name}" ) sys.exit(1) + except Exception as e: + # Capture unexpected errors to Sentry + capture_exception( + e, spec_dir=str(orchestrator.spec_dir) if orchestrator else None + ) + debug_error("spec_runner", f"Unexpected error: {e}") + print(f"\n\nUnexpected error: {e}") + sys.exit(1) if __name__ == "__main__": diff --git a/apps/backend/security/__init__.py b/apps/backend/security/__init__.py index b26311d292..a8b02c032c 100644 --- a/apps/backend/security/__init__.py +++ b/apps/backend/security/__init__.py @@ -59,6 +59,7 @@ # Validators (for advanced usage) from .validator import ( VALIDATORS, + validate_bash_command, validate_chmod_command, validate_dropdb_command, validate_dropuser_command, @@ -75,6 +76,9 @@ validate_psql_command, validate_redis_cli_command, validate_rm_command, + validate_sh_command, + validate_shell_c_command, + validate_zsh_command, ) __all__ = [ @@ -98,6 +102,10 @@ "validate_git_command", "validate_git_commit", "validate_git_config", + "validate_shell_c_command", + "validate_bash_command", + "validate_sh_command", + "validate_zsh_command", "validate_dropdb_command", "validate_dropuser_command", "validate_psql_command", diff --git a/apps/backend/security/shell_validators.py b/apps/backend/security/shell_validators.py new file mode 100644 index 0000000000..f5653df2b4 --- /dev/null +++ b/apps/backend/security/shell_validators.py @@ -0,0 +1,142 @@ +""" +Shell Interpreter Validators +============================= + +Validators for shell interpreter commands (bash, sh, zsh) that execute +inline commands via the -c flag. + +This closes a security bypass where `bash -c "npm test"` could execute +arbitrary commands since `bash` is in BASE_COMMANDS but the commands +inside -c were not being validated. +""" + +import os +import shlex +from pathlib import Path + +from project_analyzer import is_command_allowed + +from .parser import extract_commands, split_command_segments +from .profile import get_security_profile +from .validation_models import ValidationResult + +# Shell interpreters that can execute nested commands +SHELL_INTERPRETERS = {"bash", "sh", "zsh"} + + +def _extract_c_argument(command_string: str) -> str | None: + """ + Extract the command string from a shell -c invocation. + + Handles various formats: + - bash -c 'command' + - bash -c "command" + - sh -c 'cmd1 && cmd2' + - zsh -c "complex command" + + Args: + command_string: The full shell command (e.g., "bash -c 'npm test'") + + Returns: + The command string after -c, or None if not a -c invocation + """ + try: + tokens = shlex.split(command_string) + except ValueError: + # Malformed command - let it fail safely + return None + + if len(tokens) < 3: + return None + + # Look for -c flag (standalone or combined with other flags like -xc, -ec, -ic) + for i, token in enumerate(tokens): + # Check for standalone -c or combined flags containing 'c' + # Combined flags: -xc, -ec, -ic, -exc, etc. (short options bundled together) + is_c_flag = token == "-c" or ( + token.startswith("-") and not token.startswith("--") and "c" in token[1:] + ) + if is_c_flag and i + 1 < len(tokens): + # The next token is the command to execute + return tokens[i + 1] + + return None + + +def validate_shell_c_command(command_string: str) -> ValidationResult: + """ + Validate commands inside bash/sh/zsh -c '...' strings. + + This prevents using shell interpreters to bypass the security allowlist. + All commands inside the -c string must also be allowed by the profile. + + Args: + command_string: The full shell command (e.g., "bash -c 'npm test'") + + Returns: + Tuple of (is_valid, error_message) + """ + # Extract the command after -c + inner_command = _extract_c_argument(command_string) + + if inner_command is None: + # Not a -c invocation (e.g., "bash script.sh") - allow it + # The script itself would need to be in allowed commands + return True, "" + + # Get the security profile for the current project + # Use PROJECT_DIR_ENV_VAR if set, otherwise use cwd + from .constants import PROJECT_DIR_ENV_VAR + + project_dir = os.environ.get(PROJECT_DIR_ENV_VAR) + if not project_dir: + project_dir = os.getcwd() + + try: + profile = get_security_profile(Path(project_dir)) + except Exception: + # If we can't get the profile, fail safe by blocking + return False, "Could not load security profile to validate shell -c command" + + # Extract command names for allowlist validation + inner_command_names = extract_commands(inner_command) + + if not inner_command_names: + # Could not parse - be permissive for empty commands + # (e.g., bash -c "" is harmless) + if not inner_command.strip(): + return True, "" + return False, f"Could not parse commands inside shell -c: {inner_command}" + + # Validate each command name against the security profile + for cmd_name in inner_command_names: + is_allowed, reason = is_command_allowed(cmd_name, profile) + if not is_allowed: + return ( + False, + f"Command '{cmd_name}' inside shell -c is not allowed: {reason}", + ) + + # Get full command segments for recursive shell validation + # (split_command_segments gives us full commands, not just names) + inner_segments = split_command_segments(inner_command) + + for segment in inner_segments: + # Check if this segment is a shell invocation that needs recursive validation + segment_commands = extract_commands(segment) + if segment_commands: + first_cmd = segment_commands[0] + # Handle paths like /bin/bash + base_cmd = first_cmd.rsplit("/", 1)[-1] if "/" in first_cmd else first_cmd + if base_cmd in SHELL_INTERPRETERS: + valid, err = validate_shell_c_command(segment) + if not valid: + return False, f"Nested shell command not allowed: {err}" + + return True, "" + + +# Alias for common shell interpreters - they all use the same validation +validate_bash_command = validate_shell_c_command +validate_sh_command = validate_shell_c_command +validate_zsh_command = validate_shell_c_command diff --git a/apps/backend/security/validator.py b/apps/backend/security/validator.py index c1ca28983a..bfbdd27dc2 100644 --- a/apps/backend/security/validator.py +++ b/apps/backend/security/validator.py @@ -43,6 +43,12 @@ validate_killall_command, validate_pkill_command, ) +from .shell_validators import ( + validate_bash_command, + validate_sh_command, + validate_shell_c_command, + validate_zsh_command, +) from .validation_models import ValidationResult, ValidatorFunction from .validator_registry import VALIDATORS, get_validator @@ -66,6 +72,11 @@ "validate_git_commit", "validate_git_command", "validate_git_config", + # Shell validators + "validate_shell_c_command", + "validate_bash_command", + "validate_sh_command", + "validate_zsh_command", # Database validators "validate_dropdb_command", "validate_dropuser_command", diff --git a/apps/backend/security/validator_registry.py b/apps/backend/security/validator_registry.py index 64ad1820a4..530c0f360b 100644 --- a/apps/backend/security/validator_registry.py +++ b/apps/backend/security/validator_registry.py @@ -25,6 +25,11 @@ validate_killall_command, validate_pkill_command, ) +from .shell_validators import ( + validate_bash_command, + validate_sh_command, + validate_zsh_command, +) from .validation_models import ValidatorFunction # Map command names to their validation functions @@ -39,6 +44,10 @@ "init.sh": validate_init_script, # Git "git": validate_git_commit, + # Shell interpreters (validate commands inside -c) + "bash": validate_bash_command, + "sh": validate_sh_command, + "zsh": validate_zsh_command, # Database - PostgreSQL "dropdb": validate_dropdb_command, "dropuser": validate_dropuser_command, diff --git a/apps/frontend/electron.vite.config.ts b/apps/frontend/electron.vite.config.ts index 5dcaaf9f4b..6ceaa51fd5 100644 --- a/apps/frontend/electron.vite.config.ts +++ b/apps/frontend/electron.vite.config.ts @@ -2,8 +2,24 @@ import { defineConfig, externalizeDepsPlugin } from 'electron-vite'; import react from '@vitejs/plugin-react'; import { resolve } from 'path'; +/** + * Sentry configuration embedded at build time. + * + * In CI builds, these come from GitHub secrets. + * In local development, these come from apps/frontend/.env (loaded by dotenv). + * + * The `define` option replaces these values at build time, so they're + * embedded in the bundle and available at runtime in packaged apps. + */ +const sentryDefines = { + '__SENTRY_DSN__': JSON.stringify(process.env.SENTRY_DSN || ''), + '__SENTRY_TRACES_SAMPLE_RATE__': JSON.stringify(process.env.SENTRY_TRACES_SAMPLE_RATE || '0.1'), + '__SENTRY_PROFILES_SAMPLE_RATE__': JSON.stringify(process.env.SENTRY_PROFILES_SAMPLE_RATE || '0.1'), +}; + export default defineConfig({ main: { + define: sentryDefines, plugins: [externalizeDepsPlugin({ // Bundle these packages into the main process (they won't be in node_modules in packaged app) exclude: [ @@ -11,7 +27,15 @@ export default defineConfig({ 'chokidar', 'kuzu', 'electron-updater', - '@electron-toolkit/utils' + '@electron-toolkit/utils', + // Sentry and its transitive dependencies (opentelemetry -> debug -> ms) + '@sentry/electron', + '@sentry/core', + '@sentry/node', + '@sentry/utils', + '@opentelemetry/instrumentation', + 'debug', + 'ms' ] })], build: { @@ -35,6 +59,7 @@ export default defineConfig({ } }, renderer: { + define: sentryDefines, root: resolve(__dirname, 'src/renderer'), build: { rollupOptions: { diff --git a/apps/frontend/package.json b/apps/frontend/package.json index 7c77a98307..a021cc74b4 100644 --- a/apps/frontend/package.json +++ b/apps/frontend/package.json @@ -1,6 +1,6 @@ { "name": "auto-claude-ui", - "version": "2.7.3", + "version": "2.7.4", "type": "module", "description": "Desktop UI for Auto Claude autonomous coding framework", "homepage": "https://github.com/AndyMik90/Auto-Claude", diff --git a/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts b/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts index 432c5f361d..1bfd92285d 100644 --- a/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts +++ b/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts @@ -148,7 +148,8 @@ describe('IPC Bridge Integration', () => { const submitReview = electronAPI['submitReview'] as ( id: string, approved: boolean, - feedback?: string + feedback?: string, + images?: unknown[] ) => Promise; await submitReview('task-id', false, 'Needs more work'); @@ -156,7 +157,8 @@ describe('IPC Bridge Integration', () => { 'task:review', 'task-id', false, - 'Needs more work' + 'Needs more work', + undefined ); }); }); diff --git a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts index f3ca37d495..cf03c5136e 100644 --- a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts +++ b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts @@ -39,11 +39,14 @@ vi.mock('child_process', async (importOriginal) => { }); // Mock claude-profile-manager to bypass auth checks in tests +const mockProfileManager = { + hasValidAuth: () => true, + getActiveProfile: () => ({ profileId: 'default', profileName: 'Default' }) +}; + vi.mock('../../main/claude-profile-manager', () => ({ - getClaudeProfileManager: () => ({ - hasValidAuth: () => true, - getActiveProfile: () => ({ profileId: 'default', profileName: 'Default' }) - }) + getClaudeProfileManager: () => mockProfileManager, + initializeClaudeProfileManager: () => Promise.resolve(mockProfileManager) })); // Mock validatePythonPath to allow test paths (security validation is tested separately) diff --git a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts index 9eef26079a..edb6af1625 100644 --- a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts +++ b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts @@ -17,10 +17,27 @@ import { } from '../cli-tool-manager'; import { findWindowsExecutableViaWhere, - findWindowsExecutableViaWhereAsync + findWindowsExecutableViaWhereAsync, + isSecurePath } from '../utils/windows-paths'; import { findExecutable, findExecutableAsync } from '../env-utils'; +type SpawnOptions = Parameters<(typeof import('../env-utils'))['getSpawnOptions']>[1]; +type MockDirent = import('fs').Dirent; + +const createDirent = (name: string, isDir: boolean): MockDirent => + ({ + name, + parentPath: '', + isDirectory: () => isDir, + isFile: () => !isDir, + isBlockDevice: () => false, + isCharacterDevice: () => false, + isSymbolicLink: () => false, + isFIFO: () => false, + isSocket: () => false + }) as unknown as MockDirent; + // Mock Electron app vi.mock('electron', () => ({ app: { @@ -52,7 +69,7 @@ vi.mock('child_process', () => { // so when tests call vi.mocked(execFileSync).mockReturnValue(), it affects execSync too const sharedSyncMock = vi.fn(); - const mockExecFile = vi.fn((cmd: any, args: any, options: any, callback: any) => { +const mockExecFile = vi.fn((cmd: unknown, args: unknown, options: unknown, callback: unknown) => { // Return a minimal ChildProcess-like object const childProcess = { stdout: { on: vi.fn() }, @@ -62,13 +79,14 @@ vi.mock('child_process', () => { // If callback is provided, call it asynchronously if (typeof callback === 'function') { - setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + const cb = callback as (error: Error | null, stdout: string, stderr: string) => void; + setImmediate(() => cb(null, 'claude-code version 1.0.0\n', '')); } - return childProcess as any; + return childProcess as unknown as import('child_process').ChildProcess; }); - const mockExec = vi.fn((cmd: any, options: any, callback: any) => { + const mockExec = vi.fn((cmd: unknown, options: unknown, callback: unknown) => { // Return a minimal ChildProcess-like object const childProcess = { stdout: { on: vi.fn() }, @@ -78,10 +96,11 @@ vi.mock('child_process', () => { // If callback is provided, call it asynchronously if (typeof callback === 'function') { - setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + const cb = callback as (error: Error | null, stdout: string, stderr: string) => void; + setImmediate(() => cb(null, 'claude-code version 1.0.0\n', '')); } - return childProcess as any; + return childProcess as unknown as import('child_process').ChildProcess; }); return { @@ -93,18 +112,23 @@ vi.mock('child_process', () => { }); // Mock env-utils to avoid PATH augmentation complexity -vi.mock('../env-utils', () => ({ +vi.mock('../env-utils', () => { + const mockShouldUseShell = vi.fn((command: string) => { + if (process.platform !== 'win32') { + return false; + } + const trimmed = command.trim(); + const unquoted = + trimmed.startsWith('"') && trimmed.endsWith('"') ? trimmed.slice(1, -1) : trimmed; + return /\.(cmd|bat)$/i.test(unquoted); + }); + + return ({ findExecutable: vi.fn(() => null), // Return null to force platform-specific path checking findExecutableAsync: vi.fn(() => Promise.resolve(null)), getAugmentedEnv: vi.fn(() => ({ PATH: '' })), getAugmentedEnvAsync: vi.fn(() => Promise.resolve({ PATH: '' })), - shouldUseShell: vi.fn((command: string) => { - // Mock shouldUseShell to match actual behavior - if (process.platform !== 'win32') { - return false; - } - return /\.(cmd|bat)$/i.test(command); - }), + shouldUseShell: mockShouldUseShell, getSpawnCommand: vi.fn((command: string) => { // Mock getSpawnCommand to match actual behavior const trimmed = command.trim(); @@ -122,12 +146,13 @@ vi.mock('../env-utils', () => ({ } return trimmed; }), - getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({ + getSpawnOptions: vi.fn((command: string, baseOptions?: SpawnOptions) => ({ ...baseOptions, - shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32' + shell: mockShouldUseShell(command) })), existsAsync: vi.fn(() => Promise.resolve(false)) -})); + }); +}); // Mock homebrew-python utility vi.mock('../utils/homebrew-python', () => ({ @@ -137,7 +162,11 @@ vi.mock('../utils/homebrew-python', () => ({ // Mock windows-paths utility vi.mock('../utils/windows-paths', () => ({ findWindowsExecutableViaWhere: vi.fn(() => null), - findWindowsExecutableViaWhereAsync: vi.fn(() => Promise.resolve(null)) + findWindowsExecutableViaWhereAsync: vi.fn(() => Promise.resolve(null)), + isSecurePath: vi.fn(() => true), + getWindowsExecutablePaths: vi.fn(() => []), + getWindowsExecutablePathsAsync: vi.fn(() => Promise.resolve([])), + WINDOWS_GIT_PATHS: {} })); describe('cli-tool-manager - Claude CLI NVM detection', () => { @@ -176,12 +205,12 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { }); // Mock Node.js version directories (three versions) - const mockDirents = [ - { name: 'v20.0.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v18.20.0', isDirectory: () => true }, + const mockDirents: MockDirent[] = [ + createDirent('v20.0.0', true), + createDirent('v22.17.0', true), + createDirent('v18.20.0', true), ]; - vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(readdirSync).mockReturnValue(mockDirents); // Mock execFileSync to simulate successful version check vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); @@ -245,11 +274,11 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { return false; }); - const mockDirents = [ - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.0.0', isDirectory: () => true }, + const mockDirents: MockDirent[] = [ + createDirent('v22.17.0', true), + createDirent('v20.0.0', true), ]; - vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(readdirSync).mockReturnValue(mockDirents); vi.mocked(execFileSync).mockReturnValue('claude-code version 1.5.0\n'); const result = getToolInfo('claude'); @@ -272,10 +301,10 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { return false; }); - const mockDirents = [ - { name: 'v22.17.0', isDirectory: () => true }, + const mockDirents: MockDirent[] = [ + createDirent('v22.17.0', true), ]; - vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(readdirSync).mockReturnValue(mockDirents); // Mock validation failure vi.mocked(execFileSync).mockImplementation(() => { @@ -301,12 +330,12 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { }); // Versions in random order - const mockDirents = [ - { name: 'v18.20.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.5.0', isDirectory: () => true }, + const mockDirents: MockDirent[] = [ + createDirent('v18.20.0', true), + createDirent('v22.17.0', true), + createDirent('v20.5.0', true), ]; - vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(readdirSync).mockReturnValue(mockDirents); vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); const result = getToolInfo('claude'); @@ -345,6 +374,35 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { expect(result.source).toBe('system-path'); }); + it('should ignore insecure Windows Claude CLI path from where.exe', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true + }); + + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + vi.mocked(findExecutable).mockReturnValue(null); + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( + 'D:\\Tools\\claude.cmd' + ); + vi.mocked(isSecurePath).mockReturnValueOnce(false); + + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('Tools') && pathStr.includes('claude.cmd')) { + return true; + } + return false; + }); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(false); + expect(result.source).toBe('fallback'); + expect(execFileSync).not.toHaveBeenCalled(); + expect(isSecurePath).toHaveBeenCalledWith('D:\\Tools\\claude.cmd'); + }); + it('should detect Claude CLI in Unix .local/bin path', () => { vi.mocked(os.homedir).mockReturnValue('/home/user'); diff --git a/apps/frontend/src/main/__tests__/insights-config.test.ts b/apps/frontend/src/main/__tests__/insights-config.test.ts index 5775d65ab0..ef3df618dc 100644 --- a/apps/frontend/src/main/__tests__/insights-config.test.ts +++ b/apps/frontend/src/main/__tests__/insights-config.test.ts @@ -59,7 +59,9 @@ describe('InsightsConfig', () => { expect(env.CLAUDE_CODE_OAUTH_TOKEN).toBe('oauth-token'); expect(env.ANTHROPIC_BASE_URL).toBe('https://api.z.ai'); expect(env.ANTHROPIC_AUTH_TOKEN).toBe('key'); - expect(env.PYTHONPATH).toBe(['/site-packages', '/backend'].join(path.delimiter)); + expect(env.PYTHONPATH).toBe( + [path.resolve('/site-packages'), path.resolve('/backend')].join(path.delimiter) + ); }); it('should clear ANTHROPIC env vars in OAuth mode when no API profile is set', async () => { @@ -84,7 +86,7 @@ describe('InsightsConfig', () => { const env = await config.getProcessEnv(); - expect(env.PYTHONPATH).toBe('/backend'); + expect(env.PYTHONPATH).toBe(path.resolve('/backend')); }); it('should keep PYTHONPATH from python env when auto-build path is missing', async () => { @@ -94,6 +96,6 @@ describe('InsightsConfig', () => { const env = await config.getProcessEnv(); - expect(env.PYTHONPATH).toBe('/site-packages'); + expect(env.PYTHONPATH).toBe(path.resolve('/site-packages')); }); }); diff --git a/apps/frontend/src/main/agent/agent-manager.ts b/apps/frontend/src/main/agent/agent-manager.ts index 962259e3e5..400ba21cdb 100644 --- a/apps/frontend/src/main/agent/agent-manager.ts +++ b/apps/frontend/src/main/agent/agent-manager.ts @@ -5,7 +5,7 @@ import { AgentState } from './agent-state'; import { AgentEvents } from './agent-events'; import { AgentProcessManager } from './agent-process'; import { AgentQueueManager } from './agent-queue'; -import { getClaudeProfileManager } from '../claude-profile-manager'; +import { getClaudeProfileManager, initializeClaudeProfileManager } from '../claude-profile-manager'; import { SpecCreationMetadata, TaskExecutionOptions, @@ -96,7 +96,15 @@ export class AgentManager extends EventEmitter { baseBranch?: string ): Promise { // Pre-flight auth check: Verify active profile has valid authentication - const profileManager = getClaudeProfileManager(); + // Ensure profile manager is initialized to prevent race condition + let profileManager; + try { + profileManager = await initializeClaudeProfileManager(); + } catch (error) { + console.error('[AgentManager] Failed to initialize profile manager:', error); + this.emit('error', taskId, 'Failed to initialize profile manager. Please check file permissions and disk space.'); + return; + } if (!profileManager.hasValidAuth()) { this.emit('error', taskId, 'Claude authentication required. Please authenticate in Settings > Claude Profiles before starting tasks.'); return; @@ -174,7 +182,15 @@ export class AgentManager extends EventEmitter { options: TaskExecutionOptions = {} ): Promise { // Pre-flight auth check: Verify active profile has valid authentication - const profileManager = getClaudeProfileManager(); + // Ensure profile manager is initialized to prevent race condition + let profileManager; + try { + profileManager = await initializeClaudeProfileManager(); + } catch (error) { + console.error('[AgentManager] Failed to initialize profile manager:', error); + this.emit('error', taskId, 'Failed to initialize profile manager. Please check file permissions and disk space.'); + return; + } if (!profileManager.hasValidAuth()) { this.emit('error', taskId, 'Claude authentication required. Please authenticate in Settings > Claude Profiles before starting tasks.'); return; diff --git a/apps/frontend/src/main/agent/agent-process.ts b/apps/frontend/src/main/agent/agent-process.ts index fe256b96bd..6291912a4e 100644 --- a/apps/frontend/src/main/agent/agent-process.ts +++ b/apps/frontend/src/main/agent/agent-process.ts @@ -1,7 +1,12 @@ import { spawn } from 'child_process'; import path from 'path'; +import { fileURLToPath } from 'url'; import { existsSync, readFileSync } from 'fs'; import { app } from 'electron'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { EventEmitter } from 'events'; import { AgentState } from './agent-state'; import { AgentEvents } from './agent-events'; @@ -457,7 +462,7 @@ export class AgentProcessManager { let sequenceNumber = 0; // FIX (ACS-203): Track completed phases to prevent phase overlaps // When a phase completes, it's added to this array before transitioning to the next phase - let completedPhases: CompletablePhase[] = []; + const completedPhases: CompletablePhase[] = []; this.emitter.emit('execution-progress', taskId, { phase: currentPhase, diff --git a/apps/frontend/src/main/app-updater.ts b/apps/frontend/src/main/app-updater.ts index 98f1f824bf..8bdbd7fa3e 100644 --- a/apps/frontend/src/main/app-updater.ts +++ b/apps/frontend/src/main/app-updater.ts @@ -47,6 +47,9 @@ type UpdateChannel = 'latest' | 'beta'; */ export function setUpdateChannel(channel: UpdateChannel): void { autoUpdater.channel = channel; + // Clear any downloaded update info when channel changes to prevent showing + // an Install button for an update from a different channel + downloadedUpdateInfo = null; console.warn(`[app-updater] Update channel set to: ${channel}`); } @@ -62,6 +65,9 @@ if (DEBUG_UPDATER) { let mainWindow: BrowserWindow | null = null; +// Track downloaded update state so it persists across Settings page navigations +let downloadedUpdateInfo: AppUpdateInfo | null = null; + /** * Initialize the app updater system * @@ -107,6 +113,13 @@ export function initializeAppUpdater(window: BrowserWindow, betaUpdates = false) // Update downloaded - ready to install autoUpdater.on('update-downloaded', (info) => { console.warn('[app-updater] Update downloaded:', info.version); + // Store downloaded update info so it persists across Settings page navigations + // releaseNotes can be string | ReleaseNoteInfo[] | null | undefined, only use if string + downloadedUpdateInfo = { + version: info.version, + releaseNotes: typeof info.releaseNotes === 'string' ? info.releaseNotes : undefined, + releaseDate: info.releaseDate + }; if (mainWindow) { mainWindow.webContents.send(IPC_CHANNELS.APP_UPDATE_DOWNLOADED, { version: info.version, @@ -215,9 +228,10 @@ export async function checkForUpdates(): Promise { return null; } + // releaseNotes can be string | ReleaseNoteInfo[] | null | undefined, only use if string return { version: result.updateInfo.version, - releaseNotes: result.updateInfo.releaseNotes as string | undefined, + releaseNotes: typeof result.updateInfo.releaseNotes === 'string' ? result.updateInfo.releaseNotes : undefined, releaseDate: result.updateInfo.releaseDate }; } catch (error) { @@ -256,6 +270,15 @@ export function getCurrentVersion(): string { return autoUpdater.currentVersion.version; } +/** + * Get downloaded update info if an update has been downloaded and is ready to install. + * This allows the UI to show "Install and Restart" even if the user opens Settings + * after the download completed in the background. + */ +export function getDownloadedUpdateInfo(): AppUpdateInfo | null { + return downloadedUpdateInfo; +} + /** * Check if a version string represents a prerelease (beta, alpha, rc, etc.) */ @@ -422,6 +445,9 @@ export async function setUpdateChannelWithDowngradeCheck( triggerDowngradeCheck = false ): Promise { autoUpdater.channel = channel; + // Clear any downloaded update info when channel changes to prevent showing + // an Install button for an update from a different channel + downloadedUpdateInfo = null; console.warn(`[app-updater] Update channel set to: ${channel}`); // If switching to stable and downgrade check requested, look for stable version diff --git a/apps/frontend/src/main/changelog/changelog-service.ts b/apps/frontend/src/main/changelog/changelog-service.ts index bd4ba9a79f..4d3bd42ef0 100644 --- a/apps/frontend/src/main/changelog/changelog-service.ts +++ b/apps/frontend/src/main/changelog/changelog-service.ts @@ -1,7 +1,12 @@ import { EventEmitter } from 'events'; import * as path from 'path'; +import { fileURLToPath } from 'url'; import { existsSync, readFileSync, writeFileSync } from 'fs'; import { app } from 'electron'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { AUTO_BUILD_PATHS, DEFAULT_CHANGELOG_PATH } from '../../shared/constants'; import { getToolPath } from '../cli-tool-manager'; import type { diff --git a/apps/frontend/src/main/claude-profile-manager.ts b/apps/frontend/src/main/claude-profile-manager.ts index f64ef42d81..11cea61bfc 100644 --- a/apps/frontend/src/main/claude-profile-manager.ts +++ b/apps/frontend/src/main/claude-profile-manager.ts @@ -568,6 +568,7 @@ export function getClaudeProfileManager(): ClaudeProfileManager { * Initialize and get the singleton Claude profile manager instance (async) * This ensures the profile manager is fully initialized before use. * Uses promise caching to prevent concurrent initialization. + * The cached promise is reset on failure to allow retries after transient errors. */ export async function initializeClaudeProfileManager(): Promise { if (!profileManager) { @@ -581,9 +582,16 @@ export async function initializeClaudeProfileManager(): Promise { - return profileManager!; - }); + initPromise = profileManager.initialize() + .then(() => { + return profileManager!; + }) + .catch((error) => { + // Reset cached promise on failure so retries can succeed + // This allows recovery from transient errors (e.g., disk full, permission issues) + initPromise = null; + throw error; + }); } return initPromise; diff --git a/apps/frontend/src/main/claude-profile/profile-scorer.ts b/apps/frontend/src/main/claude-profile/profile-scorer.ts index fc0f8ecc0d..25b58816c8 100644 --- a/apps/frontend/src/main/claude-profile/profile-scorer.ts +++ b/apps/frontend/src/main/claude-profile/profile-scorer.ts @@ -35,12 +35,19 @@ export function getBestAvailableProfile( // 2. Lower weekly usage (more important than session) // 3. Lower session usage // 4. More recently authenticated + const isDebug = process.env.DEBUG === 'true'; + + if (isDebug) { + console.warn('[ProfileScorer] Evaluating', candidates.length, 'candidate profiles (excluding:', excludeProfileId, ')'); + } const scoredProfiles: ScoredProfile[] = candidates.map(profile => { let score = 100; // Base score + if (isDebug) console.warn('[ProfileScorer] Scoring profile:', profile.name, '(', profile.id, ')'); // Check rate limit status const rateLimitStatus = isProfileRateLimited(profile); + if (isDebug) console.warn('[ProfileScorer] Rate limit status:', rateLimitStatus); if (rateLimitStatus.limited) { // Severely penalize rate-limited profiles if (rateLimitStatus.type === 'weekly') { @@ -73,10 +80,14 @@ export function getBestAvailableProfile( } // Check if authenticated - if (!isProfileAuthenticated(profile)) { + const isAuth = isProfileAuthenticated(profile); + if (isDebug) console.warn('[ProfileScorer] isProfileAuthenticated:', isAuth, 'hasOAuthToken:', !!profile.oauthToken, 'hasConfigDir:', !!profile.configDir); + if (!isAuth) { score -= 500; // Severely penalize unauthenticated profiles + if (isDebug) console.warn('[ProfileScorer] Applied -500 penalty for no auth'); } + if (isDebug) console.warn('[ProfileScorer] Final score:', score); return { profile, score }; }); diff --git a/apps/frontend/src/main/claude-profile/profile-utils.ts b/apps/frontend/src/main/claude-profile/profile-utils.ts index 80a3c048cb..e6d8ceea83 100644 --- a/apps/frontend/src/main/claude-profile/profile-utils.ts +++ b/apps/frontend/src/main/claude-profile/profile-utils.ts @@ -56,9 +56,16 @@ export async function createProfileDirectory(profileName: string): Promise = new Map(); // profileId -> timestamp + private static AUTH_FAILURE_COOLDOWN_MS = 5 * 60 * 1000; // 5 minutes cooldown + + // Debug flag for verbose logging + private readonly isDebug = process.env.DEBUG === 'true'; private constructor() { super(); @@ -40,7 +47,7 @@ export class UsageMonitor extends EventEmitter { const settings = profileManager.getAutoSwitchSettings(); if (!settings.enabled || !settings.proactiveSwapEnabled) { - console.warn('[UsageMonitor] Proactive monitoring disabled'); + console.warn('[UsageMonitor] Proactive monitoring disabled. Settings:', JSON.stringify(settings, null, 2)); return; } @@ -118,6 +125,15 @@ export class UsageMonitor extends EventEmitter { const weeklyExceeded = usage.weeklyPercent >= settings.weeklyThreshold; if (sessionExceeded || weeklyExceeded) { + if (this.isDebug) { + console.warn('[UsageMonitor:TRACE] Threshold exceeded', { + sessionPercent: usage.sessionPercent, + weekPercent: usage.weeklyPercent, + activeProfile: activeProfile.id, + hasToken: !!decryptedToken + }); + } + console.warn('[UsageMonitor] Threshold exceeded:', { sessionPercent: usage.sessionPercent, sessionThreshold: settings.sessionThreshold, @@ -130,8 +146,48 @@ export class UsageMonitor extends EventEmitter { activeProfile.id, sessionExceeded ? 'session' : 'weekly' ); + } else { + if (this.isDebug) { + console.warn('[UsageMonitor:TRACE] Usage OK', { + sessionPercent: usage.sessionPercent, + weekPercent: usage.weeklyPercent + }); + } } } catch (error) { + // Check for auth failure (401/403) from fetchUsageViaAPI + if ((error as any).statusCode === 401 || (error as any).statusCode === 403) { + const profileManager = getClaudeProfileManager(); + const activeProfile = profileManager.getActiveProfile(); + + if (activeProfile) { + // Mark this profile as auth-failed to prevent swap loops + this.authFailedProfiles.set(activeProfile.id, Date.now()); + console.warn('[UsageMonitor] Auth failure detected, marked profile as failed:', activeProfile.id); + + // Clean up expired entries from the failed profiles map + const now = Date.now(); + this.authFailedProfiles.forEach((timestamp, profileId) => { + if (now - timestamp > UsageMonitor.AUTH_FAILURE_COOLDOWN_MS) { + this.authFailedProfiles.delete(profileId); + } + }); + + try { + const excludeProfiles = Array.from(this.authFailedProfiles.keys()); + console.warn('[UsageMonitor] Attempting proactive swap (excluding failed profiles):', excludeProfiles); + await this.performProactiveSwap( + activeProfile.id, + 'session', // Treat auth failure as session limit for immediate swap + excludeProfiles + ); + return; + } catch (swapError) { + console.error('[UsageMonitor] Failed to perform auth-failure swap:', swapError); + } + } + } + console.error('[UsageMonitor] Check failed:', error); } finally { this.isChecking = false; @@ -190,6 +246,12 @@ export class UsageMonitor extends EventEmitter { if (!response.ok) { console.error('[UsageMonitor] API error:', response.status, response.statusText); + // Throw specific error for auth failures so we can trigger a swap + if (response.status === 401 || response.status === 403) { + const error = new Error(`API Auth Failure: ${response.status}`); + (error as any).statusCode = response.status; + throw error; + } return null; } @@ -220,7 +282,12 @@ export class UsageMonitor extends EventEmitter { ? 'weekly' : 'session' }; - } catch (error) { + } catch (error: any) { + // Re-throw auth failures to be handled by checkUsageAndSwap + if (error?.statusCode === 401 || error?.statusCode === 403) { + throw error; + } + console.error('[UsageMonitor] API fetch failed:', error); return null; } @@ -270,22 +337,34 @@ export class UsageMonitor extends EventEmitter { /** * Perform proactive profile swap + * @param currentProfileId - The profile to switch from + * @param limitType - The type of limit that triggered the swap + * @param additionalExclusions - Additional profile IDs to exclude (e.g., auth-failed profiles) */ private async performProactiveSwap( currentProfileId: string, - limitType: 'session' | 'weekly' + limitType: 'session' | 'weekly', + additionalExclusions: string[] = [] ): Promise { const profileManager = getClaudeProfileManager(); - const bestProfile = profileManager.getBestAvailableProfile(currentProfileId); - - if (!bestProfile) { - console.warn('[UsageMonitor] No alternative profile for proactive swap'); + + // Get all profiles to swap to, excluding current and any additional exclusions + const allProfiles = profileManager.getProfilesSortedByAvailability(); + const excludeIds = new Set([currentProfileId, ...additionalExclusions]); + const eligibleProfiles = allProfiles.filter(p => !excludeIds.has(p.id)); + + if (eligibleProfiles.length === 0) { + console.warn('[UsageMonitor] No alternative profile for proactive swap (excluded:', Array.from(excludeIds), ')'); this.emit('proactive-swap-failed', { - reason: 'no_alternative', - currentProfile: currentProfileId + reason: additionalExclusions.length > 0 ? 'all_alternatives_failed_auth' : 'no_alternative', + currentProfile: currentProfileId, + excludedProfiles: Array.from(excludeIds) }); return; } + + // Use the best available from eligible profiles + const bestProfile = eligibleProfiles[0]; console.warn('[UsageMonitor] Proactive swap:', { from: currentProfileId, diff --git a/apps/frontend/src/main/cli-tool-manager.ts b/apps/frontend/src/main/cli-tool-manager.ts index 82cdc465ff..ef09b62563 100644 --- a/apps/frontend/src/main/cli-tool-manager.ts +++ b/apps/frontend/src/main/cli-tool-manager.ts @@ -20,7 +20,7 @@ * - Graceful fallbacks when tools not found */ -import { execFileSync, execFile, execSync, exec } from 'child_process'; +import { execFileSync, execFile } from 'child_process'; import { existsSync, readdirSync, promises as fsPromises } from 'fs'; import path from 'path'; import os from 'os'; @@ -28,10 +28,19 @@ import { promisify } from 'util'; import { app } from 'electron'; import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; import type { ToolDetectionResult } from '../shared/types'; +import { findHomebrewPython as findHomebrewPythonUtil } from './utils/homebrew-python'; const execFileAsync = promisify(execFile); -const execAsync = promisify(exec); -import { findHomebrewPython as findHomebrewPythonUtil } from './utils/homebrew-python'; + +type ExecFileSyncOptionsWithVerbatim = import('child_process').ExecFileSyncOptions & { + windowsVerbatimArguments?: boolean; +}; +type ExecFileAsyncOptionsWithVerbatim = import('child_process').ExecFileOptionsWithStringEncoding & { + windowsVerbatimArguments?: boolean; +}; + +const normalizeExecOutput = (output: string | Buffer): string => + typeof output === 'string' ? output : output.toString('utf-8'); import { getWindowsExecutablePaths, getWindowsExecutablePathsAsync, @@ -138,6 +147,16 @@ interface ClaudeDetectionPaths { * This pure function consolidates path configuration used by both sync * and async detection methods. * + * IMPORTANT: This function has a corresponding implementation in the Python backend: + * apps/backend/core/client.py (_get_claude_detection_paths) + * + * Both implementations MUST be kept in sync to ensure consistent detection behavior + * across the Electron frontend and Python backend. + * + * When adding new detection paths, update BOTH: + * 1. This function (getClaudeDetectionPaths in cli-tool-manager.ts) + * 2. _get_claude_detection_paths() in client.py + * * @param homeDir - User's home directory (from os.homedir()) * @returns Object containing homebrew, platform, and NVM paths * @@ -916,28 +935,51 @@ class CLIToolManager { */ private validateClaude(claudeCmd: string): ToolValidation { try { - const needsShell = shouldUseShell(claudeCmd); + const trimmedCmd = claudeCmd.trim(); + const unquotedCmd = + trimmedCmd.startsWith('"') && trimmedCmd.endsWith('"') + ? trimmedCmd.slice(1, -1) + : trimmedCmd; + + const needsShell = shouldUseShell(trimmedCmd); + const cmdDir = path.dirname(unquotedCmd); + const env = getAugmentedEnv(cmdDir && cmdDir !== '.' ? [cmdDir] : []); let version: string; if (needsShell) { - // For .cmd/.bat files on Windows, use cmd.exe with argument array - // This avoids shell command injection while handling spaces in paths - version = execFileSync('cmd.exe', ['/c', claudeCmd, '--version'], { + // For .cmd/.bat files on Windows, use cmd.exe with a quoted command line + // /s preserves quotes so paths with spaces are handled correctly. + if (!isSecurePath(unquotedCmd)) { + return { + valid: false, + message: `Claude CLI path failed security validation: ${unquotedCmd}`, + }; + } + const cmdExe = process.env.ComSpec + || path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe'); + const cmdLine = `""${unquotedCmd}" --version"`; + const execOptions: ExecFileSyncOptionsWithVerbatim = { encoding: 'utf-8', timeout: 5000, windowsHide: true, - env: getAugmentedEnv(), - }).trim(); + windowsVerbatimArguments: true, + env, + }; + version = normalizeExecOutput( + execFileSync(cmdExe, ['/d', '/s', '/c', cmdLine], execOptions) + ).trim(); } else { // For .exe files and non-Windows, use execFileSync - version = execFileSync(claudeCmd, ['--version'], { - encoding: 'utf-8', - timeout: 5000, - windowsHide: true, - shell: false, - env: getAugmentedEnv(), - }).trim(); + version = normalizeExecOutput( + execFileSync(unquotedCmd, ['--version'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + shell: false, + env, + }) + ).trim(); } // Claude CLI version output format: "claude-code version X.Y.Z" or similar @@ -1032,33 +1074,52 @@ class CLIToolManager { */ private async validateClaudeAsync(claudeCmd: string): Promise { try { - const needsShell = shouldUseShell(claudeCmd); + const trimmedCmd = claudeCmd.trim(); + const unquotedCmd = + trimmedCmd.startsWith('"') && trimmedCmd.endsWith('"') + ? trimmedCmd.slice(1, -1) + : trimmedCmd; + + const needsShell = shouldUseShell(trimmedCmd); + const cmdDir = path.dirname(unquotedCmd); + const env = await getAugmentedEnvAsync(cmdDir && cmdDir !== '.' ? [cmdDir] : []); let stdout: string; if (needsShell) { - // For .cmd/.bat files on Windows, use cmd.exe with argument array - // This avoids shell command injection while handling spaces in paths - const result = await execFileAsync('cmd.exe', ['/c', claudeCmd, '--version'], { + // For .cmd/.bat files on Windows, use cmd.exe with a quoted command line + // /s preserves quotes so paths with spaces are handled correctly. + if (!isSecurePath(unquotedCmd)) { + return { + valid: false, + message: `Claude CLI path failed security validation: ${unquotedCmd}`, + }; + } + const cmdExe = process.env.ComSpec + || path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe'); + const cmdLine = `""${unquotedCmd}" --version"`; + const execOptions: ExecFileAsyncOptionsWithVerbatim = { encoding: 'utf-8', timeout: 5000, windowsHide: true, - env: await getAugmentedEnvAsync(), - }); + windowsVerbatimArguments: true, + env, + }; + const result = await execFileAsync(cmdExe, ['/d', '/s', '/c', cmdLine], execOptions); stdout = result.stdout; } else { // For .exe files and non-Windows, use execFileAsync - const result = await execFileAsync(claudeCmd, ['--version'], { + const result = await execFileAsync(unquotedCmd, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, shell: false, - env: await getAugmentedEnvAsync(), + env, }); stdout = result.stdout; } - const version = stdout.trim(); + const version = normalizeExecOutput(stdout).trim(); const match = version.match(/(\d+\.\d+\.\d+)/); const versionStr = match ? match[1] : version.split('\n')[0]; diff --git a/apps/frontend/src/main/env-utils.ts b/apps/frontend/src/main/env-utils.ts index 8d4b88478c..fc54f2ac35 100644 --- a/apps/frontend/src/main/env-utils.ts +++ b/apps/frontend/src/main/env-utils.ts @@ -15,6 +15,7 @@ import * as fs from 'fs'; import { promises as fsPromises } from 'fs'; import { execFileSync, execFile } from 'child_process'; import { promisify } from 'util'; +import { getSentryEnvForSubprocess } from './sentry'; const execFileAsync = promisify(execFile); @@ -237,6 +238,11 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record { }); }); - // Pre-initialize Claude profile manager in background (non-blocking) - // This ensures profile data is loaded before user clicks "Start Claude Code" - setImmediate(() => { - initializeClaudeProfileManager().catch((error) => { - console.warn('[main] Failed to pre-initialize profile manager:', error); + // Initialize Claude profile manager, then start usage monitor + // We do this sequentially to ensure profile data (including auto-switch settings) + // is loaded BEFORE the usage monitor attempts to read settings. + // This prevents the "UsageMonitor disabled" error due to race condition. + initializeClaudeProfileManager() + .then(() => { + // Only start monitoring if window is still available (app not quitting) + if (mainWindow) { + // Setup event forwarding from usage monitor to renderer + initializeUsageMonitorForwarding(mainWindow); + + // Start the usage monitor + const usageMonitor = getUsageMonitor(); + usageMonitor.start(); + console.warn('[main] Usage monitor initialized and started (after profile load)'); + } + }) + .catch((error) => { + console.warn('[main] Failed to initialize profile manager:', error); + // Fallback: try starting usage monitor anyway (might use defaults) + if (mainWindow) { + initializeUsageMonitorForwarding(mainWindow); + const usageMonitor = getUsageMonitor(); + usageMonitor.start(); + } }); - }); - // Initialize usage monitoring after window is created if (mainWindow) { - // Setup event forwarding from usage monitor to renderer - initializeUsageMonitorForwarding(mainWindow); - - // Start the usage monitor - const usageMonitor = getUsageMonitor(); - usageMonitor.start(); - console.warn('[main] Usage monitor initialized and started'); - // Log debug mode status const isDebugMode = process.env.DEBUG === 'true'; if (isDebugMode) { diff --git a/apps/frontend/src/main/ipc-handlers/app-update-handlers.ts b/apps/frontend/src/main/ipc-handlers/app-update-handlers.ts index 66c7f3ee3d..33f4d6b107 100644 --- a/apps/frontend/src/main/ipc-handlers/app-update-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/app-update-handlers.ts @@ -13,7 +13,8 @@ import { downloadUpdate, downloadStableVersion, quitAndInstall, - getCurrentVersion + getCurrentVersion, + getDownloadedUpdateInfo } from '../app-updater'; /** @@ -123,5 +124,28 @@ export function registerAppUpdateHandlers(): void { } ); + /** + * APP_UPDATE_GET_DOWNLOADED: Get downloaded update info + * Returns info about a downloaded update that's ready to install, + * or null if no update has been downloaded yet. + * This allows the UI to show "Install and Restart" even if the user + * opens Settings after the download completed in the background. + */ + ipcMain.handle( + IPC_CHANNELS.APP_UPDATE_GET_DOWNLOADED, + async (): Promise> => { + try { + const downloadedInfo = getDownloadedUpdateInfo(); + return { success: true, data: downloadedInfo }; + } catch (error) { + console.error('[app-update-handlers] Get downloaded update info failed:', error); + return { + success: false, + error: error instanceof Error ? error.message : 'Failed to get downloaded update info' + }; + } + } + ); + console.warn('[IPC] App update handlers registered successfully'); } diff --git a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts index 2b6a151002..28f912d6c0 100644 --- a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts @@ -7,20 +7,199 @@ * - Open terminal with installation command */ -import { ipcMain, shell } from 'electron'; -import { execFileSync, spawn } from 'child_process'; -import { existsSync, statSync } from 'fs'; +import { ipcMain } from 'electron'; +import { execFileSync, spawn, execFile } from 'child_process'; +import { existsSync, promises as fsPromises } from 'fs'; import path from 'path'; -import { IPC_CHANNELS } from '../../shared/constants/ipc'; +import os from 'os'; +import { promisify } from 'util'; +import { IPC_CHANNELS, DEFAULT_APP_SETTINGS } from '../../shared/constants'; import type { IPCResult } from '../../shared/types'; -import type { ClaudeCodeVersionInfo } from '../../shared/types/cli'; -import { getToolInfo } from '../cli-tool-manager'; -import { readSettingsFile } from '../settings-utils'; +import type { ClaudeCodeVersionInfo, ClaudeInstallationList, ClaudeInstallationInfo } from '../../shared/types/cli'; +import { getToolInfo, configureTools, sortNvmVersionDirs, getClaudeDetectionPaths } from '../cli-tool-manager'; +import { readSettingsFile, writeSettingsFile } from '../settings-utils'; +import { isSecurePath } from '../utils/windows-paths'; import semver from 'semver'; +const execFileAsync = promisify(execFile); + // Cache for latest version (avoid hammering npm registry) let cachedLatestVersion: { version: string; timestamp: number } | null = null; +let cachedVersionList: { versions: string[]; timestamp: number } | null = null; const CACHE_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours +const VERSION_LIST_CACHE_DURATION_MS = 60 * 60 * 1000; // 1 hour for version list + +/** + * Validate a Claude CLI path and get its version + * @param cliPath - Path to the Claude CLI executable + * @returns Tuple of [isValid, version or null] + */ +async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string | null]> { + try { + const isWindows = process.platform === 'win32'; + + // Augment PATH with the CLI directory for proper resolution + const cliDir = path.dirname(cliPath); + const env = { + ...process.env, + PATH: cliDir ? `${cliDir}${path.delimiter}${process.env.PATH || ''}` : process.env.PATH, + }; + + let stdout: string; + // For Windows .cmd/.bat files, use cmd.exe with proper quoting + // /d = disable AutoRun registry commands + // /s = strip first and last quotes, preserving inner quotes + // /c = run command then terminate + if (isWindows && /\.(cmd|bat)$/i.test(cliPath)) { + // Get cmd.exe path from environment or use default + const cmdExe = process.env.ComSpec + || path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe'); + // Use double-quoted command line for paths with spaces + const cmdLine = `""${cliPath}" --version"`; + const result = await execFileAsync(cmdExe, ['/d', '/s', '/c', cmdLine], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + env, + }); + stdout = result.stdout; + } else { + const result = await execFileAsync(cliPath, ['--version'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + env, + }); + stdout = result.stdout; + } + + const version = String(stdout).trim(); + const match = version.match(/(\d+\.\d+\.\d+)/); + return [true, match ? match[1] : version.split('\n')[0]]; + } catch (error) { + // Log validation errors to help debug CLI detection issues + console.warn('[Claude Code] CLI validation failed for', cliPath, ':', error); + return [false, null]; + } +} + +/** + * Scan all known locations for Claude CLI installations. + * Returns all found installations with their paths, versions, and sources. + * + * Uses getClaudeDetectionPaths() from cli-tool-manager.ts as the single source + * of truth for detection paths to avoid duplication and ensure consistency. + * + * @see cli-tool-manager.ts getClaudeDetectionPaths() for path configuration + */ +async function scanClaudeInstallations(activePath: string | null): Promise { + const installations: ClaudeInstallationInfo[] = []; + const seenPaths = new Set(); + const homeDir = os.homedir(); + const isWindows = process.platform === 'win32'; + + // Get detection paths from cli-tool-manager (single source of truth) + const detectionPaths = getClaudeDetectionPaths(homeDir); + + const addInstallation = async ( + cliPath: string, + source: ClaudeInstallationInfo['source'] + ) => { + // Normalize path for comparison + const normalizedPath = path.resolve(cliPath); + if (seenPaths.has(normalizedPath)) return; + + if (!existsSync(cliPath)) return; + + // Security validation: reject paths with shell metacharacters or directory traversal + if (!isSecurePath(cliPath)) { + console.warn('[Claude Code] Rejecting insecure path:', cliPath); + return; + } + + const [isValid, version] = await validateClaudeCliAsync(cliPath); + if (!isValid) return; + + seenPaths.add(normalizedPath); + installations.push({ + path: normalizedPath, + version, + source, + isActive: activePath ? path.resolve(activePath) === normalizedPath : false, + }); + }; + + // 1. Check user-configured path first (if set) + if (activePath && existsSync(activePath)) { + await addInstallation(activePath, 'user-config'); + } + + // 2. Check system PATH via which/where + try { + if (isWindows) { + const result = await execFileAsync('where', ['claude'], { timeout: 5000 }); + const paths = result.stdout.trim().split('\n').filter(p => p.trim()); + for (const p of paths) { + await addInstallation(p.trim(), 'system-path'); + } + } else { + const result = await execFileAsync('which', ['-a', 'claude'], { timeout: 5000 }); + const paths = result.stdout.trim().split('\n').filter(p => p.trim()); + for (const p of paths) { + await addInstallation(p.trim(), 'system-path'); + } + } + } catch { + // which/where failed, continue with other methods + } + + // 3. Homebrew paths (macOS) - from getClaudeDetectionPaths + if (process.platform === 'darwin') { + for (const p of detectionPaths.homebrewPaths) { + await addInstallation(p, 'homebrew'); + } + } + + // 4. NVM paths (Unix) - check Node.js version manager + if (!isWindows && existsSync(detectionPaths.nvmVersionsDir)) { + try { + const entries = await fsPromises.readdir(detectionPaths.nvmVersionsDir, { withFileTypes: true }); + const versionDirs = sortNvmVersionDirs(entries); + for (const versionName of versionDirs) { + const nvmClaudePath = path.join(detectionPaths.nvmVersionsDir, versionName, 'bin', 'claude'); + await addInstallation(nvmClaudePath, 'nvm'); + } + } catch { + // Failed to read NVM directory + } + } + + // 5. Platform-specific standard locations - from getClaudeDetectionPaths + for (const p of detectionPaths.platformPaths) { + await addInstallation(p, 'system-path'); + } + + // 6. Additional common paths not in getClaudeDetectionPaths (for broader scanning) + const additionalPaths = isWindows + ? [] // Windows paths are well covered by detectionPaths.platformPaths + : [ + path.join(homeDir, '.npm-global', 'bin', 'claude'), + path.join(homeDir, '.yarn', 'bin', 'claude'), + path.join(homeDir, '.claude', 'local', 'claude'), + path.join(homeDir, 'node_modules', '.bin', 'claude'), + ]; + + for (const p of additionalPaths) { + await addInstallation(p, 'system-path'); + } + + // Mark the first installation as active if none is explicitly active + if (installations.length > 0 && !installations.some(i => i.isActive)) { + installations[0].isActive = true; + } + + return installations; +} /** * Fetch the latest version of Claude Code from npm registry @@ -63,6 +242,74 @@ async function fetchLatestVersion(): Promise { } } +/** + * Fetch available versions of Claude Code from npm registry + * Returns versions sorted by semver descending (newest first) + * Limited to last 20 versions for performance + */ +async function fetchAvailableVersions(): Promise { + // Check cache first + if (cachedVersionList && Date.now() - cachedVersionList.timestamp < VERSION_LIST_CACHE_DURATION_MS) { + return cachedVersionList.versions; + } + + try { + const response = await fetch('https://registry.npmjs.org/@anthropic-ai/claude-code', { + headers: { + 'Accept': 'application/json', + }, + signal: AbortSignal.timeout(15000), // 15 second timeout + }); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } + + const data = await response.json(); + const versions = Object.keys(data.versions || {}); + + if (!versions.length) { + throw new Error('No versions found in npm registry'); + } + + // Sort by semver descending (newest first) and take last 20 + const sortedVersions = versions + .filter(v => semver.valid(v)) // Only valid semver versions + .sort((a, b) => semver.rcompare(a, b)) // Sort descending + .slice(0, 20); // Limit to 20 versions + + // Validate we have versions after filtering + if (sortedVersions.length === 0) { + throw new Error('No valid semver versions found in npm registry'); + } + + // Cache the result + cachedVersionList = { versions: sortedVersions, timestamp: Date.now() }; + return sortedVersions; + } catch (error) { + console.error('[Claude Code] Failed to fetch available versions:', error); + // Return cached versions if available, even if expired + if (cachedVersionList) { + return cachedVersionList.versions; + } + throw error; + } +} + +/** + * Get the platform-specific install command for a specific version of Claude Code + * @param version - The version to install (e.g., "1.0.5") + */ +function getInstallVersionCommand(version: string): string { + if (process.platform === 'win32') { + // Windows: kill running Claude processes first, then install specific version + return `taskkill /IM claude.exe /F 2>nul; claude install --force ${version}`; + } else { + // macOS/Linux: kill running Claude processes first, then install specific version + return `pkill -x claude 2>/dev/null; sleep 1; claude install --force ${version}`; + } +} + /** * Get the platform-specific install command for Claude Code * @param isUpdate - If true, Claude is already installed and we just need to update @@ -280,7 +527,7 @@ export async function openTerminalWithCommand(command: string): Promise { 'C:\\Program Files\\Git\\git-bash.exe', 'C:\\Program Files (x86)\\Git\\git-bash.exe', ]; - let gitBashPath = gitBashPaths.find(p => existsSync(p)); + const gitBashPath = gitBashPaths.find(p => existsSync(p)); if (gitBashPath) { await runWindowsCommand(`"${gitBashPath}" -c "${escapedBashCommand}"`); } else { @@ -621,5 +868,151 @@ export function registerClaudeCodeHandlers(): void { } ); + // Get available Claude Code versions + ipcMain.handle( + IPC_CHANNELS.CLAUDE_CODE_GET_VERSIONS, + async (): Promise> => { + try { + console.log('[Claude Code] Fetching available versions...'); + const versions = await fetchAvailableVersions(); + console.log('[Claude Code] Found', versions.length, 'versions'); + return { + success: true, + data: { versions }, + }; + } catch (error) { + const errorMsg = error instanceof Error ? error.message : 'Unknown error'; + console.error('[Claude Code] Failed to fetch versions:', errorMsg, error); + return { + success: false, + error: `Failed to fetch available versions: ${errorMsg}`, + }; + } + } + ); + + // Install a specific version of Claude Code + ipcMain.handle( + IPC_CHANNELS.CLAUDE_CODE_INSTALL_VERSION, + async (_event, version: string): Promise> => { + try { + // Validate version format + if (!version || typeof version !== 'string') { + throw new Error('Invalid version specified'); + } + + // Basic semver validation + if (!semver.valid(version)) { + throw new Error(`Invalid version format: ${version}`); + } + + console.log('[Claude Code] Installing version:', version); + const command = getInstallVersionCommand(version); + console.log('[Claude Code] Install command:', command); + console.log('[Claude Code] Opening terminal...'); + await openTerminalWithCommand(command); + console.log('[Claude Code] Terminal opened successfully'); + + return { + success: true, + data: { command, version }, + }; + } catch (error) { + const errorMsg = error instanceof Error ? error.message : 'Unknown error'; + console.error('[Claude Code] Install version failed:', errorMsg, error); + return { + success: false, + error: `Failed to install version: ${errorMsg}`, + }; + } + } + ); + + // Get all Claude CLI installations found on the system + ipcMain.handle( + IPC_CHANNELS.CLAUDE_CODE_GET_INSTALLATIONS, + async (): Promise> => { + try { + console.log('[Claude Code] Scanning for installations...'); + + // Get current active path from settings + const settings = readSettingsFile(); + const activePath = settings?.claudePath as string | undefined; + + const installations = await scanClaudeInstallations(activePath || null); + console.log('[Claude Code] Found', installations.length, 'installations'); + + return { + success: true, + data: { + installations, + activePath: activePath || (installations.length > 0 ? installations[0].path : null), + }, + }; + } catch (error) { + const errorMsg = error instanceof Error ? error.message : 'Unknown error'; + console.error('[Claude Code] Failed to scan installations:', errorMsg, error); + return { + success: false, + error: `Failed to scan Claude CLI installations: ${errorMsg}`, + }; + } + } + ); + + // Set the active Claude CLI path + ipcMain.handle( + IPC_CHANNELS.CLAUDE_CODE_SET_ACTIVE_PATH, + async (_event, cliPath: string): Promise> => { + try { + console.log('[Claude Code] Setting active path:', cliPath); + + // Security validation: reject paths with shell metacharacters or directory traversal + if (!isSecurePath(cliPath)) { + throw new Error('Invalid path: contains potentially unsafe characters'); + } + + // Normalize path to prevent directory traversal + const normalizedPath = path.resolve(cliPath); + + // Validate the path exists and is executable + if (!existsSync(normalizedPath)) { + throw new Error('Claude CLI not found at specified path'); + } + + const [isValid, version] = await validateClaudeCliAsync(normalizedPath); + if (!isValid) { + throw new Error('Claude CLI at specified path is not valid or not executable'); + } + + // Save to settings using established pattern: merge with DEFAULT_APP_SETTINGS + const currentSettings = readSettingsFile() || {}; + const mergedSettings = { + ...DEFAULT_APP_SETTINGS, + ...currentSettings, + claudePath: normalizedPath, + } as Record; + writeSettingsFile(mergedSettings); + + // Update CLI tool manager cache + configureTools({ claudePath: normalizedPath }); + + console.log('[Claude Code] Active path set:', normalizedPath, 'version:', version); + + return { + success: true, + data: { path: normalizedPath }, + }; + } catch (error) { + const errorMsg = error instanceof Error ? error.message : 'Unknown error'; + console.error('[Claude Code] Failed to set active path:', errorMsg, error); + return { + success: false, + error: `Failed to set active Claude CLI path: ${errorMsg}`, + }; + } + } + ); + console.warn('[IPC] Claude Code handlers registered'); } diff --git a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts b/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts index 5b1700cf1b..e5bf026577 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts +++ b/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts @@ -9,7 +9,12 @@ import { spawn, exec } from 'child_process'; import type { ChildProcess } from 'child_process'; import { promisify } from 'util'; import path from 'path'; +import { fileURLToPath } from 'url'; import fs from 'fs'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import type { Project } from '../../../../shared/types'; import { parsePythonCommand } from '../../../python-detector'; diff --git a/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts b/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts index 34b593c8cc..4141f03203 100644 --- a/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts +++ b/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts @@ -194,29 +194,46 @@ export async function convertIdeaToTask( AUTO_BUILD_PATHS.IDEATION_FILE ); - const ideation = readIdeationFile(ideationPath); - if (!ideation) { + // Quick check that ideation file exists (actual read happens inside lock) + if (!existsSync(ideationPath)) { return { success: false, error: 'Ideation not found' }; } - try { - // Find the idea - const idea = ideation.ideas?.find((i) => i.id === ideaId); - if (!idea) { - return { success: false, error: 'Idea not found' }; - } - - // Get specs directory path - const specsBaseDir = getSpecsDir(project.autoBuildPath); - const specsDir = path.join(project.path, specsBaseDir); + // Get specs directory path + const specsBaseDir = getSpecsDir(project.autoBuildPath); + const specsDir = path.join(project.path, specsBaseDir); - // Ensure specs directory exists - if (!existsSync(specsDir)) { - mkdirSync(specsDir, { recursive: true }); - } + // Ensure specs directory exists + if (!existsSync(specsDir)) { + mkdirSync(specsDir, { recursive: true }); + } + try { // Use coordinated spec numbering with lock to prevent collisions + // CRITICAL: All state checks must happen INSIDE the lock to prevent TOCTOU race conditions return await withSpecNumberLock(project.path, async (lock) => { + // Re-read ideation file INSIDE the lock to get fresh state + const ideation = readIdeationFile(ideationPath); + if (!ideation) { + return { success: false, error: 'Ideation not found' }; + } + + // Find the idea (inside lock for fresh state) + const idea = ideation.ideas?.find((i) => i.id === ideaId); + if (!idea) { + return { success: false, error: 'Idea not found' }; + } + + // Idempotency check INSIDE lock - prevents TOCTOU race condition + // Two concurrent requests can both pass an outside check, but only one + // can hold the lock at a time, so this check is authoritative + if (idea.linked_task_id) { + return { + success: false, + error: `Idea has already been converted to task: ${idea.linked_task_id}` + }; + } + // Get next spec number from global scan (main + all worktrees) const nextNum = lock.getNextSpecNumber(project.autoBuildPath); const slugifiedTitle = slugifyTitle(idea.title); diff --git a/apps/frontend/src/main/ipc-handlers/memory-handlers.ts b/apps/frontend/src/main/ipc-handlers/memory-handlers.ts index 9ea2b79ab4..72d786a261 100644 --- a/apps/frontend/src/main/ipc-handlers/memory-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/memory-handlers.ts @@ -8,8 +8,13 @@ import { ipcMain, app } from 'electron'; import { spawn, execFileSync } from 'child_process'; import * as path from 'path'; +import { fileURLToPath } from 'url'; import * as fs from 'fs'; import * as os from 'os'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { IPC_CHANNELS } from '../../shared/constants'; import type { IPCResult, diff --git a/apps/frontend/src/main/ipc-handlers/settings-handlers.ts b/apps/frontend/src/main/ipc-handlers/settings-handlers.ts index 9aecfca97d..968c44def7 100644 --- a/apps/frontend/src/main/ipc-handlers/settings-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/settings-handlers.ts @@ -2,7 +2,12 @@ import { ipcMain, dialog, app, shell } from 'electron'; import { existsSync, writeFileSync, mkdirSync, statSync, readFileSync } from 'fs'; import { execFileSync } from 'node:child_process'; import path from 'path'; +import { fileURLToPath } from 'url'; import { is } from '@electron-toolkit/utils'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { IPC_CHANNELS, DEFAULT_APP_SETTINGS, DEFAULT_AGENT_PROFILES } from '../../shared/constants'; import type { AppSettings, diff --git a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts index 4f68e10e2e..68fa2ec404 100644 --- a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts @@ -1,15 +1,15 @@ import { ipcMain, BrowserWindow } from 'electron'; import { IPC_CHANNELS, AUTO_BUILD_PATHS, getSpecsDir } from '../../../shared/constants'; -import type { IPCResult, TaskStartOptions, TaskStatus } from '../../../shared/types'; +import type { IPCResult, TaskStartOptions, TaskStatus, ImageAttachment } from '../../../shared/types'; import path from 'path'; -import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync } from 'fs'; +import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync, mkdirSync } from 'fs'; import { spawnSync, execFileSync } from 'child_process'; import { getToolPath } from '../../cli-tool-manager'; import { AgentManager } from '../../agent'; import { fileWatcher } from '../../file-watcher'; import { findTaskAndProject } from './shared'; import { checkGitStatus } from '../../project-initializer'; -import { getClaudeProfileManager } from '../../claude-profile-manager'; +import { initializeClaudeProfileManager, type ClaudeProfileManager } from '../../claude-profile-manager'; import { getPlanPath, persistPlanStatus, @@ -74,6 +74,30 @@ function checkSubtasksCompletion(plan: Record | null): { return { allSubtasks, completedCount, totalCount, allCompleted }; } +/** + * Helper function to ensure profile manager is initialized. + * Returns a discriminated union for type-safe error handling. + * + * @returns Success with profile manager, or failure with error message + */ +async function ensureProfileManagerInitialized(): Promise< + | { success: true; profileManager: ClaudeProfileManager } + | { success: false; error: string } +> { + try { + const profileManager = await initializeClaudeProfileManager(); + return { success: true, profileManager }; + } catch (error) { + console.error('[ensureProfileManagerInitialized] Failed to initialize:', error); + // Include actual error details for debugging while providing actionable guidance + const errorMessage = error instanceof Error ? error.message : String(error); + return { + success: false, + error: `Failed to initialize profile manager. Please check file permissions and disk space. (${errorMessage})` + }; + } +} + /** * Register task execution handlers (start, stop, review, status management, recovery) */ @@ -86,7 +110,7 @@ export function registerTaskExecutionHandlers( */ ipcMain.on( IPC_CHANNELS.TASK_START, - (_, taskId: string, _options?: TaskStartOptions) => { + async (_, taskId: string, _options?: TaskStartOptions) => { console.warn('[TASK_START] Received request for taskId:', taskId); const mainWindow = getMainWindow(); if (!mainWindow) { @@ -94,6 +118,19 @@ export function registerTaskExecutionHandlers( return; } + // Ensure profile manager is initialized before checking auth + // This prevents race condition where auth check runs before profile data loads from disk + const initResult = await ensureProfileManagerInitialized(); + if (!initResult.success) { + mainWindow.webContents.send( + IPC_CHANNELS.TASK_ERROR, + taskId, + initResult.error + ); + return; + } + const profileManager = initResult.profileManager; + // Find task and project const { task, project } = findTaskAndProject(taskId); @@ -129,7 +166,6 @@ export function registerTaskExecutionHandlers( } // Check authentication - Claude requires valid auth to run tasks - const profileManager = getClaudeProfileManager(); if (!profileManager.hasValidAuth()) { console.warn('[TASK_START] No valid authentication for active profile'); mainWindow.webContents.send( @@ -318,7 +354,8 @@ export function registerTaskExecutionHandlers( _, taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ): Promise => { // Find task and project const { task, project } = findTaskAndProject(taskId); @@ -407,10 +444,65 @@ export function registerTaskExecutionHandlers( console.warn('[TASK_REVIEW] Writing QA fix request to:', fixRequestPath); console.warn('[TASK_REVIEW] hasWorktree:', hasWorktree, 'worktreePath:', worktreePath); + // Process images if provided + let imageReferences = ''; + if (images && images.length > 0) { + const imagesDir = path.join(targetSpecDir, 'feedback_images'); + try { + if (!existsSync(imagesDir)) { + mkdirSync(imagesDir, { recursive: true }); + } + const savedImages: string[] = []; + for (const image of images) { + try { + if (!image.data) { + console.warn('[TASK_REVIEW] Skipping image with no data:', image.filename); + continue; + } + // Server-side MIME type validation (defense in depth - frontend also validates) + // Reject missing mimeType to prevent bypass attacks + const ALLOWED_MIME_TYPES = ['image/png', 'image/jpeg', 'image/jpg', 'image/gif', 'image/webp', 'image/svg+xml']; + if (!image.mimeType || !ALLOWED_MIME_TYPES.includes(image.mimeType)) { + console.warn('[TASK_REVIEW] Skipping image with missing or disallowed MIME type:', image.mimeType); + continue; + } + // Sanitize filename to prevent path traversal attacks + const sanitizedFilename = path.basename(image.filename); + if (!sanitizedFilename || sanitizedFilename === '.' || sanitizedFilename === '..') { + console.warn('[TASK_REVIEW] Skipping image with invalid filename:', image.filename); + continue; + } + // Remove data URL prefix if present (e.g., "data:image/png;base64," or "data:image/svg+xml;base64,") + const base64Data = image.data.replace(/^data:image\/[^;]+;base64,/, ''); + const imageBuffer = Buffer.from(base64Data, 'base64'); + const imagePath = path.join(imagesDir, sanitizedFilename); + // Verify the resolved path is within the images directory (defense in depth) + const resolvedPath = path.resolve(imagePath); + const resolvedImagesDir = path.resolve(imagesDir); + if (!resolvedPath.startsWith(resolvedImagesDir + path.sep)) { + console.warn('[TASK_REVIEW] Skipping image with path outside target directory:', image.filename); + continue; + } + writeFileSync(imagePath, imageBuffer); + savedImages.push(`feedback_images/${sanitizedFilename}`); + console.log('[TASK_REVIEW] Saved image:', sanitizedFilename); + } catch (imgError) { + console.error('[TASK_REVIEW] Failed to save image:', image.filename, imgError); + } + } + if (savedImages.length > 0) { + imageReferences = '\n\n## Reference Images\n\n' + + savedImages.map(imgPath => `![Feedback Image](${imgPath})`).join('\n\n'); + } + } catch (dirError) { + console.error('[TASK_REVIEW] Failed to create images directory:', dirError); + } + } + try { writeFileSync( fixRequestPath, - `# QA Fix Request\n\nStatus: REJECTED\n\n## Feedback\n\n${feedback || 'No feedback provided'}\n\nCreated at: ${new Date().toISOString()}\n` + `# QA Fix Request\n\nStatus: REJECTED\n\n## Feedback\n\n${feedback || 'No feedback provided'}${imageReferences}\n\nCreated at: ${new Date().toISOString()}\n` ); } catch (error) { console.error('[TASK_REVIEW] Failed to write QA fix request:', error); @@ -609,7 +701,19 @@ export function registerTaskExecutionHandlers( } // Check authentication before auto-starting - const profileManager = getClaudeProfileManager(); + // Ensure profile manager is initialized to prevent race condition + const initResult = await ensureProfileManagerInitialized(); + if (!initResult.success) { + if (mainWindow) { + mainWindow.webContents.send( + IPC_CHANNELS.TASK_ERROR, + taskId, + initResult.error + ); + } + return { success: false, error: initResult.error }; + } + const profileManager = initResult.profileManager; if (!profileManager.hasValidAuth()) { console.warn('[TASK_UPDATE_STATUS] No valid authentication for active profile'); if (mainWindow) { @@ -940,7 +1044,22 @@ export function registerTaskExecutionHandlers( } // Check authentication before auto-restarting - const profileManager = getClaudeProfileManager(); + // Ensure profile manager is initialized to prevent race condition + const initResult = await ensureProfileManagerInitialized(); + if (!initResult.success) { + // Recovery succeeded but we can't restart without profile manager + return { + success: true, + data: { + taskId, + recovered: true, + newStatus, + message: `Task recovered but cannot restart: ${initResult.error}`, + autoRestarted: false + } + }; + } + const profileManager = initResult.profileManager; if (!profileManager.hasValidAuth()) { console.warn('[Recovery] Auth check failed, cannot auto-restart task'); // Recovery succeeded but we can't restart without auth diff --git a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts index cf2a877827..922fe281ab 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts @@ -10,6 +10,7 @@ import { terminalNameGenerator } from '../terminal-name-generator'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; import { escapeShellArg, escapeShellArgWindows } from '../../shared/utils/shell-escape'; import { getClaudeCliInvocationAsync } from '../claude-cli-utils'; +import { readSettingsFileAsync } from '../settings-utils'; /** @@ -54,8 +55,17 @@ export function registerTerminalHandlers( ipcMain.on( IPC_CHANNELS.TERMINAL_INVOKE_CLAUDE, (_, id: string, cwd?: string) => { - // Use async version to avoid blocking main process during CLI detection - terminalManager.invokeClaudeAsync(id, cwd).catch((error) => { + // Wrap in async IIFE to allow async settings read without blocking + (async () => { + // Read settings asynchronously to check for YOLO mode (dangerously skip permissions) + const settings = await readSettingsFileAsync(); + const dangerouslySkipPermissions = settings?.dangerouslySkipPermissions === true; + + debugLog('[terminal-handlers] Invoking Claude with dangerouslySkipPermissions:', dangerouslySkipPermissions); + + // Use async version to avoid blocking main process during CLI detection + await terminalManager.invokeClaudeAsync(id, cwd, undefined, dangerouslySkipPermissions); + })().catch((error) => { debugError('[terminal-handlers] Failed to invoke Claude:', error); }); } diff --git a/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts index ca91fd70fb..d9ac8cd2f3 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts @@ -138,7 +138,7 @@ function isValidProjectPath(projectPath: string): boolean { return projects.some(p => p.path === projectPath); } -const MAX_TERMINAL_WORKTREES = 12; +// No limit on terminal worktrees - users can create as many as needed /** * Get the default branch from project settings OR env config @@ -267,14 +267,6 @@ async function createTerminalWorktree( }; } - const existing = await listTerminalWorktrees(projectPath); - if (existing.length >= MAX_TERMINAL_WORKTREES) { - return { - success: false, - error: `Maximum of ${MAX_TERMINAL_WORKTREES} terminal worktrees reached.`, - }; - } - // Auto-fix any misconfigured bare repo before worktree operations // This prevents crashes when git worktree operations have incorrectly set bare=true if (fixMisconfiguredBareRepo(projectPath)) { diff --git a/apps/frontend/src/main/memory-service.ts b/apps/frontend/src/main/memory-service.ts index 9e419cc88e..6efc625edf 100644 --- a/apps/frontend/src/main/memory-service.ts +++ b/apps/frontend/src/main/memory-service.ts @@ -9,8 +9,13 @@ import { spawn } from 'child_process'; import * as path from 'path'; +import { fileURLToPath } from 'url'; import * as fs from 'fs'; import { app } from 'electron'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { findPythonCommand, parsePythonCommand } from './python-detector'; import { getConfiguredPythonPath, pythonEnvManager } from './python-env-manager'; import { getMemoriesDir } from './config-paths'; diff --git a/apps/frontend/src/main/sentry.ts b/apps/frontend/src/main/sentry.ts index 0ab4e6602a..1b319003fa 100644 --- a/apps/frontend/src/main/sentry.ts +++ b/apps/frontend/src/main/sentry.ts @@ -23,34 +23,45 @@ import { type SentryErrorEvent } from '../shared/utils/sentry-privacy'; +/** + * Build-time constants defined in electron.vite.config.ts + * These are replaced at build time with actual values from environment variables. + * In development, they come from .env file. In CI builds, from GitHub secrets. + */ +declare const __SENTRY_DSN__: string; +declare const __SENTRY_TRACES_SAMPLE_RATE__: string; +declare const __SENTRY_PROFILES_SAMPLE_RATE__: string; + // In-memory state for current setting (updated via IPC when user toggles) let sentryEnabledState = true; /** - * Get Sentry DSN from environment variable - * - * For local development/testing: - * - Add SENTRY_DSN to your .env file, or - * - Run: SENTRY_DSN=your-dsn npm start + * Get Sentry DSN from build-time constant * - * For CI/CD releases: - * - Set SENTRY_DSN as a GitHub Actions secret - * - * For forks: - * - Without SENTRY_DSN, Sentry is disabled (safe for forks) + * The DSN is embedded at build time via Vite's `define` option. + * - In local development: comes from .env file (loaded by dotenv) + * - In CI builds: comes from GitHub secrets + * - For forks: without SENTRY_DSN, Sentry is disabled (safe for forks) */ function getSentryDsn(): string { - return process.env.SENTRY_DSN || ''; + // __SENTRY_DSN__ is replaced at build time with the actual value + // Falls back to runtime env var for development flexibility + // typeof guard needed for test environments where Vite's define doesn't apply + const buildTimeValue = typeof __SENTRY_DSN__ !== 'undefined' ? __SENTRY_DSN__ : ''; + return buildTimeValue || process.env.SENTRY_DSN || ''; } /** - * Get trace sample rate from environment variable + * Get trace sample rate from build-time constant * Controls performance monitoring sampling (0.0 to 1.0) * Default: 0.1 (10%) in production, 0 in development */ function getTracesSampleRate(): number { - const envValue = process.env.SENTRY_TRACES_SAMPLE_RATE; - if (envValue !== undefined) { + // Try build-time constant first, then runtime env var + // typeof guard needed for test environments where Vite's define doesn't apply + const buildTimeValue = typeof __SENTRY_TRACES_SAMPLE_RATE__ !== 'undefined' ? __SENTRY_TRACES_SAMPLE_RATE__ : ''; + const envValue = buildTimeValue || process.env.SENTRY_TRACES_SAMPLE_RATE; + if (envValue) { const parsed = parseFloat(envValue); if (!isNaN(parsed) && parsed >= 0 && parsed <= 1) { return parsed; @@ -61,13 +72,16 @@ function getTracesSampleRate(): number { } /** - * Get profile sample rate from environment variable + * Get profile sample rate from build-time constant * Controls profiling sampling relative to traces (0.0 to 1.0) * Default: 0.1 (10%) in production, 0 in development */ function getProfilesSampleRate(): number { - const envValue = process.env.SENTRY_PROFILES_SAMPLE_RATE; - if (envValue !== undefined) { + // Try build-time constant first, then runtime env var + // typeof guard needed for test environments where Vite's define doesn't apply + const buildTimeValue = typeof __SENTRY_PROFILES_SAMPLE_RATE__ !== 'undefined' ? __SENTRY_PROFILES_SAMPLE_RATE__ : ''; + const envValue = buildTimeValue || process.env.SENTRY_PROFILES_SAMPLE_RATE; + if (envValue) { const parsed = parseFloat(envValue); if (!isNaN(parsed) && parsed >= 0 && parsed <= 1) { return parsed; @@ -165,3 +179,28 @@ export function setSentryEnabled(enabled: boolean): void { sentryEnabledState = enabled; console.log(`[Sentry] Error reporting ${enabled ? 'enabled' : 'disabled'} (programmatic)`); } + +/** + * Get Sentry environment variables for passing to Python subprocesses + * + * This returns the build-time embedded values so that Python backends + * can also report errors to Sentry in packaged apps. + * + * Usage: + * ```typescript + * const env = { ...getAugmentedEnv(), ...getSentryEnvForSubprocess() }; + * spawn(pythonPath, args, { env }); + * ``` + */ +export function getSentryEnvForSubprocess(): Record { + const dsn = getSentryDsn(); + if (!dsn) { + return {}; + } + + return { + SENTRY_DSN: dsn, + SENTRY_TRACES_SAMPLE_RATE: String(getTracesSampleRate()), + SENTRY_PROFILES_SAMPLE_RATE: String(getProfilesSampleRate()), + }; +} diff --git a/apps/frontend/src/main/settings-utils.ts b/apps/frontend/src/main/settings-utils.ts index 923658ff34..64f3903fd3 100644 --- a/apps/frontend/src/main/settings-utils.ts +++ b/apps/frontend/src/main/settings-utils.ts @@ -9,7 +9,8 @@ */ import { app } from 'electron'; -import { existsSync, readFileSync } from 'fs'; +import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'fs'; +import { promises as fsPromises } from 'fs'; import path from 'path'; /** @@ -41,3 +42,48 @@ export function readSettingsFile(): Record | undefined { return undefined; } } + +/** + * Write settings to disk. + * + * @param settings - The settings object to write + */ +export function writeSettingsFile(settings: Record): void { + const settingsPath = getSettingsPath(); + + // Ensure the directory exists + const dir = path.dirname(settingsPath); + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } + + writeFileSync(settingsPath, JSON.stringify(settings, null, 2), 'utf-8'); +} + +/** + * Read and parse settings from disk asynchronously. + * Returns the raw parsed settings object, or undefined if the file doesn't exist or fails to parse. + * + * This is the non-blocking version of readSettingsFile, safe to use in Electron main process + * without blocking the event loop. + * + * This function does NOT merge with defaults or perform any migrations. + * Callers are responsible for merging with DEFAULT_APP_SETTINGS. + */ +export async function readSettingsFileAsync(): Promise | undefined> { + const settingsPath = getSettingsPath(); + + try { + await fsPromises.access(settingsPath); + } catch { + return undefined; + } + + try { + const content = await fsPromises.readFile(settingsPath, 'utf-8'); + return JSON.parse(content); + } catch { + // Return undefined on parse error - caller will use defaults + return undefined; + } +} diff --git a/apps/frontend/src/main/terminal-name-generator.ts b/apps/frontend/src/main/terminal-name-generator.ts index d442949661..ca12cda8e3 100644 --- a/apps/frontend/src/main/terminal-name-generator.ts +++ b/apps/frontend/src/main/terminal-name-generator.ts @@ -1,7 +1,12 @@ import path from 'path'; +import { fileURLToPath } from 'url'; import { existsSync, readFileSync } from 'fs'; import { spawn } from 'child_process'; import { app } from 'electron'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { EventEmitter } from 'events'; import { detectRateLimit, createSDKRateLimitInfo, getProfileEnv } from './rate-limit-detector'; import { parsePythonCommand } from './python-detector'; diff --git a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts index 92090e0b18..8f44838aa4 100644 --- a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts +++ b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts @@ -1,8 +1,10 @@ import { writeFileSync } from 'fs'; import { tmpdir } from 'os'; +import path from 'path'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type * as pty from '@lydell/node-pty'; import type { TerminalProcess } from '../types'; +import { buildCdCommand } from '../../../shared/utils/shell-escape'; /** Escape special regex characters in a string for safe use in RegExp constructor */ const escapeForRegex = (str: string): string => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); @@ -100,7 +102,7 @@ describe('claude-integration-handler', () => { invokeClaude(terminal, '/tmp/project', undefined, () => null, vi.fn()); const written = vi.mocked(terminal.pty.write).mock.calls[0][0] as string; - expect(written).toContain("cd '/tmp/project' && "); + expect(written).toContain(buildCdCommand('/tmp/project')); expect(written).toContain("PATH='/opt/claude/bin:/usr/bin' "); expect(written).toContain("'/opt/claude bin/claude'\\''s'"); expect(mockReleaseSessionId).toHaveBeenCalledWith('term-1'); @@ -233,8 +235,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; - const tmpDir = escapeForRegex(tmpdir()); - expect(tokenPath).toMatch(new RegExp(`^${tmpDir}/\\.claude-token-1234-[0-9a-f]{16}$`)); + const tokenPrefix = path.join(tmpdir(), '.claude-token-1234-'); + expect(tokenPath).toMatch(new RegExp(`^${escapeForRegex(tokenPrefix)}[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 "); @@ -276,8 +278,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; - const tmpDir = escapeForRegex(tmpdir()); - expect(tokenPath).toMatch(new RegExp(`^${tmpDir}/\\.claude-token-5678-[0-9a-f]{16}$`)); + const tokenPrefix = path.join(tmpdir(), '.claude-token-5678-'); + expect(tokenPath).toMatch(new RegExp(`^${escapeForRegex(tokenPrefix)}[0-9a-f]{16}$`)); expect(tokenContents).toBe("export CLAUDE_CODE_OAUTH_TOKEN='token-value'\n"); const written = vi.mocked(terminal.pty.write).mock.calls[0][0] as string; expect(written).toContain(`source '${tokenPath}'`); diff --git a/apps/frontend/src/main/terminal/__tests__/output-parser.test.ts b/apps/frontend/src/main/terminal/__tests__/output-parser.test.ts new file mode 100644 index 0000000000..417affd2da --- /dev/null +++ b/apps/frontend/src/main/terminal/__tests__/output-parser.test.ts @@ -0,0 +1,258 @@ +import { describe, expect, it } from 'vitest'; +import { + detectClaudeExit, + isClaudeExitOutput, + isClaudeBusyOutput, + isClaudeIdleOutput, + detectClaudeBusyState, + extractClaudeSessionId, + extractRateLimitReset, + extractOAuthToken, + extractEmail, + hasRateLimitMessage, + hasOAuthToken, +} from '../output-parser'; + +describe('output-parser', () => { + describe('detectClaudeExit', () => { + describe('should detect shell prompts (Claude has exited)', () => { + const shellPrompts = [ + // user@hostname patterns + { input: 'user@hostname:~$ ', desc: 'bash: user@hostname:~$' }, + { input: 'user@host:~/projects$ ', desc: 'bash with path' }, + { input: 'root@server:/var/log# ', desc: 'root prompt' }, + { input: 'dev@localhost:~ % ', desc: 'zsh style' }, + + // Bracket prompts + { input: '[user@host directory]$ ', desc: 'bracket prompt' }, + { input: ' [user@host ~]$ ', desc: 'bracket prompt with leading space' }, + + // Virtual environment prompts + { input: '(venv) user@host:~$ ', desc: 'venv prompt' }, + { input: '(base) $ ', desc: 'conda base prompt' }, + { input: '(myenv) ~/projects $ ', desc: 'venv with path' }, + + // Starship/Oh-My-Zsh prompts + { input: '❯ ', desc: 'starship arrow' }, + { input: ' ❯ ', desc: 'starship with space' }, + { input: '➜ ', desc: 'oh-my-zsh arrow' }, + { input: 'λ ', desc: 'lambda prompt' }, + + // Fish shell prompts + { input: '~/projects> ', desc: 'fish path prompt' }, + { input: '/home/user> ', desc: 'fish absolute path' }, + + // Git branch prompts + { input: '(main) $ ', desc: 'git branch prompt' }, + { input: '[feature/test] > ', desc: 'git branch in brackets' }, + + // Simple hostname prompts + { input: 'hostname$ ', desc: 'simple hostname$' }, + { input: 'myserver% ', desc: 'hostname with %' }, + + // Explicit exit messages + { input: 'Goodbye!', desc: 'goodbye message' }, + { input: 'Session ended', desc: 'session ended' }, + { input: 'Exiting Claude', desc: 'exiting claude' }, + ]; + + it.each(shellPrompts)('detects: $desc', ({ input }) => { + expect(detectClaudeExit(input)).toBe(true); + }); + }); + + describe('should NOT detect these as exit (Claude is still active or these are Claude output)', () => { + const notExitPatterns = [ + // Claude's idle prompt + { input: '> ', desc: 'Claude idle prompt (just >)' }, + { input: '\n> ', desc: 'Claude idle prompt after newline' }, + + // Claude busy indicators + { input: '● Working on your request...', desc: 'Claude bullet point' }, + { input: 'Loading...', desc: 'Claude loading' }, + { input: 'Read(/path/to/file.ts)', desc: 'Claude tool execution' }, + + // Content that could false-positive match if not anchored + { input: 'The path is ~/config $HOME', desc: 'path in explanation (should NOT match)' }, + { input: 'Use arr[index] to access elements', desc: 'array access in explanation' }, + { input: 'See the arrow → for details', desc: 'arrow in text' }, + { input: 'File path: ~/projects/test.js', desc: 'file path mid-line' }, + { input: 'Contact user@example.com for help', desc: 'email in text (mid-line)' }, + { input: 'user@example.com: please review this', desc: 'email at line start with colon (should NOT match)' }, + { input: 'admin@company.org: check the logs', desc: 'email at line start with text after colon' }, + { input: 'The variable $HOME is set to /Users/dev', desc: 'shell var in explanation' }, + { input: 'Example: (main) branch is default', desc: 'branch name in explanation' }, + + // Progress indicators + { input: '[Opus 4] 50%', desc: 'Opus progress' }, + { input: '███████░░░ 70%', desc: 'progress bar' }, + ]; + + it.each(notExitPatterns)('ignores: $desc', ({ input }) => { + expect(detectClaudeExit(input)).toBe(false); + }); + }); + + describe('should return false when Claude is busy even if shell-like patterns appear', () => { + it('returns false when busy indicators present with shell pattern', () => { + // This tests the guard in detectClaudeExit that checks busy state first + const mixedOutput = '● Processing...\nuser@host:~$ '; + expect(detectClaudeExit(mixedOutput)).toBe(false); + }); + }); + }); + + describe('isClaudeExitOutput', () => { + it('detects user@hostname pattern at line start', () => { + expect(isClaudeExitOutput('user@hostname:~$ ')).toBe(true); + expect(isClaudeExitOutput('dev@server:/home$ ')).toBe(true); + }); + + it('does not match user@hostname in middle of line', () => { + // With the line-start anchor, this should NOT match + expect(isClaudeExitOutput('Contact user@hostname.com for help')).toBe(false); + }); + + it('detects goodbye message', () => { + expect(isClaudeExitOutput('Goodbye!')).toBe(true); + expect(isClaudeExitOutput('Goodbye')).toBe(true); + }); + + it('detects session ended', () => { + expect(isClaudeExitOutput('Session ended')).toBe(true); + }); + }); + + describe('isClaudeBusyOutput', () => { + const busyPatterns = [ + '● Here is my response', + 'Read(/path/to/file.ts)', + 'Write(/output.json)', + 'Loading...', + 'Thinking...', + 'Analyzing...', + '[Opus 4] 25%', + '[Sonnet 3.5] 50%', + '██████░░░░', + ]; + + it.each(busyPatterns)('detects busy pattern: %s', (input) => { + expect(isClaudeBusyOutput(input)).toBe(true); + }); + + it('returns false for normal text', () => { + expect(isClaudeBusyOutput('Hello, how can I help?')).toBe(false); + }); + }); + + describe('isClaudeIdleOutput', () => { + it('detects Claude idle prompt', () => { + expect(isClaudeIdleOutput('> ')).toBe(true); + expect(isClaudeIdleOutput('\n> ')).toBe(true); + expect(isClaudeIdleOutput(' > ')).toBe(true); + }); + + it('returns false for non-idle output', () => { + expect(isClaudeIdleOutput('Hello')).toBe(false); + expect(isClaudeIdleOutput('● Working')).toBe(false); + }); + }); + + describe('detectClaudeBusyState', () => { + it('returns "busy" when busy indicators present', () => { + expect(detectClaudeBusyState('● Thinking...')).toBe('busy'); + expect(detectClaudeBusyState('Loading...')).toBe('busy'); + }); + + it('returns "idle" when idle prompt present and no busy indicators', () => { + expect(detectClaudeBusyState('> ')).toBe('idle'); + }); + + it('returns null when no state change detected', () => { + expect(detectClaudeBusyState('Hello')).toBe(null); + }); + + it('prioritizes busy over idle', () => { + // If both patterns present, busy should win + expect(detectClaudeBusyState('● Working\n> ')).toBe('busy'); + }); + }); + + describe('extractClaudeSessionId', () => { + it('extracts session ID from "Session ID: xxx" format', () => { + expect(extractClaudeSessionId('Session ID: abc123')).toBe('abc123'); + expect(extractClaudeSessionId('Session: xyz789')).toBe('xyz789'); + }); + + it('extracts session ID from "Resuming session: xxx"', () => { + expect(extractClaudeSessionId('Resuming session: sess_456')).toBe('sess_456'); + }); + + it('returns null when no session ID found', () => { + expect(extractClaudeSessionId('Hello world')).toBe(null); + }); + }); + + describe('extractRateLimitReset', () => { + it('extracts rate limit reset time', () => { + expect(extractRateLimitReset('Limit reached · resets Dec 17 at 6am (Europe/Oslo)')).toBe('Dec 17 at 6am (Europe/Oslo)'); + expect(extractRateLimitReset('Limit reached • resets Jan 1 at noon')).toBe('Jan 1 at noon'); + }); + + it('returns null when no rate limit message', () => { + expect(extractRateLimitReset('Normal output')).toBe(null); + }); + }); + + describe('hasRateLimitMessage', () => { + it('returns true when rate limit message present', () => { + expect(hasRateLimitMessage('Limit reached · resets tomorrow')).toBe(true); + }); + + it('returns false when no rate limit message', () => { + expect(hasRateLimitMessage('Normal text')).toBe(false); + }); + }); + + describe('extractOAuthToken', () => { + it('extracts OAuth token', () => { + const token = 'sk-ant-oat01-abc123_XYZ'; + expect(extractOAuthToken(`Token: ${token}`)).toBe(token); + }); + + it('returns null when no token present', () => { + expect(extractOAuthToken('No token here')).toBe(null); + }); + }); + + describe('hasOAuthToken', () => { + it('returns true when OAuth token present', () => { + expect(hasOAuthToken('sk-ant-oat01-test123')).toBe(true); + }); + + it('returns false when no token', () => { + expect(hasOAuthToken('No token')).toBe(false); + }); + }); + + describe('extractEmail', () => { + it('extracts email from "email:" format', () => { + expect(extractEmail('email: user@example.com')).toBe('user@example.com'); + expect(extractEmail('email:dev@company.org')).toBe('dev@company.org'); + }); + + it('extracts email with whitespace after "email"', () => { + expect(extractEmail('email test@domain.com')).toBe('test@domain.com'); + }); + + it('returns null when no email found', () => { + expect(extractEmail('No email here')).toBe(null); + }); + + it('returns null for "Authenticated as" with space before email', () => { + // Note: The current pattern expects email immediately after "as" + // This documents the actual behavior of the implementation + expect(extractEmail('Authenticated as user@example.com')).toBe(null); + }); + }); +}); diff --git a/apps/frontend/src/main/terminal/claude-integration-handler.ts b/apps/frontend/src/main/terminal/claude-integration-handler.ts index ae420b2d97..606386998c 100644 --- a/apps/frontend/src/main/terminal/claude-integration-handler.ts +++ b/apps/frontend/src/main/terminal/claude-integration-handler.ts @@ -26,6 +26,12 @@ function normalizePathForBash(envPath: string): string { return process.platform === 'win32' ? envPath.replace(/;/g, ':') : envPath; } +/** + * Flag for YOLO mode (skip all permission prompts) + * Extracted as constant to ensure consistency across invokeClaude and invokeClaudeAsync + */ +const YOLO_MODE_FLAG = ' --dangerously-skip-permissions'; + // ============================================================================ // SHARED HELPERS - Used by both sync and async invokeClaude // ============================================================================ @@ -54,6 +60,7 @@ type ClaudeCommandConfig = * @param pathPrefix - PATH prefix for Claude CLI (empty string if not needed) * @param escapedClaudeCmd - Shell-escaped Claude CLI command * @param config - Configuration object with method and required options (discriminated union) + * @param extraFlags - Optional extra flags to append to the command (e.g., '--dangerously-skip-permissions') * @returns Complete shell command string ready for terminal.pty.write() * * @example @@ -69,17 +76,19 @@ export function buildClaudeShellCommand( cwdCommand: string, pathPrefix: string, escapedClaudeCmd: string, - config: ClaudeCommandConfig + config: ClaudeCommandConfig, + extraFlags?: string ): string { + const fullCmd = extraFlags ? `${escapedClaudeCmd}${extraFlags}` : escapedClaudeCmd; switch (config.method) { case 'temp-file': - return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace ${pathPrefix}bash -c "source ${config.escapedTempFile} && rm -f ${config.escapedTempFile} && exec ${escapedClaudeCmd}"\r`; + return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace ${pathPrefix}bash -c "source ${config.escapedTempFile} && rm -f ${config.escapedTempFile} && exec ${fullCmd}"\r`; case 'config-dir': - return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${config.escapedConfigDir} ${pathPrefix}bash -c "exec ${escapedClaudeCmd}"\r`; + return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${config.escapedConfigDir} ${pathPrefix}bash -c "exec ${fullCmd}"\r`; default: - return `${cwdCommand}${pathPrefix}${escapedClaudeCmd}\r`; + return `${cwdCommand}${pathPrefix}${fullCmd}\r`; } } @@ -329,6 +338,40 @@ export function handleClaudeSessionId( } } +/** + * Handle Claude exit detection (user closed Claude, returned to shell) + * + * This is called when we detect that Claude has exited and the terminal + * has returned to a shell prompt. This resets the Claude mode state + * and notifies the renderer to update the UI. + */ +export function handleClaudeExit( + terminal: TerminalProcess, + getWindow: WindowGetter +): void { + // Only handle if we're actually in Claude mode + if (!terminal.isClaudeMode) { + return; + } + + console.warn('[ClaudeIntegration] Claude exit detected, resetting mode for terminal:', terminal.id); + + // Reset Claude mode state + terminal.isClaudeMode = false; + terminal.claudeSessionId = undefined; + + // Persist the session state change + if (terminal.projectPath) { + SessionHandler.persistSession(terminal); + } + + // Notify renderer to update UI + const win = getWindow(); + if (win) { + win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_EXIT, terminal.id); + } +} + /** * Invoke Claude with optional profile override */ @@ -337,14 +380,21 @@ export function invokeClaude( cwd: string | undefined, profileId: string | undefined, getWindow: WindowGetter, - onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void + onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void, + dangerouslySkipPermissions?: boolean ): void { debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE START =========='); debugLog('[ClaudeIntegration:invokeClaude] Terminal ID:', terminal.id); debugLog('[ClaudeIntegration:invokeClaude] Requested profile ID:', profileId); debugLog('[ClaudeIntegration:invokeClaude] CWD:', cwd); + debugLog('[ClaudeIntegration:invokeClaude] Dangerously skip permissions:', dangerouslySkipPermissions); + + // Compute extra flags for YOLO mode + const extraFlags = dangerouslySkipPermissions ? YOLO_MODE_FLAG : undefined; terminal.isClaudeMode = true; + // Store YOLO mode setting so it persists across profile switches + terminal.dangerouslySkipPermissions = dangerouslySkipPermissions; SessionHandler.releaseSessionId(terminal.id); terminal.claudeSessionId = undefined; @@ -399,7 +449,7 @@ export function invokeClaude( { mode: 0o600 } ); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }, extraFlags); debugLog('[ClaudeIntegration:invokeClaude] Executing command (temp file method, history-safe)'); terminal.pty.write(command); profileManager.markProfileUsed(activeProfile.id); @@ -408,7 +458,7 @@ export function invokeClaude( return; } else if (activeProfile.configDir) { const escapedConfigDir = escapeShellArg(activeProfile.configDir); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }, extraFlags); debugLog('[ClaudeIntegration:invokeClaude] Executing command (configDir method, history-safe)'); terminal.pty.write(command); profileManager.markProfileUsed(activeProfile.id); @@ -424,7 +474,7 @@ export function invokeClaude( debugLog('[ClaudeIntegration:invokeClaude] Using terminal environment for non-default profile:', activeProfile.name); } - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }, extraFlags); debugLog('[ClaudeIntegration:invokeClaude] Executing command (default method):', command); terminal.pty.write(command); @@ -506,14 +556,21 @@ export async function invokeClaudeAsync( cwd: string | undefined, profileId: string | undefined, getWindow: WindowGetter, - onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void + onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void, + dangerouslySkipPermissions?: boolean ): Promise { debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE START (async) =========='); debugLog('[ClaudeIntegration:invokeClaudeAsync] Terminal ID:', terminal.id); debugLog('[ClaudeIntegration:invokeClaudeAsync] Requested profile ID:', profileId); debugLog('[ClaudeIntegration:invokeClaudeAsync] CWD:', cwd); + debugLog('[ClaudeIntegration:invokeClaudeAsync] Dangerously skip permissions:', dangerouslySkipPermissions); + + // Compute extra flags for YOLO mode + const extraFlags = dangerouslySkipPermissions ? YOLO_MODE_FLAG : undefined; terminal.isClaudeMode = true; + // Store YOLO mode setting so it persists across profile switches + terminal.dangerouslySkipPermissions = dangerouslySkipPermissions; SessionHandler.releaseSessionId(terminal.id); terminal.claudeSessionId = undefined; @@ -570,7 +627,7 @@ export async function invokeClaudeAsync( { mode: 0o600 } ); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }, extraFlags); debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (temp file method, history-safe)'); terminal.pty.write(command); profileManager.markProfileUsed(activeProfile.id); @@ -579,7 +636,7 @@ export async function invokeClaudeAsync( return; } else if (activeProfile.configDir) { const escapedConfigDir = escapeShellArg(activeProfile.configDir); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }, extraFlags); debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (configDir method, history-safe)'); terminal.pty.write(command); profileManager.markProfileUsed(activeProfile.id); @@ -595,7 +652,7 @@ export async function invokeClaudeAsync( debugLog('[ClaudeIntegration:invokeClaudeAsync] Using terminal environment for non-default profile:', activeProfile.name); } - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }); + const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }, extraFlags); debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (default method):', command); terminal.pty.write(command); @@ -760,7 +817,7 @@ export async function switchClaudeProfile( terminal: TerminalProcess, profileId: string, getWindow: WindowGetter, - invokeClaudeCallback: (terminalId: string, cwd: string | undefined, profileId: string) => Promise, + invokeClaudeCallback: (terminalId: string, cwd: string | undefined, profileId: string, dangerouslySkipPermissions?: boolean) => Promise, clearRateLimitCallback: (terminalId: string) => void ): Promise<{ success: boolean; error?: string }> { // Always-on tracing @@ -841,13 +898,15 @@ export async function switchClaudeProfile( clearRateLimitCallback(terminal.id); const projectPath = terminal.projectPath || terminal.cwd; - console.warn('[ClaudeIntegration:switchClaudeProfile] Invoking Claude with profile:', profileId, '| cwd:', projectPath); + console.warn('[ClaudeIntegration:switchClaudeProfile] Invoking Claude with profile:', profileId, '| cwd:', projectPath, '| YOLO:', terminal.dangerouslySkipPermissions); debugLog('[ClaudeIntegration:switchClaudeProfile] Invoking Claude with new profile:', { terminalId: terminal.id, projectPath, - profileId + profileId, + dangerouslySkipPermissions: terminal.dangerouslySkipPermissions }); - await invokeClaudeCallback(terminal.id, projectPath, profileId); + // Pass the stored dangerouslySkipPermissions value to preserve YOLO mode across profile switches + await invokeClaudeCallback(terminal.id, projectPath, profileId, terminal.dangerouslySkipPermissions); debugLog('[ClaudeIntegration:switchClaudeProfile] Setting active profile in profile manager'); profileManager.setActiveProfile(profileId); diff --git a/apps/frontend/src/main/terminal/output-parser.ts b/apps/frontend/src/main/terminal/output-parser.ts index e955935aaa..4f577c7d7d 100644 --- a/apps/frontend/src/main/terminal/output-parser.ts +++ b/apps/frontend/src/main/terminal/output-parser.ts @@ -159,3 +159,90 @@ export function detectClaudeBusyState(data: string): 'busy' | 'idle' | null { } return null; } + +/** + * Patterns indicating Claude Code has exited and returned to shell + * + * These patterns detect shell prompts that are distinct from Claude's simple ">" prompt. + * Shell prompts typically include: + * - Username and hostname (user@host) + * - Current directory + * - Git branch indicators + * - Shell-specific characters at the end ($, %, #, ❯) + * + * We look for these patterns to distinguish between Claude's idle prompt (">") + * and a proper shell prompt indicating Claude has exited. + */ +const CLAUDE_EXIT_PATTERNS = [ + // Standard shell prompts with path/context (bash/zsh) + // Matches: "user@hostname:~/path$", "hostname:path %", "[user@host path]$" + // Must be at line start to avoid matching user@host in Claude's output + // Requires path indicator after colon to avoid matching emails like "user@example.com:" + /^[a-zA-Z0-9._-]+@[a-zA-Z0-9._-]+:[~/$]/m, // user@hostname:~ or user@hostname:/path + + // Path-based prompts (often in zsh, fish, etc.) + // Matches: "~/projects $", "/home/user %" + // Anchored to line start to avoid matching paths in Claude's explanations + /^[~/][^\s]*\s*[$%#❯]\s*$/m, + + // Prompts with brackets (common in bash) + // Matches: "[user@host directory]$", "(venv) user@host:~$" + // Anchored to avoid matching array access like ${arr[0]} + /^\s*\[[^\]]+\]\s*[$%#]\s*$/m, + + // Virtual environment or conda prompts followed by standard prompt + // Matches: "(venv) $", "(base) user@host:~$" + /^\([a-zA-Z0-9_-]+\)\s*.*[$%#❯]\s*$/m, + + // Starship, Oh My Zsh, Powerlevel10k common patterns + // Matches: "❯", "➜", "λ" at end of line (often colored/styled) + // Anchored to avoid matching Unicode arrows in Claude's explanations + /^\s*[❯➜λ]\s*$/m, + + // Fish shell prompt patterns + // Matches: "user@host ~/path>", "~/path>" + // Anchored to avoid matching file paths ending with > + /^~?\/[^\s]*>\s*$/m, + + // Git branch in prompt followed by prompt character + // Matches: "(main) $", "[git:main] >" + // Anchored to avoid matching code snippets with brackets + /^\s*[([a-zA-Z0-9/_-]+[)\]]\s*[$%#>❯]\s*$/m, + + // Simple but distinctive shell prompts with hostname + // Matches: "hostname$", "hostname %" + /^[a-zA-Z0-9._-]+[$%#]\s*$/m, + + // Detect Claude exit messages (optional, catches explicit exits) + /Goodbye!?\s*$/im, + /Session ended/i, + /Exiting Claude/i, +]; + +/** + * Check if output indicates Claude has exited and returned to shell + * + * This is more specific than shell prompt detection - it looks for patterns + * that indicate we've returned to a shell AFTER being in Claude mode. + */ +export function isClaudeExitOutput(data: string): boolean { + return CLAUDE_EXIT_PATTERNS.some(pattern => pattern.test(data)); +} + +/** + * Detect if Claude has exited based on terminal output + * Returns true if output indicates Claude has exited and shell is ready + * + * This function should be called when the terminal is in Claude mode + * to detect if Claude has exited (user typed /exit, Ctrl+D, etc.) + */ +export function detectClaudeExit(data: string): boolean { + // First, make sure this doesn't look like Claude activity + // If we see Claude busy indicators, Claude hasn't exited + if (isClaudeBusyOutput(data)) { + return false; + } + + // Check for Claude exit patterns (shell prompt return) + return isClaudeExitOutput(data); +} diff --git a/apps/frontend/src/main/terminal/pty-daemon-client.ts b/apps/frontend/src/main/terminal/pty-daemon-client.ts index 3e0bd0f17c..abf1fd8982 100644 --- a/apps/frontend/src/main/terminal/pty-daemon-client.ts +++ b/apps/frontend/src/main/terminal/pty-daemon-client.ts @@ -7,9 +7,14 @@ import * as net from 'net'; import * as path from 'path'; +import { fileURLToPath } from 'url'; import { spawn, ChildProcess } from 'child_process'; import { app } from 'electron'; +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + const SOCKET_PATH = process.platform === 'win32' ? `\\\\.\\pipe\\auto-claude-pty-${process.getuid?.() || 'default'}` diff --git a/apps/frontend/src/main/terminal/terminal-event-handler.ts b/apps/frontend/src/main/terminal/terminal-event-handler.ts index 7f8b061dfc..dddb90af78 100644 --- a/apps/frontend/src/main/terminal/terminal-event-handler.ts +++ b/apps/frontend/src/main/terminal/terminal-event-handler.ts @@ -16,6 +16,7 @@ export interface EventHandlerCallbacks { onRateLimit: (terminal: TerminalProcess, data: string) => void; onOAuthToken: (terminal: TerminalProcess, data: string) => void; onClaudeBusyChange: (terminal: TerminalProcess, isBusy: boolean) => void; + onClaudeExit: (terminal: TerminalProcess) => void; } // Track the last known busy state per terminal to avoid duplicate events @@ -58,6 +59,14 @@ export function handleTerminalData( callbacks.onClaudeBusyChange(terminal, isBusy); } } + + // Detect Claude exit (returned to shell prompt) + // Only check if not busy - busy output takes precedence + if (busyState !== 'busy' && OutputParser.detectClaudeExit(data)) { + callbacks.onClaudeExit(terminal); + // Clear busy state tracking since Claude has exited + lastBusyState.delete(terminal.id); + } } } @@ -97,6 +106,9 @@ export function createEventCallbacks( if (win) { win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_BUSY, terminal.id, isBusy); } + }, + onClaudeExit: (terminal) => { + ClaudeIntegration.handleClaudeExit(terminal, getWindow); } }; } diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 22d7eaecee..bb1f914118 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -189,6 +189,9 @@ export async function restoreTerminal( const win = getWindow(); if (win) { win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, session.id, session.title); + // Always sync worktreeConfig to renderer (even if undefined) to ensure correct state + // This handles both: showing labels after recovery AND clearing stale labels when worktrees are deleted + win.webContents.send(IPC_CHANNELS.TERMINAL_WORKTREE_CONFIG_CHANGE, session.id, terminal.worktreeConfig); } // Defer Claude resume until terminal becomes active (is viewed by user) diff --git a/apps/frontend/src/main/terminal/terminal-manager.ts b/apps/frontend/src/main/terminal/terminal-manager.ts index 5e8fb4c8b8..4c2e8d5c7f 100644 --- a/apps/frontend/src/main/terminal/terminal-manager.ts +++ b/apps/frontend/src/main/terminal/terminal-manager.ts @@ -145,7 +145,7 @@ export class TerminalManager { /** * Invoke Claude in a terminal with optional profile override (async - non-blocking) */ - async invokeClaudeAsync(id: string, cwd?: string, profileId?: string): Promise { + async invokeClaudeAsync(id: string, cwd?: string, profileId?: string, dangerouslySkipPermissions?: boolean): Promise { const terminal = this.terminals.get(id); if (!terminal) { return; @@ -164,7 +164,8 @@ export class TerminalManager { this.terminals, this.getWindow ); - } + }, + dangerouslySkipPermissions ); } @@ -172,7 +173,7 @@ export class TerminalManager { * Invoke Claude in a terminal with optional profile override * @deprecated Use invokeClaudeAsync for non-blocking behavior */ - invokeClaude(id: string, cwd?: string, profileId?: string): void { + invokeClaude(id: string, cwd?: string, profileId?: string, dangerouslySkipPermissions?: boolean): void { const terminal = this.terminals.get(id); if (!terminal) { return; @@ -191,7 +192,8 @@ export class TerminalManager { this.terminals, this.getWindow ); - } + }, + dangerouslySkipPermissions ); } @@ -208,7 +210,7 @@ export class TerminalManager { terminal, profileId, this.getWindow, - async (terminalId, cwd, profileId) => this.invokeClaudeAsync(terminalId, cwd, profileId), + async (terminalId, cwd, profileId, dangerouslySkipPermissions) => this.invokeClaudeAsync(terminalId, cwd, profileId, dangerouslySkipPermissions), (terminalId) => this.lastNotifiedRateLimitReset.delete(terminalId) ); } diff --git a/apps/frontend/src/main/terminal/types.ts b/apps/frontend/src/main/terminal/types.ts index b8ef101230..402bea7a49 100644 --- a/apps/frontend/src/main/terminal/types.ts +++ b/apps/frontend/src/main/terminal/types.ts @@ -19,6 +19,8 @@ export interface TerminalProcess { worktreeConfig?: TerminalWorktreeConfig; /** Whether this terminal has a pending Claude resume that should be triggered on activation */ pendingClaudeResume?: boolean; + /** Whether Claude was invoked with --dangerously-skip-permissions (YOLO mode) */ + dangerouslySkipPermissions?: boolean; } /** diff --git a/apps/frontend/src/main/title-generator.ts b/apps/frontend/src/main/title-generator.ts index 2844449dd3..ae809ba351 100644 --- a/apps/frontend/src/main/title-generator.ts +++ b/apps/frontend/src/main/title-generator.ts @@ -1,7 +1,12 @@ import path from 'path'; +import { fileURLToPath } from 'url'; import { existsSync, readFileSync } from 'fs'; import { spawn } from 'child_process'; import { app } from 'electron'; + +// ESM-compatible __dirname +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); import { EventEmitter } from 'events'; import { detectRateLimit, createSDKRateLimitInfo, getProfileEnv } from './rate-limit-detector'; import { parsePythonCommand, getValidatedPythonPath } from './python-detector'; diff --git a/apps/frontend/src/preload/api/app-update-api.ts b/apps/frontend/src/preload/api/app-update-api.ts index 313c16eded..489fa98b9d 100644 --- a/apps/frontend/src/preload/api/app-update-api.ts +++ b/apps/frontend/src/preload/api/app-update-api.ts @@ -19,6 +19,7 @@ export interface AppUpdateAPI { downloadStableUpdate: () => Promise; installAppUpdate: () => void; getAppVersion: () => Promise; + getDownloadedAppUpdate: () => Promise>; // Event Listeners onAppUpdateAvailable: ( @@ -56,6 +57,9 @@ export const createAppUpdateAPI = (): AppUpdateAPI => ({ getAppVersion: (): Promise => invokeIpc(IPC_CHANNELS.APP_UPDATE_GET_VERSION), + getDownloadedAppUpdate: (): Promise> => + invokeIpc(IPC_CHANNELS.APP_UPDATE_GET_DOWNLOADED), + // Event Listeners onAppUpdateAvailable: ( callback: (info: AppUpdateAvailableEvent) => void diff --git a/apps/frontend/src/preload/api/modules/claude-code-api.ts b/apps/frontend/src/preload/api/modules/claude-code-api.ts index 7c39044b66..a9a4c63346 100644 --- a/apps/frontend/src/preload/api/modules/claude-code-api.ts +++ b/apps/frontend/src/preload/api/modules/claude-code-api.ts @@ -4,10 +4,12 @@ * Provides access to Claude Code CLI management: * - Check installed vs latest version * - Install or update Claude Code + * - Get available versions for rollback + * - Install specific version */ import { IPC_CHANNELS } from '../../../shared/constants'; -import type { ClaudeCodeVersionInfo } from '../../../shared/types/cli'; +import type { ClaudeCodeVersionInfo, ClaudeCodeVersionList, ClaudeInstallationList } from '../../../shared/types/cli'; import { invokeIpc } from './ipc-utils'; /** @@ -30,6 +32,47 @@ export interface ClaudeCodeVersionResult { error?: string; } +/** + * Result of fetching available versions + */ +export interface ClaudeCodeVersionsResult { + success: boolean; + data?: ClaudeCodeVersionList; + error?: string; +} + +/** + * Result of installing a specific version + */ +export interface ClaudeCodeInstallVersionResult { + success: boolean; + data?: { + command: string; + version: string; + }; + error?: string; +} + +/** + * Result of getting installations + */ +export interface ClaudeCodeInstallationsResult { + success: boolean; + data?: ClaudeInstallationList; + error?: string; +} + +/** + * Result of setting active path + */ +export interface ClaudeCodeSetActivePathResult { + success: boolean; + data?: { + path: string; + }; + error?: string; +} + /** * Claude Code API interface exposed to renderer */ @@ -45,6 +88,30 @@ export interface ClaudeCodeAPI { * Opens the user's terminal with the install command */ installClaudeCode: () => Promise; + + /** + * Get available Claude Code CLI versions + * Returns list of versions sorted newest first + */ + getClaudeCodeVersions: () => Promise; + + /** + * Install a specific version of Claude Code CLI + * Opens the user's terminal with the install command for the specified version + */ + installClaudeCodeVersion: (version: string) => Promise; + + /** + * Get all Claude CLI installations found on the system + * Returns list of installations with paths, versions, and sources + */ + getClaudeCodeInstallations: () => Promise; + + /** + * Set the active Claude CLI path + * Updates settings and CLI tool manager cache + */ + setClaudeCodeActivePath: (cliPath: string) => Promise; } /** @@ -55,5 +122,17 @@ export const createClaudeCodeAPI = (): ClaudeCodeAPI => ({ invokeIpc(IPC_CHANNELS.CLAUDE_CODE_CHECK_VERSION), installClaudeCode: (): Promise => - invokeIpc(IPC_CHANNELS.CLAUDE_CODE_INSTALL) + invokeIpc(IPC_CHANNELS.CLAUDE_CODE_INSTALL), + + getClaudeCodeVersions: (): Promise => + invokeIpc(IPC_CHANNELS.CLAUDE_CODE_GET_VERSIONS), + + installClaudeCodeVersion: (version: string): Promise => + invokeIpc(IPC_CHANNELS.CLAUDE_CODE_INSTALL_VERSION, version), + + getClaudeCodeInstallations: (): Promise => + invokeIpc(IPC_CHANNELS.CLAUDE_CODE_GET_INSTALLATIONS), + + setClaudeCodeActivePath: (cliPath: string): Promise => + invokeIpc(IPC_CHANNELS.CLAUDE_CODE_SET_ACTIVE_PATH, cliPath), }); diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index 0fbe408db5..417b73f43f 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -13,7 +13,8 @@ import type { SupportedIDE, SupportedTerminal, WorktreeCreatePROptions, - WorktreeCreatePRResult + WorktreeCreatePRResult, + ImageAttachment } from '../../shared/types'; export interface TaskAPI { @@ -35,7 +36,8 @@ export interface TaskAPI { submitReview: ( taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ) => Promise; updateTaskStatus: ( taskId: string, @@ -112,9 +114,10 @@ export const createTaskAPI = (): TaskAPI => ({ submitReview: ( taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ): Promise => - ipcRenderer.invoke(IPC_CHANNELS.TASK_REVIEW, taskId, approved, feedback), + ipcRenderer.invoke(IPC_CHANNELS.TASK_REVIEW, taskId, approved, feedback, images), updateTaskStatus: ( taskId: string, diff --git a/apps/frontend/src/preload/api/terminal-api.ts b/apps/frontend/src/preload/api/terminal-api.ts index 7ea08f7177..54c2452e04 100644 --- a/apps/frontend/src/preload/api/terminal-api.ts +++ b/apps/frontend/src/preload/api/terminal-api.ts @@ -69,6 +69,7 @@ export interface TerminalAPI { onTerminalOutput: (callback: (id: string, data: string) => void) => () => void; onTerminalExit: (callback: (id: string, exitCode: number) => void) => () => void; onTerminalTitleChange: (callback: (id: string, title: string) => void) => () => void; + onTerminalWorktreeConfigChange: (callback: (id: string, config: TerminalWorktreeConfig | undefined) => void) => () => void; onTerminalClaudeSession: (callback: (id: string, sessionId: string) => void) => () => void; onTerminalRateLimit: (callback: (info: RateLimitInfo) => void) => () => void; onTerminalOAuthToken: ( @@ -78,6 +79,7 @@ export interface TerminalAPI { callback: (info: { terminalId: string; profileId: string; profileName: string }) => void ) => () => void; onTerminalClaudeBusy: (callback: (id: string, isBusy: boolean) => void) => () => void; + onTerminalClaudeExit: (callback: (id: string) => void) => () => void; onTerminalPendingResume: (callback: (id: string, sessionId?: string) => void) => () => void; // Claude Profile Management @@ -227,6 +229,22 @@ export const createTerminalAPI = (): TerminalAPI => ({ }; }, + onTerminalWorktreeConfigChange: ( + callback: (id: string, config: TerminalWorktreeConfig | undefined) => void + ): (() => void) => { + const handler = ( + _event: Electron.IpcRendererEvent, + id: string, + config: TerminalWorktreeConfig | undefined + ): void => { + callback(id, config); + }; + ipcRenderer.on(IPC_CHANNELS.TERMINAL_WORKTREE_CONFIG_CHANGE, handler); + return () => { + ipcRenderer.removeListener(IPC_CHANNELS.TERMINAL_WORKTREE_CONFIG_CHANGE, handler); + }; + }, + onTerminalClaudeSession: ( callback: (id: string, sessionId: string) => void ): (() => void) => { @@ -304,6 +322,21 @@ export const createTerminalAPI = (): TerminalAPI => ({ }; }, + onTerminalClaudeExit: ( + callback: (id: string) => void + ): (() => void) => { + const handler = ( + _event: Electron.IpcRendererEvent, + id: string + ): void => { + callback(id); + }; + ipcRenderer.on(IPC_CHANNELS.TERMINAL_CLAUDE_EXIT, handler); + return () => { + ipcRenderer.removeListener(IPC_CHANNELS.TERMINAL_CLAUDE_EXIT, handler); + }; + }, + onTerminalPendingResume: ( callback: (id: string, sessionId?: string) => void ): (() => void) => { diff --git a/apps/frontend/src/renderer/components/ClaudeCodeStatusBadge.tsx b/apps/frontend/src/renderer/components/ClaudeCodeStatusBadge.tsx index 11a98bbf1d..a46010d698 100644 --- a/apps/frontend/src/renderer/components/ClaudeCodeStatusBadge.tsx +++ b/apps/frontend/src/renderer/components/ClaudeCodeStatusBadge.tsx @@ -9,6 +9,7 @@ import { Download, RefreshCw, ExternalLink, + FolderOpen, } from "lucide-react"; import { Button } from "./ui/button"; import { Popover, PopoverContent, PopoverTrigger } from "./ui/popover"; @@ -23,8 +24,15 @@ import { AlertDialogHeader, AlertDialogTitle, } from "./ui/alert-dialog"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "./ui/select"; import { cn } from "../lib/utils"; -import type { ClaudeCodeVersionInfo } from "../../shared/types/cli"; +import type { ClaudeCodeVersionInfo, ClaudeInstallationInfo } from "../../shared/types/cli"; interface ClaudeCodeStatusBadgeProps { className?: string; @@ -34,6 +42,8 @@ type StatusType = "loading" | "installed" | "outdated" | "not-found" | "error"; // Check every 24 hours const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; +// Delay before re-checking version after install/update +const VERSION_RECHECK_DELAY_MS = 5000; /** * Claude Code CLI status badge for the sidebar. @@ -48,6 +58,21 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) const [isOpen, setIsOpen] = useState(false); const [showUpdateWarning, setShowUpdateWarning] = useState(false); + // Version rollback state + const [availableVersions, setAvailableVersions] = useState([]); + const [isLoadingVersions, setIsLoadingVersions] = useState(false); + const [versionsError, setVersionsError] = useState(null); + const [selectedVersion, setSelectedVersion] = useState(null); + const [showRollbackWarning, setShowRollbackWarning] = useState(false); + const [installError, setInstallError] = useState(null); + + // CLI path selection state + const [installations, setInstallations] = useState([]); + const [isLoadingInstallations, setIsLoadingInstallations] = useState(false); + const [installationsError, setInstallationsError] = useState(null); + const [selectedInstallation, setSelectedInstallation] = useState(null); + const [showPathChangeWarning, setShowPathChangeWarning] = useState(false); + // Check Claude Code version const checkVersion = useCallback(async () => { try { @@ -78,6 +103,54 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) } }, []); + // Fetch available versions + const fetchVersions = useCallback(async () => { + if (!window.electronAPI?.getClaudeCodeVersions) { + return; + } + + setIsLoadingVersions(true); + setVersionsError(null); + + try { + const result = await window.electronAPI.getClaudeCodeVersions(); + if (result.success && result.data) { + setAvailableVersions(result.data.versions); + } else { + setVersionsError(result.error || "Failed to load versions"); + } + } catch (err) { + console.error("Failed to fetch versions:", err); + setVersionsError("Failed to load versions"); + } finally { + setIsLoadingVersions(false); + } + }, []); + + // Fetch CLI installations + const fetchInstallations = useCallback(async () => { + if (!window.electronAPI?.getClaudeCodeInstallations) { + return; + } + + setIsLoadingInstallations(true); + setInstallationsError(null); + + try { + const result = await window.electronAPI.getClaudeCodeInstallations(); + if (result.success && result.data) { + setInstallations(result.data.installations); + } else { + setInstallationsError(result.error || "Failed to load installations"); + } + } catch (err) { + console.error("Failed to fetch installations:", err); + setInstallationsError("Failed to load installations"); + } finally { + setIsLoadingInstallations(false); + } + }, []); + // Initial check and periodic re-check useEffect(() => { checkVersion(); @@ -90,12 +163,28 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) return () => clearInterval(interval); }, [checkVersion]); + // Fetch versions when popover opens and Claude is installed + useEffect(() => { + if (isOpen && versionInfo?.installed && availableVersions.length === 0) { + fetchVersions(); + } + }, [isOpen, versionInfo?.installed, availableVersions.length, fetchVersions]); + + // Fetch installations when popover opens + useEffect(() => { + if (isOpen && installations.length === 0) { + fetchInstallations(); + } + }, [isOpen, installations.length, fetchInstallations]); + // Perform the actual install/update const performInstall = async () => { setIsInstalling(true); setShowUpdateWarning(false); + setInstallError(null); try { if (!window.electronAPI?.installClaudeCode) { + setInstallError("Installation not available"); return; } @@ -105,15 +194,85 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) // Re-check after a delay setTimeout(() => { checkVersion(); - }, 5000); + }, VERSION_RECHECK_DELAY_MS); + } else { + setInstallError(result.error || "Installation failed"); } } catch (err) { console.error("Failed to install Claude Code:", err); + setInstallError(err instanceof Error ? err.message : "Installation failed"); } finally { setIsInstalling(false); } }; + // Perform version rollback/switch + const performVersionSwitch = async () => { + if (!selectedVersion) return; + + setIsInstalling(true); + setShowRollbackWarning(false); + setInstallError(null); + + try { + if (!window.electronAPI?.installClaudeCodeVersion) { + setInstallError("Version switching not available"); + return; + } + + const result = await window.electronAPI.installClaudeCodeVersion(selectedVersion); + + if (result.success) { + // Re-check after a delay + setTimeout(() => { + checkVersion(); + }, VERSION_RECHECK_DELAY_MS); + } else { + setInstallError(result.error || "Failed to switch version"); + } + } catch (err) { + console.error("Failed to switch Claude Code version:", err); + setInstallError(err instanceof Error ? err.message : "Failed to switch version"); + } finally { + setIsInstalling(false); + setSelectedVersion(null); + } + }; + + // Perform CLI path switch + const performPathSwitch = async () => { + if (!selectedInstallation) return; + + setIsInstalling(true); + setShowPathChangeWarning(false); + setInstallError(null); + + try { + if (!window.electronAPI?.setClaudeCodeActivePath) { + setInstallError("Path switching not available"); + return; + } + + const result = await window.electronAPI.setClaudeCodeActivePath(selectedInstallation); + + if (result.success) { + // Re-check version and refresh installations + setTimeout(() => { + checkVersion(); + fetchInstallations(); + }, VERSION_RECHECK_DELAY_MS); + } else { + setInstallError(result.error || "Failed to switch CLI path"); + } + } catch (err) { + console.error("Failed to switch Claude CLI path:", err); + setInstallError(err instanceof Error ? err.message : "Failed to switch CLI path"); + } finally { + setIsInstalling(false); + setSelectedInstallation(null); + } + }; + // Handle install/update button click const handleInstall = () => { if (status === "outdated") { @@ -125,6 +284,34 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) } }; + // Handle installation selection + const handleInstallationSelect = (cliPath: string) => { + // Don't do anything if it's the currently active installation + const installation = installations.find(i => i.path === cliPath); + if (installation?.isActive) { + return; + } + setInstallError(null); + setSelectedInstallation(cliPath); + setShowPathChangeWarning(true); + }; + + // Normalize version string by removing 'v' prefix for comparison + const normalizeVersion = (v: string) => v.replace(/^v/, ''); + + // Handle version selection + const handleVersionSelect = (version: string) => { + // Don't do anything if it's the currently installed version (normalize both for comparison) + const normalizedSelected = normalizeVersion(version); + const normalizedInstalled = versionInfo?.installed ? normalizeVersion(versionInfo.installed) : ''; + if (normalizedSelected === normalizedInstalled) { + return; + } + setInstallError(null); + setSelectedVersion(version); + setShowRollbackWarning(true); + }; + // Get status indicator color const getStatusColor = () => { switch (status) { @@ -252,6 +439,20 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps) {versionInfo.latest} )} + {versionInfo.path && ( +
+ + + {t("navigation:claudeCode.path", "Path")}: + + + {versionInfo.path} + +
+ )} {lastChecked && (
{t("navigation:claudeCode.lastChecked", "Last checked")}: @@ -292,6 +493,107 @@ export function ClaudeCodeStatusBadge({ className }: ClaudeCodeStatusBadgeProps)
+ {/* Install/Update error display */} + {installError && ( +
+ + {installError} +
+ )} + + {/* Version selector - only show when Claude is installed */} + {versionInfo?.installed && ( +
+ + +
+ )} + + {/* CLI Installation selector - show when multiple installations are found */} + {installations.length > 1 && ( +
+ + +
+ )} + {/* Learn more link */} )} @@ -112,7 +117,7 @@ export function IdeaDetailPanel({ idea, onClose, onConvert, onGoToTask, onDismis
)} diff --git a/apps/frontend/src/renderer/components/ideation/Ideation.tsx b/apps/frontend/src/renderer/components/ideation/Ideation.tsx index d372aa7062..ce5feaa0f0 100644 --- a/apps/frontend/src/renderer/components/ideation/Ideation.tsx +++ b/apps/frontend/src/renderer/components/ideation/Ideation.tsx @@ -41,6 +41,7 @@ export function Ideation({ projectId, onGoToTask }: IdeationProps) { summary, activeIdeas, selectedIds, + convertingIdeas, setSelectedIdea, setActiveTab, setShowConfigDialog, @@ -217,6 +218,7 @@ export function Ideation({ projectId, onGoToTask }: IdeationProps) { onConvert={handleConvertToTask} onGoToTask={handleGoToTask} onDismiss={handleDismiss} + isConverting={convertingIdeas.has(selectedIdea.id)} /> )} diff --git a/apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts b/apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts index 71359c98b3..0a6c7b22d6 100644 --- a/apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts +++ b/apps/frontend/src/renderer/components/ideation/hooks/useIdeation.ts @@ -1,4 +1,6 @@ -import { useEffect, useState, useCallback } from 'react'; +import { useEffect, useState, useCallback, useRef } from 'react'; +import { useTranslation } from 'react-i18next'; +import { toast } from '../../../hooks/use-toast'; import { useIdeationStore, loadIdeation, @@ -27,6 +29,7 @@ interface UseIdeationOptions { export function useIdeation(projectId: string, options: UseIdeationOptions = {}) { const { onGoToTask, showArchived: externalShowArchived } = options; + const { t } = useTranslation('common'); const session = useIdeationStore((state) => state.session); const generationStatus = useIdeationStore((state) => state.generationStatus); const isGenerating = useIdeationStore((state) => state.isGenerating); @@ -48,6 +51,9 @@ export function useIdeation(projectId: string, options: UseIdeationOptions = {}) const [pendingAction, setPendingAction] = useState<'generate' | 'refresh' | 'append' | null>(null); const [showAddMoreDialog, setShowAddMoreDialog] = useState(false); const [typesToAdd, setTypesToAdd] = useState([]); + const [convertingIdeas, setConvertingIdeas] = useState>(new Set()); + // Ref for synchronous tracking - prevents race condition from stale React state closure + const convertingIdeaRef = useRef>(new Set()); const { hasToken, isLoading: isCheckingToken, checkAuth } = useIdeationAuth(); @@ -130,11 +136,42 @@ export function useIdeation(projectId: string, options: UseIdeationOptions = {}) }; const handleConvertToTask = async (idea: Idea) => { - const result = await window.electronAPI.convertIdeaToTask(projectId, idea.id); - if (result.success && result.data) { - // Store the taskId on the idea so we can navigate to it later - useIdeationStore.getState().setIdeaTaskId(idea.id, result.data.id); - loadTasks(projectId); + // Guard: use ref for synchronous check to prevent race condition from stale state closure + // React state is captured at render time, so rapid clicks would both see empty set + if (convertingIdeaRef.current.has(idea.id)) { + return; + } + + // Mark as converting - update ref synchronously first, then state for UI + convertingIdeaRef.current.add(idea.id); + setConvertingIdeas(new Set(convertingIdeaRef.current)); + + try { + const result = await window.electronAPI.convertIdeaToTask(projectId, idea.id); + if (result.success && result.data) { + // Store the taskId on the idea so we can navigate to it later + useIdeationStore.getState().setIdeaTaskId(idea.id, result.data.id); + loadTasks(projectId); + } else { + // Show error toast when conversion fails (e.g., already converted, idea not found) + toast({ + variant: 'destructive', + title: t('ideation.conversionFailed'), + description: result.error || t('ideation.conversionFailedDescription') + }); + } + } catch (error) { + // Handle unexpected errors (network issues, etc.) + console.error('Failed to convert idea to task:', error); + toast({ + variant: 'destructive', + title: t('ideation.conversionError'), + description: t('ideation.conversionErrorDescription') + }); + } finally { + // Always clear converting state - update ref first, then state + convertingIdeaRef.current.delete(idea.id); + setConvertingIdeas(new Set(convertingIdeaRef.current)); } }; @@ -228,6 +265,7 @@ export function useIdeation(projectId: string, options: UseIdeationOptions = {}) activeIdeas, archivedIdeas, selectedIds, + convertingIdeas, // Actions setSelectedIdea, diff --git a/apps/frontend/src/renderer/components/settings/AdvancedSettings.tsx b/apps/frontend/src/renderer/components/settings/AdvancedSettings.tsx index ac3d05fa94..441fe62704 100644 --- a/apps/frontend/src/renderer/components/settings/AdvancedSettings.tsx +++ b/apps/frontend/src/renderer/components/settings/AdvancedSettings.tsx @@ -80,11 +80,60 @@ export function AdvancedSettings({ settings, onSettingsChange, section, version // Stable downgrade state (shown when user turns off beta while on prerelease) const [stableDowngradeInfo, setStableDowngradeInfo] = useState(null); - // Check for updates on mount + // Check for updates on mount, including any already-downloaded updates useEffect(() => { - if (section === 'updates') { - checkForAppUpdates(); + if (section !== 'updates') { + return; } + + let isCancelled = false; + + // First check if an update was already downloaded, then check for new updates + (async () => { + // Check if an update was already downloaded (e.g., auto-downloaded in background) + try { + const result = await window.electronAPI.getDownloadedAppUpdate(); + + // Skip state updates if component unmounted or section changed + if (isCancelled) return; + + if (result.success && result.data) { + // An update was already downloaded - show "Install and Restart" button + setAppUpdateInfo(result.data); + setIsAppUpdateDownloaded(true); + console.log('[AdvancedSettings] Found already-downloaded update:', result.data.version); + return; // Don't check for new updates if we already have one downloaded + } + } catch (err) { + console.error('Failed to check for downloaded update:', err); + if (isCancelled) return; + } + + // Only check for available updates if no update is already downloaded + // (electron-updater reports no available update when one is already downloaded, + // which would clear our appUpdateInfo and lose the version metadata) + // Inline the update check with cancellation support + setIsCheckingAppUpdate(true); + try { + const result = await window.electronAPI.checkAppUpdate(); + if (isCancelled) return; + if (result.success && result.data) { + setAppUpdateInfo(result.data); + } else { + setAppUpdateInfo(null); + } + } catch (err) { + console.error('Failed to check for app updates:', err); + } finally { + if (!isCancelled) { + setIsCheckingAppUpdate(false); + } + } + })(); + + return () => { + isCancelled = true; + }; }, [section]); // Listen for app update events diff --git a/apps/frontend/src/renderer/components/settings/DevToolsSettings.tsx b/apps/frontend/src/renderer/components/settings/DevToolsSettings.tsx index 24e1cd659b..1691f69798 100644 --- a/apps/frontend/src/renderer/components/settings/DevToolsSettings.tsx +++ b/apps/frontend/src/renderer/components/settings/DevToolsSettings.tsx @@ -1,10 +1,11 @@ import { useEffect, useState, useCallback } from 'react'; import { useTranslation } from 'react-i18next'; -import { Code, Terminal, RefreshCw, Loader2, Check, FolderOpen } from 'lucide-react'; +import { Code, Terminal, RefreshCw, Loader2, Check, FolderOpen, AlertTriangle } from 'lucide-react'; import { Label } from '../ui/label'; import { Input } from '../ui/input'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select'; import { Button } from '../ui/button'; +import { Switch } from '../ui/switch'; import { SettingsSection } from './SettingsSection'; import type { AppSettings, SupportedIDE, SupportedTerminal } from '../../../shared/types'; @@ -364,6 +365,37 @@ export function DevToolsSettings({ settings, onSettingsChange }: DevToolsSetting )} + {/* YOLO Mode Toggle */} +
+
+
+ + +
+ { + onSettingsChange({ + ...settings, + dangerouslySkipPermissions: checked + }); + }} + /> +
+

+ {t('devtools.yoloMode.description', 'Start Claude with --dangerously-skip-permissions flag, bypassing all safety prompts. Use with extreme caution.')} +

+ {settings.dangerouslySkipPermissions && ( +

+ + {t('devtools.yoloMode.warning', 'This mode bypasses Claude\'s permission system. Only enable if you fully trust the code being executed.')} +

+ )} +
+ {/* Detection Summary */} {detectedTools && !isDetecting && (
diff --git a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx index e51fb2e72c..11c6f1de7c 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx @@ -118,13 +118,15 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, }; const handleReject = async () => { - if (!state.feedback.trim()) { + // Allow submission if there's text feedback OR images attached + if (!state.feedback.trim() && state.feedbackImages.length === 0) { return; } state.setIsSubmitting(true); - await submitReview(task.id, false, state.feedback); + await submitReview(task.id, false, state.feedback, state.feedbackImages); state.setIsSubmitting(false); state.setFeedback(''); + state.setFeedbackImages([]); }; const handleDelete = async () => { @@ -516,6 +518,8 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, showConflictDialog={state.showConflictDialog} onFeedbackChange={state.setFeedback} onReject={handleReject} + images={state.feedbackImages} + onImagesChange={state.setFeedbackImages} onMerge={handleMerge} onDiscard={handleDiscard} onShowDiscardDialog={state.setShowDiscardDialog} diff --git a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx index d27a168931..c22085a263 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx @@ -1,4 +1,4 @@ -import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, WorktreeCreatePRResult } from '../../../shared/types'; +import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, ImageAttachment, WorktreeCreatePRResult } from '../../../shared/types'; import { StagedSuccessMessage, WorkspaceStatus, @@ -33,6 +33,10 @@ interface TaskReviewProps { showConflictDialog: boolean; onFeedbackChange: (value: string) => void; onReject: () => void; + /** Image attachments for visual feedback */ + images?: ImageAttachment[]; + /** Callback when images change */ + onImagesChange?: (images: ImageAttachment[]) => void; onMerge: () => void; onDiscard: () => void; onShowDiscardDialog: (show: boolean) => void; @@ -81,6 +85,8 @@ export function TaskReview({ showConflictDialog, onFeedbackChange, onReject, + images, + onImagesChange, onMerge, onDiscard, onShowDiscardDialog, @@ -157,6 +163,8 @@ export function TaskReview({ isSubmitting={isSubmitting} onFeedbackChange={onFeedbackChange} onReject={onReject} + images={images} + onImagesChange={onImagesChange} /> {/* Discard Confirmation Dialog */} diff --git a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts index ea310269d7..2a01b3b432 100644 --- a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts +++ b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts @@ -1,7 +1,7 @@ import { useState, useRef, useEffect, useCallback } from 'react'; import { useProjectStore } from '../../../stores/project-store'; import { checkTaskRunning, isIncompleteHumanReview, getTaskProgress, useTaskStore, loadTasks } from '../../../stores/task-store'; -import type { Task, TaskLogs, TaskLogPhase, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo } from '../../../../shared/types'; +import type { Task, TaskLogs, TaskLogPhase, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, ImageAttachment } from '../../../../shared/types'; /** * Validates task subtasks structure to prevent infinite loops during resume. @@ -50,6 +50,7 @@ export interface UseTaskDetailOptions { export function useTaskDetail({ task }: UseTaskDetailOptions) { const [feedback, setFeedback] = useState(''); + const [feedbackImages, setFeedbackImages] = useState([]); const [isSubmitting, setIsSubmitting] = useState(false); const [activeTab, setActiveTab] = useState('overview'); const [isUserScrolledUp, setIsUserScrolledUp] = useState(false); @@ -161,6 +162,11 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { } }, [activeTab]); + // Reset feedback images when task changes to prevent image leakage between tasks + useEffect(() => { + setFeedbackImages([]); + }, [task.id]); + // Load worktree status when task is in human_review useEffect(() => { if (needsReview) { @@ -255,6 +261,26 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { }); }, []); + // Add a feedback image + const addFeedbackImage = useCallback((image: ImageAttachment) => { + setFeedbackImages(prev => [...prev, image]); + }, []); + + // Add multiple feedback images at once + const addFeedbackImages = useCallback((images: ImageAttachment[]) => { + setFeedbackImages(prev => [...prev, ...images]); + }, []); + + // Remove a feedback image by ID + const removeFeedbackImage = useCallback((imageId: string) => { + setFeedbackImages(prev => prev.filter(img => img.id !== imageId)); + }, []); + + // Clear all feedback images + const clearFeedbackImages = useCallback(() => { + setFeedbackImages([]); + }, []); + // Track if we've already loaded preview for this task to prevent infinite loops const hasLoadedPreviewRef = useRef(null); @@ -404,6 +430,7 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { return { // State feedback, + feedbackImages, isSubmitting, activeTab, isUserScrolledUp, @@ -447,6 +474,7 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { // Setters setFeedback, + setFeedbackImages, setIsSubmitting, setActiveTab, setIsUserScrolledUp, @@ -482,6 +510,10 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { handleLogsScroll, togglePhase, loadMergePreview, + addFeedbackImage, + addFeedbackImages, + removeFeedbackImage, + clearFeedbackImages, handleReviewAgain, reloadPlanForIncompleteTask, }; diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/MergePreviewSummary.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/MergePreviewSummary.tsx index c04f54a124..4592234129 100644 --- a/apps/frontend/src/renderer/components/task-detail/task-review/MergePreviewSummary.tsx +++ b/apps/frontend/src/renderer/components/task-detail/task-review/MergePreviewSummary.tsx @@ -1,4 +1,5 @@ import { CheckCircle, AlertTriangle } from 'lucide-react'; +import { useTranslation } from 'react-i18next'; import { Button } from '../../ui/button'; import { cn } from '../../../lib/utils'; import type { MergeConflict, MergeStats, GitConflictInfo } from '../../../../shared/types'; @@ -20,6 +21,7 @@ export function MergePreviewSummary({ mergePreview, onShowConflictDialog }: MergePreviewSummaryProps) { + const { t } = useTranslation(['taskReview']); const hasGitConflicts = mergePreview.gitConflicts?.hasConflicts; const hasAIConflicts = mergePreview.conflicts.length > 0; const hasHighSeverity = mergePreview.conflicts.some( @@ -72,8 +74,8 @@ export function MergePreviewSummary({

Branch has diverged - AI will resolve

- The main branch has {mergePreview.gitConflicts.commitsBehind} new commit{mergePreview.gitConflicts.commitsBehind !== 1 ? 's' : ''} since this worktree was created. - {mergePreview.gitConflicts.conflictingFiles.length} file{mergePreview.gitConflicts.conflictingFiles.length !== 1 ? 's' : ''} will need intelligent merging: + {t('taskReview:merge.branchHasNewCommitsSinceWorktree', { branch: mergePreview.gitConflicts.baseBranch, count: mergePreview.gitConflicts.commitsBehind })} + {' '}{t('taskReview:merge.filesNeedIntelligentMerging', { count: mergePreview.gitConflicts.conflictingFiles.length })}

    {mergePreview.gitConflicts.conflictingFiles.map((file, idx) => ( diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx index 9f07d92f49..149f5dbeb4 100644 --- a/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx +++ b/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx @@ -1,54 +1,376 @@ -import { AlertCircle, RotateCcw, Loader2 } from 'lucide-react'; +import { useCallback, useRef, useState, type ClipboardEvent, type DragEvent } from 'react'; +import { useTranslation } from 'react-i18next'; +import { AlertCircle, RotateCcw, Loader2, Image as ImageIcon, X } from 'lucide-react'; import { Button } from '../../ui/button'; import { Textarea } from '../../ui/textarea'; +import { + generateImageId, + blobToBase64, + createThumbnail, + isValidImageMimeType, + resolveFilename +} from '../../ImageUpload'; +import { cn } from '../../../lib/utils'; +import type { ImageAttachment } from '../../../../shared/types'; +import { + MAX_IMAGES_PER_TASK, + ALLOWED_IMAGE_TYPES_DISPLAY +} from '../../../../shared/constants'; interface QAFeedbackSectionProps { feedback: string; isSubmitting: boolean; onFeedbackChange: (value: string) => void; onReject: () => void; + /** Image attachments for visual feedback - optional for backward compatibility */ + images?: ImageAttachment[]; + /** Callback when images change - optional for backward compatibility */ + onImagesChange?: (images: ImageAttachment[]) => void; } /** * Displays the QA feedback section where users can request changes + * Supports image paste and drag-drop for visual feedback */ export function QAFeedbackSection({ feedback, isSubmitting, onFeedbackChange, - onReject + onReject, + images = [], + onImagesChange }: QAFeedbackSectionProps) { + const { t } = useTranslation('tasks'); + + // Feature is enabled when onImagesChange callback is provided + const imageUploadEnabled = !!onImagesChange; + + // Ref for the textarea + const textareaRef = useRef(null); + + // Local state for UI feedback + const [isDragOverTextarea, setIsDragOverTextarea] = useState(false); + const [pasteSuccess, setPasteSuccess] = useState(false); + const [error, setError] = useState(null); + + /** + * Handle paste event for screenshot support + */ + const handlePaste = useCallback(async (e: ClipboardEvent) => { + // Skip image handling if feature is not enabled + if (!onImagesChange) return; + + const clipboardItems = e.clipboardData?.items; + if (!clipboardItems) return; + + // Find image items in clipboard + const imageItems: DataTransferItem[] = []; + for (let i = 0; i < clipboardItems.length; i++) { + const item = clipboardItems[i]; + if (item.type.startsWith('image/')) { + imageItems.push(item); + } + } + + // If no images, allow normal paste behavior + if (imageItems.length === 0) return; + + // Prevent default paste when we have images + e.preventDefault(); + + // Check if we can add more images + const remainingSlots = MAX_IMAGES_PER_TASK - images.length; + if (remainingSlots <= 0) { + setError(t('feedback.maxImagesError', { count: MAX_IMAGES_PER_TASK })); + return; + } + + setError(null); + + // Process image items + const newImages: ImageAttachment[] = []; + const existingFilenames = images.map(img => img.filename); + + for (const item of imageItems.slice(0, remainingSlots)) { + const file = item.getAsFile(); + if (!file) continue; + + // Validate image type + if (!isValidImageMimeType(file.type)) { + setError(t('feedback.invalidTypeError', { types: ALLOWED_IMAGE_TYPES_DISPLAY })); + continue; + } + + try { + const dataUrl = await blobToBase64(file); + const thumbnail = await createThumbnail(dataUrl); + + // Generate filename for pasted images (screenshot-timestamp.ext) + // Map MIME types to proper file extensions (handles svg+xml -> svg, etc.) + const mimeToExtension: Record = { + 'image/svg+xml': 'svg', + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + 'image/webp': 'webp', + }; + const extension = mimeToExtension[file.type] || file.type.split('/')[1] || 'png'; + const baseFilename = `screenshot-${Date.now()}.${extension}`; + const resolvedFilename = resolveFilename(baseFilename, [ + ...existingFilenames, + ...newImages.map(img => img.filename) + ]); + + newImages.push({ + id: generateImageId(), + filename: resolvedFilename, + mimeType: file.type, + size: file.size, + data: dataUrl.split(',')[1], // Store base64 without data URL prefix + thumbnail + }); + } catch (error) { + console.error('[QAFeedbackSection] Failed to process pasted image:', error); + setError(t('feedback.processingError', 'Failed to process pasted image')); + } + } + + if (newImages.length > 0) { + onImagesChange([...images, ...newImages]); + // Show success feedback + setPasteSuccess(true); + setTimeout(() => setPasteSuccess(false), 2000); + } + }, [images, onImagesChange, t]); + + /** + * Handle drag over textarea for image drops + */ + const handleTextareaDragOver = useCallback((e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(true); + }, []); + + /** + * Handle drag leave from textarea + */ + const handleTextareaDragLeave = useCallback((e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(false); + }, []); + + /** + * Handle drop on textarea for images + */ + const handleTextareaDrop = useCallback( + async (e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(false); + + // Skip image handling if feature is not enabled + if (!onImagesChange) return; + if (isSubmitting) return; + + const files = e.dataTransfer?.files; + if (!files || files.length === 0) return; + + // Filter for image files + const imageFiles: File[] = []; + for (let i = 0; i < files.length; i++) { + const file = files[i]; + if (file.type.startsWith('image/')) { + imageFiles.push(file); + } + } + + if (imageFiles.length === 0) return; + + // Check if we can add more images + const remainingSlots = MAX_IMAGES_PER_TASK - images.length; + if (remainingSlots <= 0) { + setError(t('feedback.maxImagesError', { count: MAX_IMAGES_PER_TASK })); + return; + } + + setError(null); + + // Process image files + const newImages: ImageAttachment[] = []; + const existingFilenames = images.map(img => img.filename); + + for (const file of imageFiles.slice(0, remainingSlots)) { + // Validate image type + if (!isValidImageMimeType(file.type)) { + setError(t('feedback.invalidTypeError', { types: ALLOWED_IMAGE_TYPES_DISPLAY })); + continue; + } + + try { + const dataUrl = await blobToBase64(file); + const thumbnail = await createThumbnail(dataUrl); + + // Use original filename or generate one with proper extension + // Map MIME types to proper file extensions (handles svg+xml -> svg, etc.) + const mimeToExtension: Record = { + 'image/svg+xml': 'svg', + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + 'image/webp': 'webp', + }; + const extension = mimeToExtension[file.type] || file.type.split('/')[1] || 'png'; + const baseFilename = file.name || `dropped-image-${Date.now()}.${extension}`; + const resolvedFilename = resolveFilename(baseFilename, [ + ...existingFilenames, + ...newImages.map(img => img.filename) + ]); + + newImages.push({ + id: generateImageId(), + filename: resolvedFilename, + mimeType: file.type, + size: file.size, + data: dataUrl.split(',')[1], // Store base64 without data URL prefix + thumbnail + }); + } catch (error) { + console.error('[QAFeedbackSection] Failed to process dropped image:', error); + setError(t('feedback.processingError', 'Failed to process dropped image')); + } + } + + if (newImages.length > 0) { + onImagesChange([...images, ...newImages]); + // Show success feedback + setPasteSuccess(true); + setTimeout(() => setPasteSuccess(false), 2000); + } + }, + [images, isSubmitting, onImagesChange, t] + ); + + /** + * Remove an image from the attachments + */ + const handleRemoveImage = useCallback((imageId: string) => { + if (!onImagesChange) return; + onImagesChange(images.filter(img => img.id !== imageId)); + setError(null); + }, [images, onImagesChange]); + + // Allow submission with either text feedback or images + const canSubmit = feedback.trim() || images.length > 0; + return (

    - Request Changes + {t('feedback.requestChanges', 'Request Changes')}

    - Found issues? Describe what needs to be fixed and the AI will continue working on it. + {t('feedback.description', 'Found issues? Describe what needs to be fixed and the AI will continue working on it.')}

    + + {/* Textarea with paste/drop support */}