diff --git a/.opencode/README.md b/.opencode/README.md new file mode 100644 index 0000000..cfa11c8 --- /dev/null +++ b/.opencode/README.md @@ -0,0 +1,121 @@ +# pragent `.opencode/` — the review factory + +pragent's review runs on **opencode** (the AI-coding-agent CLI). This directory is +a portable **factory**: the `opencode.json` + `.opencode/` are dropped into a +checked-out copy of the target repo at the PR head sha, then `opencode run` is +launched there. The `pragent` primary agent reviews the diff with real tools +(subagents, LSP/linters via bash, webfetch references) and emits a structured +findings JSON. A thin Python shell posts that JSON back to Gitea as inline +comments + ```suggestion blocks + a summary (dedupe + anchor validation stay +deterministic in Python). + +## Layout + +``` +opencode.json provider (headroom → glm-5.2:cloud), model, lsp, permission, default_agent +.opencode/ + agents/ + pragent.md PRIMARY reviewer — reads .pragent/brief.md, runs linters, emits findings JSON + security.md subagent — injection/auth/secrets/supply-chain lens (dormant) + tests.md subagent — missing/weak test coverage lens (dormant) + perf.md subagent — N+1 / O(n²) / hot-path lens (dormant) + skills/ + review-methodology/SKILL.md severity rubric, what to report, anchoring rules + findings-schema/SKILL.md the exact output JSON shape + commands/ + review.md /review slash command (local interactive use) + README.md this file +``` + +## How a review runs + +```mermaid +flowchart TD + WH["webhook_server.py
HMAC + AI-REVIEW gate + dedupe"] --> RP["ai_review.review_pr"] + RP --> ARCH["fetch repo archive @ head sha
→ /tmp/pragent-work/-"] + ARCH --> BRIEF["write .pragent/brief.md
(title, body, diff, config, prior, sha)"] + BRIEF --> DROP["drop opencode.json + .opencode/ into workdir"] + DROP --> OC["opencode run --pure --agent pragent --dir
--model headroom/glm-5.2:cloud"] + OC --> PR["pragent primary
load skills · run linters · read code · delegate lenses"] + PR --> JSON["final message: summary + ```json findings```"] + JSON --> PARSE["ai_review.extract_findings
{summary, findings}"] + PARSE --> ANCHOR["parse_diff_anchors → split_findings"] + ANCHOR --> POST["post_inline_review
summary + inline ```suggestion + ref links + sha marker"] +``` + +## Lean by default + +The `pragent` primary does the whole review in one pass for small/medium diffs +(no subagent calls). It delegates to `@security` / `@tests` / `@perf` subagents +ONLY on large (>~400 lines) or security-sensitive diffs. Token cost scales with +PR size. `subagent_depth: 2` caps recursion. + +`--pure` is passed at runtime so the reviewer doesn't load the host user's heavy +global opencode plugins (supermemory/dcp/morph/pty) which hang cold-start. In the +deploy pod there's no global config, so `--pure` is a no-op there — but it keeps +host-local runs deterministic. + +## Extending the factory + +### Add a review lens (subagent) + +1. Create `.opencode/agents/.md` with `mode: subagent`, `hidden: true`, a + `description`, and a read-only `permission` (deny edit/write, allow bash/webfetch, + `task: deny` so it can't recurse). The body is its system prompt; end it by + requiring the same findings-JSON shape. +2. Allow it in the primary's `permission.task` list in `pragent.md`: + ```yaml + task: + "*": "deny" + "security": "allow" + "tests": "allow" + "": "allow" # add this + ``` +3. Mention in `pragent.md`'s "Delegate on heavy diffs" step when to invoke it. + +That's it — the primary can now `@` it via the Task tool. It stays dormant +(the primary decides when), so adding it costs nothing for small PRs. + +### Add a skill + +1. `mkdir .opencode/skills/ && touch .opencode/skills//SKILL.md` +2. Frontmatter: `name: ` (kebab-case, matches dir), `description:` (specific + enough for the agent to pick it). Body = the knowledge. +3. Refer to it from `pragent.md` ("Call the `skill` tool for ``"). + +Per-language expertise is free: the host user already has 29 global skills +(golang-*, react-*, k8s, terraform, testing, typescript, …). opencode +auto-discovers them via the `skill` tool — the pragent primary loads a matching +one when the repo's language fits. To ship a pragent-specific one, just drop it +here. + +### Change the output shape + +Edit `.opencode/skills/findings-schema/SKILL.md` (the schema doc) AND the Python +parser in `pilot/ai_review.py` (`parse_findings`) + the renderers +(`inline_comment_body`, `summary_bullets`, `format_review_body`). Keep them in +sync — the parser is tolerant but the agent and parser must agree on field names. + +### Switch model / provider + +Edit `opencode.json` `provider` + `model`. The provider points at the on-network +headroom proxy (`http://100.74.17.70:8789/v1`, Anthropic `/v1/messages` format, +`apiKey: ollama`) → `glm-5.2:cloud`. To use a different model, add a provider and +reference it as `/`. + +## Local one-shot review (no webhook) + +```bash +cd ~/Projects/pragent +opencode run --pure --agent pragent --dir . \ + --model headroom/glm-5.2:cloud \ + "Read .pragent/brief.md if present, else review \`git diff HEAD\`, and output findings." +``` + +Or in the TUI: `/review` (uses `.opencode/commands/review.md`). + +## Engine flag + +`PRAGENT_ENGINE=opencode` (default once wired) uses this factory. `=ollama` +falls back to the legacy direct model call in `pilot/ai_review.py`. The two +share all Gitea I/O, dedupe, and posting logic. \ No newline at end of file diff --git a/.opencode/agents/perf.md b/.opencode/agents/perf.md new file mode 100644 index 0000000..65ccdc9 --- /dev/null +++ b/.opencode/agents/perf.md @@ -0,0 +1,44 @@ +--- +description: Performance lens subagent. Flags obvious hotspots, N+1 queries, O(n^2) in hot paths, and redundant work in a PR diff. Invoked by the pragent primary on diffs touching hot paths. +mode: subagent +hidden: true +model: headroom/glm-5.2:cloud +temperature: 0.1 +permission: + edit: deny + write: deny + bash: + "*": "allow" + "rm -rf *": "deny" + "git push *": "deny" + "git commit *": "deny" + "sudo *": "deny" + task: deny +--- + +You are a **performance reviewer** subagent. The pragent primary hands you a +PR's diff (and the checked-out repo). Flag ONLY clear, actionable perf issues — +be conservative, skip micro-optimizations: + +- **N+1 queries:** a query inside a loop, or per-item lazy loads. +- **O(n²) / nested loops** over collections that grow with input. +- **Redundant work:** repeated computation, re-fetching the same data, building + the same structure per iteration. +- **Hot-path bloat:** expensive work moved into a frequently-called path + (per-request middleware, render loops, inner loops). +- **Unbounded growth:** caches/maps/arrays that grow without eviction, recursive + calls without depth bounds. +- **Sync I/O / blocking** in an async or request-hot context. + +Read surrounding code to confirm the loop/query is actually in a hot path before +flagging — don't flag a one-time startup cost. Use `grep` to find call sites. + +Return STRICT JSON only — same shape as the pragent primary's findings, perf +findings only. `severity` `high` for an N+1 in a request path, `medium` for +O(n²) over bounded small n, `low` for redundant-but-rare work. + +```json +{"findings":[{"severity":"...","path":"...","line":0,"problem":"...","fix":"...","suggestion":"","reference":""}]} +``` + +`line` must be a post-change line. No prose outside JSON. \ No newline at end of file diff --git a/.opencode/agents/pragent.md b/.opencode/agents/pragent.md new file mode 100644 index 0000000..26098fc --- /dev/null +++ b/.opencode/agents/pragent.md @@ -0,0 +1,124 @@ +--- +description: AI code reviewer for a Gitea PR. Reads the review brief, inspects the checked-out repo, runs linters/typecheck, delegates to lens subagents on heavy diffs, and emits a structured findings JSON. +mode: primary +model: headroom/glm-5.2:cloud +temperature: 0.2 +steps: 40 +permission: + edit: deny + write: deny + apply_patch: deny + bash: + "*": "allow" + "rm -rf *": "deny" + "rm -fr *": "deny" + "git push *": "deny" + "git commit *": "deny" + "git reset --hard*": "deny" + "sudo *": "deny" + webfetch: allow + task: + "*": "deny" + "security": "allow" + "tests": "allow" + "perf": "allow" +--- + +You are **pragent**, a senior, pragmatic AI code reviewer. You review ONE pull +request per session and output a structured report. A thin Python shell posts +your output back to Gitea as inline comments + a summary — so your ONLY job is +to produce correct, well-anchored findings. + +## Input + +Start by reading `.pragent/brief.md` in the project root. It contains: +- `repo`, `pr_index`, `head_sha` — the PR identity +- `title`, `description` — PR meta +- `diff` — the full unified diff (this is what changed) +- `repo_config` — optional `.pr-review.json` focus / exclude_paths / languages / instructions +- `prior_reviews` — earlier bot reviews on this PR (do NOT repeat settled points) +- `anchor_hint` — how post-change (RIGHT-side) line numbers work for inline comments + +The **project root is the target repo checked out at the PR head sha**, so the +changed files and their surrounding code are all present on disk. Use that — +read the full file around a flagged line, not just the diff hunk. + +## Method (in order) + +1. **Load your skills.** Call the `skill` tool for `review-methodology` and + `findings-schema`. They define the severity rubric, the output JSON shape, and + the anchor rules. Honor any repo_config focus / instructions. + +2. **Map the change.** Skim the diff. Note the changed paths, the languages, and + whether the change touches security-sensitive areas (auth, crypto, SQL, file + I/O, deserialization, CI/supply-chain, secrets). + +3. **Run the repo's own checks via bash.** Detect tooling and run it on the + CHANGED files only (keep it fast, keep tokens low): + - TS/JS: `npx --no-install tsc --noEmit` if `tsconfig.json` exists; `npx --no-install eslint ` if configured. + - Python: `ruff check ` or `python -m pyright ` / `mypy` if configured. + - Go: `go vet ./` if `go` is on PATH. + - If `rtk` is on PATH, prefer `rtk grep` / `rtk git diff` for token-cheap search output. + - Never run install/build steps (`npm install`, `go mod download`, etc.) — too slow / too much output. If a check needs deps that aren't installed, skip it and note that. + - Capture only diagnostics (errors/warnings), not success prose. + +4. **Find real issues.** Combine: the diff, the surrounding code you read, and the + linter/typecheck diagnostics. Report ONLY real, actionable issues — correctness + bugs, security problems, risky changes, missing tests for changed behavior, + breaking API/contract changes. Skip praise, nitpicks, pure formatting. + +5. **References.** When a finding involves a specific library API, known + vulnerability, or footgun, use `webfetch` to confirm it (e.g. a CVE page, the + library docs) and put the URL in the finding's `reference` field. Leave + `reference` empty when there's nothing authoritative to link. Don't fetch for + the sake of it — keep it lean. + +6. **Delegate on heavy diffs.** If the diff is large (>~400 changed lines) OR + touches auth/crypto/SQL/deserialization/CI, delegate that lens to a subagent + via the Task tool: + - `@security` — injection, auth, secrets, supply-chain, unsafe deserialization. + - `@tests` — missing or weak tests for the changed behavior. + - `@perf` — obvious hotspots, N+1 queries, O(n²) in hot paths. + Each subagent returns its own findings; merge them (dedup overlapping ones, + keep highest severity). For small/medium diffs, do all lenses inline yourself — + do NOT spawn subagents. Cost must scale with PR size. + +7. **Anchor every finding.** Each finding's `line` MUST be a line that exists in + the POST-CHANGE version of `path` — a context line or an added `+` line shown + in the diff. Never a removed line. If unsure, use the closest context line you + can see in the diff. A finding with a bad line gets folded into the summary as + a bullet instead of an inline comment, so anchoring correctly is what makes a + suggestion apply-able in Gitea. + +## Output — REQUIRED exact shape + +Your FINAL message must be a short plain-prose summary (1–4 sentences: what the +PR does, overall risk, severity counts) FOLLOWED by a single fenced code block +containing STRICT JSON, nothing else after it: + +```json +{ + "summary": "One-paragraph overview of the change and its risk.", + "findings": [ + { + "severity": "critical|high|medium|low", + "path": "path exactly as in the diff `+++ b/` side", + "line": 12, + "problem": "one line: what is wrong", + "fix": "one line: how to fix it", + "suggestion": "exact replacement lines for that location, indented as in the file, or \"\" if no safe replacement", + "reference": "https://... or \"\"" + } + ] +} +``` + +Rules: +- `suggestion` is the literal new code that replaces the flagged line(s). Minimal — + just the changed lines, indented as they'd appear in the file. Empty string `""` + when no safe textual replacement exists (e.g. missing test, architectural note). +- At most ~15 findings, highest severity first. +- If the diff is clean, output `{"summary":"...","findings":[]}`. +- Do NOT repeat anything in `prior_reviews`. +- The JSON block must be the LAST thing in your message — the Python shell parses + the last ```json fenced block from your output. \ No newline at end of file diff --git a/.opencode/agents/security.md b/.opencode/agents/security.md new file mode 100644 index 0000000..aaedfbc --- /dev/null +++ b/.opencode/agents/security.md @@ -0,0 +1,47 @@ +--- +description: Security lens subagent. Scans a PR diff for injection, auth, secret, and supply-chain risks and returns findings JSON. Invoked by the pragent primary on large or security-sensitive diffs. +mode: subagent +hidden: true +model: headroom/glm-5.2:cloud +temperature: 0.1 +permission: + edit: deny + write: deny + bash: + "*": "allow" + "rm -rf *": "deny" + "git push *": "deny" + "git commit *": "deny" + "sudo *": "deny" + webfetch: allow + task: deny +--- + +You are a **security reviewer** subagent. The pragent primary hands you a PR's +diff (and the checked-out repo). Hunt ONLY for security issues: + +- **Injection:** SQL/NoSQL/LDAP/command/template injection, unsanitized input + flowing into interpreters. SQL must use parameterized queries / prepared + statements — flag string-built queries. +- **Auth & access control:** broken auth checks, missing authorization, insecure + token/session handling, password compared with `==` (use constant-time compare). +- **Secrets:** hardcoded credentials, API keys, private keys committed, secrets in + logs/URLs/error messages. +- **Supply chain:** suspicious new dependencies, typosquats, `eval`/`exec`/`new + Function` on user input, unsafe deserialization, SSRF, path traversal. +- **Crypto:** weak algorithms (MD5/SHA1 for security), homemade crypto, bad random + (`Math.random`/`random` for tokens). + +Use `webfetch` to confirm a CVE or library footgun and cite it in `reference`. +Read surrounding code from the checked-out repo when a sink's data flow isn't +clear from the diff alone. + +Return STRICT JSON only — same shape as the pragent primary's findings, but +security findings only: + +```json +{"findings":[{"severity":"critical|high|medium|low","path":"...","line":0,"problem":"...","fix":"...","suggestion":"...","reference":"https://..."}]} +``` + +`line` must be a post-change (context or `+`) line. Empty `suggestion` when no +safe replacement. No prose outside the JSON block. \ No newline at end of file diff --git a/.opencode/agents/tests.md b/.opencode/agents/tests.md new file mode 100644 index 0000000..57204e8 --- /dev/null +++ b/.opencode/agents/tests.md @@ -0,0 +1,44 @@ +--- +description: Test-coverage lens subagent. Checks whether changed behavior has matching tests and flags weak/missing assertions. Invoked by the pragent primary on diffs that change logic. +mode: subagent +hidden: true +model: headroom/glm-5.2:cloud +temperature: 0.1 +permission: + edit: deny + write: deny + bash: + "*": "allow" + "rm -rf *": "deny" + "git push *": "deny" + "git commit *": "deny" + "sudo *": "deny" + task: deny +--- + +You are a **test-coverage reviewer** subagent. The pragent primary hands you a +PR's diff (and the checked-out repo). Focus ONLY on test coverage of changed +behavior: + +- New logic (branches, conditions, error paths) with **no** test exercising it. +- Tests that **don't assert** the changed behavior (e.g. a test that runs code + but doesn't check the new return value / side effect). +- Missing edge cases: null/empty/zero/boundary inputs, error/exception paths, + concurrency, off-by-one. +- Tests that would now fail because the changed contract wasn't updated (or + shouldn't fail but will — flag the stale expectation). + +Read the checked-out repo to find existing tests near the changed code and +judge whether they cover the change. Use `grep`/`glob` to locate test files. + +Return STRICT JSON only — same shape as the pragent primary's findings, test +findings only. `severity` is `medium` for a missing test on changed logic, +`high` for an untested security/error path, `low` for a missing edge case. +`suggestion` is usually empty for "missing test" findings (no safe textual +replacement); include a sketch only if a one-line test is obvious. + +```json +{"findings":[{"severity":"...","path":"...","line":0,"problem":"...","fix":"...","suggestion":"","reference":""}]} +``` + +`line` must be a post-change line in a source or test file. No prose outside JSON. \ No newline at end of file diff --git a/.opencode/commands/review.md b/.opencode/commands/review.md new file mode 100644 index 0000000..e091882 --- /dev/null +++ b/.opencode/commands/review.md @@ -0,0 +1,16 @@ +--- +description: Review the current uncommitted/git diff as pragent (local, interactive). Not used by the webhook. +agent: pragent +--- +Review the current diff in this repository as pragent. + +The diff to review: +!`git diff HEAD` + +Also review staged + recent uncommitted changes: +!`git diff --cached` + +Inspect the surrounding code, run available linters/typecheck on the changed +files, and produce the pragent findings: a short prose summary followed by the +```json findings block per the findings-schema skill. Output findings only — +do not edit any files. \ No newline at end of file diff --git a/.opencode/skills/findings-schema/SKILL.md b/.opencode/skills/findings-schema/SKILL.md new file mode 100644 index 0000000..eebeed0 --- /dev/null +++ b/.opencode/skills/findings-schema/SKILL.md @@ -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":"","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`. \ No newline at end of file diff --git a/.opencode/skills/review-methodology/SKILL.md b/.opencode/skills/review-methodology/SKILL.md new file mode 100644 index 0000000..4f6eca0 --- /dev/null +++ b/.opencode/skills/review-methodology/SKILL.md @@ -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. \ No newline at end of file diff --git a/opencode.json b/opencode.json new file mode 100644 index 0000000..4637e40 --- /dev/null +++ b/opencode.json @@ -0,0 +1,40 @@ +{ + "$schema": "https://opencode.ai/config.json", + "default_agent": "pragent", + "model": "headroom/glm-5.2:cloud", + "small_model": "headroom/glm-5.2:cloud", + "provider": { + "headroom": { + "npm": "@ai-sdk/anthropic", + "name": "Headroom GLM", + "options": { + "baseURL": "http://100.74.17.70:8789/v1", + "apiKey": "ollama" + }, + "models": { + "glm-5.2:cloud": { + "name": "GLM 5.2 Cloud", + "limit": { "context": 200000, "output": 16000 } + } + } + } + }, + "lsp": {}, + "permission": { + "*": "allow", + "edit": "deny", + "write": "deny", + "apply_patch": "deny", + "bash": { + "*": "allow", + "rm -rf *": "deny", + "rm -fr *": "deny", + "git push *": "deny", + "git commit *": "deny", + "git reset --hard*": "deny", + "sudo *": "deny" + }, + "external_directory": "allow", + "doom_loop": "deny" + } +} \ No newline at end of file diff --git a/pilot/ai_review.py b/pilot/ai_review.py index bddcd11..0200f18 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -130,19 +130,25 @@ def parse_text_blocks(content: list) -> str: return "\n".join(out).strip() -def format_review_body(findings: str, model: str, sha: str) -> str: +def format_review_body(findings: str, model: str, sha: str, summary: str = "") -> str: """Format the posted review summary body. `findings` is the bullet text for findings that could NOT be anchored inline (or, on the legacy/no-inline path, the whole review). Empty -> "No issues - found.". The hidden sha marker is always appended for the dedupe pass. + found.". `summary` (optional, opencode engine) is rendered as a "Summary" + section right under the header. The hidden sha marker is always appended + for the dedupe pass. """ header = REVIEW_HEADER.format(model=model, sha=sha[:8] if sha else "unknown") findings = (findings or "").strip() if not findings: findings = "No issues found." marker = SHA_MARKER.format(sha=sha) if sha else "" - body = f"{header}\n\n{findings}" + parts = [header] + if summary: + parts.append(summary.strip()) + parts.append(findings) + body = "\n\n".join(parts) if marker: body += f"\n{marker}" return body @@ -258,6 +264,43 @@ def _strip_path_prefix(p: str) -> str: # --------------------------------------------------------------------------- +def _normalize_finding(f: dict) -> dict | None: + """Validate + normalize one raw finding dict. Returns None if it's unusable + (missing path/line). Normalises severity, keeps `reference` (default "").""" + if not isinstance(f, dict): + return None + path = f.get("path") + line = f.get("line") + if not isinstance(path, str) or not path.strip(): + return None + if not isinstance(line, int) or line < 1: + return None + sev = str(f.get("severity", "medium")).strip().lower() + if sev not in SEVERITIES: + sev = "medium" + reference = str(f.get("reference", "") or "").strip() + return { + "severity": sev, + "path": path.strip(), + "line": line, + "problem": str(f.get("problem", "")).strip(), + "fix": str(f.get("fix", "")).strip(), + "suggestion": str(f.get("suggestion", "") or "").strip(), + "reference": reference, + } + + +def _last_json_block(text: str) -> str | None: + """Return the substring of the last fenced ```json block in text, or None. + Falls back to _extract_first_json_object when no fence is present.""" + s = text or "" + # Find all ```json ... ``` fenced blocks; take the last. + blocks = list(re.finditer(r"```(?:json)?\s*(\{.*?\})\s*```", s, re.DOTALL)) + if blocks: + return blocks[-1].group(1) + return _extract_first_json_object(s) + + def parse_findings(text: str) -> list[dict]: """Parse the model's JSON response into a list of finding dicts. @@ -266,23 +309,7 @@ def parse_findings(text: str) -> list[dict]: Drops findings missing path/line or with an unknown severity (normalised). Never raises — returns [] on any parse failure. """ - if not text: - return [] - s = text.strip() - # Strip a single wrapping code fence if present. - if s.startswith("```"): - s = re.sub(r"^```[a-zA-Z]*\n?", "", s) - s = re.sub(r"\n?```$", "", s).strip() - data = None - try: - data = json.loads(s) - except json.JSONDecodeError: - obj = _extract_first_json_object(s) - if obj is not None: - try: - data = json.loads(obj) - except json.JSONDecodeError: - data = None + data = _parse_json_tolerant(text) if not isinstance(data, dict): return [] findings = data.get("findings") @@ -290,28 +317,74 @@ def parse_findings(text: str) -> list[dict]: return [] out = [] for f in findings: - if not isinstance(f, dict): - continue - path = f.get("path") - line = f.get("line") - if not isinstance(path, str) or not path.strip(): - continue - if not isinstance(line, int) or line < 1: - continue - sev = str(f.get("severity", "medium")).strip().lower() - if sev not in SEVERITIES: - sev = "medium" - out.append({ - "severity": sev, - "path": path.strip(), - "line": line, - "problem": str(f.get("problem", "")).strip(), - "fix": str(f.get("fix", "")).strip(), - "suggestion": str(f.get("suggestion", "") or "").strip(), - }) + n = _normalize_finding(f) + if n is not None: + out.append(n) return out +def parse_review_output(text: str) -> tuple[str, list[dict]]: + """Parse the opengine's stdout into (summary, findings). + + Accepts `{"summary": "...", "findings": [...]}` (the opencode pragent agent) + or a bare `{"findings": [...]}`. `summary` defaults to "". Uses the LAST + ```json fenced block (the pragent agent emits JSON as the final block), with + a tolerant fallback. Never raises. + """ + blob = _last_json_block(text) + if blob is None: + return "", [] + try: + data = json.loads(blob) + except json.JSONDecodeError: + return "", [] + if not isinstance(data, dict): + return "", [] + summary = str(data.get("summary", "") or "").strip() + findings = data.get("findings") + out = [] + if isinstance(findings, list): + for f in findings: + n = _normalize_finding(f) + if n is not None: + out.append(n) + return summary, out + + +def _parse_json_tolerant(text: str) -> dict | None: + """Parse a JSON object from text: try the last fenced block, then a direct + parse, then the first balanced object. Returns None on any failure.""" + if not text: + return None + blob = _last_json_block(text) + if blob is not None: + try: + d = json.loads(blob) + if isinstance(d, dict): + return d + except json.JSONDecodeError: + pass + s = text.strip() + if s.startswith("```"): + s = re.sub(r"^```[a-zA-Z]*\n?", "", s) + s = re.sub(r"\n?```$", "", s).strip() + try: + d = json.loads(s) + if isinstance(d, dict): + return d + except json.JSONDecodeError: + pass + obj = _extract_first_json_object(text) + if obj is not None: + try: + d = json.loads(obj) + if isinstance(d, dict): + return d + except json.JSONDecodeError: + pass + return None + + def _extract_first_json_object(s: str) -> str | None: """Return the substring of the first balanced top-level `{ ... }` in s.""" start = s.find("{") @@ -362,7 +435,8 @@ def inline_comment_body(f: dict) -> str: """Render one finding as a positional review-comment body. Includes a ```suggestion fence only if the model produced non-empty - replacement code. Gitea renders that as an apply-able suggestion. + replacement code. Gitea renders that as an apply-able suggestion. Appends a + `📎 ref:` link when the finding carries a `reference` URL. """ sev = f["severity"].upper() body = f"**[{sev}]** {f['problem']}" @@ -370,6 +444,9 @@ def inline_comment_body(f: dict) -> str: body += f"\n\nFix: {f['fix']}" if f["suggestion"]: body += f"\n\n```suggestion\n{f['suggestion']}\n```" + ref = f.get("reference", "") + if ref: + body += f"\n\n📎 ref: {ref}" return body @@ -379,7 +456,8 @@ def summary_bullets(findings: list[dict]) -> str: for f in findings: loc = f"{f['path']}:{f['line']}" if f["line"] else f["path"] fix = f" — fix: {f['fix']}" if f["fix"] else "" - lines.append(f"- **[{f['severity'].upper()}]** `{loc}` — {f['problem']}{fix}") + ref = f" ({f.get('reference', '')})" if f.get("reference") else "" + lines.append(f"- **[{f['severity'].upper()}]** `{loc}` — {f['problem']}{fix}{ref}") return "\n".join(lines) @@ -621,16 +699,40 @@ def review_pr( config = fetch_repo_config(api, repo, sha, token) prior = prior_review_bodies(reviews, sha) - user_prompt = build_user_prompt(title, body, diff, config, prior) - raw_findings = call_model(ollama_url, model, SYSTEM_PROMPT, user_prompt, max_tokens) - findings = parse_findings(raw_findings) + + engine = os.environ.get("PRAGENT_ENGINE", "opencode").strip().lower() + review_summary = "" + if engine == "opencode": + # The review "brain" runs on opencode: it gets the checked-out repo, + # the brief, and the pragent agent factory; returns stdout with a + # summary + findings JSON. We parse + anchor + post here. + import opencode_review # local import keeps the ollama path dep-free + # opencode wants a provider-prefixed model ref (headroom/glm-5.2:cloud); + # `model` here is the bare id (OLLAMA_MODEL). OPENCODE_MODEL overrides + # with the full ref; otherwise we prefix the configured provider. + oc_model = os.environ.get("OPENCODE_MODEL") or f"headroom/{model}" + stdout = opencode_review.run( + api=api, repo=repo, index=index, sha=sha, token=token, + title=title, body=body, diff=diff, config=config, + prior_reviews=prior, model=oc_model, + ) + review_summary, findings = parse_review_output(stdout) + if not findings and not review_summary: + # opencode produced nothing parseable — fall back to a note. + post_review(api, repo, index, token, format_review_body( + "AI review produced no parseable output.", model, sha)) + return True + else: + user_prompt = build_user_prompt(title, body, diff, config, prior) + raw_findings = call_model(ollama_url, model, SYSTEM_PROMPT, user_prompt, max_tokens) + findings = parse_findings(raw_findings) anchors = parse_diff_anchors(diff) anchored, unanchored = split_findings(findings, anchors) # Summary body: the unanchored bullets (or "No issues found."), plus a # one-line note when inline comments were posted so the summary isn't - # empty-looking. + # empty-looking. The opencode engine also carries a prose summary. bullets = summary_bullets(unanchored) summary_parts = [] if anchored: @@ -639,12 +741,12 @@ def review_pr( summary_parts.append(bullets) if not summary_parts: summary_parts.append("No issues found.") - summary_body = format_review_body("\n\n".join(summary_parts), model, sha) + summary_body = format_review_body("\n\n".join(summary_parts), model, sha, summary=review_summary) post_inline_review(api, repo, index, token, summary_body, anchored) print( f"pragent: reviewed {repo}#{index} sha={sha[:8]} " - f"findings={len(findings)} inline={len(anchored)}", + f"engine={engine} findings={len(findings)} inline={len(anchored)}", flush=True, ) return True diff --git a/pilot/opencode_review.py b/pilot/opencode_review.py new file mode 100644 index 0000000..5fcefb0 --- /dev/null +++ b/pilot/opencode_review.py @@ -0,0 +1,422 @@ +#!/usr/bin/env python3 +"""pragent pilot — opencode review engine (the "brain" host). + +When `PRAGENT_ENGINE=opencode` (the default), `ai_review.review_pr` delegates the +analysis to this module instead of making one direct model call. It: + + 1. fetches the target repo's archive at the PR head sha into a temp workdir + (so the reviewer has the real files, not just the diff text); + 2. writes a `.pragent/brief.md` (title, description, diff, repo config, prior + reviews, sha, anchor hint) for the `pragent` agent to read; + 3. drops pragent's `opencode.json` + `.opencode/` factory into the workdir; + 4. runs `opencode run --pure --agent pragent --dir --model ` + headlessly and returns the agent's stdout (the summary + findings JSON). + +The caller (`ai_review.review_pr`) parses that stdout into `(summary, findings)`, +validates the findings against diff anchors, and posts the review to Gitea — so +this module does NO Gitea I/O and NO parsing. It is pure review-engine glue. + +Stdlib only. Fail-open: `run()` raises on failure; `review_pr` catches and posts +a short failure note. + +Env: + PRAGENT_FACTORY_DIR repo root holding opencode.json + .opencode/ (default: + this file's parent's parent — the pragent repo root). + PRAGENT_OPENCODE_BIN path to the opencode CLI (default: shutil.which / the + known linuxbrew path). + PRAGENT_RTK_DIR dir holding the `rtk` binary, prepended to PATH for the + agent's bash tool (default: /home/marcos/.headroom/bin). + PRAGENT_WORK_ROOT parent for temp workdirs (default: /tmp/pragent-work). + PRAGENT_KEEP_WORK if set, leave the workdir on disk for debugging. + PRAGENT_REVIEW_TIMEOUT seconds to allow opencode to run (default: 480). +""" + +import io +import json +import os +import shutil +import subprocess +import tarfile +import tempfile +import urllib.error +import urllib.request + +# Where the factory lives (opencode.json + .opencode/). Default: the pragent +# repo root (this file is at /pilot/opencode_review.py). +_DEFAULT_FACTORY = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +RTK_DIR = os.environ.get("PRAGENT_RTK_DIR", "/home/marcos/.headroom/bin") +WORK_ROOT = os.environ.get("PRAGENT_WORK_ROOT", "/tmp/pragent-work") +TIMEOUT = int(os.environ.get("PRAGENT_REVIEW_TIMEOUT", "480")) + + +def _factory_dir() -> str: + return os.environ.get("PRAGENT_FACTORY_DIR", _DEFAULT_FACTORY) + + +def _opencode_bin() -> str: + b = os.environ.get("PRAGENT_OPENCODE_BIN") + if b: + return b + found = shutil.which("opencode") + if found: + return found + return "/home/linuxbrew/.linuxbrew/bin/opencode" + + +# --------------------------------------------------------------------------- +# Archive fetch + untar +# --------------------------------------------------------------------------- + + +def fetch_archive(api: str, repo: str, sha: str, token: str, dest: str) -> None: + """Download `GET {api}/api/v1/repos/{repo}/archive/{sha}.tar.gz` and extract + into `dest`, stripping the archive's single top-level directory so the repo + files sit directly at `dest/` (matching the diff's `+++ b/foo` paths). + """ + url = f"{api.rstrip('/')}/api/v1/repos/{repo}/archive/{sha}.tar.gz" + req = urllib.request.Request(url, headers={"Authorization": f"token {token}"}) + with urllib.request.urlopen(req, timeout=120) as r: + blob = r.read() + _extract_tar_strip_one(blob, dest) + + +def _extract_tar_strip_one(blob: bytes, dest: str) -> None: + """Extract a tar.gz blob into dest, stripping one common top-level dir. + + If every member shares a single top-level prefix, that prefix is removed + (so `repo-sha/foo` -> `dest/foo`). If members have no common prefix, extract + as-is. Handles dirs, files, symlinks; ignores absolute paths / `..` for safety. + """ + os.makedirs(dest, exist_ok=True) + with tarfile.open(fileobj=io.BytesIO(blob), mode="r:gz") as tar: + members = tar.getmembers() + # Find the common top-level prefix (the part before the first '/'). + top_levels = set() + for m in members: + name = m.name.lstrip("/") + if not name: + continue + top_levels.add(name.split("/", 1)[0]) + prefix = "" + if len(top_levels) == 1: + (prefix,) = top_levels + prefix += "/" # strip "topdir/" + for m in members: + name = m.name.lstrip("/") + if not name: + continue + # Safety: no absolute, no parent traversal. + if ".." in name.split("/"): + continue + rel = name[len(prefix):] if prefix else name + if not rel or rel == "/": + continue + target = os.path.join(dest, rel) + if m.isdir(): + os.makedirs(target, exist_ok=True) + continue + if m.issym(): + parent = os.path.dirname(target) + os.makedirs(parent, exist_ok=True) + try: + if os.path.lexists(target): + os.remove(target) + os.symlink(m.linkname, target) + except OSError: + pass + continue + if m.isreg(): + parent = os.path.dirname(target) + os.makedirs(parent, exist_ok=True) + f = tar.extractfile(m) + if f is None: + continue + with open(target, "wb") as out: + shutil.copyfileobj(f, out) + + +# --------------------------------------------------------------------------- +# Brief + factory drop +# --------------------------------------------------------------------------- + +BRIEF_PATH = ".pragent/brief.md" + +_BRIEF_TEMPLATE = """\ +# pragent review brief + +- **repo:** {repo} +- **pr:** #{index} +- **head_sha:** `{sha}` + +## Title +{title} + +## Description +{description} + +## Repo review config (.pr-review.json) +{config} + +## Prior reviews (already posted — do NOT repeat these points) +{prior} + +## How to anchor inline comments +Each finding `line` MUST be a line that exists in the POST-CHANGE version of +`path` — a context line (leading space in the diff) or an added `+` line. Never +a removed `-` line. Use the closest context line you can see if unsure. + +## Diff +```diff +{diff} +``` +""" + + +def write_brief( + workdir: str, + *, + repo: str, + index: str, + sha: str, + title: str, + description: str, + diff: str, + config: dict | None, + prior_reviews: list[str] | None, +) -> str: + """Render `.pragent/brief.md` in the workdir. Returns the path written.""" + path = os.path.join(workdir, ".pragent") + os.makedirs(path, exist_ok=True) + brief = os.path.join(path, "brief.md") + cfg = "_(none)_" + if config: + cfg = json.dumps(config, indent=2, ensure_ascii=False) + prior = "_(none)_" + if prior_reviews: + prior = "\n\n---\n\n".join(prior_reviews) + if len(prior) > 8000: + prior = prior[:8000] + "\n…[prior reviews truncated]" + content = _BRIEF_TEMPLATE.format( + repo=repo or "?", + index=index or "?", + sha=sha or "?", + title=title or "(none)", + description=description.strip() or "_(none)_", + config=cfg, + prior=prior, + diff=diff or "_(empty)_", + ) + with open(brief, "w", encoding="utf-8") as f: + f.write(content) + return brief + + +def drop_factory(workdir: str) -> None: + """Copy the pragent `opencode.json` + `.opencode/` into the workdir so + `opencode run --dir ` discovers them as project config. Overwrites + any existing ones (the workdir is a throwaway archive checkout).""" + src = _factory_dir() + oc_json = os.path.join(src, "opencode.json") + if os.path.isfile(oc_json): + shutil.copy2(oc_json, os.path.join(workdir, "opencode.json")) + src_oc = os.path.join(src, ".opencode") + dst_oc = os.path.join(workdir, ".opencode") + if os.path.isdir(dst_oc): + shutil.rmtree(dst_oc) + if os.path.isdir(src_oc): + shutil.copytree(src_oc, dst_oc) + + +# --------------------------------------------------------------------------- +# opencode invocation +# --------------------------------------------------------------------------- + +_PROMPT = ( + "Read .pragent/brief.md and review this pull request as pragent. " + "Load the review-methodology and findings-schema skills, inspect the " + "changed files and surrounding code in this repo, run any available " + "linters/typecheck on the changed files via bash, and delegate to the " + "security/tests/perf subagents only if the diff is large or " + "security-sensitive. End your message with a short prose summary followed " + "by the findings JSON code block per the findings-schema skill." +) + + +def _shared_home() -> str: + """A persistent shared HOME for opencode across reviews. + + opencode bootstraps its runtime (bun-installs `@opencode-ai` into + `$HOME/.config/opencode/node_modules` + fetches a models cache) on its FIRST + run in a fresh HOME — and that first run exits WITHOUT producing the answer. + A shared, warmed HOME makes every review a warm run (fast + reliable) and is + safe for this single-reviewer bot (one review at a time). + + The provider/model/permission config (`opencode.json`) is installed here as + the isolated home's GLOBAL config; the `.opencode/` agents/skills/commands + are dropped per-workdir as PROJECT config. Clean split: infra shared, the + review factory per-PR. + """ + home = os.path.join(WORK_ROOT, ".opencode-home") + os.makedirs(home, exist_ok=True) + return home + + +def _ensure_global_config(home: str) -> None: + """Install the pragent opencode.json as the isolated home's global config so + the provider/model/permission are always present (warm-up + every review), + regardless of --dir. Idempotent.""" + dst_dir = os.path.join(home, ".config", "opencode") + os.makedirs(dst_dir, exist_ok=True) + dst = os.path.join(dst_dir, "opencode.json") + src = os.path.join(_factory_dir(), "opencode.json") + if not os.path.isfile(src): + return + # Copy if missing or changed (compare mtime/size to avoid pointless writes). + if not os.path.isfile(dst) or os.path.getmtime(src) > os.path.getmtime(dst): + shutil.copy2(src, dst) + + +def _build_env(home: str) -> dict: + """Build the subprocess env for an opencode run. + + - HOME -> the isolated shared home (so the host user's ~/.config/opencode is + not merged; the pragent opencode.json is installed there as the global + config by _ensure_global_config). + - Drop XDG_*_HOME (force config resolution under the isolated HOME). + - Drop ANTHROPIC_* (host vars like ANTHROPIC_BASE_URL / ANTHROPIC_AUTH_TOKEN / + ANTHROPIC_DEFAULT_*_MODEL leak from the user's shell and confuse opencode's + @ai-sdk/anthropic provider — ANTHROPIC_DEFAULT_SONNET_MODEL=glm-5.2:cloud + makes opencode look for provider "glm-5.2:cloud" → ProviderModelNotFoundError. + The headroom provider's config options.baseURL/apiKey are self-contained). + - Drop stray OPENCODE_* except the LSP flag (set explicitly below). + - Prepend the rtk dir to PATH so the agent's bash tool can call `rtk`. + """ + env = dict(os.environ) + env["HOME"] = home + for k in ("XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME"): + env.pop(k, None) + for k in list(env): + if k.startswith("ANTHROPIC_") or ( + k.startswith("OPENCODE_") and k != "OPENCODE_EXPERIMENTAL_LSP_TOOL" + ): + env.pop(k, None) + path = env.get("PATH", "") + env["PATH"] = (RTK_DIR + os.pathsep + path) if RTK_DIR else path + env.setdefault("OPENCODE_EXPERIMENTAL_LSP_TOOL", "true") + return env + + +def _warm_opencode(home: str, model: str) -> None: + """One-time warm-up: trigger opencode's runtime install so the real run is a + warm run. Runs with the global config present (provider resolvable) so it + doesn't poison the models cache with a negative entry. Idempotent via a + marker file. stdin=DEVNULL + a trivial prompt make this a fast no-op once + the runtime is installed.""" + marker = os.path.join(home, ".pragent.warmed") + if os.path.exists(marker): + return + _ensure_global_config(home) + env = _build_env(home) + try: + subprocess.run( + [_opencode_bin(), "run", "--pure", "--model", model, "ok"], + cwd=home, env=env, capture_output=True, text=True, + stdin=subprocess.DEVNULL, timeout=240, + ) + except (subprocess.TimeoutExpired, Exception): + pass # warm-up output is discarded; the install is what matters + try: + open(marker, "w").close() + except OSError: + pass + + +def run_opencode(workdir: str, model: str, timeout: int | None = None) -> str: + """Run the pragent agent headlessly in `workdir`. Returns the agent's stdout. + + Isolates from the host user's global opencode config by pointing HOME at a + shared temp dir (so ~/.config/opencode is not merged) and passing --pure + (no external plugins). The workdir's opencode.json + .opencode/ (dropped by + drop_factory) are the only project config discovered; the shared home's + global opencode.json supplies the provider/model/permission. PATH prepends + the rtk dir so the agent's bash tool can call `rtk`. Warms the HOME first + (cold runs produce no output) and retries once on empty stdout. + + stdin=DEVNULL is critical: opencode blocks on stdin (permission prompt / + interactive input) when run headlessly via subprocess, hanging until timeout. + """ + bin_ = _opencode_bin() + home = _shared_home() + _warm_opencode(home, model) + env = _build_env(home) + + cmd = [ + bin_, + "run", + "--pure", + "--agent", "pragent", + "--dir", workdir, + "--model", model, + _PROMPT, + ] + last_err = "" + for attempt in range(2): + try: + proc = subprocess.run( + cmd, cwd=workdir, env=env, capture_output=True, text=True, + stdin=subprocess.DEVNULL, timeout=timeout or TIMEOUT, + ) + except subprocess.TimeoutExpired as e: + last_err = f"opencode timed out after {e.timeout}s" + continue + out = (proc.stdout or "").strip() + if out: + return proc.stdout + last_err = f"opencode empty stdout (rc={proc.returncode}); stderr: {(proc.stderr or '')[-1500:]}" + raise RuntimeError(last_err or "opencode produced no output") + + +# --------------------------------------------------------------------------- +# Orchestrator entry point +# --------------------------------------------------------------------------- + + +def run( + *, + api: str, + repo: str, + index: str, + sha: str, + token: str, + title: str, + body: str, + diff: str, + config: dict | None, + prior_reviews: list[str] | None, + model: str, +) -> str: + """End-to-end: checkout archive → brief → drop factory → opencode → stdout. + + Returns the raw opencode stdout (summary + findings JSON). Raises on any + failure; the caller (review_pr) fails open. The workdir is removed unless + PRAGENT_KEEP_WORK is set. + """ + os.makedirs(WORK_ROOT, exist_ok=True) + workdir = tempfile.mkdtemp(prefix=f"{repo.replace('/', '_')}-{sha[:8]}-", dir=WORK_ROOT) + keep = bool(os.environ.get("PRAGENT_KEEP_WORK")) + try: + fetch_archive(api, repo, sha, token, workdir) + write_brief( + workdir, + repo=repo, index=index, sha=sha, title=title, description=body, + diff=diff, config=config, prior_reviews=prior_reviews, + ) + drop_factory(workdir) + stdout = run_opencode(workdir, model) + if not stdout.strip(): + raise RuntimeError("opencode produced no output") + return stdout + finally: + if not keep: + shutil.rmtree(workdir, ignore_errors=True) \ No newline at end of file diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 0161e27..f2479e5 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -14,6 +14,7 @@ from ai_review import ( # noqa: E402 parse_diff_anchors, parse_findings, parse_repo_config, + parse_review_output, parse_text_blocks, prior_review_bodies, reviewed_shas, @@ -340,4 +341,90 @@ def test_prior_review_bodies_skips_current_sha(): def test_format_review_body_has_sha_marker(): body = format_review_body("- [high] x:1 — b", "glm-5.2:cloud", "abcdef1234567890") - assert "" in body \ No newline at end of file + assert "" in body + + +# --------------------------------------------------------------------------- +# parse_review_output (opencode engine: {summary, findings} + reference) +# --------------------------------------------------------------------------- + + +def test_parse_review_output_summary_and_findings(): + txt = ( + "This PR adds an eval helper — risky. See findings.\n\n" + "```json\n" + '{"summary":"Adds eval() — security risk.","findings":[' + '{"severity":"critical","path":"src/x.ts","line":4,"problem":"eval on user input",' + '"fix":"parse explicitly","suggestion":"const n = Number(s)","reference":"https://owasp.org/x"}' + "]}", + "\n```", + ) + summary, fs = parse_review_output("".join(txt)) + assert "eval()" in summary + assert len(fs) == 1 + assert fs[0]["severity"] == "critical" + assert fs[0]["reference"] == "https://owasp.org/x" + assert fs[0]["suggestion"] == "const n = Number(s)" + + +def test_parse_review_output_bare_findings_no_summary(): + txt = '```json\n{"findings":[{"severity":"low","path":"a","line":1,"problem":"p"}]}\n```' + summary, fs = parse_review_output(txt) + assert summary == "" + assert len(fs) == 1 + assert fs[0]["reference"] == "" # default + + +def test_parse_review_output_empty_and_bogus(): + assert parse_review_output("") == ("", []) + assert parse_review_output("no json here") == ("", []) + assert parse_review_output('{"findings":[]}') == ("", []) + + +def test_parse_review_output_uses_last_json_block(): + # Agent emits a stray json-ish block first, then the real one last. + txt = ( + "```json\n{\"findings\":[{\"path\":\"x\",\"line\":1,\"severity\":\"low\"}]}\n```\n" + "more prose\n" + "```json\n{\"summary\":\"real\",\"findings\":[{\"path\":\"y\",\"line\":2,\"severity\":\"high\"}]}\n```" + ) + summary, fs = parse_review_output(txt) + assert summary == "real" + assert len(fs) == 1 + assert fs[0]["path"] == "y" + + +# --------------------------------------------------------------------------- +# reference rendering in inline_comment_body + summary_bullets + summary section +# --------------------------------------------------------------------------- + + +def test_inline_comment_body_renders_reference(): + f = {"severity": "high", "path": "a", "line": 1, "problem": "p", "fix": "f", + "suggestion": "", "reference": "https://cve.example/X"} + body = inline_comment_body(f) + assert "📎 ref: https://cve.example/X" in body + + +def test_inline_comment_body_no_reference_no_ref_line(): + f = {"severity": "low", "path": "a", "line": 1, "problem": "p", "fix": "", + "suggestion": "", "reference": ""} + assert "📎 ref" not in inline_comment_body(f) + + +def test_summary_bullets_renders_reference(): + fs = [{"severity": "high", "path": "a.py", "line": 7, "problem": "p", "fix": "f", + "suggestion": "", "reference": "https://r.example"}] + b = summary_bullets(fs) + assert "https://r.example" in b + assert "`a.py:7`" in b + + +def test_format_review_body_with_summary_section(): + body = format_review_body("- [high] x:1 — b", "glm-5.2:cloud", "abcdef1234567890", + summary="This PR adds a risky helper.") + assert "This PR adds a risky helper." in body + assert "- [high] x:1" in body + assert "" in body + # summary appears before the findings bullets + assert body.index("risky helper") < body.index("[high]") \ No newline at end of file diff --git a/tests/pilot/test_opencode_review.py b/tests/pilot/test_opencode_review.py new file mode 100644 index 0000000..fabb95f --- /dev/null +++ b/tests/pilot/test_opencode_review.py @@ -0,0 +1,126 @@ +"""Unit tests for the opencode engine glue (no network, no opencode run).""" +import io +import os +import sys +import tarfile + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.abspath(os.path.join(HERE, "..", "..")) +sys.path.insert(0, os.path.join(ROOT, "pilot")) + +import opencode_review as oc # noqa: E402 + + +# --------------------------------------------------------------------------- +# write_brief +# --------------------------------------------------------------------------- + + +def test_write_brief_contains_key_sections(tmp_path): + brief = oc.write_brief( + str(tmp_path), + repo="masi/portfolio", index="3", sha="abcdef1234567890", + title="Add eval helper", description="Closes #1", + diff="diff --git a/x b/x\n+++ b/x\n@@ -1 +1,2 @@\n+eval(input())", + config={"focus": ["security"], "instructions": "Flag eval()."}, + prior_reviews=["🤖 AI Review …\n- [high] old finding"], + ) + assert brief.endswith(".pragent/brief.md") + text = open(brief, encoding="utf-8").read() + assert "masi/portfolio" in text + assert "#3" in text + assert "abcdef1234567890" in text + assert "Add eval helper" in text + assert "Closes #1" in text + assert "eval(input())" in text + assert "security" in text + assert "Flag eval()" in text + assert "old finding" in text + assert "POST-CHANGE" in text # anchor hint + + +def test_write_brief_none_config_and_prior(tmp_path): + brief = oc.write_brief( + str(tmp_path), repo="o/r", index="1", sha="sha1234567", + title="t", description="", diff="d", config=None, prior_reviews=None, + ) + text = open(brief, encoding="utf-8").read() + assert "_(none)_" in text # both config and prior fall back to none + assert "diff" in text + + +# --------------------------------------------------------------------------- +# _extract_tar_strip_one — strips the single top-level dir +# --------------------------------------------------------------------------- + + +def _make_tar(top: str) -> bytes: + """Build a tar.gz in memory with one top-level dir `top` containing files.""" + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + # dir + ti = tarfile.TarInfo(name=f"{top}/") + ti.type = tarfile.DIRTYPE + tar.addfile(ti) + # file src/a.py + data = b"print('a')\n" + ti = tarfile.TarInfo(name=f"{top}/src/a.py") + ti.size = len(data) + tar.addfile(ti, io.BytesIO(data)) + # file README.md + data = b"# hi\n" + ti = tarfile.TarInfo(name=f"{top}/README.md") + ti.size = len(data) + tar.addfile(ti, io.BytesIO(data)) + return buf.getvalue() + + +def test_extract_tar_strips_top_level_dir(tmp_path): + blob = _make_tar("repo-deadbeef") + oc._extract_tar_strip_one(blob, str(tmp_path)) + # files sit directly at dest root (prefix stripped) + assert os.path.isfile(tmp_path / "README.md") + assert os.path.isfile(tmp_path / "src" / "a.py") + assert not os.path.isdir(tmp_path / "repo-deadbeef") # top dir gone + + +def test_extract_tar_no_common_prefix_extracts_as_is(tmp_path): + # Two different top-level entries -> no strip. + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + for name, data in (("a.txt", b"A"), ("b.txt", b"B")): + ti = tarfile.TarInfo(name=name) + ti.size = len(data) + tar.addfile(ti, io.BytesIO(data)) + oc._extract_tar_strip_one(buf.getvalue(), str(tmp_path)) + assert os.path.isfile(tmp_path / "a.txt") + assert os.path.isfile(tmp_path / "b.txt") + + +def test_extract_tar_skips_parent_traversal(tmp_path): + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + ti = tarfile.TarInfo(name="top/../../escape.txt") + data = b"evil" + ti.size = len(data) + tar.addfile(ti, io.BytesIO(data)) + ti = tarfile.TarInfo(name="top/ok.txt") + data = b"ok" + ti.size = len(data) + tar.addfile(ti, io.BytesIO(data)) + oc._extract_tar_strip_one(buf.getvalue(), str(tmp_path)) + assert os.path.isfile(tmp_path / "ok.txt") + assert not os.path.isfile(tmp_path / "escape.txt") + assert not os.path.isfile(os.path.join(str(tmp_path), "..", "escape.txt")) + + +# --------------------------------------------------------------------------- +# drop_factory — copies opencode.json + .opencode/ from the repo +# --------------------------------------------------------------------------- + + +def test_drop_factory_copies_config_and_agents(tmp_path): + oc.drop_factory(str(tmp_path)) + assert os.path.isfile(tmp_path / "opencode.json") + assert os.path.isfile(tmp_path / ".opencode" / "agents" / "pragent.md") + assert os.path.isfile(tmp_path / ".opencode" / "skills" / "findings-schema" / "SKILL.md") \ No newline at end of file