mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-14 16:50:37 +00:00
fix: prevent contradictory security PR reviews via dedup gate (#1427)
The security reviewer agent could post contradictory reviews (CHANGES_REQUESTED then APPROVED) on the same commit because each review_all cycle spawned a fresh reviewer with no memory of prior runs. Reviews posted via `gh pr review` don't appear in `gh pr view --comments`, so the existing comment-based dedup missed them. Adds a review dedup step that fetches existing reviews via the GitHub API and stops if a prior security review exists on the current HEAD commit. Also adds commit SHA to the review body format for traceability. Co-authored-by: lab <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
979fc4a58e
commit
ac0dc485c4
1 changed files with 18 additions and 11 deletions
|
|
@ -358,40 +358,47 @@ Each pr-reviewer MUST:
|
|||
|
||||
1. **Fetch full context**:
|
||||
\`\`\`bash
|
||||
gh pr view NUMBER --repo OpenRouterTeam/spawn --json updatedAt,mergeable,title,headRefName
|
||||
gh pr view NUMBER --repo OpenRouterTeam/spawn --json updatedAt,mergeable,title,headRefName,headRefOid
|
||||
gh pr diff NUMBER --repo OpenRouterTeam/spawn
|
||||
gh pr view NUMBER --repo OpenRouterTeam/spawn --comments
|
||||
gh api repos/OpenRouterTeam/spawn/pulls/NUMBER/comments --jq '.[] | "\(.user.login): \(.body)"'
|
||||
gh api repos/OpenRouterTeam/spawn/pulls/NUMBER/reviews --jq '.[] | {state: .state, submitted_at: .submitted_at, commit_id: .commit_id, user: .user.login, bodySnippet: (.body[:200])}'
|
||||
\`\`\`
|
||||
Read ALL comments — prior discussion contains decisions, rejected approaches, and scope changes.
|
||||
Read ALL comments AND reviews — prior discussion contains decisions, rejected approaches, and scope changes. Reviews (approve/request-changes) are separate from comments and must be checked independently.
|
||||
|
||||
2. **Comment-based triage** — Close if comments indicate superseded/duplicate/abandoned:
|
||||
2. **Review dedup** — If ANY prior review from \`louisgv\` OR containing \`-- security/pr-reviewer\` already exists:
|
||||
- If prior review is **CHANGES_REQUESTED** → Do NOT post a new review. Report "already flagged by prior security review, skipping" and STOP.
|
||||
- If prior review is **APPROVED** and PR is not yet merged → The prior approval stands. Do NOT post another review. Report "already approved, skipping" and STOP.
|
||||
- Only proceed if there are **NEW COMMITS** pushed after the latest security review (compare the review's \`commit_id\` with the PR's current HEAD \`headRefOid\`). If the commit SHAs match, STOP — no new code to review.
|
||||
|
||||
3. **Comment-based triage** — Close if comments indicate superseded/duplicate/abandoned:
|
||||
\`gh pr close NUMBER --repo OpenRouterTeam/spawn --delete-branch --comment "Closing: [reason].\n\n-- security/pr-reviewer"\`
|
||||
Report and STOP.
|
||||
|
||||
3. **Staleness check** — If \`updatedAt\` > 48h AND \`mergeable\` is CONFLICTING:
|
||||
4. **Staleness check** — If \`updatedAt\` > 48h AND \`mergeable\` is CONFLICTING:
|
||||
- If PR contains valid work: file follow-up issue, then close PR referencing the new issue
|
||||
- If trivial/outdated: close without follow-up
|
||||
- Delete branch via \`--delete-branch\`. Report and STOP.
|
||||
- If > 48h but no conflicts: proceed to review. If fresh: proceed normally.
|
||||
|
||||
4. **Set up worktree**: \`git worktree add WORKTREE_BASE_PLACEHOLDER/pr-NUMBER -b review-pr-NUMBER origin/main\` → \`cd\` → \`gh pr checkout NUMBER\`
|
||||
5. **Set up worktree**: \`git worktree add WORKTREE_BASE_PLACEHOLDER/pr-NUMBER -b review-pr-NUMBER origin/main\` → \`cd\` → \`gh pr checkout NUMBER\`
|
||||
|
||||
5. **Security review** of every changed file:
|
||||
6. **Security review** of every changed file:
|
||||
- Command injection, credential leaks, path traversal, XSS/injection, unsafe eval/source, curl|bash safety, macOS bash 3.x compat
|
||||
|
||||
6. **Test** (in worktree): \`bash -n\` on .sh files, \`bun test\` for .ts files, verify source fallback pattern
|
||||
7. **Test** (in worktree): \`bash -n\` on .sh files, \`bun test\` for .ts files, verify source fallback pattern
|
||||
|
||||
7. **Decision**:
|
||||
8. **Decision** — Before posting any review, verify it applies to the **current HEAD commit**:
|
||||
- CRITICAL/HIGH found → \`gh pr review NUMBER --request-changes\` + label \`security-review-required\`
|
||||
- MEDIUM/LOW or clean → \`gh pr review NUMBER --approve\` + label \`security-approved\` + \`gh pr merge NUMBER --repo OpenRouterTeam/spawn --squash --delete-branch\`
|
||||
|
||||
8. **Clean up**: \`cd REPO_ROOT_PLACEHOLDER && git worktree remove WORKTREE_BASE_PLACEHOLDER/pr-NUMBER --force\`
|
||||
9. **Clean up**: \`cd REPO_ROOT_PLACEHOLDER && git worktree remove WORKTREE_BASE_PLACEHOLDER/pr-NUMBER --force\`
|
||||
|
||||
9. **Review body format**:
|
||||
10. **Review body format** — MUST include the HEAD commit SHA for traceability:
|
||||
\`\`\`
|
||||
## Security Review
|
||||
**Verdict**: [APPROVED / CHANGES REQUESTED]
|
||||
**Commit**: [HEAD_COMMIT_SHA]
|
||||
### Findings
|
||||
- [SEVERITY] file:line — description
|
||||
### Tests
|
||||
|
|
@ -400,7 +407,7 @@ Each pr-reviewer MUST:
|
|||
*-- security/pr-reviewer*
|
||||
\`\`\`
|
||||
|
||||
10. Report: PR number, verdict, finding count, merge status.
|
||||
11. Report: PR number, verdict, finding count, merge status.
|
||||
|
||||
## Step 3 — Branch Cleanup
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue