From b5e5cddc68c847893b35834b9471af5d56a10c7d Mon Sep 17 00:00:00 2001 From: Rasmus Widing Date: Tue, 19 Aug 2025 11:16:37 +0300 Subject: [PATCH] Remove claude-review-fork.yml workflow The Claude Code Action is not compatible with reviewing PRs from forks. It always attempts to checkout the PR branch which fails for security reasons. --- .github/workflows/claude-review-fork.yml | 358 ----------------------- 1 file changed, 358 deletions(-) delete mode 100644 .github/workflows/claude-review-fork.yml diff --git a/.github/workflows/claude-review-fork.yml b/.github/workflows/claude-review-fork.yml deleted file mode 100644 index 75b0629..0000000 --- a/.github/workflows/claude-review-fork.yml +++ /dev/null @@ -1,358 +0,0 @@ -name: Claude Review for Forked PRs - -# This workflow handles Claude reviews for PRs from forks -# Uses pull_request_target for security with first-time contributor checks - -on: - pull_request_target: - types: [opened, synchronize, reopened] - issue_comment: - types: [created] - -jobs: - claude-review: - # Trigger on: - # 1. Any PR from a fork (automatic review) - # 2. Comment with @claude-review-fork from authorized users - if: | - ( - github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.fork == true - ) || - ( - github.event_name == 'issue_comment' && - github.event.issue.pull_request && - contains(github.event.comment.body, '@claude-review-fork') && - contains(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login) - ) - - runs-on: ubuntu-latest - - permissions: - contents: read # Read repository contents - pull-requests: write # Comment on PRs - issues: write # Comment on issues - actions: read # Read CI results - id-token: write # Required for OIDC authentication - - steps: - # SECURITY: Check if this is a first-time contributor - - name: Check First-Time Contributor - id: first-time - uses: actions/github-script@v7 - with: - script: | - const pr = context.payload.pull_request || - (context.payload.issue?.pull_request ? - await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.issue.number - }).then(r => r.data) : null); - - if (!pr) { - core.setFailed('Could not find pull request'); - return; - } - - // Check if author has previous merged PRs - const author = pr.user.login; - const { data: prs } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'closed', - per_page: 1 - }); - - const previousPRs = prs.filter(p => - p.user.login === author && - p.merged_at !== null - ); - - const isFirstTime = previousPRs.length === 0; - - if (isFirstTime) { - // Post warning comment for first-time contributors - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: `⚠️ **First-Time Contributor Notice**\n\n` + - `Hi @${author}! This appears to be your first contribution to this repository.\n\n` + - `For security reasons, automated Claude review requires manual approval for first-time contributors.\n` + - `A maintainer will review your PR and may trigger Claude review if appropriate.\n\n` + - `Maintainers can trigger review with: \`@claude-review-fork\`` - }); - - // Skip the rest of the workflow for first-time contributors - core.setOutput('is_first_time', 'true'); - console.log(`First-time contributor detected: ${author}`); - } else { - core.setOutput('is_first_time', 'false'); - console.log(`Returning contributor: ${author}`); - } - - # Skip review for first-time contributors unless manually triggered - - name: Check if Should Continue - id: should-continue - run: | - if [[ "${{ steps.first-time.outputs.is_first_time }}" == "true" ]] && [[ "${{ github.event_name }}" == "pull_request_target" ]]; then - echo "Skipping automatic review for first-time contributor" - echo "should_continue=false" >> $GITHUB_OUTPUT - else - echo "Proceeding with review" - echo "should_continue=true" >> $GITHUB_OUTPUT - fi - - # SECURITY: Checkout base branch, NOT the PR branch - # This prevents executing untrusted code from forks - - name: Checkout Base Branch - if: steps.should-continue.outputs.should_continue == 'true' - uses: actions/checkout@v4 - with: - # Explicitly checkout the base branch, not the PR head - ref: ${{ github.event.pull_request.base.ref || github.event.repository.default_branch }} - fetch-depth: 0 - - # Fetch PR diff for analysis (without checking out PR code) - - name: Fetch PR Diff - if: steps.should-continue.outputs.should_continue == 'true' - id: pr-info - run: | - # Get PR number - if [[ "${{ github.event_name }}" == "pull_request_target" ]]; then - PR_NUMBER="${{ github.event.pull_request.number }}" - else - PR_NUMBER="${{ github.event.issue.number }}" - fi - - echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_ENV - - # Fetch the PR branch without checking it out - git fetch origin pull/$PR_NUMBER/head:pr-branch - - # Create diff file for Claude to analyze - git diff ${{ github.event.pull_request.base.ref || github.event.repository.default_branch }}...pr-branch > pr-diff.patch - - echo "Created diff for PR #$PR_NUMBER" - echo "Diff size: $(wc -l < pr-diff.patch) lines" - - # Create review context file to avoid PR detection - - name: Prepare Review Context - if: steps.should-continue.outputs.should_continue == 'true' - run: | - # Create a clean review context that won't trigger PR detection - mkdir -p review-context - cp pr-diff.patch review-context/changes.patch - - # Create a review instructions file - cat > review-context/REVIEW_INSTRUCTIONS.md << 'EOF' - # Code Review Task - - You are reviewing changes from PR #${{ env.PR_NUMBER }}. - - ## Files to Review - - changes.patch - Contains all the code changes - - ## Your Task - 1. Read and analyze the changes.patch file - 2. Provide a comprehensive code review - 3. Post your review as a comment on PR #${{ env.PR_NUMBER }} - - ## Important - - DO NOT attempt to checkout any code - - DO NOT fetch from git - - Work only with the provided patch file - EOF - - - name: Run Claude Code Review - if: steps.should-continue.outputs.should_continue == 'true' - id: claude - uses: anthropics/claude-code-action@beta - timeout-minutes: 15 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - github_token: ${{ secrets.GITHUB_TOKEN }} - - # Use the main prompt to guide Claude - prompt: | - You have a code review task in the review-context directory. - - Read the REVIEW_INSTRUCTIONS.md file for your task details. - Then read and analyze the changes.patch file. - - Provide a detailed code review and post it as a comment on PR #${{ env.PR_NUMBER }}. - - # Custom instructions for fork review - custom_instructions: | - You are performing a CODE REVIEW ONLY for a PULL REQUEST FROM A FORK. You cannot make any changes to files. - - ## CRITICAL: Fork Security Review - This PR is from an external fork. First, check the pr-diff.patch file for any security concerns: - - Attempts to access secrets or environment variables - - Suspicious network requests or data exfiltration - - Hardcoded credentials or API keys - - SQL injection or command injection attempts - - Malicious file system operations - - Changes to GitHub Actions workflows - - ## Your Role - You are reviewing code for Archon V2 Alpha, a local-first AI knowledge management system in early alpha stage. - - ## Architecture Context - - Frontend: React + TypeScript + Vite (port 3737) - - Backend: FastAPI + Socket.IO + Python (port 8181) - - MCP Service: MCP protocol server (port 8051) - - Agents Service: PydanticAI agents (port 8052) - - Database: Supabase (PostgreSQL + pgvector) - - ## Review Process - 1. **Understand Changes** - - Read the pr-diff.patch file to see all changes - - Check what files were changed and understand the context - - Analyze the impact across all services (frontend, backend, MCP, agents) - - Consider interactions between components - - ## Review Focus Areas - - ### 1. Code Quality - Backend (Python) - - Type hints on all functions and classes - - Pydantic v2 models for data validation (ConfigDict not class Config, model_dump() not dict()) - - No print() statements (use logging instead) - - Proper error handling with detailed error messages - - Following PEP 8 - - Google style docstrings where appropriate - - ### 2. Code Quality - Frontend (React/TypeScript) - - Proper TypeScript types (avoid 'any') - - React hooks used correctly - - Component composition and reusability - - Proper error boundaries - - Following existing component patterns - - ### 3. Structure & Architecture - - Each feature self-contained with its own models, service, and tools - - Shared components only for things used by multiple features - - Proper separation of concerns across services - - API endpoints follow RESTful conventions - - ### 4. Testing - - Unit tests co-located with code in tests/ folders - - Edge cases covered - - Mocking external dependencies - - Frontend: Vitest tests for components - - Backend: Pytest tests for services - - ### 5. Alpha Project Principles (from CLAUDE.md) - - No backwards compatibility needed - can break things - - Fail fast with detailed errors (not graceful failures) - - Remove dead code immediately - - Focus on functionality over production patterns - - ## Required Output Format - - ## Summary - [2-3 sentence overview of what the changes do and their impact] - - ## Security Review (Fork PR) - ⚠️ This is an external PR from a fork. Enhanced security checks performed: - - [ ] No unauthorized access to secrets/env vars - - [ ] No suspicious network requests - - [ ] No hardcoded credentials - - [ ] No SQL injection risks - - [ ] Safe file system operations - - [ ] No malicious workflow changes - [List any security concerns found or state "No security issues found"] - - ## Previous Review Comments - - [If this is a follow-up review, summarize unaddressed comments] - - [If first review, state: "First review - no previous comments"] - - ## Issues Found - Total: [X critical, Y important, Z minor] - - ### 🔴 Critical (Must Fix) - [Issues that will break functionality or cause data loss] - - **[Issue Title]** - `path/to/file.py:123` - Problem: [What's wrong] - Fix: [Specific solution] - - ### 🟡 Important (Should Fix) - [Issues that impact user experience or code maintainability] - - **[Issue Title]** - `path/to/file.tsx:45` - Problem: [What's wrong] - Fix: [Specific solution] - - ### 🟢 Minor (Consider) - [Nice-to-have improvements] - - **[Suggestion]** - `path/to/file.py:67` - [Brief description and why it would help] - - ## Performance Considerations - - Database query efficiency (no N+1 queries) - - Frontend bundle size impacts - - Async/await usage in Python - - React re-render optimization - [List any performance issues or state "No performance concerns"] - - ## Good Practices Observed - - [Highlight what was done well] - - [Patterns that should be replicated] - - ## Questionable Practices - - [Design decisions that might need reconsideration] - - [Architectural concerns for discussion] - - ## Test Coverage - **Current Coverage:** [Estimate based on what you see] - **Missing Tests:** - - 1. **[Component/Function Name]** - - What to test: [Specific functionality] - - Why important: [Impact if it fails] - - Suggested test: [One sentence description] - - ## Recommendations - - **Merge Decision:** - - [ ] Ready to merge as-is - - [ ] Requires fixes before merging - - [ ] Needs additional security review - - **Priority Actions:** - 1. [Most important fix needed, if any] - 2. [Second priority, if applicable] - - **Fork PR Note:** - This PR is from an external fork. Maintainers should manually verify critical changes before merging. - - --- - *Review based on Archon V2 Alpha guidelines with enhanced security checks for fork PRs* - - unauthorized-comment: - # Post message for unauthorized users trying to trigger review - if: | - github.event_name == 'issue_comment' && - github.event.issue.pull_request && - contains(github.event.comment.body, '@claude-review-fork') && - !contains(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login) - - runs-on: ubuntu-latest - - permissions: - issues: write - pull-requests: write - - steps: - - name: Post Unauthorized Message - uses: actions/github-script@v7 - with: - script: | - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: `❌ @${context.actor} - You are not authorized to trigger Claude reviews.\n\n` + - `Only maintainers can trigger Claude: @Wirasm, @coleam00, @sean-eskerium\n\n` + - `Please wait for a maintainer to review your PR.` - }); \ No newline at end of file