Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1639704
feat(risk): add Tier 1 metadata signal script and unit tests
maruiz93 Aug 18, 2026
ffaedf4
feat(risk): add scoring skill and sub-agent definition
maruiz93 Aug 18, 2026
a9b4693
feat(risk): add schema, env flag, and orchestrator integration
maruiz93 Aug 18, 2026
e0596c4
feat(risk): add risk label and sticky comment to post-review
maruiz93 Aug 18, 2026
0054ecc
test(risk): add functional eval cases for risk assessment
maruiz93 Aug 18, 2026
55d198c
fix(risk): use mapfile for file list parsing and fix dispatch note wo…
maruiz93 Aug 18, 2026
9e9f0aa
fix(risk): address review findings from fullsend and qodo bots
maruiz93 Aug 19, 2026
ef21639
fix: narrow has_ci_files to workflow/action paths only
maruiz93 Aug 21, 2026
60e8e20
fix: use established REPO_DIR pattern for clone-deepening
maruiz93 Aug 21, 2026
b1e3af1
fix: enforce score/level consistency in risk_assessment schema
maruiz93 Aug 21, 2026
5c33931
fix: remove redundant requirements.txt literal from dependency patterns
maruiz93 Aug 21, 2026
52d5ac6
fix: simplify author signal parsing and add pagination test
maruiz93 Aug 21, 2026
beb0c4b
fix: address review findings — test pattern, eval labels, post-review…
maruiz93 Aug 24, 2026
889c073
fix: apply risk labels before review submission
maruiz93 Aug 24, 2026
17b0e00
fix: address low-severity review findings
maruiz93 Aug 24, 2026
36a205f
fix(risk): address review findings from fullsend and qodo bots
maruiz93 Aug 19, 2026
a06a0a7
feat(risk): add GitLab support for risk assessment tier scripts
maruiz93 Aug 21, 2026
f00ffcb
fix(risk): address review findings — sanitize warnings, fix token fal…
maruiz93 Aug 24, 2026
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
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ script-test:
$(call run-timed,bash scripts/post-code-test.sh)
$(call run-timed,bash scripts/pre-review-test.sh)
$(call run-timed,bash scripts/post-review-test.sh)
$(call run-timed,bash scripts/risk-tier1-test.sh)
$(call run-timed,bash scripts/post-fix-test.sh)
$(call run-timed,bash scripts/post-retro-test.sh)
$(call run-timed,bash scripts/pre-scribe-test.sh)
Expand Down
1 change: 1 addition & 0 deletions agents/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ fields such as `outcome`, `summary`, `prior_review_sha`, or
| `findings` | array | conditional | Array of finding objects (min 1 item when present)|
| `reason` | string | conditional | One of: `tool-failure`, `missing-context`, `ambiguous-findings`, `token-limit` |
| `label_actions` | object | no | Contextual label recommendations (see `issue-labels` skill) |
| `risk_assessment` | object | no | Risk assessment from the pre-pass sub-agent (see `pr-risk-assessment` skill) |

**Required fields per action:**

Expand Down
17 changes: 16 additions & 1 deletion docs/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ applied — the `pull_request_review` event triggers the [fix agent](fix.md) dir
Stale outcome labels from prior review runs are removed before the new one is
applied.

When risk assessment is enabled (`REVIEW_RISK_ASSESSMENT_ENABLED`), the
post-script applies a `risk/*` label reflecting the composite risk score:

| Label | Score | Meaning |
|-------|-------|---------|
| `risk/low` | 1 | Minimal risk — small, well-scoped change |
| `risk/moderate` | 2 | Some complexity or breadth |
| `risk/elevated` | 3 | Touches sensitive areas or has notable blast radius |
| `risk/high` | 4 | Security-sensitive, large, or cross-cutting change |
| `risk/critical` | 5 | Highest risk — auth, RBAC, or critical infrastructure |

Risk labels are informational — they do not gate the review outcome.

The `issue-labels` skill may also apply contextual labels (e.g., `area/api`,
`priority/high`) but these are informational — they do not control agent
behavior.
Expand Down Expand Up @@ -91,6 +104,8 @@ See [Customizing with AGENTS.md](https://fullsend.sh/docs/guides/user/customizin
| `REVIEW_FINDING_SEVERITY_THRESHOLD` | Minimum severity for findings to include in the review. Findings below this level are filtered out at two independent stages (agent output and post-review processing) as defense-in-depth. Default is set in `harness/review.yaml` (`env.runner` and `env.sandbox`). | `low` | `info`, `low`, `medium`, `high`, `critical` |
| `REVIEW_SKIP_AUTHORS` | Comma-separated list of forge usernames to skip review for. When a PR/MR is opened by a user in this list, the review dispatch exits early without running the agent. Set in `env.runner` in your harness YAML (consumed by the pre-script on the runner). | _(empty — all PRs/MRs are reviewed)_ | Comma-separated logins, e.g. `app/renovate,app/dependabot` |
| `REVIEW_PROTECTED_PATHS` | Comma-separated list of path prefixes the review agent treats as protected. PRs that modify files under these paths cannot be approved by the agent — only a human can grant approval. Default is set in `harness/review.yaml` (`env.runner` and `env.sandbox`); an unset value is a misconfiguration (fail-closed). Set to an empty string to deliberately disable protected-path enforcement entirely. When set to a value that parses to no valid paths (e.g. stray or consecutive commas), the script aborts (fail-closed) as a likely misconfiguration. | See [`harness/review.yaml`](../harness/review.yaml) | Comma-separated path prefixes (e.g. `.github/,deploy/,manifests/`) |
| `REVIEW_RISK_ASSESSMENT_ENABLED` | Enables the risk assessment pre-pass. When `true`, the orchestrator dispatches a risk-assessment sub-agent before the main review dimensions. The sub-agent computes a composite 1–5 risk score from metadata signals, git history, and linked issue context. The post-script applies a `risk/*` label and posts a sticky risk comment. | `true` | `"true"`, `"false"` |
| `REVIEW_GIT_FETCH_DEPTH` | Controls clone deepening for git history analysis (risk assessment Tier 2). When set to `"0"`, the pre-script unshallows the target repo clone so the risk-assessment sub-agent can access full commit history. Unset or any other value disables deepening. Set in `env.runner` in harness YAML (consumed by the pre-script on the runner). | _(unset — no deepening)_ | `"0"` to fully unshallow |

Override either variable by extending the harness file via a `base` reference and setting `env.runner` / `env.sandbox` in your custom harness YAML. `base` composition merges `env.runner`/`env.sandbox` per-key — child values override, everything else inherits from the base (ADR 0045, ADR 0055). Per ADR 0080 and ADR 0081, this harness-level override is the correct path; the CI workflow `env:` block is reserved for infrastructure plumbing, not agent behavior knobs like these.

Expand All @@ -108,7 +123,7 @@ fails closed when `CI_SERVER_HOST` is not set.
The review agent follows the same pre-script / sandbox / post-script pipeline as the other agents.

1. **Pre-script** validates inputs and fetches PR metadata.
2. **Sandbox** — the agent runs the `pr-review` orchestrator skill. The orchestrator triages the change, then dispatches specialized sub-agents in parallel — each covering a distinct review dimension (correctness, security, intent & coherence, style & conventions, docs currency, and optionally cross-repo contracts). Sub-agents run concurrently and return structured findings. The orchestrator collects, deduplicates, and synthesizes findings across dimensions, runs PR-level checks (scope authorization, protected paths), and produces a structured JSON review result. The agent cannot push files, edit code, or push — it is strictly read-only.
2. **Sandbox** — the agent runs the `pr-review` orchestrator skill. The orchestrator first runs pre-pass sub-agents: security triage (for large PRs) and risk assessment (when enabled). It then dispatches specialized dimension sub-agents in parallel — each covering a distinct review dimension (correctness, security, intent & coherence, style & conventions, docs currency, and optionally cross-repo contracts). Sub-agents run concurrently and return structured findings. The orchestrator collects, deduplicates, and synthesizes findings across dimensions, runs PR-level checks (scope authorization, protected paths), and produces a structured JSON review result. The agent cannot push files, edit code, or push — it is strictly read-only.
3. **Validation loop** — the output is checked against a schema, with up to 2 retry iterations if the output is malformed.
4. **Post-script** posts the review on the PR.

Expand Down
27 changes: 27 additions & 0 deletions eval/review/cases/001-risk-low-typo-fix/annotations.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
state: open

labels:
required: []
forbidden:
- risk/elevated
- risk/high
- risk/critical
- ready-for-merge
risk_label_required: true

max_turns: 60
max_cost_usd: 5.00

review_expectations: |
Two-file change: a README typo fix and a basic CODEOWNERS file.
Risk assessment should produce a low (1) or moderate (2) score.
The .github/CODEOWNERS file is a protected path, which raises the
PROTECTED_PATH_COUNT signal — this legitimately nudges the score
toward moderate. Either risk/low or risk/moderate is acceptable;
elevated or higher would be a misclassification.

The PR touches .github/CODEOWNERS (a protected path), so
post-review.sh downgrades an approve to comment — same mechanism
as case 003. This avoids the 422 self-review error that blocks
label application. The ready-for-merge label is forbidden because
the protected-path downgrade always yields requires-manual-review.
22 changes: 22 additions & 0 deletions eval/review/cases/001-risk-low-typo-fix/input.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
forge: github
fixture:
type: pull_request
title: "docs: fix typo in README and add CODEOWNERS"
body: |
Fixes a minor typo in the project README and adds a basic
CODEOWNERS file for repository governance.
base: main
files:
- path: README.md
content: |
# Test Project

This is a test project for evaluating the review agent.

## Getting Started

Run `make build` to compile the project.
- path: .github/CODEOWNERS
content: |
# Default ownership
* @test-org/maintainers
7 changes: 7 additions & 0 deletions eval/review/cases/001-risk-low-typo-fix/repo/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Test Project

This is a test project for evaluating the review agent.

## Getting Started

Run `make biuld` to compile the project.
23 changes: 23 additions & 0 deletions eval/review/cases/002-risk-high-auth-change/annotations.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
state: open

labels:
required: []
forbidden:
- risk/low
- ready-for-merge
risk_label_required: true

max_turns: 80
max_cost_usd: 8.00

review_expectations: |
Multi-file auth refactor touching RBAC, CODEOWNERS, and security-
sensitive paths. Risk assessment should produce a moderate or higher
score (2+) due to security-sensitive path signals. The risk/low
label is forbidden to guard against misclassification.

The PR touches CODEOWNERS (a protected path). If the agent
approves, post-review.sh downgrades the approval to comment,
avoiding the 422 self-review error. The ready-for-merge label is
forbidden because the protected-path downgrade always yields
requires-manual-review.
65 changes: 65 additions & 0 deletions eval/review/cases/002-risk-high-auth-change/input.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
forge: github
fixture:
type: pull_request
title: "refactor: restructure auth middleware"
body: |
Refactors the authentication middleware to support multiple auth
providers. Touches RBAC policy evaluation and token validation.

Closes #42
base: main
files:
- path: internal/auth/handler.go
content: |
package auth

import (
"context"
"net/http"
)

// Handler validates requests against the configured auth provider.
func Handler(provider string) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
token := r.Header.Get("Authorization")
if token == "" {
http.Error(w, "unauthorized", http.StatusUnauthorized)
return
}
if !validateToken(r.Context(), token, provider) {
http.Error(w, "forbidden", http.StatusForbidden)
return
}
next.ServeHTTP(w, r)
})
}
}

func validateToken(ctx context.Context, token, provider string) bool {
switch provider {
case "oidc":
_, err := oidcVerifier.Verify(ctx, token)
return err == nil
case "static":
return constantTimeEqual(token, staticToken)
default:
return false
}
}
- path: internal/auth/rbac.go
content: |
package auth

// CheckPermission verifies the user has the required role.
func CheckPermission(userRoles []string, required string) bool {
for _, role := range userRoles {
if role == required || role == "admin" {
return true
}
}
return false
}
- path: CODEOWNERS
content: |
internal/auth/ @security-team
3 changes: 3 additions & 0 deletions eval/review/cases/002-risk-high-auth-change/repo/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Auth Service

Authentication and authorization middleware.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package auth

import "net/http"

// Handler validates requests using a static token.
func Handler(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
token := r.Header.Get("Authorization")
if token != "valid-token" {
http.Error(w, "unauthorized", http.StatusUnauthorized)
return
}
next.ServeHTTP(w, r)
})
}
19 changes: 19 additions & 0 deletions eval/review/eval.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,23 @@ judges:
return False, f"Forbidden labels present: {present} (actual: {actual})"
return True, f"No forbidden labels found (checked: {forbidden})"

- name: risk_label_present
description: When risk_label_required is set, at least one risk/* label must be present
check: |
import json
required = outputs.get("annotations", {}).get("labels", {}).get("risk_label_required", False)
if not required:
return True, "risk_label_required not set — skipping"
raw = outputs["files"].get("output/fixture-state.json")
if not raw:
return False, "fixture-state.json not found"
state = json.loads(raw)
actual = [l.lower() for l in state.get("labels", [])]
risk_labels = [l for l in actual if l.startswith("risk/")]
if not risk_labels:
return False, f"No risk/* label found (actual: {actual})"
return True, f"Risk label present: {risk_labels}"

- name: max_turns
description: Agent must complete within the declared turn budget
check: |
Expand Down Expand Up @@ -191,6 +208,8 @@ thresholds:
min_mean: 3.0
required_labels:
min_pass_rate: 1.0
risk_label_present:
min_pass_rate: 1.0
forbidden_labels:
min_pass_rate: 1.0
max_turns:
Expand Down
4 changes: 4 additions & 0 deletions harness/review.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ skills:
- skills/pr-review
- skills/code-review
- skills/docs-review
- skills/pr-risk-assessment
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

host_files:
- src: env/gcp-vertex.env
Expand Down Expand Up @@ -58,12 +59,14 @@ env:
REVIEW_FINDING_SEVERITY_THRESHOLD: "low"
REVIEW_PROTECTED_PATHS: ".claude/,.cursor/,.pi/,.gitattributes,.github/,.pre-commit-config.yaml,AGENTS.md,agents/,api-servers/,CLAUDE.md,CODEOWNERS,Containerfile,Dockerfile,harness/,images/,plugins/,policies/,profiles/,providers/,scripts/,skills/"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
REVIEW_GIT_FETCH_DEPTH: "0"
sandbox:
REVIEW_FINDING_SEVERITY_THRESHOLD: "low"
REVIEW_PROTECTED_PATHS: ".claude/,.cursor/,.pi/,.gitattributes,.github/,.pre-commit-config.yaml,AGENTS.md,agents/,api-servers/,CLAUDE.md,CODEOWNERS,Containerfile,Dockerfile,harness/,images/,plugins/,policies/,profiles/,providers/,scripts/,skills/"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
PRIOR_REVIEW_SHA: "${PRIOR_REVIEW_SHA}"
PRIOR_REVIEW_PROVENANCE: "${PRIOR_REVIEW_PROVENANCE}"
REVIEW_RISK_ASSESSMENT_ENABLED: "true"

timeout_minutes: 20

Expand Down Expand Up @@ -115,5 +118,6 @@ forge:
sandbox:
PR_NUMBER: "${MR_NUMBER}"
PR_URL: "${GITLAB_MR_URL}"
REVIEW_TOKEN: "${GITLAB_TOKEN}"
GITLAB_TOKEN: "${GITLAB_TOKEN}"
FULLSEND_FORGE: gitlab
42 changes: 42 additions & 0 deletions schemas/review-result.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,39 @@
},
"label_actions": {
"$ref": "#/$defs/label_actions"
},
"risk_assessment": {
"description": "Score and level must be consistent: 1=low, 2=moderate, 3=elevated, 4=high, 5=critical.",
"type": "object",
"required": ["score", "level", "rationale"],
"properties": {
"score": { "type": "integer", "minimum": 1, "maximum": 5 },
"level": {
"type": "string",
"enum": ["low", "moderate", "elevated", "high", "critical"]
},
"tier1_signals": {
"type": "array",
"items": { "$ref": "#/$defs/risk_signal" }
},
"tier2_signals": {
"type": "array",
"items": { "$ref": "#/$defs/risk_signal" }
},
"tier3_signals": {
"type": "array",
"items": { "$ref": "#/$defs/risk_signal" }
},
"rationale": { "type": "string", "minLength": 1 }
},
"additionalProperties": false,
"allOf": [
{ "if": { "properties": { "score": { "const": 1 } } }, "then": { "properties": { "level": { "const": "low" } } } },
{ "if": { "properties": { "score": { "const": 2 } } }, "then": { "properties": { "level": { "const": "moderate" } } } },
{ "if": { "properties": { "score": { "const": 3 } } }, "then": { "properties": { "level": { "const": "elevated" } } } },
{ "if": { "properties": { "score": { "const": 4 } } }, "then": { "properties": { "level": { "const": "high" } } } },
{ "if": { "properties": { "score": { "const": 5 } } }, "then": { "properties": { "level": { "const": "critical" } } } }
]
}
},
"allOf": [
Expand Down Expand Up @@ -112,6 +145,15 @@
}
},
"additionalProperties": false
},
"risk_signal": {
"type": "object",
"required": ["dimension", "value"],
"properties": {
"dimension": { "type": "string", "minLength": 1 },
"value": { "type": "string", "minLength": 1 }
},
"additionalProperties": false
}
}
}
Loading
Loading