mirror of
https://github.com/wilsonfreitas/awesome-quant.git
synced 2026-08-06 08:47:46 +00:00
Update spec with review feedback: fix fork permissions, merge flow
Address critical and important issues from spec review: - Use pull_request_target instead of pull_request for fork write access - Remove approval step (GITHUB_TOKEN can't self-approve) - Add duplicate check against open/closed PRs - Add reopened/edited event types - Skip auto-merge for entries with no GitHub URL - Clarify urllib.request usage and 365-day activity window - Note deliberate deviation from CONTRIBUTING.md auto-close behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,8 @@ An automated GitHub Actions workflow that evaluates every new pull request again
|
||||
- No action on non-compliant PRs (no comments, no closing, no labels)
|
||||
- Notifications via GitHub's built-in notification system
|
||||
|
||||
> **Note**: CONTRIBUTING.md states that certain PRs "will be closed" automatically. This agent deliberately deviates from that — non-compliant PRs are left open for manual review instead. The maintainer handles rejections personally.
|
||||
|
||||
## New Files
|
||||
|
||||
| File | Purpose |
|
||||
@@ -23,15 +25,18 @@ An automated GitHub Actions workflow that evaluates every new pull request again
|
||||
|
||||
## Component 1: GitHub Actions Workflow (`pr-review.yml`)
|
||||
|
||||
**Trigger**: `pull_request` events (types: `opened`, `synchronize`) targeting `main`.
|
||||
**Trigger**: `pull_request_target` events (types: `opened`, `synchronize`, `reopened`, `edited`) targeting `main`.
|
||||
|
||||
> **Why `pull_request_target`?** Most contributions come from forks. The `pull_request` event gives a read-only `GITHUB_TOKEN` for fork PRs, which cannot merge. `pull_request_target` runs in the context of the base repo and has write access. This is safe because the validation script never executes code from the PR — it only reads the diff via the GitHub API.
|
||||
|
||||
**Steps**:
|
||||
1. Checkout the repo (with the PR merge ref)
|
||||
1. Checkout the base branch (not the PR branch — security measure with `pull_request_target`)
|
||||
2. Set up Python + uv
|
||||
3. Run the validation script: `uv run python scripts/review_pr.py`
|
||||
4. On exit 0: approve the PR via GitHub API and squash-merge it
|
||||
5. On exit 0: the merge comment thanks the contributor
|
||||
6. On non-zero exit: do nothing
|
||||
4. On exit 0: squash-merge the PR directly (no approval step needed)
|
||||
5. On non-zero exit: do nothing
|
||||
|
||||
> **Why no approval step?** The default `GITHUB_TOKEN` cannot submit an approving review on a PR triggered by the same workflow context. Since the repo does not require branch protection approvals, the workflow merges directly.
|
||||
|
||||
**Permissions**: `contents: write`, `pull-requests: write`.
|
||||
|
||||
@@ -39,7 +44,7 @@ An automated GitHub Actions workflow that evaluates every new pull request again
|
||||
|
||||
## Component 2: Validation Script (`scripts/review_pr.py`)
|
||||
|
||||
A single Python script using `PyGithub` (already a project dependency) and the standard library.
|
||||
A single Python script using `PyGithub` (already a project dependency) and `urllib.request` from the standard library (for URL reachability checks).
|
||||
|
||||
### Input
|
||||
|
||||
@@ -60,7 +65,8 @@ All checks must pass for auto-merge. The script exits 0 only if every check pass
|
||||
- Any other file changed → skip (exit non-zero).
|
||||
|
||||
#### 3. Extract Added Lines
|
||||
- From the PR diff, extract only lines added to `README.md`.
|
||||
- From the PR diff via the GitHub API (not from a local checkout of the PR branch).
|
||||
- Extract only lines added to `README.md`.
|
||||
- Ignore blank lines, section headers (`##`, `###`), and other non-entry lines.
|
||||
|
||||
#### 4. Single Entry Check
|
||||
@@ -79,8 +85,9 @@ Additional checks:
|
||||
- `[GitHub](url)` link, if present, uses the exact format `[GitHub](https://github.com/owner/repo)`.
|
||||
|
||||
#### 6. Duplicate Check
|
||||
- Search the current `README.md` for the project name (case-insensitive).
|
||||
- Search the current `README.md` (from the base branch) for the project name (case-insensitive).
|
||||
- Search for all URLs found in the entry.
|
||||
- Query the GitHub API for open and recently closed PRs that contain the same project name or URL.
|
||||
- If any match is found → skip.
|
||||
|
||||
#### 7. Section Placement Check
|
||||
@@ -91,11 +98,14 @@ Additional checks:
|
||||
For any GitHub URL found in the entry (primary URL or `[GitHub](url)` link):
|
||||
- Fetch the repo via GitHub API.
|
||||
- `archived == true` → skip.
|
||||
- Last push date older than 12 months → skip.
|
||||
- Last push date older than 365 days from the workflow run date → skip.
|
||||
- No README file in the repo → skip.
|
||||
|
||||
If the entry has **no GitHub URL at all** (e.g., a CRAN-only or PyPI-only entry without a `[GitHub]` link), skip auto-merge and leave for manual review. Activity cannot be verified without a GitHub repo.
|
||||
|
||||
#### 9. URL Reachability
|
||||
- Verify the primary URL returns a non-error HTTP status (2xx or 3xx).
|
||||
- Uses `urllib.request` with a 10-second timeout.
|
||||
|
||||
### Output
|
||||
- Prints a structured summary to stdout (pass/fail per check) for workflow logs.
|
||||
@@ -104,8 +114,8 @@ For any GitHub URL found in the entry (primary URL or `[GitHub](url)` link):
|
||||
## Component 3: Merge & Notification Flow
|
||||
|
||||
### On All Checks Pass (exit 0)
|
||||
1. The workflow submits an approving review via `GITHUB_TOKEN` with message: *"All checks passed. Auto-merging. Thanks for the contribution!"*
|
||||
2. Squash-merges the PR to `main`.
|
||||
1. The workflow squash-merges the PR to `main` with commit message: `Add [Project Name] (#PR_NUMBER)`.
|
||||
2. A merge comment is posted: *"All checks passed. Thanks for the contribution!"*
|
||||
3. This triggers the existing `build.yml` workflow (push to `main` with `README.md` change), so the site auto-updates.
|
||||
|
||||
### On Any Check Fails (non-zero exit)
|
||||
@@ -123,10 +133,11 @@ No additional notification infrastructure required.
|
||||
## Dependencies
|
||||
|
||||
- `PyGithub` — already in `pyproject.toml`
|
||||
- No new dependencies needed
|
||||
- `urllib.request` — standard library, no installation needed
|
||||
- No new dependencies
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- The `GITHUB_TOKEN` provided by GitHub Actions has scoped permissions (only `contents: write` and `pull-requests: write`).
|
||||
- The script never executes code from the PR — it only reads the diff and queries the GitHub API.
|
||||
- URL reachability checks use HEAD/GET requests only.
|
||||
- Uses `pull_request_target` for fork write access, but the workflow checks out the **base branch only** — never the PR branch. The validation script reads the PR diff via the GitHub API, never executing PR code locally.
|
||||
- URL reachability checks use HEAD/GET requests only via `urllib.request`.
|
||||
|
||||
Reference in New Issue
Block a user