From 7e32eebf2fda8c0381bb6f3521d4dee0982315e1 Mon Sep 17 00:00:00 2001 From: zhangzherui Date: Mon, 13 Jul 2026 03:09:59 +0800 Subject: [PATCH] add skills code review agent --- examples/skills_code_review_agent/DESIGN.md | 7 + examples/skills_code_review_agent/README.md | 29 ++++ examples/skills_code_review_agent/agent.py | 132 +++++++++++++++ examples/skills_code_review_agent/cli.py | 47 ++++++ .../skills_code_review_agent/diff_parser.py | 64 ++++++++ .../fixtures/async_leak.diff | 8 + .../fixtures/clean.diff | 12 ++ .../fixtures/database.diff | 6 + .../fixtures/duplicate.diff | 6 + .../fixtures/sandbox_failure.diff | 6 + .../fixtures/secret.diff | 6 + .../fixtures/security.diff | 6 + .../fixtures/test_missing.diff | 6 + examples/skills_code_review_agent/models.py | 72 +++++++++ .../skills_code_review_agent/redaction.py | 42 +++++ .../skills_code_review_agent/reporting.py | 57 +++++++ .../review_policy.yaml | 25 +++ .../review_report.json | 73 +++++++++ .../skills_code_review_agent/review_report.md | 47 ++++++ examples/skills_code_review_agent/rules.py | 140 ++++++++++++++++ examples/skills_code_review_agent/sandbox.py | 139 ++++++++++++++++ examples/skills_code_review_agent/schema.sql | 51 ++++++ .../skills/code-review/SKILL.md | 17 ++ .../skills/code-review/agents/openai.yaml | 4 + .../skills/code-review/references/rules.md | 12 ++ .../skills/code-review/scripts/review_diff.py | 16 ++ examples/skills_code_review_agent/storage.py | 122 ++++++++++++++ .../test_code_review_agent.py | 151 ++++++++++++++++++ 28 files changed, 1303 insertions(+) create mode 100644 examples/skills_code_review_agent/DESIGN.md create mode 100644 examples/skills_code_review_agent/README.md create mode 100644 examples/skills_code_review_agent/agent.py create mode 100644 examples/skills_code_review_agent/cli.py create mode 100644 examples/skills_code_review_agent/diff_parser.py create mode 100644 examples/skills_code_review_agent/fixtures/async_leak.diff create mode 100644 examples/skills_code_review_agent/fixtures/clean.diff create mode 100644 examples/skills_code_review_agent/fixtures/database.diff create mode 100644 examples/skills_code_review_agent/fixtures/duplicate.diff create mode 100644 examples/skills_code_review_agent/fixtures/sandbox_failure.diff create mode 100644 examples/skills_code_review_agent/fixtures/secret.diff create mode 100644 examples/skills_code_review_agent/fixtures/security.diff create mode 100644 examples/skills_code_review_agent/fixtures/test_missing.diff create mode 100644 examples/skills_code_review_agent/models.py create mode 100644 examples/skills_code_review_agent/redaction.py create mode 100644 examples/skills_code_review_agent/reporting.py create mode 100644 examples/skills_code_review_agent/review_policy.yaml create mode 100644 examples/skills_code_review_agent/review_report.json create mode 100644 examples/skills_code_review_agent/review_report.md create mode 100644 examples/skills_code_review_agent/rules.py create mode 100644 examples/skills_code_review_agent/sandbox.py create mode 100644 examples/skills_code_review_agent/schema.sql create mode 100644 examples/skills_code_review_agent/skills/code-review/SKILL.md create mode 100644 examples/skills_code_review_agent/skills/code-review/agents/openai.yaml create mode 100644 examples/skills_code_review_agent/skills/code-review/references/rules.md create mode 100644 examples/skills_code_review_agent/skills/code-review/scripts/review_diff.py create mode 100644 examples/skills_code_review_agent/storage.py create mode 100644 examples/skills_code_review_agent/test_code_review_agent.py diff --git a/examples/skills_code_review_agent/DESIGN.md b/examples/skills_code_review_agent/DESIGN.md new file mode 100644 index 00000000..5628d826 --- /dev/null +++ b/examples/skills_code_review_agent/DESIGN.md @@ -0,0 +1,7 @@ +# 方案设计说明 + +code-review Skill 保存稳定工作流，规则与脚本按需加载。Agent 从 diff、路径或 Git 工作区提取新增行，检查代码执行、异步阻塞、资源泄漏、数据库连接、敏感信息和测试缺失。同文件同一行同类仅保留最高置信项，低于阈值的结果进入人工复核。 + +生产默认使用无网络、只读挂载且限制资源的容器；fake mode 用于验收，本地执行只是开发 fallback。Filter 前置检查命令、路径、网络和预算，deny 或 needs_human_review 均不执行。环境采用白名单，输出、finding、报告和入库前统一脱敏；沙箱失败不终止静态评审。 + +SQLite 按 task id 关联 task、sandbox run、filter block、finding 与 report，可通过 ReviewStore 接入其他 SQL。监控记录耗时、工具、拦截、finding、severity 和异常。数据库不保存原始 diff，只保存哈希与脱敏摘要；JSON 供机器消费，Markdown 供审批。静态规则和容器不能发现所有风险，生产仍需最小权限、镜像审计和人工复核。 diff --git a/examples/skills_code_review_agent/README.md b/examples/skills_code_review_agent/README.md new file mode 100644 index 00000000..1f3817e5 --- /dev/null +++ b/examples/skills_code_review_agent/README.md @@ -0,0 +1,29 @@ +# Skills code-review agent + +This prototype composes a reusable `code-review` Skill, deterministic diff rules, a Filter-governed container runner, +SQLite persistence, redaction, monitoring, and JSON/Markdown reports. It accepts unified diffs, a Git workspace, or a +file list. + +Run the complete flow without an API key or Docker: + +```bash +python examples/skills_code_review_agent/cli.py \ + --diff-file examples/skills_code_review_agent/fixtures/security.diff \ + --dry-run +``` + +Outputs are `review_report.json`, `review_report.md`, and `reviews.db`. Query a task with +`SQLiteReviewStore("reviews.db").get_task(task_id)`. The store interface separates orchestration from SQLite so another +SQL backend can implement the same three methods. + +Production policy defaults to `container`, never local execution. The runner uses Docker with no network, a read-only +workspace, memory/CPU/PID limits, a timeout, capped output, and an environment allowlist. `--dry-run` selects the fake +runner for tests. Local mode exists only as an explicit development fallback. Every command is checked before sandbox +entry; `deny` and `needs_human_review` are persisted and never executed. + +The static rules cover security, async errors, file/network resource lifetime, database connection lifetime, secrets, +and missing tests. They only inspect added lines and are intentionally conservative. Confidence below 0.75 is routed to +human-review warnings. Findings are deduplicated by file, line, and category. Static analysis and containers reduce risk +but do not prove correctness or replace least-privilege infrastructure. + +See `DESIGN.md`, `schema.sql`, and `skills/code-review/references/rules.md` for the architecture and extension points. diff --git a/examples/skills_code_review_agent/agent.py b/examples/skills_code_review_agent/agent.py new file mode 100644 index 00000000..9c418576 --- /dev/null +++ b/examples/skills_code_review_agent/agent.py @@ -0,0 +1,132 @@ +"""Automatic code-review orchestration.""" + +from __future__ import annotations + +import hashlib +import time +import uuid +from collections import Counter +from dataclasses import replace +from pathlib import Path + +from diff_parser import DiffParser +from models import Finding +from models import ReviewReport +from redaction import redact_text +from reporting import write_reports +from rules import RuleEngine +from sandbox import SandboxRunner +from storage import SQLiteReviewStore + + +class CodeReviewAgent: + def __init__( + self, + root: str | Path, + database: str | Path, + policy: str | Path, + dry_run: bool = False, + ): + self.root = Path(root) + self.store = SQLiteReviewStore(database) + self.runner = SandboxRunner(policy, self.root, dry_run=dry_run) + self.rules = RuleEngine() + self.confidence_threshold = float(self.runner.policy["confidence_threshold"]) + + def review_diff( + self, + diff: str, + output_dir: str | Path, + commands: list[list[str]] | None = None, + ) -> ReviewReport: + started = time.perf_counter() + task_id = str(uuid.uuid4()) + clean_diff, _ = redact_text(diff) + # Rules inspect the in-memory source so secret detectors retain their + # signal. Only redacted derivatives cross a logging/persistence boundary. + lines = DiffParser.parse(diff) + files = sorted({line.file for line in lines}) + summary = { + "sha256": hashlib.sha256(diff.encode("utf-8")).hexdigest(), + "files": files, + "added_lines": len(lines), + "redacted": clean_diff != diff, + } + self.store.create_task(task_id, summary["sha256"], summary) + exceptions: Counter[str] = Counter() + sandbox_runs = [] + try: + raw_findings = [self._redact_finding(item) for item in self.rules.scan(lines)] + findings = [item for item in raw_findings if item.confidence >= self.confidence_threshold] + warnings = [item for item in raw_findings if item.confidence < self.confidence_threshold] + for index, command in enumerate(commands or [["python", "-m", "compileall", "-q", "."]], 1): + run = self.runner.run(command, index) + sandbox_runs.append(run) + if run.error_type: + exceptions[run.error_type] += 1 + filter_blocks = [ + {"command": run.command, "decision": run.filter_decision, "reason": run.filter_reason} + for run in sandbox_runs if run.status == "blocked" + ] + severity = Counter(item.severity for item in findings) + conclusion = "changes_requested" if any( + item.severity in ("critical", "high") for item in findings + ) else "needs_human_review" if warnings or filter_blocks else "approve" + status = "completed_with_errors" if exceptions else "completed" + monitoring = { + "total_duration_ms": (time.perf_counter() - started) * 1000, + "sandbox_duration_ms": sum(run.duration_ms for run in sandbox_runs), + "tool_call_count": len(sandbox_runs), + "blocked_count": len(filter_blocks), + "finding_count": len(findings), + "warning_count": len(warnings), + "severity_distribution": dict(severity), + "exception_type_distribution": dict(exceptions), + } + report = ReviewReport( + task_id=task_id, + status=status, + conclusion=conclusion, + input_summary=summary, + findings=findings, + warnings=warnings, + filter_blocks=filter_blocks, + sandbox_runs=sandbox_runs, + monitoring=monitoring, + ) + except Exception as exc: + exceptions[type(exc).__name__] += 1 + report = ReviewReport( + task_id=task_id, + status="failed", + conclusion="needs_human_review", + input_summary=summary, + monitoring={ + "total_duration_ms": (time.perf_counter() - started) * 1000, + "sandbox_duration_ms": 0.0, + "tool_call_count": 0, + "blocked_count": 0, + "finding_count": 0, + "warning_count": 0, + "severity_distribution": {}, + "exception_type_distribution": dict(exceptions), + }, + ) + self.store.save_report(report) + write_reports(report, output_dir) + return report + + def review_file(self, diff_file: str | Path, output_dir: str | Path) -> ReviewReport: + diff = Path(diff_file).read_text(encoding="utf-8") + return self.review_diff(diff, output_dir) + + def review_repo(self, repo_path: str | Path, output_dir: str | Path) -> ReviewReport: + diff, _ = DiffParser.from_repo(repo_path) + return self.review_diff(diff, output_dir) + + @staticmethod + def _redact_finding(finding: Finding) -> Finding: + evidence, _ = redact_text(finding.evidence) + title, _ = redact_text(finding.title) + recommendation, _ = redact_text(finding.recommendation) + return replace(finding, evidence=evidence, title=title, recommendation=recommendation) diff --git a/examples/skills_code_review_agent/cli.py b/examples/skills_code_review_agent/cli.py new file mode 100644 index 00000000..f098c725 --- /dev/null +++ b/examples/skills_code_review_agent/cli.py @@ -0,0 +1,47 @@ +"""CLI for diff files, repositories, file lists, and database lookup.""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +from agent import CodeReviewAgent +from diff_parser import DiffParser + + +HERE = Path(__file__).resolve().parent + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + source = parser.add_mutually_exclusive_group() + source.add_argument("--diff-file", type=Path) + source.add_argument("--repo-path", type=Path) + source.add_argument("--files", type=Path, nargs="+") + parser.add_argument("--output-dir", type=Path, default=HERE) + parser.add_argument("--database", type=Path, default=HERE / "reviews.db") + parser.add_argument("--policy", type=Path, default=HERE / "review_policy.yaml") + parser.add_argument("--dry-run", action="store_true", help="Use fake sandbox; no model or Docker required") + args = parser.parse_args() + if not any((args.diff_file, args.repo_path, args.files)): + parser.error("one of --diff-file, --repo-path, or --files is required") + + agent = CodeReviewAgent( + root=args.repo_path or Path.cwd(), + database=args.database, + policy=args.policy, + dry_run=args.dry_run, + ) + if args.diff_file: + report = agent.review_file(args.diff_file, args.output_dir) + elif args.repo_path: + report = agent.review_repo(args.repo_path, args.output_dir) + else: + diff, _ = DiffParser.from_paths(args.files) + report = agent.review_diff(diff, args.output_dir) + print(f"task_id={report.task_id} conclusion={report.conclusion}") + return 0 if report.status.startswith("completed") else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/examples/skills_code_review_agent/diff_parser.py b/examples/skills_code_review_agent/diff_parser.py new file mode 100644 index 00000000..7292201e --- /dev/null +++ b/examples/skills_code_review_agent/diff_parser.py @@ -0,0 +1,64 @@ +"""Unified diff and Git workspace input adapters.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +from models import ChangedLine + + +class DiffParser: + @staticmethod + def from_file(path: str | Path) -> tuple[str, list[ChangedLine]]: + text = Path(path).read_text(encoding="utf-8") + return text, DiffParser.parse(text) + + @staticmethod + def from_repo(path: str | Path) -> tuple[str, list[ChangedLine]]: + result = subprocess.run( + ["git", "diff", "--no-ext-diff", "--unified=3", "HEAD"], + cwd=Path(path), + capture_output=True, + text=True, + check=False, + timeout=10, + ) + if result.returncode: + raise ValueError(f"git diff failed: {result.stderr.strip()}") + return result.stdout, DiffParser.parse(result.stdout) + + @staticmethod + def from_paths(paths: list[str | Path]) -> tuple[str, list[ChangedLine]]: + chunks = [] + for raw_path in paths: + path = Path(raw_path) + lines = path.read_text(encoding="utf-8").splitlines() + chunks.extend([f"diff --git a/{path} b/{path}", f"+++ b/{path}", f"@@ -0,0 +1,{len(lines)} @@"]) + chunks.extend(f"+{line}" for line in lines) + text = "\n".join(chunks) + "\n" + return text, DiffParser.parse(text) + + @staticmethod + def parse(diff: str) -> list[ChangedLine]: + changed: list[ChangedLine] = [] + current_file = "" + new_line = 0 + hunk_header = "" + for raw in diff.splitlines(): + if raw.startswith("+++ "): + current_file = raw[4:].removeprefix("b/") + continue + if raw.startswith("@@"): + hunk_header = raw + marker = raw.split("+", 1)[1].split(" ", 1)[0] + new_line = int(marker.split(",", 1)[0]) + continue + if raw.startswith("+") and not raw.startswith("+++"): + changed.append(ChangedLine(current_file, new_line, raw[1:], hunk_header)) + new_line += 1 + elif raw.startswith("-") and not raw.startswith("---"): + continue + elif hunk_header and not raw.startswith("\\"): + new_line += 1 + return changed diff --git a/examples/skills_code_review_agent/fixtures/async_leak.diff b/examples/skills_code_review_agent/fixtures/async_leak.diff new file mode 100644 index 00000000..50630e44 --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/async_leak.diff @@ -0,0 +1,8 @@ +diff --git a/worker.py b/worker.py +--- a/worker.py ++++ b/worker.py +@@ -1,1 +1,4 @@ ++async def poll(): ++ time.sleep(10) ++ asyncio.create_task(refresh()) ++ return True diff --git a/examples/skills_code_review_agent/fixtures/clean.diff b/examples/skills_code_review_agent/fixtures/clean.diff new file mode 100644 index 00000000..484c84e7 --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/clean.diff @@ -0,0 +1,12 @@ +diff --git a/app.py b/app.py +--- a/app.py ++++ b/app.py +@@ -1,1 +1,2 @@ + def add(a, b): ++ return a + b +diff --git a/tests/test_app.py b/tests/test_app.py +--- a/tests/test_app.py ++++ b/tests/test_app.py +@@ -1,1 +1,2 @@ + def test_add(): ++ assert add(1, 2) == 3 diff --git a/examples/skills_code_review_agent/fixtures/database.diff b/examples/skills_code_review_agent/fixtures/database.diff new file mode 100644 index 00000000..11a4e58a --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/database.diff @@ -0,0 +1,6 @@ +diff --git a/db.py b/db.py +--- a/db.py ++++ b/db.py +@@ -1,1 +1,2 @@ + def load(): ++ connection = sqlite3.connect("data.db") diff --git a/examples/skills_code_review_agent/fixtures/duplicate.diff b/examples/skills_code_review_agent/fixtures/duplicate.diff new file mode 100644 index 00000000..596083fc --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/duplicate.diff @@ -0,0 +1,6 @@ +diff --git a/unsafe.py b/unsafe.py +--- a/unsafe.py ++++ b/unsafe.py +@@ -1,1 +1,2 @@ + def run(value): ++ return eval(value) diff --git a/examples/skills_code_review_agent/fixtures/sandbox_failure.diff b/examples/skills_code_review_agent/fixtures/sandbox_failure.diff new file mode 100644 index 00000000..be76816a --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/sandbox_failure.diff @@ -0,0 +1,6 @@ +diff --git a/app.py b/app.py +--- a/app.py ++++ b/app.py +@@ -1,1 +1,2 @@ + def healthy(): ++ return True diff --git a/examples/skills_code_review_agent/fixtures/secret.diff b/examples/skills_code_review_agent/fixtures/secret.diff new file mode 100644 index 00000000..4a231431 --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/secret.diff @@ -0,0 +1,6 @@ +diff --git a/config.py b/config.py +--- a/config.py ++++ b/config.py +@@ -1,1 +1,2 @@ + def config(): ++ api_key = "sk-super-secret-value" diff --git a/examples/skills_code_review_agent/fixtures/security.diff b/examples/skills_code_review_agent/fixtures/security.diff new file mode 100644 index 00000000..4ca8ad8a --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/security.diff @@ -0,0 +1,6 @@ +diff --git a/app.py b/app.py +--- a/app.py ++++ b/app.py +@@ -1,1 +1,2 @@ + def run(user_input): ++ return eval(user_input) diff --git a/examples/skills_code_review_agent/fixtures/test_missing.diff b/examples/skills_code_review_agent/fixtures/test_missing.diff new file mode 100644 index 00000000..1072a480 --- /dev/null +++ b/examples/skills_code_review_agent/fixtures/test_missing.diff @@ -0,0 +1,6 @@ +diff --git a/service.py b/service.py +--- a/service.py ++++ b/service.py +@@ -1,1 +1,2 @@ + def calculate(value): ++ return value * 2 diff --git a/examples/skills_code_review_agent/models.py b/examples/skills_code_review_agent/models.py new file mode 100644 index 00000000..fcfec68c --- /dev/null +++ b/examples/skills_code_review_agent/models.py @@ -0,0 +1,72 @@ +"""Structured models used across review, persistence, and reporting.""" + +from __future__ import annotations + +from dataclasses import asdict +from dataclasses import dataclass +from dataclasses import field +from typing import Any + + +@dataclass(frozen=True) +class ChangedLine: + file: str + line: int + text: str + hunk: str + + +@dataclass(frozen=True) +class Finding: + severity: str + category: str + file: str + line: int + title: str + evidence: str + recommendation: str + confidence: float + source: str + rule_id: str + + def to_dict(self) -> dict[str, Any]: + return asdict(self) + + +@dataclass +class SandboxResult: + command: list[str] + status: str + exit_code: int | None + duration_ms: float + output: str + error_type: str | None = None + filter_decision: str = "allow" + filter_reason: str | None = None + redacted: bool = False + + +@dataclass +class ReviewReport: + task_id: str + status: str + conclusion: str + input_summary: dict[str, Any] + findings: list[Finding] = field(default_factory=list) + warnings: list[Finding] = field(default_factory=list) + filter_blocks: list[dict[str, Any]] = field(default_factory=list) + sandbox_runs: list[SandboxResult] = field(default_factory=list) + monitoring: dict[str, Any] = field(default_factory=dict) + + def to_dict(self) -> dict[str, Any]: + return { + "task_id": self.task_id, + "status": self.status, + "conclusion": self.conclusion, + "input_summary": self.input_summary, + "findings": [item.to_dict() for item in self.findings], + "warnings": [item.to_dict() for item in self.warnings], + "filter_blocks": self.filter_blocks, + "sandbox_runs": [asdict(item) for item in self.sandbox_runs], + "monitoring": self.monitoring, + } diff --git a/examples/skills_code_review_agent/redaction.py b/examples/skills_code_review_agent/redaction.py new file mode 100644 index 00000000..0f587a2c --- /dev/null +++ b/examples/skills_code_review_agent/redaction.py @@ -0,0 +1,42 @@ +"""Credential redaction applied before logs, reports, and database writes.""" + +from __future__ import annotations + +import re +from typing import Any + + +SECRET_PATTERNS = ( + re.compile(r"(?i)((?:api[_-]?key|access[_-]?token|password|secret)\s*[:=]\s*)['\"]?[^\s,'\"]+"), + re.compile(r"\b(?:sk-[A-Za-z0-9_-]{8,}|gh[pousr]_[A-Za-z0-9]{8,}|xox[baprs]-[A-Za-z0-9-]{8,})\b"), + re.compile(r"\bAKIA[0-9A-Z]{16}\b"), + re.compile(r"\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b"), + re.compile(r"-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----"), +) + + +def redact_text(value: str) -> tuple[str, bool]: + redacted = value + for pattern in SECRET_PATTERNS: + redacted = pattern.sub(lambda match: (match.group(1) if match.lastindex else "") + "[REDACTED]", redacted) + return redacted, redacted != value + + +def redact_value(value: Any) -> tuple[Any, bool]: + if isinstance(value, str): + return redact_text(value) + if isinstance(value, list): + output, changed = [], False + for item in value: + clean, item_changed = redact_value(item) + output.append(clean) + changed = changed or item_changed + return output, changed + if isinstance(value, dict): + output, changed = {}, False + for key, item in value.items(): + clean, item_changed = redact_value(item) + output[key] = clean + changed = changed or item_changed + return output, changed + return value, False diff --git a/examples/skills_code_review_agent/reporting.py b/examples/skills_code_review_agent/reporting.py new file mode 100644 index 00000000..1fb1a1f6 --- /dev/null +++ b/examples/skills_code_review_agent/reporting.py @@ -0,0 +1,57 @@ +"""Human and machine report writers.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from models import ReviewReport + + +def write_reports(report: ReviewReport, output_dir: str | Path) -> None: + output = Path(output_dir) + output.mkdir(parents=True, exist_ok=True) + (output / "review_report.json").write_text( + json.dumps(report.to_dict(), ensure_ascii=False, indent=2) + "\n", encoding="utf-8" + ) + (output / "review_report.md").write_text(render_markdown(report), encoding="utf-8") + + +def render_markdown(report: ReviewReport) -> str: + severity = report.monitoring.get("severity_distribution", {}) + lines = [ + "# Code Review Report", "", f"**Task:** `{report.task_id}`", + f"**Conclusion:** `{report.conclusion}`", "", + "## Summary", "", + f"- Findings: {len(report.findings)}", + f"- Human-review warnings: {len(report.warnings)}", + f"- Filter blocks: {len(report.filter_blocks)}", + f"- Sandbox runs: {len(report.sandbox_runs)}", + f"- Severity distribution: `{json.dumps(severity, sort_keys=True)}`", "", + "## Findings", "", + ] + if not report.findings: + lines.append("No high-confidence findings.") + for finding in report.findings: + lines.extend([ + f"### [{finding.severity.upper()}] {finding.title}", "", + f"`{finding.file}:{finding.line}` · `{finding.category}` · confidence {finding.confidence:.2f}", "", + f"Evidence: `{finding.evidence}`", "", + f"Recommendation: {finding.recommendation}", "", + ]) + lines.extend(["## Needs human review", ""]) + if not report.warnings: + lines.append("None.") + for warning in report.warnings: + lines.append(f"- `{warning.file}:{warning.line}` {warning.title}: {warning.recommendation}") + lines.extend(["", "## Filter and sandbox", ""]) + if not report.sandbox_runs: + lines.append("No sandbox checks requested.") + for run in report.sandbox_runs: + lines.append( + f"- `{run.status}` `{run.command}` in {run.duration_ms:.2f} ms" + + (f" — {run.filter_reason}" if run.filter_reason else "") + ) + lines.extend(["", "## Monitoring", "", "```json", + json.dumps(report.monitoring, ensure_ascii=False, indent=2), "```", ""]) + return "\n".join(lines) diff --git a/examples/skills_code_review_agent/review_policy.yaml b/examples/skills_code_review_agent/review_policy.yaml new file mode 100644 index 00000000..7944a59c --- /dev/null +++ b/examples/skills_code_review_agent/review_policy.yaml @@ -0,0 +1,25 @@ +sandbox: + mode: container + image: python:3.12-slim + timeout_seconds: 30 + max_output_bytes: 200000 + memory: 512m + cpus: 1.0 + environment_allowlist: + - LANG + - LC_ALL + - PYTHONPATH +filter: + allowed_commands: + - python + - pytest + - ruff + - mypy + - fake-fail + forbidden_paths: + - ~/.ssh + - .env + - /etc/shadow + allowed_domains: [] + max_tool_calls: 5 +confidence_threshold: 0.75 diff --git a/examples/skills_code_review_agent/review_report.json b/examples/skills_code_review_agent/review_report.json new file mode 100644 index 00000000..a18a4ac6 --- /dev/null +++ b/examples/skills_code_review_agent/review_report.json @@ -0,0 +1,73 @@ +{ + "task_id": "9d309279-1d77-4dc4-b9b1-f4cf833dd749", + "status": "completed", + "conclusion": "changes_requested", + "input_summary": { + "sha256": "bfa592fb25cedc20328294dcea2038af7b17e011cae83dee70c3ae010aeabdd0", + "files": [ + "app.py" + ], + "added_lines": 1, + "redacted": false + }, + "findings": [ + { + "severity": "critical", + "category": "security", + "file": "app.py", + "line": 2, + "title": "Unsafe code or command execution", + "evidence": "return eval(user_input)", + "recommendation": "Remove dynamic execution or use a fixed argv allowlist in an isolated process.", + "confidence": 0.96, + "source": "static_rule:SEC001", + "rule_id": "SEC001" + } + ], + "warnings": [ + { + "severity": "low", + "category": "test_missing", + "file": "app.py", + "line": 2, + "title": "Production change has no corresponding test change", + "evidence": "changed production file: app.py", + "recommendation": "Add focused positive, negative, and regression tests for the changed behavior.", + "confidence": 0.65, + "source": "heuristic:TEST001", + "rule_id": "TEST001" + } + ], + "filter_blocks": [], + "sandbox_runs": [ + { + "command": [ + "python", + "-m", + "compileall", + "-q", + "." + ], + "status": "passed", + "exit_code": 0, + "duration_ms": 0.006499999926745659, + "output": "fake sandbox check passed", + "error_type": null, + "filter_decision": "allow", + "filter_reason": null, + "redacted": false + } + ], + "monitoring": { + "total_duration_ms": 3.795400000171867, + "sandbox_duration_ms": 0.006499999926745659, + "tool_call_count": 1, + "blocked_count": 0, + "finding_count": 1, + "warning_count": 1, + "severity_distribution": { + "critical": 1 + }, + "exception_type_distribution": {} + } +} diff --git a/examples/skills_code_review_agent/review_report.md b/examples/skills_code_review_agent/review_report.md new file mode 100644 index 00000000..2701a910 --- /dev/null +++ b/examples/skills_code_review_agent/review_report.md @@ -0,0 +1,47 @@ +# Code Review Report + +**Task:** `9d309279-1d77-4dc4-b9b1-f4cf833dd749` +**Conclusion:** `changes_requested` + +## Summary + +- Findings: 1 +- Human-review warnings: 1 +- Filter blocks: 0 +- Sandbox runs: 1 +- Severity distribution: `{"critical": 1}` + +## Findings + +### [CRITICAL] Unsafe code or command execution + +`app.py:2` · `security` · confidence 0.96 + +Evidence: `return eval(user_input)` + +Recommendation: Remove dynamic execution or use a fixed argv allowlist in an isolated process. + +## Needs human review + +- `app.py:2` Production change has no corresponding test change: Add focused positive, negative, and regression tests for the changed behavior. + +## Filter and sandbox + +- `passed` `['python', '-m', 'compileall', '-q', '.']` in 0.01 ms + +## Monitoring + +```json +{ + "total_duration_ms": 3.795400000171867, + "sandbox_duration_ms": 0.006499999926745659, + "tool_call_count": 1, + "blocked_count": 0, + "finding_count": 1, + "warning_count": 1, + "severity_distribution": { + "critical": 1 + }, + "exception_type_distribution": {} +} +``` diff --git a/examples/skills_code_review_agent/rules.py b/examples/skills_code_review_agent/rules.py new file mode 100644 index 00000000..40cafbfb --- /dev/null +++ b/examples/skills_code_review_agent/rules.py @@ -0,0 +1,140 @@ +"""Deterministic high-signal review rules.""" + +from __future__ import annotations + +import re +from collections.abc import Iterable + +from models import ChangedLine +from models import Finding + + +class RuleEngine: + """Scan added lines and return deduplicated findings.""" + + RULES = ( + ( + "SEC001", + "security", + "critical", + re.compile(r"\b(eval|exec)\s*\(|pickle\.loads\s*\(|os\.system\s*\(|shell\s*=\s*True"), + "Unsafe code or command execution", + "Remove dynamic execution or use a fixed argv allowlist in an isolated process.", + 0.96, + ), + ( + "RES001", + "resource_leak", + "high", + re.compile(r"(? list[Finding]: + lines = list(lines) + findings = [] + async_context: dict[str, bool] = {} + for line in lines: + stripped = line.text.strip() + if re.match(r"async\s+def\s+", stripped): + async_context[line.file] = True + if async_context.get(line.file) and "time.sleep(" in line.text: + findings.append(self._finding( + line, "ASYNC001", "async_error", "high", + "Blocking sleep inside async code", + "Replace time.sleep with await asyncio.sleep.", 0.94, + )) + if "asyncio.create_task(" in line.text and "=" not in line.text: + findings.append(self._finding( + line, "ASYNC001", "async_error", "medium", + "Created task has no owner", + "Retain and await or cancel the task during shutdown.", 0.82, + )) + for rule_id, category, severity, pattern, title, recommendation, confidence in self.RULES: + if pattern.search(line.text): + findings.append(self._finding( + line, rule_id, category, severity, title, recommendation, confidence, + )) + findings.extend(self._test_coverage_warning(lines)) + return self.deduplicate(findings) + + @staticmethod + def _finding( + line: ChangedLine, + rule_id: str, + category: str, + severity: str, + title: str, + recommendation: str, + confidence: float, + ) -> Finding: + return Finding( + severity=severity, + category=category, + file=line.file, + line=line.line, + title=title, + evidence=line.text.strip()[:200], + recommendation=recommendation, + confidence=confidence, + source=f"static_rule:{rule_id}", + rule_id=rule_id, + ) + + @staticmethod + def _test_coverage_warning(lines: list[ChangedLine]) -> list[Finding]: + files = {line.file for line in lines} + production = sorted( + file for file in files + if file.endswith(".py") and not file.startswith("tests/") and "/test_" not in file + ) + tests_changed = any(file.startswith("tests/") or "/test_" in file for file in files) + if not production or tests_changed: + return [] + line = next(item for item in lines if item.file == production[0]) + return [Finding( + severity="low", + category="test_missing", + file=line.file, + line=line.line, + title="Production change has no corresponding test change", + evidence=f"changed production file: {line.file}", + recommendation="Add focused positive, negative, and regression tests for the changed behavior.", + confidence=0.65, + source="heuristic:TEST001", + rule_id="TEST001", + )] + + @staticmethod + def deduplicate(findings: Iterable[Finding]) -> list[Finding]: + unique: dict[tuple[str, int, str], Finding] = {} + for finding in findings: + key = (finding.file, finding.line, finding.category) + previous = unique.get(key) + if previous is None or finding.confidence > previous.confidence: + unique[key] = finding + return sorted(unique.values(), key=lambda item: (item.file, item.line, item.category)) diff --git a/examples/skills_code_review_agent/sandbox.py b/examples/skills_code_review_agent/sandbox.py new file mode 100644 index 00000000..b13744bd --- /dev/null +++ b/examples/skills_code_review_agent/sandbox.py @@ -0,0 +1,139 @@ +"""Filter-governed fake, container, and local sandbox runners.""" + +from __future__ import annotations + +import os +import subprocess +import time +from dataclasses import dataclass +from pathlib import Path +from urllib.parse import urlparse + +import yaml + +from models import SandboxResult +from redaction import redact_text + + +@dataclass(frozen=True) +class FilterDecision: + decision: str + reason: str | None = None + + +class ReviewExecutionFilter: + def __init__(self, config: dict): + self.config = config + + def decide(self, command: list[str], timeout: float, call_index: int) -> FilterDecision: + joined = " ".join(command) + if call_index > self.config["max_tool_calls"]: + return FilterDecision("deny", "tool call budget exceeded") + if timeout <= 0: + return FilterDecision("deny", "timeout must be positive") + if any(path in joined for path in self.config["forbidden_paths"]): + return FilterDecision("deny", "command references a forbidden path") + if command and command[0] not in self.config["allowed_commands"]: + return FilterDecision("needs_human_review", "command is not on the allowlist") + for token in command: + if token.startswith(("http://", "https://")): + host = (urlparse(token).hostname or "").lower() + if host not in self.config["allowed_domains"]: + return FilterDecision("deny", "network destination is not allowlisted") + if any(token in joined for token in ("rm -rf", "sudo ", "| sh", ";")): + return FilterDecision("deny", "high-risk shell pattern") + return FilterDecision("allow") + + +class SandboxRunner: + """Run approved checks with timeout, output cap, and environment allowlist.""" + + def __init__(self, policy_path: str | Path, workspace: str | Path, dry_run: bool = False): + self.policy = yaml.safe_load(Path(policy_path).read_text(encoding="utf-8")) + self.sandbox = self.policy["sandbox"] + self.filter = ReviewExecutionFilter(self.policy["filter"]) + self.workspace = Path(workspace).resolve() + self.mode = "fake" if dry_run else self.sandbox["mode"] + + def run(self, command: list[str], call_index: int) -> SandboxResult: + timeout = float(self.sandbox["timeout_seconds"]) + decision = self.filter.decide(command, timeout, call_index) + safe_command = [redact_text(token)[0] for token in command] + if decision.decision != "allow": + return SandboxResult( + command=safe_command, + status="blocked", + exit_code=None, + duration_ms=0.0, + output="", + filter_decision=decision.decision, + filter_reason=decision.reason, + ) + started = time.perf_counter() + try: + if self.mode == "fake": + if command and command[0] == "fake-fail": + raise subprocess.CalledProcessError(2, command, output="fixture failure") + output, exit_code = "fake sandbox check passed", 0 + else: + process_command = self._container_command(command) if self.mode == "container" else command + completed = subprocess.run( + process_command, + cwd=None if self.mode == "container" else self.workspace, + capture_output=True, + text=True, + timeout=timeout, + check=False, + env=self._allowed_environment(), + ) + output = completed.stdout + completed.stderr + exit_code = completed.returncode + output, redacted = redact_text(output[: self.sandbox["max_output_bytes"]]) + truncated = len(output.encode("utf-8")) >= self.sandbox["max_output_bytes"] + return SandboxResult( + command=safe_command, + status="passed" if exit_code == 0 else "failed", + exit_code=exit_code, + duration_ms=(time.perf_counter() - started) * 1000, + output=output, + error_type="output_truncated" if truncated else None, + redacted=redacted, + ) + except subprocess.TimeoutExpired: + return self._failure(safe_command, started, "timeout", "sandbox execution timed out") + except subprocess.CalledProcessError as exc: + output, redacted = redact_text(str(exc.output or exc)) + result = self._failure(safe_command, started, "process_error", output) + result.redacted = redacted + return result + except Exception as exc: # sandbox infrastructure failure must not abort review + output, redacted = redact_text(str(exc)) + result = self._failure(safe_command, started, type(exc).__name__, output) + result.redacted = redacted + return result + + def _container_command(self, command: list[str]) -> list[str]: + return [ + "docker", "run", "--rm", "--network", "none", "--read-only", + "--memory", str(self.sandbox["memory"]), "--cpus", str(self.sandbox["cpus"]), + "--pids-limit", "128", "-v", f"{self.workspace}:/workspace:ro", "-w", "/workspace", + str(self.sandbox["image"]), *command, + ] + + def _allowed_environment(self) -> dict[str, str]: + return { + key: os.environ[key] + for key in self.sandbox["environment_allowlist"] + if key in os.environ + } + + @staticmethod + def _failure(command: list[str], started: float, error_type: str, output: str) -> SandboxResult: + return SandboxResult( + command=command, + status="failed", + exit_code=None, + duration_ms=(time.perf_counter() - started) * 1000, + output=output, + error_type=error_type, + ) diff --git a/examples/skills_code_review_agent/schema.sql b/examples/skills_code_review_agent/schema.sql new file mode 100644 index 00000000..e6d36ae7 --- /dev/null +++ b/examples/skills_code_review_agent/schema.sql @@ -0,0 +1,51 @@ +PRAGMA foreign_keys = ON; + +CREATE TABLE review_tasks ( + task_id TEXT PRIMARY KEY, + status TEXT NOT NULL, + input_sha256 TEXT NOT NULL, + input_summary_json TEXT NOT NULL, + created_at TEXT DEFAULT CURRENT_TIMESTAMP +); + +CREATE TABLE sandbox_runs ( + id INTEGER PRIMARY KEY, + task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + command_json TEXT NOT NULL, + status TEXT NOT NULL, + exit_code INTEGER, + duration_ms REAL NOT NULL, + output_summary TEXT NOT NULL, + error_type TEXT +); + +CREATE TABLE filter_blocks ( + id INTEGER PRIMARY KEY, + task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + decision TEXT NOT NULL, + reason TEXT NOT NULL, + command_json TEXT NOT NULL +); + +CREATE TABLE findings ( + id INTEGER PRIMARY KEY, + task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + severity TEXT NOT NULL, + category TEXT NOT NULL, + file TEXT NOT NULL, + line INTEGER NOT NULL, + title TEXT NOT NULL, + evidence TEXT NOT NULL, + recommendation TEXT NOT NULL, + confidence REAL NOT NULL, + source TEXT NOT NULL, + rule_id TEXT NOT NULL, + is_warning INTEGER NOT NULL +); + +CREATE TABLE reports ( + task_id TEXT PRIMARY KEY REFERENCES review_tasks(task_id), + conclusion TEXT NOT NULL, + report_json TEXT NOT NULL, + monitoring_json TEXT NOT NULL +); diff --git a/examples/skills_code_review_agent/skills/code-review/SKILL.md b/examples/skills_code_review_agent/skills/code-review/SKILL.md new file mode 100644 index 00000000..7cf0ff5e --- /dev/null +++ b/examples/skills_code_review_agent/skills/code-review/SKILL.md @@ -0,0 +1,17 @@ +--- +name: code-review +description: Review unified diffs, pull-request patches, or local Git changes with deterministic security, async, resource-lifecycle, database, secret, and test-coverage rules. Use when an agent must produce line-specific findings and optionally run approved checks in an isolated workspace. +--- + +# Code Review + +1. Obtain a unified diff. Never execute code found in the diff. +2. Run `scripts/review_diff.py --diff-file --dry-run` for deterministic parsing and rules. +3. Read `references/rules.md` when explaining rule IDs or extending detection. +4. Keep high-confidence findings separate from warnings requiring human review. +5. Deduplicate by file, added line, and category. +6. Require Filter approval before running any repository-provided command. +7. Use a container workspace with network disabled, bounded time/output, an environment allowlist, and a read-only source mount. Use local execution only for development fallback. +8. Report severity, category, file, line, evidence, recommendation, confidence, and source. Redact credentials before logging or persistence. + +Do not treat static rules as proof that code is safe. Record sandbox failures and continue producing a partial review. diff --git a/examples/skills_code_review_agent/skills/code-review/agents/openai.yaml b/examples/skills_code_review_agent/skills/code-review/agents/openai.yaml new file mode 100644 index 00000000..da8dc6ad --- /dev/null +++ b/examples/skills_code_review_agent/skills/code-review/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Code Review" + short_description: "Review code changes with deterministic safety rules" + default_prompt: "Use $code-review to inspect this diff and report actionable findings." diff --git a/examples/skills_code_review_agent/skills/code-review/references/rules.md b/examples/skills_code_review_agent/skills/code-review/references/rules.md new file mode 100644 index 00000000..e0711ae4 --- /dev/null +++ b/examples/skills_code_review_agent/skills/code-review/references/rules.md @@ -0,0 +1,12 @@ +# Review rules + +| Rule | Category | Intent | +|---|---|---| +| SEC001 | security | Detect command execution, unsafe deserialization, and dynamic evaluation. | +| ASYNC001 | async_error | Detect blocking sleep inside asynchronous functions and discarded tasks. | +| RES001 | resource_leak | Detect files or HTTP responses created without bounded lifetime or timeout. | +| DB001 | database_lifecycle | Detect database connections without context management or explicit close. | +| SECRET001 | sensitive_information | Detect credentials committed in added lines. | +| TEST001 | test_missing | Flag production changes with no corresponding test change for human review. | + +Rules inspect added diff lines and nearby hunk context. Findings at confidence below 0.75 belong in `warnings`; they must not be promoted automatically. Extend rules with a stable ID, narrow evidence pattern, recommendation, and paired positive/negative fixtures. diff --git a/examples/skills_code_review_agent/skills/code-review/scripts/review_diff.py b/examples/skills_code_review_agent/skills/code-review/scripts/review_diff.py new file mode 100644 index 00000000..9d8e8a1d --- /dev/null +++ b/examples/skills_code_review_agent/skills/code-review/scripts/review_diff.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +"""Run the bundled review agent from a source checkout.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +EXAMPLE_ROOT = Path(__file__).resolve().parents[3] +sys.path.insert(0, str(EXAMPLE_ROOT)) + +from cli import main # noqa: E402 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/examples/skills_code_review_agent/storage.py b/examples/skills_code_review_agent/storage.py new file mode 100644 index 00000000..cdf82efe --- /dev/null +++ b/examples/skills_code_review_agent/storage.py @@ -0,0 +1,122 @@ +"""SQLite persistence with a backend-neutral review-store interface.""" + +from __future__ import annotations + +import json +import sqlite3 +from pathlib import Path +from typing import Any + +from models import ReviewReport + + +SCHEMA = """ +PRAGMA foreign_keys = ON; +CREATE TABLE IF NOT EXISTS review_tasks ( + task_id TEXT PRIMARY KEY, status TEXT NOT NULL, input_sha256 TEXT NOT NULL, + input_summary_json TEXT NOT NULL, created_at TEXT DEFAULT CURRENT_TIMESTAMP +); +CREATE TABLE IF NOT EXISTS sandbox_runs ( + id INTEGER PRIMARY KEY, task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + command_json TEXT NOT NULL, status TEXT NOT NULL, exit_code INTEGER, + duration_ms REAL NOT NULL, output_summary TEXT NOT NULL, error_type TEXT +); +CREATE TABLE IF NOT EXISTS filter_blocks ( + id INTEGER PRIMARY KEY, task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + decision TEXT NOT NULL, reason TEXT NOT NULL, command_json TEXT NOT NULL +); +CREATE TABLE IF NOT EXISTS findings ( + id INTEGER PRIMARY KEY, task_id TEXT NOT NULL REFERENCES review_tasks(task_id), + severity TEXT NOT NULL, category TEXT NOT NULL, file TEXT NOT NULL, line INTEGER NOT NULL, + title TEXT NOT NULL, evidence TEXT NOT NULL, recommendation TEXT NOT NULL, + confidence REAL NOT NULL, source TEXT NOT NULL, rule_id TEXT NOT NULL, is_warning INTEGER NOT NULL +); +CREATE TABLE IF NOT EXISTS reports ( + task_id TEXT PRIMARY KEY REFERENCES review_tasks(task_id), conclusion TEXT NOT NULL, + report_json TEXT NOT NULL, monitoring_json TEXT NOT NULL +); +""" + + +class ReviewStore: + def create_task(self, task_id: str, input_sha256: str, summary: dict[str, Any]) -> None: + raise NotImplementedError + + def save_report(self, report: ReviewReport) -> None: + raise NotImplementedError + + def get_task(self, task_id: str) -> dict[str, Any]: + raise NotImplementedError + + +class SQLiteReviewStore(ReviewStore): + def __init__(self, path: str | Path): + self.path = Path(path) + self.path.parent.mkdir(parents=True, exist_ok=True) + with self._connect() as connection: + connection.executescript(SCHEMA) + + def _connect(self) -> sqlite3.Connection: + connection = sqlite3.connect(self.path) + connection.row_factory = sqlite3.Row + return connection + + def create_task(self, task_id: str, input_sha256: str, summary: dict[str, Any]) -> None: + with self._connect() as connection: + connection.execute( + "INSERT INTO review_tasks(task_id,status,input_sha256,input_summary_json) VALUES(?,?,?,?)", + (task_id, "running", input_sha256, json.dumps(summary, ensure_ascii=False)), + ) + + def save_report(self, report: ReviewReport) -> None: + payload = report.to_dict() + with self._connect() as connection: + for run in report.sandbox_runs: + connection.execute( + "INSERT INTO sandbox_runs(task_id,command_json,status,exit_code,duration_ms," + "output_summary,error_type) VALUES(?,?,?,?,?,?,?)", + (report.task_id, json.dumps(run.command), run.status, run.exit_code, + run.duration_ms, run.output[:1000], run.error_type), + ) + if run.status == "blocked": + connection.execute( + "INSERT INTO filter_blocks(task_id,decision,reason,command_json) VALUES(?,?,?,?)", + (report.task_id, run.filter_decision, run.filter_reason or "", json.dumps(run.command)), + ) + for finding, warning in [*( (item, 0) for item in report.findings), + *( (item, 1) for item in report.warnings)]: + connection.execute( + "INSERT INTO findings(task_id,severity,category,file,line,title,evidence,recommendation," + "confidence,source,rule_id,is_warning) VALUES(?,?,?,?,?,?,?,?,?,?,?,?)", + (report.task_id, finding.severity, finding.category, finding.file, finding.line, + finding.title, finding.evidence, finding.recommendation, finding.confidence, + finding.source, finding.rule_id, warning), + ) + connection.execute( + "INSERT INTO reports(task_id,conclusion,report_json,monitoring_json) VALUES(?,?,?,?)", + (report.task_id, report.conclusion, json.dumps(payload, ensure_ascii=False), + json.dumps(report.monitoring, ensure_ascii=False)), + ) + connection.execute( + "UPDATE review_tasks SET status=? WHERE task_id=?", (report.status, report.task_id) + ) + + def get_task(self, task_id: str) -> dict[str, Any]: + with self._connect() as connection: + task = connection.execute( + "SELECT * FROM review_tasks WHERE task_id=?", (task_id,) + ).fetchone() + if task is None: + raise KeyError(task_id) + output = dict(task) + for table in ("sandbox_runs", "filter_blocks", "findings"): + output[table] = [ + dict(row) for row in connection.execute( + f"SELECT * FROM {table} WHERE task_id=? ORDER BY id", (task_id,) + ) + ] + report = connection.execute( + "SELECT * FROM reports WHERE task_id=?", (task_id,) + ).fetchone() + output["report"] = dict(report) if report else None + return output diff --git a/examples/skills_code_review_agent/test_code_review_agent.py b/examples/skills_code_review_agent/test_code_review_agent.py new file mode 100644 index 00000000..1f738842 --- /dev/null +++ b/examples/skills_code_review_agent/test_code_review_agent.py @@ -0,0 +1,151 @@ +"""Acceptance coverage for the eight public review scenarios.""" + +from __future__ import annotations + +import json +import subprocess +import time +from pathlib import Path + +import pytest + +from agent import CodeReviewAgent +from diff_parser import DiffParser +from rules import RuleEngine +from sandbox import SandboxRunner + + +HERE = Path(__file__).resolve().parent +FIXTURES = HERE / "fixtures" +POLICY = HERE / "review_policy.yaml" + + +def make_agent(tmp_path) -> CodeReviewAgent: + return CodeReviewAgent(tmp_path, tmp_path / "reviews.db", POLICY, dry_run=True) + + +def review_fixture(tmp_path, name: str, commands=None): + agent = make_agent(tmp_path) + diff = (FIXTURES / name).read_text(encoding="utf-8") + report = agent.review_diff(diff, tmp_path / "output", commands=commands) + return agent, report + + +def test_clean_diff_has_no_high_confidence_findings(tmp_path): + _, report = review_fixture(tmp_path, "clean.diff") + assert report.conclusion == "approve" + assert report.findings == [] + + +def test_security_diff_is_detected(tmp_path): + _, report = review_fixture(tmp_path, "security.diff") + assert any(item.rule_id == "SEC001" and item.severity == "critical" for item in report.findings) + assert report.conclusion == "changes_requested" + + +def test_async_resource_issues_are_detected(tmp_path): + _, report = review_fixture(tmp_path, "async_leak.diff") + async_findings = [item for item in report.findings if item.category == "async_error"] + assert len(async_findings) == 2 + assert all(item.recommendation for item in async_findings) + + +def test_database_connection_lifecycle_is_detected(tmp_path): + _, report = review_fixture(tmp_path, "database.diff") + assert any(item.rule_id == "DB001" for item in report.findings) + + +def test_missing_tests_are_routed_to_human_review(tmp_path): + _, report = review_fixture(tmp_path, "test_missing.diff") + assert report.findings == [] + assert any(item.rule_id == "TEST001" for item in report.warnings) + assert report.conclusion == "needs_human_review" + + +def test_duplicate_findings_are_suppressed(): + _, lines = DiffParser.from_file(FIXTURES / "duplicate.diff") + findings = RuleEngine().scan([*lines, *lines]) + assert len([item for item in findings if item.rule_id == "SEC001"]) == 1 + + +def test_sandbox_failure_is_persisted_without_crashing_review(tmp_path): + agent, report = review_fixture(tmp_path, "sandbox_failure.diff", [["fake-fail"]]) + assert report.status == "completed_with_errors" + assert report.sandbox_runs[0].status == "failed" + stored = agent.store.get_task(report.task_id) + assert stored["sandbox_runs"][0]["error_type"] == "process_error" + assert stored["report"] is not None + + +def test_secret_is_detected_and_never_persisted_in_plaintext(tmp_path): + agent, report = review_fixture(tmp_path, "secret.diff") + assert any(item.rule_id == "SECRET001" for item in report.findings) + report_text = (tmp_path / "output" / "review_report.json").read_text(encoding="utf-8") + database_bytes = (tmp_path / "reviews.db").read_bytes() + assert "sk-super-secret-value" not in report_text + assert b"sk-super-secret-value" not in database_bytes + assert "[REDACTED]" in report_text + assert agent.store.get_task(report.task_id)["findings"] + + +def test_filter_blocks_unapproved_network_before_sandbox(tmp_path): + agent, report = review_fixture( + tmp_path, "clean.diff", [["curl", "https://untrusted.example/data"]] + ) + assert report.sandbox_runs[0].status == "blocked" + assert report.sandbox_runs[0].filter_decision in ("deny", "needs_human_review") + assert report.monitoring["blocked_count"] == 1 + assert agent.store.get_task(report.task_id)["filter_blocks"] + + +def test_container_timeout_becomes_failed_run(monkeypatch, tmp_path): + runner = SandboxRunner(POLICY, tmp_path, dry_run=False) + + def timeout(*args, **kwargs): + raise subprocess.TimeoutExpired(args[0], timeout=1) + + monkeypatch.setattr(subprocess, "run", timeout) + result = runner.run(["python", "check.py"], 1) + assert result.status == "failed" + assert result.error_type == "timeout" + + +def test_sandbox_output_is_capped_and_redacted(monkeypatch, tmp_path): + runner = SandboxRunner(POLICY, tmp_path, dry_run=False) + runner.sandbox["max_output_bytes"] = 80 + secret_output = "api_key=sk-super-secret-value " + "x" * 200 + + def completed(*args, **kwargs): + return subprocess.CompletedProcess(args[0], 0, stdout=secret_output, stderr="") + + monkeypatch.setattr(subprocess, "run", completed) + result = runner.run(["python", "check.py"], 1) + assert len(result.output.encode("utf-8")) <= 80 + assert "sk-super-secret-value" not in result.output + assert result.redacted is True + + +def test_command_arguments_are_redacted_before_audit(tmp_path): + runner = SandboxRunner(POLICY, tmp_path, dry_run=True) + result = runner.run(["python", "check.py", "api_key=sk-super-secret-value"], 1) + assert "sk-super-secret-value" not in " ".join(result.command) + assert "[REDACTED]" in " ".join(result.command) + + +def test_all_eight_fixtures_parse_and_fake_flow_is_under_two_minutes(tmp_path): + started = time.perf_counter() + names = sorted(path.name for path in FIXTURES.glob("*.diff")) + assert len(names) == 8 + for index, name in enumerate(names): + review_fixture(tmp_path / str(index), name) + assert time.perf_counter() - started < 120 + + +def test_machine_report_contains_required_sections(tmp_path): + _, report = review_fixture(tmp_path, "security.diff") + payload = json.loads((tmp_path / "output" / "review_report.json").read_text(encoding="utf-8")) + assert {"findings", "warnings", "filter_blocks", "sandbox_runs", "monitoring"} <= payload.keys() + assert { + "severity", "category", "file", "line", "title", "evidence", + "recommendation", "confidence", "source", + } <= payload["findings"][0].keys()