Skip to content

Commit 7e32eeb

Browse files
committed
add skills code review agent
1 parent e113610 commit 7e32eeb

28 files changed

Lines changed: 1303 additions & 0 deletions
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# 方案设计说明
2+
3+
code-review Skill 保存稳定工作流,规则与脚本按需加载。Agent 从 diff、路径或 Git 工作区提取新增行,检查代码执行、异步阻塞、资源泄漏、数据库连接、敏感信息和测试缺失。同文件同一行同类仅保留最高置信项,低于阈值的结果进入人工复核。
4+
5+
生产默认使用无网络、只读挂载且限制资源的容器;fake mode 用于验收,本地执行只是开发 fallback。Filter 前置检查命令、路径、网络和预算,deny 或 needs_human_review 均不执行。环境采用白名单,输出、finding、报告和入库前统一脱敏;沙箱失败不终止静态评审。
6+
7+
SQLite 按 task id 关联 task、sandbox run、filter block、finding 与 report,可通过 ReviewStore 接入其他 SQL。监控记录耗时、工具、拦截、finding、severity 和异常。数据库不保存原始 diff,只保存哈希与脱敏摘要;JSON 供机器消费,Markdown 供审批。静态规则和容器不能发现所有风险,生产仍需最小权限、镜像审计和人工复核。
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Skills code-review agent
2+
3+
This prototype composes a reusable `code-review` Skill, deterministic diff rules, a Filter-governed container runner,
4+
SQLite persistence, redaction, monitoring, and JSON/Markdown reports. It accepts unified diffs, a Git workspace, or a
5+
file list.
6+
7+
Run the complete flow without an API key or Docker:
8+
9+
```bash
10+
python examples/skills_code_review_agent/cli.py \
11+
--diff-file examples/skills_code_review_agent/fixtures/security.diff \
12+
--dry-run
13+
```
14+
15+
Outputs are `review_report.json`, `review_report.md`, and `reviews.db`. Query a task with
16+
`SQLiteReviewStore("reviews.db").get_task(task_id)`. The store interface separates orchestration from SQLite so another
17+
SQL backend can implement the same three methods.
18+
19+
Production policy defaults to `container`, never local execution. The runner uses Docker with no network, a read-only
20+
workspace, memory/CPU/PID limits, a timeout, capped output, and an environment allowlist. `--dry-run` selects the fake
21+
runner for tests. Local mode exists only as an explicit development fallback. Every command is checked before sandbox
22+
entry; `deny` and `needs_human_review` are persisted and never executed.
23+
24+
The static rules cover security, async errors, file/network resource lifetime, database connection lifetime, secrets,
25+
and missing tests. They only inspect added lines and are intentionally conservative. Confidence below 0.75 is routed to
26+
human-review warnings. Findings are deduplicated by file, line, and category. Static analysis and containers reduce risk
27+
but do not prove correctness or replace least-privilege infrastructure.
28+
29+
See `DESIGN.md`, `schema.sql`, and `skills/code-review/references/rules.md` for the architecture and extension points.
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
"""Automatic code-review orchestration."""
2+
3+
from __future__ import annotations
4+
5+
import hashlib
6+
import time
7+
import uuid
8+
from collections import Counter
9+
from dataclasses import replace
10+
from pathlib import Path
11+
12+
from diff_parser import DiffParser
13+
from models import Finding
14+
from models import ReviewReport
15+
from redaction import redact_text
16+
from reporting import write_reports
17+
from rules import RuleEngine
18+
from sandbox import SandboxRunner
19+
from storage import SQLiteReviewStore
20+
21+
22+
class CodeReviewAgent:
23+
def __init__(
24+
self,
25+
root: str | Path,
26+
database: str | Path,
27+
policy: str | Path,
28+
dry_run: bool = False,
29+
):
30+
self.root = Path(root)
31+
self.store = SQLiteReviewStore(database)
32+
self.runner = SandboxRunner(policy, self.root, dry_run=dry_run)
33+
self.rules = RuleEngine()
34+
self.confidence_threshold = float(self.runner.policy["confidence_threshold"])
35+
36+
def review_diff(
37+
self,
38+
diff: str,
39+
output_dir: str | Path,
40+
commands: list[list[str]] | None = None,
41+
) -> ReviewReport:
42+
started = time.perf_counter()
43+
task_id = str(uuid.uuid4())
44+
clean_diff, _ = redact_text(diff)
45+
# Rules inspect the in-memory source so secret detectors retain their
46+
# signal. Only redacted derivatives cross a logging/persistence boundary.
47+
lines = DiffParser.parse(diff)
48+
files = sorted({line.file for line in lines})
49+
summary = {
50+
"sha256": hashlib.sha256(diff.encode("utf-8")).hexdigest(),
51+
"files": files,
52+
"added_lines": len(lines),
53+
"redacted": clean_diff != diff,
54+
}
55+
self.store.create_task(task_id, summary["sha256"], summary)
56+
exceptions: Counter[str] = Counter()
57+
sandbox_runs = []
58+
try:
59+
raw_findings = [self._redact_finding(item) for item in self.rules.scan(lines)]
60+
findings = [item for item in raw_findings if item.confidence >= self.confidence_threshold]
61+
warnings = [item for item in raw_findings if item.confidence < self.confidence_threshold]
62+
for index, command in enumerate(commands or [["python", "-m", "compileall", "-q", "."]], 1):
63+
run = self.runner.run(command, index)
64+
sandbox_runs.append(run)
65+
if run.error_type:
66+
exceptions[run.error_type] += 1
67+
filter_blocks = [
68+
{"command": run.command, "decision": run.filter_decision, "reason": run.filter_reason}
69+
for run in sandbox_runs if run.status == "blocked"
70+
]
71+
severity = Counter(item.severity for item in findings)
72+
conclusion = "changes_requested" if any(
73+
item.severity in ("critical", "high") for item in findings
74+
) else "needs_human_review" if warnings or filter_blocks else "approve"
75+
status = "completed_with_errors" if exceptions else "completed"
76+
monitoring = {
77+
"total_duration_ms": (time.perf_counter() - started) * 1000,
78+
"sandbox_duration_ms": sum(run.duration_ms for run in sandbox_runs),
79+
"tool_call_count": len(sandbox_runs),
80+
"blocked_count": len(filter_blocks),
81+
"finding_count": len(findings),
82+
"warning_count": len(warnings),
83+
"severity_distribution": dict(severity),
84+
"exception_type_distribution": dict(exceptions),
85+
}
86+
report = ReviewReport(
87+
task_id=task_id,
88+
status=status,
89+
conclusion=conclusion,
90+
input_summary=summary,
91+
findings=findings,
92+
warnings=warnings,
93+
filter_blocks=filter_blocks,
94+
sandbox_runs=sandbox_runs,
95+
monitoring=monitoring,
96+
)
97+
except Exception as exc:
98+
exceptions[type(exc).__name__] += 1
99+
report = ReviewReport(
100+
task_id=task_id,
101+
status="failed",
102+
conclusion="needs_human_review",
103+
input_summary=summary,
104+
monitoring={
105+
"total_duration_ms": (time.perf_counter() - started) * 1000,
106+
"sandbox_duration_ms": 0.0,
107+
"tool_call_count": 0,
108+
"blocked_count": 0,
109+
"finding_count": 0,
110+
"warning_count": 0,
111+
"severity_distribution": {},
112+
"exception_type_distribution": dict(exceptions),
113+
},
114+
)
115+
self.store.save_report(report)
116+
write_reports(report, output_dir)
117+
return report
118+
119+
def review_file(self, diff_file: str | Path, output_dir: str | Path) -> ReviewReport:
120+
diff = Path(diff_file).read_text(encoding="utf-8")
121+
return self.review_diff(diff, output_dir)
122+
123+
def review_repo(self, repo_path: str | Path, output_dir: str | Path) -> ReviewReport:
124+
diff, _ = DiffParser.from_repo(repo_path)
125+
return self.review_diff(diff, output_dir)
126+
127+
@staticmethod
128+
def _redact_finding(finding: Finding) -> Finding:
129+
evidence, _ = redact_text(finding.evidence)
130+
title, _ = redact_text(finding.title)
131+
recommendation, _ = redact_text(finding.recommendation)
132+
return replace(finding, evidence=evidence, title=title, recommendation=recommendation)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
"""CLI for diff files, repositories, file lists, and database lookup."""
2+
3+
from __future__ import annotations
4+
5+
import argparse
6+
from pathlib import Path
7+
8+
from agent import CodeReviewAgent
9+
from diff_parser import DiffParser
10+
11+
12+
HERE = Path(__file__).resolve().parent
13+
14+
15+
def main() -> int:
16+
parser = argparse.ArgumentParser(description=__doc__)
17+
source = parser.add_mutually_exclusive_group()
18+
source.add_argument("--diff-file", type=Path)
19+
source.add_argument("--repo-path", type=Path)
20+
source.add_argument("--files", type=Path, nargs="+")
21+
parser.add_argument("--output-dir", type=Path, default=HERE)
22+
parser.add_argument("--database", type=Path, default=HERE / "reviews.db")
23+
parser.add_argument("--policy", type=Path, default=HERE / "review_policy.yaml")
24+
parser.add_argument("--dry-run", action="store_true", help="Use fake sandbox; no model or Docker required")
25+
args = parser.parse_args()
26+
if not any((args.diff_file, args.repo_path, args.files)):
27+
parser.error("one of --diff-file, --repo-path, or --files is required")
28+
29+
agent = CodeReviewAgent(
30+
root=args.repo_path or Path.cwd(),
31+
database=args.database,
32+
policy=args.policy,
33+
dry_run=args.dry_run,
34+
)
35+
if args.diff_file:
36+
report = agent.review_file(args.diff_file, args.output_dir)
37+
elif args.repo_path:
38+
report = agent.review_repo(args.repo_path, args.output_dir)
39+
else:
40+
diff, _ = DiffParser.from_paths(args.files)
41+
report = agent.review_diff(diff, args.output_dir)
42+
print(f"task_id={report.task_id} conclusion={report.conclusion}")
43+
return 0 if report.status.startswith("completed") else 1
44+
45+
46+
if __name__ == "__main__":
47+
raise SystemExit(main())
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
"""Unified diff and Git workspace input adapters."""
2+
3+
from __future__ import annotations
4+
5+
import subprocess
6+
from pathlib import Path
7+
8+
from models import ChangedLine
9+
10+
11+
class DiffParser:
12+
@staticmethod
13+
def from_file(path: str | Path) -> tuple[str, list[ChangedLine]]:
14+
text = Path(path).read_text(encoding="utf-8")
15+
return text, DiffParser.parse(text)
16+
17+
@staticmethod
18+
def from_repo(path: str | Path) -> tuple[str, list[ChangedLine]]:
19+
result = subprocess.run(
20+
["git", "diff", "--no-ext-diff", "--unified=3", "HEAD"],
21+
cwd=Path(path),
22+
capture_output=True,
23+
text=True,
24+
check=False,
25+
timeout=10,
26+
)
27+
if result.returncode:
28+
raise ValueError(f"git diff failed: {result.stderr.strip()}")
29+
return result.stdout, DiffParser.parse(result.stdout)
30+
31+
@staticmethod
32+
def from_paths(paths: list[str | Path]) -> tuple[str, list[ChangedLine]]:
33+
chunks = []
34+
for raw_path in paths:
35+
path = Path(raw_path)
36+
lines = path.read_text(encoding="utf-8").splitlines()
37+
chunks.extend([f"diff --git a/{path} b/{path}", f"+++ b/{path}", f"@@ -0,0 +1,{len(lines)} @@"])
38+
chunks.extend(f"+{line}" for line in lines)
39+
text = "\n".join(chunks) + "\n"
40+
return text, DiffParser.parse(text)
41+
42+
@staticmethod
43+
def parse(diff: str) -> list[ChangedLine]:
44+
changed: list[ChangedLine] = []
45+
current_file = ""
46+
new_line = 0
47+
hunk_header = ""
48+
for raw in diff.splitlines():
49+
if raw.startswith("+++ "):
50+
current_file = raw[4:].removeprefix("b/")
51+
continue
52+
if raw.startswith("@@"):
53+
hunk_header = raw
54+
marker = raw.split("+", 1)[1].split(" ", 1)[0]
55+
new_line = int(marker.split(",", 1)[0])
56+
continue
57+
if raw.startswith("+") and not raw.startswith("+++"):
58+
changed.append(ChangedLine(current_file, new_line, raw[1:], hunk_header))
59+
new_line += 1
60+
elif raw.startswith("-") and not raw.startswith("---"):
61+
continue
62+
elif hunk_header and not raw.startswith("\\"):
63+
new_line += 1
64+
return changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
diff --git a/worker.py b/worker.py
2+
--- a/worker.py
3+
+++ b/worker.py
4+
@@ -1,1 +1,4 @@
5+
+async def poll():
6+
+ time.sleep(10)
7+
+ asyncio.create_task(refresh())
8+
+ return True
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
diff --git a/app.py b/app.py
2+
--- a/app.py
3+
+++ b/app.py
4+
@@ -1,1 +1,2 @@
5+
def add(a, b):
6+
+ return a + b
7+
diff --git a/tests/test_app.py b/tests/test_app.py
8+
--- a/tests/test_app.py
9+
+++ b/tests/test_app.py
10+
@@ -1,1 +1,2 @@
11+
def test_add():
12+
+ assert add(1, 2) == 3
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
diff --git a/db.py b/db.py
2+
--- a/db.py
3+
+++ b/db.py
4+
@@ -1,1 +1,2 @@
5+
def load():
6+
+ connection = sqlite3.connect("data.db")
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
diff --git a/unsafe.py b/unsafe.py
2+
--- a/unsafe.py
3+
+++ b/unsafe.py
4+
@@ -1,1 +1,2 @@
5+
def run(value):
6+
+ return eval(value)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
diff --git a/app.py b/app.py
2+
--- a/app.py
3+
+++ b/app.py
4+
@@ -1,1 +1,2 @@
5+
def healthy():
6+
+ return True

0 commit comments

Comments
 (0)