feat: opencode review engine + .opencode factory
Replace the single Python model-call reviewer with an opencode agent
factory. A primary 'pragent' agent reads a brief (title/body/diff/config/
prior reviews), inspects the checked-out repo, runs the repo's own linters
via bash, loads review-methodology + findings-schema skills, and emits a
{summary, findings} JSON with per-finding severity/path/line/problem/fix/
suggestion/reference. Dormant security/tests/perf subagent lenses fan out
only on large/risky diffs (lean by default).
pilot/opencode_review.py: fetches the repo archive at the head sha into a
temp workdir, writes .pragent/brief.md, drops the factory, runs
'opencode run --pure --agent pragent --dir <workdir>' headlessly. Isolates
HOME (shared, warmed), strips ANTHROPIC_* env (leaked host vars caused
ProviderModelNotFoundError), stdin=DEVNULL (opencode blocks on stdin),
maps the bare OLLAMA_MODEL to the provider-prefixed ref. No Gitea I/O —
ai_review.review_pr parses + anchors + posts (reuses all v2 logic/tests).
PRAGENT_ENGINE=opencode (default) selects it; =ollama keeps the legacy
direct-call path. Verified end-to-end: posts a real review with a summary
section, inline [CRITICAL]/[HIGH] comments + apply-able suggestions +
reference links, and the sha dedupe marker. 49 tests pass.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Marcos
@@ -0,0 +1,58 @@
|
||||
---
|
||||
name: findings-schema
|
||||
description: The exact JSON output shape pragent must emit at the end of a review. Load this before producing findings.
|
||||
---
|
||||
|
||||
# pragent findings schema
|
||||
|
||||
The review's FINAL message is a short prose summary followed by ONE fenced
|
||||
```json code block. The Python shell parses the LAST ```json fenced block in the
|
||||
message — so the JSON must be the last thing, and it must be valid.
|
||||
|
||||
## Shape
|
||||
|
||||
```json
|
||||
{
|
||||
"summary": "One-paragraph overview of the change and its risk, plus severity counts.",
|
||||
"findings": [
|
||||
{
|
||||
"severity": "critical|high|medium|low",
|
||||
"path": "path exactly as it appears in the diff's `+++ b/` side",
|
||||
"line": 12,
|
||||
"problem": "one line: what is wrong",
|
||||
"fix": "one line: how to fix it",
|
||||
"suggestion": "exact replacement lines, indented as in the file, or \"\"",
|
||||
"reference": "https://... or \"\""
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
## Field rules
|
||||
|
||||
- `severity` — one of `critical`, `high`, `medium`, `low`. Anything else is
|
||||
coerced to `medium` by the parser.
|
||||
- `path` — the post-change path, exactly as in the diff (`+++ b/foo/bar.ts`
|
||||
→ `foo/bar.ts`). Required; a finding without a real path is dropped.
|
||||
- `line` — a post-change line number (int ≥ 1) that exists in `path` after the
|
||||
PR. Required; bad/missing line → the finding becomes a summary bullet instead
|
||||
of an inline comment.
|
||||
- `problem` — one line, concrete: what is wrong and why it matters.
|
||||
- `fix` — one line, the remedy. Empty string if the fix is architectural.
|
||||
- `suggestion` — the literal new code replacing the flagged line(s). Minimal,
|
||||
just the changed lines, indented as they appear in the file. **Empty string**
|
||||
when no safe textual replacement exists (missing test, architectural note, a
|
||||
fix that needs context beyond one hunk). This is wrapped in a ```suggestion
|
||||
fence → Gitea renders an **apply button**.
|
||||
- `reference` — a URL (CVE, library docs, spec) backing the finding, or `""`.
|
||||
Only link authoritative sources; don't fabricate URLs.
|
||||
|
||||
## Clean diff
|
||||
|
||||
If there's nothing to report: `{"summary":"<what it does, why it's fine>","findings":[]}`.
|
||||
|
||||
## Don't
|
||||
|
||||
- No prose after the closing ``` of the JSON block.
|
||||
- No extra keys — unknown keys are ignored by the parser, so don't rely on them.
|
||||
- Don't repeat findings from `prior_reviews`.
|
||||
@@ -0,0 +1,57 @@
|
||||
---
|
||||
name: review-methodology
|
||||
description: pragent review methodology — severity rubric, what to report vs skip, anchoring rules, and how to honor repo focus. Load this before reviewing a PR.
|
||||
---
|
||||
|
||||
# pragent review methodology
|
||||
|
||||
## Severity rubric
|
||||
|
||||
- **critical** — exploitable security bug, data loss/corruption, or a crash on
|
||||
a normal input path. Must fix before merge.
|
||||
- **high** — correctness bug on a real input path, broken contract, or a
|
||||
missing test for security/error behavior. Should fix before merge.
|
||||
- **medium** — likely bug on an edge case, missing test for changed logic, or a
|
||||
risky pattern that isn't broken yet. Worth fixing.
|
||||
- **low** — minor risk, stale expectation, or a defensive improvement. Nice to
|
||||
have.
|
||||
|
||||
## Report vs skip
|
||||
|
||||
**Report:** correctness bugs, security problems, risky changes, missing tests
|
||||
for changed behavior, breaking API/contract changes, N+1/O(n²) in hot paths.
|
||||
|
||||
**Skip:** praise, nitpicks, pure formatting/style, personal preference,
|
||||
speculative "what if" without a concrete trigger, anything already covered in
|
||||
`prior_reviews`.
|
||||
|
||||
Cap at ~15 findings, highest severity first. Quality over quantity — an empty
|
||||
findings list for a clean diff is a correct result.
|
||||
|
||||
## Anchoring (for inline comments)
|
||||
|
||||
Each finding's `line` MUST be a line that exists in the POST-CHANGE version of
|
||||
`path`:
|
||||
- a **context** line (unchanged, shown with a leading space in the diff), or
|
||||
- an **added** line (shown with a leading `+`).
|
||||
|
||||
Never anchor on a **removed** (`-`) line — it has no post-change line number.
|
||||
If you're unsure of the exact line, use the closest context line you CAN see in
|
||||
the diff. A misanchored finding becomes a summary bullet instead of an inline
|
||||
comment, so correct anchoring is what makes a ```suggestion apply-able in Gitea.
|
||||
|
||||
## Honoring repo config
|
||||
|
||||
If `.pr-review.json` is present, honor it:
|
||||
- `focus` — weight these areas higher, but never ignore a critical issue
|
||||
outside them.
|
||||
- `exclude_paths` — skip findings in these paths.
|
||||
- `languages` — hint to the primary languages; pick matching linters.
|
||||
- `instructions` — house conventions / compliance language; treat as binding
|
||||
reviewer rules.
|
||||
|
||||
## Linters are a signal, not the verdict
|
||||
|
||||
Run the repo's own typecheck/lint on changed files, but translate their output
|
||||
into human findings — a raw `TS2322` is not a review comment. Correlate
|
||||
diagnostics with the diff; ignore diagnostics in files the PR didn't touch.
|
||||
Reference in New Issue
Block a user