Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion review-pr/agents/evals/batched-verifier-security-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"At least one finding flags the missing redirect_uri validation as a security concern",
"The agent delegated all high and medium severity findings to the verifier in a single batch delegation, not multiple individual delegations",
"The verifier returned a JSON response with a 'verdicts' array containing one verdict per finding",
"The agent decided to REQUEST_CHANGES because there is at least one high-severity confirmed or likely finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
2 changes: 1 addition & 1 deletion review-pr/agents/evals/batched-verifier-security-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"At least one finding flags the missing redirect_uri validation as a security concern",
"The agent delegated all high and medium severity findings to the verifier in a single batch delegation, not multiple individual delegations",
"The verifier returned a JSON response with a 'verdicts' array containing one verdict per finding",
"The agent decided to REQUEST_CHANGES because there is at least one high-severity confirmed or likely finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
2 changes: 1 addition & 1 deletion review-pr/agents/evals/batched-verifier-security-3.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"At least one finding flags the missing redirect_uri validation as a security concern",
"The agent delegated all high and medium severity findings to the verifier in a single batch delegation, not multiple individual delegations",
"The verifier returned a JSON response with a 'verdicts' array containing one verdict per finding",
"The agent decided to REQUEST_CHANGES because there is at least one high-severity confirmed or likely finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
2 changes: 1 addition & 1 deletion review-pr/agents/evals/security-redirect-uri-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"At least one finding flags the missing redirect_uri validation as a security concern in oidc.go or service.go",
"The finding about redirect_uri validation has severity 'high' because accepting arbitrary redirect URIs without server-side validation is a well-known OAuth security vulnerability",
"The agent decided to REQUEST_CHANGES (not APPROVE and not COMMENT) because there is at least one high-severity finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
2 changes: 1 addition & 1 deletion review-pr/agents/evals/security-redirect-uri-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"At least one finding flags the missing redirect_uri validation as a security concern in oidc.go or service.go",
"The finding about redirect_uri validation has severity 'high' because accepting arbitrary redirect URIs without server-side validation is a well-known OAuth security vulnerability",
"The agent decided to REQUEST_CHANGES (not APPROVE and not COMMENT) because there is at least one high-severity finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
2 changes: 1 addition & 1 deletion review-pr/agents/evals/security-redirect-uri-3.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"At least one finding flags the missing redirect_uri validation as a security concern in oidc.go or service.go",
"The finding about redirect_uri validation has severity 'high' because accepting arbitrary redirect URIs without server-side validation is a well-known OAuth security vulnerability",
"The agent decided to REQUEST_CHANGES (not APPROVE and not COMMENT) because there is at least one high-severity finding"
"The review assessment label is 'πŸ”΄ CRITICAL' because there is at least one high-severity confirmed or likely finding"
]
},
"messages": [
Expand Down
4 changes: 2 additions & 2 deletions review-pr/agents/evals/success-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
"relevance": [
"The agent ran 'echo $GITHUB_ACTIONS' before performing the review to detect the output mode",
"The agent output the review to the console as formatted markdown instead of posting via gh api",
"The agent decided to APPROVE the PR",
"The review assessment label is '🟒 APPROVE' or '🟑 NEEDS ATTENTION' β€” it is NOT 'πŸ”΄ CRITICAL'",
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"No findings have severity 'high' with verdict 'CONFIRMED' or 'LIKELY'",
"The review summary does not request changes or flag critical issues"
"The review does not label any finding as CRITICAL or block-worthy"
]
},
"messages": [
Expand Down
4 changes: 2 additions & 2 deletions review-pr/agents/evals/success-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
"relevance": [
"The agent ran 'echo $GITHUB_ACTIONS' before performing the review to detect the output mode",
"The agent output the review to the console as formatted markdown instead of posting via gh api",
"The agent decided to APPROVE the PR",
"The review assessment label is '🟒 APPROVE' or '🟑 NEEDS ATTENTION' β€” it is NOT 'πŸ”΄ CRITICAL'",
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"No findings have severity 'high' with verdict 'CONFIRMED' or 'LIKELY'",
"The review summary does not request changes or flag critical issues"
"The review does not label any finding as CRITICAL or block-worthy"
]
},
"messages": [
Expand Down
4 changes: 2 additions & 2 deletions review-pr/agents/evals/success-3.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
"relevance": [
"The agent ran 'echo $GITHUB_ACTIONS' before performing the review to detect the output mode",
"The agent output the review to the console as formatted markdown instead of posting via gh api",
"The agent decided to APPROVE the PR",
"The review assessment label is '🟒 APPROVE' or '🟑 NEEDS ATTENTION' β€” it is NOT 'πŸ”΄ CRITICAL'",
"The drafter response is valid JSON containing a 'findings' array and a 'summary' field",
"No findings have severity 'high' with verdict 'CONFIRMED' or 'LIKELY'",
"The review summary does not request changes or flag critical issues"
"The review does not label any finding as CRITICAL or block-worthy"
]
},
"messages": [
Expand Down
59 changes: 48 additions & 11 deletions review-pr/agents/pr-review.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ models:
sonnet:
provider: anthropic
model: claude-sonnet-4-5
temperature: 0.0
max_tokens: 8192
haiku:
provider: anthropic
Expand Down Expand Up @@ -324,17 +325,37 @@ agents:
goroutine leaks, range var capture in closures, mutex copied by value,
context not propagated, channel deadlocks, panic in library code

## When in Doubt

Err on the side of reporting. A finding that a human reviewer dismisses costs them
seconds. A missed finding that reaches production can cost much more. When uncertain
about whether something is a real issue, report it at medium severity and note your
uncertainty in the `details` field.

However, "when in doubt" does not mean "invent scenarios." You must be able to describe
a concrete trigger path in production code. Do not flag:
- Test-only code paths or standard testing patterns (mocking, stubbing, test doubles)
- Variables that are only mutated in test files
- Hypothetical issues that require ignoring the Ignore list above

## Ignore

Style, formatting, naming, documentation, test files.
Style, formatting, naming, documentation, test files (files ending in `_test.go`,
`*.test.ts`, `*.spec.js`, `test_*.py`, or in `__tests__`/`tests`/`test` directories).
Existing code not changed in this PR.
Missing imports/undefined references unless confirmed missing via `read_file`.
Standard testing patterns: overriding package-level variables for test doubles,
monkey-patching, mocking, stubbing β€” even when the variable is declared in production
code, if it is only mutated in test files it is not a production concurrency bug.

## Severity

- **high**: WILL cause harm β€” data loss, security breach, crash, outage. Rare.
- **medium**: COULD cause issues β€” race conditions, resource leaks, edge cases. Default.
- **low**: Code smells. Rarely report.
- **high**: WILL cause harm or HAS no visible mitigation β€” data loss, security vulnerabilities,
crashes, outages. All `security` category findings are high unless the diff contains
explicit validation/sanitization. Do not assume external systems validate inputs.
- **medium**: COULD cause issues under specific conditions β€” race conditions, resource leaks,
edge cases, error handling gaps.
- **low**: Code smells, minor inefficiencies. Rarely report.

## Output

Expand Down Expand Up @@ -423,11 +444,26 @@ agents:
etc.) before confirming that an API or language feature doesn't exist.

Verify each one independently.
Your job is to filter out false positives. For each finding, check if:
Your job is to verify findings, not to filter them out. Default to LIKELY unless you have
concrete evidence to DISMISS. For each finding:
- **THE CODE IS ACTUALLY CHANGED IN THIS PR** (if not, DISMISS immediately)
- The bug can actually happen given the surrounding code
- Existing safeguards already prevent it
- Tests cover this case
- Can you find explicit safeguards in the diff or source files that prevent the bug?
Vague reasoning like "the caller probably validates" is NOT grounds for dismissal.
- Do tests in the diff specifically cover this edge case? General test existence is not enough.

**DISMISS requires proof.** You must cite the specific code (file + line) that prevents
the bug. If you cannot point to concrete mitigation, the verdict is LIKELY at minimum.

**Security findings have a higher bar for dismissal.** Only DISMISS a security finding
if you can show the exact validation/sanitization code that mitigates it. Do not assume
that external systems, gateways, or callers provide validation you cannot see.

**DISMISS test-only patterns.** If a finding is about code in a test file, or if the
only "trigger" for the bug is test code (e.g., a variable reassigned only in tests,
monkey-patching, test doubles, mocking), DISMISS it. Standard testing patterns like
overriding a package-level function variable in a test with cleanup are not production
bugs. The drafter's Ignore list excludes test files, so these should not reach you β€”
but if they do, dismiss them.

## Reading Files for Context

Expand All @@ -447,9 +483,10 @@ agents:
array. Return one verdict per finding you were given. For each verdict:

- `verdict`: One of `"CONFIRMED"`, `"LIKELY"`, `"DISMISSED"`
- CONFIRMED: Definitely a bug in changed code
- LIKELY: Probably a bug in changed code
- DISMISSED: Not a bug OR not in changed code
- CONFIRMED: Bug verified β€” you found no mitigation in the source code
- LIKELY: Probable bug β€” you could not fully verify but found no evidence against it
- DISMISSED: Proven not a bug β€” you can cite the specific code that prevents it, OR
the finding is not in code changed by this PR
- `file`: Preserve the file path from the drafter's finding
- `line`: Preserve or correct the line number from the drafter's finding
- `severity`: You may adjust severity from what the drafter assigned based on full context
Expand Down
Loading