mirror of
https://github.com/coleam00/Archon.git
synced 2025-12-24 02:39:17 -05:00
Merge pull request #325 from coleam00/feature/external-pr-claude-review
Add two-stage workflow for secure Claude reviews on forked PRs
This commit is contained in:
Wirasm
150
.github/workflows/README-EXTERNAL-PRS.md
vendored
Normal file
150
.github/workflows/README-EXTERNAL-PRS.md
vendored
Normal file
@@ -0,0 +1,150 @@
|
||||
# Claude Review for External PRs (Forked Repositories)
|
||||
|
||||
## Overview
|
||||
|
||||
This document explains the two-stage workflow system for running Claude Code reviews on pull requests from forked repositories. This approach solves the GitHub Actions security limitation where forked PRs cannot access repository secrets.
|
||||
|
||||
## The Problem
|
||||
|
||||
- **Direct PRs**: Contributors with write access push branches directly to the main repository. Workflows have full access to secrets. ✅
|
||||
- **Forked PRs**: External contributors fork the repo and submit PRs. GitHub blocks secret access for security. ❌
|
||||
|
||||
## The Solution: Two-Stage Workflow
|
||||
|
||||
### Stage 1: PR Information Collection (`claude-review-ext-stage1.yml`)
|
||||
|
||||
- **Trigger**: Runs on `pull_request` events or when someone comments `@claude-review-ext`
|
||||
- **Permissions**: Read-only, no secrets
|
||||
- **Purpose**: Safely collect PR information from untrusted code
|
||||
- **Output**: Uploads PR details as an artifact
|
||||
|
||||
### Stage 2: Secure Review (`claude-review-ext.yml`)
|
||||
|
||||
- **Trigger**: Runs after Stage 1 completes via `workflow_run`
|
||||
- **Permissions**: Has access to repository secrets
|
||||
- **Purpose**: Downloads PR info and runs Claude review in secure context
|
||||
- **Security**: Never checks out untrusted PR code directly
|
||||
|
||||
## How to Use
|
||||
|
||||
### For External Contributors
|
||||
|
||||
1. Fork the repository
|
||||
2. Create your feature branch
|
||||
3. Submit a pull request
|
||||
4. Wait for automatic review or ask a maintainer to comment `@claude-review-ext`
|
||||
|
||||
### For Maintainers
|
||||
|
||||
To trigger a review on an external PR:
|
||||
```
|
||||
@claude-review-ext
|
||||
```
|
||||
|
||||
Authorized users:
|
||||
- @Wirasm
|
||||
- @coleam00
|
||||
- @sean-eskerium
|
||||
|
||||
## Security Model
|
||||
|
||||
### Stage 1 (Untrusted Environment)
|
||||
```yaml
|
||||
permissions:
|
||||
contents: read # Read-only
|
||||
pull-requests: read # Read-only
|
||||
# No secrets available
|
||||
```
|
||||
|
||||
### Stage 2 (Trusted Environment)
|
||||
```yaml
|
||||
permissions:
|
||||
contents: read # Read base repo
|
||||
pull-requests: write # Post comments
|
||||
issues: write # Post comments
|
||||
id-token: write # OIDC auth
|
||||
# Secrets ARE available
|
||||
```
|
||||
|
||||
### Key Security Features
|
||||
|
||||
1. **No Direct Code Execution**: Stage 2 never runs PR code directly
|
||||
2. **Artifact-Based Communication**: PR information passed via artifacts, not code execution
|
||||
3. **Authorization Check**: Only authorized users can trigger reviews via comments
|
||||
4. **Base Branch Checkout**: Stage 2 checks out main branch, not PR branch
|
||||
5. **Diff Analysis**: Reviews changes via git diff, not by running PR code
|
||||
|
||||
## Workflow Diagram
|
||||
|
||||
```
|
||||
┌──────────────────┐
|
||||
│ External Fork PR │
|
||||
└────────┬─────────┘
|
||||
│
|
||||
v
|
||||
┌────────────────────────┐
|
||||
│ Stage 1: Collect Info │
|
||||
│ (No Secrets) │
|
||||
│ - Checkout PR code │
|
||||
│ - Gather PR details │
|
||||
│ - Upload as artifact │
|
||||
└────────┬───────────────┘
|
||||
│
|
||||
v
|
||||
┌────────────────────────┐
|
||||
│ Stage 2: Claude Review │
|
||||
│ (Has Secrets) │
|
||||
│ - Download artifact │
|
||||
│ - Checkout base branch │
|
||||
│ - Analyze diff │
|
||||
│ - Post review │
|
||||
└────────────────────────┘
|
||||
```
|
||||
|
||||
## Comparison with Standard Workflows
|
||||
|
||||
| Feature | Standard (`@claude-review`) | External (`@claude-review-ext`) |
|
||||
|---------|----------------------------|--------------------------------|
|
||||
| Trigger | `@claude-review` | `@claude-review-ext` |
|
||||
| Works on direct branches | ✅ | ✅ |
|
||||
| Works on forked PRs | ❌ | ✅ |
|
||||
| Workflow files | 1 | 2 |
|
||||
| Security model | Direct execution | Two-stage isolation |
|
||||
| Setup complexity | Simple | Moderate |
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### Review Not Triggering
|
||||
|
||||
1. Check if you used the correct trigger: `@claude-review-ext` (not `@claude-review`)
|
||||
2. Verify you're an authorized user
|
||||
3. Check if Stage 1 workflow completed successfully
|
||||
|
||||
### Stage 1 Fails
|
||||
|
||||
- Usually indicates a problem with the PR itself
|
||||
- Check workflow logs for details
|
||||
|
||||
### Stage 2 Fails
|
||||
|
||||
- Often indicates an issue with Claude API or token
|
||||
- Check if `CLAUDE_CODE_OAUTH_TOKEN` secret is configured
|
||||
- Verify artifact was created in Stage 1
|
||||
|
||||
## Limitations
|
||||
|
||||
1. **Slight Delay**: Two-stage process takes longer than direct review
|
||||
2. **Artifact Size**: Large PRs might exceed artifact limits
|
||||
3. **Manual Trigger**: Automatic reviews on PR open require maintainer action for forks
|
||||
|
||||
## Future Improvements
|
||||
|
||||
- GitHub App integration for better fork support
|
||||
- Automatic security scanning before review
|
||||
- Caching for faster reviews on PR updates
|
||||
|
||||
## References
|
||||
|
||||
- [GitHub Actions Security Best Practices](https://docs.github.com/en/actions/security-guides)
|
||||
- [Preventing PWN Requests](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/)
|
||||
- [Claude Code GitHub Actions](https://docs.anthropic.com/en/docs/claude-code/github-actions)
|
||||
231
.github/workflows/claude-review-ext-stage1.yml
vendored
Normal file
231
.github/workflows/claude-review-ext-stage1.yml
vendored
Normal file
@@ -0,0 +1,231 @@
|
||||
name: Claude Review External - Stage 1 (PR Info Collection)
|
||||
|
||||
# This workflow runs on pull requests from forks
|
||||
# It collects PR information and saves it as an artifact for Stage 2
|
||||
# No secrets are available or needed in this stage
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened]
|
||||
issue_comment:
|
||||
types: [created]
|
||||
pull_request_review_comment:
|
||||
types: [created]
|
||||
|
||||
jobs:
|
||||
collect-pr-info:
|
||||
# Only trigger on specific conditions
|
||||
# For PRs: always collect info
|
||||
# For comments: only when @claude-review-ext is mentioned
|
||||
if: |
|
||||
github.event_name == 'pull_request' ||
|
||||
(
|
||||
(github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') &&
|
||||
contains(github.event.comment.body, '@claude-review-ext')
|
||||
)
|
||||
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
|
||||
steps:
|
||||
- name: Checkout PR code
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# For PRs, this automatically checks out the merge commit
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Collect PR Information
|
||||
id: pr-info
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
let prNumber, prTitle, prBody, prAuthor, baseBranch, headBranch, headSha;
|
||||
let triggerPhrase = '';
|
||||
let commentBody = '';
|
||||
let commentId = null;
|
||||
let commentAuthor = '';
|
||||
|
||||
if (context.eventName === 'pull_request') {
|
||||
prNumber = context.payload.pull_request.number;
|
||||
prTitle = context.payload.pull_request.title;
|
||||
prBody = context.payload.pull_request.body || '';
|
||||
prAuthor = context.payload.pull_request.user.login;
|
||||
baseBranch = context.payload.pull_request.base.ref;
|
||||
headBranch = context.payload.pull_request.head.ref;
|
||||
headSha = context.payload.pull_request.head.sha;
|
||||
} else if (context.eventName === 'issue_comment') {
|
||||
// For issue comments on PRs
|
||||
const issue = context.payload.issue;
|
||||
if (!issue.pull_request) {
|
||||
console.log('Not a PR comment, skipping');
|
||||
return;
|
||||
}
|
||||
|
||||
prNumber = issue.number;
|
||||
triggerPhrase = '@claude-review-ext';
|
||||
commentBody = context.payload.comment.body;
|
||||
commentId = context.payload.comment.id;
|
||||
commentAuthor = context.payload.comment.user.login;
|
||||
|
||||
// Fetch full PR details
|
||||
const { data: pr } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber
|
||||
});
|
||||
|
||||
prTitle = pr.title;
|
||||
prBody = pr.body || '';
|
||||
prAuthor = pr.user.login;
|
||||
baseBranch = pr.base.ref;
|
||||
headBranch = pr.head.ref;
|
||||
headSha = pr.head.sha;
|
||||
} else if (context.eventName === 'pull_request_review_comment') {
|
||||
prNumber = context.payload.pull_request.number;
|
||||
prTitle = context.payload.pull_request.title;
|
||||
prBody = context.payload.pull_request.body || '';
|
||||
prAuthor = context.payload.pull_request.user.login;
|
||||
baseBranch = context.payload.pull_request.base.ref;
|
||||
headBranch = context.payload.pull_request.head.ref;
|
||||
headSha = context.payload.pull_request.head.sha;
|
||||
triggerPhrase = '@claude-review-ext';
|
||||
commentBody = context.payload.comment.body;
|
||||
commentId = context.payload.comment.id;
|
||||
commentAuthor = context.payload.comment.user.login;
|
||||
}
|
||||
|
||||
// Get list of changed files
|
||||
const { data: files } = await github.rest.pulls.listFiles({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber
|
||||
});
|
||||
|
||||
const changedFiles = files.map(f => ({
|
||||
filename: f.filename,
|
||||
status: f.status,
|
||||
additions: f.additions,
|
||||
deletions: f.deletions,
|
||||
changes: f.changes,
|
||||
patch: f.patch
|
||||
}));
|
||||
|
||||
// Get diff
|
||||
const { data: diff } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
mediaType: {
|
||||
format: 'diff'
|
||||
}
|
||||
});
|
||||
|
||||
const prInfo = {
|
||||
prNumber,
|
||||
prTitle,
|
||||
prBody,
|
||||
prAuthor,
|
||||
baseBranch,
|
||||
headBranch,
|
||||
headSha,
|
||||
triggerPhrase,
|
||||
commentBody,
|
||||
commentId,
|
||||
commentAuthor,
|
||||
changedFiles,
|
||||
diff,
|
||||
eventName: context.eventName,
|
||||
repository: `${context.repo.owner}/${context.repo.repo}`,
|
||||
triggeredAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
// Save to file
|
||||
fs.writeFileSync('pr-info.json', JSON.stringify(prInfo, null, 2));
|
||||
|
||||
console.log(`Collected info for PR #${prNumber} by ${prAuthor}`);
|
||||
console.log(`Changed files: ${changedFiles.length}`);
|
||||
console.log(`Event: ${context.eventName}`);
|
||||
if (triggerPhrase) {
|
||||
console.log(`Triggered by: ${commentAuthor} with phrase: ${triggerPhrase}`);
|
||||
}
|
||||
|
||||
// Set outputs for job summary
|
||||
core.setOutput('pr_number', prNumber);
|
||||
core.setOutput('pr_author', prAuthor);
|
||||
core.setOutput('changed_files_count', changedFiles.length);
|
||||
|
||||
- name: Run Basic Validation
|
||||
id: validation
|
||||
run: |
|
||||
echo "Running basic validation checks..."
|
||||
|
||||
# Check if this is from a fork
|
||||
IS_FORK="${{ github.event.pull_request.head.repo.fork }}"
|
||||
echo "Is from fork: $IS_FORK"
|
||||
|
||||
# Count files changed
|
||||
CHANGED_FILES=$(jq '.changedFiles | length' pr-info.json)
|
||||
echo "Files changed: $CHANGED_FILES"
|
||||
|
||||
# Check file types
|
||||
echo "File types changed:"
|
||||
jq -r '.changedFiles[].filename' pr-info.json | sed 's/.*\.//' | sort | uniq -c
|
||||
|
||||
# Check for potential issues
|
||||
LARGE_DIFF=false
|
||||
if [ "$CHANGED_FILES" -gt 100 ]; then
|
||||
LARGE_DIFF=true
|
||||
echo "⚠️ Large PR detected: $CHANGED_FILES files changed"
|
||||
fi
|
||||
|
||||
echo "is_fork=$IS_FORK" >> $GITHUB_OUTPUT
|
||||
echo "large_diff=$LARGE_DIFF" >> $GITHUB_OUTPUT
|
||||
|
||||
- name: Upload PR Information
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: pr-info-${{ github.event.pull_request.number || github.event.issue.number }}
|
||||
path: pr-info.json
|
||||
retention-days: 1
|
||||
|
||||
- name: Post Status Comment
|
||||
if: github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment'
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const prNumber = ${{ steps.pr-info.outputs.pr_number }};
|
||||
const isFork = '${{ steps.validation.outputs.is_fork }}' === 'true';
|
||||
const filesCount = ${{ steps.pr-info.outputs.changed_files_count }};
|
||||
|
||||
let statusMessage = `🔄 **Claude Review Stage 1 Complete**\n\n`;
|
||||
statusMessage += `- PR: #${prNumber}\n`;
|
||||
statusMessage += `- Author: @${{ steps.pr-info.outputs.pr_author }}\n`;
|
||||
statusMessage += `- Files Changed: ${filesCount}\n`;
|
||||
statusMessage += `- Fork Status: ${isFork ? '🔱 External Fork' : '✅ Direct Branch'}\n\n`;
|
||||
statusMessage += `📋 Stage 2 (Claude Review) will run automatically after this workflow completes.\n`;
|
||||
statusMessage += `This two-stage process ensures secure handling of forked PRs.`;
|
||||
|
||||
// Post comment
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: prNumber,
|
||||
body: statusMessage
|
||||
});
|
||||
|
||||
- name: Job Summary
|
||||
run: |
|
||||
echo "## Claude Review Stage 1 Summary" >> $GITHUB_STEP_SUMMARY
|
||||
echo "" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **PR Number**: #${{ steps.pr-info.outputs.pr_number }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **Author**: @${{ steps.pr-info.outputs.pr_author }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **Files Changed**: ${{ steps.pr-info.outputs.changed_files_count }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **From Fork**: ${{ steps.validation.outputs.is_fork }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **Large PR**: ${{ steps.validation.outputs.large_diff }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "" >> $GITHUB_STEP_SUMMARY
|
||||
echo "✅ PR information collected and uploaded as artifact for Stage 2 processing." >> $GITHUB_STEP_SUMMARY
|
||||
393
.github/workflows/claude-review-ext.yml
vendored
Normal file
393
.github/workflows/claude-review-ext.yml
vendored
Normal file
@@ -0,0 +1,393 @@
|
||||
name: Claude Review External - Stage 2 (Secure Review)
|
||||
|
||||
# This workflow runs after Stage 1 completes
|
||||
# It has access to secrets and performs the actual Claude review
|
||||
# Security: Runs in the context of the base repository, not the fork
|
||||
|
||||
on:
|
||||
workflow_run:
|
||||
workflows: ["Claude Review External - Stage 1 (PR Info Collection)"]
|
||||
types: [completed]
|
||||
|
||||
jobs:
|
||||
check-authorization:
|
||||
# Only run if Stage 1 completed successfully
|
||||
if: github.event.workflow_run.conclusion == 'success'
|
||||
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
outputs:
|
||||
authorized: ${{ steps.check.outputs.authorized }}
|
||||
pr_number: ${{ steps.check.outputs.pr_number }}
|
||||
comment_author: ${{ steps.check.outputs.comment_author }}
|
||||
trigger_phrase: ${{ steps.check.outputs.trigger_phrase }}
|
||||
|
||||
permissions:
|
||||
actions: read
|
||||
pull-requests: read
|
||||
|
||||
steps:
|
||||
- name: Download PR Info Artifact
|
||||
uses: actions/github-script@v7
|
||||
id: download-artifact
|
||||
with:
|
||||
script: |
|
||||
// Get artifacts from the workflow run
|
||||
const artifacts = await github.rest.actions.listWorkflowRunArtifacts({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
run_id: ${{ github.event.workflow_run.id }}
|
||||
});
|
||||
|
||||
const matchArtifact = artifacts.data.artifacts.find(artifact =>
|
||||
artifact.name.startsWith('pr-info-')
|
||||
);
|
||||
|
||||
if (!matchArtifact) {
|
||||
core.setFailed('No PR info artifact found');
|
||||
return;
|
||||
}
|
||||
|
||||
// Download the artifact
|
||||
const download = await github.rest.actions.downloadArtifact({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
artifact_id: matchArtifact.id,
|
||||
archive_format: 'zip'
|
||||
});
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
fs.writeFileSync('pr-info.zip', Buffer.from(download.data));
|
||||
|
||||
// Extract the artifact
|
||||
const { exec } = require('child_process');
|
||||
await new Promise((resolve, reject) => {
|
||||
exec('unzip -o pr-info.zip', (error, stdout, stderr) => {
|
||||
if (error) reject(error);
|
||||
else resolve(stdout);
|
||||
});
|
||||
});
|
||||
|
||||
console.log('Artifact downloaded and extracted');
|
||||
|
||||
- name: Check Authorization
|
||||
id: check
|
||||
run: |
|
||||
# Read PR info
|
||||
PR_INFO=$(cat pr-info.json)
|
||||
PR_NUMBER=$(echo "$PR_INFO" | jq -r '.prNumber')
|
||||
COMMENT_AUTHOR=$(echo "$PR_INFO" | jq -r '.commentAuthor // .prAuthor')
|
||||
TRIGGER_PHRASE=$(echo "$PR_INFO" | jq -r '.triggerPhrase // ""')
|
||||
EVENT_NAME=$(echo "$PR_INFO" | jq -r '.eventName')
|
||||
|
||||
echo "PR Number: $PR_NUMBER"
|
||||
echo "Comment Author: $COMMENT_AUTHOR"
|
||||
echo "Trigger Phrase: $TRIGGER_PHRASE"
|
||||
echo "Event: $EVENT_NAME"
|
||||
|
||||
# Check if this was triggered by a comment with the right phrase
|
||||
AUTHORIZED="false"
|
||||
if [[ "$EVENT_NAME" == "pull_request" ]]; then
|
||||
# Automatic review for all PRs
|
||||
AUTHORIZED="true"
|
||||
echo "Authorized: Automatic review for PR event"
|
||||
elif [[ "$TRIGGER_PHRASE" == "@claude-review-ext" ]]; then
|
||||
# Check if user is authorized
|
||||
AUTHORIZED_USERS='["Wirasm", "coleam00", "sean-eskerium"]'
|
||||
if echo "$AUTHORIZED_USERS" | jq -e --arg user "$COMMENT_AUTHOR" 'contains([$user])' > /dev/null; then
|
||||
AUTHORIZED="true"
|
||||
echo "Authorized: $COMMENT_AUTHOR is in authorized users list"
|
||||
else
|
||||
echo "Not authorized: $COMMENT_AUTHOR is not in authorized users list"
|
||||
fi
|
||||
fi
|
||||
|
||||
echo "authorized=$AUTHORIZED" >> $GITHUB_OUTPUT
|
||||
echo "pr_number=$PR_NUMBER" >> $GITHUB_OUTPUT
|
||||
echo "comment_author=$COMMENT_AUTHOR" >> $GITHUB_OUTPUT
|
||||
echo "trigger_phrase=$TRIGGER_PHRASE" >> $GITHUB_OUTPUT
|
||||
|
||||
claude-review:
|
||||
needs: check-authorization
|
||||
if: needs.check-authorization.outputs.authorized == 'true'
|
||||
|
||||
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:
|
||||
- name: Checkout Repository (Base Branch)
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# SECURITY: Checkout base branch, not PR code
|
||||
# We'll checkout the default branch first, then switch to the PR's base branch after downloading PR info
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Download PR Info
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
// Get artifacts from the workflow run
|
||||
const artifacts = await github.rest.actions.listWorkflowRunArtifacts({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
run_id: ${{ github.event.workflow_run.id }}
|
||||
});
|
||||
|
||||
const matchArtifact = artifacts.data.artifacts.find(artifact =>
|
||||
artifact.name.startsWith('pr-info-')
|
||||
);
|
||||
|
||||
if (!matchArtifact) {
|
||||
core.setFailed('No PR info artifact found');
|
||||
return;
|
||||
}
|
||||
|
||||
// Download the artifact
|
||||
const download = await github.rest.actions.downloadArtifact({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
artifact_id: matchArtifact.id,
|
||||
archive_format: 'zip'
|
||||
});
|
||||
|
||||
const fs = require('fs');
|
||||
fs.writeFileSync('pr-info.zip', Buffer.from(download.data));
|
||||
|
||||
// Extract the artifact
|
||||
const { exec } = require('child_process');
|
||||
await new Promise((resolve, reject) => {
|
||||
exec('unzip -o pr-info.zip', (error, stdout, stderr) => {
|
||||
if (error) reject(error);
|
||||
else resolve(stdout);
|
||||
});
|
||||
});
|
||||
|
||||
// Read and parse PR info
|
||||
const prInfo = JSON.parse(fs.readFileSync('pr-info.json', 'utf8'));
|
||||
|
||||
// Save key information as environment variables
|
||||
core.exportVariable('PR_NUMBER', prInfo.prNumber);
|
||||
core.exportVariable('PR_TITLE', prInfo.prTitle);
|
||||
core.exportVariable('PR_AUTHOR', prInfo.prAuthor);
|
||||
core.exportVariable('HEAD_SHA', prInfo.headSha);
|
||||
core.exportVariable('PR_BASE_BRANCH', prInfo.baseBranch);
|
||||
|
||||
console.log(`Loaded PR #${prInfo.prNumber} information`);
|
||||
console.log(`Base branch: ${prInfo.baseBranch}`);
|
||||
|
||||
- name: Switch to PR Base Branch
|
||||
run: |
|
||||
# Switch to the PR's actual target base branch
|
||||
git checkout ${{ env.PR_BASE_BRANCH }}
|
||||
echo "Switched to base branch: ${{ env.PR_BASE_BRANCH }}"
|
||||
|
||||
- name: Fetch PR Branch for Analysis
|
||||
run: |
|
||||
# Fetch the PR branch to analyze (but don't checkout)
|
||||
git fetch origin pull/${{ env.PR_NUMBER }}/head:pr-branch
|
||||
|
||||
# Create a safe diff for analysis against the PR's target base branch
|
||||
git diff origin/${{ env.PR_BASE_BRANCH }}...pr-branch > pr-diff.patch
|
||||
|
||||
echo "Fetched PR branch for analysis (not checked out for security)"
|
||||
|
||||
- name: Post Starting Comment
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: ${{ env.PR_NUMBER }},
|
||||
body: `🤖 **Claude Review (External PR) Starting...**\n\nAnalyzing PR changes in a secure environment. This may take a few minutes.`
|
||||
});
|
||||
|
||||
- name: Run Claude Code Review
|
||||
id: claude
|
||||
uses: anthropics/claude-code-action@beta
|
||||
timeout-minutes: 15
|
||||
with:
|
||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
|
||||
# Use the external review trigger phrase
|
||||
trigger_phrase: "@claude-review-ext"
|
||||
|
||||
# Custom context for PR review
|
||||
pr_number: ${{ env.PR_NUMBER }}
|
||||
|
||||
# Review-specific instructions (same as claude-review.yml)
|
||||
custom_instructions: |
|
||||
You are performing a CODE REVIEW for an EXTERNAL PULL REQUEST.
|
||||
This is a secure two-stage review process for PRs from forks.
|
||||
|
||||
## Security Context
|
||||
You are running in a secure environment with access to repository secrets.
|
||||
The PR code has been fetched but NOT checked out for security reasons.
|
||||
Review the changes in pr-diff.patch file.
|
||||
|
||||
## 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**
|
||||
- Review 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
|
||||
|
||||
### 6. Security Considerations for External PRs
|
||||
- Check for any attempts to access secrets or environment variables
|
||||
- Look for suspicious network requests or data exfiltration
|
||||
- Verify no hardcoded credentials or API keys
|
||||
- Check for SQL injection vulnerabilities
|
||||
- Review any file system operations
|
||||
|
||||
## Required Output Format
|
||||
|
||||
## Summary
|
||||
[2-3 sentence overview of what the changes do and their impact]
|
||||
|
||||
## Security Review (External PR)
|
||||
⚠️ This is an external PR from a fork. Additional 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
|
||||
[List any security concerns found or state "No security issues found"]
|
||||
|
||||
## 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]
|
||||
|
||||
## 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 security review by maintainer
|
||||
|
||||
**Priority Actions:**
|
||||
1. [Most important fix needed, if any]
|
||||
2. [Second priority, if applicable]
|
||||
|
||||
**External PR Note:**
|
||||
This PR is from a fork. Maintainers should manually verify the changes before merging.
|
||||
|
||||
---
|
||||
*Review based on Archon V2 Alpha guidelines and secure external PR process*
|
||||
|
||||
unauthorized-message:
|
||||
needs: check-authorization
|
||||
if: needs.check-authorization.outputs.authorized == 'false' && needs.check-authorization.outputs.trigger_phrase == '@claude-review-ext'
|
||||
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
permissions:
|
||||
issues: write
|
||||
pull-requests: write
|
||||
|
||||
steps:
|
||||
- name: Post Unauthorized Message
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const prNumber = '${{ needs.check-authorization.outputs.pr_number }}';
|
||||
const commentAuthor = '${{ needs.check-authorization.outputs.comment_author }}';
|
||||
|
||||
const comment = {
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: parseInt(prNumber),
|
||||
body: `❌ @${commentAuthor} - You are not authorized to trigger Claude reviews.\n\n` +
|
||||
`Only the following maintainers can trigger Claude: @Wirasm, @coleam00, @sean-eskerium\n\n` +
|
||||
`Please ask a maintainer for review.`
|
||||
};
|
||||
|
||||
await github.rest.issues.createComment(comment);
|
||||
Reference in New Issue
Block a user