From 0bdfab85709012addc3942631296382a7e285b09 Mon Sep 17 00:00:00 2001 From: ray997 <103731990@qq.com> Date: Fri, 31 Jul 2026 01:07:50 +0800 Subject: [PATCH] feat: add durable skills-based code review agent --- .gitignore | 1 + README.md | 1 + README.zh_CN.md | 1 + examples/code_review_agent/.env.example | 4 + .../code_review_agent/.env.github.example | 19 + examples/code_review_agent/README.md | 413 ++++++++ examples/code_review_agent/__init__.py | 1 + examples/code_review_agent/agent/__init__.py | 1 + examples/code_review_agent/agent/agent.py | 30 + examples/code_review_agent/agent/config.py | 22 + examples/code_review_agent/agent/prompts.py | 24 + examples/code_review_agent/agent/reviewer.py | 56 + examples/code_review_agent/agent/skills.py | 84 ++ .../code_review_agent/code_review/__init__.py | 32 + .../code_review/context_builder.py | 133 +++ .../code_review_agent/code_review/database.py | 993 ++++++++++++++++++ .../code_review_agent/code_review/git_diff.py | 250 +++++ .../code_review_agent/code_review/models.py | 196 ++++ .../code_review/orchestrator.py | 152 +++ .../code_review_agent/code_review/policy.py | 89 ++ .../code_review_agent/code_review/reporter.py | 105 ++ .../code_review/static_analysis.py | 437 ++++++++ .../github_integration/__init__.py | 6 + .../github_integration/app.py | 153 +++ .../github_integration/checkout.py | 129 +++ .../github_integration/client.py | 247 +++++ .../github_integration/models.py | 71 ++ .../github_integration/publisher.py | 260 +++++ .../github_integration/runtime.py | 120 +++ .../github_integration/security.py | 21 + .../github_integration/service.py | 166 +++ .../github_integration/worker.py | 144 +++ examples/code_review_agent/inspect_reviews.py | 137 +++ .../code_review_agent/requirements-github.txt | 1 + .../code_review_agent/requirements-static.txt | 3 + .../code_review_agent/run_github_webhook.py | 27 + .../code_review_agent/run_github_worker.py | 92 ++ examples/code_review_agent/run_review.py | 171 +++ examples/code_review_agent/sandbox/Dockerfile | 15 + .../skills/python-correctness/SKILL.md | 39 + .../python-correctness/agents/openai.yaml | 4 + .../skills/python-security/SKILL.md | 38 + .../skills/python-security/agents/openai.yaml | 4 + .../skills/review-maintainability/SKILL.md | 35 + .../review-maintainability/agents/openai.yaml | 4 + .../skills/review-test-coverage/SKILL.md | 38 + .../review-test-coverage/agents/openai.yaml | 4 + tests/code_review/__init__.py | 1 + tests/code_review/conftest.py | 45 + tests/code_review/test_context_builder.py | 44 + tests/code_review/test_database.py | 263 +++++ tests/code_review/test_git_diff.py | 84 ++ tests/code_review/test_github_integration.py | 462 ++++++++ tests/code_review/test_pipeline.py | 113 ++ tests/code_review/test_policy.py | 98 ++ tests/code_review/test_static_analysis.py | 144 +++ 56 files changed, 6227 insertions(+) create mode 100644 examples/code_review_agent/.env.example create mode 100644 examples/code_review_agent/.env.github.example create mode 100644 examples/code_review_agent/README.md create mode 100644 examples/code_review_agent/__init__.py create mode 100644 examples/code_review_agent/agent/__init__.py create mode 100644 examples/code_review_agent/agent/agent.py create mode 100644 examples/code_review_agent/agent/config.py create mode 100644 examples/code_review_agent/agent/prompts.py create mode 100644 examples/code_review_agent/agent/reviewer.py create mode 100644 examples/code_review_agent/agent/skills.py create mode 100644 examples/code_review_agent/code_review/__init__.py create mode 100644 examples/code_review_agent/code_review/context_builder.py create mode 100644 examples/code_review_agent/code_review/database.py create mode 100644 examples/code_review_agent/code_review/git_diff.py create mode 100644 examples/code_review_agent/code_review/models.py create mode 100644 examples/code_review_agent/code_review/orchestrator.py create mode 100644 examples/code_review_agent/code_review/policy.py create mode 100644 examples/code_review_agent/code_review/reporter.py create mode 100644 examples/code_review_agent/code_review/static_analysis.py create mode 100644 examples/code_review_agent/github_integration/__init__.py create mode 100644 examples/code_review_agent/github_integration/app.py create mode 100644 examples/code_review_agent/github_integration/checkout.py create mode 100644 examples/code_review_agent/github_integration/client.py create mode 100644 examples/code_review_agent/github_integration/models.py create mode 100644 examples/code_review_agent/github_integration/publisher.py create mode 100644 examples/code_review_agent/github_integration/runtime.py create mode 100644 examples/code_review_agent/github_integration/security.py create mode 100644 examples/code_review_agent/github_integration/service.py create mode 100644 examples/code_review_agent/github_integration/worker.py create mode 100755 examples/code_review_agent/inspect_reviews.py create mode 100644 examples/code_review_agent/requirements-github.txt create mode 100644 examples/code_review_agent/requirements-static.txt create mode 100755 examples/code_review_agent/run_github_webhook.py create mode 100755 examples/code_review_agent/run_github_worker.py create mode 100755 examples/code_review_agent/run_review.py create mode 100644 examples/code_review_agent/sandbox/Dockerfile create mode 100644 examples/code_review_agent/skills/python-correctness/SKILL.md create mode 100644 examples/code_review_agent/skills/python-correctness/agents/openai.yaml create mode 100644 examples/code_review_agent/skills/python-security/SKILL.md create mode 100644 examples/code_review_agent/skills/python-security/agents/openai.yaml create mode 100644 examples/code_review_agent/skills/review-maintainability/SKILL.md create mode 100644 examples/code_review_agent/skills/review-maintainability/agents/openai.yaml create mode 100644 examples/code_review_agent/skills/review-test-coverage/SKILL.md create mode 100644 examples/code_review_agent/skills/review-test-coverage/agents/openai.yaml create mode 100644 tests/code_review/__init__.py create mode 100644 tests/code_review/conftest.py create mode 100644 tests/code_review/test_context_builder.py create mode 100644 tests/code_review/test_database.py create mode 100644 tests/code_review/test_git_diff.py create mode 100644 tests/code_review/test_github_integration.py create mode 100644 tests/code_review/test_pipeline.py create mode 100644 tests/code_review/test_policy.py create mode 100644 tests/code_review/test_static_analysis.py diff --git a/.gitignore b/.gitignore index 58eb6b48a..2edf271a0 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,4 @@ pyrightconfig.json # spec-workflow tool artifacts .spec-workflow +.code-review/ diff --git a/README.md b/README.md index b2c91e7d3..3c8ac47cc 100644 --- a/README.md +++ b/README.md @@ -461,6 +461,7 @@ skill_tool_set = SkillToolSet(repository=repository, run_tool_kwargs=tool_kwargs Recommended first: - [examples/code_executors](./examples/code_executors/README.md) - `UnsafeLocalCodeExecutor` / `ContainerCodeExecutor` +- [examples/code_review_agent](./examples/code_review_agent/README.md) - Local/GitHub App code review with Skills, sandboxed analyzers, and SQL persistence Related docs: [code_executor.md](./docs/mkdocs/en/code_executor.md) diff --git a/README.zh_CN.md b/README.zh_CN.md index ca4c5f3be..101ccb2d6 100644 --- a/README.zh_CN.md +++ b/README.zh_CN.md @@ -432,6 +432,7 @@ skill_tool_set = SkillToolSet(repository=repository, run_tool_kwargs=tool_kwargs 建议先看： - [examples/code_executors](./examples/code_executors/README.md) - UnsafeLocalCodeExecutor / ContainerCodeExecutor +- [examples/code_review_agent](./examples/code_review_agent/README.md) - 支持本地/GitHub App、Skills、沙箱分析和 SQL 持久化的代码评审 Agent 相关文档：[code_executor.md](./docs/mkdocs/zh/code_executor.md) diff --git a/examples/code_review_agent/.env.example b/examples/code_review_agent/.env.example new file mode 100644 index 000000000..5369fb261 --- /dev/null +++ b/examples/code_review_agent/.env.example @@ -0,0 +1,4 @@ +TRPC_AGENT_API_KEY=replace-me +TRPC_AGENT_BASE_URL=https://api.example.com/v1 +TRPC_AGENT_MODEL_NAME=replace-me +CODE_REVIEW_DATABASE_URL=sqlite:////absolute/path/to/reviews.db diff --git a/examples/code_review_agent/.env.github.example b/examples/code_review_agent/.env.github.example new file mode 100644 index 000000000..4eb216148 --- /dev/null +++ b/examples/code_review_agent/.env.github.example @@ -0,0 +1,19 @@ +GITHUB_WEBHOOK_SECRET=replace-with-a-high-entropy-secret +GITHUB_APP_ID=123456 +GITHUB_APP_PRIVATE_KEY_PATH=/absolute/path/to/github-app.private-key.pem +GITHUB_API_URL=https://api.github.com +GITHUB_CLONE_HOSTS=github.com +GITHUB_REVIEW_WORKSPACE_ROOT=.code-review/workspaces +GITHUB_REVIEW_STATIC_RUNTIME=docker +GITHUB_REVIEW_DOCKER_IMAGE=trpc-code-review:latest +GITHUB_REVIEW_MINIMUM_CONFIDENCE=0.75 +GITHUB_REVIEW_PUBLISH_COMMENTS=true +GITHUB_REVIEW_MAX_COMMENTS=20 +GITHUB_REVIEW_MAX_ATTEMPTS=5 +GITHUB_REVIEW_WORKER_CONCURRENCY=1 +GITHUB_REVIEW_POLL_SECONDS=2 +GITHUB_REVIEW_LEASE_SECONDS=300 +GITHUB_REVIEW_RETRY_BASE_SECONDS=5 +GITHUB_REVIEW_RETRY_MAX_SECONDS=300 +GITHUB_WEBHOOK_HOST=127.0.0.1 +GITHUB_WEBHOOK_PORT=8080 diff --git a/examples/code_review_agent/README.md b/examples/code_review_agent/README.md new file mode 100644 index 000000000..b3ce7050d --- /dev/null +++ b/examples/code_review_agent/README.md @@ -0,0 +1,413 @@ +# 自动代码评审 Agent（阶段 1～6） + +本示例提供一个可运行的代码评审闭环：从本地 Git 仓库提取 +`base...head` 变更，构造有大小边界的 Diff 上下文，运行 +Ruff/Bandit/Pytest 静态分析并加载项目 Skills，再由单个 `LlmAgent` +返回结构化 Finding，最后执行确定性的路径/行号校验、去重、排序并生成 +JSON 和 Markdown 报告，并将完整运行记录持久化到 SQL 数据库。 + +当前版本同时支持本地 Git Diff 和 GitHub App `pull_request` Webhook。 + +## 已实现能力 + +- 安全调用 Git：使用参数数组执行，不把 revision 或路径拼入 shell。 +- PR 风格比较：默认使用 `merge-base(base, head)` 到 `head` 的变更。 +- 解析新增、修改、删除、重命名文件及 unified diff hunk 行号。 +- 跳过二进制、依赖目录、构建产物、lock 和压缩文件。 +- 设置文件数量、单文件 Patch 和总上下文字符预算。 +- 使用 Pydantic Schema 约束模型的 `ReviewOutput` 和 `Finding`。 +- 将 Ruff/Bandit JSON 转换成与 LLM 相同的 Finding Schema。 +- 可选执行 Pytest，并在报告中保留命令、状态、耗时和截断日志。 +- 支持本地分析以及禁网、只读、资源受限的 Docker 静态分析。 +- 提供 correctness/security/maintainability/test-coverage 四个评审 Skills。 +- LLM 只能加载 Skill 知识，不能通过 Skill 获得 shell/exec 工具。 +- 丢弃不属于本次变更文件的 Finding。 +- 只有命中新增行的 Finding 才标记为 `publishable`。 +- 按规则、文件和行号去重，并保留优先级更高的结果。 +- 支持完全不调用模型的 `--no-llm` 模式。 +- 默认使用 SQLite 保存运行、变更文件、Finding 和分析器执行记录。 +- 基于实际 commit、完整配置和模型生成幂等键，重复运行不会重复入库。 +- 支持按 ID 查看，以及按仓库、状态查询最近的评审记录。 +- 使用 HMAC-SHA256 验证 GitHub Webhook 原始请求体。 +- 按 `X-GitHub-Delivery` 原子去重并记录处理生命周期。 +- Webhook 与持久化任务在同一事务内写入，接收进程重启不会丢任务。 +- 独立 Worker 使用租约领取任务，支持心跳、崩溃恢复、退避重试和并发限制。 +- 超过最大次数的任务进入 dead 状态，可由运维 CLI 显式重放。 +- 使用 GitHub App 安装令牌检出 PR 的精确 base/head commit。 +- 发布 Check Run annotations，并可选发布新增行 Review Comments。 + +## 目录 + +```text +code_review_agent/ +├── run_review.py +├── inspect_reviews.py # 查询已持久化的评审记录 +├── run_github_webhook.py # FastAPI/Uvicorn Webhook 服务 +├── run_github_worker.py # 持久化队列 Worker +├── agent/ +│ ├── agent.py # 单个结构化输出 LlmAgent +│ ├── config.py +│ ├── prompts.py +│ ├── reviewer.py # Agent Runtime 适配器 +│ └── skills.py # Knowledge-only SkillToolSet +├── code_review/ +│ ├── models.py # ReviewRun / ChangedFile / Finding +│ ├── git_diff.py # Git 采集和 Hunk 解析 +│ ├── context_builder.py +│ ├── static_analysis.py +│ ├── database.py # SQLAlchemy Schema 与 ReviewStore +│ ├── policy.py +│ ├── orchestrator.py +│ └── reporter.py +├── skills/ # 四个代码评审 SKILL.md +├── github_integration/ +│ ├── app.py # 验签、去重与 FastAPI 入口 +│ ├── client.py # GitHub App token 和 REST API +│ ├── checkout.py # 受控、精确 commit 检出 +│ ├── publisher.py # Check Run 与行级评论 +│ ├── service.py # GitHub 评审编排 +│ ├── runtime.py # 环境配置与依赖构造 +│ └── worker.py # 租约、重试和崩溃恢复 +└── sandbox/Dockerfile # 离线运行时使用的分析镜像 +``` + +## 安装 + +在仓库根目录安装项目： + +```bash +python3 -m venv .venv +source .venv/bin/activate +pip install -e . +``` + +本地运行静态工具时另外安装： + +```bash +pip install -r examples/code_review_agent/requirements-static.txt +``` + +## 先运行 no-LLM 模式 + +该模式不会读取模型配置。默认尝试在本地运行 Ruff 和 Bandit；工具未安装 +时会记录为 `unavailable`，不会让评审失败： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo . \ + --base HEAD~1 \ + --head HEAD \ + --no-llm +``` + +如需只采集 Diff、不运行任何工具： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo . --base HEAD~1 --head HEAD \ + --no-llm --no-static-analysis +``` + +默认在当前目录的 `.code-review/` 中生成： + +- `review.json`：完整、可机器消费的 ReviewRun。 +- `review.md`：适合人工查看或未来写入 Check Run 的报告。 +- `reviews.db`：SQLite 评审数据库。 + +相同仓库、实际 base/head commit、模型和完整运行配置会生成相同幂等键。 +再次执行仍会完成评审，但持久化时会返回已经存在的记录，不会插入重复数据。 + +如需关闭数据库持久化： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo . --base HEAD~1 --head HEAD \ + --no-llm --no-persist +``` + +## 数据库存储和查询 + +数据库包含以下表： + +- `code_review_runs` +- `code_review_changed_files` +- `code_review_findings` +- `code_review_analyzer_executions` +- `code_review_github_deliveries` +- `code_review_github_jobs` +- `code_review_github_publications` +- `code_review_schema_version` + +可以通过同步 SQLAlchemy URL 切换数据库： + +```bash +export CODE_REVIEW_DATABASE_URL=sqlite:////absolute/path/reviews.db +``` + +也可以仅对单次运行传入： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo . --base HEAD~1 --head HEAD \ + --no-llm \ + --database-url sqlite:////absolute/path/reviews.db +``` + +列出最近的运行： + +```bash +python3 examples/code_review_agent/inspect_reviews.py list --limit 20 +``` + +按状态或仓库过滤： + +```bash +python3 examples/code_review_agent/inspect_reviews.py list \ + --status failed \ + --repository /path/to/repository +``` + +查看完整 Markdown 或 JSON： + +```bash +python3 examples/code_review_agent/inspect_reviews.py show RUN_ID +python3 examples/code_review_agent/inspect_reviews.py show RUN_ID --format json +``` + +查看 GitHub Webhook 处理状态： + +```bash +python3 examples/code_review_agent/inspect_reviews.py deliveries +python3 examples/code_review_agent/inspect_reviews.py deliveries --status failed +``` + +查看持久化队列和重放 dead job： + +```bash +python3 examples/code_review_agent/inspect_reviews.py jobs +python3 examples/code_review_agent/inspect_reviews.py jobs --status dead +python3 examples/code_review_agent/inspect_reviews.py replay DELIVERY_ID +``` + +## 运行 LLM 评审 + +配置 OpenAI-compatible 模型： + +```bash +export TRPC_AGENT_API_KEY=your-api-key +export TRPC_AGENT_BASE_URL=https://your-endpoint/v1 +export TRPC_AGENT_MODEL_NAME=your-model +``` + +执行： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo /path/to/repository \ + --base main \ + --head feature-branch +``` + +模型只收到经过过滤和预算限制的 unified diff，不会收到整个仓库。模型还 +会收到静态分析的结构化结果，并可加载四个知识型评审 Skills。 + +## GitHub App Webhook + +GitHub App 最小仓库权限： + +- Contents: Read +- Checks: Read and write +- Pull requests: Read and write（关闭行级评论时可以只保留 Read） + +订阅 `pull_request` 事件。当前处理 `opened`、`reopened`、 +`synchronize` 和 `ready_for_review`，草稿 PR 会记录为 `ignored`。 +Webhook URL 指向： + +```text +https://your-service.example/webhooks/github +``` + +安装 GitHub App JWT 依赖： + +```bash +pip install -r examples/code_review_agent/requirements-github.txt +``` + +参考 `.env.github.example` 配置环境变量，至少需要： + +```bash +export GITHUB_WEBHOOK_SECRET=high-entropy-webhook-secret +export GITHUB_APP_ID=123456 +export GITHUB_APP_PRIVATE_KEY_PATH=/absolute/path/github-app.pem +export TRPC_AGENT_API_KEY=your-api-key +export TRPC_AGENT_BASE_URL=https://your-endpoint/v1 +export TRPC_AGENT_MODEL_NAME=your-model +``` + +开发环境也可以使用预生成的安装令牌： + +```bash +export GITHUB_TOKEN=installation-token +``` + +分别启动 Webhook 接收进程和至少一个 Worker： + +```bash +python3 examples/code_review_agent/run_github_webhook.py +python3 examples/code_review_agent/run_github_worker.py --concurrency 2 +``` + +健康检查： + +```bash +curl http://127.0.0.1:8080/healthz +``` + +默认 GitHub 模式使用 Docker 静态分析。处理流程为： + +1. 对原始请求体执行 HMAC-SHA256 恒定时间验签。 +2. 在同一数据库事务中写入 delivery 和 durable job，立即返回 HTTP 202。 +3. Worker 以 CAS 方式领取任务并创建带心跳的有期限租约。 +4. 创建 `in_progress` Check Run。 +5. 通过环境变量向 Git 传递认证头，不把 token 放入 URL 或命令参数。 +6. 检出签名 Payload 中的精确 base/head SHA，运行现有评审管道并持久化。 +7. 分批发布最多 50 条/请求的 Check annotations。 +8. 可选发布最多 `GITHUB_REVIEW_MAX_COMMENTS` 条右侧新增行评论。 +9. 成功后完成 job；临时错误按指数退避重试，过期租约会被其他 Worker 恢复。 +10. 达到 `GITHUB_REVIEW_MAX_ATTEMPTS` 后进入 dead/failed，等待显式重放。 + +REST 请求固定发送 `X-GitHub-Api-Version: 2026-03-10`。安装令牌请求主动 +收窄为 Contents read、Checks write 和 Pull requests write；代码不依赖 +安装令牌的固定长度。 + +生产环境应让 Webhook 与所有 Worker 共享同一个 +`CODE_REVIEW_DATABASE_URL`。SQLite 适合单机运行；多副本部署建议使用 +PostgreSQL 或 MySQL。主要队列参数： + +```text +GITHUB_REVIEW_MAX_ATTEMPTS=5 +GITHUB_REVIEW_WORKER_CONCURRENCY=1 +GITHUB_REVIEW_POLL_SECONDS=2 +GITHUB_REVIEW_LEASE_SECONDS=300 +GITHUB_REVIEW_RETRY_BASE_SECONDS=5 +GITHUB_REVIEW_RETRY_MAX_SECONDS=300 +``` + +租约时间应显著长于正常心跳间隔；Worker 每隔约三分之一租约时间自动续租。 +收到 SIGINT/SIGTERM 后 Worker 会停止领取新任务，并等待当前任务结束。 + +## Docker 沙箱 + +构建一次分析镜像： + +```bash +docker build \ + -t trpc-code-review:latest \ + examples/code_review_agent/sandbox +``` + +在容器中运行 Ruff、Bandit，并可选运行 Pytest： + +```bash +python3 examples/code_review_agent/run_review.py \ + --repo /path/to/repository \ + --base main \ + --head feature-branch \ + --no-llm \ + --static-runtime docker \ + --run-tests +``` + +运行容器时框架固定使用： + +- `--network none` +- 仓库和根文件系统只读 +- `--cap-drop ALL` +- `no-new-privileges` +- 非 root UID/GID +- CPU、内存、PID 和超时限制 +- `--pull never`，评审期间不会隐式下载镜像 + +## 常用参数 + +```text +--direct-base 不使用 merge-base，直接比较 base 与 head +--context-lines 3 每个 Hunk 携带的上下文行数 +--max-files 40 最大送审文件数 +--max-file-chars 24000 单文件最大 Patch 字符数 +--max-total-chars 120000 总 Diff 上下文字符数 +--minimum-confidence 0.75 丢弃低置信度 Finding +--output-dir PATH 报告输出目录 +--no-static-analysis 不运行 Ruff/Bandit/Pytest +--static-runtime MODE local 或 docker +--run-tests 额外执行 Pytest +--strict-static-tools 工具缺失或失败时让 ReviewRun 失败 +--static-timeout 120 每个静态工具的超时秒数 +--docker-image IMAGE 指定已经构建的分析镜像 +--database-url URL 指定同步 SQLAlchemy 数据库 URL +--no-persist 不写入数据库 +``` + +## Finding 约定 + +```json +{ + "rule_id": "python.correctness.none-sentinel", + "severity": "medium", + "confidence": 0.88, + "category": "correctness", + "file_path": "app.py", + "start_line": 10, + "end_line": 10, + "title": "Ambiguous error result", + "description": "Returning None silently changes the function contract.", + "suggestion": "Raise a documented exception or return a typed result.", + "source": "llm", + "publishable": true +} +``` + +`publishable` 不由模型或扫描器决定。所有结果经过同一个 Policy 后，只有 +文件属于本次 Diff 且行号命中新增行时才会被设置为 `true`。 + +## 测试 + +```bash +pytest tests/code_review -q +``` + +测试使用临时 Git 仓库、fake analyzer 和 fake reviewer，不调用真实模型： + +- Diff 文件、重命名和行号解析 +- revision 参数安全校验 +- 二进制/生成目录过滤 +- 上下文预算与截断 +- Finding 路径、变更行、去重和置信度策略 +- Ruff/Bandit JSON 到 Finding 的转换 +- Docker 禁网、只读和资源限制参数 +- 静态分析与 LLM Finding 的统一 Policy +- SQLite 完整对象图写入和读取 +- 幂等重复写入、失败运行持久化和查询过滤 +- 运行与查询 CLI 端到端 +- GitHub 官方 HMAC 测试向量、Payload 解析和重复 delivery 冲突 +- Webhook 原子入队、Worker 成功/重试/dead 状态转换 +- 租约续期、过期租约恢复、所有权校验和人工重放 +- GitHub API 版本/认证头、受控检出和 host allowlist +- Webhook 到评审、数据库、Check Run、精确新增行评论的模拟端到端 +- no-LLM 与 fake reviewer 端到端管道 + +## 当前边界 + +- 当前使用内建 Schema 版本表和 `create_all` 初始化版本 4；后续修改表结构时 + 应引入 Alembic，而不是依赖 `create_all` 修改既有表。 +- `ReviewStore` 使用同步 SQLAlchemy 驱动；异步数据库 URL 目前不接受。 +- 队列提供 at-least-once 执行语义。数据库与 GitHub 无法形成跨系统事务； + Check Run 使用 delivery ID 作为 `external_id`，重试时会从 GitHub 恢复； + 行级评论也使用稳定标记跳过最近的重复项。由于 GitHub 最近评论查询存在分页 + 边界，极端高流量 PR 在外部请求成功后立即崩溃时仍建议人工核对。 +- 目前只支持 GitHub/GitHub Enterprise 的 HTTPS Git URL，允许的 clone host + 必须显式配置在 `GITHUB_CLONE_HOSTS`。 +- Docker 镜像只包含通用 Python 工具，不会自动安装被评审项目的依赖； + 因此 Pytest 适合依赖自包含的项目，后续需要增加受控依赖准备策略。 +- Finding 是否真实正确仍取决于所使用模型；Policy 只保证输出结构和 + Diff 位置合法，不能代替评审质量评估。 diff --git a/examples/code_review_agent/__init__.py b/examples/code_review_agent/__init__.py new file mode 100644 index 000000000..a60173060 --- /dev/null +++ b/examples/code_review_agent/__init__.py @@ -0,0 +1 @@ +"""Local-first automatic code review example.""" diff --git a/examples/code_review_agent/agent/__init__.py b/examples/code_review_agent/agent/__init__.py new file mode 100644 index 000000000..3ea91106c --- /dev/null +++ b/examples/code_review_agent/agent/__init__.py @@ -0,0 +1 @@ +"""Agent construction and execution for the code review example.""" diff --git a/examples/code_review_agent/agent/agent.py b/examples/code_review_agent/agent/agent.py new file mode 100644 index 000000000..4be370ac6 --- /dev/null +++ b/examples/code_review_agent/agent/agent.py @@ -0,0 +1,30 @@ +"""Construct the single structured-output review agent.""" + +from __future__ import annotations + +from trpc_agent_sdk.agents import LlmAgent +from trpc_agent_sdk.models import OpenAIModel + +from ..code_review.models import ReviewOutput +from .config import get_model_config +from .prompts import INSTRUCTION +from .skills import create_review_skill_toolset + + +def create_agent() -> LlmAgent: + """Create a diff-only reviewer with schema-constrained output.""" + api_key, base_url, model_name = get_model_config() + model = OpenAIModel(model_name=model_name, api_key=api_key, base_url=base_url) + skill_toolset, skill_repository = create_review_skill_toolset() + return LlmAgent( + name="code_reviewer", + description="Reviews a bounded Git diff and returns structured findings.", + model=model, + instruction=INSTRUCTION, + tools=[skill_toolset], + skill_repository=skill_repository, + output_schema=ReviewOutput, + output_key="code_review_output", + include_previous_history=False, + max_history_messages=1, + ) diff --git a/examples/code_review_agent/agent/config.py b/examples/code_review_agent/agent/config.py new file mode 100644 index 000000000..6868ca1a5 --- /dev/null +++ b/examples/code_review_agent/agent/config.py @@ -0,0 +1,22 @@ +"""Model configuration sourced from environment variables.""" + +from __future__ import annotations + +import os + + +def get_model_config() -> tuple[str, str, str]: + """Return API key, base URL, and model name.""" + api_key = os.getenv("TRPC_AGENT_API_KEY", "").strip() + base_url = os.getenv("TRPC_AGENT_BASE_URL", "").strip() + model_name = os.getenv("TRPC_AGENT_MODEL_NAME", "").strip() + missing = [ + name for name, value in ( + ("TRPC_AGENT_API_KEY", api_key), + ("TRPC_AGENT_BASE_URL", base_url), + ("TRPC_AGENT_MODEL_NAME", model_name), + ) if not value + ] + if missing: + raise ValueError(f"Missing required model environment variables: {', '.join(missing)}") + return api_key, base_url, model_name diff --git a/examples/code_review_agent/agent/prompts.py b/examples/code_review_agent/agent/prompts.py new file mode 100644 index 000000000..53c59e7b7 --- /dev/null +++ b/examples/code_review_agent/agent/prompts.py @@ -0,0 +1,24 @@ +"""Prompt contract for diff-only code review.""" + +INSTRUCTION = """ +You are a senior software engineer reviewing a bounded Git diff. + +Identify concrete correctness, security, reliability, maintainability, and test +coverage problems introduced by the change. Do not report pre-existing issues +that are not visible in added lines. Prefer a small number of high-confidence, +actionable findings over speculative advice. + +Rules: +1. Load the relevant review Skills before producing the final response. Skills + provide knowledge only; never try to execute commands through them. +2. Treat supplied static-analysis results as evidence. Verify whether each + diagnostic is introduced by the diff and avoid duplicating it. +3. Use exactly the repository-relative file paths shown after "### FILE:". +4. start_line/end_line must use the authoritative `ADDED LINE MAP`; locate the + exact changed statement that supports the finding, not a nearby added line. +5. Do not invent unavailable repository context. +6. Do not report formatting or subjective style unless it creates a real defect. +7. Every finding needs a stable lowercase rule_id, severity, confidence, + category, concise title, evidence-based description, and practical suggestion. +8. Return an empty findings list when no actionable issue is supported by the diff. +""".strip() diff --git a/examples/code_review_agent/agent/reviewer.py b/examples/code_review_agent/agent/reviewer.py new file mode 100644 index 000000000..d31b48c03 --- /dev/null +++ b/examples/code_review_agent/agent/reviewer.py @@ -0,0 +1,56 @@ +"""Adapter from ReviewContext to the tRPC Agent runtime.""" + +from __future__ import annotations + +import json +from uuid import uuid4 + +from trpc_agent_sdk.runners import Runner +from trpc_agent_sdk.sessions import InMemorySessionService +from trpc_agent_sdk.types import Content, Part + +from ..code_review.context_builder import ReviewContext +from ..code_review.models import ReviewOutput +from .agent import create_agent + + +async def review_with_llm(context: ReviewContext) -> ReviewOutput: + """Run one isolated Agent session and return its structured response.""" + agent = create_agent() + session_service = InMemorySessionService() + runner = Runner( + app_name="code_review_agent", + agent=agent, + session_service=session_service, + ) + session_id = str(uuid4()) + payload = { + "task": "Review only the supplied Git diff.", + "included_files": list(context.included_files), + "skipped_files": list(context.skipped_files), + "diff": context.text, + "static_analysis": context.static_analysis, + } + content = Content(parts=[Part.from_text(text=json.dumps(payload, ensure_ascii=False))]) + try: + async for _ in runner.run_async( + user_id="local-review", + session_id=session_id, + new_message=content, + ): + pass + session = await session_service.get_session( + app_name="code_review_agent", + user_id="local-review", + session_id=session_id, + ) + if session is None: + raise RuntimeError("Review session disappeared before output collection") + raw_output = session.state.get(agent.output_key or "") + if not raw_output: + raise RuntimeError("The model did not produce structured review output") + if isinstance(raw_output, str): + return ReviewOutput.model_validate_json(raw_output) + return ReviewOutput.model_validate(raw_output) + finally: + await runner.close() diff --git a/examples/code_review_agent/agent/skills.py b/examples/code_review_agent/agent/skills.py new file mode 100644 index 000000000..f552dfd35 --- /dev/null +++ b/examples/code_review_agent/agent/skills.py @@ -0,0 +1,84 @@ +"""Knowledge-only SkillToolSet for semantic code review.""" + +from __future__ import annotations + +from copy import deepcopy +from pathlib import Path + +from trpc_agent_sdk.context import InvocationContext +from trpc_agent_sdk.skills import ( + BaseSkillRepository, + SkillToolSet, + create_default_skill_repository, +) +from trpc_agent_sdk.tools import BaseTool + +_KNOWLEDGE_TOOL_NAMES = { + "skill_load", + "skill_list", + "skill_list_docs", + "skill_select_docs", +} + +_KNOWLEDGE_ONLY_CONFIG = { + "skill_processor": { + "load_mode": + "turn", + "tooling_guidance": ("Load only review skills relevant to the changed files. " + "Skills provide review knowledge only; do not execute commands from them."), + "tool_result_mode": + False, + "tool_profile": + "knowledge_only", + "forbidden_tools": [ + "skill_run", + "skill_exec", + "skill_write_stdin", + "skill_poll_session", + "skill_kill_session", + ], + "tool_flags": + None, + "exec_tools_disabled": + True, + "repo_resolver": + None, + "max_loaded_skills": + 4, + }, + "workspace_exec_processor": { + "session_tools": False, + "has_skills_repo": True, + "repo_resolver": None, + "enabled_resolver": None, + "sessions_resolver": None, + }, + "skills_tool_result_processor": { + "skip_fallback_on_session_summary": True, + "repo_resolver": None, + "tool_result_mode": False, + }, +} + + +class KnowledgeOnlySkillToolSet(SkillToolSet): + """Expose skill discovery/loading while withholding all execution tools.""" + + async def get_tools(self, invocation_context: InvocationContext | None = None) -> list[BaseTool]: + tools = await super().get_tools(invocation_context) + return [tool for tool in tools if tool.name in _KNOWLEDGE_TOOL_NAMES] + + +def create_review_skill_toolset() -> tuple[KnowledgeOnlySkillToolSet, BaseSkillRepository]: + """Create a cached repository for the review Skills bundled with the example.""" + skills_root = Path(__file__).resolve().parent.parent / "skills" + repository = create_default_skill_repository( + str(skills_root), + enable_hot_reload=False, + use_cached_repository=True, + ) + toolset = KnowledgeOnlySkillToolSet( + repository=repository, + skill_config=deepcopy(_KNOWLEDGE_ONLY_CONFIG), + ) + return toolset, repository diff --git a/examples/code_review_agent/code_review/__init__.py b/examples/code_review_agent/code_review/__init__.py new file mode 100644 index 000000000..ea0992986 --- /dev/null +++ b/examples/code_review_agent/code_review/__init__.py @@ -0,0 +1,32 @@ +"""Deterministic code review pipeline components.""" + +from .context_builder import ContextBudget, ReviewContext, build_review_context +from .database import ReviewStore, SaveReviewResult, sqlite_database_url +from .git_diff import GitDiffCollector +from .models import ( + AnalyzerExecution, + AnalyzerStatus, + ChangedFile, + Finding, + ReviewOutput, + ReviewRun, +) +from .static_analysis import StaticAnalysisConfig, StaticAnalyzer + +__all__ = [ + "AnalyzerExecution", + "AnalyzerStatus", + "ChangedFile", + "ContextBudget", + "Finding", + "GitDiffCollector", + "ReviewContext", + "ReviewOutput", + "ReviewRun", + "ReviewStore", + "SaveReviewResult", + "StaticAnalysisConfig", + "StaticAnalyzer", + "build_review_context", + "sqlite_database_url", +] diff --git a/examples/code_review_agent/code_review/context_builder.py b/examples/code_review_agent/code_review/context_builder.py new file mode 100644 index 000000000..1bc5c9827 --- /dev/null +++ b/examples/code_review_agent/code_review/context_builder.py @@ -0,0 +1,133 @@ +"""Review scope filtering and prompt context budgeting.""" + +from __future__ import annotations + +from dataclasses import dataclass +from fnmatch import fnmatch + +from .models import ChangedFile, LineChangeType + +DEFAULT_EXCLUDES = ( + ".git/**", + "**/.git/**", + "__pycache__/**", + "**/__pycache__/**", + "node_modules/**", + "**/node_modules/**", + "vendor/**", + "**/vendor/**", + "dist/**", + "**/dist/**", + "build/**", + "**/build/**", + "*.lock", + "**/*.lock", + "*.min.js", + "**/*.min.js", + "*.map", + "**/*.map", +) + + +@dataclass(frozen=True) +class ContextBudget: + """Limits applied before any repository content reaches the model.""" + + max_files: int = 40 + max_patch_chars_per_file: int = 24_000 + max_total_chars: int = 120_000 + exclude_patterns: tuple[str, ...] = DEFAULT_EXCLUDES + + +@dataclass(frozen=True) +class ReviewContext: + """Prompt-ready diff context plus scope diagnostics.""" + + text: str + included_files: tuple[str, ...] + skipped_files: tuple[str, ...] + truncated_files: tuple[str, ...] + diagnostics: tuple[str, ...] + static_analysis: str = "" + + +def build_review_context(changed_files: list[ChangedFile], budget: ContextBudget | None = None) -> ReviewContext: + """Build bounded, diff-only model context.""" + budget = budget or ContextBudget() + sections: list[str] = [] + included: list[str] = [] + skipped: list[str] = [] + truncated: list[str] = [] + diagnostics: list[str] = [] + remaining = max(0, budget.max_total_chars) + + for changed_file in changed_files: + if len(included) >= budget.max_files: + skipped.append(changed_file.path) + continue + if changed_file.is_binary: + skipped.append(changed_file.path) + diagnostics.append(f"Skipped binary file: {changed_file.path}") + continue + if _is_excluded(changed_file.path, budget.exclude_patterns): + skipped.append(changed_file.path) + diagnostics.append(f"Skipped excluded path: {changed_file.path}") + continue + if not changed_file.patch: + skipped.append(changed_file.path) + diagnostics.append(f"Skipped file without textual patch: {changed_file.path}") + continue + + added_line_map = _render_added_line_map(changed_file) + header = (f"### FILE: {changed_file.path}\n" + f"change_type: {changed_file.change_type.value}\n" + f"language: {changed_file.language}\n" + f"added_lines: {changed_file.added_lines}\n" + f"deleted_lines: {changed_file.deleted_lines}\n" + f"ADDED LINE MAP (authoritative new-file locations):\n{added_line_map}\n\n" + "UNIFIED PATCH:\n") + allowance = min(budget.max_patch_chars_per_file, max(0, remaining - len(header))) + if allowance <= 0: + skipped.append(changed_file.path) + diagnostics.append("Reached total context budget") + continue + patch = changed_file.patch + truncation_marker = "\n[PATCH TRUNCATED]\n" + if len(patch) > allowance: + patch = patch[:max(0, allowance - len(truncation_marker))] + truncation_marker + changed_file.is_truncated = True + changed_file.patch = patch + truncated.append(changed_file.path) + section = header + patch + if len(section) > remaining: + skipped.append(changed_file.path) + diagnostics.append("Reached total context budget") + continue + sections.append(section) + included.append(changed_file.path) + remaining -= len(section) + + if skipped: + diagnostics.append(f"Skipped {len(skipped)} file(s)") + if truncated: + diagnostics.append(f"Truncated {len(truncated)} file patch(es)") + return ReviewContext( + text="\n\n".join(sections), + included_files=tuple(included), + skipped_files=tuple(skipped), + truncated_files=tuple(truncated), + diagnostics=tuple(diagnostics), + ) + + +def _is_excluded(path: str, patterns: tuple[str, ...]) -> bool: + normalized = path.replace("\\", "/") + return any(fnmatch(normalized, pattern) or fnmatch(f"/{normalized}", pattern) for pattern in patterns) + + +def _render_added_line_map(changed_file: ChangedFile) -> str: + lines = [ + f"L{line.new_line}: {line.content}" for hunk in changed_file.hunks for line in hunk.lines + if line.change_type == LineChangeType.ADDED and line.new_line is not None + ] + return "\n".join(lines) if lines else "(no added lines)" diff --git a/examples/code_review_agent/code_review/database.py b/examples/code_review_agent/code_review/database.py new file mode 100644 index 000000000..1bed1aa84 --- /dev/null +++ b/examples/code_review_agent/code_review/database.py @@ -0,0 +1,993 @@ +"""SQLAlchemy persistence for code review runs and their child records.""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime, timedelta, timezone +from hashlib import sha256 +from pathlib import Path +from typing import Any + +from sqlalchemy import ( + JSON, + Boolean, + DateTime, + Float, + ForeignKey, + Index, + Integer, + String, + Text, + UniqueConstraint, + create_engine, + func, + select, + text, + update, +) +from sqlalchemy.engine import Engine, make_url +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import ( + DeclarativeBase, + Mapped, + Session, + mapped_column, + relationship, + selectinload, +) + +from .models import ( + AnalyzerExecution, + AnalyzerStatus, + ChangedFile, + ChangeType, + Finding, + ReviewOutput, + ReviewRun, + ReviewStatus, + Severity, +) + +SCHEMA_VERSION = 4 +_UNSET = object() + + +class ReviewDatabaseBase(DeclarativeBase): + """Dedicated metadata for code review tables.""" + + +class ReviewSchemaVersion(ReviewDatabaseBase): + """Single-row schema marker for explicit future migrations.""" + + __tablename__ = "code_review_schema_version" + + id: Mapped[int] = mapped_column(Integer, primary_key=True, default=1) + version: Mapped[int] = mapped_column(Integer, nullable=False) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + ) + + +class ReviewRunRecord(ReviewDatabaseBase): + """Top-level persisted review execution.""" + + __tablename__ = "code_review_runs" + __table_args__ = ( + UniqueConstraint("idempotency_key", name="uq_code_review_runs_idempotency"), + Index("ix_code_review_runs_repository_started", "repository_path_hash", "started_at"), + Index("ix_code_review_runs_status_started", "status", "started_at"), + ) + + id: Mapped[str] = mapped_column(String(64), primary_key=True) + idempotency_key: Mapped[str] = mapped_column(String(64), nullable=False) + repository_path: Mapped[str] = mapped_column(Text, nullable=False) + repository_path_hash: Mapped[str] = mapped_column(String(64), nullable=False) + base_revision: Mapped[str] = mapped_column(String(255), nullable=False) + head_revision: Mapped[str] = mapped_column(String(255), nullable=False) + resolved_head_revision: Mapped[str] = mapped_column(String(64), nullable=False, default="") + effective_base_revision: Mapped[str] = mapped_column(String(64), nullable=False, default="") + status: Mapped[str] = mapped_column(String(32), nullable=False) + model_name: Mapped[str] = mapped_column(String(255), nullable=False, default="") + config_hash: Mapped[str] = mapped_column(String(64), nullable=False) + started_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + finished_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + error_message: Mapped[str] = mapped_column(Text, nullable=False, default="") + static_analysis_requested: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + summary: Mapped[str] = mapped_column(Text, nullable=False, default="") + diagnostics: Mapped[list[str]] = mapped_column(JSON, nullable=False, default=list) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + ) + + changed_files: Mapped[list[ChangedFileRecord]] = relationship( + back_populates="review_run", + cascade="all, delete-orphan", + order_by="ChangedFileRecord.position", + ) + findings: Mapped[list[FindingRecord]] = relationship( + back_populates="review_run", + cascade="all, delete-orphan", + order_by="FindingRecord.position", + ) + analyzer_executions: Mapped[list[AnalyzerExecutionRecord]] = relationship( + back_populates="review_run", + cascade="all, delete-orphan", + order_by="AnalyzerExecutionRecord.position", + ) + + +class ChangedFileRecord(ReviewDatabaseBase): + """One changed file belonging to a review run.""" + + __tablename__ = "code_review_changed_files" + __table_args__ = ( + UniqueConstraint("review_run_id", "position", name="uq_code_review_changed_file_position"), + Index("ix_code_review_changed_files_run", "review_run_id"), + ) + + id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + review_run_id: Mapped[str] = mapped_column( + ForeignKey("code_review_runs.id", ondelete="CASCADE"), + nullable=False, + ) + position: Mapped[int] = mapped_column(Integer, nullable=False) + path: Mapped[str] = mapped_column(Text, nullable=False) + old_path: Mapped[str | None] = mapped_column(Text) + change_type: Mapped[str] = mapped_column(String(32), nullable=False) + language: Mapped[str] = mapped_column(String(64), nullable=False) + patch: Mapped[str] = mapped_column(Text, nullable=False, default="") + hunks: Mapped[list[dict[str, Any]]] = mapped_column(JSON, nullable=False, default=list) + added_lines: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + deleted_lines: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + is_binary: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + is_truncated: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + + review_run: Mapped[ReviewRunRecord] = relationship(back_populates="changed_files") + + +class FindingRecord(ReviewDatabaseBase): + """One normalized finding belonging to a review run.""" + + __tablename__ = "code_review_findings" + __table_args__ = ( + UniqueConstraint("review_run_id", "position", name="uq_code_review_finding_position"), + Index("ix_code_review_findings_location", "review_run_id", "start_line"), + Index("ix_code_review_findings_publishable", "review_run_id", "publishable"), + ) + + id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + review_run_id: Mapped[str] = mapped_column( + ForeignKey("code_review_runs.id", ondelete="CASCADE"), + nullable=False, + ) + position: Mapped[int] = mapped_column(Integer, nullable=False) + rule_id: Mapped[str] = mapped_column(String(160), nullable=False) + severity: Mapped[str] = mapped_column(String(32), nullable=False) + confidence: Mapped[float] = mapped_column(Float, nullable=False) + category: Mapped[str] = mapped_column(String(80), nullable=False) + file_path: Mapped[str] = mapped_column(Text, nullable=False) + start_line: Mapped[int | None] = mapped_column(Integer) + end_line: Mapped[int | None] = mapped_column(Integer) + title: Mapped[str] = mapped_column(String(240), nullable=False) + description: Mapped[str] = mapped_column(Text, nullable=False) + suggestion: Mapped[str] = mapped_column(Text, nullable=False, default="") + source: Mapped[str] = mapped_column(String(80), nullable=False) + publishable: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + + review_run: Mapped[ReviewRunRecord] = relationship(back_populates="findings") + + +class AnalyzerExecutionRecord(ReviewDatabaseBase): + """One deterministic analyzer invocation belonging to a review run.""" + + __tablename__ = "code_review_analyzer_executions" + __table_args__ = ( + UniqueConstraint("review_run_id", "position", name="uq_code_review_analyzer_position"), + Index("ix_code_review_analyzers_tool_status", "tool", "status"), + ) + + id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + review_run_id: Mapped[str] = mapped_column( + ForeignKey("code_review_runs.id", ondelete="CASCADE"), + nullable=False, + ) + position: Mapped[int] = mapped_column(Integer, nullable=False) + tool: Mapped[str] = mapped_column(String(80), nullable=False) + runtime: Mapped[str] = mapped_column(String(32), nullable=False) + command: Mapped[list[str]] = mapped_column(JSON, nullable=False, default=list) + status: Mapped[str] = mapped_column(String(32), nullable=False) + exit_code: Mapped[int | None] = mapped_column(Integer) + duration_seconds: Mapped[float] = mapped_column(Float, nullable=False, default=0.0) + stdout: Mapped[str] = mapped_column(Text, nullable=False, default="") + stderr: Mapped[str] = mapped_column(Text, nullable=False, default="") + findings_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + + review_run: Mapped[ReviewRunRecord] = relationship(back_populates="analyzer_executions") + + +class GitHubDeliveryRecord(ReviewDatabaseBase): + """Lifecycle and publication state for one GitHub webhook delivery.""" + + __tablename__ = "code_review_github_deliveries" + __table_args__ = ( + Index("ix_code_review_github_delivery_repository", "repository_full_name", "received_at"), + Index("ix_code_review_github_delivery_status", "status", "updated_at"), + ) + + delivery_id: Mapped[str] = mapped_column(String(128), primary_key=True) + event_name: Mapped[str] = mapped_column(String(80), nullable=False) + action: Mapped[str] = mapped_column(String(80), nullable=False, default="") + payload_sha256: Mapped[str] = mapped_column(String(64), nullable=False) + repository_full_name: Mapped[str] = mapped_column(String(255), nullable=False, default="") + pull_number: Mapped[int | None] = mapped_column(Integer) + head_sha: Mapped[str] = mapped_column(String(64), nullable=False, default="") + installation_id: Mapped[int | None] = mapped_column(Integer) + status: Mapped[str] = mapped_column(String(32), nullable=False, default="received") + review_run_id: Mapped[str | None] = mapped_column(ForeignKey("code_review_runs.id", ondelete="SET NULL"), ) + check_run_id: Mapped[int | None] = mapped_column(Integer) + error_message: Mapped[str] = mapped_column(Text, nullable=False, default="") + received_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + onupdate=lambda: datetime.now(timezone.utc), + ) + + +class GitHubReviewJobRecord(ReviewDatabaseBase): + """Durable queue entry for one accepted GitHub delivery.""" + + __tablename__ = "code_review_github_jobs" + __table_args__ = ( + Index("ix_code_review_github_jobs_available", "status", "available_at"), + Index("ix_code_review_github_jobs_lease", "status", "lease_expires_at"), + ) + + delivery_id: Mapped[str] = mapped_column( + ForeignKey("code_review_github_deliveries.delivery_id", ondelete="CASCADE"), + primary_key=True, + ) + event_payload: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False) + status: Mapped[str] = mapped_column(String(32), nullable=False, default="queued") + attempt_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + max_attempts: Mapped[int] = mapped_column(Integer, nullable=False, default=5) + available_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + lease_owner: Mapped[str] = mapped_column(String(255), nullable=False, default="") + lease_expires_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + last_error: Mapped[str] = mapped_column(Text, nullable=False, default="") + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + onupdate=lambda: datetime.now(timezone.utc), + ) + + +class GitHubPublicationRecord(ReviewDatabaseBase): + """Locally durable progress for retry-safe GitHub publication.""" + + __tablename__ = "code_review_github_publications" + + delivery_id: Mapped[str] = mapped_column( + ForeignKey("code_review_github_deliveries.delivery_id", ondelete="CASCADE"), + primary_key=True, + ) + check_completed: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + comments_completed: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + nullable=False, + default=lambda: datetime.now(timezone.utc), + onupdate=lambda: datetime.now(timezone.utc), + ) + + +@dataclass(frozen=True) +class SaveReviewResult: + """Result of an idempotent save operation.""" + + review_run: ReviewRun + created: bool + + +@dataclass(frozen=True) +class ClaimedReviewJob: + """One queue job currently leased by a worker.""" + + delivery_id: str + event_payload: dict[str, Any] + attempt_count: int + max_attempts: int + lease_owner: str + lease_expires_at: datetime + + +def sqlite_database_url(path: str | Path) -> str: + """Build an absolute SQLite URL from a local database path.""" + database_path = Path(path).expanduser().resolve() + database_path.parent.mkdir(parents=True, exist_ok=True) + return f"sqlite:///{database_path}" + + +class ReviewStore: + """Synchronous SQLAlchemy repository with idempotent inserts.""" + + def __init__(self, database_url: str, *, engine: Engine | None = None): + url = make_url(database_url) + if any(async_driver in url.drivername for async_driver in ("aiosqlite", "aiomysql", "asyncpg")): + raise ValueError("ReviewStore requires a synchronous SQLAlchemy database URL") + self.database_url = database_url + self.engine = engine or create_engine(database_url, future=True) + try: + self.initialize() + except Exception: + if engine is None: + self.engine.dispose() + raise + + def initialize(self) -> None: + """Create tables and advance supported additive schema versions.""" + ReviewDatabaseBase.metadata.create_all(self.engine) + with Session(self.engine) as session, session.begin(): + marker = session.get(ReviewSchemaVersion, 1) + if marker is None: + session.add(ReviewSchemaVersion(id=1, version=SCHEMA_VERSION)) + elif marker.version in {1, 2, 3}: + # Versions 2 through 4 only add tables; create_all performs the DDL. + marker.version = SCHEMA_VERSION + marker.updated_at = datetime.now(timezone.utc) + elif marker.version != SCHEMA_VERSION: + raise RuntimeError( + f"Unsupported code review schema version {marker.version}; expected {SCHEMA_VERSION}") + + def save_run(self, review_run: ReviewRun) -> SaveReviewResult: + """Insert a complete run, returning the existing row on an idempotency hit.""" + if not review_run.idempotency_key: + raise ValueError("review run must have an idempotency key before persistence") + try: + with Session(self.engine) as session, session.begin(): + existing = self._select_by_idempotency(session, review_run.idempotency_key) + if existing is not None: + return SaveReviewResult(review_run=_record_to_model(existing), created=False) + record = _model_to_record(review_run) + session.add(record) + session.flush() + persisted = _record_to_model(record) + return SaveReviewResult(review_run=persisted, created=True) + except IntegrityError: + # Another worker may win the insert between our lookup and flush. + existing = self.get_by_idempotency_key(review_run.idempotency_key) + if existing is None: + raise + return SaveReviewResult(review_run=existing, created=False) + + def get_run(self, run_id: str) -> ReviewRun | None: + """Load one complete review run by ID.""" + with Session(self.engine) as session: + statement = _run_query().where(ReviewRunRecord.id == run_id) + record = session.scalar(statement) + return _record_to_model(record) if record is not None else None + + def get_by_idempotency_key(self, idempotency_key: str) -> ReviewRun | None: + """Load one complete review run by its deterministic key.""" + with Session(self.engine) as session: + record = self._select_by_idempotency(session, idempotency_key) + return _record_to_model(record) if record is not None else None + + def list_runs( + self, + *, + limit: int = 20, + repository_path: str | None = None, + status: ReviewStatus | None = None, + ) -> list[ReviewRun]: + """List recent complete records with optional repository/status filters.""" + if not 1 <= limit <= 500: + raise ValueError("limit must be between 1 and 500") + statement = _run_query().order_by(ReviewRunRecord.started_at.desc()).limit(limit) + if repository_path is not None: + normalized_path = (repository_path if "://" in repository_path else str( + Path(repository_path).expanduser().resolve())) + statement = statement.where( + ReviewRunRecord.repository_path_hash == _path_hash(normalized_path), + ReviewRunRecord.repository_path == normalized_path, + ) + if status is not None: + statement = statement.where(ReviewRunRecord.status == status.value) + with Session(self.engine) as session: + return [_record_to_model(record) for record in session.scalars(statement).all()] + + def count_runs(self) -> int: + """Return the number of top-level persisted runs.""" + with Session(self.engine) as session: + return int(session.scalar(select(func.count()).select_from(ReviewRunRecord)) or 0) + + def ping(self) -> None: + """Raise when the configured database cannot serve a trivial query.""" + with self.engine.connect() as connection: + connection.execute(text("SELECT 1")) + + def claim_github_delivery( + self, + *, + delivery_id: str, + event_name: str, + action: str, + payload_sha256: str, + repository_full_name: str = "", + pull_number: int | None = None, + head_sha: str = "", + installation_id: int | None = None, + ) -> bool: + """Atomically claim a delivery; return false when it was already recorded.""" + record = GitHubDeliveryRecord( + delivery_id=delivery_id, + event_name=event_name, + action=action, + payload_sha256=payload_sha256, + repository_full_name=repository_full_name, + pull_number=pull_number, + head_sha=head_sha, + installation_id=installation_id, + ) + try: + with Session(self.engine) as session, session.begin(): + session.add(record) + session.flush() + return True + except IntegrityError: + return False + + def enqueue_github_delivery( + self, + *, + delivery_id: str, + event_name: str, + action: str, + payload_sha256: str, + event_payload: dict[str, Any], + repository_full_name: str, + pull_number: int, + head_sha: str, + installation_id: int, + max_attempts: int = 5, + ) -> bool: + """Atomically record a delivery and enqueue its validated event.""" + if max_attempts <= 0: + raise ValueError("max_attempts must be positive") + now = datetime.now(timezone.utc) + try: + with Session(self.engine) as session, session.begin(): + session.add( + GitHubDeliveryRecord( + delivery_id=delivery_id, + event_name=event_name, + action=action, + payload_sha256=payload_sha256, + repository_full_name=repository_full_name, + pull_number=pull_number, + head_sha=head_sha, + installation_id=installation_id, + status="queued", + ) + ) + session.add( + GitHubReviewJobRecord( + delivery_id=delivery_id, + event_payload=event_payload, + status="queued", + max_attempts=max_attempts, + available_at=now, + ) + ) + session.flush() + return True + except IntegrityError: + return False + + def claim_github_job( + self, + *, + worker_id: str, + lease_seconds: float = 300, + ) -> ClaimedReviewJob | None: + """Lease the oldest available job using a portable compare-and-set.""" + if not worker_id or len(worker_id) > 255: + raise ValueError("worker_id must contain 1 to 255 characters") + if lease_seconds <= 0: + raise ValueError("lease_seconds must be positive") + now = datetime.now(timezone.utc) + lease_expires_at = now + timedelta(seconds=lease_seconds) + for _ in range(10): + with Session(self.engine) as session, session.begin(): + self._recover_expired_jobs(session, now) + candidate = session.scalar( + select(GitHubReviewJobRecord.delivery_id) + .where( + GitHubReviewJobRecord.status == "queued", + GitHubReviewJobRecord.available_at <= now, + ) + .order_by( + GitHubReviewJobRecord.available_at, + GitHubReviewJobRecord.created_at, + ) + .limit(1) + ) + if candidate is None: + return None + result = session.execute( + update(GitHubReviewJobRecord) + .where( + GitHubReviewJobRecord.delivery_id == candidate, + GitHubReviewJobRecord.status == "queued", + GitHubReviewJobRecord.available_at <= now, + ) + .values( + status="leased", + attempt_count=GitHubReviewJobRecord.attempt_count + 1, + lease_owner=worker_id, + lease_expires_at=lease_expires_at, + updated_at=now, + ) + ) + if result.rowcount != 1: + continue + record = session.get(GitHubReviewJobRecord, candidate) + session.get(GitHubDeliveryRecord, candidate).status = "processing" + return ClaimedReviewJob( + delivery_id=record.delivery_id, + event_payload=dict(record.event_payload), + attempt_count=record.attempt_count, + max_attempts=record.max_attempts, + lease_owner=record.lease_owner, + lease_expires_at=_as_utc(record.lease_expires_at), + ) + return None + + def renew_github_job_lease( + self, + delivery_id: str, + *, + worker_id: str, + lease_seconds: float = 300, + ) -> bool: + """Extend an active lease, returning false if ownership was lost.""" + if lease_seconds <= 0: + raise ValueError("lease_seconds must be positive") + now = datetime.now(timezone.utc) + with Session(self.engine) as session, session.begin(): + result = session.execute( + update(GitHubReviewJobRecord) + .where( + GitHubReviewJobRecord.delivery_id == delivery_id, + GitHubReviewJobRecord.status == "leased", + GitHubReviewJobRecord.lease_owner == worker_id, + GitHubReviewJobRecord.lease_expires_at > now, + ) + .values( + lease_expires_at=now + timedelta(seconds=lease_seconds), + updated_at=now, + ) + ) + return result.rowcount == 1 + + def complete_github_job(self, delivery_id: str, *, worker_id: str) -> bool: + """Mark an owned lease successful.""" + now = datetime.now(timezone.utc) + with Session(self.engine) as session, session.begin(): + result = session.execute( + update(GitHubReviewJobRecord) + .where( + GitHubReviewJobRecord.delivery_id == delivery_id, + GitHubReviewJobRecord.status == "leased", + GitHubReviewJobRecord.lease_owner == worker_id, + ) + .values( + status="succeeded", + lease_owner="", + lease_expires_at=None, + last_error="", + updated_at=now, + ) + ) + return result.rowcount == 1 + + def fail_github_job( + self, + delivery_id: str, + *, + worker_id: str, + error_message: str, + retry_delay_seconds: float, + retryable: bool = True, + ) -> str: + """Retry or permanently fail an owned job and return its new status.""" + if retry_delay_seconds < 0: + raise ValueError("retry_delay_seconds cannot be negative") + now = datetime.now(timezone.utc) + with Session(self.engine) as session, session.begin(): + record = session.get(GitHubReviewJobRecord, delivery_id) + if record is None: + raise KeyError(f"Unknown GitHub review job: {delivery_id}") + if record.status != "leased" or record.lease_owner != worker_id: + raise RuntimeError(f"GitHub review job lease was lost: {delivery_id}") + should_retry = retryable and record.attempt_count < record.max_attempts + record.status = "queued" if should_retry else "dead" + record.available_at = now + timedelta(seconds=retry_delay_seconds) + record.lease_owner = "" + record.lease_expires_at = None + record.last_error = error_message[:10_000] + record.updated_at = now + delivery = session.get(GitHubDeliveryRecord, delivery_id) + delivery.status = "retrying" if should_retry else "failed" + delivery.error_message = record.last_error + delivery.updated_at = now + return record.status + + def replay_github_job(self, delivery_id: str, *, reset_attempts: bool = True) -> bool: + """Requeue a dead job for explicit operator-controlled replay.""" + now = datetime.now(timezone.utc) + with Session(self.engine) as session, session.begin(): + record = session.get(GitHubReviewJobRecord, delivery_id) + if record is None or record.status != "dead": + return False + record.status = "queued" + if reset_attempts: + record.attempt_count = 0 + record.available_at = now + record.lease_owner = "" + record.lease_expires_at = None + record.last_error = "" + record.updated_at = now + delivery = session.get(GitHubDeliveryRecord, delivery_id) + delivery.status = "queued" + delivery.error_message = "" + delivery.updated_at = now + return True + + def get_github_job(self, delivery_id: str) -> dict[str, Any] | None: + """Return a JSON-friendly durable queue record.""" + with Session(self.engine) as session: + record = session.get(GitHubReviewJobRecord, delivery_id) + return _job_to_dict(record) if record is not None else None + + def list_github_jobs( + self, + *, + limit: int = 20, + status: str | None = None, + ) -> list[dict[str, Any]]: + """List recent durable jobs.""" + if not 1 <= limit <= 500: + raise ValueError("limit must be between 1 and 500") + statement = ( + select(GitHubReviewJobRecord) + .order_by(GitHubReviewJobRecord.created_at.desc()) + .limit(limit) + ) + if status is not None: + statement = statement.where(GitHubReviewJobRecord.status == status) + with Session(self.engine) as session: + return [_job_to_dict(record) for record in session.scalars(statement).all()] + + def get_github_publication(self, delivery_id: str) -> dict[str, Any]: + """Return retry-safe publication progress, creating it when absent.""" + with Session(self.engine) as session, session.begin(): + record = session.get(GitHubPublicationRecord, delivery_id) + if record is None: + if session.get(GitHubDeliveryRecord, delivery_id) is None: + raise KeyError(f"Unknown GitHub delivery: {delivery_id}") + record = GitHubPublicationRecord(delivery_id=delivery_id) + session.add(record) + session.flush() + return { + "delivery_id": record.delivery_id, + "check_completed": record.check_completed, + "comments_completed": record.comments_completed, + "updated_at": _as_utc(record.updated_at), + } + + def update_github_publication( + self, + delivery_id: str, + *, + check_completed: bool | None = None, + comments_completed: bool | None = None, + ) -> None: + """Persist completed publication phases without resetting prior progress.""" + now = datetime.now(timezone.utc) + with Session(self.engine) as session, session.begin(): + record = session.get(GitHubPublicationRecord, delivery_id) + if record is None: + if session.get(GitHubDeliveryRecord, delivery_id) is None: + raise KeyError(f"Unknown GitHub delivery: {delivery_id}") + record = GitHubPublicationRecord(delivery_id=delivery_id) + session.add(record) + if check_completed is not None: + record.check_completed = check_completed + if comments_completed is not None: + record.comments_completed = comments_completed + record.updated_at = now + + def update_github_delivery( + self, + delivery_id: str, + *, + status: str, + review_run_id: str | None | object = _UNSET, + check_run_id: int | None | object = _UNSET, + error_message: str | object = _UNSET, + ) -> None: + """Update processing/publication state for an already claimed delivery.""" + with Session(self.engine) as session, session.begin(): + record = session.get(GitHubDeliveryRecord, delivery_id) + if record is None: + raise KeyError(f"Unknown GitHub delivery: {delivery_id}") + record.status = status + if review_run_id is not _UNSET: + record.review_run_id = review_run_id + if check_run_id is not _UNSET: + record.check_run_id = check_run_id + if error_message is not _UNSET: + record.error_message = str(error_message) + record.updated_at = datetime.now(timezone.utc) + + def get_github_delivery(self, delivery_id: str) -> dict[str, Any] | None: + """Return a JSON-friendly delivery record for diagnostics and tests.""" + with Session(self.engine) as session: + record = session.get(GitHubDeliveryRecord, delivery_id) + if record is None: + return None + return _delivery_to_dict(record) + + def list_github_deliveries( + self, + *, + limit: int = 20, + status: str | None = None, + repository_full_name: str | None = None, + ) -> list[dict[str, Any]]: + """List recent webhook deliveries without loading review object graphs.""" + if not 1 <= limit <= 500: + raise ValueError("limit must be between 1 and 500") + statement = select(GitHubDeliveryRecord).order_by(GitHubDeliveryRecord.received_at.desc()).limit(limit) + if status is not None: + statement = statement.where(GitHubDeliveryRecord.status == status) + if repository_full_name is not None: + statement = statement.where(GitHubDeliveryRecord.repository_full_name == repository_full_name) + with Session(self.engine) as session: + return [_delivery_to_dict(record) for record in session.scalars(statement).all()] + + def close(self) -> None: + """Release database connections.""" + self.engine.dispose() + + @staticmethod + def _select_by_idempotency(session: Session, idempotency_key: str) -> ReviewRunRecord | None: + return session.scalar(_run_query().where(ReviewRunRecord.idempotency_key == idempotency_key)) + + @staticmethod + def _recover_expired_jobs(session: Session, now: datetime) -> None: + expired = list( + session.scalars( + select(GitHubReviewJobRecord).where( + GitHubReviewJobRecord.status == "leased", + GitHubReviewJobRecord.lease_expires_at <= now, + ) + ) + ) + for record in expired: + record.status = "queued" + record.available_at = now + record.lease_owner = "" + record.lease_expires_at = None + record.last_error = "Worker lease expired; job recovered" + record.updated_at = now + delivery = session.get(GitHubDeliveryRecord, record.delivery_id) + delivery.status = "retrying" + delivery.error_message = record.last_error + delivery.updated_at = now + + +def _run_query(): + return select(ReviewRunRecord).options( + selectinload(ReviewRunRecord.changed_files), + selectinload(ReviewRunRecord.findings), + selectinload(ReviewRunRecord.analyzer_executions), + ) + + +def _model_to_record(review_run: ReviewRun) -> ReviewRunRecord: + record = ReviewRunRecord( + id=review_run.id, + idempotency_key=review_run.idempotency_key, + repository_path=review_run.repository_path, + repository_path_hash=_path_hash(review_run.repository_path), + base_revision=review_run.base_revision, + head_revision=review_run.head_revision, + resolved_head_revision=review_run.resolved_head_revision, + effective_base_revision=review_run.effective_base_revision, + status=review_run.status.value, + model_name=review_run.model_name, + config_hash=review_run.config_hash, + started_at=review_run.started_at, + finished_at=review_run.finished_at, + error_message=review_run.error_message, + static_analysis_requested=review_run.static_analysis_requested, + summary=review_run.output.summary, + diagnostics=list(review_run.diagnostics), + ) + record.changed_files = [ + ChangedFileRecord( + position=position, + path=changed_file.path, + old_path=changed_file.old_path, + change_type=changed_file.change_type.value, + language=changed_file.language, + patch=changed_file.patch, + hunks=[hunk.model_dump(mode="json") for hunk in changed_file.hunks], + added_lines=changed_file.added_lines, + deleted_lines=changed_file.deleted_lines, + is_binary=changed_file.is_binary, + is_truncated=changed_file.is_truncated, + ) for position, changed_file in enumerate(review_run.changed_files) + ] + record.findings = [ + FindingRecord( + position=position, + rule_id=finding.rule_id, + severity=finding.severity.value, + confidence=finding.confidence, + category=finding.category, + file_path=finding.file_path, + start_line=finding.start_line, + end_line=finding.end_line, + title=finding.title, + description=finding.description, + suggestion=finding.suggestion, + source=finding.source, + publishable=finding.publishable, + ) for position, finding in enumerate(review_run.output.findings) + ] + record.analyzer_executions = [ + AnalyzerExecutionRecord( + position=position, + tool=execution.tool, + runtime=execution.runtime, + command=list(execution.command), + status=execution.status.value, + exit_code=execution.exit_code, + duration_seconds=execution.duration_seconds, + stdout=execution.stdout, + stderr=execution.stderr, + findings_count=execution.findings_count, + ) for position, execution in enumerate(review_run.analyzer_executions) + ] + return record + + +def _record_to_model(record: ReviewRunRecord) -> ReviewRun: + return ReviewRun( + id=record.id, + idempotency_key=record.idempotency_key, + repository_path=record.repository_path, + base_revision=record.base_revision, + head_revision=record.head_revision, + resolved_head_revision=record.resolved_head_revision, + effective_base_revision=record.effective_base_revision, + status=ReviewStatus(record.status), + model_name=record.model_name, + config_hash=record.config_hash, + started_at=_as_utc(record.started_at), + finished_at=_as_utc(record.finished_at), + error_message=record.error_message, + static_analysis_requested=record.static_analysis_requested, + changed_files=[ + ChangedFile( + path=changed_file.path, + old_path=changed_file.old_path, + change_type=ChangeType(changed_file.change_type), + language=changed_file.language, + patch=changed_file.patch, + hunks=changed_file.hunks, + added_lines=changed_file.added_lines, + deleted_lines=changed_file.deleted_lines, + is_binary=changed_file.is_binary, + is_truncated=changed_file.is_truncated, + ) for changed_file in record.changed_files + ], + analyzer_executions=[ + AnalyzerExecution( + tool=execution.tool, + runtime=execution.runtime, + command=execution.command, + status=AnalyzerStatus(execution.status), + exit_code=execution.exit_code, + duration_seconds=execution.duration_seconds, + stdout=execution.stdout, + stderr=execution.stderr, + findings_count=execution.findings_count, + ) for execution in record.analyzer_executions + ], + output=ReviewOutput( + summary=record.summary, + findings=[ + Finding( + rule_id=finding.rule_id, + severity=Severity(finding.severity), + confidence=finding.confidence, + category=finding.category, + file_path=finding.file_path, + start_line=finding.start_line, + end_line=finding.end_line, + title=finding.title, + description=finding.description, + suggestion=finding.suggestion, + source=finding.source, + publishable=finding.publishable, + ) for finding in record.findings + ], + ), + diagnostics=list(record.diagnostics or []), + ) + + +def _as_utc(value: datetime | None) -> datetime | None: + if value is None: + return None + if value.tzinfo is None: + return value.replace(tzinfo=timezone.utc) + return value.astimezone(timezone.utc) + + +def _path_hash(path: str) -> str: + return sha256(path.encode("utf-8")).hexdigest() + + +def _delivery_to_dict(record: GitHubDeliveryRecord) -> dict[str, Any]: + return { + "delivery_id": record.delivery_id, + "event_name": record.event_name, + "action": record.action, + "payload_sha256": record.payload_sha256, + "repository_full_name": record.repository_full_name, + "pull_number": record.pull_number, + "head_sha": record.head_sha, + "installation_id": record.installation_id, + "status": record.status, + "review_run_id": record.review_run_id, + "check_run_id": record.check_run_id, + "error_message": record.error_message, + "received_at": _as_utc(record.received_at), + "updated_at": _as_utc(record.updated_at), + } + + +def _job_to_dict(record: GitHubReviewJobRecord) -> dict[str, Any]: + return { + "delivery_id": record.delivery_id, + "event_payload": dict(record.event_payload), + "status": record.status, + "attempt_count": record.attempt_count, + "max_attempts": record.max_attempts, + "available_at": _as_utc(record.available_at), + "lease_owner": record.lease_owner, + "lease_expires_at": _as_utc(record.lease_expires_at), + "last_error": record.last_error, + "created_at": _as_utc(record.created_at), + "updated_at": _as_utc(record.updated_at), + } diff --git a/examples/code_review_agent/code_review/git_diff.py b/examples/code_review_agent/code_review/git_diff.py new file mode 100644 index 000000000..8bc593d19 --- /dev/null +++ b/examples/code_review_agent/code_review/git_diff.py @@ -0,0 +1,250 @@ +"""Safe, deterministic Git diff collection and parsing.""" + +from __future__ import annotations + +import re +import subprocess +from dataclasses import dataclass +from pathlib import Path + +from .models import ChangedFile, ChangedLine, ChangeType, DiffHunk, LineChangeType + +_HUNK_HEADER = re.compile(r"^@@ -(?P\d+)(?:,(?P\d+))? " + r"\+(?P\d+)(?:,(?P\d+))? @@(?P
.*)$") + +_LANGUAGES = { + ".bash": "shell", + ".c": "c", + ".cc": "cpp", + ".cpp": "cpp", + ".css": "css", + ".go": "go", + ".h": "c", + ".hpp": "cpp", + ".html": "html", + ".java": "java", + ".js": "javascript", + ".json": "json", + ".jsx": "javascript", + ".kt": "kotlin", + ".md": "markdown", + ".php": "php", + ".py": "python", + ".rb": "ruby", + ".rs": "rust", + ".sh": "shell", + ".sql": "sql", + ".swift": "swift", + ".toml": "toml", + ".ts": "typescript", + ".tsx": "typescript", + ".xml": "xml", + ".yaml": "yaml", + ".yml": "yaml", +} + +_STATUS_TYPES = { + "A": ChangeType.ADDED, + "M": ChangeType.MODIFIED, + "D": ChangeType.DELETED, + "R": ChangeType.RENAMED, + "C": ChangeType.COPIED, + "T": ChangeType.TYPE_CHANGED, + "U": ChangeType.UNMERGED, +} + + +class GitDiffError(RuntimeError): + """Raised when repository validation or Git collection fails.""" + + +@dataclass(frozen=True) +class _FileStatus: + status: str + path: str + old_path: str | None = None + + +class GitDiffCollector: + """Collect changes between two commits without invoking a shell.""" + + def __init__(self, repository: str | Path, *, context_lines: int = 3): + self.repository = Path(repository).expanduser().resolve() + self.context_lines = max(0, context_lines) + self._validate_repository() + + def collect( + self, + base_revision: str, + head_revision: str, + *, + use_merge_base: bool = True, + ) -> tuple[str, list[ChangedFile]]: + """Validate revisions and collect one parsed patch per changed file.""" + base_commit = self.resolve_revision(base_revision) + head_commit = self.resolve_revision(head_revision) + effective_base = self.merge_base(base_commit, head_commit) if use_merge_base else base_commit + statuses = self._collect_statuses(effective_base, head_commit) + changed_files = [self._collect_file(effective_base, head_commit, status) for status in statuses] + return effective_base, changed_files + + def resolve_revision(self, revision: str) -> str: + """Resolve a user-provided revision to a full commit hash.""" + if not revision or revision.startswith("-"): + raise GitDiffError(f"Invalid Git revision: {revision!r}") + return self._git("rev-parse", "--verify", f"{revision}^{{commit}}").strip() + + def merge_base(self, base_commit: str, head_commit: str) -> str: + """Resolve the merge base used by pull-request-style comparisons.""" + result = self._git("merge-base", base_commit, head_commit).strip() + if not result: + raise GitDiffError("The revisions do not have a merge base") + return result + + def _validate_repository(self) -> None: + if not self.repository.is_dir(): + raise GitDiffError(f"Repository directory does not exist: {self.repository}") + inside = self._git("rev-parse", "--is-inside-work-tree").strip() + if inside != "true": + raise GitDiffError(f"Not a Git work tree: {self.repository}") + + def _git_bytes(self, *args: str) -> bytes: + command = ["git", "-C", str(self.repository), *args] + try: + result = subprocess.run( + command, + check=False, + capture_output=True, + ) + except OSError as exc: + raise GitDiffError(f"Unable to execute Git: {exc}") from exc + if result.returncode != 0: + message = result.stderr.decode("utf-8", errors="replace").strip() + raise GitDiffError(message or f"Git command failed with exit code {result.returncode}") + return result.stdout + + def _git(self, *args: str) -> str: + return self._git_bytes(*args).decode("utf-8", errors="replace") + + def _collect_statuses(self, base_commit: str, head_commit: str) -> list[_FileStatus]: + raw = self._git_bytes( + "diff", + "--name-status", + "-z", + "--find-renames", + "--find-copies", + base_commit, + head_commit, + "--", + ) + tokens = raw.decode("utf-8", errors="surrogateescape").split("\0") + statuses: list[_FileStatus] = [] + index = 0 + while index < len(tokens) - 1: + status = tokens[index] + index += 1 + if not status: + continue + code = status[0] + if code in {"R", "C"}: + if index + 1 >= len(tokens): + raise GitDiffError("Malformed rename/copy record from git diff") + old_path, path = tokens[index], tokens[index + 1] + index += 2 + statuses.append(_FileStatus(status=status, path=path, old_path=old_path)) + else: + if index >= len(tokens): + raise GitDiffError("Malformed file record from git diff") + path = tokens[index] + index += 1 + statuses.append(_FileStatus(status=status, path=path)) + return statuses + + def _collect_file(self, base_commit: str, head_commit: str, status: _FileStatus) -> ChangedFile: + paths = [status.path] + if status.old_path: + paths.insert(0, status.old_path) + patch = self._git( + "diff", + "--no-ext-diff", + "--no-color", + f"--unified={self.context_lines}", + "--find-renames", + base_commit, + head_commit, + "--", + *paths, + ) + hunks, added, deleted, is_binary = parse_unified_patch(patch) + return ChangedFile( + path=status.path, + old_path=status.old_path, + change_type=_STATUS_TYPES.get(status.status[0], ChangeType.UNKNOWN), + language=detect_language(status.path), + patch=patch, + hunks=hunks, + added_lines=added, + deleted_lines=deleted, + is_binary=is_binary, + ) + + +def detect_language(path: str) -> str: + """Infer a useful language label from a file suffix.""" + filename = Path(path).name.lower() + if filename in {"dockerfile", "containerfile"}: + return "dockerfile" + if filename in {"makefile", "gnumakefile"}: + return "makefile" + return _LANGUAGES.get(Path(filename).suffix, "text") + + +def parse_unified_patch(patch: str) -> tuple[list[DiffHunk], int, int, bool]: + """Parse hunk line numbers while retaining the original patch.""" + is_binary = "GIT binary patch" in patch or "Binary files " in patch + hunks: list[DiffHunk] = [] + current: DiffHunk | None = None + old_line = 0 + new_line = 0 + added = 0 + deleted = 0 + + for raw_line in patch.splitlines(): + match = _HUNK_HEADER.match(raw_line) + if match: + current = DiffHunk( + header=raw_line, + old_start=int(match.group("old_start")), + old_count=int(match.group("old_count") or "1"), + new_start=int(match.group("new_start")), + new_count=int(match.group("new_count") or "1"), + section=match.group("section").strip(), + ) + hunks.append(current) + old_line = current.old_start + new_line = current.new_start + continue + if current is None or not raw_line: + continue + if raw_line.startswith("\\ No newline at end of file"): + continue + prefix, content = raw_line[0], raw_line[1:] + if prefix == "+": + current.lines.append(ChangedLine(change_type=LineChangeType.ADDED, content=content, new_line=new_line)) + new_line += 1 + added += 1 + elif prefix == "-": + current.lines.append(ChangedLine(change_type=LineChangeType.DELETED, content=content, old_line=old_line)) + old_line += 1 + deleted += 1 + elif prefix == " ": + current.lines.append( + ChangedLine( + change_type=LineChangeType.CONTEXT, + content=content, + old_line=old_line, + new_line=new_line, + )) + old_line += 1 + new_line += 1 + return hunks, added, deleted, is_binary diff --git a/examples/code_review_agent/code_review/models.py b/examples/code_review_agent/code_review/models.py new file mode 100644 index 000000000..b42f7190b --- /dev/null +++ b/examples/code_review_agent/code_review/models.py @@ -0,0 +1,196 @@ +"""Domain models for the code review example.""" + +from __future__ import annotations + +import json +from datetime import datetime, timezone +from enum import Enum +from hashlib import sha256 +from typing import Any + +from pydantic import BaseModel, Field, field_validator, model_validator + + +class ChangeType(str, Enum): + """Git file change type.""" + + ADDED = "added" + MODIFIED = "modified" + DELETED = "deleted" + RENAMED = "renamed" + COPIED = "copied" + TYPE_CHANGED = "type_changed" + UNMERGED = "unmerged" + UNKNOWN = "unknown" + + +class LineChangeType(str, Enum): + """Unified diff line type.""" + + CONTEXT = "context" + ADDED = "added" + DELETED = "deleted" + + +class Severity(str, Enum): + """Finding severity from most to least urgent.""" + + CRITICAL = "critical" + HIGH = "high" + MEDIUM = "medium" + LOW = "low" + INFO = "info" + + +class ReviewStatus(str, Enum): + """Lifecycle of a local review run.""" + + PENDING = "pending" + RUNNING = "running" + COMPLETED = "completed" + FAILED = "failed" + + +class AnalyzerStatus(str, Enum): + """Outcome of one deterministic analyzer invocation.""" + + SUCCESS = "success" + FINDINGS = "findings" + FAILED = "failed" + UNAVAILABLE = "unavailable" + TIMED_OUT = "timed_out" + + +class ChangedLine(BaseModel): + """One line in a unified diff hunk.""" + + change_type: LineChangeType + content: str + old_line: int | None = None + new_line: int | None = None + + +class DiffHunk(BaseModel): + """A parsed unified diff hunk.""" + + header: str + old_start: int + old_count: int + new_start: int + new_count: int + section: str = "" + lines: list[ChangedLine] = Field(default_factory=list) + + +class ChangedFile(BaseModel): + """A changed file and its parsed patch.""" + + path: str + old_path: str | None = None + change_type: ChangeType = ChangeType.UNKNOWN + language: str = "text" + patch: str = "" + hunks: list[DiffHunk] = Field(default_factory=list) + added_lines: int = 0 + deleted_lines: int = 0 + is_binary: bool = False + is_truncated: bool = False + + @property + def changed_new_lines(self) -> set[int]: + """Return new-file line numbers introduced by the patch.""" + return { + line.new_line + for hunk in self.hunks + for line in hunk.lines if line.change_type == LineChangeType.ADDED and line.new_line is not None + } + + +class Finding(BaseModel): + """One normalized, machine-consumable code review finding.""" + + rule_id: str = Field(min_length=1, max_length=160) + severity: Severity + confidence: float = Field(ge=0.0, le=1.0) + category: str = Field(min_length=1, max_length=80) + file_path: str = Field(min_length=1) + start_line: int | None = Field(default=None, ge=1) + end_line: int | None = Field(default=None, ge=1) + title: str = Field(min_length=1, max_length=240) + description: str = Field(min_length=1) + suggestion: str = "" + source: str = Field(default="llm", min_length=1, max_length=80) + publishable: bool = False + + @field_validator("rule_id", "category", "file_path", "title", "description", "suggestion", "source") + @classmethod + def strip_text(cls, value: str) -> str: + """Remove accidental leading/trailing whitespace.""" + return value.strip() + + @field_validator("rule_id", "category", "source") + @classmethod + def normalize_labels(cls, value: str) -> str: + """Normalize identifiers used for filtering and deduplication.""" + return value.casefold() + + @model_validator(mode="after") + def validate_line_range(self) -> Finding: + """Keep line ranges internally consistent.""" + if self.end_line is None and self.start_line is not None: + self.end_line = self.start_line + if self.start_line is None and self.end_line is not None: + raise ValueError("end_line requires start_line") + if self.start_line is not None and self.end_line is not None and self.end_line < self.start_line: + raise ValueError("end_line must be greater than or equal to start_line") + return self + + +class ReviewOutput(BaseModel): + """Structured final response produced by the review agent.""" + + summary: str = "" + findings: list[Finding] = Field(default_factory=list) + + +class AnalyzerExecution(BaseModel): + """Auditable result of one static-analysis command.""" + + tool: str + runtime: str + command: list[str] = Field(default_factory=list) + status: AnalyzerStatus + exit_code: int | None = None + duration_seconds: float = 0.0 + stdout: str = "" + stderr: str = "" + findings_count: int = 0 + + +class ReviewRun(BaseModel): + """Metadata and final result for one local review execution.""" + + id: str + repository_path: str + base_revision: str + head_revision: str + resolved_head_revision: str = "" + effective_base_revision: str + status: ReviewStatus = ReviewStatus.PENDING + model_name: str = "" + config_hash: str + idempotency_key: str = "" + started_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc)) + finished_at: datetime | None = None + error_message: str = "" + changed_files: list[ChangedFile] = Field(default_factory=list) + static_analysis_requested: bool = False + analyzer_executions: list[AnalyzerExecution] = Field(default_factory=list) + output: ReviewOutput = Field(default_factory=ReviewOutput) + diagnostics: list[str] = Field(default_factory=list) + + +def stable_config_hash(config: dict[str, Any]) -> str: + """Return a stable digest for idempotency and report traceability.""" + payload = json.dumps(config, ensure_ascii=False, sort_keys=True, separators=(",", ":")) + return sha256(payload.encode("utf-8")).hexdigest() diff --git a/examples/code_review_agent/code_review/orchestrator.py b/examples/code_review_agent/code_review/orchestrator.py new file mode 100644 index 000000000..1bd79befc --- /dev/null +++ b/examples/code_review_agent/code_review/orchestrator.py @@ -0,0 +1,152 @@ +"""End-to-end local review orchestration.""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from dataclasses import asdict, dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Any +from uuid import uuid4 + +from .context_builder import ContextBudget, ReviewContext, build_review_context +from .git_diff import GitDiffCollector +from .models import ( + ChangedFile, + ReviewOutput, + ReviewRun, + ReviewStatus, + stable_config_hash, +) +from .policy import apply_finding_policy +from .static_analysis import StaticAnalysisResult + +ReviewCallable = Callable[[ReviewContext], Awaitable[ReviewOutput]] +StaticAnalysisCallable = Callable[[Path, list[ChangedFile]], Awaitable[StaticAnalysisResult]] + + +@dataclass(frozen=True) +class ReviewConfig: + """Configuration that affects deterministic review results.""" + + context_lines: int = 3 + use_merge_base: bool = True + max_files: int = 40 + max_patch_chars_per_file: int = 24_000 + max_total_chars: int = 120_000 + minimum_confidence: float = 0.0 + + def context_budget(self) -> ContextBudget: + return ContextBudget( + max_files=self.max_files, + max_patch_chars_per_file=self.max_patch_chars_per_file, + max_total_chars=self.max_total_chars, + ) + + +async def run_review( + *, + repository: str | Path, + base_revision: str, + head_revision: str, + config: ReviewConfig | None = None, + reviewer: ReviewCallable | None = None, + static_analyzer: StaticAnalysisCallable | None = None, + model_name: str = "", + execution_config: dict[str, Any] | None = None, + repository_identity: str | None = None, +) -> ReviewRun: + """Run a local review; omit reviewer for deterministic no-LLM mode.""" + config = config or ReviewConfig() + repository_path = Path(repository).expanduser().resolve() + identity = repository_identity or str(repository_path) + config_hash = stable_config_hash({ + "review": asdict(config), + "execution": execution_config or {}, + }) + provisional_idempotency_key = stable_config_hash({ + "repository_identity": identity, + "base_revision": base_revision, + "head_revision": head_revision, + "config_hash": config_hash, + "model_name": model_name, + }) + review_run = ReviewRun( + id=str(uuid4()), + repository_path=identity, + base_revision=base_revision, + head_revision=head_revision, + effective_base_revision="", + status=ReviewStatus.RUNNING, + model_name=model_name, + config_hash=config_hash, + idempotency_key=provisional_idempotency_key, + ) + try: + collector = GitDiffCollector(repository_path, context_lines=config.context_lines) + effective_base, changed_files = collector.collect( + base_revision, + head_revision, + use_merge_base=config.use_merge_base, + ) + resolved_head = collector.resolve_revision(head_revision) + review_run.effective_base_revision = effective_base + review_run.resolved_head_revision = resolved_head + review_run.idempotency_key = stable_config_hash({ + "repository_identity": identity, + "effective_base_revision": effective_base, + "resolved_head_revision": resolved_head, + "config_hash": config_hash, + "model_name": model_name, + }) + review_run.changed_files = changed_files + context = build_review_context(changed_files, config.context_budget()) + review_run.diagnostics.extend(context.diagnostics) + + static_result = StaticAnalysisResult() + if static_analyzer is not None: + review_run.static_analysis_requested = True + static_result = await static_analyzer(repository_path, changed_files) + review_run.analyzer_executions = static_result.executions + review_run.diagnostics.extend(static_result.diagnostics) + context = ReviewContext( + text=context.text, + included_files=context.included_files, + skipped_files=context.skipped_files, + truncated_files=context.truncated_files, + diagnostics=context.diagnostics, + static_analysis=static_result.prompt_text(), + ) + + if reviewer is None: + llm_output = ReviewOutput(summary=(f"Collected {len(changed_files)} changed file(s); " + "LLM review was disabled.")) + elif not context.included_files: + llm_output = ReviewOutput(summary="No textual changed files were eligible for LLM review.") + else: + llm_output = await reviewer(context) + + summary_parts = [llm_output.summary.strip()] + if static_analyzer is not None: + summary_parts.append(f"Static analysis produced {len(static_result.findings)} finding(s) " + f"from {len(static_result.executions)} tool run(s).") + raw_output = ReviewOutput( + summary=" ".join(part for part in summary_parts if part), + findings=[*static_result.findings, *llm_output.findings], + ) + + normalized_output, diagnostics = apply_finding_policy( + raw_output, + changed_files, + minimum_confidence=config.minimum_confidence, + ) + review_run.output = normalized_output + review_run.diagnostics.extend(diagnostics) + review_run.status = ReviewStatus.COMPLETED + except Exception as exc: # noqa: BLE001 - report failures as review artifacts + review_run.status = ReviewStatus.FAILED + review_run.error_message = str(exc) + review_run.diagnostics.append(f"Review failed: {exc}") + finally: + review_run.finished_at = datetime.now(timezone.utc) + return review_run diff --git a/examples/code_review_agent/code_review/policy.py b/examples/code_review_agent/code_review/policy.py new file mode 100644 index 000000000..89a020e3f --- /dev/null +++ b/examples/code_review_agent/code_review/policy.py @@ -0,0 +1,89 @@ +"""Deterministic validation, line mapping, sorting, and deduplication.""" + +from __future__ import annotations + +from collections.abc import Iterable + +from .models import ChangedFile, Finding, ReviewOutput, Severity + +_SEVERITY_RANK = { + Severity.CRITICAL: 0, + Severity.HIGH: 1, + Severity.MEDIUM: 2, + Severity.LOW: 3, + Severity.INFO: 4, +} + + +def apply_finding_policy( + output: ReviewOutput, + changed_files: Iterable[ChangedFile], + *, + minimum_confidence: float = 0.0, +) -> tuple[ReviewOutput, list[str]]: + """Validate finding scope and return a stable, deduplicated result.""" + file_map = {changed_file.path: changed_file for changed_file in changed_files} + accepted: dict[tuple[str, str, int | None, int | None], Finding] = {} + diagnostics: list[str] = [] + + for original in output.findings: + finding = original.model_copy(deep=True) + matched_path = _match_changed_path(finding.file_path, file_map) + if matched_path is None: + diagnostics.append(f"Dropped finding for unchanged file: {finding.file_path}") + continue + finding.file_path = matched_path + if finding.confidence < minimum_confidence: + diagnostics.append(f"Dropped low-confidence finding {finding.rule_id} ({finding.confidence:.2f})") + continue + + changed_file = file_map[matched_path] + finding.publishable = _is_line_publishable(finding, changed_file) + if finding.start_line is not None and not finding.publishable: + diagnostics.append(f"Finding {finding.rule_id} does not point to an added line in {matched_path}") + + key = ( + finding.rule_id.casefold(), + finding.file_path, + finding.start_line, + finding.end_line, + ) + existing = accepted.get(key) + if existing is None or _finding_priority(finding) < _finding_priority(existing): + accepted[key] = finding + + findings = sorted( + accepted.values(), + key=lambda item: ( + _SEVERITY_RANK[item.severity], + -item.confidence, + item.file_path, + item.start_line or 0, + item.rule_id, + ), + ) + return ReviewOutput(summary=output.summary.strip(), findings=findings), diagnostics + + +def _match_changed_path(path: str, file_map: dict[str, ChangedFile]) -> str | None: + normalized = path.strip().replace("\\", "/") + candidates = [normalized] + if normalized.startswith("./"): + candidates.append(normalized[2:]) + if normalized.startswith(("a/", "b/")): + candidates.append(normalized[2:]) + for candidate in candidates: + if candidate in file_map: + return candidate + return None + + +def _is_line_publishable(finding: Finding, changed_file: ChangedFile) -> bool: + if finding.start_line is None or finding.end_line is None: + return False + changed_lines = changed_file.changed_new_lines + return any(line in changed_lines for line in range(finding.start_line, finding.end_line + 1)) + + +def _finding_priority(finding: Finding) -> tuple[int, float]: + return _SEVERITY_RANK[finding.severity], -finding.confidence diff --git a/examples/code_review_agent/code_review/reporter.py b/examples/code_review_agent/code_review/reporter.py new file mode 100644 index 000000000..2c3119e5c --- /dev/null +++ b/examples/code_review_agent/code_review/reporter.py @@ -0,0 +1,105 @@ +"""JSON and Markdown report generation.""" + +from __future__ import annotations + +import json +from collections import Counter +from pathlib import Path + +from .models import Finding, ReviewRun, Severity + + +def write_reports(review_run: ReviewRun, output_directory: str | Path) -> tuple[Path, Path]: + """Write canonical JSON and human-readable Markdown reports.""" + output_path = Path(output_directory).expanduser().resolve() + output_path.mkdir(parents=True, exist_ok=True) + json_path = output_path / "review.json" + markdown_path = output_path / "review.md" + json_path.write_text( + json.dumps(review_run.model_dump(mode="json"), ensure_ascii=False, indent=2) + "\n", + encoding="utf-8", + ) + markdown_path.write_text(render_markdown(review_run), encoding="utf-8") + return json_path, markdown_path + + +def render_markdown(review_run: ReviewRun) -> str: + """Render a compact report suitable for a check summary.""" + counts = Counter(finding.severity for finding in review_run.output.findings) + publishable = sum(finding.publishable for finding in review_run.output.findings) + lines = [ + "# Code Review Report", + "", + f"- Run: `{review_run.id}`", + f"- Status: `{review_run.status.value}`", + f"- Repository: `{review_run.repository_path}`", + f"- Base: `{review_run.base_revision}`", + f"- Effective base: `{review_run.effective_base_revision}`", + f"- Head: `{review_run.head_revision}`", + f"- Changed files: {len(review_run.changed_files)}", + f"- Findings: {len(review_run.output.findings)}", + f"- Line-comment eligible: {publishable}", + "", + "## Summary", + "", + review_run.output.summary or "No summary was produced.", + "", + "## Severity", + "", + ] + for severity in Severity: + lines.append(f"- {severity.value}: {counts[severity]}") + + lines.extend(["", "## Static Analysis", ""]) + if not review_run.static_analysis_requested: + lines.extend(["Static analysis was not requested.", ""]) + elif not review_run.analyzer_executions: + lines.extend(["Static analysis was requested, but no eligible files required a tool run.", ""]) + else: + lines.extend([ + "| Tool | Runtime | Status | Exit | Findings | Duration |", + "| --- | --- | --- | ---: | ---: | ---: |", + ]) + for execution in review_run.analyzer_executions: + exit_code = "" if execution.exit_code is None else str(execution.exit_code) + lines.append(f"| {execution.tool} | {execution.runtime} | {execution.status.value} | " + f"{exit_code} | {execution.findings_count} | {execution.duration_seconds:.2f}s |") + lines.append("") + + lines.extend(["", "## Findings", ""]) + if not review_run.output.findings: + lines.extend(["No actionable findings.", ""]) + else: + for index, finding in enumerate(review_run.output.findings, start=1): + lines.extend(_render_finding(index, finding)) + + if review_run.diagnostics: + lines.extend(["## Diagnostics", ""]) + lines.extend(f"- {diagnostic}" for diagnostic in review_run.diagnostics) + lines.append("") + return "\n".join(lines) + + +def _render_finding(index: int, finding: Finding) -> list[str]: + location = finding.file_path + if finding.start_line is not None: + location += f":{finding.start_line}" + if finding.end_line != finding.start_line: + location += f"-{finding.end_line}" + publishable = "yes" if finding.publishable else "no" + lines = [ + f"### {index}. [{finding.severity.value.upper()}] {finding.title}", + "", + f"- Rule: `{finding.rule_id}`", + f"- Category: `{finding.category}`", + f"- Location: `{location}`", + f"- Confidence: {finding.confidence:.2f}", + f"- Line-comment eligible: {publishable}", + f"- Source: `{finding.source}`", + "", + finding.description, + "", + ] + if finding.suggestion: + lines.extend(["**Suggestion:**", "", finding.suggestion, ""]) + return lines diff --git a/examples/code_review_agent/code_review/static_analysis.py b/examples/code_review_agent/code_review/static_analysis.py new file mode 100644 index 000000000..e91276e27 --- /dev/null +++ b/examples/code_review_agent/code_review/static_analysis.py @@ -0,0 +1,437 @@ +"""Deterministic Ruff, Bandit, and Pytest execution with optional Docker isolation.""" + +from __future__ import annotations + +import asyncio +import json +import re +import subprocess +import time +from dataclasses import dataclass, field +from pathlib import Path +from typing import Protocol + +from .models import AnalyzerExecution, AnalyzerStatus, ChangedFile, Finding, Severity + + +@dataclass(frozen=True) +class StaticAnalysisConfig: + """Static analyzer selection, isolation, and resource limits.""" + + runtime: str = "local" + enable_ruff: bool = True + enable_bandit: bool = True + run_tests: bool = False + strict_tools: bool = False + timeout_seconds: float = 120.0 + max_output_chars: int = 100_000 + docker_image: str = "trpc-code-review:latest" + docker_memory: str = "512m" + docker_cpus: str = "1.0" + docker_pids_limit: int = 128 + + def __post_init__(self) -> None: + if self.runtime not in {"local", "docker"}: + raise ValueError("static analysis runtime must be 'local' or 'docker'") + if self.timeout_seconds <= 0: + raise ValueError("static analysis timeout must be positive") + if self.max_output_chars <= 0: + raise ValueError("static analysis output limit must be positive") + if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._/:@-]*", self.docker_image): + raise ValueError("docker image must be a valid image reference") + if not re.fullmatch(r"[1-9][0-9]*(?:[bkmgBKMG])?", self.docker_memory): + raise ValueError("docker memory must be a positive integer with an optional b/k/m/g suffix") + try: + cpus = float(self.docker_cpus) + except ValueError as exc: + raise ValueError("docker cpus must be a positive number") from exc + if cpus <= 0: + raise ValueError("docker cpus must be a positive number") + if self.docker_pids_limit <= 0: + raise ValueError("docker pids limit must be positive") + + +@dataclass(frozen=True) +class CommandResult: + """Raw subprocess outcome independent of analyzer semantics.""" + + command: list[str] + exit_code: int | None + stdout: str = "" + stderr: str = "" + duration_seconds: float = 0.0 + timed_out: bool = False + unavailable: bool = False + + +@dataclass +class StaticAnalysisResult: + """Aggregate static analyzer findings and execution evidence.""" + + findings: list[Finding] = field(default_factory=list) + executions: list[AnalyzerExecution] = field(default_factory=list) + diagnostics: list[str] = field(default_factory=list) + + def prompt_text(self) -> str: + """Return bounded evidence for semantic review and deduplication.""" + payload = { + "findings": [finding.model_dump(mode="json") for finding in self.findings], + "executions": [{ + "tool": execution.tool, + "status": execution.status.value, + "exit_code": execution.exit_code, + "stderr": execution.stderr, + } for execution in self.executions], + } + return json.dumps(payload, ensure_ascii=False, indent=2) + + +class CommandRunner(Protocol): + """Execution boundary used by real runtimes and unit tests.""" + + runtime_name: str + + def run(self, command: list[str], *, repository: Path, timeout: float) -> CommandResult: + """Execute an analyzer command.""" + + +class LocalCommandRunner: + """Execute analyzer binaries directly without a shell.""" + + runtime_name = "local" + + def run(self, command: list[str], *, repository: Path, timeout: float) -> CommandResult: + started = time.monotonic() + try: + result = subprocess.run( + command, + cwd=repository, + check=False, + capture_output=True, + text=True, + timeout=timeout, + ) + return CommandResult( + command=command, + exit_code=result.returncode, + stdout=result.stdout, + stderr=result.stderr, + duration_seconds=time.monotonic() - started, + ) + except FileNotFoundError as exc: + return CommandResult( + command=command, + exit_code=None, + stderr=str(exc), + duration_seconds=time.monotonic() - started, + unavailable=True, + ) + except subprocess.TimeoutExpired as exc: + return CommandResult( + command=command, + exit_code=None, + stdout=_decode_timeout_stream(exc.stdout), + stderr=_decode_timeout_stream(exc.stderr), + duration_seconds=time.monotonic() - started, + timed_out=True, + ) + + +class DockerCommandRunner(LocalCommandRunner): + """Run analyzers in an offline, read-only, resource-limited container.""" + + runtime_name = "docker" + + def __init__( + self, + *, + image: str, + memory: str, + cpus: str, + pids_limit: int, + ): + self.image = image + self.memory = memory + self.cpus = cpus + self.pids_limit = pids_limit + + def build_command(self, command: list[str], repository: Path) -> list[str]: + """Build a Docker invocation without interpolating a shell command.""" + return [ + "docker", + "run", + "--rm", + "--pull", + "never", + "--network", + "none", + "--read-only", + "--cap-drop", + "ALL", + "--security-opt", + "no-new-privileges", + "--pids-limit", + str(self.pids_limit), + "--memory", + self.memory, + "--cpus", + self.cpus, + "--user", + "65532:65532", + "--tmpfs", + "/tmp:rw,nosuid,nodev,noexec,size=64m", + "--mount", + f"type=bind,src={repository},dst=/workspace,readonly", + "--workdir", + "/workspace", + "--env", + "PYTHONDONTWRITEBYTECODE=1", + "--env", + "PYTEST_ADDOPTS=-p no:cacheprovider", + self.image, + *command, + ] + + def run(self, command: list[str], *, repository: Path, timeout: float) -> CommandResult: + return super().run(self.build_command(command, repository), repository=repository, timeout=timeout) + + +class StaticAnalyzer: + """Run enabled analyzers only against changed Python files.""" + + def __init__( + self, + config: StaticAnalysisConfig | None = None, + *, + command_runner: CommandRunner | None = None, + ): + self.config = config or StaticAnalysisConfig() + self.command_runner = command_runner or self._create_runner(self.config) + + async def analyze(self, repository: Path, changed_files: list[ChangedFile]) -> StaticAnalysisResult: + """Run blocking analyzer processes off the event loop.""" + return await asyncio.to_thread(self.analyze_sync, repository, changed_files) + + def analyze_sync(self, repository: Path, changed_files: list[ChangedFile]) -> StaticAnalysisResult: + """Run enabled analyzers and normalize their native JSON.""" + repository = repository.resolve() + python_paths = [ + changed_file.path for changed_file in changed_files + if changed_file.language == "python" and changed_file.change_type.value != "deleted" + ] + result = StaticAnalysisResult() + if not python_paths: + result.diagnostics.append("Static analysis skipped: no changed Python files") + return result + + if self.config.enable_ruff: + self._run_tool( + result, + tool="ruff", + command=["ruff", "check", "--output-format", "json", "--no-cache", "--", *python_paths], + repository=repository, + parser=lambda text: parse_ruff_output(text, repository), + finding_exit_codes={1}, + ) + if self.config.enable_bandit: + self._run_tool( + result, + tool="bandit", + command=["bandit", "-f", "json", "-q", "--", *python_paths], + repository=repository, + parser=lambda text: parse_bandit_output(text, repository), + finding_exit_codes={1}, + ) + if self.config.run_tests: + self._run_tool( + result, + tool="pytest", + command=["pytest", "-q", "--disable-warnings", "--maxfail=20"], + repository=repository, + parser=lambda _text: [], + finding_exit_codes=set(), + ) + return result + + def _run_tool( + self, + aggregate: StaticAnalysisResult, + *, + tool: str, + command: list[str], + repository: Path, + parser, + finding_exit_codes: set[int], + ) -> None: + raw = self.command_runner.run( + command, + repository=repository, + timeout=self.config.timeout_seconds, + ) + stdout = _truncate(raw.stdout, self.config.max_output_chars) + stderr = _truncate(raw.stderr, self.config.max_output_chars) + findings: list[Finding] = [] + status = AnalyzerStatus.SUCCESS + if raw.unavailable or _docker_command_unavailable(raw): + status = AnalyzerStatus.UNAVAILABLE + elif raw.timed_out: + status = AnalyzerStatus.TIMED_OUT + elif raw.exit_code in {0, *finding_exit_codes}: + try: + findings = parser(stdout) + status = AnalyzerStatus.FINDINGS if findings else AnalyzerStatus.SUCCESS + except (json.JSONDecodeError, KeyError, TypeError, ValueError) as exc: + status = AnalyzerStatus.FAILED + stderr = _truncate(f"{stderr}\nUnable to parse {tool} output: {exc}".strip(), + self.config.max_output_chars) + else: + status = AnalyzerStatus.FAILED + + execution = AnalyzerExecution( + tool=tool, + runtime=self.command_runner.runtime_name, + command=raw.command, + status=status, + exit_code=raw.exit_code, + duration_seconds=raw.duration_seconds, + stdout=stdout, + stderr=stderr, + findings_count=len(findings), + ) + aggregate.executions.append(execution) + aggregate.findings.extend(findings) + if status not in {AnalyzerStatus.SUCCESS, AnalyzerStatus.FINDINGS}: + message = f"{tool} {status.value}" + if stderr: + message += f": {stderr.splitlines()[-1]}" + aggregate.diagnostics.append(message) + if self.config.strict_tools: + raise RuntimeError(message) + + @staticmethod + def _create_runner(config: StaticAnalysisConfig) -> CommandRunner: + if config.runtime == "local": + return LocalCommandRunner() + return DockerCommandRunner( + image=config.docker_image, + memory=config.docker_memory, + cpus=config.docker_cpus, + pids_limit=config.docker_pids_limit, + ) + + +def parse_ruff_output(text: str, repository: Path) -> list[Finding]: + """Map Ruff JSON diagnostics to the common Finding schema.""" + if not text.strip(): + return [] + diagnostics = json.loads(text) + findings: list[Finding] = [] + for diagnostic in diagnostics: + code = str(diagnostic["code"]) + location = diagnostic["location"] + end_location = diagnostic.get("end_location") or location + category, severity = _ruff_classification(code) + fix = diagnostic.get("fix") or {} + findings.append( + Finding( + rule_id=f"ruff.{code.casefold()}", + severity=severity, + confidence=0.99, + category=category, + file_path=_relative_path(str(diagnostic["filename"]), repository), + start_line=int(location["row"]), + end_line=int(end_location["row"]), + title=str(diagnostic["message"]), + description=f"Ruff reported {code}: {diagnostic['message']}", + suggestion=str(fix.get("message", "")), + source="static:ruff", + )) + return findings + + +def parse_bandit_output(text: str, repository: Path) -> list[Finding]: + """Map Bandit JSON diagnostics to the common Finding schema.""" + if not text.strip(): + return [] + payload = json.loads(text) + findings: list[Finding] = [] + for diagnostic in payload.get("results", []): + severity = _bandit_severity(str(diagnostic.get("issue_severity", ""))) + confidence = _bandit_confidence(str(diagnostic.get("issue_confidence", ""))) + line_range = diagnostic.get("line_range") or [diagnostic["line_number"]] + test_id = str(diagnostic["test_id"]) + findings.append( + Finding( + rule_id=f"bandit.{test_id.casefold()}", + severity=severity, + confidence=confidence, + category="security", + file_path=_relative_path(str(diagnostic["filename"]), repository), + start_line=min(int(line) for line in line_range), + end_line=max(int(line) for line in line_range), + title=str(diagnostic["issue_text"]), + description=f"Bandit reported {test_id}: {diagnostic['issue_text']}", + suggestion=str(diagnostic.get("more_info", "")), + source="static:bandit", + )) + return findings + + +def _ruff_classification(code: str) -> tuple[str, Severity]: + if code.startswith("S"): + return "security", Severity.HIGH + if code.startswith(("F", "E9", "B")): + return "correctness", Severity.MEDIUM + if code.startswith(("PERF", "PL", "C4", "SIM")): + return "maintainability", Severity.LOW + return "quality", Severity.LOW + + +def _bandit_severity(value: str) -> Severity: + return { + "HIGH": Severity.HIGH, + "MEDIUM": Severity.MEDIUM, + "LOW": Severity.LOW, + }.get(value.upper(), Severity.LOW) + + +def _bandit_confidence(value: str) -> float: + return { + "HIGH": 0.95, + "MEDIUM": 0.8, + "LOW": 0.6, + }.get(value.upper(), 0.6) + + +def _relative_path(path: str, repository: Path) -> str: + candidate = Path(path) + if candidate.is_absolute(): + try: + return candidate.resolve().relative_to(repository.resolve()).as_posix() + except ValueError: + return candidate.as_posix() + normalized = candidate.as_posix() + return normalized.removeprefix("./") + + +def _truncate(value: str, limit: int) -> str: + if len(value) <= limit: + return value + marker = "\n[OUTPUT TRUNCATED]\n" + return value[:max(0, limit - len(marker))] + marker + + +def _decode_timeout_stream(value: str | bytes | None) -> str: + if value is None: + return "" + if isinstance(value, bytes): + return value.decode("utf-8", errors="replace") + return value + + +def _docker_command_unavailable(result: CommandResult) -> bool: + if not result.command or result.command[0] != "docker": + return False + message = f"{result.stdout}\n{result.stderr}".casefold() + return result.exit_code in {125, 127} and ("no such image" in message or "executable file not found" in message + or "cannot connect" in message or "permission denied" in message) diff --git a/examples/code_review_agent/github_integration/__init__.py b/examples/code_review_agent/github_integration/__init__.py new file mode 100644 index 000000000..223c37bc3 --- /dev/null +++ b/examples/code_review_agent/github_integration/__init__.py @@ -0,0 +1,6 @@ +"""GitHub App webhook integration for the code review example.""" + +from .models import GitHubPullRequestEvent +from .security import verify_webhook_signature + +__all__ = ["GitHubPullRequestEvent", "verify_webhook_signature"] diff --git a/examples/code_review_agent/github_integration/app.py b/examples/code_review_agent/github_integration/app.py new file mode 100644 index 000000000..17ac5eac6 --- /dev/null +++ b/examples/code_review_agent/github_integration/app.py @@ -0,0 +1,153 @@ +"""FastAPI webhook entrypoint and environment-backed service construction.""" + +from __future__ import annotations + +import asyncio +import hashlib +import json +import os +from contextlib import asynccontextmanager +from pathlib import Path + +from fastapi import FastAPI, HTTPException, Request, status +from pydantic import ValidationError + +from ..code_review.database import ReviewStore, sqlite_database_url +from .models import GitHubPullRequestEvent +from .security import verify_webhook_signature + + +def create_webhook_app( + *, + store: ReviewStore, + webhook_secret: str, + max_payload_bytes: int = 2_000_000, + max_attempts: int = 5, + close_resources: bool = False, +) -> FastAPI: + """Create a receiver that atomically persists accepted webhook jobs.""" + if max_payload_bytes <= 0: + raise ValueError("max_payload_bytes must be positive") + if max_attempts <= 0: + raise ValueError("max_attempts must be positive") + + @asynccontextmanager + async def lifespan(_app: FastAPI): + yield + if close_resources: + store.close() + + app = FastAPI(title="tRPC Code Review GitHub App", lifespan=lifespan) + + @app.get("/healthz") + async def healthz() -> dict[str, str]: + return {"status": "ok"} + + @app.get("/readyz") + async def readyz() -> dict[str, str]: + try: + await asyncio.to_thread(store.ping) + except Exception as exc: + raise HTTPException(status_code=503, detail="Database is unavailable") from exc + return {"status": "ready"} + + @app.post("/webhooks/github", status_code=status.HTTP_202_ACCEPTED) + async def github_webhook(request: Request): + content_length = request.headers.get("content-length") + if content_length and content_length.isdigit() and int(content_length) > max_payload_bytes: + raise HTTPException(status_code=413, detail="Webhook payload is too large") + body = await request.body() + if len(body) > max_payload_bytes: + raise HTTPException(status_code=413, detail="Webhook payload is too large") + try: + verify_webhook_signature( + body, + webhook_secret, + request.headers.get("x-hub-signature-256"), + ) + except ValueError as exc: + raise HTTPException(status_code=401, detail=str(exc)) from exc + + delivery_id = request.headers.get("x-github-delivery", "") + event_name = request.headers.get("x-github-event", "") + if (not delivery_id or len(delivery_id) > 128 or "\n" in delivery_id or "\r" in delivery_id or not event_name + or len(event_name) > 80): + raise HTTPException(status_code=400, detail="Missing GitHub delivery headers") + try: + payload = json.loads(body) + if not isinstance(payload, dict): + raise TypeError("payload must be a JSON object") + event = GitHubPullRequestEvent.from_webhook( + delivery_id=delivery_id, + event_name=event_name, + payload=payload, + ) + except (KeyError, TypeError, ValueError, ValidationError) as exc: + raise HTTPException(status_code=400, detail=f"Invalid GitHub webhook payload: {exc}") from exc + + action = str(payload.get("action", "")) + payload_digest = hashlib.sha256(body).hexdigest() + if event is None or event.draft: + claimed = await asyncio.to_thread( + store.claim_github_delivery, + delivery_id=delivery_id, + event_name=event_name, + action=action, + payload_sha256=payload_digest, + repository_full_name=event.repository_full_name if event else "", + pull_number=event.pull_number if event else None, + head_sha=event.head_sha if event else "", + installation_id=event.installation_id if event else None, + ) + else: + claimed = await asyncio.to_thread( + store.enqueue_github_delivery, + delivery_id=delivery_id, + event_name=event_name, + action=action, + payload_sha256=payload_digest, + event_payload=event.model_dump(mode="json"), + repository_full_name=event.repository_full_name, + pull_number=event.pull_number, + head_sha=event.head_sha, + installation_id=event.installation_id, + max_attempts=max_attempts, + ) + if not claimed: + existing = await asyncio.to_thread(store.get_github_delivery, delivery_id) + if existing is not None and existing["payload_sha256"] != payload_digest: + raise HTTPException(status_code=409, detail="Delivery ID was reused with a different payload") + return {"status": "duplicate", "delivery_id": delivery_id} + if event is None or event.draft: + await asyncio.to_thread( + store.update_github_delivery, + delivery_id, + status="ignored", + ) + return {"status": "ignored", "delivery_id": delivery_id} + + return {"status": "queued", "delivery_id": delivery_id} + + return app + + +def build_app_from_environment() -> FastAPI: + """Build the durable webhook receiver using environment variables.""" + webhook_secret = _required_env("GITHUB_WEBHOOK_SECRET") + database_url = (os.getenv("CODE_REVIEW_DATABASE_URL") + or sqlite_database_url(Path(".code-review") / "github-reviews.db")) + store = ReviewStore(database_url) + return create_webhook_app( + store=store, + webhook_secret=webhook_secret, + max_payload_bytes=int(os.getenv("GITHUB_WEBHOOK_MAX_BYTES", "2000000")), + max_attempts=int(os.getenv("GITHUB_REVIEW_MAX_ATTEMPTS", "5")), + close_resources=True, + ) + + +def _required_env(name: str) -> str: + value = os.getenv(name, "") + if not value: + raise ValueError(f"Missing required environment variable: {name}") + return value diff --git a/examples/code_review_agent/github_integration/checkout.py b/examples/code_review_agent/github_integration/checkout.py new file mode 100644 index 000000000..01ebd3009 --- /dev/null +++ b/examples/code_review_agent/github_integration/checkout.py @@ -0,0 +1,129 @@ +"""Isolated Git checkout for one GitHub pull request delivery.""" + +from __future__ import annotations + +import hashlib +import os +import shutil +import subprocess +from pathlib import Path +from urllib.parse import urlparse + +from .models import GitHubPullRequestEvent + + +class CheckoutError(RuntimeError): + """Raised when an authenticated Git checkout cannot be prepared safely.""" + + +class GitHubWorkspaceManager: + """Fetch exact webhook commits without placing credentials in command arguments.""" + + def __init__( + self, + root: str | Path, + *, + allowed_hosts: tuple[str, ...] = ("github.com", ), + command_timeout: float = 300.0, + ): + self.root = Path(root).expanduser().resolve() + self.root.mkdir(parents=True, exist_ok=True) + self.allowed_hosts = {host.casefold() for host in allowed_hosts} + if not self.allowed_hosts: + raise ValueError("at least one GitHub clone host must be allowed") + if command_timeout <= 0: + raise ValueError("Git command timeout must be positive") + self.command_timeout = command_timeout + + def checkout(self, event: GitHubPullRequestEvent, token: str) -> Path: + """Create a detached worktree at the exact PR head SHA.""" + base_url = self._validate_clone_url(event.base_clone_url) + head_url = self._validate_clone_url(event.head_clone_url) + workspace = self.root / f"delivery-{hashlib.sha256(event.delivery_id.encode()).hexdigest()[:24]}" + if workspace.exists(): + raise CheckoutError(f"Workspace already exists for delivery {event.delivery_id}") + workspace.mkdir(mode=0o700) + environment = self._git_environment(token) + try: + self._git(workspace, environment, "init", "--quiet") + self._git(workspace, environment, "remote", "add", "base", base_url) + self._git(workspace, environment, "remote", "add", "head", head_url) + self._git( + workspace, + environment, + "fetch", + "--quiet", + "--no-tags", + "--filter=blob:none", + "base", + f"+{event.base_sha}:refs/code-review/base", + ) + self._git( + workspace, + environment, + "fetch", + "--quiet", + "--no-tags", + "--filter=blob:none", + "head", + f"+{event.head_sha}:refs/code-review/head", + ) + self._git(workspace, environment, "checkout", "--quiet", "--detach", "refs/code-review/head") + resolved_head = self._git(workspace, environment, "rev-parse", "HEAD").strip() + if resolved_head.casefold() != event.head_sha.casefold(): + raise CheckoutError("Fetched head commit does not match the signed webhook payload") + return workspace + except Exception: + self.cleanup(workspace) + raise + + def cleanup(self, workspace: str | Path) -> None: + """Remove only delivery workspaces directly beneath the configured root.""" + path = Path(workspace).resolve() + if path.parent != self.root or not path.name.startswith("delivery-"): + raise CheckoutError(f"Refusing to remove unsafe workspace path: {path}") + if path.exists(): + shutil.rmtree(path) + + def _validate_clone_url(self, value: str) -> str: + parsed = urlparse(value) + if parsed.scheme != "https" or not parsed.hostname: + raise CheckoutError("GitHub clone URL must use HTTPS") + if parsed.username or parsed.password: + raise CheckoutError("GitHub clone URL must not contain credentials") + if parsed.hostname.casefold() not in self.allowed_hosts: + raise CheckoutError(f"GitHub clone host is not allowed: {parsed.hostname}") + if not parsed.path.endswith(".git") or parsed.query or parsed.fragment: + raise CheckoutError("Invalid GitHub clone URL") + return value + + def _git(self, workspace: Path, environment: dict[str, str], *arguments: str) -> str: + command = ["git", "-C", str(workspace), *arguments] + try: + result = subprocess.run( + command, + check=False, + capture_output=True, + text=True, + timeout=self.command_timeout, + env=environment, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise CheckoutError(f"Unable to execute Git checkout command: {exc}") from exc + if result.returncode != 0: + message = result.stderr.strip() + raise CheckoutError(message or f"Git exited with status {result.returncode}") + return result.stdout + + @staticmethod + def _git_environment(token: str) -> dict[str, str]: + if not token or "\n" in token or "\r" in token: + raise CheckoutError("Invalid GitHub installation token") + environment = os.environ.copy() + environment.update({ + "GIT_TERMINAL_PROMPT": "0", + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.extraHeader", + "GIT_CONFIG_VALUE_0": f"Authorization: Bearer {token}", + }) + return environment diff --git a/examples/code_review_agent/github_integration/client.py b/examples/code_review_agent/github_integration/client.py new file mode 100644 index 000000000..b4fc40d86 --- /dev/null +++ b/examples/code_review_agent/github_integration/client.py @@ -0,0 +1,247 @@ +"""Small async GitHub REST client and GitHub App token provider.""" + +from __future__ import annotations + +import asyncio +from dataclasses import dataclass +from datetime import datetime, timedelta, timezone +from typing import Any, Protocol +from urllib.parse import quote + +import httpx + +GITHUB_API_VERSION = "2026-03-10" + + +class GitHubApiError(RuntimeError): + """Raised when GitHub returns a non-success response.""" + + def __init__(self, message: str, *, status_code: int): + super().__init__(message) + self.status_code = status_code + + +class TokenProvider(Protocol): + """Resolve a token scoped to one GitHub App installation.""" + + async def get_token(self, installation_id: int) -> str: + """Return a currently valid token.""" + + +@dataclass(frozen=True) +class StaticTokenProvider: + """Use a pre-generated token for development and local smoke tests.""" + + token: str + + async def get_token(self, installation_id: int) -> str: + del installation_id + if not self.token: + raise ValueError("GitHub token is not configured") + return self.token + + +@dataclass +class _CachedToken: + token: str + expires_at: datetime + + +class GitHubAppTokenProvider: + """Mint and cache one-hour GitHub App installation access tokens.""" + + def __init__( + self, + *, + app_id: str, + private_key: str, + api_url: str = "https://api.github.com", + permissions: dict[str, str] | None = None, + http_client: httpx.AsyncClient | None = None, + ): + if not app_id or not private_key: + raise ValueError("GitHub App ID and private key are required") + self.app_id = app_id + self.private_key = private_key + self.api_url = api_url.rstrip("/") + self.permissions = permissions or { + "contents": "read", + "checks": "write", + "pull_requests": "write", + } + self._client = http_client or httpx.AsyncClient(base_url=self.api_url, timeout=30.0) + self._owns_client = http_client is None + self._cache: dict[int, _CachedToken] = {} + self._lock = asyncio.Lock() + + async def get_token(self, installation_id: int) -> str: + now = datetime.now(timezone.utc) + cached = self._cache.get(installation_id) + if cached is not None and cached.expires_at - timedelta(seconds=60) > now: + return cached.token + async with self._lock: + cached = self._cache.get(installation_id) + if cached is not None and cached.expires_at - timedelta(seconds=60) > now: + return cached.token + token = await self._mint_token(installation_id, now) + self._cache[installation_id] = token + return token.token + + async def _mint_token(self, installation_id: int, now: datetime) -> _CachedToken: + try: + import jwt + except ImportError as exc: + raise RuntimeError("Install PyJWT[crypto] to authenticate as a GitHub App") from exc + app_jwt = jwt.encode( + { + "iat": int((now - timedelta(seconds=60)).timestamp()), + "exp": int((now + timedelta(minutes=9)).timestamp()), + "iss": self.app_id, + }, + self.private_key, + algorithm="RS256", + ) + response = await self._client.post( + f"/app/installations/{installation_id}/access_tokens", + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {app_jwt}", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + }, + json={"permissions": self.permissions}, + ) + _raise_for_github(response) + payload = response.json() + expires_at = datetime.fromisoformat(str(payload["expires_at"]).replace("Z", "+00:00")) + return _CachedToken(token=str(payload["token"]), expires_at=expires_at) + + async def close(self) -> None: + if self._owns_client: + await self._client.aclose() + + +class GitHubClient: + """Only the GitHub endpoints required by the review publisher.""" + + def __init__( + self, + *, + token_provider: TokenProvider, + installation_id: int, + api_url: str = "https://api.github.com", + http_client: httpx.AsyncClient | None = None, + ): + self.token_provider = token_provider + self.installation_id = installation_id + self.api_url = api_url.rstrip("/") + self._client = http_client or httpx.AsyncClient(base_url=self.api_url, timeout=30.0) + self._owns_client = http_client is None + + async def create_check_run( + self, + owner: str, + repository: str, + payload: dict[str, Any], + ) -> dict[str, Any]: + return await self._request("POST", f"/repos/{_segment(owner)}/{_segment(repository)}/check-runs", payload) + + async def update_check_run( + self, + owner: str, + repository: str, + check_run_id: int, + payload: dict[str, Any], + ) -> dict[str, Any]: + path = f"/repos/{_segment(owner)}/{_segment(repository)}/check-runs/{check_run_id}" + return await self._request("PATCH", path, payload) + + async def create_review_comment( + self, + owner: str, + repository: str, + pull_number: int, + payload: dict[str, Any], + ) -> dict[str, Any]: + path = f"/repos/{_segment(owner)}/{_segment(repository)}/pulls/{pull_number}/comments" + return await self._request("POST", path, payload) + + async def list_check_runs_for_ref( + self, + owner: str, + repository: str, + ref: str, + *, + check_name: str, + ) -> list[dict[str, Any]]: + """List recent matching check runs for external-id recovery.""" + path = ( + f"/repos/{_segment(owner)}/{_segment(repository)}/commits/" + f"{_segment(ref)}/check-runs" + ) + payload = await self._request( + "GET", + path, + params={"check_name": check_name, "filter": "latest", "per_page": "100"}, + ) + return list(payload.get("check_runs", [])) + + async def list_review_comments( + self, + owner: str, + repository: str, + pull_number: int, + ) -> list[dict[str, Any]]: + """List the latest comments used to suppress retry duplicates.""" + path = f"/repos/{_segment(owner)}/{_segment(repository)}/pulls/{pull_number}/comments" + payload = await self._request( + "GET", + path, + params={ + "per_page": "100", + "sort": "created", + "direction": "desc", + }, + ) + return list(payload) + + async def _request( + self, + method: str, + path: str, + payload: dict[str, Any] | None = None, + *, + params: dict[str, str] | None = None, + ) -> Any: + token = await self.token_provider.get_token(self.installation_id) + response = await self._client.request( + method, + path, + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + }, + json=payload, + params=params, + ) + _raise_for_github(response) + return response.json() + + async def close(self) -> None: + if self._owns_client: + await self._client.aclose() + + +def _segment(value: str) -> str: + return quote(value, safe="") + + +def _raise_for_github(response: httpx.Response) -> None: + if response.is_success: + return + request_id = response.headers.get("x-github-request-id", "") + body = response.text[:2_000] + raise GitHubApiError( + f"GitHub API {response.status_code} request_id={request_id!r}: {body}", + status_code=response.status_code, + ) diff --git a/examples/code_review_agent/github_integration/models.py b/examples/code_review_agent/github_integration/models.py new file mode 100644 index 000000000..cd8b6255a --- /dev/null +++ b/examples/code_review_agent/github_integration/models.py @@ -0,0 +1,71 @@ +"""Validated domain models for supported GitHub webhook payloads.""" + +from __future__ import annotations + +from pydantic import BaseModel, Field, field_validator + +SUPPORTED_PULL_REQUEST_ACTIONS = { + "opened", + "reopened", + "synchronize", + "ready_for_review", +} + + +class GitHubPullRequestEvent(BaseModel): + """Minimal immutable coordinates needed to review one pull request head.""" + + delivery_id: str = Field(min_length=1, max_length=128) + action: str = Field(min_length=1, max_length=80) + installation_id: int = Field(gt=0) + repository_full_name: str = Field(pattern=r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") + owner: str + repository: str + pull_number: int = Field(gt=0) + base_sha: str = Field(pattern=r"^[0-9a-fA-F]{40,64}$") + head_sha: str = Field(pattern=r"^[0-9a-fA-F]{40,64}$") + base_clone_url: str + head_clone_url: str + draft: bool = False + + @field_validator("owner", "repository") + @classmethod + def validate_repository_component(cls, value: str) -> str: + if not value or any(character not in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.-" + for character in value): + raise ValueError("invalid GitHub repository component") + return value + + @classmethod + def from_webhook( + cls, + *, + delivery_id: str, + event_name: str, + payload: dict, + ) -> GitHubPullRequestEvent | None: + """Return a supported PR event, or none for events/actions to ignore.""" + if event_name != "pull_request": + return None + action = str(payload.get("action", "")) + if action not in SUPPORTED_PULL_REQUEST_ACTIONS: + return None + pull_request = payload["pull_request"] + repository = payload["repository"] + installation = payload["installation"] + full_name = str(repository["full_name"]) + owner, repository_name = full_name.split("/", maxsplit=1) + return cls( + delivery_id=delivery_id, + action=action, + installation_id=int(installation["id"]), + repository_full_name=full_name, + owner=owner, + repository=repository_name, + pull_number=int(payload["number"]), + base_sha=str(pull_request["base"]["sha"]), + head_sha=str(pull_request["head"]["sha"]), + base_clone_url=str(pull_request["base"]["repo"]["clone_url"]), + head_clone_url=str(pull_request["head"]["repo"]["clone_url"]), + draft=bool(pull_request.get("draft", False)), + ) diff --git a/examples/code_review_agent/github_integration/publisher.py b/examples/code_review_agent/github_integration/publisher.py new file mode 100644 index 000000000..b6f635678 --- /dev/null +++ b/examples/code_review_agent/github_integration/publisher.py @@ -0,0 +1,260 @@ +"""Translate normalized findings into GitHub Checks and review comments.""" + +from __future__ import annotations + +import hashlib +from datetime import timezone +from typing import Any + +from ..code_review.models import Finding, ReviewRun, ReviewStatus, Severity +from .client import GitHubClient +from .models import GitHubPullRequestEvent + +_ANNOTATION_BATCH_SIZE = 50 +_SEVERE = {Severity.CRITICAL, Severity.HIGH} + + +class GitHubReviewPublisher: + """Publish one check run and a bounded number of right-side PR comments.""" + + def __init__( + self, + client: GitHubClient, + *, + check_name: str = "tRPC Code Review", + publish_comments: bool = True, + max_comments: int = 20, + ): + self.client = client + self.check_name = check_name + self.publish_comments = publish_comments + if not 0 <= max_comments <= 100: + raise ValueError("max_comments must be between 0 and 100") + self.max_comments = max_comments + + async def create_started_check(self, event: GitHubPullRequestEvent) -> int: + response = await self.client.create_check_run( + event.owner, + event.repository, + { + "name": self.check_name, + "head_sha": event.head_sha, + "status": "in_progress", + "external_id": event.delivery_id, + "started_at": _now_iso(), + "output": { + "title": "Code review in progress", + "summary": "The signed pull request delivery is being reviewed.", + }, + }, + ) + return int(response["id"]) + + async def find_existing_check(self, event: GitHubPullRequestEvent) -> int | None: + """Recover a check created before a worker crash persisted its ID.""" + check_runs = await self.client.list_check_runs_for_ref( + event.owner, + event.repository, + event.head_sha, + check_name=self.check_name, + ) + for check_run in check_runs: + if str(check_run.get("external_id", "")) == event.delivery_id: + return int(check_run["id"]) + return None + + async def complete_check( + self, + event: GitHubPullRequestEvent, + review_run: ReviewRun, + check_run_id: int, + ) -> None: + annotations = _annotations(review_run) + summary = _summary(review_run) + batches = [ + annotations[index:index + _ANNOTATION_BATCH_SIZE] + for index in range(0, len(annotations), _ANNOTATION_BATCH_SIZE) + ] or [[]] + for batch in batches[:-1]: + await self.client.update_check_run( + event.owner, + event.repository, + check_run_id, + { + "status": "in_progress", + "output": { + "title": "Code review findings", + "summary": summary, + "annotations": batch, + }, + }, + ) + await self.client.update_check_run( + event.owner, + event.repository, + check_run_id, + { + "status": "completed", + "conclusion": _conclusion(review_run), + "completed_at": _now_iso(), + "output": { + "title": "Code review completed", + "summary": summary, + "annotations": batches[-1], + }, + }, + ) + + async def fail_check( + self, + event: GitHubPullRequestEvent, + check_run_id: int, + message: str, + ) -> None: + await self.client.update_check_run( + event.owner, + event.repository, + check_run_id, + { + "status": "completed", + "conclusion": "failure", + "completed_at": _now_iso(), + "output": { + "title": "Code review failed", + "summary": _truncate_utf8(message, 65_535), + }, + }, + ) + + async def publish_line_comments( + self, + event: GitHubPullRequestEvent, + review_run: ReviewRun, + ) -> int: + if not self.publish_comments: + return 0 + existing_comments = await self.client.list_review_comments( + event.owner, + event.repository, + event.pull_number, + ) + existing_bodies = { + str(comment.get("body", "")) + for comment in existing_comments + if isinstance(comment, dict) + } + published = 0 + considered = 0 + for finding, line in _publishable_locations(review_run): + if considered >= self.max_comments: + break + considered += 1 + marker = _comment_marker(review_run.id, finding, line) + if any(marker in body for body in existing_bodies): + continue + await self.client.create_review_comment( + event.owner, + event.repository, + event.pull_number, + { + "body": _comment_body(review_run.id, finding, line), + "commit_id": event.head_sha, + "path": finding.file_path, + "line": line, + "side": "RIGHT", + }, + ) + published += 1 + return published + + +def _annotations(review_run: ReviewRun) -> list[dict[str, Any]]: + annotations = [] + for finding, line in _publishable_locations(review_run): + annotations.append({ + "path": finding.file_path, + "start_line": line, + "end_line": line, + "annotation_level": _annotation_level(finding.severity), + "title": finding.title[:255], + "message": _truncate_utf8(finding.description, 65_535), + "raw_details": _truncate_utf8(finding.suggestion, 65_535), + }) + return annotations + + +def _publishable_locations(review_run: ReviewRun): + changed_lines = {changed_file.path: changed_file.changed_new_lines for changed_file in review_run.changed_files} + for finding in review_run.output.findings: + if not finding.publishable or finding.start_line is None or finding.end_line is None: + continue + valid_lines = changed_lines.get(finding.file_path, set()) + line = next( + (candidate for candidate in range(finding.start_line, finding.end_line + 1) if candidate in valid_lines), + None, + ) + if line is not None: + yield finding, line + + +def _summary(review_run: ReviewRun) -> str: + publishable = sum(finding.publishable for finding in review_run.output.findings) + summary = review_run.output.summary or "No model summary was produced." + return _truncate_utf8( + f"{summary}\n\n" + f"- Findings: {len(review_run.output.findings)}\n" + f"- Line annotations: {publishable}\n" + f"- Review run: `{review_run.id}`", + 65_535, + ) + + +def _conclusion(review_run: ReviewRun) -> str: + if review_run.status != ReviewStatus.COMPLETED: + return "failure" + if any(finding.publishable and finding.severity in _SEVERE for finding in review_run.output.findings): + return "failure" + if review_run.output.findings: + return "neutral" + return "success" + + +def _annotation_level(severity: Severity) -> str: + if severity in _SEVERE: + return "failure" + if severity == Severity.MEDIUM: + return "warning" + return "notice" + + +def _comment_body(run_id: str, finding: Finding, line: int) -> str: + marker = _comment_marker(run_id, finding, line) + suggestion = f"\n\n**Suggestion:** {finding.suggestion}" if finding.suggestion else "" + return _truncate_utf8( + f"{marker}\n" + f"**[{finding.severity.value.upper()}] {finding.title}**\n\n" + f"{finding.description}{suggestion}\n\n" + f"`{finding.rule_id}` · confidence {finding.confidence:.2f}", + 65_000, + ) + + +def _comment_marker(run_id: str, finding: Finding, line: int) -> str: + identity = "\0".join( + (run_id, finding.rule_id, finding.file_path, str(line), finding.title) + ) + digest = hashlib.sha256(identity.encode("utf-8")).hexdigest()[:24] + return f"" + + +def _now_iso() -> str: + from datetime import datetime + + return datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") + + +def _truncate_utf8(value: str, max_bytes: int) -> str: + encoded = value.encode("utf-8") + if len(encoded) <= max_bytes: + return value + return encoded[:max_bytes].decode("utf-8", errors="ignore") diff --git a/examples/code_review_agent/github_integration/runtime.py b/examples/code_review_agent/github_integration/runtime.py new file mode 100644 index 000000000..5dea9cba5 --- /dev/null +++ b/examples/code_review_agent/github_integration/runtime.py @@ -0,0 +1,120 @@ +"""Environment-backed construction shared by GitHub worker processes.""" + +from __future__ import annotations + +import os +from dataclasses import asdict +from pathlib import Path + +from ..code_review.database import ReviewStore, sqlite_database_url +from ..code_review.orchestrator import ReviewConfig +from ..code_review.static_analysis import StaticAnalysisConfig, StaticAnalyzer +from .checkout import GitHubWorkspaceManager +from .client import GitHubAppTokenProvider, StaticTokenProvider +from .service import GitHubReviewService + + +def database_url_from_environment() -> str: + """Return the shared synchronous database URL.""" + return ( + os.getenv("CODE_REVIEW_DATABASE_URL") + or sqlite_database_url(Path(".code-review") / "github-reviews.db") + ) + + +def build_store_from_environment() -> ReviewStore: + """Open the durable review store.""" + return ReviewStore(database_url_from_environment()) + + +def build_service_from_environment(store: ReviewStore) -> GitHubReviewService: + """Build checkout, analyzer, model, and publication dependencies.""" + api_url = os.getenv("GITHUB_API_URL", "https://api.github.com") + publish_comments = env_bool("GITHUB_REVIEW_PUBLISH_COMMENTS", True) + token = os.getenv("GITHUB_TOKEN", "") + if token: + token_provider = StaticTokenProvider(token) + else: + token_provider = GitHubAppTokenProvider( + app_id=required_env("GITHUB_APP_ID"), + private_key=_load_private_key(), + api_url=api_url, + permissions={ + "contents": "read", + "checks": "write", + "pull_requests": "write" if publish_comments else "read", + }, + ) + + allowed_hosts = tuple( + host.strip() + for host in os.getenv("GITHUB_CLONE_HOSTS", "github.com").split(",") + if host.strip() + ) + workspace_manager = GitHubWorkspaceManager( + os.getenv("GITHUB_REVIEW_WORKSPACE_ROOT", ".code-review/workspaces"), + allowed_hosts=allowed_hosts, + command_timeout=float(os.getenv("GITHUB_REVIEW_GIT_TIMEOUT", "300")), + ) + static_config = StaticAnalysisConfig( + runtime=os.getenv("GITHUB_REVIEW_STATIC_RUNTIME", "docker"), + run_tests=env_bool("GITHUB_REVIEW_RUN_TESTS", False), + strict_tools=env_bool("GITHUB_REVIEW_STRICT_TOOLS", False), + timeout_seconds=float(os.getenv("GITHUB_REVIEW_STATIC_TIMEOUT", "120")), + docker_image=os.getenv("GITHUB_REVIEW_DOCKER_IMAGE", "trpc-code-review:latest"), + ) + analyzer = StaticAnalyzer(static_config) + reviewer = None + model_name = "" + if not env_bool("GITHUB_REVIEW_NO_LLM", False): + from ..agent.reviewer import review_with_llm + + reviewer = review_with_llm + model_name = os.getenv("TRPC_AGENT_MODEL_NAME", "") + return GitHubReviewService( + store=store, + token_provider=token_provider, + workspace_manager=workspace_manager, + reviewer=reviewer, + static_analyzer=analyzer.analyze, + review_config=ReviewConfig( + minimum_confidence=float( + os.getenv("GITHUB_REVIEW_MINIMUM_CONFIDENCE", "0.75") + ), + ), + model_name=model_name, + execution_config={ + "static_analysis": asdict(static_config), + "model_base_url": os.getenv("TRPC_AGENT_BASE_URL", "") if reviewer else "", + }, + api_url=api_url, + publish_comments=publish_comments, + max_comments=int(os.getenv("GITHUB_REVIEW_MAX_COMMENTS", "20")), + ) + + +def required_env(name: str) -> str: + value = os.getenv(name, "") + if not value: + raise ValueError(f"Missing required environment variable: {name}") + return value + + +def env_bool(name: str, default: bool) -> bool: + value = os.getenv(name) + if value is None: + return default + normalized = value.strip().casefold() + if normalized in {"1", "true", "yes", "on"}: + return True + if normalized in {"0", "false", "no", "off"}: + return False + raise ValueError(f"{name} must be a boolean value") + + +def _load_private_key() -> str: + inline = os.getenv("GITHUB_APP_PRIVATE_KEY", "") + if inline: + return inline.replace("\\n", "\n") + path = required_env("GITHUB_APP_PRIVATE_KEY_PATH") + return Path(path).expanduser().read_text(encoding="utf-8") diff --git a/examples/code_review_agent/github_integration/security.py b/examples/code_review_agent/github_integration/security.py new file mode 100644 index 000000000..baa52eb6f --- /dev/null +++ b/examples/code_review_agent/github_integration/security.py @@ -0,0 +1,21 @@ +"""GitHub webhook request authentication helpers.""" + +from __future__ import annotations + +import hashlib +import hmac + + +def verify_webhook_signature(body: bytes, secret: str, signature_header: str | None) -> None: + """Validate GitHub's HMAC-SHA256 signature using constant-time comparison.""" + if not secret: + raise ValueError("GitHub webhook secret is not configured") + if not signature_header or not signature_header.startswith("sha256="): + raise ValueError("Missing or invalid X-Hub-Signature-256 header") + expected = "sha256=" + hmac.new( + secret.encode("utf-8"), + body, + hashlib.sha256, + ).hexdigest() + if not hmac.compare_digest(expected, signature_header): + raise ValueError("GitHub webhook signature mismatch") diff --git a/examples/code_review_agent/github_integration/service.py b/examples/code_review_agent/github_integration/service.py new file mode 100644 index 000000000..ea6d1e804 --- /dev/null +++ b/examples/code_review_agent/github_integration/service.py @@ -0,0 +1,166 @@ +"""Orchestrate checkout, review, persistence, and GitHub publication.""" + +from __future__ import annotations + +import asyncio +import logging +from collections.abc import Callable +from typing import Any + +from ..code_review.database import ReviewStore +from ..code_review.orchestrator import ( + ReviewCallable, + ReviewConfig, + StaticAnalysisCallable, + run_review, +) +from .checkout import GitHubWorkspaceManager +from .client import GitHubClient, TokenProvider +from .models import GitHubPullRequestEvent +from .publisher import GitHubReviewPublisher + +GitHubClientFactory = Callable[[GitHubPullRequestEvent], GitHubClient] +logger = logging.getLogger(__name__) + + +class GitHubReviewService: + """Run the existing deterministic pipeline for one validated PR event.""" + + def __init__( + self, + *, + store: ReviewStore, + token_provider: TokenProvider, + workspace_manager: GitHubWorkspaceManager, + reviewer: ReviewCallable | None = None, + static_analyzer: StaticAnalysisCallable | None = None, + review_config: ReviewConfig | None = None, + model_name: str = "", + execution_config: dict[str, Any] | None = None, + api_url: str = "https://api.github.com", + publish_comments: bool = True, + max_comments: int = 20, + client_factory: GitHubClientFactory | None = None, + ): + self.store = store + self.token_provider = token_provider + self.workspace_manager = workspace_manager + self.reviewer = reviewer + self.static_analyzer = static_analyzer + self.review_config = review_config or ReviewConfig() + self.model_name = model_name + self.execution_config = execution_config or {} + self.api_url = api_url + self.publish_comments = publish_comments + self.max_comments = max_comments + self.client_factory = client_factory + + async def process(self, event: GitHubPullRequestEvent) -> None: + """Process a claimed delivery and persist every terminal outcome.""" + await asyncio.to_thread( + self.store.update_github_delivery, + event.delivery_id, + status="processing", + ) + client = (self.client_factory(event) if self.client_factory is not None else GitHubClient( + token_provider=self.token_provider, + installation_id=event.installation_id, + api_url=self.api_url, + )) + publisher = GitHubReviewPublisher( + client, + publish_comments=self.publish_comments, + max_comments=self.max_comments, + ) + workspace = None + delivery = await asyncio.to_thread( + self.store.get_github_delivery, + event.delivery_id, + ) + publication = await asyncio.to_thread( + self.store.get_github_publication, + event.delivery_id, + ) + check_completed = publication["check_completed"] + comments_completed = publication["comments_completed"] + check_run_id = delivery["check_run_id"] if delivery is not None else None + review_run_id = None + try: + if check_run_id is None: + check_run_id = await publisher.find_existing_check(event) + if check_run_id is None: + check_run_id = await publisher.create_started_check(event) + await asyncio.to_thread( + self.store.update_github_delivery, + event.delivery_id, + status="processing", + check_run_id=check_run_id, + error_message="", + ) + token = await self.token_provider.get_token(event.installation_id) + workspace = await asyncio.to_thread(self.workspace_manager.checkout, event, token) + review_run = await run_review( + repository=workspace, + repository_identity=f"github://{event.repository_full_name}", + base_revision=event.base_sha, + head_revision=event.head_sha, + config=self.review_config, + reviewer=self.reviewer, + static_analyzer=self.static_analyzer, + model_name=self.model_name, + execution_config={ + **self.execution_config, + "source": "github_webhook", + "repository": event.repository_full_name, + "pull_number": event.pull_number, + }, + ) + save_result = await asyncio.to_thread(self.store.save_run, review_run) + persisted_run = save_result.review_run + review_run_id = persisted_run.id + if not check_completed: + await publisher.complete_check(event, persisted_run, check_run_id) + await asyncio.to_thread( + self.store.update_github_publication, + event.delivery_id, + check_completed=True, + ) + check_completed = True + if not comments_completed: + await publisher.publish_line_comments(event, persisted_run) + await asyncio.to_thread( + self.store.update_github_publication, + event.delivery_id, + comments_completed=True, + ) + await asyncio.to_thread( + self.store.update_github_delivery, + event.delivery_id, + status="completed", + review_run_id=review_run_id, + check_run_id=check_run_id, + error_message="", + ) + except Exception as exc: + if check_run_id is not None and not check_completed: + try: + await publisher.fail_check(event, check_run_id, str(exc)) + except Exception: + logger.warning( + "Unable to mark GitHub check run %s as failed", + check_run_id, + exc_info=True, + ) + await asyncio.to_thread( + self.store.update_github_delivery, + event.delivery_id, + status="failed", + review_run_id=review_run_id, + check_run_id=check_run_id, + error_message=str(exc)[:10_000], + ) + raise + finally: + if workspace is not None: + await asyncio.to_thread(self.workspace_manager.cleanup, workspace) + await client.close() diff --git a/examples/code_review_agent/github_integration/worker.py b/examples/code_review_agent/github_integration/worker.py new file mode 100644 index 000000000..637568416 --- /dev/null +++ b/examples/code_review_agent/github_integration/worker.py @@ -0,0 +1,144 @@ +"""Durable GitHub review worker with leases, retries, and crash recovery.""" + +from __future__ import annotations + +import asyncio +import logging +import random +import socket +import uuid +from collections.abc import Callable + +import httpx +from pydantic import ValidationError + +from ..code_review.database import ClaimedReviewJob, ReviewStore +from .client import GitHubApiError +from .models import GitHubPullRequestEvent +from .service import GitHubReviewService + +logger = logging.getLogger(__name__) + + +class GitHubReviewWorker: + """Poll and execute durable review jobs with bounded retry behavior.""" + + def __init__( + self, + *, + store: ReviewStore, + service: GitHubReviewService, + worker_id: str | None = None, + lease_seconds: float = 300, + poll_seconds: float = 2, + base_retry_seconds: float = 5, + max_retry_seconds: float = 300, + jitter: Callable[[], float] = random.random, + ): + if lease_seconds <= 0 or poll_seconds <= 0: + raise ValueError("lease_seconds and poll_seconds must be positive") + if base_retry_seconds < 0 or max_retry_seconds < base_retry_seconds: + raise ValueError("invalid retry delay configuration") + self.store = store + self.service = service + self.worker_id = worker_id or f"{socket.gethostname()}-{uuid.uuid4().hex[:12]}" + self.lease_seconds = lease_seconds + self.poll_seconds = poll_seconds + self.base_retry_seconds = base_retry_seconds + self.max_retry_seconds = max_retry_seconds + self.jitter = jitter + self._stopping = asyncio.Event() + + async def run_forever(self) -> None: + """Process jobs until stop is requested or the task is cancelled.""" + while not self._stopping.is_set(): + processed = await self.run_once() + if not processed: + try: + await asyncio.wait_for(self._stopping.wait(), timeout=self.poll_seconds) + except TimeoutError: + pass + + def stop(self) -> None: + """Request graceful shutdown after the current job.""" + self._stopping.set() + + async def run_once(self) -> bool: + """Process at most one available job.""" + job = await asyncio.to_thread( + self.store.claim_github_job, + worker_id=self.worker_id, + lease_seconds=self.lease_seconds, + ) + if job is None: + return False + heartbeat = asyncio.create_task(self._heartbeat(job)) + try: + event = GitHubPullRequestEvent.model_validate(job.event_payload) + await self.service.process(event) + completed = await asyncio.to_thread( + self.store.complete_github_job, + job.delivery_id, + worker_id=self.worker_id, + ) + if not completed: + raise RuntimeError(f"GitHub review job lease was lost: {job.delivery_id}") + except asyncio.CancelledError: + raise + except Exception as exc: # noqa: BLE001 - job boundary must persist all failures + retryable = is_retryable_error(exc) + delay = self._retry_delay(job.attempt_count) + try: + state = await asyncio.to_thread( + self.store.fail_github_job, + job.delivery_id, + worker_id=self.worker_id, + error_message=str(exc), + retry_delay_seconds=delay, + retryable=retryable, + ) + except RuntimeError: + logger.exception("Unable to fail job after its lease was lost: %s", job.delivery_id) + else: + logger.warning( + "GitHub review job %s %s after attempt %s/%s: %s", + job.delivery_id, + "will retry" if state == "queued" else "is dead", + job.attempt_count, + job.max_attempts, + exc, + ) + finally: + heartbeat.cancel() + await asyncio.gather(heartbeat, return_exceptions=True) + return True + + async def _heartbeat(self, job: ClaimedReviewJob) -> None: + interval = max(0.1, self.lease_seconds / 3) + while True: + await asyncio.sleep(interval) + renewed = await asyncio.to_thread( + self.store.renew_github_job_lease, + job.delivery_id, + worker_id=self.worker_id, + lease_seconds=self.lease_seconds, + ) + if not renewed: + logger.error("Lost lease for GitHub review job %s", job.delivery_id) + return + + def _retry_delay(self, attempt_count: int) -> float: + exponential = min( + self.max_retry_seconds, + self.base_retry_seconds * (2 ** max(0, attempt_count - 1)), + ) + return exponential * (0.5 + self.jitter() * 0.5) + + +def is_retryable_error(error: Exception) -> bool: + """Classify transient infrastructure/API failures conservatively.""" + if isinstance(error, GitHubApiError): + return error.status_code == 429 or error.status_code >= 500 + if isinstance(error, (httpx.TransportError, TimeoutError, OSError)): + return True + return not isinstance(error, (ValidationError, ValueError, KeyError)) diff --git a/examples/code_review_agent/inspect_reviews.py b/examples/code_review_agent/inspect_reviews.py new file mode 100755 index 000000000..b0a057c86 --- /dev/null +++ b/examples/code_review_agent/inspect_reviews.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python3 +"""List and inspect persisted code review runs.""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path + +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from examples.code_review_agent.code_review.database import ( + ReviewStore, + sqlite_database_url, +) +from examples.code_review_agent.code_review.models import ReviewStatus +from examples.code_review_agent.code_review.reporter import render_markdown + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Inspect persisted code review runs.") + parser.add_argument( + "--database-url", + help=("Synchronous SQLAlchemy URL. Defaults to CODE_REVIEW_DATABASE_URL or " + ".code-review/reviews.db using SQLite."), + ) + subparsers = parser.add_subparsers(dest="command", required=True) + + list_parser = subparsers.add_parser("list", help="List recent review runs.") + list_parser.add_argument("--limit", type=int, default=20) + list_parser.add_argument("--repository") + list_parser.add_argument("--status", choices=tuple(status.value for status in ReviewStatus)) + + show_parser = subparsers.add_parser("show", help="Show one complete review run.") + show_parser.add_argument("run_id") + show_parser.add_argument("--format", choices=("json", "markdown"), default="markdown") + + delivery_parser = subparsers.add_parser("deliveries", help="List recent GitHub webhook deliveries.") + delivery_parser.add_argument("--limit", type=int, default=20) + delivery_parser.add_argument( + "--status", + choices=("received", "queued", "processing", "retrying", "completed", "failed", "ignored"), + ) + delivery_parser.add_argument("--repository") + + jobs_parser = subparsers.add_parser("jobs", help="List durable GitHub review jobs.") + jobs_parser.add_argument("--limit", type=int, default=20) + jobs_parser.add_argument( + "--status", + choices=("queued", "leased", "succeeded", "dead"), + ) + + replay_parser = subparsers.add_parser("replay", help="Requeue one dead GitHub review job.") + replay_parser.add_argument("delivery_id") + return parser + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + database_url = (args.database_url or os.getenv("CODE_REVIEW_DATABASE_URL") + or sqlite_database_url(Path(".code-review") / "reviews.db")) + store = ReviewStore(database_url) + try: + if args.command == "list": + status = ReviewStatus(args.status) if args.status else None + runs = store.list_runs( + limit=args.limit, + repository_path=args.repository, + status=status, + ) + if not runs: + print("No persisted review runs.") + return 0 + print("RUN ID STATUS STARTED FILES FINDINGS") + for review_run in runs: + started = review_run.started_at.isoformat(timespec="seconds") + print(f"{review_run.id:<36} {review_run.status.value:<10} " + f"{started:<26} {len(review_run.changed_files):>5} " + f"{len(review_run.output.findings):>8}") + return 0 + + if args.command == "deliveries": + deliveries = store.list_github_deliveries( + limit=args.limit, + status=args.status, + repository_full_name=args.repository, + ) + if not deliveries: + print("No GitHub webhook deliveries.") + return 0 + print("DELIVERY ID STATUS REPOSITORY PR") + for delivery in deliveries: + pull_number = delivery["pull_number"] or "" + print(f"{delivery['delivery_id']:<35} {delivery['status']:<11} " + f"{delivery['repository_full_name']:<26} {pull_number}") + return 0 + + if args.command == "jobs": + jobs = store.list_github_jobs(limit=args.limit, status=args.status) + if not jobs: + print("No durable GitHub review jobs.") + return 0 + print("DELIVERY ID STATUS ATTEMPTS AVAILABLE") + for job in jobs: + available = job["available_at"].isoformat(timespec="seconds") + attempts = f"{job['attempt_count']}/{job['max_attempts']}" + print( + f"{job['delivery_id']:<35} {job['status']:<11} " + f"{attempts:<9} {available}" + ) + return 0 + + if args.command == "replay": + if not store.replay_github_job(args.delivery_id): + print(f"Dead GitHub review job not found: {args.delivery_id}") + return 2 + print(f"Requeued GitHub review job: {args.delivery_id}") + return 0 + + review_run = store.get_run(args.run_id) + if review_run is None: + print(f"Review run not found: {args.run_id}") + return 2 + if args.format == "json": + print(json.dumps(review_run.model_dump(mode="json"), ensure_ascii=False, indent=2)) + else: + print(render_markdown(review_run), end="") + return 0 + finally: + store.close() + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/examples/code_review_agent/requirements-github.txt b/examples/code_review_agent/requirements-github.txt new file mode 100644 index 000000000..f124ac10d --- /dev/null +++ b/examples/code_review_agent/requirements-github.txt @@ -0,0 +1 @@ +PyJWT[crypto]>=2.10.1,<3 diff --git a/examples/code_review_agent/requirements-static.txt b/examples/code_review_agent/requirements-static.txt new file mode 100644 index 000000000..1c2295bc4 --- /dev/null +++ b/examples/code_review_agent/requirements-static.txt @@ -0,0 +1,3 @@ +ruff>=0.12.5,<1.0.0 +bandit>=1.8.6,<2.0.0 +pytest>=8.4.1,<9.0.0 diff --git a/examples/code_review_agent/run_github_webhook.py b/examples/code_review_agent/run_github_webhook.py new file mode 100755 index 000000000..7291d2ccb --- /dev/null +++ b/examples/code_review_agent/run_github_webhook.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python3 +"""Run the GitHub webhook receiver with Uvicorn.""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +import uvicorn + +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + + +def main() -> None: + uvicorn.run( + "examples.code_review_agent.github_integration.app:build_app_from_environment", + factory=True, + host=os.getenv("GITHUB_WEBHOOK_HOST", "127.0.0.1"), + port=int(os.getenv("GITHUB_WEBHOOK_PORT", "8080")), + log_level=os.getenv("GITHUB_WEBHOOK_LOG_LEVEL", "info"), + ) + + +if __name__ == "__main__": + main() diff --git a/examples/code_review_agent/run_github_worker.py b/examples/code_review_agent/run_github_worker.py new file mode 100755 index 000000000..627cbb6ca --- /dev/null +++ b/examples/code_review_agent/run_github_worker.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +"""Run durable GitHub code review workers.""" + +from __future__ import annotations + +import argparse +import asyncio +import logging +import os +import signal +import sys +from pathlib import Path + +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from examples.code_review_agent.github_integration.runtime import ( + build_service_from_environment, + build_store_from_environment, +) +from examples.code_review_agent.github_integration.worker import GitHubReviewWorker + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Run durable GitHub review jobs.") + parser.add_argument( + "--concurrency", + type=int, + default=int(os.getenv("GITHUB_REVIEW_WORKER_CONCURRENCY", "1")), + ) + parser.add_argument( + "--poll-seconds", + type=float, + default=float(os.getenv("GITHUB_REVIEW_POLL_SECONDS", "2")), + ) + parser.add_argument( + "--lease-seconds", + type=float, + default=float(os.getenv("GITHUB_REVIEW_LEASE_SECONDS", "300")), + ) + return parser + + +async def run_workers(*, concurrency: int, poll_seconds: float, lease_seconds: float) -> None: + if concurrency <= 0: + raise ValueError("concurrency must be positive") + store = build_store_from_environment() + service = build_service_from_environment(store) + workers = [ + GitHubReviewWorker( + store=store, + service=service, + worker_id=f"worker-{os.getpid()}-{index + 1}", + poll_seconds=poll_seconds, + lease_seconds=lease_seconds, + base_retry_seconds=float(os.getenv("GITHUB_REVIEW_RETRY_BASE_SECONDS", "5")), + max_retry_seconds=float(os.getenv("GITHUB_REVIEW_RETRY_MAX_SECONDS", "300")), + ) + for index in range(concurrency) + ] + loop = asyncio.get_running_loop() + for signal_name in (signal.SIGINT, signal.SIGTERM): + try: + loop.add_signal_handler(signal_name, lambda: [worker.stop() for worker in workers]) + except NotImplementedError: + pass + try: + await asyncio.gather(*(worker.run_forever() for worker in workers)) + finally: + close = getattr(service.token_provider, "close", None) + if close is not None: + await close() + store.close() + + +def main(argv: list[str] | None = None) -> None: + args = build_parser().parse_args(argv) + logging.basicConfig( + level=os.getenv("GITHUB_WORKER_LOG_LEVEL", "INFO"), + format="%(asctime)s %(levelname)s %(name)s %(message)s", + ) + asyncio.run( + run_workers( + concurrency=args.concurrency, + poll_seconds=args.poll_seconds, + lease_seconds=args.lease_seconds, + ) + ) + + +if __name__ == "__main__": + main() diff --git a/examples/code_review_agent/run_review.py b/examples/code_review_agent/run_review.py new file mode 100755 index 000000000..c8862bae3 --- /dev/null +++ b/examples/code_review_agent/run_review.py @@ -0,0 +1,171 @@ +#!/usr/bin/env python3 +"""Run a local, diff-only code review.""" + +from __future__ import annotations + +import argparse +import asyncio +import os +import sys +from dataclasses import asdict +from pathlib import Path + +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from examples.code_review_agent.code_review.models import ReviewStatus +from examples.code_review_agent.code_review.orchestrator import ReviewConfig, run_review +from examples.code_review_agent.code_review.reporter import write_reports + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Review the Git diff between two revisions.") + parser.add_argument("--repo", default=".", help="Path to a local Git work tree.") + parser.add_argument("--base", required=True, help="Base commit or revision.") + parser.add_argument("--head", default="HEAD", help="Head commit or revision (default: HEAD).") + parser.add_argument("--output-dir", default=".code-review", help="Report output directory.") + parser.add_argument("--no-llm", action="store_true", help="Collect and report the diff without calling a model.") + parser.add_argument( + "--direct-base", + action="store_true", + help="Compare the exact base commit instead of its merge base with head.", + ) + parser.add_argument("--context-lines", type=int, default=3) + parser.add_argument("--max-files", type=int, default=40) + parser.add_argument("--max-file-chars", type=int, default=24_000) + parser.add_argument("--max-total-chars", type=int, default=120_000) + parser.add_argument("--minimum-confidence", type=float, default=0.0) + parser.add_argument( + "--no-static-analysis", + action="store_true", + help="Disable Ruff and Bandit execution.", + ) + parser.add_argument( + "--static-runtime", + choices=("local", "docker"), + default="local", + help="Run static analyzers locally or in the hardened Docker image.", + ) + parser.add_argument("--run-tests", action="store_true", help="Also run Pytest in the selected static runtime.") + parser.add_argument( + "--strict-static-tools", + action="store_true", + help="Fail the review when an enabled analyzer is missing or fails.", + ) + parser.add_argument("--static-timeout", type=float, default=120.0) + parser.add_argument("--docker-image", default="trpc-code-review:latest") + parser.add_argument( + "--database-url", + help=("Synchronous SQLAlchemy URL. Defaults to CODE_REVIEW_DATABASE_URL or " + "/reviews.db using SQLite."), + ) + parser.add_argument( + "--no-persist", + action="store_true", + help="Do not persist the review run to the database.", + ) + return parser + + +async def async_main(args: argparse.Namespace) -> int: + reviewer = None + model_name = "" + if not args.no_llm: + from examples.code_review_agent.agent.reviewer import review_with_llm + + reviewer = review_with_llm + model_name = os.getenv("TRPC_AGENT_MODEL_NAME", "") + + static_analyzer = None + static_config = None + if not args.no_static_analysis: + from examples.code_review_agent.code_review.static_analysis import ( + StaticAnalysisConfig, + StaticAnalyzer, + ) + + static_config = StaticAnalysisConfig( + runtime=args.static_runtime, + run_tests=args.run_tests, + strict_tools=args.strict_static_tools, + timeout_seconds=args.static_timeout, + docker_image=args.docker_image, + ) + analyzer = StaticAnalyzer(static_config) + static_analyzer = analyzer.analyze + + config = ReviewConfig( + context_lines=args.context_lines, + use_merge_base=not args.direct_base, + max_files=args.max_files, + max_patch_chars_per_file=args.max_file_chars, + max_total_chars=args.max_total_chars, + minimum_confidence=args.minimum_confidence, + ) + review_run = await run_review( + repository=args.repo, + base_revision=args.base, + head_revision=args.head, + config=config, + reviewer=reviewer, + static_analyzer=static_analyzer, + model_name=model_name, + execution_config={ + "llm_enabled": reviewer is not None, + "model_base_url": os.getenv("TRPC_AGENT_BASE_URL", "") if reviewer is not None else "", + "static_analysis": asdict(static_config) if static_config is not None else None, + }, + ) + persistence_error = "" + persistence_created: bool | None = None + if not args.no_persist: + from examples.code_review_agent.code_review.database import ( + ReviewStore, + sqlite_database_url, + ) + + database_url = (args.database_url or os.getenv("CODE_REVIEW_DATABASE_URL") + or sqlite_database_url(Path(args.output_dir) / "reviews.db")) + store = None + try: + store = ReviewStore(database_url) + save_result = store.save_run(review_run) + review_run = save_result.review_run + persistence_created = save_result.created + except Exception as exc: # noqa: BLE001 - preserve a report when persistence fails + persistence_error = str(exc) + review_run.diagnostics.append(f"Persistence failed: {exc}") + finally: + if store is not None: + store.close() + + json_path, markdown_path = write_reports(review_run, args.output_dir) + print(f"Status: {review_run.status.value}") + print(f"Changed files: {len(review_run.changed_files)}") + print(f"Findings: {len(review_run.output.findings)}") + print(f"JSON report: {json_path}") + print(f"Markdown report: {markdown_path}") + if persistence_created is not None: + action = "created" if persistence_created else "reused existing idempotent run" + print(f"Persistence: {action}") + print(f"Persisted run ID: {review_run.id}") + if persistence_error: + print(f"Persistence error: {persistence_error}", file=sys.stderr) + if review_run.error_message: + print(f"Error: {review_run.error_message}", file=sys.stderr) + return 0 if review_run.status == ReviewStatus.COMPLETED and not persistence_error else 1 + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + if not 0.0 <= args.minimum_confidence <= 1.0: + raise SystemExit("--minimum-confidence must be between 0 and 1") + if args.context_lines < 0 or args.max_files < 1 or args.max_file_chars < 1 or args.max_total_chars < 1: + raise SystemExit("context and file limits must be positive") + if args.static_timeout <= 0: + raise SystemExit("--static-timeout must be positive") + return asyncio.run(async_main(args)) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/examples/code_review_agent/sandbox/Dockerfile b/examples/code_review_agent/sandbox/Dockerfile new file mode 100644 index 000000000..670555995 --- /dev/null +++ b/examples/code_review_agent/sandbox/Dockerfile @@ -0,0 +1,15 @@ +FROM python:3.12-slim + +ARG RUFF_VERSION=0.12.5 +ARG BANDIT_VERSION=1.8.6 +ARG PYTEST_VERSION=8.4.1 + +RUN python -m pip install --no-cache-dir \ + "ruff==${RUFF_VERSION}" \ + "bandit==${BANDIT_VERSION}" \ + "pytest==${PYTEST_VERSION}" + +WORKDIR /workspace +USER 65532:65532 + +ENTRYPOINT [] diff --git a/examples/code_review_agent/skills/python-correctness/SKILL.md b/examples/code_review_agent/skills/python-correctness/SKILL.md new file mode 100644 index 000000000..578f65679 --- /dev/null +++ b/examples/code_review_agent/skills/python-correctness/SKILL.md @@ -0,0 +1,39 @@ +--- +name: python-correctness +description: Review changed Python code for concrete behavioral defects, including invalid control flow, broken contracts, exception mistakes, state corruption, async/concurrency errors, resource leaks, and boundary-condition failures. Use when a diff contains Python source or tests and the task is to identify correctness regressions introduced by the change. +--- + +# Review Python Correctness + +Review only behavior introduced or changed by the diff. + +## Establish evidence + +1. Trace changed inputs through changed branches and outputs. +2. Compare new behavior with nearby call sites, annotations, tests, and error handling visible in the diff. +3. Report an issue only when a concrete input or execution path demonstrates failure. +4. Point to the exact statement in the supplied `ADDED LINE MAP`. Never use a nearby added line + merely because it is commentable. + +## Check high-value failure modes + +- Return values no longer match the visible contract or callers. +- Exceptions are swallowed, converted incorrectly, or raised after partial mutation. +- A new branch omits a required state transition, cleanup, or return. +- `None`, empty, zero, false, and boundary values take an invalid path. +- Mutable defaults, aliases, or shared state leak across calls. +- Async work is not awaited, cancellation is swallowed, or concurrent writes race. +- Files, locks, sessions, processes, or transactions are leaked on an error path. +- Indexing, iteration, timezone, encoding, or numeric assumptions fail at boundaries. + +## Reject weak findings + +Do not report: + +- Pure style preferences. +- Hypothetical behavior requiring code not shown in the diff. +- A warning already disproved by visible guards or tests. +- An old defect on an unchanged line. + +Assign `high` only when the changed path can corrupt data, break a primary flow, or fail broadly. +Use `medium` for reproducible functional defects and `low` for narrow edge cases. diff --git a/examples/code_review_agent/skills/python-correctness/agents/openai.yaml b/examples/code_review_agent/skills/python-correctness/agents/openai.yaml new file mode 100644 index 000000000..b3123335b --- /dev/null +++ b/examples/code_review_agent/skills/python-correctness/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Python Correctness Review" + short_description: "Find concrete correctness defects in Python diffs" + default_prompt: "Use $python-correctness to review this Python diff for correctness defects." diff --git a/examples/code_review_agent/skills/python-security/SKILL.md b/examples/code_review_agent/skills/python-security/SKILL.md new file mode 100644 index 000000000..9dcc2aac0 --- /dev/null +++ b/examples/code_review_agent/skills/python-security/SKILL.md @@ -0,0 +1,38 @@ +--- +name: python-security +description: Review changed Python code for exploitable security defects involving trust boundaries, injection, authentication, authorization, secrets, unsafe deserialization, path handling, network access, and sandbox escape. Use when a Python diff processes external input, executes commands, accesses files or databases, handles credentials, exposes an API, or changes security-sensitive behavior. +--- + +# Review Python Security + +Require an explicit source, dangerous operation, and missing or ineffective control. + +## Trace trust boundaries + +1. Identify attacker-controlled values from requests, events, files, environment variables, model output, or repository content. +2. Trace the value into a security-sensitive sink. +3. Verify that validation, encoding, parameterization, authorization, or isolation occurs before the sink. +4. Report the exact changed statement from the supplied `ADDED LINE MAP` that introduces or + exposes the exploitable path; never substitute a nearby commentable line. + +## Check high-value sinks + +- Shell/process execution, `eval`, `exec`, dynamic imports, or template evaluation. +- SQL, query languages, regular expressions, and structured command construction. +- Filesystem paths, archives, symlinks, uploads, and path traversal. +- Pickle/YAML/object deserialization or dynamic class construction. +- Authentication, authorization, tenant scoping, and ownership checks. +- Tokens, passwords, keys, logs, traces, and error responses. +- HTTP fetches, redirects, callback URLs, SSRF, and unrestricted egress. +- Docker/socket access, privileged mounts, writable host paths, and disabled isolation. +- Weak randomness, signature verification, TLS validation, and unsafe defaults. + +## Calibrate severity + +- `critical`: direct compromise with broad impact and little precondition. +- `high`: realistic exploit causing code execution, auth bypass, secret disclosure, or major data access. +- `medium`: meaningful exposure requiring additional conditions. +- `low`: defense-in-depth weakness with limited direct exploitability. + +Do not infer that any string is attacker controlled without evidence. Do not report a scanner +diagnostic when the visible code parameterizes, validates, or otherwise neutralizes the input. diff --git a/examples/code_review_agent/skills/python-security/agents/openai.yaml b/examples/code_review_agent/skills/python-security/agents/openai.yaml new file mode 100644 index 000000000..fa7fd3556 --- /dev/null +++ b/examples/code_review_agent/skills/python-security/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Python Security Review" + short_description: "Find exploitable security defects in Python diffs" + default_prompt: "Use $python-security to review this Python diff for security defects." diff --git a/examples/code_review_agent/skills/review-maintainability/SKILL.md b/examples/code_review_agent/skills/review-maintainability/SKILL.md new file mode 100644 index 000000000..e5295ca91 --- /dev/null +++ b/examples/code_review_agent/skills/review-maintainability/SKILL.md @@ -0,0 +1,35 @@ +--- +name: review-maintainability +description: Review changed code for material maintainability risks that are likely to cause defects or costly future changes, such as duplicated policy, incompatible abstractions, hidden coupling, unbounded complexity, and misleading interfaces. Use for non-trivial diffs where long-term change safety matters; do not use for cosmetic style commentary. +--- + +# Review Maintainability + +Report maintainability only when the diff creates a concrete future failure mode or makes a +required change unsafe across multiple locations. + +## Evaluate the change boundary + +1. Identify the behavior or policy being added. +2. Check whether the same rule is now encoded in multiple changed locations. +3. Check whether names, types, and interfaces still communicate the actual contract. +4. Check whether callers must know internal sequencing or representation details. +5. Explain the next realistic change that would break or require synchronized edits. +6. Locate the exact supporting statement in the supplied `ADDED LINE MAP`; do not attach the + finding to a nearby added line. + +## Report material risks + +- Duplicated authorization, validation, serialization, or lifecycle policy. +- A public interface that accepts states the implementation cannot safely handle. +- Hidden global state or order-dependent initialization. +- Branching or nesting that obscures cleanup, errors, or invariants. +- Cross-layer coupling that bypasses the existing abstraction boundary. +- Configuration spread across code paths with conflicting defaults. +- Compatibility behavior implemented without a bounded removal or migration path. + +## Avoid noise + +Do not report naming, formatting, line length, minor duplication, or personal design preferences. +Do not request a broad refactor when a small local correction removes the concrete risk. +Default to `low`; use `medium` only when the design is already likely to cause incorrect changes. diff --git a/examples/code_review_agent/skills/review-maintainability/agents/openai.yaml b/examples/code_review_agent/skills/review-maintainability/agents/openai.yaml new file mode 100644 index 000000000..c0175df37 --- /dev/null +++ b/examples/code_review_agent/skills/review-maintainability/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Maintainability Review" + short_description: "Find costly maintainability risks in changed code" + default_prompt: "Use $review-maintainability to review this diff for material maintainability risks." diff --git a/examples/code_review_agent/skills/review-test-coverage/SKILL.md b/examples/code_review_agent/skills/review-test-coverage/SKILL.md new file mode 100644 index 000000000..d25fd31fb --- /dev/null +++ b/examples/code_review_agent/skills/review-test-coverage/SKILL.md @@ -0,0 +1,38 @@ +--- +name: review-test-coverage +description: Review a code diff for important missing or ineffective tests around changed behavior, regressions, error paths, boundaries, security controls, and compatibility guarantees. Use when production behavior changes, a defect is fixed, a new branch or public contract is added, or existing tests are modified without clearly exercising the risky path. +--- + +# Review Test Coverage + +Request tests only for behavior whose failure would matter and whose coverage gap is visible. + +## Derive test obligations + +1. List observable behavior introduced or changed by the diff. +2. Identify the highest-risk success, failure, boundary, and compatibility paths. +3. Match visible tests to those obligations. +4. Report a gap only when no visible assertion exercises a material changed path. +5. Point to the exact production statement in the supplied `ADDED LINE MAP` that creates the + untested obligation, not a nearby added line or an arbitrary test file line. + +## Prioritize + +- Security checks and authorization denials. +- Regression fixes that lack a reproducing test. +- New exception, rollback, timeout, retry, and cleanup paths. +- Zero, empty, maximum, malformed, and concurrent inputs. +- Schema, protocol, persistence, or public API compatibility. +- Failure behavior of external dependencies. + +## Avoid weak requests + +Do not ask for: + +- Coverage of trivial getters or declarative constants. +- Tests merely to increase a percentage. +- Duplicates of an existing test visible in the diff. +- Tests for speculative behavior outside the available context. + +Use `medium` for an untested path likely to regress a material behavior. Use `low` for a narrower +gap. Describe the input, action, and assertion required for one focused regression test. diff --git a/examples/code_review_agent/skills/review-test-coverage/agents/openai.yaml b/examples/code_review_agent/skills/review-test-coverage/agents/openai.yaml new file mode 100644 index 000000000..12d8542c2 --- /dev/null +++ b/examples/code_review_agent/skills/review-test-coverage/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Test Coverage Review" + short_description: "Find missing tests for changed behavior and risks" + default_prompt: "Use $review-test-coverage to identify important missing tests for this diff." diff --git a/tests/code_review/__init__.py b/tests/code_review/__init__.py new file mode 100644 index 000000000..02995b404 --- /dev/null +++ b/tests/code_review/__init__.py @@ -0,0 +1 @@ +"""Tests for the local code review example.""" diff --git a/tests/code_review/conftest.py b/tests/code_review/conftest.py new file mode 100644 index 000000000..1f4f9a896 --- /dev/null +++ b/tests/code_review/conftest.py @@ -0,0 +1,45 @@ +"""Shared Git repository fixtures.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + + +def git(repository: Path, *args: str) -> str: + result = subprocess.run( + ["git", "-C", str(repository), *args], + check=True, + capture_output=True, + text=True, + ) + return result.stdout.strip() + + +@pytest.fixture +def sample_repository(tmp_path: Path) -> tuple[Path, str, str]: + repository = tmp_path / "repository" + repository.mkdir() + git(repository, "init", "-q") + git(repository, "config", "user.email", "review@example.com") + git(repository, "config", "user.name", "Review Test") + source = repository / "app.py" + source.write_text("def divide(a, b):\n return a / b\n", encoding="utf-8") + git(repository, "add", "app.py") + git(repository, "commit", "-qm", "base") + base = git(repository, "rev-parse", "HEAD") + + source.write_text( + "def divide(a, b):\n" + " if b == 0:\n" + " return None\n" + " return a / b\n", + encoding="utf-8", + ) + (repository / "README.md").write_text("# Demo\n", encoding="utf-8") + git(repository, "add", "app.py", "README.md") + git(repository, "commit", "-qm", "head") + head = git(repository, "rev-parse", "HEAD") + return repository, base, head diff --git a/tests/code_review/test_context_builder.py b/tests/code_review/test_context_builder.py new file mode 100644 index 000000000..769da4f7f --- /dev/null +++ b/tests/code_review/test_context_builder.py @@ -0,0 +1,44 @@ +"""Prompt context filtering and budget tests.""" + +from examples.code_review_agent.code_review.context_builder import ( + ContextBudget, + build_review_context, +) +from examples.code_review_agent.code_review.models import ChangedFile, ChangeType + + +def _file(path: str, patch: str, *, binary: bool = False) -> ChangedFile: + return ChangedFile( + path=path, + change_type=ChangeType.MODIFIED, + language="python", + patch=patch, + is_binary=binary, + ) + + +def test_skips_binary_and_generated_paths() -> None: + context = build_review_context([ + _file("src/app.py", "+print('ok')\n"), + _file("vendor/lib.py", "+ignored\n"), + _file("image.png", "", binary=True), + ]) + + assert context.included_files == ("src/app.py", ) + assert set(context.skipped_files) == {"vendor/lib.py", "image.png"} + assert "### FILE: src/app.py" in context.text + assert "ADDED LINE MAP" in context.text + + +def test_truncates_each_patch_without_exceeding_total_budget() -> None: + changed_file = _file("src/app.py", "+" + ("x" * 500)) + context = build_review_context( + [changed_file], + ContextBudget(max_files=2, max_patch_chars_per_file=80, max_total_chars=500), + ) + + assert context.included_files == ("src/app.py", ) + assert context.truncated_files == ("src/app.py", ) + assert "[PATCH TRUNCATED]" in context.text + assert len(changed_file.patch) <= 80 + assert len(context.text) <= 500 diff --git a/tests/code_review/test_database.py b/tests/code_review/test_database.py new file mode 100644 index 000000000..e5d489f65 --- /dev/null +++ b/tests/code_review/test_database.py @@ -0,0 +1,263 @@ +"""SQLite persistence and idempotency integration tests.""" + +import subprocess +import sys +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import pytest +from sqlalchemy import select, update +from sqlalchemy.orm import Session + +from examples.code_review_agent.code_review.database import ( + SCHEMA_VERSION, + GitHubReviewJobRecord, + ReviewSchemaVersion, + ReviewStore, + sqlite_database_url, +) +from examples.code_review_agent.code_review.models import ReviewStatus +from examples.code_review_agent.code_review.orchestrator import run_review + + +@pytest.mark.asyncio +async def test_round_trips_complete_review_graph( + sample_repository: tuple[Path, str, str], + tmp_path: Path, +) -> None: + repository, base, head = sample_repository + review_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + ) + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + + saved = store.save_run(review_run) + loaded = store.get_run(review_run.id) + + assert saved.created is True + assert loaded is not None + assert loaded.model_dump(mode="json") == saved.review_run.model_dump(mode="json") + assert len(loaded.changed_files) == 2 + assert loaded.changed_files[0].hunks + assert loaded.resolved_head_revision == head + store.close() + + +@pytest.mark.asyncio +async def test_duplicate_idempotency_key_returns_existing_run( + sample_repository: tuple[Path, str, str], + tmp_path: Path, +) -> None: + repository, base, head = sample_repository + first_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + ) + second_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + ) + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + + first = store.save_run(first_run) + duplicate = store.save_run(second_run) + + assert first.created is True + assert duplicate.created is False + assert duplicate.review_run.id == first_run.id + assert first_run.idempotency_key == second_run.idempotency_key + assert store.count_runs() == 1 + store.close() + + +@pytest.mark.asyncio +async def test_lists_and_filters_runs( + sample_repository: tuple[Path, str, str], + tmp_path: Path, +) -> None: + repository, base, head = sample_repository + completed = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + ) + failed = await run_review( + repository=repository, + base_revision="missing-revision", + head_revision=head, + ) + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + store.save_run(completed) + store.save_run(failed) + + failed_runs = store.list_runs( + repository_path=str(repository), + status=ReviewStatus.FAILED, + ) + + assert [run.id for run in failed_runs] == [failed.id] + assert failed_runs[0].error_message + assert store.get_by_idempotency_key(completed.idempotency_key).id == completed.id + store.close() + + +def test_initializes_schema_version_and_validates_limits(tmp_path: Path) -> None: + database_url = sqlite_database_url(tmp_path / "nested" / "reviews.db") + store = ReviewStore(database_url) + + with Session(store.engine) as session: + marker = session.scalar(select(ReviewSchemaVersion)) + + assert marker is not None + assert marker.version == SCHEMA_VERSION + with pytest.raises(ValueError): + store.list_runs(limit=0) + store.close() + + with pytest.raises(ValueError, match="synchronous"): + ReviewStore("sqlite+aiosqlite:///:memory:") + + version_store = ReviewStore(database_url) + with Session(version_store.engine) as session, session.begin(): + session.get(ReviewSchemaVersion, 1).version = 1 + version_store.close() + migrated_store = ReviewStore(database_url) + with Session(migrated_store.engine) as session: + assert session.get(ReviewSchemaVersion, 1).version == SCHEMA_VERSION + migrated_store.close() + + version_store = ReviewStore(database_url) + with Session(version_store.engine) as session, session.begin(): + session.get(ReviewSchemaVersion, 1).version = SCHEMA_VERSION + 1 + version_store.close() + with pytest.raises(RuntimeError, match="Unsupported"): + ReviewStore(database_url) + + +def test_durable_job_lease_recovery_and_replay(tmp_path: Path) -> None: + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + event_payload = { + "delivery_id": "delivery-job", + "action": "opened", + "installation_id": 123, + "repository_full_name": "octo/demo", + "owner": "octo", + "repository": "demo", + "pull_number": 17, + "base_sha": "a" * 40, + "head_sha": "b" * 40, + "base_clone_url": "https://github.com/octo/demo.git", + "head_clone_url": "https://github.com/octo/demo.git", + "draft": False, + } + assert store.enqueue_github_delivery( + delivery_id="delivery-job", + event_name="pull_request", + action="opened", + payload_sha256="f" * 64, + event_payload=event_payload, + repository_full_name="octo/demo", + pull_number=17, + head_sha="b" * 40, + installation_id=123, + max_attempts=2, + ) + assert not store.enqueue_github_delivery( + delivery_id="delivery-job", + event_name="pull_request", + action="opened", + payload_sha256="f" * 64, + event_payload=event_payload, + repository_full_name="octo/demo", + pull_number=17, + head_sha="b" * 40, + installation_id=123, + ) + + first = store.claim_github_job(worker_id="worker-a", lease_seconds=30) + assert first is not None + assert first.attempt_count == 1 + assert store.renew_github_job_lease( + "delivery-job", + worker_id="worker-a", + lease_seconds=30, + ) + assert store.claim_github_job(worker_id="worker-b") is None + + with Session(store.engine) as session, session.begin(): + session.execute( + update(GitHubReviewJobRecord) + .where(GitHubReviewJobRecord.delivery_id == "delivery-job") + .values(lease_expires_at=datetime.now(timezone.utc) - timedelta(seconds=1)) + ) + recovered = store.claim_github_job(worker_id="worker-b", lease_seconds=30) + assert recovered is not None + assert recovered.attempt_count == 2 + assert not store.complete_github_job("delivery-job", worker_id="worker-a") + assert ( + store.fail_github_job( + "delivery-job", + worker_id="worker-b", + error_message="still failing", + retry_delay_seconds=0, + ) + == "dead" + ) + assert store.replay_github_job("delivery-job") + replayed = store.claim_github_job(worker_id="worker-c", lease_seconds=30) + assert replayed is not None + assert replayed.attempt_count == 1 + assert store.complete_github_job("delivery-job", worker_id="worker-c") + assert store.get_github_job("delivery-job")["status"] == "succeeded" + store.close() + + +def test_cli_persists_and_inspects_idempotent_run( + sample_repository: tuple[Path, str, str], + tmp_path: Path, +) -> None: + repository, base, head = sample_repository + database_url = sqlite_database_url(tmp_path / "reviews.db") + output_directory = tmp_path / "reports" + run_command = [ + sys.executable, + "examples/code_review_agent/run_review.py", + "--repo", + str(repository), + "--base", + base, + "--head", + head, + "--output-dir", + str(output_directory), + "--database-url", + database_url, + "--no-llm", + "--no-static-analysis", + ] + + first = subprocess.run(run_command, check=True, capture_output=True, text=True) + second = subprocess.run(run_command, check=True, capture_output=True, text=True) + listed = subprocess.run( + [ + sys.executable, + "examples/code_review_agent/inspect_reviews.py", + "--database-url", + database_url, + "list", + ], + check=True, + capture_output=True, + text=True, + ) + + assert "Persistence: created" in first.stdout + assert "Persistence: reused existing idempotent run" in second.stdout + assert "completed" in listed.stdout + store = ReviewStore(database_url) + assert store.count_runs() == 1 + store.close() diff --git a/tests/code_review/test_git_diff.py b/tests/code_review/test_git_diff.py new file mode 100644 index 000000000..cdd74a86b --- /dev/null +++ b/tests/code_review/test_git_diff.py @@ -0,0 +1,84 @@ +"""Git collection and unified patch parsing tests.""" + +from pathlib import Path + +import pytest + +from examples.code_review_agent.code_review.git_diff import ( + GitDiffCollector, + GitDiffError, + parse_unified_patch, +) +from examples.code_review_agent.code_review.models import ChangeType, LineChangeType + +from .conftest import git + + +def test_collects_changed_files_and_new_line_numbers(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + + effective_base, files = GitDiffCollector(repository).collect(base, head) + + assert effective_base == base + assert [changed.path for changed in files] == ["README.md", "app.py"] + app = next(changed for changed in files if changed.path == "app.py") + assert app.change_type == ChangeType.MODIFIED + assert app.language == "python" + assert app.added_lines == 2 + assert app.deleted_lines == 0 + assert app.changed_new_lines == {2, 3} + + +def test_parse_patch_tracks_added_deleted_and_context_lines() -> None: + patch = ("diff --git a/a.py b/a.py\n" + "--- a/a.py\n" + "+++ b/a.py\n" + "@@ -1,3 +1,3 @@\n" + " keep\n" + "-old\n" + "+new\n" + " end\n") + + hunks, added, deleted, is_binary = parse_unified_patch(patch) + + assert (added, deleted, is_binary) == (1, 1, False) + assert [line.change_type for line in hunks[0].lines] == [ + LineChangeType.CONTEXT, + LineChangeType.DELETED, + LineChangeType.ADDED, + LineChangeType.CONTEXT, + ] + assert hunks[0].lines[2].new_line == 2 + + +def test_rejects_revision_that_looks_like_an_option(sample_repository: tuple[Path, str, str], ) -> None: + repository, _, _ = sample_repository + collector = GitDiffCollector(repository) + + with pytest.raises(GitDiffError, match="Invalid Git revision"): + collector.resolve_revision("--output=/tmp/surprise") + + +def test_collects_renamed_file_with_spaces(tmp_path: Path) -> None: + repository = tmp_path / "rename-repository" + repository.mkdir() + git(repository, "init", "-q") + git(repository, "config", "user.email", "review@example.com") + git(repository, "config", "user.name", "Review Test") + old_path = repository / "old name.py" + old_path.write_text("value = 1\nkeep = 2\nother = 3\n", encoding="utf-8") + git(repository, "add", "old name.py") + git(repository, "commit", "-qm", "base") + base = git(repository, "rev-parse", "HEAD") + git(repository, "mv", "old name.py", "new name.py") + (repository / "new name.py").write_text("value = 4\nkeep = 2\nother = 3\n", encoding="utf-8") + git(repository, "commit", "-qam", "rename") + head = git(repository, "rev-parse", "HEAD") + + _, files = GitDiffCollector(repository).collect(base, head) + + assert len(files) == 1 + assert files[0].change_type == ChangeType.RENAMED + assert files[0].old_path == "old name.py" + assert files[0].path == "new name.py" + assert files[0].changed_new_lines == {1} diff --git a/tests/code_review/test_github_integration.py b/tests/code_review/test_github_integration.py new file mode 100644 index 000000000..613243de1 --- /dev/null +++ b/tests/code_review/test_github_integration.py @@ -0,0 +1,462 @@ +"""GitHub webhook security, API, checkout, and orchestration tests.""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import sys +from pathlib import Path +from types import SimpleNamespace + +import httpx +import pytest +from fastapi.testclient import TestClient + +from examples.code_review_agent.code_review.database import ( + ReviewStore, + sqlite_database_url, +) +from examples.code_review_agent.code_review.models import ( + Finding, + ReviewOutput, + Severity, +) +from examples.code_review_agent.github_integration.app import create_webhook_app +from examples.code_review_agent.github_integration.checkout import ( + CheckoutError, + GitHubWorkspaceManager, +) +from examples.code_review_agent.github_integration.client import ( + GITHUB_API_VERSION, + GitHubApiError, + GitHubAppTokenProvider, + GitHubClient, + StaticTokenProvider, +) +from examples.code_review_agent.github_integration.models import GitHubPullRequestEvent +from examples.code_review_agent.github_integration.security import ( + verify_webhook_signature, +) +from examples.code_review_agent.github_integration.service import GitHubReviewService +from examples.code_review_agent.github_integration.worker import ( + GitHubReviewWorker, + is_retryable_error, +) + + +def _payload(base: str, head: str, *, action: str = "opened", draft: bool = False) -> dict: + return { + "action": action, + "number": 17, + "installation": { + "id": 123 + }, + "repository": { + "full_name": "octo/demo" + }, + "pull_request": { + "draft": draft, + "base": { + "sha": base, + "repo": { + "clone_url": "https://github.com/octo/demo.git" + }, + }, + "head": { + "sha": head, + "repo": { + "clone_url": "https://github.com/contributor/demo.git" + }, + }, + }, + } + + +def _signature(body: bytes, secret: str) -> str: + return "sha256=" + hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() + + +def test_signature_matches_github_official_vector() -> None: + verify_webhook_signature( + b"Hello, World!", + "It's a Secret to Everybody", + "sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17", + ) + with pytest.raises(ValueError, match="mismatch"): + verify_webhook_signature(b"tampered", "secret", "sha256=" + "0" * 64) + + +def test_parses_only_supported_pull_request_actions() -> None: + base, head = "a" * 40, "b" * 40 + event = GitHubPullRequestEvent.from_webhook( + delivery_id="delivery-1", + event_name="pull_request", + payload=_payload(base, head), + ) + + assert event is not None + assert event.repository_full_name == "octo/demo" + assert event.pull_number == 17 + assert GitHubPullRequestEvent.from_webhook( + delivery_id="delivery-2", + event_name="pull_request", + payload=_payload(base, head, action="closed"), + ) is None + + +def test_webhook_authenticates_deduplicates_and_ignores_drafts(tmp_path: Path) -> None: + secret = "webhook-secret" + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + app = create_webhook_app(store=store, webhook_secret=secret) + base, head = "a" * 40, "b" * 40 + payload = _payload(base, head) + body = json.dumps(payload).encode() + headers = { + "X-Hub-Signature-256": _signature(body, secret), + "X-GitHub-Delivery": "delivery-1", + "X-GitHub-Event": "pull_request", + "Content-Type": "application/json", + } + + with TestClient(app) as client: + ready = client.get("/readyz") + accepted = client.post("/webhooks/github", content=body, headers=headers) + duplicate = client.post("/webhooks/github", content=body, headers=headers) + conflicting_body = json.dumps(_payload(base, "c" * 40)).encode() + conflict = client.post( + "/webhooks/github", + content=conflicting_body, + headers={ + **headers, + "X-Hub-Signature-256": _signature(conflicting_body, secret), + }, + ) + rejected = client.post( + "/webhooks/github", + content=body, + headers={ + **headers, "X-Hub-Signature-256": "sha256=" + "0" * 64 + }, + ) + draft_body = json.dumps(_payload(base, head, draft=True)).encode() + ignored = client.post( + "/webhooks/github", + content=draft_body, + headers={ + **headers, + "X-GitHub-Delivery": "delivery-2", + "X-Hub-Signature-256": _signature(draft_body, secret), + }, + ) + + assert ready.json() == {"status": "ready"} + assert accepted.status_code == 202 + assert accepted.json()["status"] == "queued" + assert duplicate.json()["status"] == "duplicate" + assert conflict.status_code == 409 + assert rejected.status_code == 401 + assert ignored.json()["status"] == "ignored" + assert store.get_github_job("delivery-1")["status"] == "queued" + assert store.get_github_delivery("delivery-2")["status"] == "ignored" + store.close() + + +@pytest.mark.asyncio +async def test_worker_completes_retries_and_permanently_fails_jobs(tmp_path: Path) -> None: + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + base, head = "a" * 40, "b" * 40 + + class RecordingService: + + def __init__(self): + self.events = [] + self.failures = [] + + async def process(self, event): + self.events.append(event.delivery_id) + if self.failures: + raise self.failures.pop(0) + + service = RecordingService() + + def enqueue(delivery_id: str, max_attempts: int = 3) -> None: + event = GitHubPullRequestEvent.from_webhook( + delivery_id=delivery_id, + event_name="pull_request", + payload=_payload(base, head), + ) + assert store.enqueue_github_delivery( + delivery_id=delivery_id, + event_name="pull_request", + action=event.action, + payload_sha256="f" * 64, + event_payload=event.model_dump(mode="json"), + repository_full_name=event.repository_full_name, + pull_number=event.pull_number, + head_sha=event.head_sha, + installation_id=event.installation_id, + max_attempts=max_attempts, + ) + + enqueue("worker-success") + worker = GitHubReviewWorker( + store=store, + service=service, + worker_id="test-worker", + lease_seconds=30, + base_retry_seconds=0, + max_retry_seconds=0, + jitter=lambda: 0, + ) + assert await worker.run_once() is True + assert store.get_github_job("worker-success")["status"] == "succeeded" + + enqueue("worker-retry", max_attempts=2) + service.failures = [httpx.ConnectError("offline"), httpx.ConnectError("offline")] + assert await worker.run_once() is True + assert store.get_github_job("worker-retry")["status"] == "queued" + assert store.get_github_delivery("worker-retry")["status"] == "retrying" + assert await worker.run_once() is True + assert store.get_github_job("worker-retry")["status"] == "dead" + assert store.get_github_delivery("worker-retry")["status"] == "failed" + + enqueue("worker-permanent") + service.failures = [ValueError("invalid configuration")] + assert await worker.run_once() is True + assert store.get_github_job("worker-permanent")["status"] == "dead" + assert is_retryable_error(ValueError("bad")) is False + assert is_retryable_error( + GitHubApiError("rate limited", status_code=429) + ) is True + store.close() + + +@pytest.mark.asyncio +async def test_github_client_pins_api_version_and_handles_errors() -> None: + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + if request.url.path.endswith("/comments"): + return httpx.Response(422, text="invalid line", headers={"x-github-request-id": "req-1"}) + return httpx.Response(201, json={"id": 99}) + + http_client = httpx.AsyncClient( + base_url="https://api.github.test", + transport=httpx.MockTransport(handler), + ) + client = GitHubClient( + token_provider=StaticTokenProvider("installation-token"), + installation_id=123, + api_url="https://api.github.test", + http_client=http_client, + ) + + result = await client.create_check_run("octo", "demo", {"head_sha": "a" * 40}) + with pytest.raises(GitHubApiError, match="req-1"): + await client.create_review_comment("octo", "demo", 17, {"line": 3}) + + assert result["id"] == 99 + assert requests[0].headers["x-github-api-version"] == GITHUB_API_VERSION + assert requests[0].headers["authorization"] == "Bearer installation-token" + await http_client.aclose() + + +@pytest.mark.asyncio +async def test_app_token_provider_requests_least_privilege_and_caches(monkeypatch) -> None: + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response( + 201, + json={ + "token": "ghs_new_variable_length_token", + "expires_at": "2099-01-01T00:00:00Z", + }, + ) + + monkeypatch.setitem(sys.modules, "jwt", SimpleNamespace(encode=lambda *_args, **_kwargs: "app-jwt")) + http_client = httpx.AsyncClient( + base_url="https://api.github.test", + transport=httpx.MockTransport(handler), + ) + provider = GitHubAppTokenProvider( + app_id="123", + private_key="test-key", + api_url="https://api.github.test", + permissions={ + "contents": "read", + "checks": "write", + "pull_requests": "read", + }, + http_client=http_client, + ) + + first = await provider.get_token(42) + second = await provider.get_token(42) + request_payload = json.loads(requests[0].content) + + assert first == second == "ghs_new_variable_length_token" + assert len(requests) == 1 + assert request_payload["permissions"]["pull_requests"] == "read" + assert requests[0].headers["authorization"] == "Bearer app-jwt" + await http_client.aclose() + + +def test_checkout_keeps_token_out_of_arguments_and_rejects_hosts(tmp_path: Path) -> None: + + class RecordingWorkspace(GitHubWorkspaceManager): + + def __init__(self): + super().__init__(tmp_path) + self.calls = [] + + def _git(self, workspace, environment, *arguments): + self.calls.append((arguments, environment)) + return "b" * 40 if arguments[:2] == ("rev-parse", "HEAD") else "" + + manager = RecordingWorkspace() + event = GitHubPullRequestEvent.from_webhook( + delivery_id="delivery-1", + event_name="pull_request", + payload=_payload("a" * 40, "b" * 40), + ) + workspace = manager.checkout(event, "secret-token") + + assert all("secret-token" not in argument for call, _environment in manager.calls for argument in call) + assert all(environment["GIT_CONFIG_VALUE_0"] == "Authorization: Bearer secret-token" + for _call, environment in manager.calls) + manager.cleanup(workspace) + + unsafe = event.model_copy(update={"head_clone_url": "https://evil.example/demo.git"}) + with pytest.raises(CheckoutError, match="not allowed"): + manager.checkout(unsafe, "secret-token") + + +@pytest.mark.asyncio +async def test_service_reviews_persists_and_publishes_exact_added_line( + sample_repository: tuple[Path, str, str], + tmp_path: Path, +) -> None: + repository, base, head = sample_repository + store = ReviewStore(sqlite_database_url(tmp_path / "reviews.db")) + event = GitHubPullRequestEvent.from_webhook( + delivery_id="delivery-service", + event_name="pull_request", + payload=_payload(base, head), + ) + store.claim_github_delivery( + delivery_id=event.delivery_id, + event_name="pull_request", + action=event.action, + payload_sha256="f" * 64, + repository_full_name=event.repository_full_name, + pull_number=event.pull_number, + head_sha=event.head_sha, + installation_id=event.installation_id, + ) + + async def reviewer(_context): + return ReviewOutput( + summary="Found a contract issue.", + findings=[ + Finding( + rule_id="python.correctness.none", + severity=Severity.HIGH, + confidence=0.9, + category="correctness", + file_path="app.py", + start_line=3, + title="Do not return None", + description="This changes the function contract.", + ) + ], + ) + + class FakeWorkspace: + + def __init__(self): + self.cleaned = False + + def checkout(self, _event, _token): + return repository + + def cleanup(self, _workspace): + self.cleaned = True + + class FakeClient: + + def __init__(self): + self.updates = [] + self.comments = [] + + async def create_check_run(self, _owner, _repository, _payload): + return {"id": 77} + + async def update_check_run(self, _owner, _repository, _check_run_id, payload): + self.updates.append(payload) + return {"id": 77} + + async def create_review_comment(self, _owner, _repository, _pull_number, payload): + self.comments.append(payload) + return {"id": 88} + + async def list_check_runs_for_ref( + self, + _owner, + _repository, + _ref, + *, + check_name, + ): + del check_name + return [] + + async def list_review_comments(self, _owner, _repository, _pull_number): + return self.comments + + async def close(self): + return None + + fake_client = FakeClient() + workspace_manager = FakeWorkspace() + service = GitHubReviewService( + store=store, + token_provider=StaticTokenProvider("token"), + workspace_manager=workspace_manager, + reviewer=reviewer, + client_factory=lambda _event: fake_client, + ) + + await service.process(event) + + delivery = store.get_github_delivery(event.delivery_id) + assert delivery["status"] == "completed" + assert delivery["check_run_id"] == 77 + assert store.get_run(delivery["review_run_id"]).repository_path == "github://octo/demo" + assert fake_client.updates[-1]["conclusion"] == "failure" + assert fake_client.comments[0]["line"] == 3 + assert fake_client.comments[0]["side"] == "RIGHT" + publication = store.get_github_publication(event.delivery_id) + assert publication["check_completed"] is True + assert publication["comments_completed"] is True + assert store.list_github_deliveries(status="completed")[0]["delivery_id"] == event.delivery_id + assert workspace_manager.cleaned is True + + repeated_event = event.model_copy(update={"delivery_id": "delivery-service-repeat"}) + store.claim_github_delivery( + delivery_id=repeated_event.delivery_id, + event_name="pull_request", + action=repeated_event.action, + payload_sha256="e" * 64, + repository_full_name=repeated_event.repository_full_name, + pull_number=repeated_event.pull_number, + head_sha=repeated_event.head_sha, + installation_id=repeated_event.installation_id, + ) + await service.process(repeated_event) + assert len(fake_client.comments) == 1 + assert store.count_runs() == 1 + store.close() diff --git a/tests/code_review/test_pipeline.py b/tests/code_review/test_pipeline.py new file mode 100644 index 000000000..790cafbf8 --- /dev/null +++ b/tests/code_review/test_pipeline.py @@ -0,0 +1,113 @@ +"""No-LLM and fake-reviewer integration tests.""" + +from pathlib import Path + +import pytest + +from examples.code_review_agent.code_review.models import ( + AnalyzerExecution, + AnalyzerStatus, + Finding, + ReviewOutput, + ReviewStatus, + Severity, +) +from examples.code_review_agent.code_review.orchestrator import run_review +from examples.code_review_agent.code_review.reporter import render_markdown +from examples.code_review_agent.code_review.static_analysis import StaticAnalysisResult + + +@pytest.mark.asyncio +async def test_no_llm_pipeline_collects_diff(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + + review_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + ) + + assert review_run.status == ReviewStatus.COMPLETED + assert len(review_run.changed_files) == 2 + assert review_run.output.findings == [] + assert "LLM review was disabled" in review_run.output.summary + assert "# Code Review Report" in render_markdown(review_run) + + +@pytest.mark.asyncio +async def test_fake_reviewer_output_is_line_validated(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + + async def reviewer(_context): + assert "L3: return None" in _context.text + return ReviewOutput( + summary="Found one issue.", + findings=[ + Finding( + rule_id="python.correctness.none-sentinel", + severity=Severity.MEDIUM, + confidence=0.88, + category="correctness", + file_path="app.py", + start_line=3, + title="Ambiguous zero-division result", + description="Returning None changes the function contract.", + suggestion="Raise a documented exception or return a typed result.", + ) + ], + ) + + review_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + reviewer=reviewer, + model_name="fake-model", + ) + + assert review_run.status == ReviewStatus.COMPLETED + assert review_run.output.findings[0].publishable is True + + +@pytest.mark.asyncio +async def test_static_and_llm_findings_share_the_same_policy(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + + async def static_analyzer(_repository, _changed_files): + return StaticAnalysisResult( + findings=[ + Finding( + rule_id="ruff.f821", + severity=Severity.MEDIUM, + confidence=0.99, + category="correctness", + file_path="app.py", + start_line=3, + title="Undefined name", + description="The new line references an undefined name.", + source="static:ruff", + ) + ], + executions=[ + AnalyzerExecution( + tool="ruff", + runtime="fake", + command=["ruff"], + status=AnalyzerStatus.FINDINGS, + exit_code=1, + findings_count=1, + ) + ], + ) + + review_run = await run_review( + repository=repository, + base_revision=base, + head_revision=head, + static_analyzer=static_analyzer, + ) + + assert review_run.status == ReviewStatus.COMPLETED + assert review_run.output.findings[0].source == "static:ruff" + assert review_run.output.findings[0].publishable is True + assert review_run.analyzer_executions[0].tool == "ruff" diff --git a/tests/code_review/test_policy.py b/tests/code_review/test_policy.py new file mode 100644 index 000000000..9ff1571e9 --- /dev/null +++ b/tests/code_review/test_policy.py @@ -0,0 +1,98 @@ +"""Finding normalization and publishing policy tests.""" + +from examples.code_review_agent.code_review.models import ( + ChangedFile, + ChangedLine, + ChangeType, + DiffHunk, + Finding, + LineChangeType, + ReviewOutput, + Severity, +) +from examples.code_review_agent.code_review.policy import apply_finding_policy + + +def _changed_file() -> ChangedFile: + return ChangedFile( + path="src/app.py", + change_type=ChangeType.MODIFIED, + language="python", + patch="patch", + hunks=[ + DiffHunk( + header="@@ -9 +9,2 @@", + old_start=9, + old_count=1, + new_start=9, + new_count=2, + lines=[ + ChangedLine( + change_type=LineChangeType.CONTEXT, + content="before", + old_line=9, + new_line=9, + ), + ChangedLine( + change_type=LineChangeType.ADDED, + content="danger()", + new_line=10, + ), + ], + ) + ], + ) + + +def _finding(**overrides) -> Finding: + values = { + "rule_id": "python.security.command-injection", + "severity": Severity.HIGH, + "confidence": 0.9, + "category": "security", + "file_path": "src/app.py", + "start_line": 10, + "end_line": 10, + "title": "Unsafe command", + "description": "Untrusted input reaches a command.", + "suggestion": "Use an argument array.", + } + values.update(overrides) + return Finding(**values) + + +def test_marks_only_added_lines_as_publishable_and_deduplicates() -> None: + output, diagnostics = apply_finding_policy( + ReviewOutput( + summary=" summary ", + findings=[ + _finding(confidence=0.8), + _finding(confidence=0.95), + _finding(rule_id="python.correctness.other", start_line=9, end_line=9), + ], + ), + [_changed_file()], + ) + + assert output.summary == "summary" + assert len(output.findings) == 2 + command_finding = next(item for item in output.findings if "command-injection" in item.rule_id) + assert command_finding.confidence == 0.95 + assert command_finding.publishable is True + context_finding = next(item for item in output.findings if item.start_line == 9) + assert context_finding.publishable is False + assert diagnostics + + +def test_drops_unchanged_files_and_low_confidence_findings() -> None: + output, diagnostics = apply_finding_policy( + ReviewOutput(findings=[ + _finding(file_path="other.py"), + _finding(rule_id="low", confidence=0.2), + ]), + [_changed_file()], + minimum_confidence=0.5, + ) + + assert output.findings == [] + assert len(diagnostics) == 2 diff --git a/tests/code_review/test_static_analysis.py b/tests/code_review/test_static_analysis.py new file mode 100644 index 000000000..98064dd4f --- /dev/null +++ b/tests/code_review/test_static_analysis.py @@ -0,0 +1,144 @@ +"""Static analyzer parsing, execution, and Docker hardening tests.""" + +from pathlib import Path + +import pytest + +from examples.code_review_agent.code_review.git_diff import GitDiffCollector +from examples.code_review_agent.code_review.models import AnalyzerStatus, Severity +from examples.code_review_agent.code_review.static_analysis import ( + CommandResult, + DockerCommandRunner, + StaticAnalysisConfig, + StaticAnalyzer, + parse_bandit_output, + parse_ruff_output, +) + +RUFF_JSON = """[ + { + "code": "F821", + "message": "Undefined name `missing`", + "filename": "app.py", + "location": {"row": 3, "column": 12}, + "end_location": {"row": 3, "column": 19}, + "fix": null + } +]""" + +BANDIT_JSON = """{ + "results": [ + { + "test_id": "B602", + "issue_text": "subprocess call with shell=True", + "issue_severity": "HIGH", + "issue_confidence": "HIGH", + "filename": "app.py", + "line_number": 3, + "line_range": [3], + "more_info": "https://bandit.readthedocs.io/" + } + ] +}""" + + +def test_parses_ruff_and_bandit_findings(tmp_path: Path) -> None: + ruff = parse_ruff_output(RUFF_JSON, tmp_path) + bandit = parse_bandit_output(BANDIT_JSON, tmp_path) + + assert ruff[0].rule_id == "ruff.f821" + assert ruff[0].severity == Severity.MEDIUM + assert ruff[0].file_path == "app.py" + assert bandit[0].rule_id == "bandit.b602" + assert bandit[0].severity == Severity.HIGH + assert bandit[0].confidence == 0.95 + + +class _FakeRunner: + runtime_name = "fake" + + def run(self, command: list[str], *, repository: Path, timeout: float) -> CommandResult: + del repository, timeout + if command[0] == "ruff": + return CommandResult(command=command, exit_code=1, stdout=RUFF_JSON) + if command[0] == "bandit": + return CommandResult(command=command, exit_code=1, stdout=BANDIT_JSON) + return CommandResult(command=command, exit_code=0, stdout="10 passed") + + +@pytest.mark.asyncio +async def test_runs_enabled_tools_only_on_changed_python_files(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + _, changed_files = GitDiffCollector(repository).collect(base, head) + analyzer = StaticAnalyzer( + StaticAnalysisConfig(run_tests=True), + command_runner=_FakeRunner(), + ) + + result = await analyzer.analyze(repository, changed_files) + + assert [execution.tool for execution in result.executions] == ["ruff", "bandit", "pytest"] + assert [execution.status for execution in result.executions] == [ + AnalyzerStatus.FINDINGS, + AnalyzerStatus.FINDINGS, + AnalyzerStatus.SUCCESS, + ] + assert len(result.findings) == 2 + assert all("README.md" not in execution.command for execution in result.executions[:2]) + bandit_command = result.executions[1].command + assert bandit_command[bandit_command.index("--") + 1:] == ["app.py"] + + +def test_docker_command_is_offline_read_only_and_resource_limited(tmp_path: Path) -> None: + runner = DockerCommandRunner( + image="review:test", + memory="256m", + cpus="0.5", + pids_limit=64, + ) + + command = runner.build_command(["ruff", "check", "app.py"], tmp_path.resolve()) + joined = " ".join(command) + + assert "--network none" in joined + assert "--read-only" in command + assert "--cap-drop ALL" in joined + assert "--security-opt no-new-privileges" in joined + assert "--memory 256m" in joined + assert "--cpus 0.5" in joined + assert "readonly" in joined + assert command[-3:] == ["ruff", "check", "app.py"] + + +@pytest.mark.parametrize( + ("field", "value"), + [ + ("docker_image", "--privileged"), + ("docker_image", "review image"), + ("docker_memory", "-1g"), + ("docker_cpus", "0"), + ("docker_pids_limit", 0), + ], +) +def test_rejects_unsafe_docker_configuration(field: str, value: str | int) -> None: + with pytest.raises(ValueError): + StaticAnalysisConfig(**{field: value}) + + +def test_missing_local_tool_is_non_fatal_by_default(sample_repository: tuple[Path, str, str], ) -> None: + repository, base, head = sample_repository + _, changed_files = GitDiffCollector(repository).collect(base, head) + + class MissingRunner: + runtime_name = "local" + + def run(self, command: list[str], *, repository: Path, timeout: float) -> CommandResult: + del repository, timeout + return CommandResult(command=command, exit_code=None, stderr="not found", unavailable=True) + + analyzer = StaticAnalyzer(StaticAnalysisConfig(), command_runner=MissingRunner()) + result = analyzer.analyze_sync(repository, changed_files) + + assert [execution.status + for execution in result.executions] == [AnalyzerStatus.UNAVAILABLE, AnalyzerStatus.UNAVAILABLE] + assert len(result.diagnostics) == 2