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.
This commit is contained in:
Rasmus Widing
parent
db142280e2
commit
b5e5cddc68
358
.github/workflows/claude-review-fork.yml
vendored
358
.github/workflows/claude-review-fork.yml
vendored
@ -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.`
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user