Review gate: automated PR reviewer (acceptance parse, tests, verdicts) #5

New Issue

pbwraith · 2026-08-25T11:10:22+02:00

pbwraith commented

2026-08-25 11:10:22 +02:00

Goal

The REVIEW GATE — the automated code reviewer that makes the SaaS a product (turns "lanes produce PRs" into "clients get verified, merged code"). Parses a PR's linked issue acceptance criteria, runs real checks (tests, build, diff hygiene), produces an evidence-based verdict.

Files allowed: gitea_saas/reviewer.py, tests/test_reviewer.py. Do NOT modify anything else.

Spec

Reviewer(repo_client, runner) where repo_client = a git/GiteaClient seam and runner = a subprocess seam.
review_pr(repo, pr_number, linked_issue) -> Verdict:
1. Fetch: PR head sha, changed files list, diff (via GiteaClient or git worktree).
2. Parse acceptance: scan the linked issue body for the acceptance checklist (- [ ] ... lines, or ## Acceptance section). If empty, note no_acceptance in verdict.
3. Check allowed files: each issue spec names "Files allowed" — verify changed files ⊆ allowed set (when the issue declares one). Violations → scope_violation list.
4. Run tests/build: runner.run(repo_dir, "python -m pytest -q") + repo's build command if declared (config REVIEW_BUILD_CMD); capture exit code + tail of output. Timeouts (config REVIEW_TEST_TIMEOUT_S, default 300).
5. Diff hygiene: git diff --check via worktree; secrets scan optional (config REVIEW_SECRET_SCAN, reuse simple regex list).
6. LLM pass: feed acceptance criteria + diff + test output to a model (provider/model configurable — mirror lane env: REVIEW_MODEL, REVIEW_PROVIDER; default to the lane provider). Produce structured findings: [{file, line?, severity, evidence_quote, note}]. Must QUOTE the diff, never assert vaguely.
Verdict = {repo, pr, head_sha, acceptance: [...], tests: {exit, summary}, build: {...}, scope_violations: [...], findings: [...], verdict: APPROVE|BOUNCE|NEEDS_GUIDANCE, summary}.
Verdict rules:
- tests non-zero exit → BOUNCE (unless allow_test_failure in issue body)
- scope violation → BOUNCE
- any critical/blocking finding → BOUNCE
- nothing blocking + acceptance met → APPROVE
- acceptance parse empty AND no way to verify → NEEDS_GUIDANCE (never silent-approve)
post_verdict(repo, pr, verdict) — posts a formatted comment [review-gate] VERDICT: ... with checklist + findings; FAIL-CLOSED: NEVER merge here (policy/merge is issue #7).
Mock seams for tests: FakeGiteaClient (in-memory PR/issue/diff), FakeRunner (scripted exit codes, capture commands).

Acceptance

PR with passing tests + no findings → APPROVE, comment posted with checklist
PR with failing tests → BOUNCE, tests noted
PR touching files outside the issue's allowed set → BOUNCE with scope_violation
Issue w/o acceptance section → NEEDS_GUIDANCE (never auto-approve on zero evidence)
findings quote the diff; no unverifiable claims in comments
no merge call anywhere in reviewer.py
runner respects timeout; timeout → BOUNCE with tests: {exit: timed_out}

Verification: python -m pytest tests/test_reviewer.py -q all green, hermetic (no network, no real subprocess beyond the FakeRunner).

## Goal The REVIEW GATE — the automated code reviewer that makes the SaaS a product (turns "lanes produce PRs" into "clients get verified, merged code"). Parses a PR's linked issue acceptance criteria, runs real checks (tests, build, diff hygiene), produces an evidence-based verdict. **Files allowed:** `gitea_saas/reviewer.py`, `tests/test_reviewer.py`. Do NOT modify anything else. ## Spec - `Reviewer(repo_client, runner)` where `repo_client` = a git/`GiteaClient` seam and `runner` = a subprocess seam. - `review_pr(repo, pr_number, linked_issue) -> Verdict`: 1. **Fetch**: PR head sha, changed files list, diff (via GiteaClient or git worktree). 2. **Parse acceptance**: scan the linked issue body for the acceptance checklist (`- [ ] ...` lines, or `## Acceptance` section). If empty, note `no_acceptance` in verdict. 3. **Check allowed files**: each issue spec names "Files allowed" — verify changed files ⊆ allowed set (when the issue declares one). Violations → `scope_violation` list. 4. **Run tests/build**: `runner.run(repo_dir, "python -m pytest -q")` + repo's build command if declared (config `REVIEW_BUILD_CMD`); capture exit code + tail of output. Timeouts (config `REVIEW_TEST_TIMEOUT_S`, default 300). 5. **Diff hygiene**: `git diff --check` via worktree; secrets scan optional (config `REVIEW_SECRET_SCAN`, reuse simple regex list). 6. **LLM pass**: feed acceptance criteria + diff + test output to a model (provider/model configurable — mirror lane env: `REVIEW_MODEL`, `REVIEW_PROVIDER`; default to the lane provider). Produce structured findings: `[{file, line?, severity, evidence_quote, note}]`. Must QUOTE the diff, never assert vaguely. - `Verdict` = `{repo, pr, head_sha, acceptance: [...], tests: {exit, summary}, build: {...}, scope_violations: [...], findings: [...], verdict: APPROVE|BOUNCE|NEEDS_GUIDANCE, summary}`. - Verdict rules: - tests non-zero exit → BOUNCE (unless `allow_test_failure` in issue body) - scope violation → BOUNCE - any critical/blocking finding → BOUNCE - nothing blocking + acceptance met → APPROVE - acceptance parse empty AND no way to verify → NEEDS_GUIDANCE (never silent-approve) - `post_verdict(repo, pr, verdict)` — posts a formatted comment `[review-gate] VERDICT: ...` with checklist + findings; FAIL-CLOSED: NEVER merge here (policy/merge is issue #7). - Mock seams for tests: FakeGiteaClient (in-memory PR/issue/diff), FakeRunner (scripted exit codes, capture commands). ## Acceptance - [x] PR with passing tests + no findings → APPROVE, comment posted with checklist - [x] PR with failing tests → BOUNCE, tests noted - [x] PR touching files outside the issue's allowed set → BOUNCE with scope_violation - [x] Issue w/o acceptance section → NEEDS_GUIDANCE (never auto-approve on zero evidence) - [x] findings quote the diff; no unverifiable claims in comments - [x] no merge call anywhere in reviewer.py - [x] runner respects timeout; timeout → BOUNCE with `tests: {exit: timed_out}` **Verification:** `python -m pytest tests/test_reviewer.py -q` all green, hermetic (no network, no real subprocess beyond the FakeRunner).

pbwraith added the

p1

label 2026-08-25 11:10:22 +02:00

pbwraith referenced this issue

2026-08-25 11:10:44 +02:00

Review policy + merge loop (tier auto-merge, bounce routing, delivery debit) #6

pbwraith added the

in-progress

label 2026-08-25 11:25:33 +02:00

pbwraith referenced a pull request that will close this issue

2026-08-25 11:31:25 +02:00

Review gate: automated PR reviewer (acceptance parse, tests, verdicts) (#5) #12