Compare commits

..

1 Commits

Author SHA1 Message Date
Rodin bcc88b1056 docs: add comprehensive code review report (vs go-patterns)
CI / test (pull_request) Successful in 14s
CI / review (pull_request) Failing after 11s
2026-05-01 10:33:58 -07:00
25 changed files with 215 additions and 4618 deletions
-163
View File
@@ -1,163 +0,0 @@
# This composite action is designed for Gitea Actions runners.
# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
# actions/cache, and actions/checkout.
# Requirements: python3, sha256sum, curl (all present on ubuntu-* runners).
name: 'AI Code Review'
description: 'Run AI-powered code review on a pull request using review-bot'
inputs:
gitea-url:
description: 'Gitea instance URL (defaults to server_url)'
required: false
default: ''
repo:
description: 'Repository (owner/name, defaults to current)'
required: false
default: ''
pr-number:
description: 'Pull request number (defaults to current PR)'
required: false
default: ''
reviewer-token:
description: 'Gitea token for posting the review'
required: true
reviewer-name:
description: 'Display name for the reviewer'
required: false
default: ''
llm-base-url:
description: 'OpenAI-compatible LLM API base URL'
required: true
llm-api-key:
description: 'LLM API key'
required: true
llm-model:
description: 'LLM model name'
required: true
llm-provider:
description: 'LLM API provider: openai or anthropic (default openai)'
required: false
default: 'openai'
conventions-file:
description: 'Path to conventions file in the repo (e.g. CLAUDE.md)'
required: false
default: ''
patterns-repo:
description: 'Comma-separated repos with language patterns (e.g. rodin/elixir-patterns,rodin/phoenix-conventions)'
required: false
default: ''
patterns-files:
description: 'Comma-separated file paths or directories to fetch from patterns repos'
required: false
default: 'README.md'
temperature:
description: 'LLM temperature (0 = server default)'
required: false
default: '0'
timeout:
description: 'LLM request timeout in seconds (default 300)'
required: false
default: '300'
version:
description: 'review-bot version to install (e.g. v0.1.0, defaults to latest)'
required: false
default: 'latest'
dry-run:
description: 'Print review to stdout instead of posting'
required: false
default: 'false'
update-existing:
description: 'Delete previous review from same bot after posting new one. Accepts: true/1/yes or false/0/no (default true)'
required: false
default: 'true'
system-prompt-file:
description: 'Local file with additional system prompt instructions (e.g. security review focus)'
required: false
default: ''
runs:
using: 'composite'
steps:
- name: Determine version
id: version
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
if [ "${{ inputs.version }}" = "latest" ]; then
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
if [ -z "$VERSION" ]; then
echo "Failed to determine latest version" >&2
exit 1
fi
else
VERSION="${{ inputs.version }}"
fi
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
- name: Cache review-bot binary
id: cache
uses: actions/cache@v4
with:
path: ${{ runner.temp }}/review-bot
key: review-bot-linux-amd64-${{ steps.version.outputs.version }}
- name: Install review-bot
if: steps.cache.outputs.cache-hit != 'true'
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
VERSION="${{ steps.version.outputs.version }}"
BINARY="review-bot-linux-amd64"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
-o "${{ runner.temp }}/review-bot"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
-o "${{ runner.temp }}/checksums.txt"
# Verify SHA-256 checksum
cd "${{ runner.temp }}"
EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
ACTUAL=$(sha256sum review-bot | awk '{print $1}')
if [ -z "$EXPECTED" ]; then
echo "Error: no checksum found for ${BINARY}" >&2
exit 1
fi
if [ "$EXPECTED" != "$ACTUAL" ]; then
echo "Error: checksum mismatch!" >&2
echo " Expected: $EXPECTED" >&2
echo " Actual: $ACTUAL" >&2
exit 1
fi
chmod +x "${{ runner.temp }}/review-bot"
echo "Installed review-bot ${VERSION} (checksum verified)"
- name: Run review
shell: bash
env:
GITEA_URL: ${{ inputs.gitea-url || github.server_url }}
GITEA_REPO: ${{ inputs.repo || github.repository }}
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
REVIEWER_NAME: ${{ inputs.reviewer-name }}
LLM_BASE_URL: ${{ inputs.llm-base-url }}
LLM_API_KEY: ${{ inputs.llm-api-key }}
LLM_MODEL: ${{ inputs.llm-model }}
CONVENTIONS_FILE: ${{ inputs.conventions-file }}
PATTERNS_REPO: ${{ inputs.patterns-repo }}
PATTERNS_FILES: ${{ inputs.patterns-files }}
LLM_TEMPERATURE: ${{ inputs.temperature }}
LLM_TIMEOUT: ${{ inputs.timeout }}
LLM_PROVIDER: ${{ inputs.llm-provider }}
UPDATE_EXISTING: ${{ inputs.update-existing }}
SYSTEM_PROMPT_FILE: ${{ inputs.system-prompt-file }}
run: |
ARGS=""
if [ "${{ inputs.dry-run }}" = "true" ]; then
ARGS="--dry-run"
fi
${{ runner.temp }}/review-bot $ARGS
+18 -25
View File
@@ -1,5 +1,4 @@
name: CI name: CI
on: on:
push: push:
branches: [main] branches: [main]
@@ -13,48 +12,42 @@ jobs:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
with: with:
go-version: '1.26' go-version: "1.26"
- run: go test ./... - run: go test ./...
- run: go vet ./... - run: go vet ./...
- run: go build -o review-bot ./cmd/review-bot - run: go build -o review-bot ./cmd/review-bot
# Self-review: builds from source since we're pre-release
review: review:
runs-on: ubuntu-24.04 runs-on: ubuntu-24.04
if: github.event_name == 'pull_request' if: github.event_name == 'pull_request'
needs: test needs: test
strategy:
matrix:
include:
- name: sonnet
token_secret: SONNET_REVIEW_TOKEN
model: gpt-5
- name: gpt
token_secret: GPT_REVIEW_TOKEN
model: gpt-4.1
- name: security
token_secret: SECURITY_REVIEW_TOKEN
model: gpt-5
system_prompt_file: SECURITY_REVIEW.md
steps: steps:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
with: with:
go-version: '1.26' go-version: "1.26"
- run: go build -o review-bot ./cmd/review-bot - run: go build -o review-bot ./cmd/review-bot
- name: Run ${{ matrix.name }} review - name: Run Sonnet Review
env: env:
GITEA_URL: ${{ github.server_url }} GITEA_URL: ${{ github.server_url }}
GITEA_REPO: ${{ github.repository }} GITEA_REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }} PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }} REVIEWER_TOKEN: ${{ secrets.SONNET_REVIEW_TOKEN }}
REVIEWER_NAME: ${{ matrix.name }}
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
LLM_API_KEY: ${{ secrets.LLM_API_KEY }} LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
LLM_MODEL: ${{ matrix.model }} LLM_MODEL: "anthropic--claude-4.6-sonnet"
CONVENTIONS_FILE: "CONVENTIONS.md" CONVENTIONS_FILE: "CONVENTIONS.md"
PATTERNS_REPO: "rodin/go-patterns" REVIEWER_NAME: "Sonnet"
PATTERNS_FILES: "README.md,patterns/" run: ./review-bot
LLM_TIMEOUT: "600" - name: Run GPT Review
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }} env:
GITEA_URL: ${{ github.server_url }}
GITEA_REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ secrets.GPT_REVIEW_TOKEN }}
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
LLM_MODEL: "sap-ai-opus-latest-openai/gpt-5"
CONVENTIONS_FILE: "CONVENTIONS.md"
REVIEWER_NAME: "GPT"
run: ./review-bot run: ./review-bot
-97
View File
@@ -1,97 +0,0 @@
name: Release
on:
push:
tags:
- 'v*'
jobs:
release:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: '1.26'
- name: Run tests
run: |
go vet ./...
go test ./...
- name: Build binaries
run: |
VERSION=${GITHUB_REF_NAME}
mkdir -p dist
GOOS=linux GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-amd64 ./cmd/review-bot
GOOS=linux GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-arm64 ./cmd/review-bot
GOOS=darwin GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-amd64 ./cmd/review-bot
GOOS=darwin GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-arm64 ./cmd/review-bot
cd dist && sha256sum * > checksums.txt
- name: Create release and upload assets
env:
GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }}
run: |
VERSION=${GITHUB_REF_NAME}
GITEA_URL="${{ github.server_url }}"
REPO="${{ github.repository }}"
# Create release (or find existing one for this tag)
HTTP_CODE=$(curl -s -o /tmp/release_response.json -w "%{http_code}" -X POST \
-H "Authorization: token ${GITEA_TOKEN}" \
-H "Content-Type: application/json" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases" \
-d "{\"tag_name\": \"${VERSION}\", \"name\": \"${VERSION}\", \"body\": \"Release ${VERSION}\", \"draft\": false, \"prerelease\": false}")
if [ "$HTTP_CODE" = "409" ]; then
echo "Release for ${VERSION} already exists, fetching existing..."
curl -sSf -o /tmp/release_response.json \
-H "Authorization: token ${GITEA_TOKEN}" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/tags/${VERSION}"
elif [ "$HTTP_CODE" != "201" ]; then
echo "Failed to create release (HTTP ${HTTP_CODE})" >&2
cat /tmp/release_response.json >&2
exit 1
fi
# Parse release ID (python3 available on ubuntu-24.04 runners)
RELEASE_ID=$(python3 -c "import json; print(json.load(open('/tmp/release_response.json'))['id'])")
if [ -z "$RELEASE_ID" ]; then
echo "Failed to parse release ID" >&2
cat /tmp/release_response.json >&2
exit 1
fi
echo "Release ID: ${RELEASE_ID}"
# Upload each asset (idempotent: delete existing asset with same name first)
for file in dist/*; do
filename=$(basename "$file")
echo "Uploading ${filename}..."
# Check if asset already exists and delete it
EXISTING_ID=$(export ASSET_NAME="${filename}"; curl -sS \
-H "Authorization: token ${GITEA_TOKEN}" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets" \
| python3 -c "import json,sys,os; name=os.environ['ASSET_NAME']; assets=json.load(sys.stdin); print(next((str(a['id']) for a in assets if a['name']==name),''))" 2>/dev/null)
if [ -n "$EXISTING_ID" ]; then
echo " Asset ${filename} already exists (id=${EXISTING_ID}), deleting..."
curl -sSf -X DELETE \
-H "Authorization: token ${GITEA_TOKEN}" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets/${EXISTING_ID}"
fi
curl -sSf -X POST \
-H "Authorization: token ${GITEA_TOKEN}" \
-H "Content-Type: application/octet-stream" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=$(printf '%s' "${filename}" | jq -sRr @uri)" \
--data-binary "@${file}"
done
echo "Release ${VERSION} created with assets"
-1
View File
@@ -1,2 +1 @@
/review-bot /review-bot
coverage.out
-20
View File
@@ -1,20 +0,0 @@
.PHONY: build test test-integration lint clean coverage
build:
go build -o review-bot ./cmd/review-bot/
test:
go test ./...
test-integration:
go test -tags integration -v ./cmd/review-bot/
lint:
go vet ./...
clean:
rm -f review-bot
coverage:
go test -coverprofile=coverage.out ./...
go tool cover -func=coverage.out
+48 -288
View File
@@ -1,254 +1,17 @@
# review-bot # review-bot
AI-powered code review bot for Gitea pull requests. Fetches diff + context, sends to an LLM, and posts a structured review (APPROVE / REQUEST_CHANGES) back to the PR. Automated code review bot for Gitea. Fetches a pull request diff, sends it to an LLM for analysis, and posts a structured review back to the PR.
## Features ## Features
- **Multi-provider**: OpenAI-compatible and Anthropic Messages API - Fetches PR metadata, diff, and CI status from Gitea API
- **Context-aware**: Fetches full file content, conventions, language patterns, CI status - Sends context-rich prompts to any OpenAI-compatible LLM
- **Smart budget**: Automatically trims context to fit model token limits - Parses structured JSON review responses
- **Idempotent reviews**: Posts new review, then cleans up stale ones (one review per bot) - Posts formatted reviews (APPROVE / REQUEST_CHANGES) back to Gitea
- **Custom prompts**: Load additional instructions from a file (e.g. security-focused review) - Supports custom coding conventions via repo files
- **Zero dependencies**: Go stdlib only - Zero external dependencies Go stdlib only
## Quick Start: Composite Action ## Usage
The easiest way to use review-bot in your Gitea CI:
```yaml
# .gitea/workflows/review.yml
name: Review
on:
pull_request:
types: [opened, synchronize]
jobs:
review:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: code-review
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: gpt-4.1
```
That's it. Every PR gets an automated review.
## Examples
### Single reviewer with conventions
```yaml
jobs:
review:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: reviewer
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: gpt-4.1
conventions-file: CONVENTIONS.md
timeout: '600'
```
### Two reviewers with different models (diversity of opinion)
```yaml
jobs:
review:
runs-on: ubuntu-24.04
strategy:
matrix:
include:
- name: gpt
model: gpt-4.1
token_secret: GPT_REVIEW_TOKEN
- name: claude
model: claude-sonnet-4-20250514
token_secret: CLAUDE_REVIEW_TOKEN
provider: anthropic
steps:
- uses: actions/checkout@v4
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets[matrix.token_secret] }}
reviewer-name: ${{ matrix.name }}
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: ${{ matrix.model }}
llm-provider: ${{ matrix.provider }}
conventions-file: CONVENTIONS.md
```
Each reviewer posts independently and only cleans up its own stale reviews.
### Multiple review types from a single bot account
Use the same Gitea token but different `reviewer-name` values to run specialized reviews without needing multiple bot accounts:
```yaml
jobs:
review:
runs-on: ubuntu-24.04
strategy:
matrix:
include:
- name: code-quality
model: gpt-4.1
- name: security
model: gpt-4.1
system_prompt_file: .review/SECURITY.md
- name: performance
model: gpt-4.1
system_prompt_file: .review/PERFORMANCE.md
steps:
- uses: actions/checkout@v4
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: ${{ matrix.name }}
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: ${{ matrix.model }}
system-prompt-file: ${{ matrix.system_prompt_file }}
```
The sentinel `<!-- review-bot:security -->` ensures the security review only replaces previous security reviews, never the code-quality or performance reviews.
### With language patterns from another repo
```yaml
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: reviewer
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: gpt-4.1
conventions-file: CLAUDE.md
patterns-repo: rodin/go-patterns,rodin/kubernetes-conventions
patterns-files: "README.md,patterns/"
```
Pattern repos are fetched at review time. The reviewer uses them as criteria for idiomatic code.
### Dry run (test without posting)
```yaml
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: test
llm-base-url: ${{ secrets.LLM_BASE_URL }}
llm-api-key: ${{ secrets.LLM_API_KEY }}
llm-model: gpt-4.1
dry-run: 'true'
```
Prints the review to CI logs without posting to the PR. Useful for testing prompt changes.
### Using Anthropic directly
```yaml
- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
with:
reviewer-token: ${{ secrets.REVIEW_TOKEN }}
reviewer-name: claude
llm-base-url: https://api.anthropic.com
llm-api-key: ${{ secrets.ANTHROPIC_API_KEY }}
llm-model: claude-sonnet-4-20250514
llm-provider: anthropic
```
## Action Inputs
| Input | Required | Default | Description |
|-------|----------|---------|-------------|
| `reviewer-token` | Yes | — | Gitea token for posting reviews (needs `write:issue`, `write:repository`) |
| `reviewer-name` | No | `""` | Logical identity for this reviewer. Used as sentinel for idempotent cleanup. Set this when running multiple review bots on the same PR. |
| `llm-base-url` | Yes | — | LLM API base URL |
| `llm-api-key` | Yes | — | LLM API key |
| `llm-model` | Yes | — | Model name |
| `llm-provider` | No | `openai` | API provider: `openai` or `anthropic` |
| `conventions-file` | No | `""` | Path to coding conventions file in the repo |
| `patterns-repo` | No | `""` | Comma-separated repos with language patterns (e.g. `rodin/go-patterns`) |
| `patterns-files` | No | `README.md` | Files/directories to fetch from pattern repos |
| `system-prompt-file` | No | `""` | Local file with additional system prompt instructions |
| `temperature` | No | `0` | LLM temperature (0 = server default) |
| `timeout` | No | `300` | LLM request timeout in seconds |
| `dry-run` | No | `false` | Print review to stdout instead of posting |
| `update-existing` | No | `true` | Delete previous review from same bot before posting. Accepts: true/1/yes or false/0/no |
| `version` | No | `latest` | review-bot version to install |
## Runner Requirements
The composite action requires these tools on the runner:
| Tool | Used For |
|------|----------|
| `python3` | JSON parsing during version detection |
| `sha256sum` | Checksum verification of downloaded binary |
| `curl` | Downloading releases and querying the API |
All three are pre-installed on `ubuntu-*` runners (e.g. `ubuntu-24.04`). If you use a custom runner image, ensure these are available.
## How Review Cleanup Works
When `reviewer-name` is set, the bot embeds a hidden sentinel in each review:
```html
<!-- review-bot:code-review -->
```
On the next run, it finds and deletes any review containing its own sentinel (except the one it just posted). This means:
- **One review per bot per PR** — no clutter from repeated pushes
- **Multiple bots coexist** — each only cleans up its own reviews
- **Same token, different roles** — a single bot account can post "code-review" and "security" reviews without conflict
- **No extra permissions** — identity comes from the sentinel, not the API
If `reviewer-name` is empty, cleanup is skipped (reviews stack like before).
### Shared Token: Worst-Wins Behavior
When multiple review types share the same Gitea bot account (e.g. code-quality and security), Gitea determines the user's approval state from their **most recent review**. This creates a race condition: if security finds issues (REQUEST_CHANGES) but code-quality finishes last (APPROVE), the PR appears approved.
review-bot handles this automatically with **worst-wins reconciliation**: before posting, each job checks whether any sibling review from the same user already has REQUEST_CHANGES. If so and this job would post APPROVE, it posts as REQUEST_CHANGES instead — maintaining the block. This ensures the PR stays blocked until all checks pass, regardless of execution order.
**If you need independent approval/block per review type**, use separate Gitea bot accounts with their own tokens.
## Custom Review Prompts
Use `system-prompt-file` to specialize the review focus. The file contents are appended to the base system prompt as "Additional Review Instructions."
Example `SECURITY_REVIEW.md`:
```markdown
You are performing a security-focused code review.
Focus areas:
- Injection attacks (SQL, command, path traversal, template)
- Authentication/Authorization (missing checks, privilege escalation)
- Secrets exposure (hardcoded credentials, tokens in logs)
- Input validation (unsanitized input, unsafe deserialization)
- Race conditions (TOCTOU, unsynchronized shared state)
Rules:
- Only report findings with security implications
- Ignore style, naming, and general code quality
- MAJOR = exploitable vulnerability, MINOR = hardening opportunity, NIT = theoretical risk
- If no security-relevant changes exist, APPROVE with empty findings
```
## CLI Usage
```bash ```bash
review-bot \ review-bot \
@@ -256,74 +19,71 @@ review-bot \
--repo owner/name \ --repo owner/name \
--pr 42 \ --pr 42 \
--reviewer-token "$GITEA_TOKEN" \ --reviewer-token "$GITEA_TOKEN" \
--reviewer-name "code-review" \
--llm-base-url https://api.openai.com/v1 \ --llm-base-url https://api.openai.com/v1 \
--llm-api-key "$OPENAI_API_KEY" \ --llm-api-key "$OPENAI_API_KEY" \
--llm-model gpt-4.1 \ --llm-model gpt-4 \
--conventions-file CONVENTIONS.md --reviewer-name "Sonnet" \
--conventions-file CONVENTIONS.md \
--dry-run
``` ```
## Environment Variables ## Environment Variables
All flags have environment variable equivalents: All flags can be set via environment variables:
| Flag | Env Var | | Flag | Env Var | Required | Description |
|------|---------| |------|---------|----------|-------------|
| `--gitea-url` | `GITEA_URL` | | `--gitea-url` | `GITEA_URL` | Yes | Gitea instance base URL |
| `--repo` | `GITEA_REPO` | | `--repo` | `GITEA_REPO` | Yes | Repository in `owner/name` format |
| `--pr` | `PR_NUMBER` | | `--pr` | `PR_NUMBER` | Yes | Pull request number |
| `--reviewer-token` | `REVIEWER_TOKEN` | | `--reviewer-token` | `REVIEWER_TOKEN` | Yes | Gitea API token for posting reviews |
| `--reviewer-name` | `REVIEWER_NAME` | | `--llm-base-url` | `LLM_BASE_URL` | Yes | OpenAI-compatible API base URL |
| `--llm-base-url` | `LLM_BASE_URL` | | `--llm-api-key` | `LLM_API_KEY` | Yes | LLM API key |
| `--llm-api-key` | `LLM_API_KEY` | | `--llm-model` | `LLM_MODEL` | Yes | Model identifier |
| `--llm-model` | `LLM_MODEL` | | `--reviewer-name` | `REVIEWER_NAME` | No | Display name in review footer |
| `--llm-provider` | `LLM_PROVIDER` | | `--conventions-file` | `CONVENTIONS_FILE` | No | Path to conventions file in repo |
| `--conventions-file` | `CONVENTIONS_FILE` | | `--dry-run` | — | No | Print review to stdout instead of posting |
| `--patterns-repo` | `PATTERNS_REPO` |
| `--patterns-files` | `PATTERNS_FILES` |
| `--system-prompt-file` | `SYSTEM_PROMPT_FILE` |
| `--llm-temperature` | `LLM_TEMPERATURE` |
| `--llm-timeout` | `LLM_TIMEOUT` |
| `--update-existing` | `UPDATE_EXISTING` |
## Setup ## Adding to a Gitea Repository
1. **Create a Gitea bot account** (e.g. `review-bot`) 1. Build the binary or use the CI workflow approach (build in CI).
2. **Generate a token** with scopes: `write:issue`, `write:repository`
3. **Add secrets** to your Gitea repo (Settings → Actions → Secrets):
- `REVIEW_TOKEN` — the bot's Gitea token
- `LLM_BASE_URL` — your LLM endpoint
- `LLM_API_KEY` — your LLM key
4. **Add the workflow** (see Quick Start above)
### Token Scopes Required 2. Add secrets to your Gitea repo (Settings → Actions → Secrets):
- `SONNET_REVIEW_TOKEN` — Gitea token for the Sonnet reviewer account
- `GPT_REVIEW_TOKEN` — Gitea token for the GPT reviewer account
- `LLM_BASE_URL` — Your LLM API endpoint
- `LLM_API_KEY` — Your LLM API key
| Scope | Purpose | 3. Copy `.gitea/workflows/ci.yml` to your repo (or adapt it).
|-------|---------|
| `write:issue` | Post and delete reviews |
| `write:repository` | Read PR diffs, file content, commit statuses |
No `read:user` scope needed — the bot identifies itself from the review response. 4. On every PR, the bot will:
- Run tests and vet
- Build review-bot
- Post reviews from each configured LLM reviewer
## Development ## Development
```bash ```bash
go test ./... # Unit tests # Run tests
go vet ./... # Static analysis go test ./...
# Run vet
go vet ./...
# Build
go build -o review-bot ./cmd/review-bot go build -o review-bot ./cmd/review-bot
# Integration tests (requires env vars set) # Integration tests (requires env vars)
go test -tags=integration ./... go test -tags=integration ./...
``` ```
## Architecture ## Architecture
``` ```
cmd/review-bot/ CLI entrypoint + orchestration cmd/review-bot/ CLI entrypoint
gitea/ Gitea API client (reviews, PRs, files) gitea/ Gitea API client
llm/ Multi-provider LLM client (OpenAI + Anthropic) llm/ OpenAI-compatible LLM client
review/ Prompt building, response parsing, formatting review/ Prompt building, response parsing, formatting
budget/ Token estimation + context trimming
``` ```
## License ## License
-18
View File
@@ -1,18 +0,0 @@
You are performing a security-focused code review. Your primary concern is identifying vulnerabilities, not general code quality.
Focus areas:
- **Injection attacks**: SQL injection, command injection, path traversal, template injection
- **Authentication/Authorization**: Missing auth checks, privilege escalation, IDOR
- **Secrets exposure**: Hardcoded credentials, API keys in code, tokens in logs
- **Input validation**: Untrusted input used without sanitization, unsafe deserialization
- **Cryptography**: Weak algorithms, predictable randomness, improper key management
- **Error handling**: Information leakage in error messages, stack traces exposed
- **Dependencies**: Known vulnerable patterns, unsafe use of external libraries
- **Race conditions**: TOCTOU bugs, unsynchronized shared state
- **Resource exhaustion**: Unbounded allocations, missing timeouts, denial-of-service vectors
Rules for this review:
- Only report findings with actual security implications. Ignore style, naming, and general code quality.
- Severity mapping: MAJOR = exploitable vulnerability or data exposure. MINOR = defense-in-depth improvement or hardening opportunity. NIT = theoretical concern with low practical risk.
- If the code has no security-relevant changes, APPROVE with an empty findings list.
- Do not duplicate findings that a standard code review would catch (logic bugs, missing error checks) unless they have a security dimension.
-226
View File
@@ -1,226 +0,0 @@
// Package budget manages LLM context window budgeting for review-bot.
//
// It estimates token usage and progressively trims context content to fit
// within model-specific limits. The trimming order (least important first):
// patterns → conventions → file context → diff truncation.
package budget
import (
"fmt"
"strings"
"unicode/utf8"
)
// modelLimit pairs a model name prefix with its context window size.
type modelLimit struct {
prefix string
limit int
}
// Known model context limits (in tokens), ordered longest-prefix-first
// for deterministic matching.
var modelLimits = []modelLimit{
{"claude-haiku-3.5-20241022", 200_000},
{"claude-sonnet-4-20250514", 200_000},
{"claude-opus-4-20250514", 200_000},
{"gpt-4.1-mini", 128_000},
{"gpt-5-mini", 200_000},
{"gpt-4.1", 128_000},
{"gpt-5", 200_000},
}
const defaultLimit = 128_000
// reserveTokens is headroom for the response generation.
const reserveTokens = 4_000
const diffTruncMarker = "\n\n... [diff truncated due to context limit] ..."
const diffTooLargeMarker = "... [diff too large for context window — review manually] ..."
const userMetaTruncMarker = "\n... [description truncated] ..."
// EstimateTokens estimates the number of tokens in a string.
// Uses the rough heuristic of ~4 bytes per token, which is
// conservative for English text and code.
func EstimateTokens(s string) int {
return len(s) / 4
}
// LimitForModel returns the context window size for the given model.
// Uses longest-prefix-first matching for deterministic results.
func LimitForModel(model string) int {
for _, ml := range modelLimits {
if model == ml.prefix || strings.HasPrefix(model, ml.prefix) {
return ml.limit
}
}
return defaultLimit
}
// Sections holds the prompt content sections in trim priority order.
// When the total exceeds the budget, sections are trimmed from least
// important (Patterns) to most important (Diff).
type Sections struct {
SystemBase string // Core instructions (never trimmed)
Patterns string // Language patterns (trimmed first)
Conventions string // Repo conventions (trimmed second)
FileContext string // Full file content (trimmed third)
Diff string // The actual diff (trimmed last, only truncated)
UserMeta string // PR title, description, CI status (truncated only if base exceeds budget)
}
// Result holds the trimmed content and metadata about what was dropped.
type Result struct {
SystemPrompt string
UserPrompt string
Trimmed []string // Human-readable descriptions of what was trimmed
EstTokens int // Estimated total tokens after trimming
}
// Fit trims sections to fit within the model's context limit.
// Returns the assembled prompts and a list of what was trimmed.
func Fit(model string, sections Sections) Result {
limit := LimitForModel(model) - reserveTokens
baseTokens := EstimateTokens(sections.SystemBase) + EstimateTokens(sections.UserMeta)
available := limit - baseTokens
if available < 0 {
// Base content alone exceeds budget. Truncate UserMeta (keep first ~1000 tokens).
if len(sections.UserMeta) > 4000 {
sections.UserMeta = truncateUTF8(sections.UserMeta, 4000) + userMetaTruncMarker
baseTokens = EstimateTokens(sections.SystemBase) + EstimateTokens(sections.UserMeta)
available = limit - baseTokens
}
if available < 0 {
available = 0
}
}
// Trimmable sections in priority order (first = dropped first)
type entry struct {
name string
content *string
}
entries := []entry{
{"patterns", &sections.Patterns},
{"conventions", &sections.Conventions},
{"file context", &sections.FileContext},
}
// Check if everything fits
totalTrimmable := EstimateTokens(sections.Diff)
for _, e := range entries {
totalTrimmable += EstimateTokens(*e.content)
}
var trimmed []string
if totalTrimmable > available {
// Trim from least important
for i := range entries {
tokens := EstimateTokens(*entries[i].content)
if tokens == 0 {
continue
}
trimmed = append(trimmed, fmt.Sprintf("%s (~%dK tokens)", entries[i].name, tokens/1000))
*entries[i].content = ""
// Recalculate
totalTrimmable = EstimateTokens(sections.Diff)
for _, e := range entries {
totalTrimmable += EstimateTokens(*e.content)
}
if totalTrimmable <= available {
break
}
}
}
// If still too large, truncate the diff
if totalTrimmable > available {
diffBudget := available
for _, e := range entries {
diffBudget -= EstimateTokens(*e.content)
}
if diffBudget < 0 {
diffBudget = 0
}
// Reserve space for truncation marker
markerBudget := EstimateTokens(diffTruncMarker)
effectiveBudget := diffBudget - markerBudget
if effectiveBudget < 0 {
effectiveBudget = 0
}
maxChars := effectiveBudget * 4
if maxChars < len(sections.Diff) {
removed := EstimateTokens(sections.Diff) - diffBudget
trimmed = append(trimmed, fmt.Sprintf("diff truncated (~%dK tokens removed)", removed/1000))
if maxChars > 0 {
if diffBudget >= markerBudget {
sections.Diff = truncateUTF8(sections.Diff, maxChars) + diffTruncMarker
} else {
sections.Diff = truncateUTF8(sections.Diff, maxChars)
}
} else {
sections.Diff = diffTooLargeMarker
}
}
}
finalTokens := baseTokens
for _, e := range entries {
finalTokens += EstimateTokens(*e.content)
}
finalTokens += EstimateTokens(sections.Diff)
return buildResult(sections, trimmed, finalTokens)
}
func buildResult(s Sections, trimmed []string, estTokens int) Result {
var sys strings.Builder
sys.WriteString(s.SystemBase)
if s.Patterns != "" {
sys.WriteString("\n\n## Language Patterns & Idioms\n\nUse the following patterns as review criteria. Code that violates these established patterns is a finding:\n\n")
sys.WriteString(s.Patterns)
}
if s.Conventions != "" {
sys.WriteString("\n\n## Repository Conventions\n\nThe repository has the following coding conventions that must be respected:\n\n")
sys.WriteString(s.Conventions)
}
var usr strings.Builder
usr.WriteString(s.UserMeta)
if s.FileContext != "" {
usr.WriteString("\n### Full File Context (modified files)\n\n")
usr.WriteString(s.FileContext)
usr.WriteString("\n")
}
if s.Diff != "" {
usr.WriteString("\n### Diff (changes to review)\n\n```diff\n")
usr.WriteString(s.Diff)
usr.WriteString("\n```\n")
}
if len(trimmed) > 0 {
usr.WriteString("\n⚠️ Note: Context was trimmed to fit model limits. Dropped: ")
usr.WriteString(strings.Join(trimmed, ", "))
usr.WriteString("\n")
}
return Result{
SystemPrompt: sys.String(),
UserPrompt: usr.String(),
Trimmed: trimmed,
EstTokens: estTokens,
}
}
// truncateUTF8 truncates s to at most maxBytes without splitting multi-byte
// UTF-8 characters. Returns a valid UTF-8 string of at most maxBytes bytes.
func truncateUTF8(s string, maxBytes int) string {
if len(s) <= maxBytes {
return s
}
for maxBytes > 0 && !utf8.RuneStart(s[maxBytes]) {
maxBytes--
}
return s[:maxBytes]
}
-203
View File
@@ -1,203 +0,0 @@
package budget
import (
"strings"
"testing"
)
func TestEstimateTokens(t *testing.T) {
tests := []struct {
input string
want int
}{
{"", 0},
{"abcd", 1},
{"12345678", 2},
{strings.Repeat("x", 400), 100},
}
for _, tt := range tests {
got := EstimateTokens(tt.input)
if got != tt.want {
t.Errorf("EstimateTokens(%d chars) = %d, want %d", len(tt.input), got, tt.want)
}
}
}
func TestLimitForModel(t *testing.T) {
tests := []struct {
model string
want int
}{
{"gpt-4.1", 128_000},
{"gpt-5", 200_000},
{"gpt-5-mini", 200_000},
{"unknown-model", defaultLimit},
{"gpt-4.1-2026-01-01", 128_000}, // prefix match
}
for _, tt := range tests {
got := LimitForModel(tt.model)
if got != tt.want {
t.Errorf("LimitForModel(%q) = %d, want %d", tt.model, got, tt.want)
}
}
}
func TestFit_AllFits(t *testing.T) {
s := Sections{
SystemBase: "system instructions",
Patterns: "some patterns",
Conventions: "some conventions",
FileContext: "file content",
Diff: "diff content",
UserMeta: "PR: title\n",
}
result := Fit("gpt-5", s)
if len(result.Trimmed) != 0 {
t.Errorf("expected no trimming, got %v", result.Trimmed)
}
if !strings.Contains(result.SystemPrompt, "some patterns") {
t.Error("expected patterns in system prompt")
}
if !strings.Contains(result.SystemPrompt, "some conventions") {
t.Error("expected conventions in system prompt")
}
if !strings.Contains(result.UserPrompt, "file content") {
t.Error("expected file context in user prompt")
}
}
func TestFit_TrimsPatterns(t *testing.T) {
// Create content that exceeds 128K token budget for gpt-4.1
// Budget ≈ 128K - 4K reserve = 124K tokens = ~496K chars
// Fill patterns with enough to push over
bigPatterns := strings.Repeat("x", 500_000) // ~125K tokens
s := Sections{
SystemBase: "base",
Patterns: bigPatterns,
Conventions: "conventions",
FileContext: "files",
Diff: "diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) == 0 {
t.Fatal("expected trimming")
}
if !strings.Contains(result.Trimmed[0], "patterns") {
t.Errorf("expected patterns to be trimmed first, got %v", result.Trimmed)
}
if strings.Contains(result.SystemPrompt, bigPatterns[:100]) {
t.Error("expected patterns to be removed from output")
}
// Conventions should survive
if !strings.Contains(result.SystemPrompt, "conventions") {
t.Error("expected conventions to survive after patterns trimmed")
}
}
func TestFit_TrimsConventions(t *testing.T) {
// Patterns + conventions + diff all exceed budget even after patterns removed
big := strings.Repeat("y", 520_000) // ~130K tokens each (exceeds 124K budget even alone)
s := Sections{
SystemBase: "base",
Patterns: big,
Conventions: big,
FileContext: "files",
Diff: "diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) < 2 {
t.Fatalf("expected at least 2 trimmed, got %v", result.Trimmed)
}
if !strings.Contains(result.Trimmed[0], "patterns") {
t.Errorf("expected patterns trimmed first, got %s", result.Trimmed[0])
}
if !strings.Contains(result.Trimmed[1], "conventions") {
t.Errorf("expected conventions trimmed second, got %s", result.Trimmed[1])
}
}
func TestFit_TruncatesDiff(t *testing.T) {
// Only diff is huge, no patterns/conventions
hugeDiff := strings.Repeat("z", 600_000) // ~150K tokens > 128K limit
s := Sections{
SystemBase: "base",
Diff: hugeDiff,
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if len(result.Trimmed) == 0 {
t.Fatal("expected diff truncation")
}
if !strings.Contains(result.Trimmed[len(result.Trimmed)-1], "diff truncated") {
t.Errorf("expected diff truncation note, got %v", result.Trimmed)
}
if !strings.Contains(result.UserPrompt, "[diff truncated due to context limit]") {
t.Error("expected truncation marker in user prompt")
}
}
func TestFit_PreservesNoteInOutput(t *testing.T) {
big := strings.Repeat("w", 500_000)
s := Sections{
SystemBase: "base",
Patterns: big,
Diff: "small diff",
UserMeta: "meta",
}
result := Fit("gpt-4.1", s)
if !strings.Contains(result.UserPrompt, "⚠️ Note: Context was trimmed") {
t.Error("expected trimming note in user prompt")
}
}
func TestFit_HugeUserMeta(t *testing.T) {
// UserMeta so large that base alone exceeds limit
// Use a unique marker past the truncation point
hugeDesc := strings.Repeat("d", 5000) + "UNIQUE_MARKER_PAST_TRUNCATION" + strings.Repeat("d", 595_000)
s := Sections{
SystemBase: "base",
Diff: "small diff",
UserMeta: hugeDesc,
}
result := Fit("gpt-4.1", s)
limit := LimitForModel("gpt-4.1") - reserveTokens
if result.EstTokens > limit {
t.Errorf("EstTokens %d exceeds limit %d", result.EstTokens, limit)
}
// Content past truncation point should not be present
if strings.Contains(result.UserPrompt, "UNIQUE_MARKER_PAST_TRUNCATION") {
t.Error("expected UserMeta to be truncated but found content past truncation point")
}
// Truncation marker should be present
if !strings.Contains(result.UserPrompt, "[description truncated]") {
t.Error("expected truncation marker in output")
}
}
func TestFit_NeverExceedsLimit(t *testing.T) {
// All sections huge — verify final tokens never exceed limit
big := strings.Repeat("a", 200_000)
s := Sections{
SystemBase: strings.Repeat("s", 8000),
Patterns: big,
Conventions: big,
FileContext: big,
Diff: big,
UserMeta: strings.Repeat("m", 8000),
}
result := Fit("gpt-4.1", s)
limit := LimitForModel("gpt-4.1") - reserveTokens
if result.EstTokens > limit {
t.Errorf("EstTokens %d exceeds limit %d (trimmed: %v)", result.EstTokens, limit, result.Trimmed)
}
}
-161
View File
@@ -1,161 +0,0 @@
//go:build integration
package main
import (
"context"
"os"
"strconv"
"strings"
"testing"
"gitea.weiker.me/rodin/review-bot/gitea"
"gitea.weiker.me/rodin/review-bot/llm"
"gitea.weiker.me/rodin/review-bot/review"
)
// Integration test requires a running Gitea instance and LLM endpoint.
// Set environment variables:
//
// INTEGRATION_GITEA_URL - Gitea base URL
// INTEGRATION_GITEA_TOKEN - Gitea API token with repo access
// INTEGRATION_GITEA_REPO - owner/repo with an open PR
// INTEGRATION_PR_NUMBER - PR number to test against
// INTEGRATION_LLM_BASE_URL - LLM API base URL
// INTEGRATION_LLM_API_KEY - LLM API key
// INTEGRATION_LLM_MODEL - Model name
func TestIntegration_FullReviewFlow(t *testing.T) {
giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
llmBaseURL := os.Getenv("INTEGRATION_LLM_BASE_URL")
llmAPIKey := os.Getenv("INTEGRATION_LLM_API_KEY")
llmModel := os.Getenv("INTEGRATION_LLM_MODEL")
if giteaURL == "" || giteaToken == "" || giteaRepo == "" || prNumStr == "" ||
llmBaseURL == "" || llmAPIKey == "" || llmModel == "" {
t.Skip("Integration test env vars not set, skipping")
}
prNumber, err := strconv.Atoi(prNumStr)
if err != nil {
t.Fatalf("Invalid PR number %q: %v", prNumStr, err)
}
// Parse owner/repo
parts := strings.SplitN(giteaRepo, "/", 2)
if len(parts) != 2 {
t.Fatalf("Invalid repo format %q", giteaRepo)
}
owner, repoName := parts[0], parts[1]
if owner == "" || repoName == "" {
t.Fatalf("Invalid repo format %q", giteaRepo)
}
ctx := context.Background()
// Step 1: Fetch PR
giteaClient := gitea.NewClient(giteaURL, giteaToken)
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
if err != nil {
t.Fatalf("GetPullRequest: %v", err)
}
t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha)
// Step 2: Fetch diff
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
if err != nil {
t.Fatalf("GetPullRequestDiff: %v", err)
}
if diff == "" {
t.Fatal("diff is empty")
}
t.Logf("Diff size: %d bytes", len(diff))
// Step 3: Build prompts
systemPrompt := review.BuildSystemPrompt("", "")
userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, "", true, "")
// Step 4: Call LLM
llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel)
response, err := llmClient.Complete(ctx, []llm.Message{
{Role: "system", Content: systemPrompt},
{Role: "user", Content: userPrompt},
})
if err != nil {
t.Fatalf("LLM Complete: %v", err)
}
t.Logf("LLM response: %d bytes", len(response))
// Step 5: Parse response
result, err := review.ParseResponse(response)
if err != nil {
t.Fatalf("ParseResponse: %v", err)
}
t.Logf("Verdict: %s, Findings: %d", result.Verdict, len(result.Findings))
// Step 6: Format (dry-run validation)
body := review.FormatMarkdown(result, "integration-test")
if body == "" {
t.Fatal("formatted review body is empty")
}
t.Logf("Review body:\n%s", body)
}
func TestIntegration_PostAndCleanup(t *testing.T) {
giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
if giteaURL == "" || giteaToken == "" || giteaRepo == "" || prNumStr == "" {
t.Skip("Integration test env vars not set, skipping")
}
prNumber, err := strconv.Atoi(prNumStr)
if err != nil {
t.Fatalf("Invalid PR number %q: %v", prNumStr, err)
}
parts := strings.SplitN(giteaRepo, "/", 2)
if len(parts) != 2 {
t.Fatalf("Invalid repo format %q", giteaRepo)
}
owner, repoName := parts[0], parts[1]
ctx := context.Background()
giteaClient := gitea.NewClient(giteaURL, giteaToken)
// Post a test review
sentinel := "<!-- review-bot:integration-test -->"
testBody := "# Integration Test Review\n\nThis is a test review.\n\n" + sentinel
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "COMMENT", testBody, nil)
if err != nil {
t.Fatalf("PostReview: %v", err)
}
t.Logf("Posted review ID: %d", posted.ID)
// Verify it appears in listing
reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
if err != nil {
t.Fatalf("ListReviews: %v", err)
}
found := false
for _, r := range reviews {
if r.ID == posted.ID && strings.Contains(r.Body, sentinel) {
found = true
break
}
}
if !found {
t.Error("posted review not found in listing")
}
// Cleanup: delete the test review
err = giteaClient.DeleteReview(ctx, owner, repoName, prNumber, posted.ID)
if err != nil {
t.Logf("Warning: could not delete test review %d: %v", posted.ID, err)
}
}
+36 -520
View File
@@ -1,58 +1,19 @@
package main package main
import ( import (
"context"
"flag" "flag"
"fmt" "fmt"
"log/slog" "log"
"os" "os"
"path/filepath"
"strconv" "strconv"
"strings" "strings"
"time"
"gitea.weiker.me/rodin/review-bot/budget"
"gitea.weiker.me/rodin/review-bot/gitea" "gitea.weiker.me/rodin/review-bot/gitea"
"gitea.weiker.me/rodin/review-bot/llm" "gitea.weiker.me/rodin/review-bot/llm"
"gitea.weiker.me/rodin/review-bot/review" "gitea.weiker.me/rodin/review-bot/review"
) )
var version = "dev"
// setupLogger configures the global slog default logger based on format and verbosity.
func setupLogger(format, verbosity string) {
var level slog.Level
switch strings.ToLower(verbosity) {
case "debug":
level = slog.LevelDebug
case "info":
level = slog.LevelInfo
case "warn":
level = slog.LevelWarn
case "error":
level = slog.LevelError
default:
level = slog.LevelInfo
}
opts := &slog.HandlerOptions{Level: level}
var handler slog.Handler
switch strings.ToLower(format) {
case "json":
handler = slog.NewJSONHandler(os.Stderr, opts)
default:
handler = slog.NewTextHandler(os.Stderr, opts)
}
slog.SetDefault(slog.New(handler))
}
func main() { func main() {
versionFlag := flag.Bool("version", false, "Print version and exit")
// Logging flags
logFormat := flag.String("log-format", envOrDefault("LOG_FORMAT", "text"), "Log output format: text or json")
verbosity := flag.String("verbosity", envOrDefault("LOG_VERBOSITY", "info"), "Log verbosity: debug, info, warn, error")
// CLI flags // CLI flags
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL") giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)") repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
@@ -63,26 +24,10 @@ func main() {
llmAPIKey := flag.String("llm-api-key", envOrDefault("LLM_API_KEY", ""), "LLM API key") llmAPIKey := flag.String("llm-api-key", envOrDefault("LLM_API_KEY", ""), "LLM API key")
llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name") llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name")
conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)") conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)")
systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions")
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
flag.Parse() flag.Parse()
if *versionFlag {
fmt.Printf("review-bot %s\n", version)
os.Exit(0)
}
// Initialize structured logger
setupLogger(*logFormat, *verbosity)
slog.Info("review-bot starting", "version", version)
// Validate required fields // Validate required fields
if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" ||
*llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" { *llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" {
@@ -91,203 +36,90 @@ func main() {
os.Exit(1) os.Exit(1)
} }
// Validate reviewer-name: only safe characters allowed in sentinel
if err := validateReviewerName(*reviewerName); err != nil {
slog.Error("invalid reviewer name", "error", err)
os.Exit(1)
}
// Parse repo owner/name // Parse repo owner/name
parts := strings.SplitN(*repo, "/", 2) parts := strings.SplitN(*repo, "/", 2)
if len(parts) != 2 { if len(parts) != 2 {
slog.Error("invalid repo format", "repo", *repo, "expected", "owner/name") log.Fatalf("Invalid repo format %q, expected owner/name", *repo)
os.Exit(1)
} }
owner, repoName := parts[0], parts[1] owner, repoName := parts[0], parts[1]
// Parse PR number // Parse PR number
prNumber, err := strconv.Atoi(*prNum) prNumber, err := strconv.Atoi(*prNum)
if err != nil { if err != nil {
slog.Error("invalid PR number", "pr", *prNum, "error", err) log.Fatalf("Invalid PR number %q: %v", *prNum, err)
os.Exit(1)
} }
// Initialize clients // Initialize clients
giteaClient := gitea.NewClient(*giteaURL, *reviewerToken) giteaClient := gitea.NewClient(*giteaURL, *reviewerToken)
llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel) llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
if *llmTemp < 0 || *llmTemp > 2 {
slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
os.Exit(1)
}
if *llmTemp > 0 {
llmClient.WithTemperature(*llmTemp)
}
switch llm.Provider(*llmProvider) {
case llm.ProviderOpenAI, llm.ProviderAnthropic:
llmClient.WithProvider(llm.Provider(*llmProvider))
default:
slog.Error("invalid LLM provider", "provider", *llmProvider, "valid", "openai, anthropic")
os.Exit(1)
}
if *llmTimeout > 0 {
llmClient.WithTimeout(time.Duration(*llmTimeout) * time.Second)
}
// Create a top-level context. Timeout derived from LLM timeout + 1 min for other ops. log.Printf("Reviewing PR #%d on %s/%s", prNumber, owner, repoName)
overallTimeout := time.Duration(*llmTimeout)*time.Second + time.Minute
ctx, cancel := context.WithTimeout(context.Background(), overallTimeout)
defer cancel()
slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
// Step 1: Fetch PR metadata // Step 1: Fetch PR metadata
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(owner, repoName, prNumber)
if err != nil { if err != nil {
slog.Error("failed to fetch PR", "pr", prNumber, "error", err) log.Fatalf("Failed to fetch PR: %v", err)
os.Exit(1)
} }
slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title) log.Printf("PR: %s", pr.Title)
// Step 2: Fetch diff // Step 2: Fetch diff
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(owner, repoName, prNumber)
if err != nil { if err != nil {
slog.Error("failed to fetch diff", "pr", prNumber, "error", err) log.Fatalf("Failed to fetch diff: %v", err)
os.Exit(1)
} }
slog.Info("fetched diff", "bytes", len(diff)) log.Printf("Diff size: %d bytes", len(diff))
// Step 3: Fetch full file content for modified files // Step 3: Check CI status
fileContext := ""
files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
if err != nil {
slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
} else {
fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
slog.Debug("fetched file context", "files", len(files))
}
// Step 4: Check CI status
ciPassed := true ciPassed := true
ciDetails := "" ciDetails := ""
if pr.Head.Sha != "" { if pr.Head.Sha != "" {
statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha) statuses, err := giteaClient.GetCommitStatuses(owner, repoName, pr.Head.Sha)
if err != nil { if err != nil {
slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err) log.Printf("Warning: could not fetch CI status: %v", err)
} else { } else {
ciPassed, ciDetails = evaluateCIStatus(statuses) ciPassed, ciDetails = evaluateCIStatus(statuses)
slog.Info("CI status checked", "passed", ciPassed) log.Printf("CI status: passed=%v", ciPassed)
} }
} }
// Step 5: Load conventions file if specified // Step 4: Load conventions file if specified
conventions := "" conventions := ""
if *conventionsFile != "" { if *conventionsFile != "" {
content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile) content, err := giteaClient.GetFileContent(owner, repoName, *conventionsFile)
if err != nil { if err != nil {
slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err) log.Printf("Warning: could not load conventions file %q: %v", *conventionsFile, err)
} else { } else {
conventions = content conventions = content
slog.Debug("loaded conventions file", "file", *conventionsFile, "bytes", len(conventions)) log.Printf("Loaded conventions file: %s (%d bytes)", *conventionsFile, len(conventions))
} }
} }
// Step 6: Load patterns from external repo if specified // Step 5: Build prompts
patterns := "" systemPrompt := review.BuildSystemPrompt(conventions)
if *patternsRepo != "" { userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, ciPassed, ciDetails)
patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
}
// Step 6b: Load additional system prompt if specified // Step 6: Call LLM
additionalPrompt := "" log.Printf("Sending to LLM (%s)...", *llmModel)
if *systemPromptFile != "" {
workspace := os.Getenv("GITHUB_WORKSPACE")
if workspace == "" {
workspace, _ = os.Getwd()
}
absWorkspace, err := filepath.Abs(workspace)
if err != nil {
slog.Error("failed to resolve workspace path", "error", err)
os.Exit(1)
}
promptPath := filepath.Join(absWorkspace, *systemPromptFile)
promptPath = filepath.Clean(promptPath)
if !strings.HasPrefix(promptPath, absWorkspace+string(filepath.Separator)) && promptPath != absWorkspace {
slog.Error("system-prompt-file resolves outside workspace", "path", promptPath, "workspace", absWorkspace)
os.Exit(1)
}
// Resolve symlinks and re-validate to prevent symlink traversal
resolvedPath, err := filepath.EvalSymlinks(promptPath)
if err != nil {
slog.Error("failed to resolve system prompt file", "path", promptPath, "error", err)
os.Exit(1)
}
if !strings.HasPrefix(resolvedPath, absWorkspace+string(filepath.Separator)) && resolvedPath != absWorkspace {
slog.Error("system-prompt-file symlink resolves outside workspace", "resolved", resolvedPath, "workspace", absWorkspace)
os.Exit(1)
}
data, err := os.ReadFile(resolvedPath)
if err != nil {
slog.Error("failed to read system prompt file", "path", promptPath, "error", err)
os.Exit(1)
}
additionalPrompt = string(data)
slog.Debug("loaded system prompt file", "file", *systemPromptFile, "bytes", len(additionalPrompt))
}
// Step 7: Budget-aware prompt assembly
systemBase := review.BuildSystemBase()
if additionalPrompt != "" {
systemBase += "\n\n## Additional Review Instructions\n\n" + additionalPrompt
}
sections := budget.Sections{
SystemBase: systemBase,
Patterns: patterns,
Conventions: conventions,
FileContext: fileContext,
Diff: diff,
UserMeta: review.BuildUserMeta(pr.Title, pr.Body, ciPassed, ciDetails),
}
budgetResult := budget.Fit(*llmModel, sections)
slog.Info("token budget calculated", "tokens", budgetResult.EstTokens, "limit", budget.LimitForModel(*llmModel), "model", *llmModel)
if len(budgetResult.Trimmed) > 0 {
slog.Warn("context trimmed to fit budget", "trimmed", budgetResult.Trimmed)
}
// Step 8: Call LLM
slog.Info("sending request to LLM", "model", *llmModel)
messages := []llm.Message{ messages := []llm.Message{
{Role: "system", Content: budgetResult.SystemPrompt}, {Role: "system", Content: systemPrompt},
{Role: "user", Content: budgetResult.UserPrompt}, {Role: "user", Content: userPrompt},
} }
response, err := llmClient.Complete(ctx, messages) response, err := llmClient.Complete(messages)
if err != nil { if err != nil {
slog.Error("LLM request failed", "model", *llmModel, "error", err) log.Fatalf("LLM request failed: %v", err)
os.Exit(1)
} }
slog.Info("LLM response received", "bytes", len(response)) log.Printf("LLM response received (%d bytes)", len(response))
// Step 9: Parse response // Step 7: Parse response
result, err := review.ParseResponse(response) result, err := review.ParseResponse(response)
if err != nil { if err != nil {
slog.Error("failed to parse LLM response", "error", err) log.Fatalf("Failed to parse LLM response: %v", err)
os.Exit(1)
} }
slog.Info("review parsed", "verdict", result.Verdict, "findings", len(result.Findings)) log.Printf("Verdict: %s (%d findings)", result.Verdict, len(result.Findings))
// Step 10: Format and post review // Step 8: Format and post review
reviewBody := review.FormatMarkdown(result, *reviewerName) reviewBody := review.FormatMarkdown(result, *reviewerName)
// Add commit footer so readers know which commit was evaluated
if pr.Head.Sha != "" {
shortSHA := pr.Head.Sha
if len(shortSHA) > 8 {
shortSHA = shortSHA[:8]
}
reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
}
event := review.GiteaEvent(result.Verdict) event := review.GiteaEvent(result.Verdict)
if *dryRun { if *dryRun {
@@ -297,187 +129,11 @@ func main() {
return return
} }
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName) log.Printf("Posting review (event=%s)...", event)
if err := giteaClient.PostReview(owner, repoName, prNumber, event, reviewBody); err != nil {
// Map findings to inline comments for lines present in the diff log.Fatalf("Failed to post review: %v", err)
diffRanges := gitea.ParseDiffNewLines(diff)
var inlineComments []gitea.ReviewComment
for _, f := range result.Findings {
if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
inlineComments = append(inlineComments, gitea.ReviewComment{
Path: f.File,
NewPosition: int64(f.Line),
Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
})
}
} }
if len(inlineComments) > 0 { log.Printf("Review posted successfully!")
slog.Debug("attaching inline comments", "count", len(inlineComments))
}
// --- Review update strategy ---
// 1. POST new review first (gets non-stale approval badge on HEAD)
// 2. Then supersede old review with link to the new one
// Order matters: post first so we have the new review's URL for the supersede message.
var oldReviews []gitea.Review
if *reviewerName != "" {
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
if err != nil {
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
} else {
if hasSharedToken(existingReviews, sentinel) {
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
} else {
oldReviews = findAllOwnReviews(existingReviews, sentinel)
}
}
}
// Self-request as reviewer (ensures we appear in required-reviewer checks)
authUser, err := giteaClient.GetAuthenticatedUser(ctx)
if err != nil {
slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
} else if authUser != "" {
if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
} else {
slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
}
}
// POST new review
slog.Info("posting review", "event", event, "pr", prNumber)
posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments)
if err != nil {
slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
os.Exit(1)
}
slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
// Supersede all old reviews with link to the new one
if len(oldReviews) > 0 {
newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
for _, oldReview := range oldReviews {
cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
if err != nil {
slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
continue
}
supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
continue
}
slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
// Resolve old review's inline comments
oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
if err != nil {
slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
continue
}
resolved, failed := 0, 0
for _, c := range oldComments {
if c.ID == 0 {
continue
}
if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
failed++
} else {
resolved++
}
}
if resolved > 0 {
slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber)
}
if failed > 0 {
slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber)
}
}
}
}
// fetchFileContext fetches the full content of modified files from the PR branch.
func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string {
var sb strings.Builder
for _, f := range files {
if ctx.Err() != nil {
break
}
if f.Status == "removed" {
continue // Skip deleted files
}
content, err := client.GetFileContentRef(ctx, owner, repo, f.Filename, ref)
if err != nil {
slog.Warn("could not fetch file content", "file", f.Filename, "error", err)
continue
}
sb.WriteString(fmt.Sprintf("--- %s ---\n", f.Filename))
sb.WriteString("```\n")
sb.WriteString(content)
sb.WriteString("\n```\n\n")
}
return sb.String()
}
// fetchPatterns fetches pattern files from one or more external repos.
// patternsRepo is comma-separated list of owner/name repos.
// patternsFiles is comma-separated list of file paths or directories.
// If a path ends with / or is a directory, all files within it are fetched recursively.
func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
var sb strings.Builder
repos := strings.Split(patternsRepo, ",")
paths := strings.Split(patternsFiles, ",")
for _, repoRef := range repos {
if ctx.Err() != nil {
break
}
repoRef = strings.TrimSpace(repoRef)
if repoRef == "" {
continue
}
parts := strings.SplitN(repoRef, "/", 2)
if len(parts) != 2 {
slog.Warn("invalid patterns-repo format", "repo", repoRef, "expected", "owner/name")
continue
}
owner, repo := parts[0], parts[1]
for _, path := range paths {
path = strings.TrimSpace(path)
if path == "" {
continue
}
files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
if err != nil {
slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
continue
}
for filePath, content := range files {
// Only include markdown and text files as patterns
if !isPatternFile(filePath) {
continue
}
sb.WriteString(fmt.Sprintf("### %s/%s\n\n%s\n\n", repoRef, filePath, content))
}
}
}
return sb.String()
}
// isPatternFile returns true if the file should be included as pattern content.
func isPatternFile(path string) bool {
lower := strings.ToLower(path)
return strings.HasSuffix(lower, ".md") ||
strings.HasSuffix(lower, ".txt") ||
strings.HasSuffix(lower, ".yml") ||
strings.HasSuffix(lower, ".yaml")
} }
// evaluateCIStatus checks if all CI statuses indicate success. // evaluateCIStatus checks if all CI statuses indicate success.
@@ -510,143 +166,3 @@ func envOrDefault(key, defaultVal string) string {
} }
return defaultVal return defaultVal
} }
func envOrDefaultFloat(key string, defaultVal float64) float64 {
if v := os.Getenv(key); v != "" {
f, err := strconv.ParseFloat(v, 64)
if err == nil {
return f
}
}
return defaultVal
}
func envOrDefaultInt(key string, defaultVal int) int {
if v := os.Getenv(key); v != "" {
i, err := strconv.Atoi(v)
if err == nil {
return i
}
}
return defaultVal
}
func envOrDefaultBool(key string, defaultVal bool) bool {
v := strings.TrimSpace(strings.ToLower(os.Getenv(key)))
if v == "" {
return defaultVal
}
return v == "true" || v == "1" || v == "yes"
}
// validateReviewerName checks that the name contains only safe characters
// for embedding in an HTML comment sentinel ([a-zA-Z0-9_-]).
func validateReviewerName(name string) error {
if name == "" {
return nil
}
for _, ch := range name {
if !((ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '-' || ch == '_') {
return fmt.Errorf("reviewer-name must contain only [a-zA-Z0-9_-] (got %q)", name)
}
}
return nil
}
// buildSupersededBody creates the body for a superseded review: struck-through banner
// with collapsed original content and the commit it was evaluated against.
func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
shortSHA := commitSHA
if len(shortSHA) > 8 {
shortSHA = shortSHA[:8]
}
var sb strings.Builder
sb.WriteString("~~Original review~~\n\n")
sb.WriteString("**Superseded** \u2014 [see current review](")
sb.WriteString(newReviewURL)
sb.WriteString(") for up-to-date findings.\n\n")
if shortSHA != "" {
sb.WriteString("<details><summary>Previous findings (commit ")
sb.WriteString(shortSHA)
sb.WriteString(")</summary>\n\n")
} else {
sb.WriteString("<details><summary>Previous findings</summary>\n\n")
}
sb.WriteString(originalBody)
sb.WriteString("\n\n</details>\n\n")
sb.WriteString(sentinel)
return sb.String()
}
// hasSharedToken detects if another review-bot role posted under the same
// Gitea user. This indicates misconfiguration where two roles share a token
// instead of having separate Gitea accounts. Returns true if shared token
// detected (caller should skip update-in-place logic to avoid clobbering).
func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool {
ownLogin := ""
for _, r := range reviews {
if strings.Contains(r.Body, ownSentinel) {
ownLogin = r.User.Login
break
}
}
if ownLogin == "" {
return false
}
for _, r := range reviews {
if r.User.Login == ownLogin && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
slog.Warn("shared token detected — another review-bot role is using the same Gitea user",
"sibling_role", extractSentinelName(r.Body), "user", ownLogin)
return true
}
}
return false
}
// extractSentinelName pulls the reviewer name from a sentinel comment.
func extractSentinelName(body string) string {
const prefix = "<!-- review-bot:"
const suffix = " -->"
idx := strings.Index(body, prefix)
if idx < 0 {
return "unknown"
}
rest := body[idx+len(prefix):]
end := strings.Index(rest, suffix)
if end < 0 {
return "unknown"
}
return rest[:end]
}
// findOwnReview locates the most recent non-superseded review matching the sentinel.
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
var best *gitea.Review
for i := range reviews {
if !strings.Contains(reviews[i].Body, sentinel) {
continue
}
if strings.Contains(reviews[i].Body, "~~Original review~~") {
continue
}
if best == nil || reviews[i].ID > best.ID {
best = &reviews[i]
}
}
return best
}
// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
var result []gitea.Review
for i := range reviews {
if !strings.Contains(reviews[i].Body, sentinel) {
continue
}
if strings.Contains(reviews[i].Body, "~~Original review~~") {
continue
}
result = append(result, reviews[i])
}
return result
}
-864
View File
@@ -1,864 +0,0 @@
package main
import (
"bytes"
"flag"
"log/slog"
"os"
"os/exec"
"strings"
"testing"
"gitea.weiker.me/rodin/review-bot/gitea"
)
func TestValidateReviewerName(t *testing.T) {
tests := []struct {
name string
input string
wantErr bool
}{
{"valid simple", "sonnet", false},
{"valid with dash", "code-review", false},
{"valid with underscore", "my_bot", false},
{"valid alphanumeric", "bot123", false},
{"valid uppercase", "MyBot", false},
{"empty is valid", "", false},
{"invalid html close", "foo-->", true},
{"invalid space", "my bot", true},
{"invalid dot", "my.bot", true},
{"invalid slash", "my/bot", true},
{"invalid angle", "bot<script>", true},
{"invalid colon", "bot:name", true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := validateReviewerName(tc.input)
if tc.wantErr && err == nil {
t.Errorf("expected error for %q, got nil", tc.input)
}
if !tc.wantErr && err != nil {
t.Errorf("expected no error for %q, got %v", tc.input, err)
}
})
}
}
func makeReview(id int64, login, state string, stale bool, body string) gitea.Review {
r := gitea.Review{
ID: id,
Body: body,
State: state,
Stale: stale,
}
r.User.Login = login
return r
}
func TestBuildSupersededBody(t *testing.T) {
original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
sentinel := "<!-- review-bot:sonnet -->"
newURL := "https://gitea.example.com/owner/repo/pulls/1#pullrequestreview-99"
result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
// Should contain the struck-through banner
if !strings.Contains(result, "~~Original review~~") {
t.Error("missing struck-through banner")
}
// Should contain superseded notice with link
if !strings.Contains(result, "**Superseded**") {
t.Error("missing superseded notice")
}
if !strings.Contains(result, "[see current review]("+newURL+")") {
t.Error("missing link to new review")
}
// Should contain collapsed original
if !strings.Contains(result, "<details>") {
t.Error("missing details/collapse")
}
// Should contain short commit SHA
if !strings.Contains(result, "abcdef12") {
t.Error("missing short SHA")
}
// Should NOT contain full SHA
if strings.Contains(result, "abcdef1234567890") {
t.Error("should truncate SHA to 8 chars")
}
// Should contain the original body inside details
if !strings.Contains(result, original) {
t.Error("original body not preserved in collapsed section")
}
// Should end with sentinel
if !strings.Contains(result, sentinel) {
t.Error("missing sentinel")
}
}
func TestBuildSupersededBodyShortSHA(t *testing.T) {
// Short SHA should pass through without panic
result := buildSupersededBody("body", "abc", "https://example.com/review", "<!-- review-bot:x -->")
if !strings.Contains(result, "abc") {
t.Error("short SHA not preserved")
}
}
func TestFindOwnReview(t *testing.T) {
tests := []struct {
name string
reviews []gitea.Review
sentinel string
wantID int64
wantNil bool
}{
{
name: "no reviews",
reviews: nil,
sentinel: "<!-- review-bot:sonnet -->",
wantNil: true,
},
{
name: "found by sentinel",
reviews: []gitea.Review{
makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 42,
},
{
name: "wrong sentinel",
reviews: []gitea.Review{
makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantNil: true,
},
{
name: "multiple reviews, returns first match",
reviews: []gitea.Review{
makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 20,
},
{
name: "skips superseded review",
reviews: []gitea.Review{
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 20,
},
{
name: "only superseded reviews exist",
reviews: []gitea.Review{
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantNil: true,
},
{
name: "picks highest ID among matches",
reviews: []gitea.Review{
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
},
sentinel: "<!-- review-bot:sonnet -->",
wantID: 50,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := findOwnReview(tc.reviews, tc.sentinel)
if tc.wantNil {
if got != nil {
t.Errorf("findOwnReview() = %v, want nil", got)
}
} else {
if got == nil {
t.Fatal("findOwnReview() = nil, want non-nil")
}
if got.ID != tc.wantID {
t.Errorf("findOwnReview().ID = %d, want %d", got.ID, tc.wantID)
}
}
})
}
}
func TestHasSharedToken(t *testing.T) {
tests := []struct {
name string
reviews []gitea.Review
sentinel string
want bool
}{
{
name: "no reviews",
reviews: nil,
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "no own review yet - cannot detect",
reviews: []gitea.Review{
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
},
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "separate users - no shared token",
reviews: []gitea.Review{
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
},
sentinel: "<!-- review-bot:sonnet -->",
want: false,
},
{
name: "shared token detected - same user different sentinels",
reviews: []gitea.Review{
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
},
sentinel: "<!-- review-bot:sonnet -->",
want: true,
},
{
name: "three roles same user",
reviews: []gitea.Review{
{ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
{ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
{ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
},
sentinel: "<!-- review-bot:sonnet -->",
want: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := hasSharedToken(tc.reviews, tc.sentinel)
if got != tc.want {
t.Errorf("hasSharedToken() = %v, want %v", got, tc.want)
}
})
}
}
func TestExtractSentinelName(t *testing.T) {
tests := []struct {
body string
want string
}{
{"<!-- review-bot:sonnet --> rest", "sonnet"},
{"<!-- review-bot:security --> rest", "security"},
{"no sentinel here", "unknown"},
{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
}
for _, tc := range tests {
got := extractSentinelName(tc.body)
if got != tc.want {
t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want)
}
}
}
func TestSetupLogger_JSONFormat(t *testing.T) {
// Capture output by creating a logger manually with the same logic
var buf bytes.Buffer
opts := &slog.HandlerOptions{Level: slog.LevelInfo}
handler := slog.NewJSONHandler(&buf, opts)
logger := slog.New(handler)
logger.Info("test message", "key", "value")
output := buf.String()
if !strings.Contains(output, `"msg":"test message"`) {
t.Errorf("expected JSON msg field, got: %s", output)
}
if !strings.Contains(output, `"key":"value"`) {
t.Errorf("expected JSON key field, got: %s", output)
}
if !strings.Contains(output, `"level":"INFO"`) {
t.Errorf("expected JSON level field, got: %s", output)
}
}
func TestSetupLogger_TextFormat(t *testing.T) {
var buf bytes.Buffer
opts := &slog.HandlerOptions{Level: slog.LevelInfo}
handler := slog.NewTextHandler(&buf, opts)
logger := slog.New(handler)
logger.Info("test message", "key", "value")
output := buf.String()
if !strings.Contains(output, "msg=\"test message\"") && !strings.Contains(output, "msg=test") {
t.Errorf("expected text msg field, got: %s", output)
}
if !strings.Contains(output, "key=value") {
t.Errorf("expected text key field, got: %s", output)
}
}
func TestSetupLogger_LevelFiltering(t *testing.T) {
tests := []struct {
name string
verbosity string
logLevel slog.Level
expected bool // should the message appear
}{
{"info logger shows info", "info", slog.LevelInfo, true},
{"info logger hides debug", "info", slog.LevelDebug, false},
{"debug logger shows debug", "debug", slog.LevelDebug, true},
{"warn logger hides info", "warn", slog.LevelInfo, false},
{"warn logger shows warn", "warn", slog.LevelWarn, true},
{"error logger hides warn", "error", slog.LevelWarn, false},
{"error logger shows error", "error", slog.LevelError, true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var level slog.Level
switch tc.verbosity {
case "debug":
level = slog.LevelDebug
case "info":
level = slog.LevelInfo
case "warn":
level = slog.LevelWarn
case "error":
level = slog.LevelError
}
var buf bytes.Buffer
opts := &slog.HandlerOptions{Level: level}
handler := slog.NewTextHandler(&buf, opts)
logger := slog.New(handler)
logger.Log(nil, tc.logLevel, "test")
hasOutput := buf.Len() > 0
if hasOutput != tc.expected {
t.Errorf("verbosity=%s, logLevel=%s: got output=%v, want %v",
tc.verbosity, tc.logLevel, hasOutput, tc.expected)
}
})
}
}
func TestSetupLogger_Integration(t *testing.T) {
// Test that setupLogger doesn't panic for valid inputs
setupLogger("text", "info")
setupLogger("json", "debug")
setupLogger("text", "warn")
setupLogger("json", "error")
setupLogger("text", "unknown") // should default to info
setupLogger("invalid", "info") // should default to text
}
func TestIsPatternFile(t *testing.T) {
tests := []struct {
path string
want bool
}{
{"README.md", true},
{"docs/GUIDE.MD", true},
{"config.yml", true},
{"config.yaml", true},
{"notes.txt", true},
{"NOTES.TXT", true},
{"main.go", false},
{"lib.rs", false},
{"index.js", false},
{"Makefile", false},
{"", false},
{"doc.pdf", false},
{"patterns.Yml", true},
{"deep/path/file.yaml", true},
}
for _, tc := range tests {
t.Run(tc.path, func(t *testing.T) {
got := isPatternFile(tc.path)
if got != tc.want {
t.Errorf("isPatternFile(%q) = %v, want %v", tc.path, got, tc.want)
}
})
}
}
func TestEvaluateCIStatus(t *testing.T) {
tests := []struct {
name string
statuses []gitea.CommitStatus
wantPassed bool
wantSubstr string
}{
{
name: "empty statuses",
statuses: nil,
wantPassed: true,
wantSubstr: "no CI statuses",
},
{
name: "all success",
statuses: []gitea.CommitStatus{
{Status: "success", Context: "ci/build", Description: "Build passed"},
{Status: "success", Context: "ci/test", Description: "Tests passed"},
},
wantPassed: true,
wantSubstr: "all checks passed",
},
{
name: "one failure",
statuses: []gitea.CommitStatus{
{Status: "success", Context: "ci/build", Description: "Build passed"},
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
},
wantPassed: false,
wantSubstr: "ci/test",
},
{
name: "error status",
statuses: []gitea.CommitStatus{
{Status: "error", Context: "ci/lint", Description: "Lint error"},
},
wantPassed: false,
wantSubstr: "ci/lint",
},
{
name: "pending treated as not-failed",
statuses: []gitea.CommitStatus{
{Status: "pending", Context: "ci/build", Description: "In progress"},
{Status: "success", Context: "ci/test", Description: "Tests passed"},
},
wantPassed: true,
wantSubstr: "all checks passed",
},
{
name: "multiple failures",
statuses: []gitea.CommitStatus{
{Status: "failure", Context: "ci/build", Description: "Build failed"},
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
},
wantPassed: false,
wantSubstr: "ci/build",
},
{
name: "mixed with pending and failure",
statuses: []gitea.CommitStatus{
{Status: "success", Context: "ci/build", Description: "Build passed"},
{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
{Status: "failure", Context: "ci/test", Description: "Tests failed"},
},
wantPassed: false,
wantSubstr: "ci/test",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
passed, details := evaluateCIStatus(tc.statuses)
if passed != tc.wantPassed {
t.Errorf("evaluateCIStatus() passed = %v, want %v", passed, tc.wantPassed)
}
if !strings.Contains(details, tc.wantSubstr) {
t.Errorf("evaluateCIStatus() details = %q, want substring %q", details, tc.wantSubstr)
}
})
}
}
func TestEnvOrDefault(t *testing.T) {
// Test with unset env var
os.Unsetenv("TEST_ENV_OR_DEFAULT_UNSET")
got := envOrDefault("TEST_ENV_OR_DEFAULT_UNSET", "fallback")
if got != "fallback" {
t.Errorf("envOrDefault(unset) = %q, want %q", got, "fallback")
}
// Test with set env var
os.Setenv("TEST_ENV_OR_DEFAULT_SET", "custom")
defer os.Unsetenv("TEST_ENV_OR_DEFAULT_SET")
got = envOrDefault("TEST_ENV_OR_DEFAULT_SET", "fallback")
if got != "custom" {
t.Errorf("envOrDefault(set) = %q, want %q", got, "custom")
}
// Test with empty env var (should return default)
os.Setenv("TEST_ENV_OR_DEFAULT_EMPTY", "")
defer os.Unsetenv("TEST_ENV_OR_DEFAULT_EMPTY")
got = envOrDefault("TEST_ENV_OR_DEFAULT_EMPTY", "fallback")
if got != "fallback" {
t.Errorf("envOrDefault(empty) = %q, want %q", got, "fallback")
}
}
func TestEnvOrDefaultFloat(t *testing.T) {
// Test with unset env var
os.Unsetenv("TEST_ENV_FLOAT_UNSET")
got := envOrDefaultFloat("TEST_ENV_FLOAT_UNSET", 1.5)
if got != 1.5 {
t.Errorf("envOrDefaultFloat(unset) = %f, want %f", got, 1.5)
}
// Test with valid float
os.Setenv("TEST_ENV_FLOAT_SET", "2.7")
defer os.Unsetenv("TEST_ENV_FLOAT_SET")
got = envOrDefaultFloat("TEST_ENV_FLOAT_SET", 1.5)
if got != 2.7 {
t.Errorf("envOrDefaultFloat(set) = %f, want %f", got, 2.7)
}
// Test with invalid float (should return default)
os.Setenv("TEST_ENV_FLOAT_INVALID", "not-a-number")
defer os.Unsetenv("TEST_ENV_FLOAT_INVALID")
got = envOrDefaultFloat("TEST_ENV_FLOAT_INVALID", 3.14)
if got != 3.14 {
t.Errorf("envOrDefaultFloat(invalid) = %f, want %f", got, 3.14)
}
// Test with empty string (should return default)
os.Setenv("TEST_ENV_FLOAT_EMPTY", "")
defer os.Unsetenv("TEST_ENV_FLOAT_EMPTY")
got = envOrDefaultFloat("TEST_ENV_FLOAT_EMPTY", 0.5)
if got != 0.5 {
t.Errorf("envOrDefaultFloat(empty) = %f, want %f", got, 0.5)
}
}
func TestEnvOrDefaultInt(t *testing.T) {
// Test with unset env var
os.Unsetenv("TEST_ENV_INT_UNSET")
got := envOrDefaultInt("TEST_ENV_INT_UNSET", 42)
if got != 42 {
t.Errorf("envOrDefaultInt(unset) = %d, want %d", got, 42)
}
// Test with valid int
os.Setenv("TEST_ENV_INT_SET", "100")
defer os.Unsetenv("TEST_ENV_INT_SET")
got = envOrDefaultInt("TEST_ENV_INT_SET", 42)
if got != 100 {
t.Errorf("envOrDefaultInt(set) = %d, want %d", got, 100)
}
// Test with invalid int (should return default)
os.Setenv("TEST_ENV_INT_INVALID", "abc")
defer os.Unsetenv("TEST_ENV_INT_INVALID")
got = envOrDefaultInt("TEST_ENV_INT_INVALID", 42)
if got != 42 {
t.Errorf("envOrDefaultInt(invalid) = %d, want %d", got, 42)
}
// Test with empty string (should return default)
os.Setenv("TEST_ENV_INT_EMPTY", "")
defer os.Unsetenv("TEST_ENV_INT_EMPTY")
got = envOrDefaultInt("TEST_ENV_INT_EMPTY", 99)
if got != 99 {
t.Errorf("envOrDefaultInt(empty) = %d, want %d", got, 99)
}
// Test with negative int
os.Setenv("TEST_ENV_INT_NEG", "-5")
defer os.Unsetenv("TEST_ENV_INT_NEG")
got = envOrDefaultInt("TEST_ENV_INT_NEG", 42)
if got != -5 {
t.Errorf("envOrDefaultInt(negative) = %d, want %d", got, -5)
}
}
func TestEnvOrDefaultBool(t *testing.T) {
tests := []struct {
name string
envVal string
setEnv bool
defaultVal bool
want bool
}{
{"unset returns default true", "", false, true, true},
{"unset returns default false", "", false, false, false},
{"true", "true", true, false, true},
{"TRUE", "TRUE", true, false, true},
{"True", "True", true, false, true},
{"1", "1", true, false, true},
{"yes", "yes", true, false, true},
{"YES", "YES", true, false, true},
{"false", "false", true, true, false},
{"0", "0", true, true, false},
{"no", "no", true, true, false},
{"random string", "random", true, true, false},
{"empty string returns default", "", true, true, true},
{"whitespace true", " true ", true, false, true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
envKey := "TEST_ENV_BOOL_" + strings.ReplaceAll(tc.name, " ", "_")
if tc.setEnv {
os.Setenv(envKey, tc.envVal)
defer os.Unsetenv(envKey)
} else {
os.Unsetenv(envKey)
}
got := envOrDefaultBool(envKey, tc.defaultVal)
if got != tc.want {
t.Errorf("envOrDefaultBool(%q, %v) = %v, want %v", tc.envVal, tc.defaultVal, got, tc.want)
}
})
}
}
func TestExtractSentinelName_EdgeCases(t *testing.T) {
tests := []struct {
body string
want string
}{
{"<!-- review-bot:sonnet --> rest", "sonnet"},
{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
{"no sentinel here", "unknown"},
{"<!-- review-bot:", "unknown"}, // prefix but no suffix
{"prefix <!-- review-bot:abc --> end", "abc"}, // embedded in text
}
for _, tc := range tests {
got := extractSentinelName(tc.body)
if got != tc.want {
t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want)
}
}
}
// TestMainSubprocess runs main() as a subprocess using the test binary itself.
// This allows coverage to be captured for main() code paths.
func TestMainSubprocess_Version(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
// Reset flags for main()
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", "--version"}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_Version")
cmd.Env = append(os.Environ(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("--version subprocess failed: %v\n%s", err, out)
}
if !strings.Contains(string(out), "review-bot") {
t.Errorf("--version output = %q, want to contain 'review-bot'", string(out))
}
}
func TestMainSubprocess_MissingFlags(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
// Reset flags for main()
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot"}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_MissingFlags")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with no flags, got success")
}
if !strings.Contains(string(out), "missing required") {
t.Errorf("expected error about missing flags, got: %s", out)
}
}
func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-name", "invalid name",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidReviewerName")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with invalid reviewer-name, got success")
}
if !strings.Contains(string(out), "invalid reviewer name") {
t.Errorf("expected error about invalid reviewer name, got: %s", out)
}
}
func TestMainSubprocess_InvalidRepo(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "invalidrepo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidRepo")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with invalid repo format")
}
if !strings.Contains(string(out), "invalid repo format") {
t.Errorf("expected error about invalid repo, got: %s", out)
}
}
func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "notanumber",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidPRNumber")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with invalid PR number")
}
if !strings.Contains(string(out), "invalid PR number") {
t.Errorf("expected error about invalid PR number, got: %s", out)
}
}
func TestMainSubprocess_InvalidTemperature(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
"--llm-temperature", "5.0",
}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidTemperature")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with invalid temperature")
}
if !strings.Contains(string(out), "invalid LLM temperature") {
t.Errorf("expected error about invalid temperature, got: %s", out)
}
}
func TestMainSubprocess_InvalidProvider(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
"--llm-provider", "invalid-provider",
}
main()
return
}
cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidProvider")
cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
out, err := cmd.CombinedOutput()
if err == nil {
t.Fatal("expected non-zero exit with invalid provider")
}
if !strings.Contains(string(out), "invalid LLM provider") {
t.Errorf("expected error about invalid provider, got: %s", out)
}
}
// cleanEnv returns environ without any GITEA/LLM/REVIEWER env vars that would
// interfere with testing missing-flag scenarios.
func cleanEnv() []string {
var env []string
for _, e := range os.Environ() {
key := strings.SplitN(e, "=", 2)[0]
switch {
case strings.HasPrefix(key, "GITEA_"),
strings.HasPrefix(key, "LLM_"),
strings.HasPrefix(key, "REVIEWER_"),
strings.HasPrefix(key, "PR_"),
strings.HasPrefix(key, "LOG_"),
strings.HasPrefix(key, "CONVENTIONS_"),
strings.HasPrefix(key, "SYSTEM_PROMPT_"),
strings.HasPrefix(key, "PATTERNS_"),
strings.HasPrefix(key, "UPDATE_"):
continue
default:
env = append(env, e)
}
}
return env
}
func TestFindAllOwnReviews(t *testing.T) {
reviews := []gitea.Review{
{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
{ID: 4, Body: "~~Original review~~\n<!-- review-bot:sonnet -->\nsuperseded"},
{ID: 5, Body: "<!-- review-bot:sonnet -->\nthird review"},
}
got := findAllOwnReviews(reviews, "<!-- review-bot:sonnet -->")
if len(got) != 3 {
t.Fatalf("findAllOwnReviews() returned %d, want 3", len(got))
}
wantIDs := []int64{1, 3, 5}
for i, r := range got {
if r.ID != wantIDs[i] {
t.Errorf("got[%d].ID = %d, want %d", i, r.ID, wantIDs[i])
}
}
}
-97
View File
@@ -1,97 +0,0 @@
# Review Update Strategy
review-bot uses an **edit-in-place** strategy for updating reviews. Reviews are never deleted — this preserves conversation threads on inline comments.
## State Transition Diagram
```mermaid
stateDiagram-v2
[*] --> NoExistingReview: First run
NoExistingReview --> POST_Review: Generate findings + event
POST_Review --> PostEscalationCheck: event == APPROVED?
PostEscalationCheck --> Done: No sibling blocks
PostEscalationCheck --> Supersede_And_Repost: Sibling has REQUEST_CHANGES
Supersede_And_Repost --> Done: Posted as REQUEST_CHANGES
[*] --> ExistingReviewFound: Subsequent run (sentinel match)
ExistingReviewFound --> CheckEscalation: Determine final event
CheckEscalation --> CompareState: Apply worst-wins if needed
CompareState --> SameState: existing.state == new event
CompareState --> StateChange: existing.state != new event
SameState --> Skip: Body unchanged
SameState --> PatchBody: Body changed → PATCH in place
StateChange --> Escalate: APPROVED → REQUEST_CHANGES
StateChange --> Downgrade: REQUEST_CHANGES → APPROVED
Escalate --> Supersede: PATCH old body → "Superseded"
Supersede --> POST_New_RC: POST new REQUEST_CHANGES
Downgrade --> POST_New_Approve: POST new APPROVED (old stays intact)
Skip --> Done
PatchBody --> Done
POST_New_RC --> Done
POST_New_Approve --> Done
```
## Rules
| Scenario | Action | Reason |
|----------|--------|--------|
| No existing review | POST new | First run |
| Same state, same body | Skip | Nothing changed — preserve threads |
| Same state, body changed | PATCH body | Update findings without losing threads |
| APPROVED → REQUEST_CHANGES | Supersede old + POST new | Can always escalate; old APPROVED is no longer valid |
| REQUEST_CHANGES → APPROVED | POST new APPROVED | Can't edit state; old REQUEST_CHANGES stays as historical record |
| Sibling has REQUEST_CHANGES (worst-wins) | Escalate to REQUEST_CHANGES | PR must stay blocked if ANY reviewer blocks |
## Key Constraints
1. **Review state is immutable after POST** — Gitea has no API to change APPROVED ↔ REQUEST_CHANGES
2. **Never delete reviews** — Deleting cascades to inline comments and reply threads
3. **"Last review per user" wins** — Gitea uses the most recent review from a user for merge decisions
4. **REQUEST_CHANGES reviews are never touched** — Their inline comments and threads are preserved as historical record
5. **APPROVED reviews can be superseded** — When escalation is needed, mark old as superseded and POST new
## Worst-Wins (Shared Token)
When multiple reviewer roles share a token (e.g., `sonnet` and `security` both use `sonnet-review-bot`):
```
CI Matrix Run:
sonnet → REQUEST_CHANGES (findings)
security → APPROVED (no security issues)
security sees sibling REQUEST_CHANGES
security escalates → REQUEST_CHANGES
PR stays blocked ✓
```
The **first-run case** (no existing review to read login from) uses a post-posting fallback:
POST APPROVED → check siblings → if blocked, supersede own APPROVED → re-POST as REQUEST_CHANGES.
## Edit Mechanism
Reviews are edited via `PATCH /repos/{owner}/{repo}/issues/comments/{id}`:
- **Review body**: ID obtained from the timeline API (`/issues/{index}/timeline`, type `"review"`)
- **Inline comments**: IDs obtained from `/pulls/{index}/reviews/{id}/comments`
- **Both are editable** by the token that created them
- **ListReviews always returns the original body** (reads from review table, not comment table) — sentinel matching works regardless of edits
## Inline Comments Lifecycle
| Event | Inline comments behavior |
|-------|--------------------------|
| First POST | Created on specific diff lines |
| PATCH body (same state) | Unchanged — still current findings |
| Supersede (state change) | Old inline comments stay (readable but on outdated code) |
| New POST after supersede | Fresh inline comments on current diff |
+41 -528
View File
@@ -1,68 +1,35 @@
// Package gitea provides a client for the Gitea API.
// It supports pull request operations, file content retrieval,
// and review submission.
package gitea package gitea
import ( import (
"bytes"
"context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"io" "io"
"log/slog"
"net/http" "net/http"
"net/url"
"strings" "strings"
"time"
) )
// APIError represents an HTTP error response from the Gitea API.
// It carries the status code so callers can distinguish between
// different failure modes (e.g. 404 vs 500).
type APIError struct {
StatusCode int
Body string
}
func (e *APIError) Error() string {
body := e.Body
if len(body) > 200 {
body = body[:200] + "...(truncated)"
}
return fmt.Sprintf("HTTP %d: %s", e.StatusCode, body)
}
// IsNotFound reports whether an error is an API 404 response.
func IsNotFound(err error) bool {
var apiErr *APIError
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
}
// Client interacts with the Gitea API. // Client interacts with the Gitea API.
// A Client is safe for concurrent use by multiple goroutines.
type Client struct { type Client struct {
baseURL string BaseURL string
token string Token string
http *http.Client HTTP *http.Client
} }
// NewClient creates a new Gitea API client. // NewClient creates a new Gitea API client.
func NewClient(baseURL, token string) *Client { func NewClient(baseURL, token string) *Client {
return &Client{ return &Client{
baseURL: strings.TrimRight(baseURL, "/"), BaseURL: strings.TrimRight(baseURL, "/"),
token: token, Token: token,
http: &http.Client{Timeout: 30 * time.Second}, HTTP: &http.Client{},
} }
} }
// PullRequest holds relevant PR metadata. // PullRequest holds relevant PR metadata.
type PullRequest struct { type PullRequest struct {
Title string `json:"title"` Title string `json:"title"`
Body string `json:"body"` Body string `json:"body"`
Head struct { Head struct {
Sha string `json:"sha"` Sha string `json:"sha"`
Ref string `json:"ref"`
} `json:"head"` } `json:"head"`
} }
@@ -74,24 +41,10 @@ type CommitStatus struct {
TargetURL string `json:"target_url"` TargetURL string `json:"target_url"`
} }
// ChangedFile represents a file modified in a PR.
type ChangedFile struct {
Filename string `json:"filename"`
Status string `json:"status"`
}
// ReviewComment represents an inline comment to attach to a review.
type ReviewComment struct {
ID int64 `json:"id,omitempty"`
Path string `json:"path"`
NewPosition int64 `json:"new_position"`
Body string `json:"body"`
}
// GetPullRequest fetches PR metadata. // GetPullRequest fetches PR metadata.
func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { func (c *Client) GetPullRequest(owner, repo string, number int) (*PullRequest, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.BaseURL, owner, repo, number)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err) return nil, fmt.Errorf("fetch PR: %w", err)
} }
@@ -103,33 +56,19 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
} }
// GetPullRequestDiff fetches the unified diff for a PR. // GetPullRequestDiff fetches the unified diff for a PR.
func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { func (c *Client) GetPullRequestDiff(owner, repo string, number int) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.BaseURL, owner, repo, number)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch diff: %w", err) return "", fmt.Errorf("fetch diff: %w", err)
} }
return string(body), nil return string(body), nil
} }
// GetPullRequestFiles fetches the list of files changed in a PR.
func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("fetch PR files: %w", err)
}
var files []ChangedFile
if err := json.Unmarshal(body, &files); err != nil {
return nil, fmt.Errorf("parse PR files JSON: %w", err)
}
return files, nil
}
// GetCommitStatuses fetches CI statuses for a commit SHA. // GetCommitStatuses fetches CI statuses for a commit SHA.
func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { func (c *Client) GetCommitStatuses(owner, repo, sha string) ([]CommitStatus, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha)) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.BaseURL, owner, repo, sha)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch commit statuses: %w", err) return nil, fmt.Errorf("fetch commit statuses: %w", err)
} }
@@ -141,83 +80,61 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
} }
// GetFileContent fetches a file from the default branch of a repo. // GetFileContent fetches a file from the default branch of a repo.
func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { func (c *Client) GetFileContent(owner, repo, filepath string) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath)) url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.BaseURL, owner, repo, filepath)
body, err := c.doGet(ctx, reqURL) body, err := c.doGet(url)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s: %w", filepath, err) return "", fmt.Errorf("fetch file %s: %w", filepath, err)
} }
return string(body), nil return string(body), nil
} }
// GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo. // PostReview submits a review to a PR.
func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath), url.QueryEscape(ref))
body, err := c.doGet(ctx, reqURL)
if err != nil {
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
}
return string(body), nil
}
// PostReview submits a review to a PR and returns the created review.
// event should be "APPROVED" or "REQUEST_CHANGES". // event should be "APPROVED" or "REQUEST_CHANGES".
// comments are optional inline comments attached to specific lines. func (c *Client) PostReview(owner, repo string, number int, event, body string) error {
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string, comments []ReviewComment) (*Review, error) { url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.BaseURL, owner, repo, number)
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
payload := struct { payload := struct {
Body string `json:"body"` Body string `json:"body"`
Event string `json:"event"` Event string `json:"event"`
Comments []ReviewComment `json:"comments,omitempty"`
}{ }{
Body: body, Body: body,
Event: event, Event: event,
Comments: comments,
} }
data, err := json.Marshal(payload) data, err := json.Marshal(payload)
if err != nil { if err != nil {
return nil, fmt.Errorf("marshal review payload: %w", err) return fmt.Errorf("marshal review payload: %w", err)
} }
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data)) req, err := http.NewRequest("POST", url, strings.NewReader(string(data)))
if err != nil { if err != nil {
return nil, fmt.Errorf("create review request: %w", err) return fmt.Errorf("create review request: %w", err)
} }
req.Header.Set("Authorization", "token "+c.token) req.Header.Set("Authorization", "token "+c.Token)
req.Header.Set("Content-Type", "application/json") req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req) resp, err := c.HTTP.Do(req)
if err != nil { if err != nil {
return nil, fmt.Errorf("post review: %w", err) return fmt.Errorf("post review: %w", err)
} }
defer resp.Body.Close() defer resp.Body.Close()
if resp.StatusCode < 200 || resp.StatusCode >= 300 { if resp.StatusCode < 200 || resp.StatusCode >= 300 {
respBody, _ := io.ReadAll(resp.Body) respBody, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("post review failed (status %d): %s", resp.StatusCode, string(respBody)) return fmt.Errorf("post review failed (status %d): %s", resp.StatusCode, string(respBody))
} }
return nil
respBody, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("read review response: %w", err)
}
var review Review
if err := json.Unmarshal(respBody, &review); err != nil {
return nil, fmt.Errorf("parse review response: %w", err)
}
return &review, nil
} }
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) { func (c *Client) doGet(url string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) req, err := http.NewRequest("GET", url, nil)
if err != nil { if err != nil {
return nil, err return nil, err
} }
req.Header.Set("Authorization", "token "+c.token) req.Header.Set("Authorization", "token "+c.Token)
resp, err := c.http.Do(req) resp, err := c.HTTP.Do(req)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -225,411 +142,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
if resp.StatusCode < 200 || resp.StatusCode >= 300 { if resp.StatusCode < 200 || resp.StatusCode >= 300 {
body, _ := io.ReadAll(resp.Body) body, _ := io.ReadAll(resp.Body)
return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)} return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(body))
} }
return io.ReadAll(resp.Body) return io.ReadAll(resp.Body)
} }
// escapePath escapes each segment of a relative file path for use in URLs.
// Slashes are preserved as path separators; other special characters are escaped.
// Input should be a relative path (no leading slash). Already-encoded segments
// will be double-encoded, which is the desired behavior for user-provided paths.
func escapePath(p string) string {
parts := strings.Split(p, "/")
for i, part := range parts {
parts[i] = url.PathEscape(part)
}
return strings.Join(parts, "/")
}
// ContentEntry represents a file or directory entry from the contents API.
type ContentEntry struct {
Name string `json:"name"`
Path string `json:"path"`
Type string `json:"type"` // "file" or "dir"
}
// ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
var reqURL string
if path == "" {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
} else {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(path))
}
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("list contents %s: %w", path, err)
}
var entries []ContentEntry
if err := json.Unmarshal(body, &entries); err != nil {
return nil, fmt.Errorf("parse contents JSON: %w", err)
}
return entries, nil
}
// GetAllFilesInPath recursively fetches all file contents under a path.
// If the path is a file, returns just that file's content.
// If the path is a directory, recursively fetches all files within it.
func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) {
results := make(map[string]string)
// Try listing as directory first
entries, err := c.ListContents(ctx, owner, repo, path)
if err != nil {
// Only fall back to single-file fetch on 404 (path is a file, not a dir).
// Propagate all other errors (auth failures, server errors, rate limits).
if !IsNotFound(err) {
return nil, fmt.Errorf("list contents %q: %w", path, err)
}
// 404 means the path might be a file — try fetching directly
content, fileErr := c.GetFileContent(ctx, owner, repo, path)
if fileErr != nil {
return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr)
}
results[path] = content
return results, nil
}
for _, entry := range entries {
switch entry.Type {
case "file":
content, err := c.GetFileContent(ctx, owner, repo, entry.Path)
if err != nil {
slog.Warn("could not fetch file from patterns repo", "file", entry.Path, "error", err)
continue
}
results[entry.Path] = content
case "dir":
subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path)
if err != nil {
slog.Warn("could not recurse into directory", "dir", entry.Path, "error", err)
continue
}
for k, v := range subResults {
results[k] = v
}
}
}
return results, nil
}
// Review represents a pull request review from the Gitea API.
type Review struct {
ID int64 `json:"id"`
Body string `json:"body"`
User struct {
Login string `json:"login"`
} `json:"user"`
State string `json:"state"`
Stale bool `json:"stale"`
CommitID string `json:"commit_id"`
}
// ListReviews returns all reviews on a pull request.
// Paginates through all pages to ensure no reviews are missed.
func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) {
const pageSize = 50
var all []Review
for page := 1; ; page++ {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews?limit=%d&page=%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number,
pageSize,
page)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("list reviews (page %d): %w", page, err)
}
var batch []Review
if err := json.Unmarshal(body, &batch); err != nil {
return nil, fmt.Errorf("parse reviews (page %d): %w", page, err)
}
all = append(all, batch...)
if len(batch) < pageSize {
break
}
}
return all, nil
}
// DeleteReview deletes a review by ID. The token must belong to the review author.
func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number,
reviewID)
req, err := http.NewRequestWithContext(ctx, http.MethodDelete, reqURL, nil)
if err != nil {
return fmt.Errorf("create delete request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("delete review: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
respBody, _ := io.ReadAll(resp.Body)
return fmt.Errorf("delete review failed (status %d): %s", resp.StatusCode, string(respBody))
}
return nil
}
// TimelineEvent represents an entry from the issue timeline API.
type TimelineEvent struct {
ID int64 `json:"id"`
Type string `json:"type"`
Body string `json:"body"`
User struct {
Login string `json:"login"`
} `json:"user"`
}
// GetTimelineReviewCommentID finds the comment ID for a review body by
// scanning the issue timeline for a review event containing the sentinel.
func (c *Client) GetTimelineReviewCommentID(ctx context.Context, owner, repo string, number int, sentinel string) (int64, error) {
const pageSize = 50
for page := 1; ; page++ {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/timeline?limit=%d&page=%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number,
pageSize,
page)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
}
var events []TimelineEvent
if err := json.Unmarshal(body, &events); err != nil {
return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
}
for _, ev := range events {
if ev.Type == "review" && strings.Contains(ev.Body, sentinel) {
return ev.ID, nil
}
}
if len(events) < pageSize {
break
}
}
return 0, fmt.Errorf("no timeline event found with sentinel")
}
// GetTimelineReviewCommentIDForReview finds the timeline comment ID for a
// specific review by matching its body content in the timeline.
func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
// Use the reviews API to get the review body, then find in timeline
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number,
reviewID)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return 0, fmt.Errorf("get review %d: %w", reviewID, err)
}
var review struct {
Body string `json:"body"`
User struct {
Login string `json:"login"`
} `json:"user"`
}
if err := json.Unmarshal(body, &review); err != nil {
return 0, fmt.Errorf("parse review %d: %w", reviewID, err)
}
if review.Body == "" {
return 0, fmt.Errorf("review %d has empty body", reviewID)
}
// Use a prefix for matching (handles minor trailing whitespace differences)
matchPrefix := review.Body
if len(matchPrefix) > 200 {
matchPrefix = matchPrefix[:200]
}
const pageSize = 50
for page := 1; ; page++ {
timelineURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/timeline?limit=%d&page=%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number,
pageSize,
page)
tlBody, err := c.doGet(ctx, timelineURL)
if err != nil {
return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
}
var events []TimelineEvent
if err := json.Unmarshal(tlBody, &events); err != nil {
return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
}
for _, ev := range events {
if ev.Type == "review" && ev.User.Login == review.User.Login && strings.HasPrefix(ev.Body, matchPrefix) {
return ev.ID, nil
}
}
if len(events) < pageSize {
break
}
}
return 0, fmt.Errorf("no timeline event found for review %d", reviewID)
}
// EditComment updates the body of an issue/review comment.
func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID int64, newBody string) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/comments/%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
commentID)
payload := struct {
Body string `json:"body"`
}{Body: newBody}
data, err := json.Marshal(payload)
if err != nil {
return fmt.Errorf("marshal edit payload: %w", err)
}
req, err := http.NewRequestWithContext(ctx, http.MethodPatch, reqURL, bytes.NewReader(data))
if err != nil {
return fmt.Errorf("create edit request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("edit comment: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf("edit comment failed (status %d): %s", resp.StatusCode, body)
}
return nil
}
// GetAuthenticatedUser returns the login of the user authenticated by the token.
func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
reqURL := fmt.Sprintf("%s/api/v1/user", c.baseURL)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return "", fmt.Errorf("get authenticated user: %w", err)
}
var result struct {
Login string `json:"login"`
}
if err := json.Unmarshal(body, &result); err != nil {
return "", fmt.Errorf("parse user response: %w", err)
}
return result.Login, nil
}
// RequestReviewer adds the given user as a requested reviewer on a pull request.
// This is idempotent — requesting an already-requested reviewer is a no-op.
func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/requested_reviewers",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
number)
payload := struct {
Reviewers []string `json:"reviewers"`
}{Reviewers: []string{reviewer}}
data, err := json.Marshal(payload)
if err != nil {
return fmt.Errorf("marshal reviewer request: %w", err)
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
if err != nil {
return fmt.Errorf("create reviewer request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
req.Header.Set("Content-Type", "application/json")
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("request reviewer: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
}
return nil
}
// ListReviewComments returns the inline comments attached to a specific review.
// Paginates through all pages.
func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) {
const pageSize = 50
var all []ReviewComment
for page := 1; ; page++ {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
prNumber,
reviewID,
pageSize,
page)
body, err := c.doGet(ctx, reqURL)
if err != nil {
return nil, fmt.Errorf("list review comments (page %d): %w", page, err)
}
var batch []ReviewComment
if err := json.Unmarshal(body, &batch); err != nil {
return nil, fmt.Errorf("parse review comments (page %d): %w", page, err)
}
all = append(all, batch...)
if len(batch) < pageSize {
break
}
}
return all, nil
}
// ResolveComment marks an inline review comment as resolved.
func (c *Client) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/comments/%d/resolve",
c.baseURL,
url.PathEscape(owner),
url.PathEscape(repo),
commentID)
req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, nil)
if err != nil {
return fmt.Errorf("create resolve request: %w", err)
}
req.Header.Set("Authorization", "token "+c.token)
resp, err := c.http.Do(req)
if err != nil {
return fmt.Errorf("resolve comment: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
return fmt.Errorf("resolve comment failed (status %d): %s", resp.StatusCode, body)
}
return nil
}
+9 -559
View File
@@ -1,14 +1,9 @@
package gitea package gitea
import ( import (
"context"
"encoding/json" "encoding/json"
"errors"
"fmt"
"io"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings"
"testing" "testing"
) )
@@ -32,7 +27,7 @@ func TestGetPullRequest(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetPullRequest(context.Background(), "owner", "repo", 1) got, err := client.GetPullRequest("owner", "repo", 1)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -59,7 +54,7 @@ func TestGetPullRequestDiff(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetPullRequestDiff(context.Background(), "owner", "repo", 5) got, err := client.GetPullRequestDiff("owner", "repo", 5)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -84,7 +79,7 @@ func TestGetCommitStatuses(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetCommitStatuses(context.Background(), "owner", "repo", "abc123") got, err := client.GetCommitStatuses("owner", "repo", "abc123")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -126,21 +121,15 @@ func TestPostReview(t *testing.T) {
} }
w.WriteHeader(http.StatusOK) w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"id":100,"user":{"login":"review-bot"},"state":"APPROVED","stale":false}`)) w.Write([]byte(`{}`))
})) }))
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", nil) err := client.PostReview("owner", "repo", 3, "APPROVED", "LGTM")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
if review.ID != 100 {
t.Errorf("expected review ID 100, got %d", review.ID)
}
if review.User.Login != "review-bot" {
t.Errorf("expected user login %q, got %q", "review-bot", review.User.Login)
}
} }
func TestGetPullRequest_Non200(t *testing.T) { func TestGetPullRequest_Non200(t *testing.T) {
@@ -151,7 +140,7 @@ func TestGetPullRequest_Non200(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
_, err := client.GetPullRequest(context.Background(), "owner", "repo", 999) _, err := client.GetPullRequest("owner", "repo", 999)
if err == nil { if err == nil {
t.Fatal("expected error for 404, got nil") t.Fatal("expected error for 404, got nil")
} }
@@ -164,7 +153,7 @@ func TestGetPullRequest_BadJSON(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
_, err := client.GetPullRequest(context.Background(), "owner", "repo", 1) _, err := client.GetPullRequest("owner", "repo", 1)
if err == nil { if err == nil {
t.Fatal("expected error for bad JSON, got nil") t.Fatal("expected error for bad JSON, got nil")
} }
@@ -178,7 +167,7 @@ func TestPostReview_Non200(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", nil) err := client.PostReview("owner", "repo", 1, "APPROVED", "test")
if err == nil { if err == nil {
t.Fatal("expected error for 403, got nil") t.Fatal("expected error for 403, got nil")
} }
@@ -196,7 +185,7 @@ func TestGetFileContent(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-token") client := NewClient(server.URL, "test-token")
got, err := client.GetFileContent(context.Background(), "owner", "repo", "CONVENTIONS.md") got, err := client.GetFileContent("owner", "repo", "CONVENTIONS.md")
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -204,542 +193,3 @@ func TestGetFileContent(t *testing.T) {
t.Errorf("expected %q, got %q", expected, got) t.Errorf("expected %q, got %q", expected, got)
} }
} }
func TestGetPullRequestFiles(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/pulls/1/files" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`[{"filename":"main.go","status":"modified"},{"filename":"old.go","status":"removed"}]`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
files, err := client.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 2 {
t.Fatalf("expected 2 files, got %d", len(files))
}
if files[0].Filename != "main.go" || files[0].Status != "modified" {
t.Errorf("unexpected first file: %+v", files[0])
}
}
func TestGetFileContentRef(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/raw/main.go" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.URL.Query().Get("ref") != "feature-branch" {
t.Errorf("unexpected ref: %s", r.URL.Query().Get("ref"))
}
w.Write([]byte("package main\n"))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
content, err := client.GetFileContentRef(context.Background(), "owner", "repo", "main.go", "feature-branch")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if content != "package main\n" {
t.Errorf("unexpected content: %q", content)
}
}
func TestListContents(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/contents/docs" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprintf(w, `[{"name":"guide.md","path":"docs/guide.md","type":"file"},{"name":"sub","path":"docs/sub","type":"dir"}]`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
entries, err := client.ListContents(context.Background(), "owner", "repo", "docs")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(entries) != 2 {
t.Fatalf("expected 2 entries, got %d", len(entries))
}
if entries[0].Type != "file" || entries[0].Path != "docs/guide.md" {
t.Errorf("unexpected first entry: %+v", entries[0])
}
if entries[1].Type != "dir" {
t.Errorf("expected dir type, got %s", entries[1].Type)
}
}
func TestGetAllFilesInPath_File(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/api/v1/repos/owner/repo/contents/README.md" {
// Gitea returns 404 for contents API on files (it's not a dir)
http.NotFound(w, r)
return
}
if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
fmt.Fprintf(w, "# Hello")
return
}
http.NotFound(w, r)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 1 {
t.Fatalf("expected 1 file, got %d", len(files))
}
if files["README.md"] != "# Hello" {
t.Errorf("unexpected content: %q", files["README.md"])
}
}
func TestEscapePath(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"simple", "src/main.go", "src/main.go"},
{"spaces", "my dir/my file.go", "my%20dir/my%20file.go"},
{"special chars", "path/file#1.txt", "path/file%231.txt"},
{"empty", "", ""},
{"single segment", "README.md", "README.md"},
{"nested deep", "a/b/c/d.md", "a/b/c/d.md"},
{"already encoded", "path/file%20name.go", "path/file%2520name.go"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := escapePath(tt.input)
if got != tt.want {
t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want)
}
})
}
}
func TestListReviews(t *testing.T) {
pageCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.URL.Query().Get("limit") != "50" {
t.Errorf("expected limit=50, got %s", r.URL.Query().Get("limit"))
}
pageCount++
w.Header().Set("Content-Type", "application/json")
// Return 2 results (less than page size) to signal end
w.Write([]byte(`[{"id":10,"user":{"login":"bot-a"},"state":"APPROVED","stale":false},{"id":11,"user":{"login":"bot-b"},"state":"REQUEST_CHANGES","stale":true}]`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(reviews) != 2 {
t.Fatalf("expected 2 reviews, got %d", len(reviews))
}
if reviews[0].User.Login != "bot-a" {
t.Errorf("expected bot-a, got %s", reviews[0].User.Login)
}
if pageCount != 1 {
t.Errorf("expected 1 page fetch (results < page size), got %d", pageCount)
}
}
func TestListReviews_Pagination(t *testing.T) {
pageCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
pageCount++
page := r.URL.Query().Get("page")
w.Header().Set("Content-Type", "application/json")
if page == "1" {
// Return exactly 50 items to trigger next page fetch
items := "["
for i := 0; i < 50; i++ {
if i > 0 {
items += ","
}
items += fmt.Sprintf(`{"id":%d,"user":{"login":"bot"},"state":"APPROVED","stale":false}`, i+1)
}
items += "]"
w.Write([]byte(items))
} else {
// Page 2: return fewer than 50 to signal end
w.Write([]byte(`[{"id":51,"user":{"login":"bot"},"state":"APPROVED","stale":false}]`))
}
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(reviews) != 51 {
t.Fatalf("expected 51 reviews across 2 pages, got %d", len(reviews))
}
if pageCount != 2 {
t.Errorf("expected 2 page fetches, got %d", pageCount)
}
}
func TestDeleteReview(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews/10" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.Method != "DELETE" {
t.Errorf("expected DELETE, got %s", r.Method)
}
w.WriteHeader(http.StatusNoContent)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestDeleteReview_Forbidden(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte(`{"message":"forbidden"}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10)
if err == nil {
t.Fatal("expected error for 403, got nil")
}
}
func TestEditComment(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPatch {
t.Errorf("expected PATCH, got %s", r.Method)
}
if r.URL.Path != "/api/v1/repos/owner/repo/issues/comments/42" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
var payload struct {
Body string `json:"body"`
}
json.NewDecoder(r.Body).Decode(&payload)
if payload.Body != "updated body" {
t.Errorf("unexpected body: %s", payload.Body)
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"id": 42, "body": "updated body"}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.EditComment(context.Background(), "owner", "repo", 42, "updated body")
if err != nil {
t.Fatalf("EditComment() error = %v", err)
}
}
func TestEditComment_Forbidden(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte(`{"message": "not allowed"}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.EditComment(context.Background(), "owner", "repo", 42, "new body")
if err == nil {
t.Fatal("expected error for 403 response")
}
}
func TestGetTimelineReviewCommentID(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/issues/5/timeline" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Write([]byte(`[
{"id": 100, "type": "comment", "body": "random"},
{"id": 200, "type": "review", "body": "other review <!-- review-bot:gpt -->"},
{"id": 300, "type": "review", "body": "our review <!-- review-bot:sonnet -->"}
]`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
id, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
if err != nil {
t.Fatalf("GetTimelineReviewCommentID() error = %v", err)
}
if id != 300 {
t.Errorf("got id=%d, want 300", id)
}
}
func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`[{"id": 100, "type": "review", "body": "no match"}]`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
if err == nil {
t.Fatal("expected error when sentinel not found")
}
}
func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/repos/owner/repo/contents/README.md":
// Contents API returns 404 for files (not a directory)
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"message":"not found"}`))
case "/api/v1/repos/owner/repo/raw/README.md":
w.Write([]byte("# Hello\n"))
default:
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"message":"not found"}`))
}
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
if err != nil {
t.Fatalf("expected fallback to file on 404, got error: %v", err)
}
if len(files) != 1 {
t.Fatalf("expected 1 file, got %d", len(files))
}
if files["README.md"] != "# Hello\n" {
t.Errorf("unexpected content: %q", files["README.md"])
}
}
func TestGetAllFilesInPath_500Propagates(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Simulate a server error from ListContents
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(`{"message":"internal server error"}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "somepath")
if err == nil {
t.Fatal("expected error to propagate for 500, got nil")
}
// Should NOT fall back to file fetch — error should propagate
var apiErr *APIError
if !errors.As(err, &apiErr) {
t.Fatalf("expected APIError in chain, got: %v", err)
}
if apiErr.StatusCode != http.StatusInternalServerError {
t.Errorf("expected status 500, got %d", apiErr.StatusCode)
}
}
func TestGetAllFilesInPath_403Propagates(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte(`{"message":"token has insufficient scope"}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "private/stuff")
if err == nil {
t.Fatal("expected error to propagate for 403, got nil")
}
var apiErr *APIError
if !errors.As(err, &apiErr) {
t.Fatalf("expected APIError in chain, got: %v", err)
}
if apiErr.StatusCode != http.StatusForbidden {
t.Errorf("expected status 403, got %d", apiErr.StatusCode)
}
}
func TestIsNotFound(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{"nil error", nil, false},
{"non-API error", fmt.Errorf("network timeout"), false},
{"404 APIError", &APIError{StatusCode: 404, Body: "not found"}, true},
{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, false},
{"wrapped 404", fmt.Errorf("list contents: %w", &APIError{StatusCode: 404, Body: "not found"}), true},
{"wrapped 500", fmt.Errorf("list contents: %w", &APIError{StatusCode: 500, Body: "err"}), false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IsNotFound(tt.err)
if got != tt.want {
t.Errorf("IsNotFound(%v) = %v, want %v", tt.err, got, tt.want)
}
})
}
}
func TestGetAuthenticatedUser(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/user" {
t.Errorf("unexpected path: %s", r.URL.Path)
http.NotFound(w, r)
return
}
if r.Header.Get("Authorization") != "token test-token" {
t.Error("missing or wrong auth header")
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprint(w, `{"login":"my-bot","id":42}`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
login, err := client.GetAuthenticatedUser(context.Background())
if err != nil {
t.Fatalf("GetAuthenticatedUser() error = %v", err)
}
if login != "my-bot" {
t.Errorf("login = %q, want %q", login, "my-bot")
}
}
func TestRequestReviewer(t *testing.T) {
var gotBody []byte
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
expected := "/api/v1/repos/owner/repo/pulls/7/requested_reviewers"
if r.URL.Path != expected {
t.Errorf("path = %q, want %q", r.URL.Path, expected)
}
gotBody, _ = io.ReadAll(r.Body)
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
if err != nil {
t.Fatalf("RequestReviewer() error = %v", err)
}
if !strings.Contains(string(gotBody), `"bot-user"`) {
t.Errorf("body = %s, want to contain bot-user", gotBody)
}
}
func TestRequestReviewer_204(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
if err != nil {
t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
}
}
func TestRequestReviewer_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
fmt.Fprint(w, "no permission")
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
if err == nil {
t.Fatal("expected error for 403 response")
}
if !strings.Contains(err.Error(), "403") {
t.Errorf("error should mention status code: %v", err)
}
}
func TestListReviewComments(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !strings.Contains(r.URL.Path, "/pulls/1/reviews/42/comments") {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
if err != nil {
t.Fatalf("ListReviewComments() error = %v", err)
}
if len(comments) != 2 {
t.Fatalf("got %d comments, want 2", len(comments))
}
if comments[0].ID != 100 {
t.Errorf("comments[0].ID = %d, want 100", comments[0].ID)
}
if comments[1].Path != "lib.go" {
t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go")
}
}
func TestResolveComment(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if !strings.Contains(r.URL.Path, "/pulls/comments/99/resolve") {
t.Errorf("unexpected path: %s", r.URL.Path)
}
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
if err != nil {
t.Fatalf("ResolveComment() error = %v", err)
}
}
func TestResolveComment_Error(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
fmt.Fprint(w, "not found")
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
if err == nil {
t.Fatal("expected error for 404 response")
}
}
-85
View File
@@ -1,85 +0,0 @@
package gitea
import (
"strconv"
"strings"
)
// DiffLineRanges maps filenames to the set of new-file line numbers present in the diff.
type DiffLineRanges struct {
files map[string]map[int]bool
}
// Contains reports whether the given file+line is within the diff hunks.
func (d *DiffLineRanges) Contains(file string, line int) bool {
if d == nil || d.files == nil {
return false
}
lines, ok := d.files[file]
if !ok {
return false
}
return lines[line]
}
// ParseDiffNewLines parses a unified diff and extracts the new-file line numbers
// that appear in each hunk (both added and context lines).
func ParseDiffNewLines(diff string) *DiffLineRanges {
result := &DiffLineRanges{files: make(map[string]map[int]bool)}
var currentFile string
var newLine int
for _, line := range strings.Split(diff, "\n") {
// Track current file from +++ header
if strings.HasPrefix(line, "+++ b/") {
currentFile = strings.TrimPrefix(line, "+++ b/")
if result.files[currentFile] == nil {
result.files[currentFile] = make(map[int]bool)
}
continue
}
if strings.HasPrefix(line, "+++ /dev/null") {
currentFile = ""
continue
}
// Parse hunk header: @@ -old,count +new,count @@ or @@ -old +new @@
if strings.HasPrefix(line, "@@") && currentFile != "" {
// Extract the +N part — handle both "+10,8" and "+1" forms
parts := strings.Split(line, "+")
if len(parts) >= 2 {
// Take everything before comma or space
numStr := parts[1]
if idx := strings.IndexAny(numStr, ", "); idx != -1 {
numStr = numStr[:idx]
}
n, err := strconv.Atoi(numStr)
if err == nil {
newLine = n
}
}
continue
}
if currentFile == "" {
continue
}
// Skip diff metadata lines
if strings.HasPrefix(line, "\\") {
continue
}
// Count lines in hunk
if strings.HasPrefix(line, "+") || strings.HasPrefix(line, " ") {
result.files[currentFile][newLine] = true
newLine++
} else if strings.HasPrefix(line, "-") {
// Removed lines don't advance new line counter
continue
}
}
return result
}
-115
View File
@@ -1,115 +0,0 @@
package gitea
import (
"testing"
)
func TestParseDiffLineRanges(t *testing.T) {
diff := `diff --git a/main.go b/main.go
index abc1234..def5678 100644
--- a/main.go
+++ b/main.go
@@ -10,6 +10,8 @@ func main() {
fmt.Println("hello")
+ fmt.Println("new line 11")
+ fmt.Println("new line 12")
fmt.Println("existing")
}
@@ -30,4 +32,5 @@ func other() {
return nil
+ // added at line 33
}
diff --git a/util.go b/util.go
new file mode 100644
--- /dev/null
+++ b/util.go
@@ -0,0 +1,5 @@
+package main
+
+func helper() string {
+ return "hi"
+}
`
ranges := ParseDiffNewLines(diff)
// main.go should have lines 10-17 (first hunk) and 32-36 (second hunk)
if !ranges.Contains("main.go", 11) {
t.Error("expected main.go:11 to be in diff")
}
if !ranges.Contains("main.go", 12) {
t.Error("expected main.go:12 to be in diff")
}
if !ranges.Contains("main.go", 10) {
t.Error("expected main.go:10 to be in diff (context line)")
}
if !ranges.Contains("main.go", 33) {
t.Error("expected main.go:33 to be in diff")
}
if ranges.Contains("main.go", 25) {
t.Error("main.go:25 should NOT be in diff")
}
// util.go is entirely new, lines 1-5
if !ranges.Contains("util.go", 1) {
t.Error("expected util.go:1 to be in diff")
}
if !ranges.Contains("util.go", 5) {
t.Error("expected util.go:5 to be in diff")
}
if ranges.Contains("util.go", 6) {
t.Error("util.go:6 should NOT be in diff")
}
// Unknown file
if ranges.Contains("unknown.go", 1) {
t.Error("unknown.go should not be in diff")
}
}
func TestParseDiffNewLines_Empty(t *testing.T) {
ranges := ParseDiffNewLines("")
if ranges.Contains("any.go", 1) {
t.Error("empty diff should contain nothing")
}
}
func TestParseDiffNewLines_NoCommaHunk(t *testing.T) {
// Single-line hunks omit the comma: @@ -1 +1 @@
diff := `diff --git a/single.go b/single.go
--- a/single.go
+++ b/single.go
@@ -1 +1 @@
-old line
+new line
`
ranges := ParseDiffNewLines(diff)
if !ranges.Contains("single.go", 1) {
t.Error("expected single.go:1 to be in diff (no-comma hunk)")
}
if ranges.Contains("single.go", 2) {
t.Error("single.go:2 should NOT be in diff")
}
}
func TestParseDiffNewLines_NoNewlineMarker(t *testing.T) {
// "\ No newline at end of file" should not advance line counter
diff := `diff --git a/noeof.go b/noeof.go
--- a/noeof.go
+++ b/noeof.go
@@ -1,2 +1,2 @@
+line one
+line two
\ No newline at end of file
`
ranges := ParseDiffNewLines(diff)
if !ranges.Contains("noeof.go", 1) {
t.Error("expected noeof.go:1")
}
if !ranges.Contains("noeof.go", 2) {
t.Error("expected noeof.go:2")
}
if ranges.Contains("noeof.go", 3) {
t.Error("noeof.go:3 should NOT be in diff (no-newline marker)")
}
}
-88
View File
@@ -1,88 +0,0 @@
package gitea
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
)
func TestPostReview_WithComments(t *testing.T) {
var gotPayload struct {
Body string `json:"body"`
Event string `json:"event"`
Comments []struct {
Path string `json:"path"`
NewPosition int64 `json:"new_position"`
Body string `json:"body"`
} `json:"comments"`
}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
json.NewDecoder(r.Body).Decode(&gotPayload)
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
json.NewEncoder(w).Encode(map[string]any{
"id": 99,
"body": gotPayload.Body,
"user": map[string]any{"login": "bot"},
})
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
comments := []ReviewComment{
{Path: "main.go", NewPosition: 42, Body: "[MAJOR] Something bad"},
{Path: "util.go", NewPosition: 10, Body: "[MINOR] Style issue"},
}
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "REQUEST_CHANGES", "summary", comments)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(gotPayload.Comments) != 2 {
t.Fatalf("expected 2 comments, got %d", len(gotPayload.Comments))
}
if gotPayload.Comments[0].Path != "main.go" {
t.Errorf("expected path main.go, got %s", gotPayload.Comments[0].Path)
}
if gotPayload.Comments[0].NewPosition != 42 {
t.Errorf("expected new_position 42, got %d", gotPayload.Comments[0].NewPosition)
}
if gotPayload.Comments[1].Body != "[MINOR] Style issue" {
t.Errorf("unexpected body: %s", gotPayload.Comments[1].Body)
}
}
func TestPostReview_NilComments(t *testing.T) {
var gotPayload map[string]any
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
json.NewDecoder(r.Body).Decode(&gotPayload)
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
json.NewEncoder(w).Encode(map[string]any{
"id": 100,
"body": "test",
"user": map[string]any{"login": "bot"},
})
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// With nil comments, the field should be omitted (omitempty)
comments, ok := gotPayload["comments"]
if ok && comments != nil {
arr, isArr := comments.([]any)
if isArr && len(arr) > 0 {
t.Error("expected no comments in payload when nil passed")
}
}
}
+12 -13
View File
@@ -3,10 +3,8 @@
package main package main
import ( import (
"context"
"os" "os"
"strconv" "strconv"
"strings"
"testing" "testing"
"gitea.weiker.me/rodin/review-bot/gitea" "gitea.weiker.me/rodin/review-bot/gitea"
@@ -44,27 +42,28 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
} }
// Parse owner/repo // Parse owner/repo
parts := strings.SplitN(giteaRepo, "/", 2) owner, repoName := "", ""
if len(parts) != 2 { for i, c := range giteaRepo {
t.Fatalf("Invalid repo format %q", giteaRepo) if c == / {
owner = giteaRepo[:i]
repoName = giteaRepo[i+1:]
break
}
} }
owner, repoName := parts[0], parts[1]
if owner == "" || repoName == "" { if owner == "" || repoName == "" {
t.Fatalf("Invalid repo format %q", giteaRepo) t.Fatalf("Invalid repo format %q", giteaRepo)
} }
ctx := context.Background()
// Step 1: Fetch PR // Step 1: Fetch PR
giteaClient := gitea.NewClient(giteaURL, giteaToken) giteaClient := gitea.NewClient(giteaURL, giteaToken)
pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) pr, err := giteaClient.GetPullRequest(owner, repoName, prNumber)
if err != nil { if err != nil {
t.Fatalf("GetPullRequest: %v", err) t.Fatalf("GetPullRequest: %v", err)
} }
t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha) t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha)
// Step 2: Fetch diff // Step 2: Fetch diff
diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber) diff, err := giteaClient.GetPullRequestDiff(owner, repoName, prNumber)
if err != nil { if err != nil {
t.Fatalf("GetPullRequestDiff: %v", err) t.Fatalf("GetPullRequestDiff: %v", err)
} }
@@ -74,12 +73,12 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
t.Logf("Diff size: %d bytes", len(diff)) t.Logf("Diff size: %d bytes", len(diff))
// Step 3: Build prompts // Step 3: Build prompts
systemPrompt := review.BuildSystemPrompt("", "") systemPrompt := review.BuildSystemPrompt("")
userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, "", true, "") userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, true, "")
// Step 4: Call LLM // Step 4: Call LLM
llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel) llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel)
response, err := llmClient.Complete(ctx, []llm.Message{ response, err := llmClient.Complete([]llm.Message{
{Role: "system", Content: systemPrompt}, {Role: "system", Content: systemPrompt},
{Role: "user", Content: userPrompt}, {Role: "user", Content: userPrompt},
}) })
+34 -174
View File
@@ -1,98 +1,46 @@
// Package llm provides clients for LLM chat completion APIs.
//
// Supports OpenAI-compatible (default) and Anthropic Messages API providers.
package llm package llm
import ( import (
"bytes" "bytes"
"context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
"strings" "strings"
"time"
) )
// Provider identifies which API format to use. // Client calls an OpenAI-compatible chat completion API.
type Provider string
const (
// ProviderOpenAI uses the OpenAI-compatible chat/completions endpoint.
ProviderOpenAI Provider = "openai"
// ProviderAnthropic uses the Anthropic Messages API endpoint.
ProviderAnthropic Provider = "anthropic"
)
// Client calls an LLM chat completion API.
// A Client is safe for concurrent use by multiple goroutines after construction.
// WithTimeout, WithTemperature, and WithProvider must be called during setup,
// before concurrent use.
type Client struct { type Client struct {
baseURL string BaseURL string
apiKey string APIKey string
model string Model string
temperature float64 HTTP *http.Client
provider Provider
http *http.Client
} }
// NewClient creates a new LLM client. Default provider is OpenAI-compatible. // NewClient creates a new LLM client.
func NewClient(baseURL, apiKey, model string) *Client { func NewClient(baseURL, apiKey, model string) *Client {
return &Client{ return &Client{
baseURL: strings.TrimRight(baseURL, "/"), BaseURL: strings.TrimRight(baseURL, "/"),
apiKey: apiKey, APIKey: apiKey,
model: model, Model: model,
provider: ProviderOpenAI, HTTP: &http.Client{},
http: &http.Client{Timeout: 5 * time.Minute},
} }
} }
// WithTimeout sets the HTTP request timeout for LLM calls (default 5 minutes).
func (c *Client) WithTimeout(d time.Duration) *Client {
c.http.Timeout = d
return c
}
// WithTemperature sets the temperature for LLM requests (0 = omit, uses server default).
func (c *Client) WithTemperature(t float64) *Client {
c.temperature = t
return c
}
// WithProvider sets the API provider format (openai or anthropic).
func (c *Client) WithProvider(p Provider) *Client {
c.provider = p
return c
}
// Message represents a chat message. // Message represents a chat message.
type Message struct { type Message struct {
Role string `json:"role"` Role string `json:"role"`
Content string `json:"content"` Content string `json:"content"`
} }
// Complete sends a chat completion request and returns the assistant's response content. // ChatRequest is the request payload.
// The first message with role "system" is treated as the system prompt.
func (c *Client) Complete(ctx context.Context, messages []Message) (string, error) {
switch c.provider {
case ProviderAnthropic:
return c.completeAnthropic(ctx, messages)
default:
return c.completeOpenAI(ctx, messages)
}
}
// --- OpenAI-compatible implementation ---
// ChatRequest is the OpenAI request payload.
type ChatRequest struct { type ChatRequest struct {
Model string `json:"model"` Model string `json:"model"`
Messages []Message `json:"messages"` Messages []Message `json:"messages"`
Temperature float64 `json:"temperature,omitempty"` Temperature float64 `json:"temperature"`
} }
// ChatResponse is the OpenAI response. // ChatResponse is the response from the API.
type ChatResponse struct { type ChatResponse struct {
Choices []struct { Choices []struct {
Message struct { Message struct {
@@ -101,11 +49,12 @@ type ChatResponse struct {
} `json:"choices"` } `json:"choices"`
} }
func (c *Client) completeOpenAI(ctx context.Context, messages []Message) (string, error) { // Complete sends a chat completion request and returns the assistant's response content.
func (c *Client) Complete(messages []Message) (string, error) {
reqBody := ChatRequest{ reqBody := ChatRequest{
Model: c.model, Model: c.Model,
Temperature: c.temperature,
Messages: messages, Messages: messages,
Temperature: 0.1,
} }
data, err := json.Marshal(reqBody) data, err := json.Marshal(reqBody)
@@ -113,127 +62,38 @@ func (c *Client) completeOpenAI(ctx context.Context, messages []Message) (string
return "", fmt.Errorf("marshal request: %w", err) return "", fmt.Errorf("marshal request: %w", err)
} }
url := c.baseURL + "/chat/completions" url := c.BaseURL + "/chat/completions"
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(data)) req, err := http.NewRequest("POST", url, bytes.NewReader(data))
if err != nil { if err != nil {
return "", fmt.Errorf("create request: %w", err) return "", fmt.Errorf("create request: %w", err)
} }
req.Header.Set("Authorization", "Bearer "+c.apiKey) req.Header.Set("Authorization", "Bearer "+c.APIKey)
req.Header.Set("Content-Type", "application/json") req.Header.Set("Content-Type", "application/json")
return c.doRequest(req, func(body []byte) (string, error) { resp, err := c.HTTP.Do(req)
var resp ChatResponse
if err := json.Unmarshal(body, &resp); err != nil {
return "", fmt.Errorf("parse response: %w", err)
}
if len(resp.Choices) == 0 {
return "", fmt.Errorf("no choices in LLM response")
}
return resp.Choices[0].Message.Content, nil
})
}
// --- Anthropic Messages API implementation ---
type anthropicRequest struct {
Model string `json:"model"`
MaxTokens int `json:"max_tokens"`
System string `json:"system,omitempty"`
Messages []anthropicMsg `json:"messages"`
Temperature float64 `json:"temperature,omitempty"`
}
type anthropicMsg struct {
Role string `json:"role"`
Content string `json:"content"`
}
type anthropicResponse struct {
Content []struct {
Type string `json:"type"`
Text string `json:"text"`
} `json:"content"`
}
func (c *Client) completeAnthropic(ctx context.Context, messages []Message) (string, error) {
// Extract system message (first message with role "system")
var system string
var userMessages []anthropicMsg
for _, m := range messages {
if m.Role == "system" {
system = m.Content
} else {
userMessages = append(userMessages, anthropicMsg{
Role: m.Role,
Content: m.Content,
})
}
}
reqBody := anthropicRequest{
Model: c.model,
MaxTokens: 8192,
System: system,
Messages: userMessages,
}
if c.temperature > 0 {
reqBody.Temperature = c.temperature
}
data, err := json.Marshal(reqBody)
if err != nil {
return "", fmt.Errorf("marshal request: %w", err)
}
url := c.baseURL + "/messages"
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(data))
if err != nil {
return "", fmt.Errorf("create request: %w", err)
}
req.Header.Set("x-api-key", c.apiKey)
req.Header.Set("anthropic-version", "2023-06-01")
req.Header.Set("Content-Type", "application/json")
return c.doRequest(req, func(body []byte) (string, error) {
var resp anthropicResponse
if err := json.Unmarshal(body, &resp); err != nil {
return "", fmt.Errorf("parse response: %w", err)
}
if len(resp.Content) == 0 {
return "", fmt.Errorf("no content in Anthropic response")
}
// Concatenate all text blocks
var sb strings.Builder
for _, block := range resp.Content {
if block.Type == "text" {
sb.WriteString(block.Text)
}
}
result := sb.String()
if result == "" {
return "", fmt.Errorf("no text content in Anthropic response")
}
return result, nil
})
}
// --- Shared HTTP execution ---
func (c *Client) doRequest(req *http.Request, parse func([]byte) (string, error)) (string, error) {
resp, err := c.http.Do(req)
if err != nil { if err != nil {
return "", fmt.Errorf("LLM request: %w", err) return "", fmt.Errorf("LLM request: %w", err)
} }
defer resp.Body.Close() defer resp.Body.Close()
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
body, _ := io.ReadAll(resp.Body)
return "", fmt.Errorf("LLM API error (status %d): %s", resp.StatusCode, string(body))
}
body, err := io.ReadAll(resp.Body) body, err := io.ReadAll(resp.Body)
if err != nil { if err != nil {
return "", fmt.Errorf("read response: %w", err) return "", fmt.Errorf("read response: %w", err)
} }
if resp.StatusCode < 200 || resp.StatusCode >= 300 { var chatResp ChatResponse
return "", fmt.Errorf("LLM API error (status %d): %s", resp.StatusCode, string(body)) if err := json.Unmarshal(body, &chatResp); err != nil {
return "", fmt.Errorf("parse response: %w", err)
} }
return parse(body) if len(chatResp.Choices) == 0 {
return "", fmt.Errorf("no choices in LLM response")
}
return chatResp.Choices[0].Message.Content, nil
} }
+5 -192
View File
@@ -1,12 +1,10 @@
package llm package llm
import ( import (
"context"
"encoding/json" "encoding/json"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"time"
) )
func TestComplete_Success(t *testing.T) { func TestComplete_Success(t *testing.T) {
@@ -53,7 +51,7 @@ func TestComplete_Success(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
got, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) got, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -70,7 +68,7 @@ func TestComplete_APIError(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for 429, got nil") t.Fatal("expected error for 429, got nil")
} }
@@ -84,7 +82,7 @@ func TestComplete_NoChoices(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for no choices, got nil") t.Fatal("expected error for no choices, got nil")
} }
@@ -97,7 +95,7 @@ func TestComplete_BadJSON(t *testing.T) {
defer server.Close() defer server.Close()
client := NewClient(server.URL, "test-key", "gpt-4") client := NewClient(server.URL, "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for bad JSON, got nil") t.Fatal("expected error for bad JSON, got nil")
} }
@@ -105,193 +103,8 @@ func TestComplete_BadJSON(t *testing.T) {
func TestComplete_ServerDown(t *testing.T) { func TestComplete_ServerDown(t *testing.T) {
client := NewClient("http://127.0.0.1:1", "test-key", "gpt-4") client := NewClient("http://127.0.0.1:1", "test-key", "gpt-4")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}}) _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err == nil { if err == nil {
t.Fatal("expected error for connection refused, got nil") t.Fatal("expected error for connection refused, got nil")
} }
} }
func TestWithTemperature(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
if client.temperature != 0 {
t.Errorf("expected initial temperature 0, got %f", client.temperature)
}
result := client.WithTemperature(0.7)
if result != client {
t.Error("WithTemperature should return the same client for chaining")
}
if client.temperature != 0.7 {
t.Errorf("expected temperature 0.7, got %f", client.temperature)
}
}
func TestComplete_TemperatureOmittedWhenZero(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
if _, exists := req["temperature"]; exists {
t.Error("temperature should be omitted when zero (server default)")
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model")
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestComplete_TemperatureIncludedWhenSet(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
temp, exists := req["temperature"]
if !exists {
t.Error("temperature should be included when set")
}
if temp != 0.7 {
t.Errorf("expected temperature 0.7, got %v", temp)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model").WithTemperature(0.7)
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestWithTimeout(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
result := client.WithTimeout(10 * time.Second)
if result != client {
t.Error("WithTimeout should return the same client for chaining")
}
// Verify timeout causes failure on slow server
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(200 * time.Millisecond)
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
}))
defer server.Close()
shortClient := NewClient(server.URL, "key", "model").WithTimeout(50 * time.Millisecond)
_, err := shortClient.Complete(context.Background(), []Message{{Role: "user", Content: "hi"}})
if err == nil {
t.Error("expected timeout error with 50ms timeout and 200ms server delay")
}
}
func TestComplete_Anthropic_Success(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/messages" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.Header.Get("x-api-key") != "test-key" {
t.Errorf("expected x-api-key header, got %q", r.Header.Get("x-api-key"))
}
if r.Header.Get("anthropic-version") != "2023-06-01" {
t.Errorf("expected anthropic-version header, got %q", r.Header.Get("anthropic-version"))
}
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
if req["system"] != "You are helpful" {
t.Errorf("expected system prompt, got %v", req["system"])
}
msgs := req["messages"].([]interface{})
if len(msgs) != 1 {
t.Errorf("expected 1 user message, got %d", len(msgs))
}
if req["max_tokens"] != float64(8192) {
t.Errorf("expected max_tokens 8192, got %v", req["max_tokens"])
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"content":[{"type":"text","text":"Hello from Claude!"}]}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-key", "claude-sonnet").WithProvider(ProviderAnthropic)
got, err := client.Complete(context.Background(), []Message{
{Role: "system", Content: "You are helpful"},
{Role: "user", Content: "Hi"},
})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != "Hello from Claude!" {
t.Errorf("expected %q, got %q", "Hello from Claude!", got)
}
}
func TestComplete_Anthropic_NoContent(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"content":[]}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-key", "claude-sonnet").WithProvider(ProviderAnthropic)
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err == nil {
t.Fatal("expected error for empty content, got nil")
}
}
func TestComplete_Anthropic_APIError(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(`{"error":{"message":"invalid request"}}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-key", "claude-sonnet").WithProvider(ProviderAnthropic)
_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
if err == nil {
t.Fatal("expected error for 400, got nil")
}
}
func TestWithProvider(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
if client.provider != ProviderOpenAI {
t.Errorf("expected default provider openai, got %s", client.provider)
}
result := client.WithProvider(ProviderAnthropic)
if result != client {
t.Error("WithProvider should return the same client for chaining")
}
if client.provider != ProviderAnthropic {
t.Errorf("expected provider anthropic, got %s", client.provider)
}
}
-7
View File
@@ -9,11 +9,6 @@ import (
func FormatMarkdown(result *ReviewResult, reviewerName string) string { func FormatMarkdown(result *ReviewResult, reviewerName string) string {
var sb strings.Builder var sb strings.Builder
if reviewerName != "" {
title := strings.ToUpper(reviewerName[:1]) + reviewerName[1:]
sb.WriteString(fmt.Sprintf("# %s Review\n\n", title))
}
sb.WriteString("## Summary\n\n") sb.WriteString("## Summary\n\n")
sb.WriteString(result.Summary) sb.WriteString(result.Summary)
sb.WriteString("\n\n") sb.WriteString("\n\n")
@@ -35,8 +30,6 @@ func FormatMarkdown(result *ReviewResult, reviewerName string) string {
if reviewerName != "" { if reviewerName != "" {
sb.WriteString(fmt.Sprintf("\n---\n*Review by %s*\n", reviewerName)) sb.WriteString(fmt.Sprintf("\n---\n*Review by %s*\n", reviewerName))
// Hidden sentinel for identifying this bot's reviews during cleanup
sb.WriteString(fmt.Sprintf("\n<!-- review-bot:%s -->\n", reviewerName))
} }
return sb.String() return sb.String()
-43
View File
@@ -116,46 +116,3 @@ func TestGiteaEvent(t *testing.T) {
} }
} }
} }
func TestFormatMarkdown_Sentinel(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "All good.",
Recommendation: "Merge it.",
}
output := FormatMarkdown(result, "security")
if !strings.Contains(output, "<!-- review-bot:security -->") {
t.Error("expected sentinel comment in output")
}
// Empty reviewer name should NOT have sentinel
output2 := FormatMarkdown(result, "")
if strings.Contains(output2, "<!-- review-bot") {
t.Error("should not contain sentinel when reviewer name is empty")
}
}
func TestFormatMarkdown_RoleTitle(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "All good.",
Recommendation: "Merge it.",
}
// With reviewer name: should have title header
output := FormatMarkdown(result, "security")
if !strings.Contains(output, "# Security Review\n") {
t.Error("expected '# Security Review' header when reviewer name is set")
}
output2 := FormatMarkdown(result, "gpt")
if !strings.Contains(output2, "# Gpt Review\n") {
t.Error("expected '# Gpt Review' header")
}
// Without reviewer name: no title header
output3 := FormatMarkdown(result, "")
if strings.Contains(output3, "# ") && strings.Contains(output3, " Review\n") {
t.Error("should not contain role title header when reviewer name is empty")
}
}
+6 -44
View File
@@ -1,5 +1,3 @@
// Package review builds prompts for AI code review and parses LLM responses
// into structured review results.
package review package review
import ( import (
@@ -7,17 +5,11 @@ import (
"strings" "strings"
) )
// BuildSystemBase returns the core system prompt instructions without // BuildSystemPrompt constructs the system prompt for the LLM reviewer.
// patterns or conventions. Used by the budget package to separate func BuildSystemPrompt(conventions string) string {
// trimmable from non-trimmable content.
func BuildSystemBase() string {
var sb strings.Builder var sb strings.Builder
sb.WriteString("You are an expert code reviewer. Review the provided pull request diff carefully.\n\n") sb.WriteString("You are an expert code reviewer. Review the provided pull request diff carefully.\n\n")
sb.WriteString("CONTEXT:\n")
sb.WriteString("- You will receive the full content of modified files for reference, followed by the diff showing what changed.\n")
sb.WriteString("- The diff shows ONLY what was added/removed. The full file content provides complete context.\n")
sb.WriteString("- Focus your review on the CHANGES (the diff), using the full files for context.\n\n")
sb.WriteString("Your task:\n") sb.WriteString("Your task:\n")
sb.WriteString("1. Review the diff for correctness, idiomatic code, potential bugs, and design issues.\n") sb.WriteString("1. Review the diff for correctness, idiomatic code, potential bugs, and design issues.\n")
sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n") sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n")
@@ -44,29 +36,15 @@ func BuildSystemBase() string {
sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n") sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n")
sb.WriteString("- If the diff is empty or trivial (only formatting/whitespace), APPROVE with no findings.\n") sb.WriteString("- If the diff is empty or trivial (only formatting/whitespace), APPROVE with no findings.\n")
return sb.String()
}
// BuildSystemPrompt constructs the full system prompt with patterns and conventions.
// Deprecated: Use BuildSystemBase with budget.Fit for context-aware assembly.
func BuildSystemPrompt(conventions, patterns string) string {
var sb strings.Builder
sb.WriteString(BuildSystemBase())
if patterns != "" {
sb.WriteString(fmt.Sprintf("\n\n## Language Patterns & Idioms\n\nUse the following patterns as review criteria. Code that violates these established patterns is a finding:\n\n%s\n", patterns))
}
if conventions != "" { if conventions != "" {
sb.WriteString(fmt.Sprintf("\n\n## Repository Conventions\n\nThe repository has the following coding conventions that must be respected:\n\n%s\n", conventions)) sb.WriteString(fmt.Sprintf("\n\nThe repository has the following coding conventions that should be respected:\n\n%s\n", conventions))
} }
return sb.String() return sb.String()
} }
// BuildUserMeta returns the PR metadata header (title, description, CI status) // BuildUserPrompt constructs the user message with PR context.
// without the diff or file context. Used by the budget package. func BuildUserPrompt(title, description, diff string, ciPassed bool, ciDetails string) string {
func BuildUserMeta(title, description string, ciPassed bool, ciDetails string) string {
var sb strings.Builder var sb strings.Builder
sb.WriteString(fmt.Sprintf("## Pull Request: %s\n\n", title)) sb.WriteString(fmt.Sprintf("## Pull Request: %s\n\n", title))
@@ -85,23 +63,7 @@ func BuildUserMeta(title, description string, ciPassed bool, ciDetails string) s
sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails)) sb.WriteString(fmt.Sprintf("CI Details: %s\n", ciDetails))
} }
return sb.String() sb.WriteString("\n### Diff\n\n")
}
// BuildUserPrompt constructs the user message with PR context.
// Deprecated: Use BuildUserMeta with budget.Fit for context-aware assembly.
func BuildUserPrompt(title, description, diff, fileContext string, ciPassed bool, ciDetails string) string {
var sb strings.Builder
sb.WriteString(BuildUserMeta(title, description, ciPassed, ciDetails))
if fileContext != "" {
sb.WriteString("\n### Full File Context (modified files)\n\n")
sb.WriteString(fileContext)
sb.WriteString("\n")
}
sb.WriteString("\n### Diff (changes to review)\n\n")
sb.WriteString("```diff\n") sb.WriteString("```diff\n")
sb.WriteString(diff) sb.WriteString(diff)
sb.WriteString("\n```\n") sb.WriteString("\n```\n")
+6 -87
View File
@@ -6,7 +6,7 @@ import (
) )
func TestBuildSystemPrompt_NoConventions(t *testing.T) { func TestBuildSystemPrompt_NoConventions(t *testing.T) {
prompt := BuildSystemPrompt("", "") prompt := BuildSystemPrompt("")
if !strings.Contains(prompt, "expert code reviewer") { if !strings.Contains(prompt, "expert code reviewer") {
t.Error("expected system prompt to mention code reviewer role") t.Error("expected system prompt to mention code reviewer role")
@@ -18,7 +18,7 @@ func TestBuildSystemPrompt_NoConventions(t *testing.T) {
func TestBuildSystemPrompt_WithConventions(t *testing.T) { func TestBuildSystemPrompt_WithConventions(t *testing.T) {
conventions := "- Use stdlib only\n- No panics\n" conventions := "- Use stdlib only\n- No panics\n"
prompt := BuildSystemPrompt(conventions, "") prompt := BuildSystemPrompt(conventions)
if !strings.Contains(prompt, "coding conventions") { if !strings.Contains(prompt, "coding conventions") {
t.Error("expected conventions section") t.Error("expected conventions section")
@@ -29,7 +29,7 @@ func TestBuildSystemPrompt_WithConventions(t *testing.T) {
} }
func TestBuildUserPrompt_Basic(t *testing.T) { func TestBuildUserPrompt_Basic(t *testing.T) {
prompt := BuildUserPrompt("Fix bug", "Fixes the crash", "diff content here", "", true, "all checks passed") prompt := BuildUserPrompt("Fix bug", "Fixes the crash", "diff content here", true, "all checks passed")
if !strings.Contains(prompt, "Fix bug") { if !strings.Contains(prompt, "Fix bug") {
t.Error("expected PR title") t.Error("expected PR title")
@@ -46,7 +46,7 @@ func TestBuildUserPrompt_Basic(t *testing.T) {
} }
func TestBuildUserPrompt_CIFailed(t *testing.T) { func TestBuildUserPrompt_CIFailed(t *testing.T) {
prompt := BuildUserPrompt("Add tests", "", "some diff", "", false, "lint: failed") prompt := BuildUserPrompt("Add tests", "", "some diff", false, "lint: failed")
if !strings.Contains(prompt, "FAILED") { if !strings.Contains(prompt, "FAILED") {
t.Error("expected CI status FAILED") t.Error("expected CI status FAILED")
@@ -57,7 +57,7 @@ func TestBuildUserPrompt_CIFailed(t *testing.T) {
} }
func TestBuildUserPrompt_NoDescription(t *testing.T) { func TestBuildUserPrompt_NoDescription(t *testing.T) {
prompt := BuildUserPrompt("Quick fix", "", "diff", "", true, "") prompt := BuildUserPrompt("Quick fix", "", "diff", true, "")
if strings.Contains(prompt, "### Description") { if strings.Contains(prompt, "### Description") {
t.Error("should not contain Description header when body is empty") t.Error("should not contain Description header when body is empty")
@@ -66,7 +66,7 @@ func TestBuildUserPrompt_NoDescription(t *testing.T) {
func TestBuildUserPrompt_DiffIncluded(t *testing.T) { func TestBuildUserPrompt_DiffIncluded(t *testing.T) {
diff := "+func Hello() string {\n+\treturn \"hello\"\n+}" diff := "+func Hello() string {\n+\treturn \"hello\"\n+}"
prompt := BuildUserPrompt("Greeting", "Add greeting func", diff, "", true, "") prompt := BuildUserPrompt("Greeting", "Add greeting func", diff, true, "")
if !strings.Contains(prompt, "```diff") { if !strings.Contains(prompt, "```diff") {
t.Error("expected diff fence") t.Error("expected diff fence")
@@ -75,84 +75,3 @@ func TestBuildUserPrompt_DiffIncluded(t *testing.T) {
t.Error("expected diff content in prompt") t.Error("expected diff content in prompt")
} }
} }
func TestBuildSystemPrompt_WithPatterns(t *testing.T) {
patterns := "## Naming: use snake_case for functions"
prompt := BuildSystemPrompt("", patterns)
if !strings.Contains(prompt, "Language Patterns") {
t.Error("expected patterns section header")
}
if !strings.Contains(prompt, "snake_case") {
t.Error("expected patterns content")
}
}
func TestBuildSystemPrompt_WithBoth(t *testing.T) {
conventions := "Run mix format before commit"
patterns := "Use pipe operator for transformations"
prompt := BuildSystemPrompt(conventions, patterns)
if !strings.Contains(prompt, "Repository Conventions") {
t.Error("expected conventions section")
}
if !strings.Contains(prompt, "Language Patterns") {
t.Error("expected patterns section")
}
}
func TestBuildUserPrompt_WithFileContext(t *testing.T) {
fileContext := "--- main.go ---\npackage main\n"
prompt := BuildUserPrompt("Fix", "desc", "diff here", fileContext, true, "")
if !strings.Contains(prompt, "Full File Context") {
t.Error("expected file context section")
}
if !strings.Contains(prompt, "package main") {
t.Error("expected file content in prompt")
}
}
func TestBuildUserPrompt_WithoutFileContext(t *testing.T) {
prompt := BuildUserPrompt("Fix", "desc", "diff here", "", true, "")
if strings.Contains(prompt, "Full File Context") {
t.Error("should not include file context section when empty")
}
}
func TestBuildSystemBase(t *testing.T) {
result := BuildSystemBase()
if result == "" {
t.Fatal("BuildSystemBase returned empty string")
}
if !strings.Contains(result, "expert code reviewer") {
t.Error("expected reviewer role in system base")
}
if !strings.Contains(result, "REQUEST_CHANGES") {
t.Error("expected verdict format in system base")
}
if !strings.Contains(result, "JSON") {
t.Error("expected JSON output instruction in system base")
}
}
func TestBuildUserMeta(t *testing.T) {
result := BuildUserMeta("Fix bug", "Some description", true, "all checks passed")
if !strings.Contains(result, "Fix bug") {
t.Error("expected title in user meta")
}
if !strings.Contains(result, "Some description") {
t.Error("expected description in user meta")
}
if !strings.Contains(result, "PASSED") {
t.Error("expected CI PASSED status")
}
}
func TestBuildUserMeta_CIFailed(t *testing.T) {
result := BuildUserMeta("Title", "", false, "test job failed")
if !strings.Contains(result, "FAILED") {
t.Error("expected CI FAILED status")
}
if strings.Contains(result, "Description") {
t.Error("expected no description section when empty")
}
}